From ef186cccb33f1b6c631b034793b04fefe6850d67 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 18:50:01 -0700 Subject: [PATCH] fix(security): delete dead exports the ripped chat path left behind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three-way split by importer class: (a) Zero importers, deleted: the whole attack-attempt logging cluster in security.ts (logAttempt, AttemptRecord, salted hashPayload + device-salt, attempts.jsonl rotation, telemetry spawn plumbing incl. buildTelemetrySpawnCommand/resolveBashBinary — the LIVE attempts.jsonl writer is tunnel-denial-log.ts with its own rotation); the decision-file handshake (writeDecision/readDecision/clearDecision/excerptForReview — written for sidebar-agent's poll loop, which no longer exists); sidebar-utils.ts (whole module — its sanitizeExtensionUrl 'sanitized before embedding in a prompt' for the deleted prompt builder); 8 dead server.ts imports (sanitizeExtensionUrl, generateCanary, injectCanary, writeDecision, rotateRoot, serializeRegistry, restoreRegistry, clearAgentRecord); buildPtyClearCookie + buildSseClearCookie; WEBDRIVER_MASK_SCRIPT (orphaned by the D7 stealth narrowing — applyStealth never used it). (b) Dead-pin tests edited with their exports: the 'still exported' pin in stealth-layer-c, the string-content describe in stealth-webdriver (its live applyStealth behavioral coverage untouched), the clear-cookie assertions, security-review-flow.test.ts deleted whole (all 4 describes exercised the dead decision mechanism, incl. a 'simulated sidebar-agent poll loop'). (c) KEPT deliberately: leaseCount (live behavioral coverage), extractPtyCookie + validatePtySessionToken (extractPtyCookie is adopted by the terminal-agent cookie-parse unification later in this wave), resetSessionMarker + clearContentFilters (test-support API for the live content-security layer). Also fixes two pre-existing red pins found while here, invisible until the free suite got a CI job: the v1.44 spawnClaude->maybeSpawnPty rename in terminal-agent.test.ts, and a cross-file test-isolation bug where content-security.test.ts's clearContentFilters() wiped the auto-registered url-blocklist filter for every later file in the same bun process (security-integration.test.ts failed on co-run; afterAll now restores it). Co-Authored-By: Claude Fable 5 --- browse/src/pty-session-cookie.ts | 5 - browse/src/security.ts | 289 +---------------------- browse/src/server.ts | 7 +- browse/src/sidebar-utils.ts | 21 -- browse/src/sse-session-cookie.ts | 5 - browse/src/stealth.ts | 8 - browse/test/content-security.test.ts | 10 + browse/test/security-review-flow.test.ts | 194 --------------- browse/test/security.test.ts | 109 --------- browse/test/sidebar-unit.test.ts | 96 -------- browse/test/sse-session-cookie.test.ts | 7 +- browse/test/stealth-layer-c.test.ts | 5 - browse/test/stealth-webdriver.test.ts | 16 +- browse/test/terminal-agent.test.ts | 13 +- 14 files changed, 26 insertions(+), 759 deletions(-) delete mode 100644 browse/src/sidebar-utils.ts delete mode 100644 browse/test/security-review-flow.test.ts delete mode 100644 browse/test/sidebar-unit.test.ts diff --git a/browse/src/pty-session-cookie.ts b/browse/src/pty-session-cookie.ts index 8871fe471..cfeb97104 100644 --- a/browse/src/pty-session-cookie.ts +++ b/browse/src/pty-session-cookie.ts @@ -98,11 +98,6 @@ export function buildPtySetCookie(token: string): string { return `${PTY_COOKIE_NAME}=${token}; HttpOnly; SameSite=Strict; Path=/; Max-Age=${maxAge}`; } -/** Clear the PTY session cookie. */ -export function buildPtyClearCookie(): string { - return `${PTY_COOKIE_NAME}=; HttpOnly; SameSite=Strict; Path=/; Max-Age=0`; -} - function pruneExpired(now: number): void { let checked = 0; for (const [token, session] of sessions) { diff --git a/browse/src/security.ts b/browse/src/security.ts index d0e403971..1cbd0f530 100644 --- a/browse/src/security.ts +++ b/browse/src/security.ts @@ -309,222 +309,13 @@ export function checkCanaryInStructure(value: unknown, canary: string): boolean return false; } -// ─── Attack logging ────────────────────────────────────────── - -export interface AttemptRecord { - ts: string; - urlDomain: string; - payloadHash: string; - confidence: number; - layer: LayerName; - verdict: Verdict; - gstackVersion?: string; -} +// NOTE: attack-attempt logging (logAttempt + salted payload hashing + +// attempts.jsonl rotation + telemetry spawn plumbing) lived here until the +// chat-path scanner that called it was ripped with sidebar-agent.ts. The +// LIVE attempts.jsonl writer is tunnel-denial-log.ts, which owns its own +// rotation. const SECURITY_DIR = path.join(os.homedir(), '.gstack', 'security'); -const ATTEMPTS_LOG = path.join(SECURITY_DIR, 'attempts.jsonl'); -const SALT_FILE = path.join(SECURITY_DIR, 'device-salt'); -const MAX_LOG_BYTES = 10 * 1024 * 1024; // 10MB rotate threshold (eng review 4.1) -const MAX_LOG_GENERATIONS = 5; - -/** - * Read-or-create the per-device salt used for payload hashing. Salt lives at - * ~/.gstack/security/device-salt (0600). Random per-device, prevents rainbow - * table attacks across devices (Codex tier-2 finding). - */ -let cachedSalt: string | null = null; - -function getDeviceSalt(): string { - if (cachedSalt) return cachedSalt; - try { - if (fs.existsSync(SALT_FILE)) { - cachedSalt = fs.readFileSync(SALT_FILE, 'utf8').trim(); - return cachedSalt; - } - } catch { - // fall through to generate - } - try { - mkdirSecure(SECURITY_DIR); - } catch {} - cachedSalt = randomBytes(16).toString('hex'); - try { - writeSecureFile(SALT_FILE, cachedSalt); - } catch { - // Can't persist (read-only fs, disk full). Keep the in-memory salt - // for this process so cross-log correlation still works within a - // session. Next process gets a new salt, but that's a degraded-mode - // acceptable cost. - } - return cachedSalt; -} - -export function hashPayload(payload: string): string { - const salt = getDeviceSalt(); - return createHash('sha256').update(salt).update(payload).digest('hex'); -} - -/** - * Rotate attempts.jsonl when it exceeds 10MB. Keeps 5 generations. - */ -function rotateIfNeeded(): void { - try { - const st = fs.statSync(ATTEMPTS_LOG); - if (st.size < MAX_LOG_BYTES) return; - } catch { - return; // doesn't exist, nothing to rotate - } - // Shift .N -> .N+1, drop oldest - for (let i = MAX_LOG_GENERATIONS - 1; i >= 1; i--) { - const src = `${ATTEMPTS_LOG}.${i}`; - const dst = `${ATTEMPTS_LOG}.${i + 1}`; - try { - if (fs.existsSync(src)) fs.renameSync(src, dst); - } catch {} - } - try { - fs.renameSync(ATTEMPTS_LOG, `${ATTEMPTS_LOG}.1`); - } catch {} -} - -/** - * Try to locate the gstack-telemetry-log binary. Resolution order matches - * the existing skill preamble pattern (never relies on PATH — packaged - * binary layouts can break that). - * - * Order: - * 1. ~/.claude/skills/gstack/bin/gstack-telemetry-log (global install) - * 2. .claude/skills/gstack/bin/gstack-telemetry-log (symlinked dev) - * 3. bin/gstack-telemetry-log (in-repo dev) - */ -function findTelemetryBinary(): string | null { - const candidates = [ - path.join(os.homedir(), '.claude', 'skills', 'gstack', 'bin', 'gstack-telemetry-log'), - path.resolve(process.cwd(), '.claude', 'skills', 'gstack', 'bin', 'gstack-telemetry-log'), - path.resolve(process.cwd(), 'bin', 'gstack-telemetry-log'), - ]; - for (const c of candidates) { - try { - fs.accessSync(c, fs.constants.X_OK); - return c; - } catch { - // try next - } - } - return null; -} - -/** - * Resolve a bash binary for invoking shebang scripts on Windows. Mirrors the - * GSTACK_*_BIN override pattern from `browse/src/claude-bin.ts:resolveClaudeCommand` - * (introduced in v1.24.0.0 #1252) so users on WSL/MSYS2/non-default Git Bash - * installs can redirect. - * - * Override precedence: - * 1. GSTACK_BASH_BIN (or BASH_BIN) — absolute path or PATH-resolvable command. - * 2. Plain Bun.which('bash') — finds Git Bash on the standard Windows install. - * - * Returns null if nothing resolves; callers must degrade gracefully (telemetry - * already swallows spawn errors, so a null here means the local attempts.jsonl - * audit trail keeps working without surfacing a Windows-only failure). - */ -export function resolveBashBinary(env: NodeJS.ProcessEnv = process.env): string | null { - const PATH = env.PATH ?? env.Path ?? ''; - const override = (env.GSTACK_BASH_BIN ?? env.BASH_BIN)?.trim(); - if (override) { - const trimmed = override.replace(/^"(.*)"$/, '$1'); - return path.isAbsolute(trimmed) ? trimmed : (Bun.which(trimmed, { PATH }) ?? null); - } - return Bun.which('bash', { PATH }) ?? null; -} - -/** - * Build the [cmd, args] tuple for invoking a bash-script telemetry binary - * in a way that works on both POSIX and Windows. - * - * POSIX: returns [bin, args] unchanged — shebang gets honored by execve. - * Win32: wraps in bash explicitly. `gstack-telemetry-log` is a shell script - * (`#!/usr/bin/env bash`) and Windows `CreateProcess` can't dispatch on a - * shebang — it tries to load the file as a PE image, fails with ENOEXEC, - * and our 'error' handler silently swallows it. Resolves bash via the same - * Bun.which + GSTACK_*_BIN override pattern as claude-bin.ts. - * - * Returns null when bash can't be resolved on Windows (rare — Git Bash ships - * with the standard gstack install path). Caller skips spawn; the local - * attempts.jsonl write still gives the audit trail. - * - * Exported for testability — resolution is a pure function of (platform, - * env, bin, args) so we can assert on it without actually spawning. - */ -export function buildTelemetrySpawnCommand( - bin: string, - args: string[], - env: NodeJS.ProcessEnv = process.env, -): { cmd: string; cmdArgs: string[] } | null { - if (process.platform === 'win32') { - const bashPath = resolveBashBinary(env); - if (!bashPath) return null; - return { cmd: bashPath, cmdArgs: [bin, ...args] }; - } - return { cmd: bin, cmdArgs: args }; -} - -/** - * Fire-and-forget subprocess invocation of gstack-telemetry-log with the - * attack_attempt event type. The binary handles tier gating internally - * (community → upload, anonymous → local only, off → no-op), so we don't - * need to re-check here. - * - * Never throws. Never blocks. If the binary isn't found or spawn fails, the - * local attempts.jsonl write from logAttempt() still gives us the audit trail. - */ -function reportAttemptTelemetry(record: AttemptRecord): void { - const bin = findTelemetryBinary(); - if (!bin) return; - try { - const result = buildTelemetrySpawnCommand(bin, [ - '--event-type', 'attack_attempt', - '--url-domain', record.urlDomain || '', - '--payload-hash', record.payloadHash, - '--confidence', String(record.confidence), - '--layer', record.layer, - '--verdict', record.verdict, - ]); - if (!result) return; - const child = spawn(result.cmd, result.cmdArgs, { - stdio: 'ignore', - detached: true, - }); - // unref so this subprocess doesn't hold the event loop open - child.unref(); - child.on('error', () => { /* swallow — telemetry must never break sidebar */ }); - } catch { - // Spawn failure is non-fatal. - } -} - -/** - * Append an attempt to the local log AND fire telemetry via - * gstack-telemetry-log (which respects the user's telemetry tier setting). - * Never throws — logging failure should not break the sidebar. - * Returns true if the local write succeeded. - */ -export function logAttempt(record: AttemptRecord): boolean { - // Fire telemetry first, async — even if local write fails, we still want - // the event reported (it goes to a different directory anyway). - reportAttemptTelemetry(record); - try { - mkdirSecure(SECURITY_DIR); - rotateIfNeeded(); - const line = JSON.stringify(record) + '\n'; - appendSecureFile(ATTEMPTS_LOG, line); - return true; - } catch (err) { - // Non-fatal. Log to stderr for debugging but don't block. - console.error('[security] logAttempt write failed:', (err as Error).message); - return false; - } -} // ─── Cross-process session state ───────────────────────────── @@ -565,76 +356,6 @@ export function readSessionState(): SessionState | null { } } -// ─── User-in-the-loop review on BLOCK ──────────────────────── -// -// When a tool-output BLOCK fires, the user gets to see the suspected text -// and decide. The sidepanel posts to /security-decision, server writes a -// per-tab file under ~/.gstack/security/decisions/, sidebar-agent polls -// for it. File-based on purpose: sidebar-agent.ts is a separate subprocess -// and this is the same pattern the existing per-tab cancel file uses. - -const DECISIONS_DIR = path.join(SECURITY_DIR, 'decisions'); - -export type SecurityDecision = 'allow' | 'block'; - -export function decisionFileForTab(tabId: number): string { - return path.join(DECISIONS_DIR, `tab-${tabId}.json`); -} - -export interface DecisionRecord { - tabId: number; - decision: SecurityDecision; - ts: string; - reason?: string; -} - -export function writeDecision(record: DecisionRecord): void { - try { - mkdirSecure(DECISIONS_DIR); - const file = decisionFileForTab(record.tabId); - const tmp = `${file}.tmp.${process.pid}`; - writeSecureFile(tmp, JSON.stringify(record)); - fs.renameSync(tmp, file); - } catch (err) { - console.error('[security] writeDecision failed:', (err as Error).message); - } -} - -export function readDecision(tabId: number): DecisionRecord | null { - try { - const file = decisionFileForTab(tabId); - if (!fs.existsSync(file)) return null; - return JSON.parse(fs.readFileSync(file, 'utf8')); - } catch { - return null; - } -} - -export function clearDecision(tabId: number): void { - try { - const file = decisionFileForTab(tabId); - if (fs.existsSync(file)) fs.unlinkSync(file); - } catch { - // best effort - } -} - -/** - * Truncate + sanitize tool output for display in the review banner. - * - Max 500 chars (UI budget) - * - Strip control chars, collapse whitespace - * - Append "…" if truncated - */ -export function excerptForReview(text: string, max = 500): string { - if (!text) return ''; - const cleaned = text - .replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, '') - .replace(/\s+/g, ' ') - .trim(); - if (cleaned.length <= max) return cleaned; - return cleaned.slice(0, max) + '…'; -} - // ─── Status reporting (for shield icon via /health) ────────── export function getStatus(): StatusDetail { diff --git a/browse/src/server.ts b/browse/src/server.ts index a20f125be..57b989cdf 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -18,21 +18,20 @@ import { handleReadCommand, hasOutArg } from './read-commands'; import { handleWriteCommand } from './write-commands'; import { handleMetaCommand } from './meta-commands'; import { handleCookiePickerRoute, hasActivePicker } from './cookie-picker-routes'; -import { sanitizeExtensionUrl } from './sidebar-utils'; import { COMMAND_DESCRIPTIONS, PAGE_CONTENT_COMMANDS, DOM_CONTENT_COMMANDS, wrapUntrustedContent, canonicalizeCommand, buildUnknownCommandError, ALL_COMMANDS } from './commands'; import { wrapUntrustedPageContent, datamarkContent, runContentFilters, type ContentFilterResult, markHiddenElements, getCleanTextWithStripping, cleanupHiddenMarkers, } from './content-security'; -import { generateCanary, injectCanary, getStatus as getSecurityStatus, writeDecision } from './security'; +import { getStatus as getSecurityStatus } from './security'; import { isSidecarAvailable, scanWithSidecar } from './security-sidecar-client'; import { writeSecureFile, mkdirSecure } from './file-permissions'; import { handleSnapshot, SNAPSHOT_FLAGS } from './snapshot'; import { initRegistry, validateToken as validateScopedToken, checkScope, checkDomain, checkRate, createToken, createSetupKey, exchangeSetupKey, revokeToken, - rotateRoot, listTokens, serializeRegistry, restoreRegistry, recordCommand, + listTokens, recordCommand, isRootToken, checkConnectRateLimit, type TokenInfo, } from './token-registry'; import { validateTempPath } from './path-security'; @@ -44,7 +43,7 @@ import { inspectElement, modifyStyle, resetModifications, getModificationHistory // Bun.spawn used instead of child_process.spawn (compiled bun binaries // fail posix_spawn on all executables including /bin/bash) import { safeUnlink, safeUnlinkQuiet, safeKill } from './error-handling'; -import { readAgentRecord, killAgentByRecord, clearAgentRecord, agentRecordPath, spawnTerminalAgent } from './terminal-agent-control'; +import { readAgentRecord, killAgentByRecord, agentRecordPath, spawnTerminalAgent } from './terminal-agent-control'; import { isProcessAlive } from './error-handling'; import { sanitizeBody, stripLoneSurrogateEscapes } from './sanitize'; import { startSocksBridge, testUpstream, type BridgeHandle } from './socks-bridge'; diff --git a/browse/src/sidebar-utils.ts b/browse/src/sidebar-utils.ts deleted file mode 100644 index c5ff201d0..000000000 --- a/browse/src/sidebar-utils.ts +++ /dev/null @@ -1,21 +0,0 @@ -/** - * Shared sidebar utilities — extracted for testability. - */ - -/** - * Sanitize a URL from the Chrome extension before embedding in a prompt. - * Only accepts http/https, strips control characters, truncates to 2048 chars. - * Returns null if the URL is invalid or uses a non-http scheme. - */ -export function sanitizeExtensionUrl(url: string | null | undefined): string | null { - if (!url) return null; - try { - const u = new URL(url); - if (u.protocol === 'http:' || u.protocol === 'https:') { - return u.href.replace(/[\x00-\x1f\x7f]/g, '').slice(0, 2048); - } - return null; - } catch { - return null; - } -} diff --git a/browse/src/sse-session-cookie.ts b/browse/src/sse-session-cookie.ts index bae8ba5f8..feb53ccbb 100644 --- a/browse/src/sse-session-cookie.ts +++ b/browse/src/sse-session-cookie.ts @@ -96,11 +96,6 @@ export function buildSseSetCookie(token: string): string { return `${SSE_COOKIE_NAME}=${token}; HttpOnly; SameSite=Strict; Path=/; Max-Age=${maxAge}`; } -/** Build a Set-Cookie header that clears the SSE session cookie. */ -export function buildSseClearCookie(): string { - return `${SSE_COOKIE_NAME}=; HttpOnly; SameSite=Strict; Path=/; Max-Age=0`; -} - function pruneExpired(now: number): void { // Opportunistic cleanup: check up to 20 entries per call so we don't // stall on a massive registry. O(1) amortized. Runs on every mint diff --git a/browse/src/stealth.ts b/browse/src/stealth.ts index 18831400a..fc3c8a502 100644 --- a/browse/src/stealth.ts +++ b/browse/src/stealth.ts @@ -461,14 +461,6 @@ export async function applyStealth(context: BrowserContext): Promise { } } -/** - * The legacy single-line webdriver mask, exported for backwards - * compatibility with any caller that uses it directly. New callers - * should use applyStealth() which includes this plus the Layer C - * additions. - */ -export const WEBDRIVER_MASK_SCRIPT = `Object.defineProperty(navigator, 'webdriver', { get: () => false });`; - /** * Args added to chromium.launch's `args` to suppress the * AutomationControlled blink feature. This is independent of the init diff --git a/browse/test/content-security.test.ts b/browse/test/content-security.test.ts index 1682fb7a3..7332786d4 100644 --- a/browse/test/content-security.test.ts +++ b/browse/test/content-security.test.ts @@ -124,6 +124,16 @@ describe('Content filter hooks', () => { clearContentFilters(); }); + // clearContentFilters() wipes MODULE state shared across every test file in + // the same bun process — without restoring the built-in registration, + // security-integration.test.ts (which asserts the auto-registered blocklist + // pipeline) fails whenever the two files co-run. Pre-existing co-run bug, + // invisible until the free suite got a CI job. + afterAll(() => { + clearContentFilters(); + registerContentFilter(urlBlocklistFilter); + }); + test('URL blocklist detects requestbin', () => { const result = urlBlocklistFilter('', 'https://requestbin.com/r/abc', 'text'); expect(result.safe).toBe(false); diff --git a/browse/test/security-review-flow.test.ts b/browse/test/security-review-flow.test.ts deleted file mode 100644 index a87554997..000000000 --- a/browse/test/security-review-flow.test.ts +++ /dev/null @@ -1,194 +0,0 @@ -/** - * Review-on-BLOCK regression tests. - * - * Covers the user-in-the-loop path added to resolve false positives on - * benign developer content (e.g., HN comments discussing a prompt injection - * incident getting flagged as prompt injection). Instead of hard-stopping - * the session on a tool-output BLOCK, the agent emits a reviewable - * security_event and polls for the user's decision via a per-tab file. - * - * These tests pin the file-based handshake and the excerpt sanitization. - */ -import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; -import * as fs from 'fs'; -import * as os from 'os'; -import * as path from 'path'; -import { - writeDecision, - readDecision, - clearDecision, - decisionFileForTab, - excerptForReview, - type Verdict, -} from '../src/security'; - -const ORIG_HOME = process.env.HOME; -let tmpHome = ''; - -beforeEach(() => { - tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'sec-review-')); - process.env.HOME = tmpHome; -}); - -afterEach(() => { - process.env.HOME = ORIG_HOME; - try { fs.rmSync(tmpHome, { recursive: true, force: true }); } catch {} -}); - -describe('security decision file handshake', () => { - test('writeDecision + readDecision round-trips', () => { - // SECURITY_DIR is computed at module load time from the original HOME. - // The function writes relative to its own SECURITY_DIR constant, so we - // verify the API shape rather than the exact path. The file lives where - // decisionFileForTab says it does. - const file = decisionFileForTab(42); - expect(file.endsWith('/tab-42.json')).toBe(true); - - // Ensure the directory exists (writeDecision creates it). - writeDecision({ tabId: 42, decision: 'allow', ts: new Date().toISOString(), reason: 'user' }); - const rec = readDecision(42); - expect(rec).not.toBeNull(); - expect(rec?.tabId).toBe(42); - expect(rec?.decision).toBe('allow'); - expect(rec?.reason).toBe('user'); - }); - - test('clearDecision removes the file', () => { - writeDecision({ tabId: 7, decision: 'block', ts: new Date().toISOString() }); - expect(readDecision(7)).not.toBeNull(); - clearDecision(7); - expect(readDecision(7)).toBeNull(); - }); - - test('readDecision returns null for a tab with no decision', () => { - expect(readDecision(99999)).toBeNull(); - }); - - test('writeDecision + readDecision handles both values', () => { - writeDecision({ tabId: 1, decision: 'allow', ts: '2026-04-20T12:00:00Z' }); - writeDecision({ tabId: 2, decision: 'block', ts: '2026-04-20T12:00:01Z' }); - expect(readDecision(1)?.decision).toBe('allow'); - expect(readDecision(2)?.decision).toBe('block'); - }); - - test('atomic write: temp file is cleaned up after rename', () => { - writeDecision({ tabId: 10, decision: 'allow', ts: new Date().toISOString() }); - const file = decisionFileForTab(10); - const dir = path.dirname(file); - const leftover = fs.readdirSync(dir).filter((f) => f.startsWith('tab-10.json.tmp')); - expect(leftover.length).toBe(0); - }); - - test('file perms are 0600 on the decision file', () => { - writeDecision({ tabId: 3, decision: 'allow', ts: new Date().toISOString() }); - const stat = fs.statSync(decisionFileForTab(3)); - // mode & 0o777 = lower 9 bits of permission - const perms = stat.mode & 0o777; - // On some filesystems the sticky/group bits may vary; we assert the - // owner-only pattern. - expect(perms & 0o077).toBe(0); // no group/other read or write - }); -}); - -describe('excerptForReview sanitization', () => { - test('passes short clean text through', () => { - expect(excerptForReview('hello world')).toBe('hello world'); - }); - - test('truncates at the default max with ellipsis', () => { - const long = 'a'.repeat(800); - const out = excerptForReview(long); - expect(out.length).toBe(501); // 500 chars + ellipsis - expect(out.endsWith('…')).toBe(true); - }); - - test('strips control chars that would break the UI', () => { - const input = 'before\x00\x01\x02\x1Fafter'; - expect(excerptForReview(input)).toBe('beforeafter'); - }); - - test('collapses whitespace for compact display', () => { - expect(excerptForReview('foo \n\n\t bar')).toBe('foo bar'); - }); - - test('returns empty string for empty input', () => { - expect(excerptForReview('')).toBe(''); - expect(excerptForReview(null as any)).toBe(''); - }); - - test('custom max parameter', () => { - expect(excerptForReview('abcdefghij', 5)).toBe('abcde…'); - }); -}); - -describe('Verdict type includes user_overrode', () => { - test('user_overrode is a valid Verdict value', () => { - // TypeScript compile-time check that the type accepts the value. - // If 'user_overrode' were removed from the Verdict union, this file - // would fail to type-check. - const v: Verdict = 'user_overrode'; - expect(v).toBe('user_overrode'); - }); -}); - -describe('review-flow smoke — simulated sidebar-agent poll loop', () => { - test('agent-side poll sees user allow decision', async () => { - const tabId = 123; - clearDecision(tabId); - - // Simulate the sidepanel POST happening after a short delay. - setTimeout(() => { - writeDecision({ tabId, decision: 'allow', ts: new Date().toISOString(), reason: 'user' }); - }, 50); - - // Simulate the sidebar-agent poll loop. - const deadline = Date.now() + 2000; - let decision: 'allow' | 'block' | null = null; - while (Date.now() < deadline) { - const rec = readDecision(tabId); - if (rec?.decision) { - decision = rec.decision; - break; - } - await new Promise((r) => setTimeout(r, 20)); - } - expect(decision).toBe('allow'); - }); - - test('agent-side poll sees user block decision', async () => { - const tabId = 456; - clearDecision(tabId); - setTimeout(() => { - writeDecision({ tabId, decision: 'block', ts: new Date().toISOString() }); - }, 50); - - const deadline = Date.now() + 2000; - let decision: 'allow' | 'block' | null = null; - while (Date.now() < deadline) { - const rec = readDecision(tabId); - if (rec?.decision) { - decision = rec.decision; - break; - } - await new Promise((r) => setTimeout(r, 20)); - } - expect(decision).toBe('block'); - }); - - test('poll times out when no decision arrives', async () => { - const tabId = 789; - clearDecision(tabId); - - const deadline = Date.now() + 200; - let decision: 'allow' | 'block' | null = null; - while (Date.now() < deadline) { - const rec = readDecision(tabId); - if (rec?.decision) { - decision = rec.decision; - break; - } - await new Promise((r) => setTimeout(r, 20)); - } - expect(decision).toBeNull(); - }); -}); diff --git a/browse/test/security.test.ts b/browse/test/security.test.ts index a452b435e..6ef73cd04 100644 --- a/browse/test/security.test.ts +++ b/browse/test/security.test.ts @@ -14,14 +14,10 @@ import { generateCanary, injectCanary, checkCanaryInStructure, - hashPayload, - logAttempt, writeSessionState, readSessionState, getStatus, extractDomain, - buildTelemetrySpawnCommand, - resolveBashBinary, type LayerSignal, } from '../src/security'; @@ -239,46 +235,9 @@ describe('canary', () => { // ─── Payload hashing ───────────────────────────────────────── -describe('hashPayload', () => { - test('same payload produces same hash (deterministic with persistent salt)', () => { - const h1 = hashPayload('attack string'); - const h2 = hashPayload('attack string'); - expect(h1).toBe(h2); - }); - - test('different payloads produce different hashes', () => { - expect(hashPayload('a')).not.toBe(hashPayload('b')); - }); - - test('hash is sha256 hex (64 chars)', () => { - const h = hashPayload('test'); - expect(h).toMatch(/^[0-9a-f]{64}$/); - }); -}); // ─── Attack log + rotation ─────────────────────────────────── -describe('logAttempt', () => { - test('writes attempts.jsonl with correct shape', () => { - const ok = logAttempt({ - ts: '2026-04-19T12:34:56Z', - urlDomain: 'example.com', - payloadHash: 'deadbeef', - confidence: 0.9, - layer: 'testsavant_content', - verdict: 'block', - }); - expect(ok).toBe(true); - - const logPath = path.join(os.homedir(), '.gstack', 'security', 'attempts.jsonl'); - const content = fs.readFileSync(logPath, 'utf8'); - const lines = content.split('\n').filter(Boolean); - const last = JSON.parse(lines[lines.length - 1]); - expect(last.urlDomain).toBe('example.com'); - expect(last.payloadHash).toBe('deadbeef'); - expect(last.verdict).toBe('block'); - }); -}); // ─── Session state (cross-process, atomic) ─────────────────── @@ -330,74 +289,6 @@ describe('extractDomain', () => { // ─── Bash binary resolution (Windows shebang-script invocation) ───── -describe('resolveBashBinary', () => { - test('on POSIX, returns the system bash via Bun.which', () => { - if (process.platform === 'win32') return; - const out = resolveBashBinary({ PATH: process.env.PATH ?? '' }); - expect(out).toBeTruthy(); - expect(out!.endsWith('bash')).toBe(true); - }); - - test('honors GSTACK_BASH_BIN absolute-path override', () => { - // Construct a synthetic absolute path; the helper short-circuits on - // path.isAbsolute and never touches the filesystem, so this is portable. - const fake = process.platform === 'win32' ? 'C:\\opt\\bash.exe' : '/opt/custom/bash'; - const out = resolveBashBinary({ GSTACK_BASH_BIN: fake, PATH: '' }); - expect(out).toBe(fake); - }); - - test('strips wrapping double quotes from override values', () => { - const fake = process.platform === 'win32' ? 'C:\\opt\\bash.exe' : '/opt/custom/bash'; - const out = resolveBashBinary({ GSTACK_BASH_BIN: `"${fake}"`, PATH: '' }); - expect(out).toBe(fake); - }); - - test('BASH_BIN works as a fallback when GSTACK_BASH_BIN is unset', () => { - const fake = process.platform === 'win32' ? 'C:\\opt\\bash.exe' : '/opt/custom/bash'; - const out = resolveBashBinary({ BASH_BIN: fake, PATH: '' }); - expect(out).toBe(fake); - }); - - test('returns null when nothing resolves (override is unset and PATH is empty)', () => { - // Empty PATH means Bun.which finds nothing. - const out = resolveBashBinary({ PATH: '' }); - expect(out).toBeNull(); - }); -}); // ─── Telemetry spawn command (Windows bash wrapper, v1.24-aligned) ── -describe('buildTelemetrySpawnCommand', () => { - const bin = '/home/user/.claude/skills/gstack/bin/gstack-telemetry-log'; - const args = ['--event-type', 'attack_attempt', '--confidence', '0.95']; - - test('on POSIX, returns the binary path and args unchanged', () => { - if (process.platform === 'win32') return; - const out = buildTelemetrySpawnCommand(bin, args); - expect(out).not.toBeNull(); - expect(out!.cmd).toBe(bin); - expect(out!.cmdArgs).toEqual(args); - }); - - test('on win32 with bash resolvable, wraps the call in bash with the script as first arg', () => { - if (process.platform !== 'win32') return; - const fakeBash = 'C:\\Program Files\\Git\\bin\\bash.exe'; - const out = buildTelemetrySpawnCommand(bin, args, { GSTACK_BASH_BIN: fakeBash, PATH: '' }); - expect(out).not.toBeNull(); - expect(out!.cmd).toBe(fakeBash); - expect(out!.cmdArgs).toEqual([bin, ...args]); - }); - - test('on win32 with bash unresolvable, returns null so caller skips spawn', () => { - if (process.platform !== 'win32') return; - // No override, empty PATH — Bun.which finds nothing on Windows. - const out = buildTelemetrySpawnCommand(bin, args, { PATH: '' }); - expect(out).toBeNull(); - }); - - test('does not mutate the caller-supplied args array', () => { - const originalArgs = [...args]; - buildTelemetrySpawnCommand(bin, args); - expect(args).toEqual(originalArgs); - }); -}); diff --git a/browse/test/sidebar-unit.test.ts b/browse/test/sidebar-unit.test.ts deleted file mode 100644 index 3c0459a04..000000000 --- a/browse/test/sidebar-unit.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -/** - * Layer 1: Unit tests for sidebar utilities. - * Tests pure functions — no server, no processes, no network. - */ - -import { describe, test, expect } from 'bun:test'; -import { sanitizeExtensionUrl } from '../src/sidebar-utils'; - -describe('sanitizeExtensionUrl', () => { - test('passes valid http URL', () => { - expect(sanitizeExtensionUrl('http://example.com')).toBe('http://example.com/'); - }); - - test('passes valid https URL', () => { - expect(sanitizeExtensionUrl('https://example.com/page?q=1')).toBe('https://example.com/page?q=1'); - }); - - test('rejects chrome:// URLs', () => { - expect(sanitizeExtensionUrl('chrome://extensions')).toBeNull(); - }); - - test('rejects chrome-extension:// URLs', () => { - expect(sanitizeExtensionUrl('chrome-extension://abcdef/popup.html')).toBeNull(); - }); - - test('rejects javascript: URLs', () => { - expect(sanitizeExtensionUrl('javascript:alert(1)')).toBeNull(); - }); - - test('rejects file:// URLs', () => { - expect(sanitizeExtensionUrl('file:///etc/passwd')).toBeNull(); - }); - - test('rejects data: URLs', () => { - expect(sanitizeExtensionUrl('data:text/html,

hi

')).toBeNull(); - }); - - test('strips raw control characters from URL', () => { - // URL constructor percent-encodes \x00 as %00, which is safe - // The regex strips any remaining raw control chars after .href normalization - const result = sanitizeExtensionUrl('https://example.com/\x00page\x1f'); - expect(result).not.toBeNull(); - expect(result!).not.toMatch(/[\x00-\x1f\x7f]/); - }); - - test('strips newlines (prompt injection vector)', () => { - const result = sanitizeExtensionUrl('https://evil.com/%0AUser:%20ignore'); - // URL constructor normalizes %0A, control char stripping removes any raw newlines - expect(result).not.toBeNull(); - expect(result!).not.toContain('\n'); - }); - - test('truncates URLs longer than 2048 chars', () => { - const longUrl = 'https://example.com/' + 'a'.repeat(3000); - const result = sanitizeExtensionUrl(longUrl); - expect(result).not.toBeNull(); - expect(result!.length).toBeLessThanOrEqual(2048); - }); - - test('returns null for null input', () => { - expect(sanitizeExtensionUrl(null)).toBeNull(); - }); - - test('returns null for undefined input', () => { - expect(sanitizeExtensionUrl(undefined)).toBeNull(); - }); - - test('returns null for empty string', () => { - expect(sanitizeExtensionUrl('')).toBeNull(); - }); - - test('returns null for invalid URL string', () => { - expect(sanitizeExtensionUrl('not a url at all')).toBeNull(); - }); - - test('does not crash on weird input', () => { - expect(sanitizeExtensionUrl(':///')).toBeNull(); - expect(sanitizeExtensionUrl(' ')).toBeNull(); - expect(sanitizeExtensionUrl('\x00\x01\x02')).toBeNull(); - }); - - test('preserves query parameters and fragments', () => { - const url = 'https://example.com/search?q=test&page=2#results'; - expect(sanitizeExtensionUrl(url)).toBe(url); - }); - - test('preserves port numbers', () => { - expect(sanitizeExtensionUrl('http://localhost:3000/api')).toBe('http://localhost:3000/api'); - }); - - test('handles URL with auth (user:pass@host)', () => { - const result = sanitizeExtensionUrl('https://user:pass@example.com/'); - expect(result).not.toBeNull(); - expect(result).toContain('example.com'); - }); -}); diff --git a/browse/test/sse-session-cookie.test.ts b/browse/test/sse-session-cookie.test.ts index 0e27a9167..2948ca2d4 100644 --- a/browse/test/sse-session-cookie.test.ts +++ b/browse/test/sse-session-cookie.test.ts @@ -12,7 +12,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { mintSseSessionToken, validateSseSessionToken, extractSseCookie, - buildSseSetCookie, buildSseClearCookie, SSE_COOKIE_NAME, + buildSseSetCookie, SSE_COOKIE_NAME, __resetSseSessions, } from '../src/sse-session-cookie'; @@ -106,11 +106,6 @@ describe('SSE session cookie: cookie flag invariants', () => { // add Secure then. expect(buildSseSetCookie(token)).not.toContain('Secure'); }); - - test('Clear-Cookie has Max-Age=0', () => { - expect(buildSseClearCookie()).toContain('Max-Age=0'); - expect(buildSseClearCookie()).toContain('HttpOnly'); - }); }); describe('SSE session cookie: extract from request', () => { diff --git a/browse/test/stealth-layer-c.test.ts b/browse/test/stealth-layer-c.test.ts index db6a4c4d3..fbc5fed37 100644 --- a/browse/test/stealth-layer-c.test.ts +++ b/browse/test/stealth-layer-c.test.ts @@ -14,7 +14,6 @@ import { buildGStackLaunchArgs, readHostProfile, AUTOMATION_ARTIFACT_CLEANUP_SCRIPT, - WEBDRIVER_MASK_SCRIPT, STEALTH_LAUNCH_ARGS, STEALTH_IGNORE_DEFAULT_ARGS, } from '../src/stealth'; @@ -235,10 +234,6 @@ describe('buildGStackLaunchArgs — Pack 1 cmdline-switch construction', () => { }); describe('backwards-compat exports', () => { - test('WEBDRIVER_MASK_SCRIPT still exported', () => { - expect(WEBDRIVER_MASK_SCRIPT).toContain("'webdriver'"); - expect(WEBDRIVER_MASK_SCRIPT).toContain('false'); - }); test('STEALTH_LAUNCH_ARGS still includes blink-features=AutomationControlled', () => { expect(STEALTH_LAUNCH_ARGS).toContain('--disable-blink-features=AutomationControlled'); }); diff --git a/browse/test/stealth-webdriver.test.ts b/browse/test/stealth-webdriver.test.ts index 9ef595cb9..466b06bdd 100644 --- a/browse/test/stealth-webdriver.test.ts +++ b/browse/test/stealth-webdriver.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { chromium, type Browser, type BrowserContext } from 'playwright'; -import { applyStealth, WEBDRIVER_MASK_SCRIPT, STEALTH_LAUNCH_ARGS } from '../src/stealth'; +import { applyStealth, STEALTH_LAUNCH_ARGS } from '../src/stealth'; let browser: Browser; @@ -18,20 +18,6 @@ describe('STEALTH_LAUNCH_ARGS', () => { }); }); -describe('WEBDRIVER_MASK_SCRIPT', () => { - test('contains a single Object.defineProperty for navigator.webdriver', () => { - expect(WEBDRIVER_MASK_SCRIPT).toContain('navigator'); - expect(WEBDRIVER_MASK_SCRIPT).toContain('webdriver'); - expect(WEBDRIVER_MASK_SCRIPT).toContain('false'); - }); - - test('does NOT touch plugins, languages, or window.chrome (D7 narrowing)', () => { - expect(WEBDRIVER_MASK_SCRIPT).not.toMatch(/plugins/i); - expect(WEBDRIVER_MASK_SCRIPT).not.toMatch(/languages/i); - expect(WEBDRIVER_MASK_SCRIPT).not.toMatch(/window\.chrome/); - }); -}); - describe('applyStealth — context level', () => { let context: BrowserContext; diff --git a/browse/test/terminal-agent.test.ts b/browse/test/terminal-agent.test.ts index d908052d2..7793587c3 100644 --- a/browse/test/terminal-agent.test.ts +++ b/browse/test/terminal-agent.test.ts @@ -20,7 +20,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { mintPtySessionToken, validatePtySessionToken, revokePtySessionToken, - extractPtyCookie, buildPtySetCookie, buildPtyClearCookie, + extractPtyCookie, buildPtySetCookie, PTY_COOKIE_NAME, __resetPtySessions, } from '../src/pty-session-cookie'; @@ -61,10 +61,6 @@ describe('pty-session-cookie: mint/validate/revoke', () => { expect(cookie).not.toContain('Secure'); }); - test('clear-cookie has Max-Age=0', () => { - expect(buildPtyClearCookie()).toContain('Max-Age=0'); - }); - test('extractPtyCookie reads gstack_pty from a Cookie header', () => { const { token } = mintPtySessionToken(); const req = new Request('http://127.0.0.1/ws', { @@ -150,10 +146,13 @@ describe('Source-level guard: terminal-agent', () => { AGENT_SRC.indexOf("if (url.pathname === '/ws')"), AGENT_SRC.indexOf("websocket: {"), ); - expect(upgradeBlock).not.toContain('spawnClaude('); + // v1.44 renamed spawnClaude -> maybeSpawnPty (explicit `start` frame + + // lazy first-byte spawn share one helper). Pin was stale from then until + // the free suite got a CI job. + expect(upgradeBlock).not.toContain('maybeSpawnPty('); // Spawn must be invoked from the message handler (lazy on first byte). const messageHandler = AGENT_SRC.slice(AGENT_SRC.indexOf('message(ws, raw)')); - expect(messageHandler).toContain('spawnClaude('); + expect(messageHandler).toContain('maybeSpawnPty('); expect(messageHandler).toContain('!session.spawned'); });