Skip to content

security(phase-5): webview hygiene (L1)#22

Merged
rkm1 merged 1 commit into
mainfrom
security/phase-5-webview-hygiene
May 11, 2026
Merged

security(phase-5): webview hygiene (L1)#22
rkm1 merged 1 commit into
mainfrom
security/phase-5-webview-hygiene

Conversation

@rkm1
Copy link
Copy Markdown
Member

@rkm1 rkm1 commented May 11, 2026

Phase 5 of plans/security-fixes-2026-05.md.

Summary

The webviews shipped with this extension are trusted today (we own their HTML and enforce CSP via nonce), but defense-in-depth: every message-derived value is now validated at the trust boundary.

Finding Site Fix
L1 `src/extension/settingsPanel.ts` `openUrl` Parse with `new URL`, allow only `http`/`https`; drop silently otherwise
`src/extension/searchPanel.ts` `openResult` Validate `msg.file` via `isSafeRelativeRef` before joining onto workspaceRoot

Audited `mcpSetupPanel` (only clipboard.writeText, user-driven) and `indexStatusPanel` (no external sink, no path inputs) — both fine as-is.

Test plan

  • 367 tests pass (was 353; added 14: 7 scheme cases + 1 http acceptance + 6 search-panel path cases)
  • `npm run lint` clean
  • `npx tsc --noEmit` clean

🤖 Generated with Claude Code

The webviews shipped with this extension are trusted (we own their
HTML and enforce CSP via nonce), but defense-in-depth: validate every
message-derived value at the trust boundary so a future render bug or
injected content can't pivot a webview message into an arbitrary
filesystem or URL sink.

L1: settingsPanel.ts `openUrl`
  Validate the scheme before calling vscode.env.openExternal. Only
  http(s) URLs are passed through; file://, vscode://, javascript:,
  data:, ftp:, malformed inputs, etc. are dropped silently.

  searchPanel.ts `openResult`
  Validate `msg.file` via isSafeRelativeRef before joining onto
  workspaceRoot. Rejects absolute paths and any `..` segment, so a
  result-render bug can't trigger markdown.showPreviewToSide on
  ../../etc/passwd.

  Audit of the other webview message handlers:
    - mcpSetupPanel: only "copy" → clipboard.writeText, user-driven.
    - indexStatusPanel: "ready" / "refresh" / "reindex" — no
      external sink, no path inputs.
  Both are fine as-is.

Tests
  - settingsPanel: 7 cases asserting non-http schemes drop silently
    (file:, vscode:, javascript:, data:, ftp:, empty string, plain
    text) and one that http URLs still open.
  - searchPanel: 6 cases — safe ref opens; unsafe refs (leading ..,
    absolute, mid-path .., Windows backslashes, empty) drop.

367 tests pass (was 353; added 14).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rkm1 rkm1 merged commit f890dbc into main May 11, 2026
10 checks passed
@rkm1 rkm1 deleted the security/phase-5-webview-hygiene branch May 11, 2026 18:35
@rkm1 rkm1 mentioned this pull request May 11, 2026
4 tasks
rkm1 added a commit that referenced this pull request May 11, 2026
Security release. Bundles the five security-fix PRs from
plans/security-fixes-2026-05.md (PRs #18 through #22). Closes
findings H1, H2, H3, M1, M2, M3, M4, L1, L2, L3 from the 2026-05-09
internal review (three HIGH, four MEDIUM, three LOW).

Minor-version bump because the API-key handling and path-validation
changes affect user-visible behavior; see CHANGELOG.md.

367 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant