fix(security): 20 additional bugs found via sqry semantic code graph (round 2)

bin/gstack-learnings-search

@@ -43,13 +43,13 @@ if [ ${#FILES[@]} -eq 0 ]; then
 fi
 
 # Process all files through bun for JSON parsing, decay, dedup, filtering
-cat "${FILES[@]}" 2>/dev/null | bun -e "
+cat "${FILES[@]}" 2>/dev/null | GSTACK_FILTER_TYPE="$TYPE" GSTACK_FILTER_QUERY="$QUERY" GSTACK_FILTER_LIMIT="$LIMIT" GSTACK_FILTER_SLUG="$SLUG" GSTACK_FILTER_CROSS="$CROSS_PROJECT" bun -e "
 const lines = (await Bun.stdin.text()).trim().split('\n').filter(Boolean);
 const now = Date.now();
-const type = '${TYPE}';
-const query = '${QUERY}'.toLowerCase();
-const limit = ${LIMIT};
-const slug = '${SLUG}';
+const type = process.env.GSTACK_FILTER_TYPE || '';
+const query = (process.env.GSTACK_FILTER_QUERY || '').toLowerCase();
+const limit = parseInt(process.env.GSTACK_FILTER_LIMIT || '10', 10) || 10;
+const slug = process.env.GSTACK_FILTER_SLUG || '';
 
 const entries = [];
 for (const line of lines) {
@@ -67,7 +67,7 @@ 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) && '${CROSS_PROJECT}' === 'true';
+    e._crossProject = !line.includes(slug) && (process.env.GSTACK_FILTER_CROSS || '') === 'true';
 
     entries.push(e);
   } catch {}

browse/src/browser-manager.ts

@@ -822,6 +822,12 @@ export class BrowserManager {
       this.wirePageEvents(page);
 
       if (saved.url) {
+        try {
+          await validateNavigationUrl(saved.url);
+        } catch (err: any) {
+          console.warn(`[browse] Skipping invalid URL in state file: ${saved.url} — ${err.message}`);
+          continue;
+        }
         await page.goto(saved.url, { waitUntil: 'domcontentloaded', timeout: 15000 }).catch(() => {});
       }
 

browse/src/cdp-inspector.ts

@@ -472,6 +472,12 @@ export async function modifyStyle(
     throw new Error(`Invalid CSS property name: ${property}. Only letters and hyphens allowed.`);
   }
 
+  // Validate CSS value — block data exfiltration patterns
+  const DANGEROUS_CSS = /url\s*\(|expression\s*\(|@import|javascript:|data:/i;
+  if (DANGEROUS_CSS.test(value)) {
+    throw new Error('CSS value rejected: contains potentially dangerous pattern.');
+  }
+
   let oldValue = '';
   let source = 'inline';
   let sourceLine = 0;

browse/src/cli.ts

@@ -587,7 +587,10 @@ Refs:           After 'snapshot', use @e1, @e2... as selectors:
         }
         // Clear old agent queue
         const agentQueue = path.join(process.env.HOME || '/tmp', '.gstack', 'sidebar-agent-queue.jsonl');
-        try { fs.writeFileSync(agentQueue, ''); } catch {}
+        try {
+          fs.mkdirSync(path.dirname(agentQueue), { recursive: true, mode: 0o700 });
+          fs.writeFileSync(agentQueue, '', { mode: 0o600 });
+        } catch {}
 
         // Resolve browse binary path the same way — execPath-relative
         let browseBin = path.resolve(__dirname, '..', 'dist', 'browse');

browse/src/meta-commands.ts

@@ -15,16 +15,42 @@ import { resolveConfig } from './config';
 import type { Frame } from 'playwright';
 
 // 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 validateOutputPath(filePath: string): void {
-  const resolved = path.resolve(filePath);
-  const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(resolved, dir));
+  // Always resolve to absolute first (fixes relative path symlink bypass)
+  const absolute = path.resolve(filePath);
+  // Resolve symlinks — for new files, resolve the parent directory
+  let realPath: string;
+  try {
+    realPath = fs.realpathSync(absolute);
+  } 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(absolute));
+        realPath = path.join(dir, path.basename(absolute));
+      } catch {
+        realPath = absolute;
+      }
+    } else {
+      throw new Error(`Cannot resolve real path: ${filePath} (${err.code})`);
+    }
+  }
+  const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realPath, dir));
   if (!isSafe) {
     throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
   }
 }
 
