From b141fb351108b8538fb7d9ba9180cacb854ac763 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 12:43:24 -0700 Subject: [PATCH] =?UTF-8?q?fix(pair-agent):=20tunnel=20activation=20is=20c?= =?UTF-8?q?onsent-gated=20=E2=80=94=20and=20the=20receipt's=20consent=20cl?= =?UTF-8?q?aim=20is=20now=20real?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tunnel egress receipts have claimed consent: 'pair_agent=on' since v1.63 while no such key or gate existed — ngrok installed+authed was enough for the CLI to auto-start an internet-facing tunnel. isPairAgentEnabled() (fail- closed, env-overridable) now gates all three activation points: CLI auto-start, POST /tunnel/start (refuses with the enable hint), and the BROWSE_TUNNEL=1 startup bind. Consent-on-first-use, not silent breakage: the /pair-agent skill asks once (one-way-door posture), sets pair_agent via gstack-config (registered with on|off validation, default off), and never asks again; direct API callers get the same hint in the refusal. Adapted from the fork's gate: their reader targeted config.json, which on main would have made the gate silently un-enableable — ours reads the canonical ~/.gstack/config.yaml with the JSON shape as fallback, pinned by tests either way (11 cases, gate wiring tripwires included). Ported from time-attack/gstack (GStack 2), store adaptation ours. Co-authored-by: Sina Matian Co-Authored-By: Claude Fable 5 --- bin/gstack-config | 5 + browse/src/cli.ts | 10 +- browse/src/config.ts | 35 +++++++ browse/src/server.ts | 18 +++- browse/test/pair-agent-optin-gate.test.ts | 108 ++++++++++++++++++++++ pair-agent/SKILL.md.tmpl | 22 ++++- 6 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 browse/test/pair-agent-optin-gate.test.ts diff --git a/bin/gstack-config b/bin/gstack-config index 01176c44c..1248788e5 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -133,6 +133,7 @@ lookup_default() { redact_repo_visibility) echo "" ;; # empty → fall through to gh/glab detection redact_prepush_hook) echo "false" ;; + pair_agent) echo "off" ;; # remote tunnel consent — fail-closed until /pair-agent asks # Brain-aware planning (v1.48 / T5+T10+T16). Defaults documented inline: # brain_trust_policy@ — unset on fresh install; setup-gbrain # writes 'personal' for local engines, @@ -310,6 +311,10 @@ case "${1:-}" in echo "Warning: redact_prepush_hook '$VALUE' not recognized. Valid values: true, false. Using false." >&2 VALUE="false" fi + if [ "$KEY" = "pair_agent" ] && [ "$VALUE" != "on" ] && [ "$VALUE" != "off" ]; then + echo "Warning: pair_agent '$VALUE' not recognized. Valid values: on, off. Using off." >&2 + VALUE="off" + fi if [ "$KEY" = "plan_tune_hooks" ] && [ "$VALUE" != "prompt" ] && [ "$VALUE" != "yes" ] && [ "$VALUE" != "no" ]; then echo "Warning: plan_tune_hooks '$VALUE' not recognized. Valid values: prompt, yes, no. Using prompt." >&2 VALUE="prompt" diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 59327b792..bd231743f 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -14,7 +14,7 @@ import * as path from 'path'; import { spawn as nodeSpawn } from 'child_process'; import { safeUnlink, safeUnlinkQuiet, safeKill, isProcessAlive } from './error-handling'; import { writeSecureFile, mkdirSecure } from './file-permissions'; -import { resolveConfig, ensureStateDir, readVersionHash } from './config'; +import { resolveConfig, ensureStateDir, readVersionHash, isPairAgentEnabled } from './config'; import { parseProxyConfig, computeConfigHash, ProxyConfigError } from './proxy-config'; import { redactProxyUrl } from './proxy-redact'; import { spawnTerminalAgent } from './terminal-agent-control'; @@ -898,8 +898,12 @@ async function handlePairAgent(state: ServerState, args: string[]): Promise | null): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-pair-')); + tmpHomes.push(dir); + if (config !== null) { + // Canonical store: flat YAML lines, the shape bin/gstack-config writes. + const yaml = Object.entries(config).map(([k, v]) => `${k}: ${v}`).join('\n') + '\n'; + fs.writeFileSync(path.join(dir, 'config.yaml'), yaml); + } + process.env.GSTACK_HOME = dir; + delete process.env.GSTACK_PAIR_AGENT; + return dir; +} + +afterEach(() => { + for (const k of ['GSTACK_HOME', 'GSTACK_PAIR_AGENT'] as const) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + while (tmpHomes.length) fs.rmSync(tmpHomes.pop()!, { recursive: true, force: true }); +}); + +describe('isPairAgentEnabled — fail-closed default', () => { + test('OFF when no config store exists', () => { + tmpHomeWith(null); + expect(isPairAgentEnabled()).toBe(false); + }); + + test('OFF when config has no pair_agent key', () => { + tmpHomeWith({ telemetry: 'off' }); + expect(isPairAgentEnabled()).toBe(false); + }); + + test('ON via the config.json fallback shape too', () => { + const dir = tmpHomeWith(null); + fs.writeFileSync(path.join(dir, 'config.json'), JSON.stringify({ pair_agent: 'on' })); + expect(isPairAgentEnabled()).toBe(true); + }); + + test('OFF when pair_agent is explicitly "off"', () => { + tmpHomeWith({ pair_agent: 'off' }); + expect(isPairAgentEnabled()).toBe(false); + }); + + test('ON only when pair_agent is exactly "on"', () => { + tmpHomeWith({ pair_agent: 'on' }); + expect(isPairAgentEnabled()).toBe(true); + }); + + test('OFF when the store is malformed (fail-closed)', () => { + const dir = tmpHomeWith(null); + fs.writeFileSync(path.join(dir, 'config.yaml'), 'pair_agent: banana\n'); + fs.writeFileSync(path.join(dir, 'config.json'), '{ not json'); + expect(isPairAgentEnabled()).toBe(false); + }); + + test('env override wins: GSTACK_PAIR_AGENT=on forces ON even with config off', () => { + tmpHomeWith({ pair_agent: 'off' }); + process.env.GSTACK_PAIR_AGENT = 'on'; + expect(isPairAgentEnabled()).toBe(true); + }); + + test('env override wins: GSTACK_PAIR_AGENT=off forces OFF even with config on', () => { + tmpHomeWith({ pair_agent: 'on' }); + process.env.GSTACK_PAIR_AGENT = 'off'; + expect(isPairAgentEnabled()).toBe(false); + }); +}); + +describe('gate wiring — every tunnel activation point consults the guard', () => { + test('CLI auto-start is gated (never auto-starts when disabled)', () => { + // pairEnabled short-circuits the ngrok probe so the tunnel can't auto-start. + expect(CLI_SRC).toContain('const pairEnabled = isPairAgentEnabled();'); + expect(CLI_SRC).toContain('const ngrokAvailable = pairEnabled && isNgrokAvailable();'); + }); + + test('/tunnel/start refuses with the enable hint when disabled', () => { + const startIdx = SERVER_SRC.indexOf("url.pathname === '/tunnel/start'"); + const block = SERVER_SRC.slice(startIdx, startIdx + 1200); + expect(block).toContain('if (!isPairAgentEnabled())'); + expect(block).toContain('gstack-config set pair_agent on'); + }); + + test('BROWSE_TUNNEL=1 startup skips tunnel bind when disabled', () => { + expect(SERVER_SRC).toContain("process.env.BROWSE_TUNNEL === '1' && !isPairAgentEnabled()"); + }); +}); diff --git a/pair-agent/SKILL.md.tmpl b/pair-agent/SKILL.md.tmpl index 75ed42d59..0393c236f 100644 --- a/pair-agent/SKILL.md.tmpl +++ b/pair-agent/SKILL.md.tmpl @@ -125,7 +125,27 @@ using the generic remote flow instead. ### If different machine (option B): -First, detect ngrok status: +**Consent gate (once per machine).** The tunnel exposes this browser beyond +the machine, so it is OFF until the user opts in — the daemon refuses +`/tunnel/start` and `BROWSE_TUNNEL=1` otherwise. Check the standing consent: + +```bash +~/.claude/skills/gstack/bin/gstack-config get pair_agent 2>/dev/null || echo "unset" +``` + +If the value is not `on`, ask via AskUserQuestion (one-way-door posture — +this opens a path from the internet to the local browser): + +> "Remote pairing runs an ngrok tunnel from the internet to this machine's +> browser (locked to a 26-command allowlist + scoped token, but still an +> exposure). Enable pair-agent on this machine?" + +Options: A) Enable — run `~/.claude/skills/gstack/bin/gstack-config set pair_agent on`, confirm it reads back `on`, and continue. B) No — stop here; local pairing (option A above) still works. + +If the value is already `on`, say nothing and continue — consent stands until +`gstack-config set pair_agent off`. + +Then detect ngrok status: ```bash which ngrok 2>/dev/null && echo "NGROK_INSTALLED" || echo "NGROK_NOT_INSTALLED"