mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-14 00:49:00 +02:00
fix(security): delete dead exports the ripped chat path left behind
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
44d58aa6ee
commit
ef186cccb3
@@ -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) {
|
||||
|
||||
+5
-284
@@ -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 {
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -461,14 +461,6 @@ export async function applyStealth(context: BrowserContext): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
|
||||
Reference in New Issue
Block a user