Merge remote-tracking branch 'origin/main' into garrytan/chrome-extension-ctrl

# Conflicts:
#	.agents/skills/gstack-browse/SKILL.md
#	.agents/skills/gstack-design-review/SKILL.md
#	.agents/skills/gstack-qa/SKILL.md
#	.agents/skills/gstack-setup-browser-cookies/SKILL.md
#	.agents/skills/gstack/SKILL.md
#	README.md
#	browse/src/cli.ts

browse/src/browser-manager.ts

@@ -329,7 +329,7 @@ export class BrowserManager {
 
     // Validate URL before allocating page to avoid zombie tabs on rejection
     if (url) {
-      validateNavigationUrl(url);
+      await validateNavigationUrl(url);
     }
 
     const page = await this.context.newPage();

browse/src/cli.ts

@@ -207,6 +207,34 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
   throw new Error(`Server failed to start within ${MAX_START_WAIT / 1000}s`);
 }
 
+/**
+ * Acquire an exclusive lockfile to prevent concurrent ensureServer() races (TOCTOU).
+ * Returns a cleanup function that releases the lock.
+ */
+function acquireServerLock(): (() => void) | null {
+  const lockPath = `${config.stateFile}.lock`;
+  try {
+    // O_CREAT | O_EXCL — fails if file already exists (atomic check-and-create)
+    const fd = fs.openSync(lockPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY);
+    fs.writeSync(fd, `${process.pid}\n`);
+    fs.closeSync(fd);
+    return () => { try { fs.unlinkSync(lockPath); } catch {} };
+  } catch {
+    // Lock already held — check if the holder is still alive
+    try {
+      const holderPid = parseInt(fs.readFileSync(lockPath, 'utf8').trim(), 10);
+      if (holderPid && isProcessAlive(holderPid)) {
+        return null; // Another live process holds the lock
+      }
+      // Stale lock — remove and retry
+      fs.unlinkSync(lockPath);
+      return acquireServerLock();
+    } catch {
+      return null;
+    }
+  }
+}
+
 async function ensureServer(): Promise<ServerState> {
   const state = readState();
 
@@ -244,9 +272,36 @@ async function ensureServer(): Promise<ServerState> {
     process.exit(1);
   }
 
-  // Need to (re)start (headless only — safe to auto-start)
-  console.error('[browse] Starting server...');
-  return startServer();
+  // Acquire lock to prevent concurrent restart races (TOCTOU)
+  const releaseLock = acquireServerLock();
+  if (!releaseLock) {
+    // Another process is starting the server — wait for it
+    console.error('[browse] Another instance is starting the server, waiting...');
+    const start = Date.now();
+    while (Date.now() - start < MAX_START_WAIT) {
+      const freshState = readState();
+      if (freshState && isProcessAlive(freshState.pid)) return freshState;
+      await Bun.sleep(200);
+    }
+    throw new Error('Timed out waiting for another instance to start the server');
+  }
+
+  try {
+    // Re-read state under lock in case another process just started the server
+    const freshState = readState();
+    if (freshState && isProcessAlive(freshState.pid)) {
+      return freshState;
+    }
+
+    // Kill the old server to avoid orphaned chromium processes
+    if (state && state.pid) {
+      await killServer(state.pid);
+    }
+    console.error('[browse] Starting server...');
+    return await startServer();
+  } finally {
+    releaseLock();
+  }
 }
 
 // ─── Command Dispatch ──────────────────────────────────────────
@@ -299,6 +354,11 @@ async function sendCommand(state: ServerState, command: string, args: string[],
     if (err.code === 'ECONNREFUSED' || err.code === 'ECONNRESET' || err.message?.includes('fetch failed')) {
       if (retries >= 1) throw new Error('[browse] Server crashed twice in a row — aborting');
       console.error('[browse] Server connection lost. Restarting...');
+      // Kill the old server to avoid orphaned chromium processes
+      const oldState = readState();
+      if (oldState && oldState.pid) {
+        await killServer(oldState.pid);
+      }
       const newState = await startServer();
       return sendCommand(newState, command, args, retries + 1);
     }

browse/src/meta-commands.ts

@@ -225,11 +225,11 @@ export async function handleMetaCommand(
       if (!url1 || !url2) throw new Error('Usage: browse diff <url1> <url2>');
 
       const page = bm.getPage();
-      validateNavigationUrl(url1);
+      await validateNavigationUrl(url1);
       await page.goto(url1, { waitUntil: 'domcontentloaded', timeout: 15000 });
       const text1 = await getCleanText(page);
 
-      validateNavigationUrl(url2);
+      await validateNavigationUrl(url2);
       await page.goto(url2, { waitUntil: 'domcontentloaded', timeout: 15000 });
       const text2 = await getCleanText(page);
 

browse/src/read-commands.ts

@@ -290,7 +290,21 @@ export async function handleReadCommand(
         localStorage: { ...localStorage },
         sessionStorage: { ...sessionStorage },
       }));
