Skip to content

Defense-in-depth: webview/path/parser hardening and Skill Finder model switch - #94

Merged
Sanjay Singh (san360) merged 6 commits into
mainfrom
security/webview-path-parser-hardening
Jun 2, 2026
Merged

Sanjay Singh (san360) merged 6 commits into
mainfrom
security/webview-path-parser-hardening

Conversation

@aymenfurter

@aymenfurter Aymen (aymenfurter) commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Defense-in-depth hardening of the extension's webview message surface, parser, and supply chain, derived from a 10-point security review and a local CodeQL scan. These changes reduce the blast radius if a webview, a parsed log file, or a remote catalog response were ever malicious.

Security hardening

Webview message boundary

  • openExternal now validates the URL through isSafeExternalHttpsUrl (HTTPS + host allowed) before vscode.env.openExternal, instead of opening any user-supplied URI/scheme. (panel.ts, panel-shared.ts)
  • Sidebar webview can no longer trigger arbitrary commands — executeCommand is gated to an allowlist (aiEngineerCoach.open / .reload). (panel-sidebar.ts)
  • The RPC envelope is validated at the boundary (isRequestMessage: shape of type/id/method/params), and unknown methods are rejected. (panel-shared.ts, panel.ts)

Filesystem writes (path-traversal defense-in-depth)

  • saveRule resolves the target path and refuses to write outside the personal/project rules directories (classifyRuleWritePath / isPathUnder), covering both the new-file and existing-sourceFilePath branches. Auto-trust on save is scoped to personal rules only — project rules stay in the trust-on-first-use review flow. (panel-rpc.ts)
  • Skill / catalog installs go through safeJoinUnder: per-segment ^[A-Za-z0-9._-]+$ validation (rejects .., \, C:\, NTFS streams, leading //), a .md extension allowlist, and resolved-containment under ~/.agents/.... Parent dirs are created before write. (panel-request-service.ts, panel-shared.ts)

Parser

  • Prototype-pollution guard: __proto__ / constructor / prototype keys are rejected and own-property checks replace in when walking JSON paths from parsed log files. (parser-vscode-files.ts)

ReDoS

  • Overlap-aware alternation detection in the rule-DSL regex compiler, with a correctness fix so legitimate disjoint classes (e.g. ^([a-z]|_)+$) are no longer wrongly rejected. (safe-regex.ts)

Network / resource limits

  • Remote skills-catalog fetch is pinned to raw.githubusercontent.com under /github/awesome-copilot/ and read through a 1 MB byte cap, preventing unbounded memory from a large/hostile response. (panel-catalog.ts, fetch-utils.ts, panel-request-service.ts)

Packaging

  • Dev-only tooling and marketing assets excluded from the shipped VSIX. (.vscodeignore)

Skill Finder fix

  • Root cause: requests to gpt-4.1 were returning an empty stream due to insufficient_quota; empty text surfaced as "LLM returned invalid JSON after 3 attempts." Switched the preferred LLM family to gpt-5.4-mini with a mini-first fallback chain. (panel-llm.ts)
  • Added a panel-llm | call-failed diagnostic line (model id, response length, error, preview) to the runtime log so future LLM failures are debuggable.

Tests

New/expanded unit tests: safe-regex.test.ts, panel-shared.test.ts, parser-vscode-files.test.ts, fetch-utils.test.ts.

Validation

  • npm run check — typecheck, lint, build clean; 1059/1059 tests pass.
  • Static analysis: local CodeQL scan with the official CLI bundle (v2.25.5), javascript-security-extended suite (104 queries), DB built from src/ — 183 TS files scanned, 0 query failures, no actionable findings (only benign warnings/false-positives).

…l switch

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Comment thread src/core/dsl/safe-regex.test.ts Fixed
…expression'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Comment thread src/core/dsl/safe-regex.test.ts Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR strengthens the extension’s security posture around webview messaging, filesystem writes, parsing untrusted log input, regex safety checks, and network fetch limits, while also switching the Skill Finder’s preferred LLM family and improving diagnostics for LLM failures.

Changes:

  • Hardened webview RPC boundary (message shape validation, command allowlisting, safer external URL opening).
  • Added defense-in-depth for filesystem writes and network fetches (safe path joins, containment checks, redirect blocking, byte caps).
  • Improved robustness against prototype pollution and ReDoS; added/expanded unit coverage for the new hardening utilities.
Show a summary per file
File Description
src/webview/panel.ts Routes openExternal through URL validation and safer response handling.
src/webview/panel-sidebar.ts Restricts sidebar-triggered commands to an allowlist.
src/webview/panel-shared.ts Tightens request envelope validation; adds isSafeExternalHttpsUrl and safeJoinUnder.
src/webview/panel-shared.test.ts Adds unit tests for new shared validators/helpers.
src/webview/panel-rpc.ts Adds rule-write path containment checks and safer RPC handler lookup.
src/webview/panel-request-service.ts Applies safe path joining, redirect blocking, fetch size caps, and input validation for install/catalog/GitHub flows.
src/webview/panel-llm.ts Switches model family preference, improves JSON repair, and adds runtime diagnostics for LLM failures.
src/webview/panel-catalog.ts Adds redirect blocking and a byte cap when reading catalog HTML pages.
src/webview/fetch-utils.ts Introduces a reusable “read with byte limit” helper.
src/webview/fetch-utils.test.ts Adds tests for the byte-limit reader utility.
src/core/parser-vscode-files.ts Adds prototype-pollution key guards and safer own-property checks for path walking.
src/core/parser-vscode-files.test.ts Adds regression tests for prototype pollution via parsed JSONL paths.
src/core/dsl/safe-regex.ts Enhances alternation overlap detection for ReDoS defense.
src/core/dsl/safe-regex.test.ts Expands tests for alternation overlap detection behavior.
.vscodeignore Excludes dev-only tooling and marketing assets from the VSIX.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 15/15 changed files
  • Comments generated: 2

Comment thread src/webview/panel-llm.ts Outdated
Comment thread src/webview/panel-shared.ts
Aymen (aymenfurter) and others added 4 commits June 2, 2026 12:56
…expression'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The pattern is a test fixture verifying compileSafe rejects unsafe regexes.
compileSafe runs isLikelySafe() first and returns null before constructing
any RegExp, so the backtracking pattern is never executed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sitive

CodeQL taint-tracks the literal pattern into compileSafe's internal RegExp
and cannot see the isLikelySafe() gate that rejects it first. Constructing
the overlapping pattern at runtime keeps the test behavior identical while
removing the analyzable regex literal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@san360
Sanjay Singh (san360) merged commit a7c1391 into main Jun 2, 2026
6 checks passed
@san360
Sanjay Singh (san360) deleted the security/webview-path-parser-hardening branch June 3, 2026 07:11
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.

4 participants