From 7450b5160b69d54eff1feb78e3e99a97e16fb4d5 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 28 Mar 2026 08:35:24 -0600 Subject: [PATCH] =?UTF-8?q?fix:=20security=20audit=20remediation=20?= =?UTF-8?q?=E2=80=94=2012=20fixes,=2020=20tests=20(v0.13.1.0)=20(#595)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: remove auth token from /health, secure extension bootstrap (CRITICAL-02 + HIGH-03) - Remove token from /health response (was leaked to any localhost process) - Write .auth.json to extension dir for Manifest V3 bootstrap - sidebar-agent reads token from state file via BROWSE_STATE_FILE env var - Remove getToken handler from extension (token via health broadcast) - Extension loads token before first health poll to prevent race condition * fix: require auth on cookie-picker data routes (CRITICAL-01) - Add Bearer token auth gate on all /cookie-picker/* data/action routes - GET /cookie-picker HTML page stays unauthenticated (UI shell) - Token embedded in served HTML for picker's fetch calls - CORS preflight now allows Authorization header * fix: add state file TTL and plaintext cookie warning (HIGH-02) - Add savedAt timestamp to state save output - Warn on load if state file older than 7 days - Auto-delete stale state files (>7 days) on server startup - Warning about plaintext cookie storage in save message * fix: innerHTML XSS in extension content script and sidepanel (MEDIUM-01) - content.js: replace innerHTML with createElement/textContent for ref panel - sidepanel.js: escape entry.command with escapeHtml() in activity feed - Both found by security audit + Codex adversarial red team * fix: symlink bypass in validateReadPath (MEDIUM-02) - Always resolve to absolute path first (fixes relative path bypass) - Use realpathSync to follow symlinks before boundary check - Throw on non-ENOENT realpathSync failures (explicit over silent) - Resolve SAFE_DIRECTORIES through realpathSync (macOS /tmp → /private/tmp) - Resolve directory part for non-existent files (ENOENT with symlinked parent) * fix: freeze hook symlink bypass and prefix collision (MEDIUM-03) - Add POSIX-portable path resolution (cd + pwd -P, works on macOS) - Fix prefix collision: /project-evil no longer matches /project freeze dir - Use trailing slash in boundary check to require directory boundary * fix: shell script injection in gstack-config and telemetry (MEDIUM-04) - gstack-config: validate keys (alphanumeric+underscore only) - gstack-config: use grep -F (fixed string) instead of -E (regex) - gstack-config: escape sed special chars in values, drop newlines - gstack-telemetry-log: sanitize REPO_SLUG and BRANCH via json_safe() * test: 20 security tests for audit remediation - server-auth: verify token removed from /health, auth on /refs, /activity/* - cookie-picker: auth required on data routes, HTML page unauthenticated - path-validation: symlink bypass blocked, realpathSync failure throws - gstack-config: regex key rejected, sed special chars preserved - state-ttl: savedAt timestamp, 7-day TTL warning - telemetry: branch/repo with quotes don't corrupt JSON - adversarial: sidepanel escapes entry.command, freeze prefix collision * chore: bump version and changelog (v0.13.1.0) Co-Authored-By: Claude Opus 4.6 * docs: tone down changelog — defense in depth, not catastrophic bugs Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .gitignore | 1 + CHANGELOG.md | 19 +++++++ VERSION | 2 +- bin/gstack-config | 18 +++++-- bin/gstack-telemetry-log | 2 + browse/src/browser-manager.ts | 27 +++++++++- browse/src/cookie-picker-routes.ts | 16 +++++- browse/src/cookie-picker-ui.ts | 7 ++- browse/src/meta-commands.ts | 11 +++- browse/src/read-commands.ts | 33 ++++++++---- browse/src/server.ts | 62 ++++++++++++++++------ browse/src/sidebar-agent.ts | 7 +-- browse/test/adversarial-security.test.ts | 32 ++++++++++++ browse/test/commands.test.ts | 6 +-- browse/test/cookie-picker-routes.test.ts | 55 ++++++++++++++++++++ browse/test/gstack-config.test.ts | 13 +++++ browse/test/path-validation.test.ts | 36 +++++++++++-- browse/test/server-auth.test.ts | 65 ++++++++++++++++++++++++ browse/test/state-ttl.test.ts | 35 +++++++++++++ extension/background.js | 42 ++++++++++----- extension/content.js | 11 +++- extension/sidepanel.js | 15 +++--- freeze/bin/check-freeze.sh | 13 ++++- test/telemetry.test.ts | 28 ++++++++++ 24 files changed, 489 insertions(+), 67 deletions(-) create mode 100644 browse/test/adversarial-security.test.ts create mode 100644 browse/test/server-auth.test.ts create mode 100644 browse/test/state-ttl.test.ts diff --git a/.gitignore b/.gitignore index e1e6ed0e08ea1a2afc54af1a6950703b7a6a6b26..38b0fc28238fff5aecb8e96ff005ff953b67957e 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ bin/gstack-global-discover .claude/skills/ .agents/ .context/ +extension/.auth.json .gstack-worktrees/ /tmp/ *.log diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a5b925fd5688d47a2876696ef26eb614abe7ed2..24b7111a5d16bdcb3a8831f112b700d1566ddea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## [0.13.1.0] - 2026-03-28 — Defense in Depth + +The browse server runs on localhost and requires a token for access, so these issues only matter if a malicious process is already running on your machine (e.g., a compromised npm postinstall script). This release hardens the attack surface so that even in that scenario, the damage is contained. + +### Fixed + +- **Auth token removed from `/health` endpoint.** Token now distributed via `.auth.json` file (0o600 permissions) instead of an unauthenticated HTTP response. +- **Cookie picker data routes now require Bearer auth.** The HTML picker page is still open (it's the UI shell), but all data and action endpoints check the token. +- **CORS tightened on `/refs` and `/activity/*`.** Removed wildcard origin header so websites can't read browse activity cross-origin. +- **State files auto-expire after 7 days.** Cookie state files now include a timestamp and warn on load if stale. Server startup cleans up files older than 7 days. +- **Extension uses `textContent` instead of `innerHTML`.** Prevents DOM injection if server-provided data ever contained markup. Standard defense-in-depth for browser extensions. +- **Path validation resolves symlinks before boundary checks.** `validateReadPath` now calls `realpathSync` and handles macOS `/tmp` symlink correctly. +- **Freeze hook uses portable path resolution.** POSIX-compatible (works on macOS without coreutils), fixes edge case where `/project-evil` could match a freeze boundary set to `/project`. +- **Shell config scripts validate input.** `gstack-config` rejects regex-special keys and escapes sed patterns. `gstack-telemetry-log` sanitizes branch/repo names in JSON output. + +### Added + +- 20 regression tests covering all hardening changes. + ## [0.13.0.0] - 2026-03-27 — Your Agent Can Design Now gstack can generate real UI mockups. Not ASCII art, not text descriptions of hex codes, real visual designs you can look at, compare, pick from, and iterate on. Run `/office-hours` on a UI idea and you'll get 3 visual concepts in Chrome with a comparison board where you pick your favorite, rate the others, and tell the agent what to change. diff --git a/VERSION b/VERSION index b6963e15b5e004190b56d1cffffdc1e8a3c90c1c..883dcff5e3bd9b90ca6f1a5a06a5a81a66c0177b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.13.0.0 +0.13.1.0 diff --git a/bin/gstack-config b/bin/gstack-config index 1147adddb94e1029f8756f69a5ec78786011b15b..821a342a7d70e4240c3a9ffd3adefc6998ac0f71 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -16,16 +16,28 @@ CONFIG_FILE="$STATE_DIR/config.yaml" case "${1:-}" in get) KEY="${2:?Usage: gstack-config get }" - grep -E "^${KEY}:" "$CONFIG_FILE" 2>/dev/null | tail -1 | awk '{print $2}' | tr -d '[:space:]' || true + # Validate key (alphanumeric + underscore only) + if ! printf '%s' "$KEY" | grep -qE '^[a-zA-Z0-9_]+$'; then + echo "Error: key must contain only alphanumeric characters and underscores" >&2 + exit 1 + fi + grep -F "${KEY}:" "$CONFIG_FILE" 2>/dev/null | tail -1 | awk '{print $2}' | tr -d '[:space:]' || true ;; set) KEY="${2:?Usage: gstack-config set }" VALUE="${3:?Usage: gstack-config set }" + # Validate key (alphanumeric + underscore only) + if ! printf '%s' "$KEY" | grep -qE '^[a-zA-Z0-9_]+$'; then + echo "Error: key must contain only alphanumeric characters and underscores" >&2 + exit 1 + fi mkdir -p "$STATE_DIR" - if grep -qE "^${KEY}:" "$CONFIG_FILE" 2>/dev/null; then + # Escape sed special chars in value and drop embedded newlines + ESC_VALUE="$(printf '%s' "$VALUE" | head -1 | sed 's/[&/\]/\\&/g')" + if grep -qF "${KEY}:" "$CONFIG_FILE" 2>/dev/null; then # Portable in-place edit (BSD sed uses -i '', GNU sed uses -i without arg) _tmpfile="$(mktemp "${CONFIG_FILE}.XXXXXX")" - sed "s/^${KEY}:.*/${KEY}: ${VALUE}/" "$CONFIG_FILE" > "$_tmpfile" && mv "$_tmpfile" "$CONFIG_FILE" + sed "s/^${KEY}:.*/${KEY}: ${ESC_VALUE}/" "$CONFIG_FILE" > "$_tmpfile" && mv "$_tmpfile" "$CONFIG_FILE" else echo "${KEY}: ${VALUE}" >> "$CONFIG_FILE" fi diff --git a/bin/gstack-telemetry-log b/bin/gstack-telemetry-log index da371c38bd920dae91b667101339a042eb2e333e..93db82077aa199806ea6dcbe5592ad5243f09e46 100755 --- a/bin/gstack-telemetry-log +++ b/bin/gstack-telemetry-log @@ -158,6 +158,8 @@ OUTCOME="$(json_safe "$OUTCOME")" SESSION_ID="$(json_safe "$SESSION_ID")" SOURCE="$(json_safe "$SOURCE")" EVENT_TYPE="$(json_safe "$EVENT_TYPE")" +REPO_SLUG="$(json_safe "$REPO_SLUG")" +BRANCH="$(json_safe "$BRANCH")" # Escape null fields — sanitize ERROR_CLASS and FAILED_STEP via json_safe() ERR_FIELD="null" diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 1ef58e36ada19b4b014755e1c19ddb5dab75c9d9..a6eda991ba1074c669f448bc0973a49f2aa1c618 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -211,7 +211,7 @@ export class BrowserManager { * The browser launches headed with a visible window — the user sees * every action Claude takes in real time. */ - async launchHeaded(): Promise { + async launchHeaded(authToken?: string): Promise { // Clear old state before repopulating this.pages.clear(); this.refMap.clear(); @@ -223,6 +223,17 @@ export class BrowserManager { if (extensionPath) { launchArgs.push(`--disable-extensions-except=${extensionPath}`); launchArgs.push(`--load-extension=${extensionPath}`); + // Write auth token for extension bootstrap (read via chrome.runtime.getURL) + if (authToken) { + const fs = require('fs'); + const path = require('path'); + const authFile = path.join(extensionPath, '.auth.json'); + try { + fs.writeFileSync(authFile, JSON.stringify({ token: authToken }), { mode: 0o600 }); + } catch (err: any) { + console.warn(`[browse] Could not write .auth.json: ${err.message}`); + } + } } // Launch headed Chromium via Playwright's persistent context. @@ -751,6 +762,20 @@ export class BrowserManager { if (extensionPath) { launchArgs.push(`--disable-extensions-except=${extensionPath}`); launchArgs.push(`--load-extension=${extensionPath}`); + // Write auth token for extension bootstrap during handoff + if (this.serverPort) { + try { + const { resolveConfig } = require('./config'); + const config = resolveConfig(); + const stateFile = path.join(config.stateDir, 'browse.json'); + if (fs.existsSync(stateFile)) { + const stateData = JSON.parse(fs.readFileSync(stateFile, 'utf-8')); + if (stateData.token) { + fs.writeFileSync(path.join(extensionPath, '.auth.json'), JSON.stringify({ token: stateData.token }), { mode: 0o600 }); + } + } + } catch {} + } console.log(`[browse] Handoff: loading extension from ${extensionPath}`); } else { console.log('[browse] Handoff: extension not found — headed mode without side panel'); diff --git a/browse/src/cookie-picker-routes.ts b/browse/src/cookie-picker-routes.ts index 0e6972484510bcf0d0ad2776585c40e223492138..f36a666000b9621c831640db816d68a801c19f70 100644 --- a/browse/src/cookie-picker-routes.ts +++ b/browse/src/cookie-picker-routes.ts @@ -53,6 +53,7 @@ export async function handleCookiePickerRoute( url: URL, req: Request, bm: BrowserManager, + authToken?: string, ): Promise { const pathname = url.pathname; const port = parseInt(url.port, 10) || 9400; @@ -64,7 +65,7 @@ export async function handleCookiePickerRoute( headers: { 'Access-Control-Allow-Origin': corsOrigin(port), 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type', + 'Access-Control-Allow-Headers': 'Content-Type, Authorization', }, }); } @@ -72,13 +73,24 @@ export async function handleCookiePickerRoute( try { // GET /cookie-picker — serve the picker UI if (pathname === '/cookie-picker' && req.method === 'GET') { - const html = getCookiePickerHTML(port); + const html = getCookiePickerHTML(port, authToken); return new Response(html, { status: 200, headers: { 'Content-Type': 'text/html; charset=utf-8' }, }); } + // ─── Auth gate: all data/action routes below require Bearer token ─── + if (authToken) { + const authHeader = req.headers.get('authorization'); + if (!authHeader || authHeader !== `Bearer ${authToken}`) { + return new Response(JSON.stringify({ error: 'Unauthorized' }), { + status: 401, + headers: { 'Content-Type': 'application/json' }, + }); + } + } + // GET /cookie-picker/browsers — list installed browsers if (pathname === '/cookie-picker/browsers' && req.method === 'GET') { const browsers = findInstalledBrowsers(); diff --git a/browse/src/cookie-picker-ui.ts b/browse/src/cookie-picker-ui.ts index 381cf2e2f2f997369aa31fe8488f8726f64159b2..70faa5621a6d4359bb54d6f258b7c7983d86e406 100644 --- a/browse/src/cookie-picker-ui.ts +++ b/browse/src/cookie-picker-ui.ts @@ -7,7 +7,7 @@ * No cookie values exposed anywhere. */ -export function getCookiePickerHTML(serverPort: number): string { +export function getCookiePickerHTML(serverPort: number, authToken?: string): string { const baseUrl = `http://127.0.0.1:${serverPort}`; return ` @@ -330,6 +330,7 @@ export function getCookiePickerHTML(serverPort: number): string {