fix: pre-landing review findings (4 auto-fixes)

Addresses 4 findings from the Claude adversarial subagent on the
v1.6.0.0 security wave diff.  No user-visible behavior change; all
are defense-in-depth hardening of newly-introduced code.

1. GET /connect rate-limited (was POST-only) [HIGH conf 8/10]
   Attacker discovering the ngrok URL could probe unlimited GETs for
   daemon enumeration.  Now shares the global /connect counter.

2. ngrok listener leak on tunnel startup failure [MEDIUM conf 8/10]
   If ngrok.forward() resolved but tunnelListener.url() or the
   state-file write threw, the Bun listener was torn down but the
   ngrok session was leaked.  Fixed in BOTH /tunnel/start and
   BROWSE_TUNNEL=1 startup paths.

3. GSTACK_SKILL_ROOT path-traversal gate [MEDIUM conf 8/10]
   Symmetric with E3's GSTACK_SLUG regex gate — reject values
   containing '..' before interpolating into the welcome-page path.

4. SSE session registry pruning [LOW conf 7/10]
   pruneExpired() only checked 10 entries per mint call.  Now runs
   on every validate too, checks 20 entries, with a hard 10k cap as
   backstop.  Prevents registry growth under sustained extension
   reconnect pressure.

Tests remain green (56/56 in sse-session-cookie + dual-listener +
pair-agent-e2e suites).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

browse/src/server.ts

@@ -1571,7 +1571,16 @@ async function start() {
     // GET /connect — alive probe.  Unauth on both surfaces.  Used by /pair
     // and /tunnel/start to detect dead ngrok tunnels via the tunnel URL,
     // since /health is not tunnel-reachable under the dual-listener design.
+    //
+    // Shares the same rate limit as POST /connect — otherwise a tunnel
+    // caller can probe unlimited GETs and lock out nothing, which makes
+    // the endpoint a free daemon-enumeration surface.
     if (url.pathname === '/connect' && req.method === 'GET') {
+      if (!checkConnectRateLimit()) {
+        return new Response(JSON.stringify({ error: 'Rate limited' }), {
+          status: 429, headers: { 'Content-Type': 'application/json' },
+        });
+      }
       return new Response(JSON.stringify({ alive: true }), {
         status: 200, headers: { 'Content-Type': 'application/json' },
       });
@@ -1595,9 +1604,13 @@ async function start() {
           const homeDir = process.env.HOME || process.env.USERPROFILE || '/tmp';
           const projectWelcome = `${homeDir}/.gstack/projects/${slug}/designs/welcome-page-20260331/finalized.html`;
           if (fs.existsSync(projectWelcome)) return projectWelcome;
-          // Fallback: built-in welcome page from gstack install
-          const skillRoot = process.env.GSTACK_SKILL_ROOT || `${homeDir}/.claude/skills/gstack`;
-          const builtinWelcome = `${skillRoot}/browse/src/welcome.html`;
+          // Fallback: built-in welcome page from gstack install.  Reject
+          // SKILL_ROOT values containing '..' for the same defense-in-depth
+          // reason as the GSTACK_SLUG regex above.  Not exploitable today
+          // (env set at install time), but the gate is one check.
+          const rawSkillRoot = process.env.GSTACK_SKILL_ROOT || `${homeDir}/.claude/skills/gstack`;
+          if (rawSkillRoot.includes('..')) return null;
+          const builtinWelcome = `${rawSkillRoot}/browse/src/welcome.html`;
           if (fs.existsSync(builtinWelcome)) return builtinWelcome;
           return null;
         })();
@@ -1907,7 +1920,13 @@ async function start() {
             status: 200, headers: { 'Content-Type': 'application/json' },
           });
         } catch (err: any) {
+          // Clean up BOTH ngrok and the Bun listener on failure.  If
+          // ngrok.forward() succeeded but tunnelListener.url() or the
+          // state-file write threw, we'd otherwise leak an active ngrok
+          // session on the user's account.
+          try { if (tunnelListener) await tunnelListener.close(); } catch {}
           try { boundTunnel.stop(true); } catch {}
+          tunnelListener = null;
           return new Response(JSON.stringify({
             error: `Failed to open ngrok tunnel: ${err.message}`,
           }), { status: 500, headers: { 'Content-Type': 'application/json' } });
@@ -2769,7 +2788,12 @@ async function start() {
         fs.renameSync(tmpState, config.stateFile);
       } catch (err: any) {
         console.error(`[browse] Failed to start tunnel: ${err.message}`);
+        // Same cleanup as /tunnel/start's error path: tear down BOTH
+        // ngrok and the Bun listener so we don't leak an ngrok session
+        // if the error happened after ngrok.forward() resolved.
+        try { if (tunnelListener) await tunnelListener.close(); } catch {}
         try { if (boundTunnel) boundTunnel.stop(true); } catch {}
+        tunnelListener = null;
       }
     }
   }

browse/src/sse-session-cookie.ts

@@ -30,6 +30,7 @@ interface Session {
 }
 
 const TTL_MS = 30 * 60 * 1000; // 30 minutes
+const MAX_SESSIONS = 10_000; // Upper bound on registry size
 const sessions = new Map<string, Session>();
 
 export const SSE_COOKIE_NAME = 'gstack_sse';
@@ -47,14 +48,20 @@ export function mintSseSessionToken(): { token: string; expiresAt: number } {
 
 /**
  * Validate a token. Returns true only if the token exists AND is not expired.
- * Expired tokens are lazily removed.
+ * Expired tokens are lazily removed, and we opportunistically prune a few
+ * additional expired entries on every validate so the registry can't grow
+ * unboundedly under sustained mint + reconnect pressure.
  */
 export function validateSseSessionToken(token: string | null | undefined): boolean {
   if (!token) return false;
   const s = sessions.get(token);
-  if (!s) return false;
+  if (!s) {
+    pruneExpired(Date.now());
+    return false;
+  }
   if (Date.now() > s.expiresAt) {
     sessions.delete(token);
+    pruneExpired(Date.now());
     return false;
   }
   return true;
@@ -95,13 +102,21 @@ export function buildSseClearCookie(): string {
 }
 
 function pruneExpired(now: number): void {
-  // Opportunistic cleanup: at most 10 per mint call so we don't stall
-  // on a massive registry. O(1) amortized.
+  // 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
+  // AND on every validate so a steady reconnect flow can't outpace it.
   let checked = 0;
   for (const [token, session] of sessions) {
-    if (checked++ >= 10) break;
+    if (checked++ >= 20) break;
     if (session.expiresAt <= now) sessions.delete(token);
   }
+  // Hard cap as a backstop — if something still gets past opportunistic
+  // cleanup (e.g., all unexpired but registry enormous), drop the oldest.
+  while (sessions.size > MAX_SESSIONS) {
+    const first = sessions.keys().next().value;
+    if (!first) break;
+    sessions.delete(first);
+  }
 }
 
 // Test-only reset.