-      return JSON.stringify(storage, null, 2);
+      // Redact values that look like secrets (tokens, keys, passwords, JWTs)
+      const SENSITIVE_KEY = /(^|[_.-])(token|secret|key|password|credential|auth|jwt|session|csrf)($|[_.-])|api.?key/i;
+      const SENSITIVE_VALUE = /^(eyJ|sk-|sk_live_|sk_test_|pk_live_|pk_test_|rk_live_|sk-ant-|ghp_|gho_|github_pat_|xox[bpsa]-|AKIA[A-Z0-9]{16}|AIza|SG\.|Bearer\s|sbp_)/;
+      const redacted = JSON.parse(JSON.stringify(storage));
+      for (const storeType of ['localStorage', 'sessionStorage'] as const) {
+        const store = redacted[storeType];
+        if (!store) continue;
+        for (const [key, value] of Object.entries(store)) {
+          if (typeof value !== 'string') continue;
+          if (SENSITIVE_KEY.test(key) || SENSITIVE_VALUE.test(value)) {
+            store[key] = `[REDACTED — ${value.length} chars]`;
+          }
+        }
+      }
+      return JSON.stringify(redacted, null, 2);
     }
 
     case 'perf': {

browse/src/url-validation.ts

@@ -7,6 +7,7 @@ const BLOCKED_METADATA_HOSTS = new Set([
   '169.254.169.254',  // AWS/GCP/Azure instance metadata
   'fd00::',           // IPv6 unique local (metadata in some cloud setups)
   'metadata.google.internal', // GCP metadata
+  'metadata.azure.internal',  // Azure IMDS
 ]);
 
 /**
@@ -43,7 +44,23 @@ function isMetadataIp(hostname: string): boolean {
   return false;
 }
 
-export function validateNavigationUrl(url: string): void {
+/**
+ * Resolve a hostname to its IP addresses and check if any resolve to blocked metadata IPs.
+ * Mitigates DNS rebinding: even if the hostname looks safe, the resolved IP might not be.
+ */
+async function resolvesToBlockedIp(hostname: string): Promise<boolean> {
+  try {
+    const dns = await import('node:dns');
+    const { resolve4 } = dns.promises;
+    const addresses = await resolve4(hostname);
+    return addresses.some(addr => BLOCKED_METADATA_HOSTS.has(addr));
+  } catch {
+    // DNS resolution failed — not a rebinding risk
+    return false;
+  }
+}
+
+export async function validateNavigationUrl(url: string): Promise<void> {
   let parsed: URL;
   try {
     parsed = new URL(url);
@@ -64,4 +81,11 @@ export function validateNavigationUrl(url: string): void {
       `Blocked: ${parsed.hostname} is a cloud metadata endpoint. Access is denied for security.`
     );
   }
+
+  // DNS rebinding protection: resolve hostname and check if it points to metadata IPs
+  if (await resolvesToBlockedIp(hostname)) {
+    throw new Error(
+      `Blocked: ${parsed.hostname} resolves to a cloud metadata IP. Possible DNS rebinding attack.`
+    );
+  }
 }

browse/src/write-commands.ts

@@ -23,7 +23,7 @@ export async function handleWriteCommand(
     case 'goto': {
       const url = args[0];
       if (!url) throw new Error('Usage: browse goto <url>');
-      validateNavigationUrl(url);
+      await validateNavigationUrl(url);
       const response = await page.goto(url, { waitUntil: 'domcontentloaded', timeout: 15000 });
       const status = response?.status() || 'unknown';
       return `Navigated to ${url} (${status})`;