Skip to content

Commit f834261

Browse files
nullvariantclaude
andauthored
security(git-id-switcher): tighten CSP img-src from wildcard to explicit subdomain (#462)
- Replace `*.githubusercontent.com` with `avatars.githubusercontent.com` in buildCspString img-src to prevent loading arbitrary files from attacker-controlled repositories via raw.githubusercontent.com - Add scoped absence tests that extract the img-src directive and verify wildcard / raw.githubusercontent.com are not present - Move `coerceLang` and `STYLE_CLOSE_PATTERN` from csp.ts to shell.ts to align module boundaries (shell-rendering vs CSP concerns) - Update ARCHITECTURE.md CSP section to reflect current policy - Add shell.ts to ESLint regex-constant allowlist 🖥️ 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 1b6b175 commit f834261

6 files changed

Lines changed: 76 additions & 49 deletions

File tree

extensions/git-id-switcher/docs/ARCHITECTURE.md

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -645,32 +645,40 @@ src/
645645

646646
## Webview Content Security Policy
647647

648-
This extension currently does not use Webviews for rendering. If Webviews are added in the future, the following CSP **must** be applied:
648+
The authoritative CSP implementation lives in `src/ui/htmlTemplates/csp.ts` (`buildCspString`). The current policy:
649649

650-
```
650+
```text
651651
default-src 'none';
652-
script-src 'nonce-${nonce}';
652+
base-uri 'none';
653+
form-action 'none';
654+
frame-ancestors 'none';
655+
img-src ${cspSource} https://assets.nullvariant.com https://img.shields.io https://avatars.githubusercontent.com;
653656
style-src 'nonce-${nonce}';
654-
img-src ${webview.cspSource} data:;
655-
font-src ${webview.cspSource};
657+
script-src 'nonce-${nonce}';
658+
connect-src https://assets.nullvariant.com;
659+
font-src ${cspSource};
656660
```
657661

658662
### Policy Rationale
659663

660-
| Directive | Value | Why |
661-
| ------------- | ----------------- | -------------------------------------------------------- |
662-
| `default-src` | `'none'` | Deny everything by default |
663-
| `script-src` | `'nonce-...'` | Only allow scripts with a per-render cryptographic nonce |
664-
| `style-src` | `'nonce-...'` | Only allow styles with a nonce (no `'unsafe-inline'`) |
665-
| `img-src` | `cspSource data:` | Allow VS Code resource images and inline SVG icons |
666-
| `font-src` | `cspSource` | Allow VS Code bundled fonts only |
664+
| Directive | Value | Why |
665+
| ----------------- | ------------------------------------- | -------------------------------------------------------------------------- |
666+
| `default-src` | `'none'` | Deny everything by default |
667+
| `base-uri` | `'none'` | Not covered by `default-src`; prevents `<base>` injection (CSP3 §6.1) |
668+
| `form-action` | `'none'` | Not covered by `default-src`; prevents form submission to attacker origins |
669+
| `frame-ancestors` | `'none'` | Not covered by `default-src`; prevents clickjacking via iframe embedding |
670+
| `img-src` | `cspSource`, CDN, shields.io, avatars | VS Code resources, CDN assets, README badges, GitHub contributor avatars |
671+
| `style-src` | `'nonce-...'` | Nonce-only; `cspSource` removed to close stylesheet bypass |
672+
| `script-src` | `'nonce-...'` | Only allow scripts with a per-render cryptographic nonce |
673+
| `connect-src` | CDN only | Fetch README/documentation from CDN; no other network access |
674+
| `font-src` | `cspSource` | Allow VS Code bundled fonts only |
667675

668-
### Prohibited Patterns
676+
### CSP Prohibited Patterns
669677

670678
- `'unsafe-inline'` — Enables XSS via injected `<script>` tags
671679
- `'unsafe-eval'` — Enables code injection via `eval()`
672680
- `*` or `https:` in any directive — Allows loading from arbitrary origins
673-
- `connect-src`No network access from Webviews
681+
- Wildcard subdomains (e.g. `*.githubusercontent.com`)Only specific subdomains are allowed
674682

675683
---
676684

extensions/git-id-switcher/eslint.config.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ export default [
160160
},
161161
},
162162
{
163-
files: ["**/validators/common.ts", "**/core/constants.ts", "**/ui/htmlTemplates/csp.ts"],
163+
files: ["**/validators/common.ts", "**/core/constants.ts", "**/ui/htmlTemplates/csp.ts", "**/ui/htmlTemplates/shell.ts"],
164164
rules: {
165165
"no-restricted-syntax": "off",
166166
"no-magic-numbers": "off",

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,18 +94,36 @@ function testBuildCspString(): void {
9494
'CSP should have default-src none'
9595
);
9696

97-
// Should include img-src with cspSource and CDN
97+
// img-src: extract the directive to scope assertions precisely.
98+
const imgSrc = csp.split('; ').find(d => d.startsWith('img-src')) ?? '';
9899
assert.ok(
99-
csp.includes(`img-src ${TEST_CSP_SOURCE}`),
100-
'CSP should include cspSource in img-src'
100+
imgSrc.includes(`img-src ${TEST_CSP_SOURCE}`),
101+
'img-src should include cspSource'
101102
);
102103
assert.ok(
103-
csp.includes('https://assets.nullvariant.com'),
104-
'CSP should include CDN in img-src'
104+
imgSrc.includes('https://assets.nullvariant.com'),
105+
'img-src should include CDN'
105106
);
106107
assert.ok(
107-
csp.includes('https://img.shields.io'),
108-
'CSP should include shields.io in img-src'
108+
imgSrc.includes('https://img.shields.io'),
109+
'img-src should include shields.io'
110+
);
111+
assert.ok(
112+
imgSrc.includes('https://avatars.githubusercontent.com'),
113+
'img-src should include avatars.githubusercontent.com'
114+
);
115+
116+
// img-src absence checks — wildcard *.githubusercontent.com must not
117+
// appear; only the avatars subdomain is needed. raw.githubusercontent.com
118+
// would allow loading arbitrary files from attacker-controlled
119+
// repositories (Issue-00196).
120+
assert.ok(
121+
!imgSrc.includes('*.githubusercontent.com'),
122+
'img-src must not contain wildcard *.githubusercontent.com'
123+
);
124+
assert.ok(
125+
!imgSrc.includes('raw.githubusercontent.com'),
126+
'img-src must not include raw.githubusercontent.com'
109127
);
110128

111129
// style-src must be nonce-only — cspSource removed to close

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const DOCUMENT_HASHES: Record<string, string> = {
3434
'CODE_OF_CONDUCT.md': 'a0e9cb2e004663cdedef4e1adc0e429417ccfc479e367cbc17b869f62ae759d2',
3535
'CONTRIBUTING.md': 'ed4d1f391ffe04e3031dfc9f16fd8fd5dcd54ba23af3b3202c07adac5ba23da7',
3636
'extensions/git-id-switcher/CHANGELOG.md': 'b11d9b619f23b9e55c31302b9a55f455ade9c58f65ce485b0d6ae4ddeb289e7a',
37-
'extensions/git-id-switcher/docs/ARCHITECTURE.md': 'db6ba2f7809b2c7aa831eda3a4b9bb80521577e4e267c7b6ccad17ffba847548',
37+
'extensions/git-id-switcher/docs/ARCHITECTURE.md': 'd5d879d988054d208739497962a0f937f2f21bdaab51776c4e8363cba99d634c',
3838
'extensions/git-id-switcher/docs/CONTRIBUTING.md': '7d6ad2bc4d8c838790754cb9df848cb65f9fdce7e1a13e5c965b83a3d5b6378c',
3939
'extensions/git-id-switcher/docs/DESIGN_PHILOSOPHY.md': 'f9718b61ac161cb466dbc76845688e7acacf4e5fdc4b8b9553269dba4a094f6b',
4040
'extensions/git-id-switcher/docs/i18n/ain/README.md': 'bd740f8772789dfca74a67339d18ffc8bf1d2501fe4c4091120830410e4b5abd',

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

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44
* Owns the CSP trust boundary for the webview shell:
55
* - `CspValidationError` (the narrow-able class thrown on format failures)
66
* - Format patterns for nonce / cspSource / lang
7-
* - `assertValidNonce` / `assertValidLang` / `coerceLang`
7+
* - `assertValidNonce` / `assertValidLang`
88
* - `buildCspString` (assembles the final `content` attribute value)
99
*
10+
* `coerceLang` and `STYLE_CLOSE_PATTERN` were moved to `shell.ts` as
11+
* they are shell-rendering concerns, not CSP concerns.
12+
*
1013
* Split from shell.ts in Issue-00243 so shell.ts can shrink to the skeleton
1114
* wrapper alone. All functions are pure and free of VS Code dependencies.
1215
*
@@ -92,8 +95,8 @@ export function assertValidNonce(nonce: string): void {
9295
* Validate a BCP 47 language tag for interpolation into `<html lang="…">`.
9396
* Exported for defense-in-depth alongside `assertValidNonce`. Callers that
9497
* may legitimately receive an empty locale (e.g. bootstrap before i18n is
95-
* ready) should pass the empty string through `coerceLang` first rather than
96-
* duplicating the fallback logic.
98+
* ready) should coerce the empty string to a safe default first rather than
99+
* duplicating the fallback logic (see `coerceLang` in `shell.ts`).
97100
*
98101
* @throws {CspValidationError} with a static, scrubbed message.
99102
*/
@@ -103,26 +106,6 @@ export function assertValidLang(lang: string): void {
103106
}
104107
}
105108

106-
/**
107-
* Coerce a possibly-empty locale to a safe default before validation. Kept
108-
* separate from `assertValidLang` so that the validator remains fail-closed
109-
* for *all* callers — only the shell, which owns the rendering contract,
110-
* opts into the fallback.
111-
*/
112-
export function coerceLang(lang: string): string {
113-
return lang === '' ? 'en' : lang;
114-
}
115-
116-
/**
117-
* Detects the `</style` substring (case-insensitive) in a raw-text element
118-
* context. HTML5 raw-text elements (`<style>`, `<script>`) terminate at the
119-
* first occurrence of their own closing tag — any `</style` inside a `<style>`
120-
* block therefore breaks out of the element and re-enters HTML context,
121-
* enabling attribute injection. Used by `buildHtmlShell` to fail-closed on
122-
* `extraStyles` before interpolation.
123-
*/
124-
export const STYLE_CLOSE_PATTERN = /<\/style/i;
125-
126109
/**
127110
* Build Content Security Policy header value
128111
*
@@ -159,7 +142,7 @@ export function buildCspString(cspSource: string, nonce: string): string {
159142
`form-action 'none'`,
160143
`frame-ancestors 'none'`,
161144
// Allow images from: VSCode, our CDN, shields.io badges, GitHub avatars
162-
`img-src ${cspSource} https://assets.nullvariant.com https://img.shields.io https://*.githubusercontent.com`,
145+
`img-src ${cspSource} https://assets.nullvariant.com https://img.shields.io https://avatars.githubusercontent.com`,
163146
// style-src is nonce-only: dropping cspSource closes the
164147
// `<link rel="stylesheet" href="${cspSource}/…">` bypass.
165148
`style-src 'nonce-${nonce}'`,

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,30 @@ import { escapeHtmlEntities } from '../documentationInternal';
2121
import { getBaseStyles } from './baseStyles';
2222
import {
2323
CspValidationError,
24-
STYLE_CLOSE_PATTERN,
2524
assertValidLang,
2625
assertValidNonce,
2726
buildCspString,
28-
coerceLang,
2927
} from './csp';
28+
29+
/**
30+
* Coerce a possibly-empty locale to a safe default before validation. Kept
31+
* separate from `assertValidLang` so that the validator remains fail-closed
32+
* for *all* callers — only the shell, which owns the rendering contract,
33+
* opts into the fallback.
34+
*/
35+
function coerceLang(lang: string): string {
36+
return lang === '' ? 'en' : lang;
37+
}
38+
39+
/**
40+
* Detects the `</style` substring (case-insensitive) in a raw-text element
41+
* context. HTML5 raw-text elements (`<style>`, `<script>`) terminate at the
42+
* first occurrence of their own closing tag — any `</style` inside a `<style>`
43+
* block therefore breaks out of the element and re-enters HTML context,
44+
* enabling attribute injection. Used by `buildHtmlShell` to fail-closed on
45+
* `extraStyles` before interpolation.
46+
*/
47+
const STYLE_CLOSE_PATTERN = /<\/style/i;
3048
import { type BodyClass, type SanitizedHtml, VALID_BODY_CLASSES } from './types';
3149

3250
/**

0 commit comments

Comments
 (0)