Skip to content

fix(security): strip fake BEGIN UNTRUSTED markers from page content - #30

Merged
RapierCraft merged 1 commit into
mainfrom
fix/strip-begin-untrusted-markers-25
Jun 22, 2026
Merged

fix(security): strip fake BEGIN UNTRUSTED markers from page content#30
RapierCraft merged 1 commit into
mainfrom
fix/strip-begin-untrusted-markers-25

Conversation

@RapierCraft

Copy link
Copy Markdown
Owner

Summary

Adds symmetric stripping of [BEGIN UNTRUSTED PAGE CONTENT nonce= patterns from page content inside wrapUntrustedPageContent(), matching the existing [END UNTRUSTED nonce=] strip.

Without this, an attacker-controlled page could inject a fake opening marker into the content body to confuse downstream parsers into treating attacker-controlled content as a trusted wrapper frame.

Changes

  • src/index.ts: Chain .replace(/\[BEGIN UNTRUSTED PAGE CONTENT nonce=/g, "[BEGIN_UNTRUSTED_PAGE_CONTENT_nonce=") before the existing END strip in wrapUntrustedPageContent()
  • src/index.ts: Update JSDoc comment to document both BEGIN and END marker stripping

Testing

  • Content containing [BEGIN UNTRUSTED PAGE CONTENT nonce=xxx is neutralized to [BEGIN_UNTRUSTED_PAGE_CONTENT_nonce=xxx in output [type:unit]
  • Content containing [END UNTRUSTED nonce=xxx is neutralized to [END_UNTRUSTED_nonce=xxx in output [type:unit]
  • Real outer markers (added by the function itself) are unaffected [type:unit]
  • npm run build passes [type:unit]

Closes #25
Implementation branch: fix/strip-begin-untrusted-markers-25
Base: main

)

Add a second replace in wrapUntrustedPageContent() to neutralize
[BEGIN UNTRUSTED PAGE CONTENT nonce=] patterns injected by
attacker-controlled pages, symmetric with the existing [END UNTRUSTED
nonce=] strip. Without this, a malicious page could inject a fake
opening marker to confuse downstream parsers into treating
attacker-controlled content as a trusted wrapper frame.

Also update JSDoc to document both strips.

@RapierCraft RapierCraft left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

APPROVED: commit 10afaf2 after context-aware security review (1 agent: General Security). 0 findings.

Security Review

Reviewed: src/index.tswrapUntrustedPageContent()

Fix verified correct:

  • The new .replace(/\[BEGIN UNTRUSTED PAGE CONTENT nonce=/g, "[BEGIN_UNTRUSTED_PAGE_CONTENT_nonce=") (lines 76-79) correctly neutralizes fake opening markers in attacker-controlled content
  • The existing .replace(/\[END UNTRUSTED nonce=/g, "[END_UNTRUSTED_nonce=") (line 80) retains the END strip
  • Crucially: both replaces run on body (the input content) BEFORE the real markers are joined in at lines 82-84. The real markers use a runtime-generated ${nonce} value and are never affected by the strip patterns
  • No injection risk, no secrets, no new env vars, no infrastructure changes
  • Prettier reformatting in other parts of the file is cosmetic — no logic changes

Automated checks: TypeScript build passes, Prettier formatting passes, no secrets detected.

Safe to merge.

@RapierCraft

Copy link
Copy Markdown
Owner Author

APPROVED: commit 10afaf2 after context-aware security review (1 agent: General Security). 0 findings. Safe to merge.

@RapierCraft
RapierCraft merged commit a71577a into main Jun 22, 2026
1 check passed
@RapierCraft

Copy link
Copy Markdown
Owner Author

PR Review Summary: #30 - fix(security): strip fake BEGIN UNTRUSTED markers from page content

Review Integrity

Reviewed commit: 10afaf2 | Current HEAD: 10afaf2 | Status: CURRENT

Verdict: APPROVED

Context-Aware Review

Domains: Security (UNTRUSTED_INPUT_PROCESSING) | Agents: 1 (General Security inline review)

Integration Checks (Phase 2.5)

Code registration: pass — single-function change, all 8+ call sites in same file, no registration needed
SOPS deploy chain: N/A — no env var or infra changes
Purpose Regression Gate (7A): N/A — non-milestone PR

Risk Matrix

Category Risk Blocking? Confidence
Fake marker injection FIXED by this PR No CONFIRMED
Real marker corruption None — replaces run on input before real markers join No CONFIRMED

Findings

No findings — review is clean.

Automated Checks

Check Result
TypeScript build (npm run build) PASS
Prettier format check PASS
Secrets scan PASS — no secrets
New env vars N/A — only pre-existing COMET_DISABLE_UNTRUSTED_MARKERS (reformatted)

Recommendation

Merge approved. The fix correctly adds symmetric BEGIN-marker stripping to match the existing END-marker strip. Implementation is correct — strips run on input body before real nonce-bearing markers are added.


Context-aware review complete. 1 inline security review. 0 findings triaged. PR merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(security): strip fake BEGIN UNTRUSTED markers from page content

1 participant