Skip to content

feat: security hardening — path validation, env filtering, XSS preven…#18

Open
whatnick wants to merge 2 commits into
formulahendry:mainfrom
whatnick:security-hardening
Open

feat: security hardening — path validation, env filtering, XSS preven…#18
whatnick wants to merge 2 commits into
formulahendry:mainfrom
whatnick:security-hardening

Conversation

@whatnick

Copy link
Copy Markdown

Addresses critical and high-severity findings from security audit:

  • Add SecurityPolicy module with centralized path validation, env filtering, Windows arg escaping, webview command allowlist, and log redaction
  • FileSystemHandler: enforce workspace boundary on all file operations (C1)
  • TerminalHandler: validate cwd, filter env vars, disable shell mode (C2)
  • AgentManager: escape Windows args, filter config env vars (C3/H4)
  • PermissionHandler: serial promise queue for concurrent requests (PR#2), granular per-capability auto-approve replacing blanket allowAll (H2)
  • AgentConfig: ignore workspace-level agent configs in untrusted workspaces (C3)
  • ChatWebviewProvider: sanitize markdown via DOMPurify, restrict executeCommand to allowlisted commands (H1/M4)
  • Logger: default traffic logging off, truncate payloads, redact secrets (H3)
  • SessionManager: use defaultWorkingDirectory setting for cwd resolution (PR#6)
  • ConnectionManager: pass workspaceRoot to handlers for boundary enforcement

Incorporates content from Robert Pyke's PRs:

  • PR#2: queue concurrent permission requests to prevent silent cancellations
  • PR#6: spawn agent process in workspace cwd so workspace-scoped configs work

…tion, permission queue

Addresses critical and high-severity findings from security audit:

- Add SecurityPolicy module with centralized path validation, env filtering,
  Windows arg escaping, webview command allowlist, and log redaction
- FileSystemHandler: enforce workspace boundary on all file operations (C1)
- TerminalHandler: validate cwd, filter env vars, disable shell mode (C2)
- AgentManager: escape Windows args, filter config env vars (C3/H4)
- PermissionHandler: serial promise queue for concurrent requests (PR#2),
  granular per-capability auto-approve replacing blanket allowAll (H2)
- AgentConfig: ignore workspace-level agent configs in untrusted workspaces (C3)
- ChatWebviewProvider: sanitize markdown via DOMPurify, restrict executeCommand
  to allowlisted commands (H1/M4)
- Logger: default traffic logging off, truncate payloads, redact secrets (H3)
- SessionManager: use defaultWorkingDirectory setting for cwd resolution (PR#6)
- ConnectionManager: pass workspaceRoot to handlers for boundary enforcement

Incorporates content from Robert Pyke's PRs:
- PR#2: queue concurrent permission requests to prevent silent cancellations
- PR#6: spawn agent process in workspace cwd so workspace-scoped configs work
Copilot AI review requested due to automatic review settings April 17, 2026 05:49

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

Security-hardening PR that centralizes several safety controls (path boundary enforcement, env filtering, Windows arg escaping, webview sanitization/allowlisting, safer logging) to address findings from the included security audit and reduce the extension’s attack surface when interacting with ACP agents.

Changes:

  • Add SecurityPolicy utilities (path validation, env filtering, Windows arg escaping, webview command allowlist, log redaction patterns).
  • Enforce workspace boundary checks for filesystem operations and terminal CWD; disable shell mode for terminal spawn.
  • Reduce webview risk (DOMPurify sanitize markdown; allowlist executeCommand) and harden permission prompting (serial queue + granular auto-approve settings).

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/utils/Logger.ts Defaults traffic logging off; truncates and attempts redaction of traffic payloads.
src/ui/ChatWebviewProvider.ts Sanitizes markdown output and restricts webview-triggered VS Code commands to an allowlist.
src/security/SecurityPolicy.ts New centralized security utilities (path/env/escaping/allowlist/redaction).
src/handlers/TerminalHandler.ts Validates terminal CWD within workspace root; filters injected env vars; disables shell.
src/handlers/PermissionHandler.ts Serializes permission prompts and introduces per-capability auto-approve settings.
src/handlers/FileSystemHandler.ts Enforces workspace-root path validation for read/write operations.
src/core/SessionManager.ts Adds defaultWorkingDirectory handling for agent session CWD selection.
src/core/ConnectionManager.ts Passes a workspace root into handlers to enforce boundary checks.
src/core/AgentManager.ts Filters env overrides and escapes Windows args when spawning with a shell.
src/config/AgentConfig.ts Ignores workspace-level agent configs when workspace is untrusted.
package.json Adds new security-related settings; adds isomorphic-dompurify; changes traffic logging default.
SECURITY_AUDIT.md Adds a security audit report documenting findings and recommendations.

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

Comment thread src/config/AgentConfig.ts Outdated
Comment thread package.json Outdated
Comment thread src/handlers/PermissionHandler.ts
Comment thread src/core/SessionManager.ts
Comment thread src/handlers/FileSystemHandler.ts
Comment thread src/security/SecurityPolicy.ts Outdated
Comment thread src/security/SecurityPolicy.ts Outdated
alanjds pushed a commit to alanjds/vscode-acp that referenced this pull request Jun 30, 2026
Cherry-picked and adapted from formulahendry/vscode-acp:

- fix(formulahendry#46): use correct Copilot package (@github/copilot@latest)
- fix(#2): serialize concurrent permission requests to prevent silent
  cancellations when multiple QuickPick dialogs overlap
- fix(formulahendry#6): respect acp.defaultWorkingDirectory in CWD resolution so
  workspace-scoped agents (kiro-cli etc.) find their config files
- fix(formulahendry#13): add 30s AbortController timeout to registry fetch;
  make connect/newConversation/restart progress notifications cancellable
- fix(formulahendry#47): redirect ACP pipe to fd 3 on login shells to prevent
  profile scripts (e.g. distrobox_profile.sh) from consuming fd 0
- feat(formulahendry#18): security hardening — workspace path validation in
  FileSystemHandler, env var filtering and shell:false in TerminalHandler
- feat(formulahendry#42): editor context link — toggle to prepend active file/cursor
  info to chat prompts (acp.toggleEditorContext command)
- feat(formulahendry#40): add Grok Build as pre-configured agent
- feat(formulahendry#35): add openHarness as pre-configured agent
- feat(formulahendry#44): add siGit Code as pre-configured agent
- feat(formulahendry#37/agent): add Reasonix as pre-configured agent
  (diff preview implementation deferred to separate PR using formulahendry#43 approach)

Co-Authored-By: Claude Sonnet 4.6 <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.

2 participants