security(git-id-switcher): SanitizedHtml branded type for buildDocumentHtml content#455
Conversation
…uildDocumentHtml content
Promote the `content` parameter of `buildDocumentHtml` from a plain
`string` (contract-by-docstring) to a `SanitizedHtml` branded string type
so that the trust boundary between the markdown sanitizer and the webview
HTML shell is enforced at compile time. Previously, a future caller could
pass an unsanitized `string` and the only remaining defence was CSP —
which does not block `<img onerror>` or `javascript:` URLs outside the
link interceptor path.
Changes:
- htmlTemplates.ts: export `SanitizedHtml = string & { readonly __brand }`
and retype `buildDocumentHtml(content: SanitizedHtml)`.
- documentationInternal.ts: `renderMarkdown` now returns `SanitizedHtml`,
with a single sanctioned `as SanitizedHtml` cast at its return site
(the sole origin of the brand in production code). Type-only import
from htmlTemplates to avoid promoting the existing one-way runtime
import edge into a circular dependency.
- webview.ts: `getDocumentHtml` wrapper signature upgraded and type
re-exported so downstream consumers inherit the constraint.
- htmlTemplates.test.ts: isolated, documented `asSanitizedHtml` test
helper for structural tests that exercise the shell rather than the
sanitizer (renderMarkdown has its own dedicated coverage in
documentation.test.ts).
Security review (delegated, security-engineer persona): no Critical or
Medium findings. Single Low finding (circular-import fragility) folded
in as an inline comment on the type-only import.
tsc, eslint (--max-warnings=0), and test:coverage all pass;
htmlTemplates.ts and documentationInternal.ts retain 100% statement
coverage.
Signed-off-by: Null;Variant <null@nullvariant.com>
🖥️ IDE: [VS Code](https://code.visualstudio.com/)
🔌 Extension: [Claude Code](https://claude.ai/download)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Model-Raw: claude-opus-4-6
🐰 Mimi's Validation Report ✅All checks are looking good! Great job! 🎉 ⏳ Some checks are still running. I will keep watching!
This report was carefully prepared by nullvariant-mimi[bot] |
🦥 Slow's Code Review 😩...yawn... Do I really have to review this?
| Split it up... reading long files is exhausting.
This review was reluctantly filed by nullvariant-slow[bot] |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
🕊️ Ciel's Mediation 🌤️*~~ floating down from the clouds ~~ The zoo seems a bit noisy today...* 2 zoo members have reviewed this PR.
⚖️ The zoo has mixed opinions. Some are concerned, some are fine with it. Please review each comment carefully and make the final call.
This mediation was peacefully delivered by nullvariant-ciel[bot] |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![nullvariant-justice[bot]](https://avatars.githubusercontent.com/in/2610732?s=60&v=4){kind=small}
1 participant
Summary
buildDocumentHtml'scontentparameter from a plainstring(contract-by-docstring) to aSanitizedHtmlbranded string type, enforcing the trust boundary between the markdown sanitizer and the webview HTML shell at compile time.renderMarkdownbecomes the single sanctioned origin of the brand (oneas SanitizedHtmlcast at its return site); all production call sites indocumentationPublic.tsalready flow through it.getDocumentHtmlwrapper signature upgraded and type re-exported fromwebview.tsso downstream consumers inherit the constraint.Rationale
CSP (
script-src 'nonce-…') blocks<script>execution but does not stop<img onerror>orjavascript:URLs outside the link interceptor path. With the previousstringtype, a future caller could forget the sanitizer and the compiler would stay silent — the brand makes that class of regression a type error.Notes
import typeis used fromdocumentationInternal.tstohtmlTemplates.tsto avoid promoting the existing one-way runtime edge (htmlTemplates→escapeHtmlEntities) into a real circular dependency. Rationale is commented inline on the import.asSanitizedHtmlhelper lets structural tests forbuildDocumentHtml(shell/nav/escaping) bypass the sanitizer; the sanitizer itself has its own dedicated coverage indocumentation.test.ts.Test plan
npx tsc --noEmitpassesnpm run lintpasses with--max-warnings=0npm run test:coveragepasses;htmlTemplates.tsanddocumentationInternal.tsretain 100% statement coverageas SanitizedHtmlcasts exist in production code