mirror of
https://github.com/garrytan/gstack.git
synced 2026-06-03 00:28:02 +02:00
fix: security audit remediation — 12 fixes, 20 tests (v0.13.1.0) (#595)
* fix: remove auth token from /health, secure extension bootstrap (CRITICAL-02 + HIGH-03) - Remove token from /health response (was leaked to any localhost process) - Write .auth.json to extension dir for Manifest V3 bootstrap - sidebar-agent reads token from state file via BROWSE_STATE_FILE env var - Remove getToken handler from extension (token via health broadcast) - Extension loads token before first health poll to prevent race condition * fix: require auth on cookie-picker data routes (CRITICAL-01) - Add Bearer token auth gate on all /cookie-picker/* data/action routes - GET /cookie-picker HTML page stays unauthenticated (UI shell) - Token embedded in served HTML for picker's fetch calls - CORS preflight now allows Authorization header * fix: add state file TTL and plaintext cookie warning (HIGH-02) - Add savedAt timestamp to state save output - Warn on load if state file older than 7 days - Auto-delete stale state files (>7 days) on server startup - Warning about plaintext cookie storage in save message * fix: innerHTML XSS in extension content script and sidepanel (MEDIUM-01) - content.js: replace innerHTML with createElement/textContent for ref panel - sidepanel.js: escape entry.command with escapeHtml() in activity feed - Both found by security audit + Codex adversarial red team * fix: symlink bypass in validateReadPath (MEDIUM-02) - Always resolve to absolute path first (fixes relative path bypass) - Use realpathSync to follow symlinks before boundary check - Throw on non-ENOENT realpathSync failures (explicit over silent) - Resolve SAFE_DIRECTORIES through realpathSync (macOS /tmp → /private/tmp) - Resolve directory part for non-existent files (ENOENT with symlinked parent) * fix: freeze hook symlink bypass and prefix collision (MEDIUM-03) - Add POSIX-portable path resolution (cd + pwd -P, works on macOS) - Fix prefix collision: /project-evil no longer matches /project freeze dir - Use trailing slash in boundary check to require directory boundary * fix: shell script injection in gstack-config and telemetry (MEDIUM-04) - gstack-config: validate keys (alphanumeric+underscore only) - gstack-config: use grep -F (fixed string) instead of -E (regex) - gstack-config: escape sed special chars in values, drop newlines - gstack-telemetry-log: sanitize REPO_SLUG and BRANCH via json_safe() * test: 20 security tests for audit remediation - server-auth: verify token removed from /health, auth on /refs, /activity/* - cookie-picker: auth required on data routes, HTML page unauthenticated - path-validation: symlink bypass blocked, realpathSync failure throws - gstack-config: regex key rejected, sed special chars preserved - state-ttl: savedAt timestamp, 7-day TTL warning - telemetry: branch/repo with quotes don't corrupt JSON - adversarial: sidepanel escapes entry.command, freeze prefix collision * chore: bump version and changelog (v0.13.1.0) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: tone down changelog — defense in depth, not catastrophic bugs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -211,7 +211,7 @@ export class BrowserManager {
|
||||
* The browser launches headed with a visible window — the user sees
|
||||
* every action Claude takes in real time.
|
||||
*/
|
||||
async launchHeaded(): Promise<void> {
|
||||
async launchHeaded(authToken?: string): Promise<void> {
|
||||
// Clear old state before repopulating
|
||||
this.pages.clear();
|
||||
this.refMap.clear();
|
||||
@@ -223,6 +223,17 @@ export class BrowserManager {
|
||||
if (extensionPath) {
|
||||
launchArgs.push(`--disable-extensions-except=${extensionPath}`);
|
||||
launchArgs.push(`--load-extension=${extensionPath}`);
|
||||
// Write auth token for extension bootstrap (read via chrome.runtime.getURL)
|
||||
if (authToken) {
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const authFile = path.join(extensionPath, '.auth.json');
|
||||
try {
|
||||
fs.writeFileSync(authFile, JSON.stringify({ token: authToken }), { mode: 0o600 });
|
||||
} catch (err: any) {
|
||||
console.warn(`[browse] Could not write .auth.json: ${err.message}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Launch headed Chromium via Playwright's persistent context.
|
||||
@@ -751,6 +762,20 @@ export class BrowserManager {
|
||||
if (extensionPath) {
|
||||
launchArgs.push(`--disable-extensions-except=${extensionPath}`);
|
||||
launchArgs.push(`--load-extension=${extensionPath}`);
|
||||
// Write auth token for extension bootstrap during handoff
|
||||
if (this.serverPort) {
|
||||
try {
|
||||
const { resolveConfig } = require('./config');
|
||||
const config = resolveConfig();
|
||||
const stateFile = path.join(config.stateDir, 'browse.json');
|
||||
if (fs.existsSync(stateFile)) {
|
||||
const stateData = JSON.parse(fs.readFileSync(stateFile, 'utf-8'));
|
||||
if (stateData.token) {
|
||||
fs.writeFileSync(path.join(extensionPath, '.auth.json'), JSON.stringify({ token: stateData.token }), { mode: 0o600 });
|
||||
}
|
||||
}
|
||||
} catch {}
|
||||
}
|
||||
console.log(`[browse] Handoff: loading extension from ${extensionPath}`);
|
||||
} else {
|
||||
console.log('[browse] Handoff: extension not found — headed mode without side panel');
|
||||
|
||||
@@ -53,6 +53,7 @@ export async function handleCookiePickerRoute(
|
||||
url: URL,
|
||||
req: Request,
|
||||
bm: BrowserManager,
|
||||
authToken?: string,
|
||||
): Promise<Response> {
|
||||
const pathname = url.pathname;
|
||||
const port = parseInt(url.port, 10) || 9400;
|
||||
@@ -64,7 +65,7 @@ export async function handleCookiePickerRoute(
|
||||
headers: {
|
||||
'Access-Control-Allow-Origin': corsOrigin(port),
|
||||
'Access-Control-Allow-Methods': 'GET, POST, OPTIONS',
|
||||
'Access-Control-Allow-Headers': 'Content-Type',
|
||||
'Access-Control-Allow-Headers': 'Content-Type, Authorization',
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -72,13 +73,24 @@ export async function handleCookiePickerRoute(
|
||||
try {
|
||||
// GET /cookie-picker — serve the picker UI
|
||||
if (pathname === '/cookie-picker' && req.method === 'GET') {
|
||||
const html = getCookiePickerHTML(port);
|
||||
const html = getCookiePickerHTML(port, authToken);
|
||||
return new Response(html, {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'text/html; charset=utf-8' },
|
||||
});
|
||||
}
|
||||
|
||||
// ─── Auth gate: all data/action routes below require Bearer token ───
|
||||
if (authToken) {
|
||||
const authHeader = req.headers.get('authorization');
|
||||
if (!authHeader || authHeader !== `Bearer ${authToken}`) {
|
||||
return new Response(JSON.stringify({ error: 'Unauthorized' }), {
|
||||
status: 401,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// GET /cookie-picker/browsers — list installed browsers
|
||||
if (pathname === '/cookie-picker/browsers' && req.method === 'GET') {
|
||||
const browsers = findInstalledBrowsers();
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
* No cookie values exposed anywhere.
|
||||
*/
|
||||
|
||||
export function getCookiePickerHTML(serverPort: number): string {
|
||||
export function getCookiePickerHTML(serverPort: number, authToken?: string): string {
|
||||
const baseUrl = `http://127.0.0.1:${serverPort}`;
|
||||
|
||||
return `<!DOCTYPE html>
|
||||
@@ -330,6 +330,7 @@ export function getCookiePickerHTML(serverPort: number): string {
|
||||
<script>
|
||||
(function() {
|
||||
const BASE = '${baseUrl}';
|
||||
const AUTH_TOKEN = '${authToken || ''}';
|
||||
let activeBrowser = null;
|
||||
let activeProfile = 'Default';
|
||||
let allProfiles = [];
|
||||
@@ -372,7 +373,9 @@ export function getCookiePickerHTML(serverPort: number): string {
|
||||
|
||||
// ─── API ────────────────────────────────
|
||||
async function api(path, opts) {
|
||||
const res = await fetch(BASE + '/cookie-picker' + path, opts);
|
||||
const headers = { ...(opts?.headers || {}) };
|
||||
if (AUTH_TOKEN) headers['Authorization'] = 'Bearer ' + AUTH_TOKEN;
|
||||
const res = await fetch(BASE + '/cookie-picker' + path, { ...opts, headers });
|
||||
const data = await res.json();
|
||||
if (!res.ok) {
|
||||
const err = new Error(data.error || 'Request failed');
|
||||
|
||||
@@ -474,11 +474,12 @@ export async function handleMetaCommand(
|
||||
// V1: cookies + URLs only (not localStorage — breaks on load-before-navigate)
|
||||
const saveData = {
|
||||
version: 1,
|
||||
savedAt: new Date().toISOString(),
|
||||
cookies: state.cookies,
|
||||
pages: state.pages.map(p => ({ url: p.url, isActive: p.isActive })),
|
||||
};
|
||||
fs.writeFileSync(statePath, JSON.stringify(saveData, null, 2), { mode: 0o600 });
|
||||
return `State saved: ${statePath} (${state.cookies.length} cookies, ${state.pages.length} pages — treat as sensitive)`;
|
||||
return `State saved: ${statePath} (${state.cookies.length} cookies, ${state.pages.length} pages)\n⚠️ Cookies stored in plaintext. Delete when no longer needed.`;
|
||||
}
|
||||
|
||||
if (action === 'load') {
|
||||
@@ -487,6 +488,14 @@ export async function handleMetaCommand(
|
||||
if (!Array.isArray(data.cookies) || !Array.isArray(data.pages)) {
|
||||
throw new Error('Invalid state file: expected cookies and pages arrays');
|
||||
}
|
||||
// Warn on state files older than 7 days
|
||||
if (data.savedAt) {
|
||||
const ageMs = Date.now() - new Date(data.savedAt).getTime();
|
||||
const SEVEN_DAYS = 7 * 24 * 60 * 60 * 1000;
|
||||
if (ageMs > SEVEN_DAYS) {
|
||||
console.warn(`[browse] Warning: State file is ${Math.round(ageMs / 86400000)} days old. Consider re-saving.`);
|
||||
}
|
||||
}
|
||||
// Close existing pages, then restore (replace, not merge)
|
||||
bm.setFrame(null);
|
||||
await bm.closeAllPages();
|
||||
|
||||
@@ -37,19 +37,34 @@ function wrapForEvaluate(code: string): string {
|
||||
}
|
||||
|
||||
// Security: Path validation to prevent path traversal attacks
|
||||
const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()];
|
||||
// Resolve safe directories through realpathSync to handle symlinks (e.g., macOS /tmp → /private/tmp)
|
||||
const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()].map(d => {
|
||||
try { return fs.realpathSync(d); } catch { return d; }
|
||||
});
|
||||
|
||||
export function validateReadPath(filePath: string): void {
|
||||
if (path.isAbsolute(filePath)) {
|
||||
const resolved = path.resolve(filePath);
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(resolved, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Absolute path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
// Always resolve to absolute first (fixes relative path symlink bypass)
|
||||
const resolved = path.resolve(filePath);
|
||||
// Resolve symlinks — throw on non-ENOENT errors
|
||||
let realPath: string;
|
||||
try {
|
||||
realPath = fs.realpathSync(resolved);
|
||||
} catch (err: any) {
|
||||
if (err.code === 'ENOENT') {
|
||||
// File doesn't exist — resolve directory part for symlinks (e.g., /tmp → /private/tmp)
|
||||
try {
|
||||
const dir = fs.realpathSync(path.dirname(resolved));
|
||||
realPath = path.join(dir, path.basename(resolved));
|
||||
} catch {
|
||||
realPath = resolved;
|
||||
}
|
||||
} else {
|
||||
throw new Error(`Cannot resolve real path: ${filePath} (${err.code})`);
|
||||
}
|
||||
}
|
||||
const normalized = path.normalize(filePath);
|
||||
if (normalized.includes('..')) {
|
||||
throw new Error('Path traversal sequences (..) are not allowed');
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realPath, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+46
-16
@@ -805,7 +805,7 @@ async function start() {
|
||||
if (!skipBrowser) {
|
||||
const headed = process.env.BROWSE_HEADED === '1';
|
||||
if (headed) {
|
||||
await browserManager.launchHeaded();
|
||||
await browserManager.launchHeaded(AUTH_TOKEN);
|
||||
console.log(`[browse] Launched headed Chromium with extension`);
|
||||
} else {
|
||||
await browserManager.launch();
|
||||
@@ -819,9 +819,9 @@ async function start() {
|
||||
fetch: async (req) => {
|
||||
const url = new URL(req.url);
|
||||
|
||||
// Cookie picker routes — no auth required (localhost-only)
|
||||
// Cookie picker routes — HTML page unauthenticated, data/action routes require auth
|
||||
if (url.pathname.startsWith('/cookie-picker')) {
|
||||
return handleCookiePickerRoute(url, req, browserManager);
|
||||
return handleCookiePickerRoute(url, req, browserManager, AUTH_TOKEN);
|
||||
}
|
||||
|
||||
// Health check — no auth required, does NOT reset idle timer
|
||||
@@ -833,7 +833,7 @@ async function start() {
|
||||
uptime: Math.floor((Date.now() - startTime) / 1000),
|
||||
tabs: browserManager.getTabCount(),
|
||||
currentUrl: browserManager.getCurrentUrl(),
|
||||
token: AUTH_TOKEN, // Extension uses this for Bearer auth
|
||||
// token removed — see .auth.json for extension bootstrap
|
||||
chatEnabled: true,
|
||||
agent: {
|
||||
status: agentStatus,
|
||||
@@ -848,8 +848,14 @@ async function start() {
|
||||
});
|
||||
}
|
||||
|
||||
// Refs endpoint — no auth required (localhost-only), does NOT reset idle timer
|
||||
// Refs endpoint — auth required, does NOT reset idle timer
|
||||
if (url.pathname === '/refs') {
|
||||
if (!validateAuth(req)) {
|
||||
return new Response(JSON.stringify({ error: 'Unauthorized' }), {
|
||||
status: 401,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
const refs = browserManager.getRefMap();
|
||||
return new Response(JSON.stringify({
|
||||
refs,
|
||||
@@ -857,15 +863,20 @@ async function start() {
|
||||
mode: browserManager.getConnectionMode(),
|
||||
}), {
|
||||
status: 200,
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
'Access-Control-Allow-Origin': '*',
|
||||
},
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
|
||||
// Activity stream — SSE, no auth (localhost-only), does NOT reset idle timer
|
||||
// Activity stream — SSE, auth required, does NOT reset idle timer
|
||||
if (url.pathname === '/activity/stream') {
|
||||
// Inline auth: accept Bearer header OR ?token= query param (EventSource can't send headers)
|
||||
const streamToken = url.searchParams.get('token');
|
||||
if (!validateAuth(req) && streamToken !== AUTH_TOKEN) {
|
||||
return new Response(JSON.stringify({ error: 'Unauthorized' }), {
|
||||
status: 401,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
const afterId = parseInt(url.searchParams.get('after') || '0', 10);
|
||||
const encoder = new TextEncoder();
|
||||
|
||||
@@ -913,21 +924,23 @@ async function start() {
|
||||
'Content-Type': 'text/event-stream',
|
||||
'Cache-Control': 'no-cache',
|
||||
'Connection': 'keep-alive',
|
||||
'Access-Control-Allow-Origin': '*',
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
// Activity history — REST, no auth (localhost-only), does NOT reset idle timer
|
||||
// Activity history — REST, auth required, does NOT reset idle timer
|
||||
if (url.pathname === '/activity/history') {
|
||||
if (!validateAuth(req)) {
|
||||
return new Response(JSON.stringify({ error: 'Unauthorized' }), {
|
||||
status: 401,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
const limit = parseInt(url.searchParams.get('limit') || '50', 10);
|
||||
const { entries, totalAdded } = getActivityHistory(limit);
|
||||
return new Response(JSON.stringify({ entries, totalAdded, subscribers: getSubscriberCount() }), {
|
||||
status: 200,
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
'Access-Control-Allow-Origin': '*',
|
||||
},
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1139,6 +1152,23 @@ async function start() {
|
||||
fs.renameSync(tmpFile, config.stateFile);
|
||||
|
||||
browserManager.serverPort = port;
|
||||
|
||||
// Clean up stale state files (older than 7 days)
|
||||
try {
|
||||
const stateDir = path.join(config.stateDir, 'browse-states');
|
||||
if (fs.existsSync(stateDir)) {
|
||||
const SEVEN_DAYS = 7 * 24 * 60 * 60 * 1000;
|
||||
for (const file of fs.readdirSync(stateDir)) {
|
||||
const filePath = path.join(stateDir, file);
|
||||
const stat = fs.statSync(filePath);
|
||||
if (Date.now() - stat.mtimeMs > SEVEN_DAYS) {
|
||||
fs.unlinkSync(filePath);
|
||||
console.log(`[browse] Deleted stale state file: ${file}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {}
|
||||
|
||||
console.log(`[browse] Server running on http://127.0.0.1:${port} (PID: ${process.pid})`);
|
||||
console.log(`[browse] State file: ${config.stateFile}`);
|
||||
console.log(`[browse] Idle timeout: ${IDLE_TIMEOUT_MS / 1000}s`);
|
||||
|
||||
@@ -66,10 +66,11 @@ function writeToInbox(message: string, pageUrl?: string, sessionId?: string): vo
|
||||
// ─── Auth ────────────────────────────────────────────────────────
|
||||
|
||||
async function refreshToken(): Promise<string | null> {
|
||||
// Read token from state file (same-user, mode 0o600) instead of /health
|
||||
try {
|
||||
const resp = await fetch(`${SERVER_URL}/health`, { signal: AbortSignal.timeout(3000) });
|
||||
if (!resp.ok) return null;
|
||||
const data = await resp.json() as any;
|
||||
const stateFile = process.env.BROWSE_STATE_FILE ||
|
||||
path.join(process.env.HOME || '/tmp', '.gstack', 'browse.json');
|
||||
const data = JSON.parse(fs.readFileSync(stateFile, 'utf-8'));
|
||||
authToken = data.token || null;
|
||||
return authToken;
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user