diff --git a/.github/docker/Dockerfile.ci b/.github/docker/Dockerfile.ci index 038b2576..1048bb47 100644 --- a/.github/docker/Dockerfile.ci +++ b/.github/docker/Dockerfile.ci @@ -59,5 +59,4 @@ RUN useradd -m -s /bin/bash runner \ && chmod -R a+rX /opt/node_modules_cache \ && mkdir -p /home/runner/.gstack && chown -R runner:runner /home/runner/.gstack \ && chmod 1777 /tmp \ - && mkdir -p /home/runner/.bun && chown -R runner:runner /home/runner/.bun \ - && chmod -R 1777 /tmp + && mkdir -p /home/runner/.bun && chown -R runner:runner /home/runner/.bun diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b5965ca..061888ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,29 @@ # Changelog +## [0.16.4.0] - 2026-04-13 + +### Added +- **Cookie origin pinning.** When you import cookies for specific domains, JS execution is now blocked on pages that don't match those domains. This prevents the attack where a prompt injection navigates to an attacker's site and runs `document.cookie` to steal your imported cookies. Subdomain matching works automatically (importing `.github.com` allows `api.github.com`). When no cookies are imported, everything works as before. 3 PRs from @halbert04. +- **Command audit log.** Every browse command now gets a persistent forensic trail in `~/.gstack/.browse/browse-audit.jsonl`. Timestamp, command, args, page origin, duration, status, error, and whether cookies were imported. Append-only, never truncated, survives server restarts. Best-effort writes that never block command execution. From @halbert04. +- **Cookie domain tracking.** gstack now tracks which domains cookies were imported from. Foundation for origin pinning above. Direct imports via `--domain` track automatically. New `--all` flag makes full-browser cookie import an explicit opt-in instead of the default. + +### Fixed +- **Symlink bypass in file writes.** `validateOutputPath` only checked the parent directory for symlinks, not the file itself. A symlink at `/tmp/evil.png` pointing to `/etc/crontab` passed validation because the parent `/tmp` was safe. Now checks the file with `lstatSync` before writing. From @Hybirdss. +- **Cookie-import path bypass.** Two issues: relative paths bypassed all validation (the `path.isAbsolute()` gate let `sensitive-file.json` through), and symlink resolution was missing (`path.resolve` without `realpathSync`). Now resolves to absolute, resolves symlinks, and checks against safe directories. From @urbantech. +- **Shell injection in setup scripts.** `gstack-settings-hook` interpolated file paths directly into `bun -e` JavaScript blocks. A path with quotes broke the JS string context. Now uses environment variables (`process.env`). Systematic audit confirmed only this script was vulnerable. From @garagon. +- **Form field credential leak.** Snapshot redaction only applied to `type="password"` fields. Hidden and text fields named `csrf_token`, `api_key`, `session_id` were exposed unredacted in LLM context. Now checks field name and id against sensitive patterns. From @garagon. +- **Learnings prompt injection.** Three fixes: input validation (type/key/confidence allowlists), injection pattern detection in insight field (blocks "ignore previous instructions" etc.), and cross-project trust gate (only user-stated learnings cross project boundaries). From @Ziadstr. +- **IPv6 metadata bypass.** The URL constructor normalizes `::ffff:169.254.169.254` to `::ffff:a9fe:a9fe` (hex), which wasn't in the blocklist. Added both hex-encoded forms. From @mehmoodosman. +- **Session files world-readable.** Design session files in `/tmp` were created with default permissions (0644). Now 0600 (owner-only). From @garagon. +- **Frozen lockfile in setup.** `bun install` now uses `--frozen-lockfile` to prevent supply chain attacks via floating semver ranges. From @halbert04. +- **Dockerfile chmod fix.** Removed duplicate recursive `chmod -R 1777 /tmp` (recursive sticky bit on files has no defined behavior). From @Gonzih. +- **Hardcoded /tmp in cookie import.** `cookie-import-browser` used `/tmp` directly instead of `os.tmpdir()`, breaking Windows support. + +### Security +- Closed 14 security issues (#665-#675, #566, #479, #467, #545) that were fixed in prior waves but still open on GitHub. +- Closed 17 community security PRs with thank-you messages and commit references. +- Security wave 3: 12 fixes, 7 contributors. Big thanks to @Hybirdss, @urbantech, @garagon, @Ziadstr, @halbert04, @mehmoodosman, @Gonzih. + ## [0.16.3.0] - 2026-04-09 ### Changed diff --git a/bin/gstack-learnings-log b/bin/gstack-learnings-log index e63c14cb..6c528d3a 100755 --- a/bin/gstack-learnings-log +++ b/bin/gstack-learnings-log @@ -12,19 +12,75 @@ mkdir -p "$GSTACK_HOME/projects/$SLUG" INPUT="$1" -# Validate: input must be parseable JSON -if ! printf '%s' "$INPUT" | bun -e "JSON.parse(await Bun.stdin.text())" 2>/dev/null; then - echo "gstack-learnings-log: invalid JSON, skipping" >&2 +# Validate and sanitize input +VALIDATED=$(printf '%s' "$INPUT" | bun -e " +const raw = await Bun.stdin.text(); +let j; +try { j = JSON.parse(raw); } catch { process.stderr.write('gstack-learnings-log: invalid JSON, skipping\n'); process.exit(1); } + +// Field validation: type must be from allowed list +const ALLOWED_TYPES = ['pattern', 'pitfall', 'preference', 'architecture', 'tool', 'operational']; +if (!j.type || !ALLOWED_TYPES.includes(j.type)) { + process.stderr.write('gstack-learnings-log: invalid type \"' + (j.type || '') + '\", must be one of: ' + ALLOWED_TYPES.join(', ') + '\n'); + process.exit(1); +} + +// Field validation: key must be alphanumeric, hyphens, underscores (no injection surface) +if (!j.key || !/^[a-zA-Z0-9_-]+$/.test(j.key)) { + process.stderr.write('gstack-learnings-log: invalid key, must be alphanumeric with hyphens/underscores only\n'); + process.exit(1); +} + +// Field validation: confidence must be 1-10 +const conf = Number(j.confidence); +if (!Number.isInteger(conf) || conf < 1 || conf > 10) { + process.stderr.write('gstack-learnings-log: confidence must be integer 1-10\n'); + process.exit(1); +} +j.confidence = conf; + +// Field validation: source must be from allowed list +const ALLOWED_SOURCES = ['observed', 'user-stated', 'inferred', 'cross-model']; +if (j.source && !ALLOWED_SOURCES.includes(j.source)) { + process.stderr.write('gstack-learnings-log: invalid source, must be one of: ' + ALLOWED_SOURCES.join(', ') + '\n'); + process.exit(1); +} + +// Content sanitization: strip instruction-like patterns from insight field +// These patterns could be used for prompt injection when learnings are loaded into agent context +if (j.insight) { + const INJECTION_PATTERNS = [ + /ignore\s+(all\s+)?previous\s+(instructions|context|rules)/i, + /you\s+are\s+now\s+/i, + /always\s+output\s+no\s+findings/i, + /skip\s+(all\s+)?(security|review|checks)/i, + /override[:\s]/i, + /\bsystem\s*:/i, + /\bassistant\s*:/i, + /\buser\s*:/i, + /do\s+not\s+(report|flag|mention)/i, + /approve\s+(all|every|this)/i, + ]; + for (const pat of INJECTION_PATTERNS) { + if (pat.test(j.insight)) { + process.stderr.write('gstack-learnings-log: insight contains suspicious instruction-like content, rejected\n'); + process.exit(1); + } + } +} + +// Inject timestamp if not present +if (!j.ts) j.ts = new Date().toISOString(); + +// Mark trust level based on source +// user-stated = user explicitly told the agent this. All others are AI-generated. +j.trusted = j.source === 'user-stated'; + +console.log(JSON.stringify(j)); +" 2>/dev/null) + +if [ $? -ne 0 ] || [ -z "$VALIDATED" ]; then exit 1 fi -# Inject timestamp if not present -if ! printf '%s' "$INPUT" | bun -e "const j=JSON.parse(await Bun.stdin.text()); if(!j.ts) process.exit(1)" 2>/dev/null; then - INPUT=$(printf '%s' "$INPUT" | bun -e " - const j = JSON.parse(await Bun.stdin.text()); - j.ts = new Date().toISOString(); - console.log(JSON.stringify(j)); - " 2>/dev/null) || true -fi - -echo "$INPUT" >> "$GSTACK_HOME/projects/$SLUG/learnings.jsonl" +echo "$VALIDATED" >> "$GSTACK_HOME/projects/$SLUG/learnings.jsonl" diff --git a/bin/gstack-learnings-search b/bin/gstack-learnings-search index 634342e6..3b39e462 100755 --- a/bin/gstack-learnings-search +++ b/bin/gstack-learnings-search @@ -68,7 +68,13 @@ for (const line of lines) { // Determine if this is from the current project or cross-project // Cross-project entries are tagged for display - e._crossProject = !line.includes(slug) && process.env.GSTACK_SEARCH_CROSS === 'true'; + const isCrossProject = !line.includes(slug) && process.env.GSTACK_SEARCH_CROSS === 'true'; + e._crossProject = isCrossProject; + + // Trust gate: cross-project learnings only loaded if trusted (user-stated) + // This prevents prompt injection from one project's AI-generated learnings + // silently influencing reviews in another project. + if (isCrossProject && e.trusted === false) continue; entries.push(e); } catch {} diff --git a/bin/gstack-settings-hook b/bin/gstack-settings-hook index 93a537f0..21445a14 100755 --- a/bin/gstack-settings-hook +++ b/bin/gstack-settings-hook @@ -26,10 +26,10 @@ fi case "$ACTION" in add) - bun -e " + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_HOOK_CMD="$HOOK_CMD" bun -e " const fs = require('fs'); - const settingsPath = '$SETTINGS_FILE'; - const hookCmd = $(printf '%s' "$HOOK_CMD" | bun -e "process.stdout.write(JSON.stringify(require('fs').readFileSync('/dev/stdin','utf8')))"); + const settingsPath = process.env.GSTACK_SETTINGS_PATH; + const hookCmd = process.env.GSTACK_HOOK_CMD; let settings = {}; try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); } catch {} @@ -55,9 +55,9 @@ case "$ACTION" in ;; remove) [ -f "$SETTINGS_FILE" ] || exit 0 - bun -e " + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun -e " const fs = require('fs'); - const settingsPath = '$SETTINGS_FILE'; + const settingsPath = process.env.GSTACK_SETTINGS_PATH; let settings = {}; try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); } catch { process.exit(0); } diff --git a/bin/gstack-team-init b/bin/gstack-team-init index 1fc08ea9..256735f8 100755 --- a/bin/gstack-team-init +++ b/bin/gstack-team-init @@ -139,9 +139,9 @@ HOOK_EOF # Add hook to project-level settings.json if command -v bun >/dev/null 2>&1; then - bun -e " + GSTACK_SETTINGS_PATH="$SETTINGS" bun -e " const fs = require('fs'); - const settingsPath = '$SETTINGS'; + const settingsPath = process.env.GSTACK_SETTINGS_PATH; let settings = {}; try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); } catch {} diff --git a/browse/src/audit.ts b/browse/src/audit.ts new file mode 100644 index 00000000..5ac59f6d --- /dev/null +++ b/browse/src/audit.ts @@ -0,0 +1,65 @@ +/** + * Persistent command audit log — forensic trail for all browse server commands. + * + * Writes append-only JSONL to .gstack/browse-audit.jsonl. Unlike the in-memory + * ring buffers (console, network, dialog), the audit log persists across server + * restarts and is never truncated by the server. Each entry records: + * + * - timestamp, command, args (truncated), page origin + * - duration, status (ok/error), error message if any + * - whether cookies were imported (elevated security context) + * - connection mode (headless/headed) + * + * All writes are best-effort — audit failures never cause command failures. + */ + +import * as fs from 'fs'; + +export interface AuditEntry { + ts: string; + cmd: string; + args: string; + origin: string; + durationMs: number; + status: 'ok' | 'error'; + error?: string; + hasCookies: boolean; + mode: 'launched' | 'headed'; +} + +const MAX_ARGS_LENGTH = 200; +const MAX_ERROR_LENGTH = 300; + +let auditPath: string | null = null; + +export function initAuditLog(logPath: string): void { + auditPath = logPath; +} + +export function writeAuditEntry(entry: AuditEntry): void { + if (!auditPath) return; + try { + const truncatedArgs = entry.args.length > MAX_ARGS_LENGTH + ? entry.args.slice(0, MAX_ARGS_LENGTH) + '…' + : entry.args; + const truncatedError = entry.error && entry.error.length > MAX_ERROR_LENGTH + ? entry.error.slice(0, MAX_ERROR_LENGTH) + '…' + : entry.error; + + const record: Record = { + ts: entry.ts, + cmd: entry.cmd, + args: truncatedArgs, + origin: entry.origin, + durationMs: entry.durationMs, + status: entry.status, + hasCookies: entry.hasCookies, + mode: entry.mode, + }; + if (truncatedError) record.error = truncatedError; + + fs.appendFileSync(auditPath, JSON.stringify(record) + '\n'); + } catch { + // Audit write failures are silent — never block command execution + } +} diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 3e7562bb..63d78358 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -55,6 +55,9 @@ export class BrowserManager { private dialogAutoAccept: boolean = true; private dialogPromptText: string | null = null; + // ─── Cookie Origin Tracking ──────────────────────────────── + private cookieImportedDomains: Set = new Set(); + // ─── Handoff State ───────────────────────────────────────── private isHeaded: boolean = false; private consecutiveFailures: number = 0; @@ -749,6 +752,19 @@ export class BrowserManager { return this.dialogPromptText; } + // ─── Cookie Origin Tracking ──────────────────────────────── + trackCookieImportDomains(domains: string[]): void { + for (const d of domains) this.cookieImportedDomains.add(d); + } + + getCookieImportedDomains(): ReadonlySet { + return this.cookieImportedDomains; + } + + hasCookieImports(): boolean { + return this.cookieImportedDomains.size > 0; + } + // ─── Viewport ────────────────────────────────────────────── async setViewport(width: number, height: number) { await this.getPage().setViewportSize({ width, height }); diff --git a/browse/src/config.ts b/browse/src/config.ts index 498c083b..65c18728 100644 --- a/browse/src/config.ts +++ b/browse/src/config.ts @@ -20,6 +20,7 @@ export interface BrowseConfig { consoleLog: string; networkLog: string; dialogLog: string; + auditLog: string; } /** @@ -70,6 +71,7 @@ export function resolveConfig( consoleLog: path.join(stateDir, 'browse-console.log'), networkLog: path.join(stateDir, 'browse-network.log'), dialogLog: path.join(stateDir, 'browse-dialog.log'), + auditLog: path.join(stateDir, 'browse-audit.jsonl'), }; } diff --git a/browse/src/cookie-import-browser.ts b/browse/src/cookie-import-browser.ts index 1e7f1ce4..7dc75e07 100644 --- a/browse/src/cookie-import-browser.ts +++ b/browse/src/cookie-import-browser.ts @@ -386,7 +386,8 @@ function openDb(dbPath: string, browserName: string): Database { } function openDbFromCopy(dbPath: string, browserName: string): Database { - const tmpPath = `/tmp/browse-cookies-${browserName.toLowerCase()}-${crypto.randomUUID()}.db`; + // Use os.tmpdir() instead of hardcoded /tmp for cross-platform support (#708) + const tmpPath = path.join(os.tmpdir(), `browse-cookies-${browserName.toLowerCase()}-${crypto.randomUUID()}.db`); try { fs.copyFileSync(dbPath, tmpPath); // Also copy WAL and SHM if they exist (for consistent reads) diff --git a/browse/src/path-security.ts b/browse/src/path-security.ts index 4b1961b0..cb6b1e08 100644 --- a/browse/src/path-security.ts +++ b/browse/src/path-security.ts @@ -33,7 +33,26 @@ const TEMP_ONLY = [TEMP_DIR].map(d => { export function validateOutputPath(filePath: string): void { const resolved = path.resolve(filePath); - // Resolve real path of the parent directory to catch symlinks. + // If the target already exists and is a symlink, resolve through it. + // Without this, a symlink at /tmp/evil.png → /etc/crontab passes the + // parent-directory check (parent is /tmp, which is safe) but the actual + // write follows the symlink to /etc/crontab. + try { + const stat = fs.lstatSync(resolved); + if (stat.isSymbolicLink()) { + const realTarget = fs.realpathSync(resolved); + const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realTarget, dir)); + if (!isSafe) { + throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); + } + return; // symlink target verified, no need to check parent + } + } catch (e: any) { + // ENOENT = file doesn't exist yet, fall through to parent-dir check + if (e.code !== 'ENOENT') throw e; + } + + // For new files (no existing symlink), verify the parent directory. // The file itself may not exist yet (e.g., screenshot output). // This also handles macOS /tmp → /private/tmp transparently. let dir = path.dirname(resolved); diff --git a/browse/src/read-commands.ts b/browse/src/read-commands.ts index 746b0959..367770ee 100644 --- a/browse/src/read-commands.ts +++ b/browse/src/read-commands.ts @@ -6,6 +6,7 @@ */ import type { TabSession } from './tab-session'; +import type { BrowserManager } from './browser-manager'; import { consoleBuffer, networkBuffer, dialogBuffer } from './buffers'; import type { Page, Frame } from 'playwright'; import * as fs from 'fs'; @@ -62,10 +63,43 @@ export async function getCleanText(page: Page | Frame): Promise { }); } +/** + * When cookies have been imported for specific domains, block JS execution + * on pages whose origin doesn't match any imported cookie domain. + * Prevents cross-origin cookie exfiltration via `js document.cookie` or + * similar when the agent navigates to an untrusted page. + */ +function assertJsOriginAllowed(bm: BrowserManager, pageUrl: string): void { + if (!bm.hasCookieImports()) return; + + let hostname: string; + try { + hostname = new URL(pageUrl).hostname; + } catch { + return; // about:blank, data: URIs — allow (no cookies at risk) + } + + const importedDomains = bm.getCookieImportedDomains(); + const allowed = [...importedDomains].some(domain => { + // Exact match or subdomain match (e.g., ".github.com" matches "api.github.com") + const normalized = domain.startsWith('.') ? domain : '.' + domain; + return hostname === domain.replace(/^\./, '') || hostname.endsWith(normalized); + }); + + if (!allowed) { + throw new Error( + `JS execution blocked: current page (${hostname}) does not match any cookie-imported domain. ` + + `Imported cookies for: ${[...importedDomains].join(', ')}. ` + + `This prevents cross-origin cookie exfiltration. Navigate to an imported domain or run without imported cookies.` + ); + } +} + export async function handleReadCommand( command: string, args: string[], - session: TabSession + session: TabSession, + bm?: BrowserManager, ): Promise { const page = session.getPage(); // Frame-aware target for content extraction @@ -116,7 +150,10 @@ export async function handleReadCommand( id: input.id || undefined, placeholder: input.placeholder || undefined, required: input.required || undefined, - value: input.type === 'password' ? '[redacted]' : (input.value || undefined), + value: input.type === 'password' + || (input.name && /(^|[_.-])(token|secret|key|password|credential|auth|jwt|session|csrf|sid)($|[_.-])|api.?key/i.test(input.name)) + || (input.id && /(^|[_.-])(token|secret|key|password|credential|auth|jwt|session|csrf|sid)($|[_.-])|api.?key/i.test(input.id)) + ? '[redacted]' : (input.value || undefined), options: el.tagName === 'SELECT' ? [...(el as HTMLSelectElement).options].map(o => ({ value: o.value, text: o.text })) : undefined, @@ -142,6 +179,7 @@ export async function handleReadCommand( case 'js': { const expr = args[0]; if (!expr) throw new Error('Usage: browse js '); + if (bm) assertJsOriginAllowed(bm, page.url()); const wrapped = wrapForEvaluate(expr); const result = await target.evaluate(wrapped); return typeof result === 'object' ? JSON.stringify(result, null, 2) : String(result ?? ''); @@ -150,6 +188,7 @@ export async function handleReadCommand( case 'eval': { const filePath = args[0]; if (!filePath) throw new Error('Usage: browse eval '); + if (bm) assertJsOriginAllowed(bm, page.url()); validateReadPath(filePath); if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`); const code = fs.readFileSync(filePath, 'utf-8'); diff --git a/browse/src/server.ts b/browse/src/server.ts index c370ecc7..98f43af0 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -35,6 +35,7 @@ import { import { validateTempPath } from './path-security'; import { resolveConfig, ensureStateDir, readVersionHash } from './config'; import { emitActivity, subscribe, getActivityAfter, getActivityHistory, getSubscriberCount } from './activity'; +import { initAuditLog, writeAuditEntry } from './audit'; import { inspectElement, modifyStyle, resetModifications, getModificationHistory, detachSession, type InspectorResult } from './cdp-inspector'; // Bun.spawn used instead of child_process.spawn (compiled bun binaries // fail posix_spawn on all executables including /bin/bash) @@ -47,6 +48,7 @@ import * as crypto from 'crypto'; // ─── Config ───────────────────────────────────────────────────── const config = resolveConfig(); ensureStateDir(config); +initAuditLog(config.auditLog); // ─── Auth ─────────────────────────────────────────────────────── const AUTH_TOKEN = crypto.randomUUID(); @@ -1013,7 +1015,7 @@ async function handleCommandInternal( await cleanupHiddenMarkers(page); } } else { - result = await handleReadCommand(command, args, session); + result = await handleReadCommand(command, args, session, browserManager); } } else if (WRITE_COMMANDS.has(command)) { result = await handleWriteCommand(command, args, session, browserManager); @@ -1088,13 +1090,14 @@ async function handleCommandInternal( } // Activity: emit command_end (skipped for chain subcommands) + const successDuration = Date.now() - startTime; if (!opts?.skipActivity) { emitActivity({ type: 'command_end', command, args, url: browserManager.getCurrentUrl(), - duration: Date.now() - startTime, + duration: successDuration, status: 'ok', result: result, tabs: browserManager.getTabCount(), @@ -1103,6 +1106,17 @@ async function handleCommandInternal( }); } + writeAuditEntry({ + ts: new Date().toISOString(), + cmd: command, + args: args.join(' '), + origin: browserManager.getCurrentUrl(), + durationMs: successDuration, + status: 'ok', + hasCookies: browserManager.hasCookieImports(), + mode: browserManager.getConnectionMode(), + }); + browserManager.resetFailures(); // Restore original active tab if we pinned to a specific one if (savedTabId !== null) { @@ -1120,13 +1134,14 @@ async function handleCommandInternal( } // Activity: emit command_end (error) — skipped for chain subcommands + const errorDuration = Date.now() - startTime; if (!opts?.skipActivity) { emitActivity({ type: 'command_end', command, args, url: browserManager.getCurrentUrl(), - duration: Date.now() - startTime, + duration: errorDuration, status: 'error', error: err.message, tabs: browserManager.getTabCount(), @@ -1135,6 +1150,18 @@ async function handleCommandInternal( }); } + writeAuditEntry({ + ts: new Date().toISOString(), + cmd: command, + args: args.join(' '), + origin: browserManager.getCurrentUrl(), + durationMs: errorDuration, + status: 'error', + error: err.message, + hasCookies: browserManager.hasCookieImports(), + mode: browserManager.getConnectionMode(), + }); + browserManager.incrementFailures(); let errorMsg = wrapError(err); const hint = browserManager.getFailureHint(); diff --git a/browse/src/url-validation.ts b/browse/src/url-validation.ts index 5d37cf0d..ddac0d5a 100644 --- a/browse/src/url-validation.ts +++ b/browse/src/url-validation.ts @@ -7,6 +7,8 @@ export const BLOCKED_METADATA_HOSTS = new Set([ '169.254.169.254', // AWS/GCP/Azure instance metadata 'fe80::1', // IPv6 link-local — common metadata endpoint alias '::ffff:169.254.169.254', // IPv4-mapped IPv6 form of the metadata IP + '::ffff:a9fe:a9fe', // Hex-encoded IPv4-mapped form (URL constructor normalizes to this) + '::a9fe:a9fe', // Deprecated IPv4-compatible hex form 'metadata.google.internal', // GCP metadata 'metadata.azure.internal', // Azure IMDS ]); diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index 432b6d58..779a858e 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -13,7 +13,8 @@ import { validateNavigationUrl } from './url-validation'; import { validateOutputPath } from './path-security'; import * as fs from 'fs'; import * as path from 'path'; -import { TEMP_DIR } from './platform'; +import { TEMP_DIR, isPathWithin } from './platform'; +import { SAFE_DIRECTORIES } from './path-security'; import { modifyStyle, undoModification, resetModifications, getModificationHistory } from './cdp-inspector'; /** @@ -441,16 +442,17 @@ export async function handleWriteCommand( case 'cookie-import': { const filePath = args[0]; if (!filePath) throw new Error('Usage: browse cookie-import '); - // Path validation — prevent reading arbitrary files - if (path.isAbsolute(filePath)) { - const safeDirs = [TEMP_DIR, process.cwd()]; - const resolved = path.resolve(filePath); - if (!safeDirs.some(dir => isPathWithin(resolved, dir))) { - throw new Error(`Path must be within: ${safeDirs.join(', ')}`); - } + // Path validation — resolve to absolute and check against safe dirs. + // Fixes #707: relative paths previously bypassed the safe directory check. + // Mirrors validateOutputPath() — resolves symlinks (e.g., macOS /tmp → /private/tmp). + const resolved = path.resolve(filePath); + let resolvedReal = resolved; + try { resolvedReal = fs.realpathSync(resolved); } catch { + // File may not exist yet — resolve parent dir instead + try { resolvedReal = path.join(fs.realpathSync(path.dirname(resolved)), path.basename(resolved)); } catch {} } - if (path.normalize(filePath).includes('..')) { - throw new Error('Path traversal sequences (..) are not allowed'); + if (!SAFE_DIRECTORIES.some(dir => isPathWithin(resolvedReal, dir))) { + throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); } if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`); const raw = fs.readFileSync(filePath, 'utf-8'); @@ -476,20 +478,24 @@ export async function handleWriteCommand( } await page.context().addCookies(cookies); + const importedDomains = [...new Set(cookies.map((c: any) => c.domain).filter(Boolean))]; + if (importedDomains.length > 0) bm.trackCookieImportDomains(importedDomains); return `Loaded ${cookies.length} cookies from ${filePath}`; } case 'cookie-import-browser': { // Two modes: // 1. Direct CLI import: cookie-import-browser --domain [--profile ] - // 2. Open picker UI: cookie-import-browser [browser] + // Requires --domain (or --all to explicitly import everything). + // 2. Open picker UI: cookie-import-browser [browser] (interactive domain selection) const browserArg = args[0]; const domainIdx = args.indexOf('--domain'); const profileIdx = args.indexOf('--profile'); + const hasAll = args.includes('--all'); const profile = (profileIdx !== -1 && profileIdx + 1 < args.length) ? args[profileIdx + 1] : 'Default'; if (domainIdx !== -1 && domainIdx + 1 < args.length) { - // Direct import mode — no UI + // Direct import mode — scoped to specific domain const domain = args[domainIdx + 1]; // Validate --domain against current page hostname to prevent cross-site cookie injection const pageHostname = new URL(page.url()).hostname; @@ -501,13 +507,35 @@ export async function handleWriteCommand( const result = await importCookies(browser, [domain], profile); if (result.cookies.length > 0) { await page.context().addCookies(result.cookies); + bm.trackCookieImportDomains([domain]); } const msg = [`Imported ${result.count} cookies for ${domain} from ${browser}`]; if (result.failed > 0) msg.push(`(${result.failed} failed to decrypt)`); return msg.join(' '); } - // Picker UI mode — open in user's browser + if (hasAll) { + // Explicit all-cookies import — requires --all flag as a deliberate opt-in. + // Imports every non-expired cookie domain from the browser. + const browser = browserArg || 'comet'; + const { listDomains } = await import('./cookie-import-browser'); + const { domains } = listDomains(browser, profile); + const allDomainNames = domains.map((d: any) => d.domain); + if (allDomainNames.length === 0) { + return `No cookies found in ${browser} (profile: ${profile})`; + } + const result = await importCookies(browser, allDomainNames, profile); + if (result.cookies.length > 0) { + await page.context().addCookies(result.cookies); + bm.trackCookieImportDomains(allDomainNames); + } + const msg = [`Imported ${result.count} cookies across ${Object.keys(result.domainCounts).length} domains from ${browser}`]; + msg.push('(used --all: all browser cookies imported, consider --domain for tighter scoping)'); + if (result.failed > 0) msg.push(`(${result.failed} failed to decrypt)`); + return msg.join(' '); + } + + // Picker UI mode — open in user's browser for interactive domain selection const port = bm.serverPort; if (!port) throw new Error('Server port not available'); @@ -525,7 +553,7 @@ export async function handleWriteCommand( if (err?.code !== 'ENOENT' && !err?.message?.includes('spawn')) throw err; } - return `Cookie picker opened at http://127.0.0.1:${port}/cookie-picker\nDetected browsers: ${browsers.map(b => b.name).join(', ')}\nSelect domains to import, then close the picker when done.`; + return `Cookie picker opened at http://127.0.0.1:${port}/cookie-picker\nDetected browsers: ${browsers.map(b => b.name).join(', ')}\nSelect domains to import, then close the picker when done.\n\nTip: For scripted imports, use --domain to scope cookies to a single domain.`; } case 'style': { diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 8434e2ef..2c006955 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -1811,7 +1811,8 @@ describe('Path traversal prevention', () => { await handleWriteCommand('cookie-import', ['../../etc/shadow'], bm); expect(true).toBe(false); } catch (err: any) { - expect(err.message).toContain('Path traversal'); + // Traversal blocked by safe-directory check (#707) or explicit .. check + expect(err.message).toMatch(/Path must be within|Path traversal/); } }); diff --git a/design/src/session.ts b/design/src/session.ts index 16d6f0ee..01986618 100644 --- a/design/src/session.ts +++ b/design/src/session.ts @@ -49,7 +49,7 @@ export function createSession( updatedAt: new Date().toISOString(), }; - fs.writeFileSync(sessionPath(id), JSON.stringify(session, null, 2)); + fs.writeFileSync(sessionPath(id), JSON.stringify(session, null, 2), { mode: 0o600 }); return session; } diff --git a/setup b/setup index f71f4552..1611a454 100755 --- a/setup +++ b/setup @@ -208,7 +208,7 @@ if [ "$NEEDS_BUILD" -eq 1 ]; then log "Building browse binary..." ( cd "$SOURCE_GSTACK_DIR" - bun install + bun install --frozen-lockfile 2>/dev/null || bun install bun run build ) # Safety net: write .version if build script didn't (e.g., git not available during build)