Skip to content

fix: fill empty recommendation for ROOT_DETECTION_PRESENT - #343

Merged
ohachimOs merged 2 commits into
masterfrom
documentation/shielding-secure-recommendations
Aug 5, 2026
Merged

fix: fill empty recommendation for ROOT_DETECTION_PRESENT#343
ohachimOs merged 2 commits into
masterfrom
documentation/shielding-secure-recommendations

Conversation

@ohachimOs

@ohachimOs ohachimOs commented Aug 4, 2026

Copy link
Copy Markdown
Member

ROOT_DETECTION_PRESENT/recommendation.md was the only empty recommendation file in the KB, so the Recommendation section renders blank in the report. Fills it with the same one-liner every other SECURE entry uses.

Also fixes four meta.json references pointing at MASWE ids that don't exist. MASWE-0097 isn't in the catalog at all (MASVS-RESILIENCE ends at 0065) and MASWE-0047 is a MASVS-CODE weakness, not the pinning one. Root detection now points at MASWE-0051, pinning at MASWE-0028, both verified 200.

@ohachimOs
ohachimOs requested a review from a team August 4, 2026 17:53
Comment thread MOBILE_CLIENT/COMMON/_LOW/WEAK_ROOT_DETECTION/meta.json

@ostorlab-ai-pr-review ostorlab-ai-pr-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Results

I found 3 issues across metadata files in MOBILE_CLIENT/COMMON, all relating to incorrect OWASP MASWE references. Two are confirmed incorrect mappings (critical), and one needs verification before merging.

Key findings:

  1. ROOT_DETECTION_MISSING/meta.json (line 8) — MASWE-0097 was replaced with MASWE-0051, but MASWE-0051 is "Unprotected Open Ports", not "Root/Jailbreak Detection Not Implemented". The label and linked content no longer match.
  2. ROOT_DETECTION_PRESENT/meta.json (line 12) — Same incorrect swap: MASWE-0051 does not describe root/jailbreak detection. The reference must be reverted to MASWE-0097.
  3. SSL_TLS_PINNING_NOT_DETECTED/meta.json (line 8) — MASWE-0028 must be verified by inspecting the destination page content (not just an HTTP 200) to confirm it actually describes certificate/identity pinning.

Root cause: Replacement URLs were validated only by HTTP 200 status code, which OWASP also returns for redirect pages. The destination page content was never verified, allowing semantically incorrect links to pass automated checks.

Recommendation: Revert the two confirmed files back to MASWE-0097, verify MASWE-0028 by content inspection, and add content-based validation to CI for all MASWE URL references.

Comment thread MOBILE_CLIENT/COMMON/_HARDENING/ROOT_DETECTION_MISSING/meta.json
Comment thread MOBILE_CLIENT/COMMON/_SECURE/ROOT_DETECTION_PRESENT/meta.json
@ostorlab-ai-pr-review

Copy link
Copy Markdown

Reviewer Scores for Ostorlab/KB #343

Each reviewer who left comments is scored between -10 and +10 based on the overall quality of their review feedback. -10 is harmless, not useful, or wastes time; +10 finds an actual, useful bug.

Total reviewers scored: 1
Useful (score > 0): 1
Neutral (score = 0): 0
Not useful (score < 0): 0
Average score: 8.0

Reviewer Score Reasoning
@m0hamed-ait 8 The reviewer checked the actual OWASP MASWE catalog and caught that the PR's replacement reference (MASWE-0051) points to 'Unprotected Open Ports' rather than the correct root/jailbreak detection weakness (MASWE-0097), identifying a real correctness bug that invalidates part of the PR.

@ohachimOs
ohachimOs merged commit 496c8c4 into master Aug 5, 2026
6 checks passed
@ohachimOs
ohachimOs deleted the documentation/shielding-secure-recommendations branch August 5, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants