fix(report): stop over-escaping the CSP meta-tag content across all HTML reports - #1230
Closed
seonghobae wants to merge 1 commit into
Closed
seonghobae wants to merge 1 commit into
seonghobae wants to merge 1 commit into
Conversation
…TML reports Every standalone HTML report generator (python/fast_mlsirm/report.py, the essay report/calibration/validation HTML family, and the benchmark, buyer-packet, commercial-release, Figma-evidence-sync, PR-queue-governance, procurement-due-diligence, and release-evidence-index builder scripts) embedded its Content-Security-Policy through `escape(_content_security_policy(), quote=True)` inside the meta tag's `content` attribute. html.escape converts the CSP's literal `'none'` / `'unsafe-inline'` source expressions to `'none'`, which browsers do not parse as valid CSP syntax, so the meta-delivered defense-in-depth policy was silently disabled on every emitted report while the tests asserted the broken, escaped string as the expected output. Found by Strix (CWE-693, CVSS 6.5) while scanning an unrelated in-progress PR (#1081) that introduced a new copy of the same pattern; the bug already existed in the 11 files above on main. Each `_content_security_policy()` implementation returns a fixed, non-user-controlled string, so removing the escape call is safe. Updated the four tests that pinned the escaped form to assert the literal CSP directive and added a negative assertion against re-escaping. Verified: 107 passed across the affected report/CSP test files; scripts/render_changelog_fragments.py --check passes with the new fragment.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change removes quote escaping from CSP meta-tag rendering across report generators and builder scripts. Tests now require literal CSP directives. Changelog files document the CSP correction and additional release entries. ChangesCSP escaping correction
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The PR correctly changes CSP meta-tag rendering across the HTML reports, but the changelog wording about escaped quotes disabling CSP may be technically inaccurate. The change is otherwise mergeable with owner follow-up to revise that claim or provide browser-level confirmation. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
added a commit
that referenced
this pull request
Aug 22, 2026
…ine report Strix flagged this exact pattern on this PR (CWE-693, CVSS 6.5): escape(_CSP, quote=True) converts the CSP's literal 'none' source expression to 'none', which browsers do not parse as valid CSP syntax, silently disabling the meta-delivered policy. _CSP is a fixed module constant with no user-controlled content, so interpolating it directly is safe. This mirrors the same fix already applied to every other report generator on main in #1230. Added a regression assertion that the CSP is embedded unescaped. Verified: pytest tests/test_cross_engine_conformance_report.py -- 8 passed.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
Author
|
RCA correction: this PR is based on a false-positive browser-parsing assumption and should not land as a security fix. HTML character references in attribute source markup are decoded by the HTML parser before the DOM/content attribute is consumed. In particular, source text such as The new tests assert a serialization spelling ( Closing unmerged. If we add CSP acceptance evidence later, it should verify the parsed DOM/policy semantics (or a browser-level enforcement fixture) rather than require one source-serialization spelling. |
seonghobae
added a commit
that referenced
this pull request
Aug 24, 2026
) * test(validation): define cross-engine conformance inventory contract * feat(validation): add cross-engine conformance inventory * docs(changelog): record cross-engine conformance inventory * test(validation): cover conformance contract fail-closed edges * fix(validation): seal conformance package records * test(validation): seal conformance record subclasses * fix(validation): complete conformance commit provenance boundary * fix(validation): require executed conformance evidence * test(validation): reproduce unsupported conformance coverage claim * test(validation): pin execution-backed coverage invariant * test(validation): reproduce Unicode conformance identity drift * fix(validation): normalize conformance text before hashing * test(validation): make schema-version regex literal * test(conformance): expose post-init mutation replay gap * fix(conformance): revalidate records on manifest replay * docs(conformance): record replay integrity hardening * test(conformance): document replay fixtures * fix(changelog): restore authoritative fragment format * feat(validation): add conformance run provenance (#1085) * feat(validation): add conformance run provenance * fix(changelog): restore fragment headings * test(validation): reproduce run provenance replay bypass * fix(validation): replay run provenance before serialization * docs(validation): record provenance replay integrity * fix(validation): bind executed conformance to run output provenance (#1093) * test(conformance): require output provenance for executed evidence * fix(conformance): bind executed evidence to run outputs * test(conformance): align fixtures with executed provenance contract * docs(changelog): record executed conformance provenance gate * test(conformance): seal mutated output provenance before execution gate * test(conformance): provide provenance for executed mutation fixture * Revert "test(conformance): provide provenance for executed mutation fixture" This reverts commit b94cf2e. * test(conformance): provide legacy provenance fixture * feat(validation): complete conformance runtime provenance (#1095) * test(conformance): require runtime and redistribution provenance * feat(conformance): bind runtime and redistribution provenance * test(conformance): align inventory fixtures with runtime provenance * test(conformance): carry runtime identities in provenance replay fixture * test(conformance): carry runtime identities in execution fixture * docs(changelog): record runtime provenance contract * test(conformance): cover malformed runtime provenance controls * test(conformance): provide provenance for executed mutation fixture * feat(validation): strictly replay persisted conformance manifests (#1097) * test(conformance): require strict persisted manifest replay * feat(conformance): strictly replay persisted manifests * docs(changelog): record strict conformance manifest replay * test(conformance): cover nested manifest replay boundaries * test(conformance): require stable JSON resource failures * fix(conformance): stabilize bounded JSON replay failures * fix(conformance): bound parsed manifest nesting * test(conformance): provide provenance for executed mutation fixture * feat(validation): render accessible cross-engine conformance evidence (#1164) * test(validation): require accessible conformance evidence report * feat(validation): render accessible conformance evidence * docs(changelog): record conformance evidence report * test(validation): require downloadable long-form conformance rows * feat(validation): export long-form conformance evidence rows * docs(changelog): record long-form conformance export * fix(report): stop over-escaping the CSP meta-tag content in cross-engine report Strix flagged this exact pattern on this PR (CWE-693, CVSS 6.5): escape(_CSP, quote=True) converts the CSP's literal 'none' source expression to 'none', which browsers do not parse as valid CSP syntax, silently disabling the meta-delivered policy. _CSP is a fixed module constant with no user-controlled content, so interpolating it directly is safe. This mirrors the same fix already applied to every other report generator on main in #1230. Added a regression assertion that the CSP is embedded unescaped. Verified: pytest tests/test_cross_engine_conformance_report.py -- 8 passed. * Revert "fix(report): stop over-escaping the CSP meta-tag content in cross-engine report" This reverts commit 051bc1b.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
Strix found a MEDIUM (CVSS 6.5, CWE-693) finding while scanning unrelated,
in-progress PR #1081 (
feat/cross-engine-conformance-inventory-1077): thenew report generator it added embeds its Content-Security-Policy through
escape(_CSP, quote=True)inside the meta tag'scontentattribute.html.escapeconverts the CSP's literal'none'source expressions to'none', which browsers do not parse as valid CSP syntax — so themeta-delivered defense-in-depth policy silently does nothing.
Auditing showed the exact same
escape(_content_security_policy(), quote=True)pattern already exists on
main, in every standalone HTML report thisproject emits:
python/fast_mlsirm/report.pypython/fast_mlsirm/scoring/essay/report_html.py,calibration_report_html.py,validation_report_html.pyscripts/build_benchmark_report.pyscripts/build_buyer_packet.pyscripts/build_commercial_release.pyscripts/build_figma_evidence_sync.pyscripts/build_pr_queue_governance.pyscripts/build_procurement_due_diligence.pyscripts/build_release_evidence_index.pyFour of the corresponding tests (
test_report.py,test_scoring_essay_report_html.py,test_scoring_essay_validation_report_html.py,test_scoring_essay_facets_report_html.py) actively pinned the broken,escaped string (
assert "default-src 'none'" in html) as theexpected output, so the regression was invisible to the suite.
Fix
Each
_content_security_policy()implementation returns a fixed,non-user-controlled string literal — verified individually for all 9
definitions before changing anything. Removed the
escape(..., quote=True)wrapper at all 11 call sites so the CSP is embedded verbatim.
escapestaysimported and used elsewhere in every touched file (titles, table cells,
disclaimers), so no unused-import fallout.
Updated the 4 tests that pinned the escaped form to assert the literal
default-src 'none'directive and added a negative assertion(
"'" not in <csp-slice>) so the policy cannot silently regress backinto an escaped, inert state.
Scope boundary
Pure-Python report-rendering fix. No change to CLI behavior, data models,
Rust core, or any other report content — only the CSP meta-tag value.
Verification
pytest tests/test_report.py tests/test_scoring_essay_report_html.py tests/test_scoring_essay_validation_report_html.py tests/test_scoring_essay_facets_report_html.py tests/test_benchmark_report.py tests/test_buyer_evidence_packet.py tests/test_commercial_release_builder.py tests/test_figma_evidence_sync.py tests/test_pr_queue_governance.py tests/test_procurement_due_diligence.py tests/test_release_evidence_index.py— 107 passed.
python scripts/render_changelog_fragments.py --check CHANGELOG.mdpasseswith the new
docs/changelog.d/csp-meta-content-escaping.mdfragment.pytest tests/test_changelog_fragment_contract.py— 8 passed.Summary by CodeRabbit
Bug Fixes
Tests