Skip to content

Commit 0fee85f

Browse files
nullvariantclaude
andauthored
security(git-id-switcher): introduce SanitizedHtml branded type for buildDocumentHtml content (#455)
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. 🖥️ IDE: [VS Code](https://code.visualstudio.com/) 🔌 Extension: [Claude Code](https://claude.ai/download) Model-Raw: claude-opus-4-6 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2adbb54 commit 0fee85f

4 files changed

Lines changed: 64 additions & 22 deletions

File tree

extensions/git-id-switcher/src/test/htmlTemplates.test.ts

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import * as assert from 'node:assert';
3636
import {
37+
type SanitizedHtml,
3738
buildCspString,
3839
CspValidationError,
3940
getBaseStyles,
@@ -42,6 +43,16 @@ import {
4243
buildErrorHtml,
4344
} from '../ui/htmlTemplates';
4445

46+
/**
47+
* Test-only brand lift. Production code MUST obtain `SanitizedHtml` exclusively
48+
* through `renderMarkdown` (see documentationInternal.ts), which is the single
49+
* sanctioned origin of the brand. Tests bypass the sanitizer because they
50+
* exercise `buildDocumentHtml`'s structural contract (shell, nav, escaping),
51+
* not the sanitizer's correctness — `renderMarkdown` has its own dedicated
52+
* test suite in documentation.test.ts.
53+
*/
54+
const asSanitizedHtml = (s: string): SanitizedHtml => s as SanitizedHtml;
55+
4556
// ============================================================================
4657
// Test Constants
4758
// ============================================================================
@@ -342,7 +353,7 @@ function testAllTemplatesCssQuality(): void {
342353

343354
const templates: ReadonlyArray<readonly [string, () => string]> = [
344355
['document', (): string => buildDocumentHtml(
345-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/README.md', TEST_NONCE, false
356+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/README.md', TEST_NONCE, false
346357
)],
347358
['loading', (): string => buildLoadingHtml(TEST_CSP_SOURCE, TEST_NONCE)],
348359
['error', (): string => buildErrorHtml(TEST_CSP_SOURCE, 'network', TEST_NONCE)],
@@ -362,7 +373,7 @@ function testAllTemplatesCssQuality(): void {
362373

363374
// Document template uses both tokens explicitly.
364375
const docHtml = buildDocumentHtml(
365-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/README.md', TEST_NONCE, false
376+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/README.md', TEST_NONCE, false
366377
);
367378
assert.ok(
368379
docHtml.includes('border-radius: var(--gis-radius-sm)'),
@@ -443,7 +454,7 @@ function testShellSkeletonIsShared(): void {
443454

444455
const docShell = extractShell(
445456
buildDocumentHtml(
446-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/README.md', TEST_NONCE, false
457+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/README.md', TEST_NONCE, false
447458
)
448459
);
449460
const loadingShell = extractShell(buildLoadingHtml(TEST_CSP_SOURCE, TEST_NONCE));
@@ -490,7 +501,7 @@ function testBodyClassOverrides(): void {
490501

491502
const cases: ReadonlyArray<readonly [string, string, () => string]> = [
492503
['document', 'gis-doc', (): string => buildDocumentHtml(
493-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/README.md', TEST_NONCE, false
504+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/README.md', TEST_NONCE, false
494505
)],
495506
['loading', 'gis-loading', (): string => buildLoadingHtml(TEST_CSP_SOURCE, TEST_NONCE)],
496507
['error', 'gis-error', (): string => buildErrorHtml(TEST_CSP_SOURCE, 'network', TEST_NONCE)],
@@ -512,7 +523,7 @@ function testBodyClassOverrides(): void {
512523
// Document & error templates must scope their overrides via the class
513524
// selector. Loading has no body override, only the base rule.
514525
const docHtml = buildDocumentHtml(
515-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/README.md', TEST_NONCE, false
526+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/README.md', TEST_NONCE, false
516527
);
517528
assert.match(
518529
docHtml, /body\.gis-doc\s*\{/,
@@ -567,7 +578,7 @@ function testDesignTokenCoverage(): void {
567578
const literalPattern = /1px solid var\(--vscode-panel-border\)/g;
568579
const allTemplates: ReadonlyArray<readonly [string, string]> = [
569580
['document', buildDocumentHtml(
570-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/README.md', TEST_NONCE, false
581+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/README.md', TEST_NONCE, false
571582
)],
572583
['loading', buildLoadingHtml(TEST_CSP_SOURCE, TEST_NONCE)],
573584
['error', buildErrorHtml(TEST_CSP_SOURCE, 'network', TEST_NONCE)],
@@ -603,7 +614,7 @@ function testBuildDocumentHtmlStructure(): void {
603614

604615
const html = buildDocumentHtml(
605616
TEST_CSP_SOURCE,
606-
'<p>Test content</p>',
617+
asSanitizedHtml('<p>Test content</p>'),
607618
'en',
608619
'docs/README.md',
609620
TEST_NONCE,
@@ -650,7 +661,7 @@ function testBuildDocumentHtmlNavigation(): void {
650661

651662
// nav element with aria-label (semantic HTML)
652663
const htmlWithBack = buildDocumentHtml(
653-
TEST_CSP_SOURCE, '<p>Content</p>', 'ja', 'docs/test.md', TEST_NONCE, true
664+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'ja', 'docs/test.md', TEST_NONCE, true
654665
);
655666
assert.ok(
656667
htmlWithBack.includes('<nav class="nav-bar" aria-label="Document navigation">'),
@@ -665,7 +676,7 @@ function testBuildDocumentHtmlNavigation(): void {
665676

666677
// Back button disabled when canGoBack=false
667678
const htmlNoBack = buildDocumentHtml(
668-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/test.md', TEST_NONCE, false
679+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/test.md', TEST_NONCE, false
669680
);
670681
assert.ok(
671682
htmlNoBack.includes('<button id="back-btn" disabled>'),
@@ -696,7 +707,7 @@ function testBuildDocumentHtmlContentEscaping(): void {
696707
// Locale with XSS payload should be escaped
697708
const html = buildDocumentHtml(
698709
TEST_CSP_SOURCE,
699-
'<p>Safe content</p>',
710+
asSanitizedHtml('<p>Safe content</p>'),
700711
'"><script>alert(1)</script>',
701712
'docs/<script>alert(2)</script>.md',
702713
TEST_NONCE,
@@ -851,15 +862,15 @@ function testLangAttributes(): void {
851862

852863
// Document HTML uses provided locale
853864
const docHtml = buildDocumentHtml(
854-
TEST_CSP_SOURCE, '<p>Content</p>', 'ja', 'docs/README.md', TEST_NONCE, false
865+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'ja', 'docs/README.md', TEST_NONCE, false
855866
);
856867
assert.ok(
857868
docHtml.includes('<html lang="ja">'),
858869
'Document HTML should use provided locale as lang'
859870
);
860871

861872
const docHtmlEn = buildDocumentHtml(
862-
TEST_CSP_SOURCE, '<p>Content</p>', 'en', 'docs/README.md', TEST_NONCE, false
873+
TEST_CSP_SOURCE, asSanitizedHtml('<p>Content</p>'), 'en', 'docs/README.md', TEST_NONCE, false
863874
);
864875
assert.ok(
865876
docHtmlEn.includes('<html lang="en">'),

extensions/git-id-switcher/src/ui/documentationInternal.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
* allowing them to be tested in isolation without VS Code environment.
1010
*/
1111

12+
// Type-only import: erased at runtime. htmlTemplates.ts has a runtime
13+
// import of `escapeHtmlEntities` from this file, so promoting this to a
14+
// value import would create a real circular dependency. Keep `import type`.
15+
import type { SanitizedHtml } from './htmlTemplates';
16+
1217
// ============================================================================
1318
// Constants
1419
// ============================================================================
@@ -320,9 +325,12 @@ export function getDocumentDisplayName(path: string): string {
320325
* We preserve HTML while converting Markdown-only sections.
321326
*
322327
* @param raw - Raw Markdown/HTML content
323-
* @returns Safe HTML string
328+
* @returns Safe HTML string, branded as `SanitizedHtml` so downstream
329+
* consumers (e.g. `buildDocumentHtml`) can enforce the trust boundary at
330+
* compile time. The single `as SanitizedHtml` cast at the return site is
331+
* the only sanctioned origin of the brand in the codebase.
324332
*/
325-
export function renderMarkdown(raw: string): string {
333+
export function renderMarkdown(raw: string): SanitizedHtml {
326334
// Content is trusted (self-managed CDN with hash verification)
327335
let html = raw;
328336

@@ -445,7 +453,9 @@ export function renderMarkdown(raw: string): string {
445453
html = html.replaceAll(new RegExp(String.raw`<p>\s*(</(?:${blockElements})>)`, 'g'), '$1');
446454
html = html.replaceAll(new RegExp(String.raw`(<(?:${blockElements})[^>]*>)\s*</p>`, 'g'), '$1');
447455

448-
return html;
456+
// Single sanctioned origin of the SanitizedHtml brand. All other callers
457+
// MUST go through this function; do not introduce additional casts.
458+
return html as SanitizedHtml;
449459
}
450460

451461
// ============================================================================

extensions/git-id-switcher/src/ui/htmlTemplates.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,24 @@ import { escapeHtmlEntities } from './documentationInternal';
2020
/** Error types for documentation display */
2121
export type ErrorType = 'network' | 'notfound' | 'server';
2222

23+
/**
24+
* Branded string type marking HTML that has already passed through a strict
25+
* allowlist sanitizer (see `renderMarkdown`). The brand exists purely at the
26+
* type level — there is no runtime tag — and acts as a trust-boundary marker:
27+
*
28+
* - Only the sanitizer is allowed to produce a value of this type (via a
29+
* single `as SanitizedHtml` cast at its return site).
30+
* - `buildDocumentHtml` accepts only `SanitizedHtml` for its verbatim-injected
31+
* `content` parameter, so a plain `string` (raw markdown, untrusted input,
32+
* a forgotten sanitizer call) fails at compile time instead of reaching the
33+
* webview and relying on CSP as the sole defence.
34+
*
35+
* Why not a class/wrapper object? The value flows through HTML string
36+
* concatenation inside the template, so we need structural `string` identity;
37+
* a branded alias gives us the type check with zero runtime cost.
38+
*/
39+
export type SanitizedHtml = string & { readonly __brand: 'SanitizedHtml' };
40+
2341
// ============================================================================
2442
// CSP Configuration
2543
// ============================================================================
@@ -229,11 +247,13 @@ ${opts.body}
229247
* Generate the document HTML with content
230248
*
231249
* @param cspSource - Webview CSP source
232-
* @param content - Pre-sanitized HTML content. Caller MUST pass HTML that
233-
* has already been run through a strict allowlist sanitizer (e.g. a
234-
* markdown renderer with HTML escaping). This function injects `content`
250+
* @param content - Pre-sanitized HTML content, typed as `SanitizedHtml` so
251+
* the compiler forbids passing a raw `string`. The brand can only originate
252+
* at the sanitizer boundary (`renderMarkdown`), making the trust boundary
253+
* structurally impossible to bypass. This function injects `content`
235254
* verbatim; CSP blocks `<script>` execution but does NOT stop `<img
236-
* onerror>` or `javascript:` URLs outside the linkInterceptScript path.
255+
* onerror>` or `javascript:` URLs outside the linkInterceptScript path,
256+
* which is exactly why we want the type-level guarantee on top of CSP.
237257
* @param locale - Current locale
238258
* @param currentPath - Current document path (for relative link resolution)
239259
* @param nonce - CSP nonce for inline styles and scripts
@@ -242,7 +262,7 @@ ${opts.body}
242262
*/
243263
export function buildDocumentHtml(
244264
cspSource: string,
245-
content: string,
265+
content: SanitizedHtml,
246266
locale: string,
247267
currentPath: string,
248268
nonce: string,

extensions/git-id-switcher/src/ui/webview.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,15 @@ import * as crypto from 'node:crypto';
1616

1717
import {
1818
type ErrorType,
19+
type SanitizedHtml,
1920
buildCspString,
2021
buildDocumentHtml,
2122
buildLoadingHtml,
2223
buildErrorHtml,
2324
} from './htmlTemplates';
2425
import { renderWithFallback } from './webviewFallback';
2526

26-
export type { ErrorType } from './htmlTemplates';
27+
export type { ErrorType, SanitizedHtml } from './htmlTemplates';
2728

2829
// ============================================================================
2930
// Nonce Generation
@@ -69,7 +70,7 @@ export function buildCsp(webview: vscode.Webview, nonce: string): string {
6970
*/
7071
export function getDocumentHtml(
7172
webview: vscode.Webview,
72-
content: string,
73+
content: SanitizedHtml,
7374
locale: string,
7475
currentPath: string,
7576
nonce: string,

0 commit comments

Comments
 (0)