Harden extension against local network and webview attacks - #40
Merged
Merged
Conversation
Security hardening across six advisories: Webview (SA-001, SA-002): - Add Content-Security-Policy with per-render nonce to debugger webview - Fix postMessage bridge: gate iframe commands on event.source + event.origin check - Replace postMessage(data, '*') with postMessage(data, FRAME_ORIGIN) on host responses - Restrict openExternal to http/https schemes only; refuse other URI handlers Debugger server auth (SA-003): - Generate per-session token on DebuggerServerManager construction - Inject FASTEDGE_DEBUG_TOKEN and FASTEDGE_BIND_HOST=127.0.0.1 into forked server - Add x-fastedge-token header to all extension-side /api/* fetch calls - Deliver token to iframe via URL fragment (#token=...) for frontend auth Autorun trigger file (SA-004): - Skip execution in untrusted workspaces - Require explicit user confirmation before running any trigger command - Remove commandArgs forwarding from trigger file Port file trust (SA-006): - Gate .debug-port reuse on vscode.workspace.isTrusted; always spawn fresh in untrusted workspaces MCP server config (SA-005): - Mask API key input (password: true) - Pin Docker image via mcp-server.version file injected at build time by esbuild - chmod 0600 on written mcp.json for local file URIs Cleanup: - Delete src/dotenv/ — superseded by server-side dotenv handling in fastedge-test - Add MCP_INTEGRATION.md context doc covering version pinning mechanism - Add vitest.config.ts to mirror esbuild define for __MCP_SERVER_VERSION__ - Add tests: webview HTML security properties, trigger file guards, mcpJson image pin
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
godronus
added a commit
that referenced
this pull request
Sep 8, 2026
- Revert isHealthyOnPort to probe /health without auth — the previous /api/client-count probe sent the session token to an unverified endpoint, allowing a malicious loopback listener to steal the token and be adopted as the debugger backend - mcp.json: open with O_CREAT|O_TRUNC at mode 0o600 so new files are never briefly readable at 0o644; chmodSync after also fixes pre-existing files; remote-scheme paths (vscode-remote, Codespaces) fall back to workspace.fs with best-effort permissions since Node fs.chmod targets the local host
vscodeignore test/ remove internal security advisory context from public repo Audit docs moved to fastedge-coordinator/context/security-advisories/ so they do not ship to users of this public package. fix: address MoM security findings from PR #40 review - Revert isHealthyOnPort to probe /health without auth — the previous /api/client-count probe sent the session token to an unverified endpoint, allowing a malicious loopback listener to steal the token and be adopted as the debugger backend - mcp.json: open with O_CREAT|O_TRUNC at mode 0o600 so new files are never briefly readable at 0o644; chmodSync after also fixes pre-existing files; remote-scheme paths (vscode-remote, Codespaces) fall back to workspace.fs with best-effort permissions since Node fs.chmod targets the local host
…d permissions - DebuggerServerManager.readPortFile() now requires PORT:sha256(token) format; rejects legacy plain-number format so a malicious workspace server cannot be adopted and receive the session token via the webview URL fragment (P1) - mcpJson.ts: fchmodSync(fd, 0o600) before writeSync so a pre-existing 0o644 mcp.json has its permissions locked down before any API key is written, closing the post-write chmod race window (P2) - Update tests: readPortFile tests use new PORT:HASH format; probe URL assertion updated from /api/client-count to /health; Codespaces fork tests updated to supply correct port file format via setupForSpawn helper
openSync with O_TRUNC follows symlinks — a workspace-controlled .vscode/mcp.json -> /some/external/file could redirect the write outside the workspace. lstatSync + unlinkSync removes any symlink first so the subsequent open always creates a real file at the intended path (P2). security: close TOCTOU and parent-dir symlink attack vectors in mcpJson Replace lstat+unlink+openSync with O_NOFOLLOW on the openSync call — atomically rejects a symlink at the mcp.json leaf path with no TOCTOU window. Also verify the .vscode parent directory realpath is within the workspace root before writing, blocking the case where .vscode itself is a symlink pointing outside the workspace. security: address MoM round-4 findings in mcpJson 1. Add Windows fallback lstat check when O_NOFOLLOW is unavailable; refuse write if mcp.json file is a symlink. 2. Remote workspace branch: use vscode.workspace.fs.stat() to check for symlinks on both the .vscode parent directory and the mcp.json target file before writing, blocking redirect attacks on remote FS. security: use readDirectory (lstat semantics) for remote symlink detection workspace.fs.stat() follows symlinks and throws FileNotFound for dangling ones, causing the previous check to treat a dangling symlink as a non-existent file and proceed with the write. workspace.fs.readDirectory() uses lstat semantics and returns FileType.SymbolicLink for both live and dangling symlinks, so the check now reliably detects both cases for .vscode and mcp.json. security: case-insensitive symlink name matching for remote workspaces On case-insensitive remote filesystems (macOS, Windows), a malicious workspace can name links .VSCODE or MCP.JSON. readDirectory() returns the preserved casing, so exact comparisons missed them while writeFile() would still resolve to the same path. Lowercase both sides of the comparison so all case variants are caught.
godronus
force-pushed
the
fix/security-advisories
branch
from
September 8, 2026 12:13
2ae42f6 to
da204a1
Compare
- triggerFileHandler: decode Uint8Array with Buffer.from().toString('utf8')
instead of .toString() which produces a comma-separated byte list
- mcpJson local branch: mkdirSync .vscode/ before openSync — O_CREAT does
not create parent directories, so the command failed when .vscode/ was absent
- mcpJson remote branch: createDirectory .vscode/ before writeFile for the
same reason
- DebuggerServerManager.test.ts: correct misleading "token-authenticated probe"
section header — the /health probe is intentionally unauthenticated
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The tightened webview CSP currently blocks inline style usage used for loading/iframe visibility toggling, likely breaking the debugger UI.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/debugger/DebuggerWebviewProvider.ts:378
- The new CSP uses
style-src 'nonce-…', which blocks inline style attributes and style-attribute mutations. This HTML relies onstyle="display:none"and laterloading.style.display/iframe.style.display, so the iframe/load overlay toggling can break under CSP enforcement.
- Files reviewed: 21/22 changed files
- Comments generated: 0 new
- Review effort level: Lite
NOT AN ISSUE: styles are injected correctly and this tightening is for security reasons to stop malicous injection
qrdl
approved these changes
Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security hardening across six advisories:
Webview (SA-001, SA-002):
Debugger server auth (SA-003):
Autorun trigger file (SA-004):
Port file trust (SA-006):
MCP server config (SA-005):
Cleanup: