Skip to content

chore(webview): declare asset resource roots and unit-test the CSP helpers - #305

Merged
eFAILution merged 1 commit into
eFAILution:betafrom
X-Guardian:chore/webview-resource-roots-and-harness
Sep 21, 2026
Merged

eFAILution merged 1 commit into
eFAILution:betafrom
X-Guardian:chore/webview-resource-roots-and-harness

Conversation

@X-Guardian

Copy link
Copy Markdown
Contributor

Summary

  • Second step of the webview asset work. Nothing consumes webview assets outside the loading view yet, so this PR is deliberately inert: it clears the way for the extractions that follow and adds the test coverage they will rely on.
  • Fixes a latent blocker: the detached details panel declares localResourceRoots: [], which permits no local files at all. The moment that panel's CSS or scripts move to files they would 404 — on that path only, while the identical document rendered from the Component Browser looked correct.
  • Splits the vscode-free half of webviewHtml.ts into src/webview/csp.ts so the nonce and CSP helpers can be unit-tested, and closes two findings from the refactor(webview): serve loading-view CSS from linted external file under CSP #275 review while writing those tests.
  • Link related issue(s): refactor(webview): extend the CSP to the remaining five Component Browser documents #288

Change Type

  • feat
  • fix
  • refactor
  • docs
  • test
  • chore (lint config)

Context

User-facing impact

  • None. The only document currently using an external asset is the loading view, which is unaffected.

GitLab scope

  • gitlab.com
  • self-managed GitLab
  • both (webview plumbing only)

Affected areas

  • Component Browser (panel options only)
  • Hover provider (the detached details panel's options)
  • Completion provider
  • Validation provider
  • Cache and refresh behavior
  • GitLab API calls/auth/token storage
  • Docs only

What changed by bucket

Bucket Files Approach
Resource roots src/extension.ts, src/providers/componentBrowserProvider.ts, src/webview/webviewHtml.ts New assetRoots(extensionUri) beside assetUri, which already owns the out/webview layout. All three panels use it: the detached panel replaces localResourceRoots: [], the browser-path details panel gains an explicit declaration instead of relying on the default, and the Component Browser drops its hand-written path.
Testable CSP helpers src/webview/csp.ts, src/webview/webviewHtml.ts createNonce and cspMetaTag move to a vscode-free module, with cspMetaTag(cspSource, nonce) taking the origin as a string. webviewHtml.ts keeps assetUri/assetRoots and re-exports the pair, so callers still have one import.
Nonce distribution src/webview/csp.ts The nonce alphabet is 64 characters, so indexing by a random byte draws uniformly. At 62 the first two characters were over-represented by ~25%.
Asset path guard src/webview/webviewHtml.ts assetUri rejects empty paths and ./.. segments, so a computed path cannot resolve outside the root its panel declares.
Sourcemap delivery esbuild.js Webview assets get an inline sourcemap in development. A sibling .map file makes the webview fetch it, which the document's default-src 'none' CSP blocks and reports — noise in the console that PRs 3-6 will be watching for real violations. Production builds emit no map either way.
Client lint eslint.config.js A block scoped to src/webview/client/**/*.ts with globals.browser plus acquireVsCodeApi. Scoped rather than applied to all TypeScript so extension-host code referencing document or window is still flagged.
Test coverage tests/unit/csp.test.ts Seven cases over the nonce (length, alphabet, uniqueness, distribution) and the CSP tag (default-src 'none', nonce-gated scripts, no 'unsafe-inline' on either directive).

Validation

Local checks

  • npm run compile
  • npm test — 438 passing (+7 new); npm run lint clean
  • Extension-host suite — 25 passing
  • Manual verification in VS Code Extension Host

Manual test notes

  • Opened the Component Browser and both details panels; the loading view still renders its spinner from styles/loading.css.

Breaking Changes

  • No breaking changes
  • Breaking changes (describe below)

Risk and Rollback

  • Main risks: low. Declaring a resource root only widens what a panel may load, and the one document using an asset today already worked. The assetUri guard could in principle reject a path that previously resolved, but its only caller passes the literal styles/loading.css.
  • Rollback strategy: revert the commit.

Release Notes Draft

  • Internal: webview panels now declare the resource root their assets are served from; no user-facing change.

Checklist

  • Branch is up to date with target branch
  • Commit messages follow conventional commits
  • Added/updated docs for behavior or settings changes (this PR description)
  • Added/updated tests for new behavior
  • No secrets or tokens in code, logs, screenshots, or test fixtures

@eFAILution eFAILution left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approving. Closes #284, #285 and #286 exactly as they were written, and picks up a real bug on the way.

The empty localResourceRoots: [] on the detached panel is the one worth calling out — it permits no local files at all, so the identical document rendered from the Component Browser would have kept working while the hover path 404'd, which is the shape of bug that gets found late and blamed on the wrong change.

Verified:

  • createNonce alphabet is 64 characters, so byte % 64 divides 256 evenly and the bias is gone. The distribution test is the good kind — 2000 nonces, every character required to appear, max under 1.15x expected. It would fail on the old 62-character alphabet, which is what makes it worth having.
  • assetUri throws on empty, . and .., and ASSET_ROOT is now the single place the out/webview layout is written down, shared with assetRoots.
  • All three panels declare assetRoots(...): the detached one, the browser-path details panel, and the Component Browser.

438 passing, lint and types clean, CI green. Scoping the browser globals block to src/webview/client/** rather than all TypeScript is the right call — extension-host code touching document still gets flagged.

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.

2 participants