+/** Escape special regex metacharacters in a user-supplied string to prevent ReDoS. */
+export function escapeRegExp(s: string): string {
+  return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+}
+
 /** Tokenize a pipe segment respecting double-quoted strings. */
 function tokenizePipeSegment(segment: string): string[] {
   const tokens: string[] = [];
@@ -195,9 +221,10 @@ export async function handleMetaCommand(
 
       for (const vp of viewports) {
         await page.setViewportSize({ width: vp.width, height: vp.height });
-        const path = `${prefix}-${vp.name}.png`;
-        await page.screenshot({ path, fullPage: true });
-        results.push(`${vp.name} (${vp.width}x${vp.height}): ${path}`);
+        const screenshotPath = `${prefix}-${vp.name}.png`;
+        validateOutputPath(screenshotPath);
+        await page.screenshot({ path: screenshotPath, fullPage: true });
+        results.push(`${vp.name} (${vp.width}x${vp.height}): ${screenshotPath}`);
       }
 
       // Restore original viewport
@@ -238,6 +265,10 @@ export async function handleMetaCommand(
         try {
           let result: string;
           if (WRITE_COMMANDS.has(name)) {
+            if (bm.isWatching()) {
+              results.push(`[${name}] BLOCKED: write commands disabled in watch mode`);
+              continue;
+            }
             result = await handleWriteCommand(name, cmdArgs, bm);
             lastWasWrite = true;
           } else if (READ_COMMANDS.has(name)) {
@@ -443,8 +474,8 @@ export async function handleMetaCommand(
 
       for (const msg of messages) {
         const ts = msg.timestamp ? `[${msg.timestamp}]` : '[unknown]';
-        lines.push(`${ts} ${msg.url}`);
-        lines.push(`  "${msg.userMessage}"`);
+        lines.push(`${ts} ${wrapUntrustedContent(msg.url, 'inbox:url')}`);
+        lines.push(`  "${wrapUntrustedContent(msg.userMessage, 'inbox:message')}"`);
         lines.push('');
       }
 
@@ -495,6 +526,18 @@ export async function handleMetaCommand(
         if (!Array.isArray(data.cookies) || !Array.isArray(data.pages)) {
           throw new Error('Invalid state file: expected cookies and pages arrays');
         }
+        // Validate and filter cookies — reject malformed or internal-network cookies
+        const validatedCookies = data.cookies.filter((c: any) => {
+          if (typeof c !== 'object' || !c) return false;
+          if (typeof c.name !== 'string' || typeof c.value !== 'string') return false;
+          if (typeof c.domain !== 'string' || !c.domain) return false;
+          const d = c.domain.startsWith('.') ? c.domain.slice(1) : c.domain;
+          if (d === 'localhost' || d.endsWith('.internal') || d === '169.254.169.254') return false;
+          return true;
+        });
+        if (validatedCookies.length < data.cookies.length) {
+          console.warn(`[browse] Filtered ${data.cookies.length - validatedCookies.length} invalid cookies from state file`);
+        }
         // Warn on state files older than 7 days
         if (data.savedAt) {
           const ageMs = Date.now() - new Date(data.savedAt).getTime();
@@ -507,7 +550,7 @@ export async function handleMetaCommand(
         bm.setFrame(null);
         await bm.closeAllPages();
         await bm.restoreState({
-          cookies: data.cookies,
+          cookies: validatedCookies,
           pages: data.pages.map((p: any) => ({ ...p, storage: null })),
         });
         return `State loaded: ${data.cookies.length} cookies, ${data.pages.length} pages`;
@@ -535,7 +578,7 @@ export async function handleMetaCommand(
         frame = page.frame({ name: args[1] });
       } else if (target === '--url') {
         if (!args[1]) throw new Error('Usage: frame --url <pattern>');
-        frame = page.frame({ url: new RegExp(args[1]) });
+        frame = page.frame({ url: new RegExp(escapeRegExp(args[1])) });
       } else {
         // CSS selector or @ref for the iframe element
         const resolved = await bm.resolveRef(target);

browse/src/server.ts

@@ -282,6 +282,10 @@ function loadSession(): SidebarSession | null {
   try {
     const activeFile = path.join(SESSIONS_DIR, 'active.json');
     const activeData = JSON.parse(fs.readFileSync(activeFile, 'utf-8'));
+    if (typeof activeData.id !== 'string' || !/^[a-zA-Z0-9_-]+$/.test(activeData.id)) {
+      console.warn('[browse] Invalid session ID in active.json — ignoring');
+      return null;
+    }
     const sessionFile = path.join(SESSIONS_DIR, activeData.id, 'session.json');
     const session = JSON.parse(fs.readFileSync(sessionFile, 'utf-8')) as SidebarSession;
     // Validate worktree still exists — crash may have left stale path
@@ -558,8 +562,9 @@ function spawnClaude(userMessage: string, extensionUrl?: string | null, forTabId
     tabId: agentTabId,
   });
   try {
-    fs.mkdirSync(gstackDir, { recursive: true });
+    fs.mkdirSync(gstackDir, { recursive: true, mode: 0o700 });
     fs.appendFileSync(agentQueue, entry + '\n');
+    try { fs.chmodSync(agentQueue, 0o600); } catch {}
   } catch (err: any) {
     addChatEntry({ ts: new Date().toISOString(), role: 'agent', type: 'agent_error', error: `Failed to queue: ${err.message}` });
     agentStatus = 'idle';
@@ -1085,7 +1090,6 @@ async function start() {
           mode: browserManager.getConnectionMode(),
           uptime: Math.floor((Date.now() - startTime) / 1000),
           tabs: browserManager.getTabCount(),
-          currentUrl: browserManager.getCurrentUrl(),
           // Auth token for extension bootstrap. Safe: /health is localhost-only.
           // Previously served via .auth.json in extension dir, but that breaks
           // read-only .app bundles and codesigning. Extension reads token from here.
@@ -1094,7 +1098,6 @@ async function start() {
           agent: {
             status: agentStatus,
             runningFor: agentStartTime ? Date.now() - agentStartTime : null,
-            currentMessage,
             queueLength: messageQueue.length,
           },
           session: sidebarSession ? { id: sidebarSession.id, name: sidebarSession.name } : null,
@@ -1217,7 +1220,8 @@ async function start() {
           // Sync active tab from Chrome extension — detects manual tab switches
           const activeUrl = url.searchParams.get('activeUrl');
           if (activeUrl) {
-            browserManager.syncActiveTabByUrl(activeUrl);
+            const sanitized = sanitizeExtensionUrl(activeUrl);
+            if (sanitized) browserManager.syncActiveTabByUrl(sanitized);
           }
           const tabs = await browserManager.getTabListWithTitles();
           return new Response(JSON.stringify({ tabs }), {
@@ -1290,7 +1294,8 @@ async function start() {
         // Sync active tab BEFORE reading the ID — the user may have switched
         // tabs manually and the server's activeTabId is stale.
         if (extensionUrl) {
-          browserManager.syncActiveTabByUrl(extensionUrl);
+          const sanitized = sanitizeExtensionUrl(extensionUrl);
+          if (sanitized) browserManager.syncActiveTabByUrl(sanitized);
         }
         const msgTabId = browserManager?.getActiveTabId?.() ?? 0;
         const ts = new Date().toISOString();

browse/src/sidebar-agent.ts

@@ -223,7 +223,7 @@ async function handleStreamEvent(event: any, tabId?: number): Promise<void> {
   }
 }
 
-async function askClaude(queueEntry: any): Promise<void> {
+async function askClaude(queueEntry: QueueEntry): Promise<void> {
   const { prompt, args, stateFile, cwd, tabId } = queueEntry;
   const tid = tabId ?? 0;
 
@@ -313,9 +313,10 @@ async function askClaude(queueEntry: any): Promise<void> {
     // Timeout (default 300s / 5 min — multi-page tasks need time)
     const timeoutMs = parseInt(process.env.SIDEBAR_AGENT_TIMEOUT || '300000', 10);
     setTimeout(() => {
-      try { proc.kill(); } catch (killErr: any) {
-        console.warn(`[sidebar-agent] Tab ${tid}: Failed to kill timed-out process:`, killErr.message);
+      try { proc.kill('SIGTERM'); } catch (killErr: any) {
+        console.warn(`[sidebar-agent] Tab ${tid}: Failed to SIGTERM timed-out process:`, killErr.message);
       }
+      setTimeout(() => { try { proc.kill('SIGKILL'); } catch {} }, 3000);
       const timeoutMsg = stderrBuffer.trim()
         ? `Timed out after ${timeoutMs / 1000}s\nstderr: ${stderrBuffer.trim().slice(-500)}`
         : `Timed out after ${timeoutMs / 1000}s`;
@@ -327,6 +328,44 @@ async function askClaude(queueEntry: any): Promise<void> {
   });
 }
 
+// ─── Queue entry validation ─────────────────────────────────────
+
+interface QueueEntry {
+  prompt: string;
+  args?: string[];
+  stateFile?: string;
+  cwd?: string;
+  tabId?: number | null;
+  message?: string | null;
+  pageUrl?: string | null;
+  sessionId?: string | null;
+  ts?: string;
+}
+
+function isValidQueueEntry(e: unknown): e is QueueEntry {
+  if (typeof e !== 'object' || e === null) return false;
+  const obj = e as Record<string, unknown>;
+  // Required
+  if (typeof obj.prompt !== 'string' || obj.prompt.length === 0) return false;
+  // Optional typed fields
+  if (obj.args !== undefined && (!Array.isArray(obj.args) || !obj.args.every(a => typeof a === 'string'))) return false;
+  if (obj.stateFile !== undefined) {
+    if (typeof obj.stateFile !== 'string') return false;
+    if (obj.stateFile.includes('..')) return false;
+  }
+  if (obj.cwd !== undefined) {
+    if (typeof obj.cwd !== 'string') return false;
+    if (obj.cwd.includes('..')) return false;
+  }
+  // tabId: optional number or null (writer emits null when no tab)
+  if (obj.tabId !== undefined && obj.tabId !== null && typeof obj.tabId !== 'number') return false;
+  if (obj.message !== undefined && obj.message !== null && typeof obj.message !== 'string') return false;
+  if (obj.pageUrl !== undefined && obj.pageUrl !== null && typeof obj.pageUrl !== 'string') return false;
+  // sessionId: optional string or null (writer emits sessionId: ... || null)
+  if (obj.sessionId !== undefined && obj.sessionId !== null && typeof obj.sessionId !== 'string') return false;
+  return true;
+}
+
 // ─── Poll loop ───────────────────────────────────────────────────
 
 function countLines(): number {
@@ -357,12 +396,16 @@ async function poll() {
     const line = readLine(lastLine);
     if (!line) continue;
 
-    let entry: any;
-    try { entry = JSON.parse(line); } catch (err: any) {
+    let parsed: unknown;
+    try { parsed = JSON.parse(line); } catch (err: any) {
       console.warn(`[sidebar-agent] Skipping malformed queue entry at line ${lastLine}:`, line.slice(0, 80), err.message);
       continue;
     }
-    if (!entry.message && !entry.prompt) continue;
+    if (!isValidQueueEntry(parsed)) {
+      console.warn(`[sidebar-agent] Skipping invalid queue entry at line ${lastLine}: failed schema validation`);
+      continue;
+    }
+    const entry = parsed;
 
     const tid = entry.tabId ?? 0;
     // Skip if this tab already has an agent running — server queues per-tab
@@ -383,8 +426,9 @@ async function poll() {
 
 async function main() {
   const dir = path.dirname(QUEUE);
-  fs.mkdirSync(dir, { recursive: true });
+  fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
   if (!fs.existsSync(QUEUE)) fs.writeFileSync(QUEUE, '');
+  try { fs.chmodSync(QUEUE, 0o600); } catch {}
 
   lastLine = countLines();
   await refreshToken();

browse/src/snapshot.ts

@@ -313,11 +313,32 @@ export async function handleSnapshot(
   // ─── Annotated screenshot (-a) ────────────────────────────
   if (opts.annotate) {
     const screenshotPath = opts.outputPath || `${TEMP_DIR}/browse-annotated.png`;
-    // Validate output path (consistent with screenshot/pdf/responsive)
-    const resolvedPath = require('path').resolve(screenshotPath);
-    const safeDirs = [TEMP_DIR, process.cwd()];
-    if (!safeDirs.some((dir: string) => isPathWithin(resolvedPath, dir))) {
-      throw new Error(`Path must be within: ${safeDirs.join(', ')}`);
+    // Validate output path — resolve symlinks to prevent symlink traversal attacks
+    {
+      const nodePath = require('path') as typeof import('path');
+      const nodeFs = require('fs') as typeof import('fs');
+      const absolute = nodePath.resolve(screenshotPath);
+      const safeDirs = [TEMP_DIR, process.cwd()].map((d: string) => {
+        try { return nodeFs.realpathSync(d); } catch { return d; }
+      });
+      let realPath: string;
+      try {
+        realPath = nodeFs.realpathSync(absolute);
+      } catch (err: any) {
+        if (err.code === 'ENOENT') {
+          try {
+            const dir = nodeFs.realpathSync(nodePath.dirname(absolute));
+            realPath = nodePath.join(dir, nodePath.basename(absolute));
+          } catch {
+            realPath = absolute;
+          }
+        } else {
+          throw new Error(`Cannot resolve real path: ${screenshotPath} (${err.code})`);
+        }
+      }
+      if (!safeDirs.some((dir: string) => isPathWithin(realPath, dir))) {
+        throw new Error(`Path must be within: ${safeDirs.join(', ')}`);
+      }
     }
     try {
       // Inject overlay divs at each ref's bounding box