mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-13 16:38:56 +02:00
fix(browse): remove the dead security shield + unfed /health.security (re-derived from #2557)
The sidebar's SEC shield has been dead UI since the PTY terminal rewrite: nothing set its data-status, nothing unhid it, and the /health.security field behind it read getStatus() off ~/.gstack/security/session-state.json — a file whose ONLY writer (sidebar-agent.ts) was deleted with the chat path. /health therefore reported a permanent 'inactive', or a stale FALSE-GREEN 'protected' wherever an old state file survived on disk (a single unit-test run was enough to plant one). A green shield sourced from leftover state reads as "no threats detected" when the real state is "not measured" — the same fail-open class as #2026. Removed (dead surfaces only): the shield markup/CSS and the stale sidepanel.js comment; the /health security field and server.ts's getStatus import; getStatus / SecurityStatus / StatusDetail / SessionState / read+writeSessionState (and security.ts's dead child_process import); the session-state + getStatus unit tests — including the round-trip test that wrote real fixture data into ~/.gstack and left /health green forever. (The PR's security-sidepanel-dom.test.ts deletion already happened on main via #2230; its resolveDisconnectCause guard landed via the #2085 typeof fix. Neither re-applied.) Kept, per ENG-OV9 — security.ts has LIVE consumers: the pure combiner (combineVerdict + THRESHOLDS), canary utilities, and extractDomain stay; server.ts's /pty-inject-scan L4 path (isSidecarAvailable + scanWithSidecar) is untouched. browse/test/server-security-surface.test.ts pins BOTH directions: the dead surface stays dead (no /health security field, no getStatus import, no reader of the security session-state file, shield markup gone) and the live half stays live (sidecar wiring in server.ts, combiner/canary exports in security.ts, /health carries no token — the v1.63 regression wall). A future re-feed from LIVE signals must update that test deliberately rather than resurrect the state-file path. F13 (same commit): CLAUDE.md's Sidebar security stack section, ARCHITECTURE.md's prompt-injection Visibility + critical-constraint paragraphs, and BROWSER.md's security section now describe the removed surfaces as history, not live features. Net -166 lines. Tests: server-security-surface + security + security-adversarial(+fixes) + security-integration + server-auth 114 pass; sidepanel-* + extension-token + extension-sender-auth 58 pass / 2 skip. Re-derived from PR #2557 by @frederik-kaster-noygear. Co-authored-by: Frederik Kaster <frederik.kaster@noygear.ai> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Frederik Kaster
Claude Fable 5
parent
af23375aa2
commit
f4e84b4dc9
+13
-94
@@ -23,18 +23,19 @@
|
||||
* host process): the combiner is pure and tested, and server.ts's
|
||||
* inline L4 path is the consumer of record.
|
||||
*
|
||||
* Cross-process state lives at ~/.gstack/security/session-state.json.
|
||||
* classifierStatus in that state has no live writer since the chat-path rip
|
||||
* (the sidecar reports status over its own NDJSON protocol instead).
|
||||
* There is no longer any cross-process session state (#2557).
|
||||
* ~/.gstack/security/session-state.json existed to carry classifier status
|
||||
* across the server.ts / sidebar-agent.ts boundary; sidebar-agent.ts went
|
||||
* away with the PTY terminal rewrite, leaving nothing to write the file and
|
||||
* a /health.security status that reported stale or empty data — a permanent
|
||||
* 'inactive', or a false-green 'protected' wherever an old state file
|
||||
* survived on disk. getStatus / SessionState / read+writeSessionState and
|
||||
* the /health field were removed together. Per-tab decision files under
|
||||
* ~/.gstack/security/decisions/ are unaffected, and the L4 sidecar reports
|
||||
* status over its own NDJSON protocol (security-sidecar-client.ts).
|
||||
*/
|
||||
|
||||
import { randomBytes, createHash } from 'crypto';
|
||||
import { spawn } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import { restrictFilePermissions, appendSecureFile, mkdirSecure } from './file-permissions';
|
||||
import { atomicWriteQuiet } from '../../lib/fs-atomic';
|
||||
import { randomBytes } from 'crypto';
|
||||
|
||||
// ─── Thresholds + verdict types ──────────────────────────────
|
||||
|
||||
@@ -83,17 +84,6 @@ export interface SecurityResult {
|
||||
confidence: number;
|
||||
}
|
||||
|
||||
export type SecurityStatus = 'protected' | 'degraded' | 'inactive';
|
||||
|
||||
export interface StatusDetail {
|
||||
status: SecurityStatus;
|
||||
layers: {
|
||||
testsavant: 'ok' | 'degraded' | 'off';
|
||||
canary: 'ok' | 'off';
|
||||
};
|
||||
lastUpdated: string;
|
||||
}
|
||||
|
||||
// ─── Verdict combiner (ensemble rule, label-first for transcript) ────
|
||||
|
||||
/**
|
||||
@@ -322,79 +312,8 @@ export function checkCanaryInStructure(value: unknown, canary: string): boolean
|
||||
// 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');
|
||||
|
||||
// ─── Cross-process session state ─────────────────────────────
|
||||
|
||||
const STATE_FILE = path.join(SECURITY_DIR, 'session-state.json');
|
||||
|
||||
/**
|
||||
* SessionState is a DISK FORMAT (~/.gstack/security/session-state.json).
|
||||
* Old files may carry a `transcript` field inside classifierStatus from the
|
||||
* removed Haiku layer — readSessionState tolerates it (JSON.parse keeps the
|
||||
* extra key; getStatus ignores it), but we never write it.
|
||||
*/
|
||||
export interface SessionState {
|
||||
sessionId: string;
|
||||
canary: string;
|
||||
warnedDomains: string[]; // per-session rate limit for special telemetry
|
||||
classifierStatus: {
|
||||
testsavant: 'ok' | 'degraded' | 'off';
|
||||
};
|
||||
lastUpdated: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomic write of session state (via lib/fs-atomic). Writes are safe
|
||||
* across process boundaries. Swallow-with-log polarity: a failed write
|
||||
* must never take down the caller (security state is best-effort cache).
|
||||
*/
|
||||
export function writeSessionState(state: SessionState): void {
|
||||
try { mkdirSecure(SECURITY_DIR); } catch { /* write below fails and logs */ }
|
||||
if (atomicWriteQuiet(STATE_FILE, JSON.stringify(state, null, 2), { mode: 0o600 })) {
|
||||
// Windows ACL hardening (POSIX chmod is redundant with mode above).
|
||||
restrictFilePermissions(STATE_FILE);
|
||||
} else {
|
||||
console.error('[security] writeSessionState failed');
|
||||
}
|
||||
}
|
||||
|
||||
export function readSessionState(): SessionState | null {
|
||||
try {
|
||||
if (!fs.existsSync(STATE_FILE)) return null;
|
||||
return JSON.parse(fs.readFileSync(STATE_FILE, 'utf8'));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Status reporting (for shield icon via /health) ──────────
|
||||
|
||||
export function getStatus(): StatusDetail {
|
||||
const state = readSessionState();
|
||||
// Read the field explicitly (never spread classifierStatus): old on-disk
|
||||
// state may carry a stale `transcript` key from the removed Haiku layer,
|
||||
// and spreading would leak it into the /health payload.
|
||||
const testsavant = state?.classifierStatus?.testsavant ?? 'off';
|
||||
const canary = state?.canary ? 'ok' : 'off';
|
||||
|
||||
let status: SecurityStatus;
|
||||
if (testsavant === 'ok' && canary === 'ok') {
|
||||
status = 'protected';
|
||||
} else if (testsavant === 'off' && canary === 'off') {
|
||||
status = 'inactive';
|
||||
} else {
|
||||
status = 'degraded';
|
||||
}
|
||||
|
||||
return {
|
||||
status,
|
||||
layers: { testsavant, canary: canary as 'ok' | 'off' },
|
||||
lastUpdated: state?.lastUpdated ?? new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
// rotation. The cross-process session state + getStatus shield feed went
|
||||
// the same way (#2557) — see the module header.
|
||||
|
||||
/**
|
||||
* Extract url domain for logging. Never logs path or query string.
|
||||
|
||||
@@ -24,7 +24,6 @@ import {
|
||||
runContentFilters, type ContentFilterResult,
|
||||
markHiddenElements, getCleanTextWithStripping, cleanupHiddenMarkers,
|
||||
} from './content-security';
|
||||
import { getStatus as getSecurityStatus } from './security';
|
||||
import { isSidecarAvailable, scanWithSidecar } from './security-sidecar-client';
|
||||
import { writeSecureFile, mkdirSecure, appendSecureFile } from './file-permissions';
|
||||
import { handleSnapshot, SNAPSHOT_FLAGS } from './snapshot';
|
||||
@@ -1899,10 +1898,13 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
mode: browserManager.getConnectionMode(),
|
||||
uptime: Math.floor((Date.now() - startTime) / 1000),
|
||||
tabs: browserManager.getTabCount(),
|
||||
// Security module status — drives the shield icon in the sidepanel.
|
||||
// Returns {status: 'protected'|'degraded'|'inactive', layers: {...}}.
|
||||
// Fed by the page-content side (testsavant sidecar, canary state).
|
||||
security: getSecurityStatus(),
|
||||
// No `security` field (#2557): the only writer of the status it
|
||||
// reported (sidebar-agent.ts's session-state file) went away with
|
||||
// the chat path, so it read from a file nothing wrote — reporting
|
||||
// a permanent 'inactive', or a stale false-green 'protected'
|
||||
// wherever an old state file survived on disk. The live defenses
|
||||
// (content-security L1-L3, the L4 sidecar on /pty-inject-scan)
|
||||
// report through their own call sites, not through /health.
|
||||
// Terminal-agent discovery. ONLY a port number — never a token.
|
||||
// Tokens flow via the /pty-session HttpOnly cookie path. See
|
||||
// `pty-session-cookie.ts` for the rationale (codex outside-voice
|
||||
|
||||
Reference in New Issue
Block a user