mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-29 17:30:40 +02:00
* fix(browse): revokeToken deletes ALL tokens for a clientId, not the first Map hit
revokeToken deleted the first Map entry matching the clientId and returned
true. After a normal pairing, two entries share one clientId: the spent setup
key (kept by exchangeSetupKey for idempotent re-exchange) and the session
token, in that insertion order. Revoke ate the setup key, reported success,
and the live session survived: DELETE /token/<id> returned a false 200 while
/agents kept listing the agent. Worse, an unspent setup key created after the
session survived revoke, so a "revoked" agent could POST /connect and mint a
fresh session within the key's 5-minute validity window.
revokeToken now deletes every matching entry and returns the delete count
(truthy-compatible with the old boolean). The DELETE /token handler logs
"Revoked N token(s)" and returns tokens_deleted so the multi-token class
stays visible; revokeSkillToken wraps Boolean() to keep its documented
contract. Regression tests pin shapes a (spent-key shadowing), b (re-grant
hole), c (multiple pending keys), and bystander isolation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(browse): tunnel revoke/agents CLI with post-revoke verification
`$B tunnel revoke <name>` was documented in the instruction block,
pair-agent/SKILL.md, and REMOTE_BROWSER_ACCESS.md but implemented nowhere:
the CLI forwarded it to the daemon as Unknown command 'tunnel', and nothing
in the repo called DELETE /token/:clientId or GET /agents.
New pre-server short-circuit (#2254 pattern: tokens are memory-only, never
boot a daemon to revoke against it). `tunnel revoke <name>` DELETEs the
token, prints the deleted count ("(count unknown)" for old daemons that
answer {revoked} without tokens_deleted), then RE-READS GET /agents to prove
the agent is gone. The still-listed branch is the version-skew net: a new
CLI against a still-running old daemon with the first-match revoke bug exits
1 and says to re-run (each old-daemon call deletes the next match) or stop.
An alive pid with an unreachable port reports "Could not reach daemon"
(exit 1), never a false "no daemon". `tunnel agents` lists sessions plus
pending (unexchanged) setup keys, which GET /agents now exposes via
listTokens({includeSetup}) — without them the revocation view was blind to
a paired-but-never-connected agent. Setup-key tokens never leave the server.
DELETE /token/ now decodeURIComponents the clientId (400 on malformed
encoding) so CLI-encoded names round-trip.
Tests: subprocess CLI coverage (usage paths, no-daemon exit 0 without
spawning, live pair/connect/revoke loop, pending-key listing), stub-daemon
pins for the skew and unreachable branches, and e2e pins for revoke-all
semantics, percent-encoded ids, and the second-DELETE-is-404 regression.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(browse): CLI always sends explicit pair scopes via shared DEFAULT_PAIR_SCOPES
The effective pairing default lived in two places: the CLI omitted scopes
unless --restrict was passed, and the server filled in its own literal.
handlePairAgent now always sends an explicit scopes list and both sides
reference one exported constant, DEFAULT_PAIR_SCOPES, so the default cannot
silently drift again (pinned by a server-auth source tripwire).
Three input traps closed in the same surface:
- Bare --restrict (or --restrict swallowing the next flag) parsed as "no
restriction" and silently granted FULL access, the opposite of the user's
intent. validatePairAgentFlags rejects it pre-server, before any consent
gate, so an arg error never boots a daemon.
- A scopes list could smuggle the control scope past the explicit flag:
--restrict "read,control" minted a control-scoped session with no
--control. /pair now 400s on control in a scopes list without the control
flag, and the CLI points the user at --control.
- Option typos validated only at exchange time: createSetupKey stored any
scope string and any rateLimit, so /pair returned 200 with a poisoned
setup key whose failure surfaced to the REMOTE agent at /connect as a
misleading "Invalid request body". Shared validation now runs in both
creators and throws typed InvalidScopeError; /pair and /token 400 with the
message, naming the bad scope or negative rateLimit. Also
`opts.rateLimit || 10` became `?? 10` so the documented "0 = unlimited"
survives the /pair path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(browse): 403 hint stops recommending --admin; invariant names both scope defaults
The scope-denied hint told restricted agents to "re-pair with --admin for
eval/cookies/storage" — but --admin is a legacy alias for --control, so
following it over-granted browser-wide destructive commands on top of the
admin scope the default already carries. The hint now matches the CLI's
sibling wording: re-pair without --restrict for page access, --control for
browser control.
Registry invariant #2 claimed "admin scope denied by default" three releases
after b73f3644 deliberately made /pair grant admin. It now names BOTH
defaults precisely (registry API functions default read+write; the /pair
ceremony grants DEFAULT_PAIR_SCOPES) so the header cannot lie one layer down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(pair-agent): document the full-access default, --restrict, and real revocation
The pairing docs still described the pre-b73f3644 model: read+write default,
--admin as the opt-in for JS/cookies/storage. Reality for three releases:
/pair grants read+write+admin+meta (the pairing ceremony is the trust
boundary) and --admin is a legacy alias for --control. A user following the
skill believed they granted a sandboxed session and actually granted JS
execution on their logged-in browser.
pair-agent/SKILL.md.tmpl (SKILL.md regenerated in this commit) now states
the real default, the tunnel-allowlist nuance (eval works remotely; the
js/cookies/storage commands are local-only), --restrict for sandboxed
sessions with an untrusted-content advisory (scope caps prompt-injection
blast radius), and --control for browser-wide ops. "Revoking access"
documents the now-real tunnel revoke (deletes session + pending setup keys,
verifies against the agent list) and tunnel agents, and replaces the
never-implemented `tunnel rotate` with `$B stop` — tokens are memory-only,
so a daemon restart already rotates everything.
REMOTE_BROWSER_ACCESS.md: /connect example shows the real default scopes,
the scope table gains the control row, the 403 hint row matches the new
server wording, and the false claim that /sidebar-chat is on the tunnel
allowlist is gone (TUNNEL_PATHS is /connect + /command; /sidebar-chat no
longer exists in server.ts at all). ARCHITECTURE.md drops the same phantom
endpoint from the allowlist prose and endpoint table.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* v1.68.2.0: revoke-all, real tunnel revoke, truthful pairing docs
Version slot allocated against the live remote via bin/gstack-next-version
(clean patch bump from 1.68.1.0, no collision). CHANGELOG entry covers the
revoke-all fix, the new tunnel revoke/agents CLI, the explicit-scopes wire
contract, and the pairing-docs truth pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(browse): adversarial-review hardening — 6 findings fixed, regression-pinned
Pre-push adversarial review (4 lenses, refute-style verification: 13 raw
findings, 7 refuted, 6 confirmed) caught these; each fix carries a pin:
1. --restrict=read (equals form) sailed past validatePairAgentFlags —
hasFlag/parseFlag are exact-token matches — so the user asked for a
read-only sandbox and silently got FULL access: the exact failure mode
this branch claims to close. The equals form is now a hard error before
any server work.
2. handleTunnel trimmed the agent name but clientIds are stored verbatim,
so a space-padded agent was unrevocable by the documented kill switch
(trimmed DELETE 404'd while the grant stayed live). Names now pass
through verbatim; the live-daemon test revokes ' padded'.
3. The sole pin for "CLI always sends explicit scopes" passed vacuously on
a simulated revert: toContain('DEFAULT_PAIR_SCOPES') was satisfied by a
comment. The tripwire now matches the code shape with a regex and bans
the conditional spread formatting-insensitively.
4. The rewritten 403 scope hint was unpinned — new e2e asserts it names
--restrict and --control and never --admin.
5. tunnelRevoke's verify-failure and HTTP-error branches and tunnelAgents'
unreadable-list branch had no coverage — three stub-daemon pins added
(an unreadable list must never render as "No paired agents").
6. CHANGELOG claimed "40+ new test cases"; the honest count is 35.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
443 lines
24 KiB
TypeScript
443 lines
24 KiB
TypeScript
/**
|
|
* Server auth security tests — verify security remediation in server.ts
|
|
*
|
|
* Tests are source-level: they read server.ts and verify that auth checks,
|
|
* CORS restrictions, and token removal are correctly in place.
|
|
*/
|
|
|
|
import { describe, test, expect } from 'bun:test';
|
|
import * as fs from 'fs';
|
|
import * as path from 'path';
|
|
|
|
const SERVER_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8');
|
|
const CLI_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/cli.ts'), 'utf-8');
|
|
|
|
// Helper: extract a block of source between two markers
|
|
function sliceBetween(source: string, startMarker: string, endMarker: string): string {
|
|
const startIdx = source.indexOf(startMarker);
|
|
if (startIdx === -1) throw new Error(`Marker not found: ${startMarker}`);
|
|
const endIdx = source.indexOf(endMarker, startIdx + startMarker.length);
|
|
if (endIdx === -1) throw new Error(`End marker not found: ${endMarker}`);
|
|
return source.slice(startIdx, endIdx);
|
|
}
|
|
|
|
describe('Server auth security', () => {
|
|
// Test 1 (IRON RULE, inverted in v1.62): /health NEVER serves a token in
|
|
// ANY mode. Both carve-outs (headed-mode disjunct + chrome-extension://
|
|
// Origin disjunct) are gone. Token bootstrap moved to POST /extension-token
|
|
// with a pinned extension Origin.
|
|
test('/health never serves a token — no headed-mode or chrome-extension carve-out', () => {
|
|
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
|
expect(healthBlock).not.toContain('token: authToken');
|
|
expect(healthBlock).not.toContain("getConnectionMode() === 'headed'");
|
|
expect(healthBlock).not.toContain("startsWith('chrome-extension://')");
|
|
});
|
|
|
|
// Test 1a: the pinned-origin bootstrap endpoint exists and gates on both
|
|
// the exact extension Origin and a loopback Host.
|
|
test('POST /extension-token gates on pinned Origin and loopback Host', () => {
|
|
const tokenBlock = sliceBetween(SERVER_SRC, "url.pathname === '/extension-token'", "url.pathname === '/health'");
|
|
expect(tokenBlock).toContain('GSTACK_EXTENSION_ID');
|
|
expect(tokenBlock).toContain('token: authToken');
|
|
// Host is parsed to a hostname (arrives as '127.0.0.1:34567'), never
|
|
// compared literally against the raw header.
|
|
expect(tokenBlock).toContain('.hostname');
|
|
expect(tokenBlock).toContain("'127.0.0.1'");
|
|
expect(tokenBlock).toContain("'localhost'");
|
|
expect(tokenBlock).toContain('403');
|
|
});
|
|
|
|
// Test 1b: /health does not expose sensitive browsing state
|
|
test('/health does not expose currentUrl or currentMessage', () => {
|
|
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
|
expect(healthBlock).not.toContain('currentUrl');
|
|
expect(healthBlock).not.toContain('currentMessage');
|
|
});
|
|
|
|
// Test 1c: newtab must check domain restrictions (CSO finding #5)
|
|
// Domain check for newtab is now unified with goto in the scope check section:
|
|
// (command === 'goto' || command === 'newtab') && args[0] → checkDomain
|
|
test('newtab enforces domain restrictions', () => {
|
|
const scopeBlock = sliceBetween(SERVER_SRC, "Scope check (for scoped tokens)", "Pin to a specific tab");
|
|
expect(scopeBlock).toContain("command === 'newtab'");
|
|
expect(scopeBlock).toContain('checkDomain');
|
|
expect(scopeBlock).toContain('Domain not allowed');
|
|
});
|
|
|
|
// Test 1d: validateAuth compares the bearer token in CONSTANT TIME with a
|
|
// length gate. A revert to `header === \`Bearer ${authToken}\`` keeps
|
|
// accept/reject behavior identical (functional tests still pass) but silently
|
|
// reintroduces the byte-by-byte timing side-channel; dropping the length gate
|
|
// makes timingSafeEqual throw RangeError (500 instead of 401) on a wrong-length
|
|
// token. Pin both properties, mirroring the token-registry sibling guard.
|
|
test('validateAuth uses constant-time comparison with a length gate', () => {
|
|
const authBlock = sliceBetween(SERVER_SRC, 'function validateAuth(req: Request): boolean {', '// Factory-scoped shutdown');
|
|
expect(authBlock).toContain('crypto.timingSafeEqual');
|
|
expect(authBlock).toContain('got.length === want.length');
|
|
// The null-header guard must remain (Buffer.from(null) would otherwise throw).
|
|
expect(authBlock).toContain('header === null');
|
|
// The raw === comparison of the header against the bearer string must be gone.
|
|
expect(authBlock).not.toContain('header === `Bearer ${authToken}`');
|
|
});
|
|
|
|
// Test 2: /refs endpoint requires auth via validateAuth
|
|
test('/refs endpoint requires authentication', () => {
|
|
const refsBlock = sliceBetween(SERVER_SRC, "url.pathname === '/refs'", "url.pathname === '/activity/stream'");
|
|
expect(refsBlock).toContain('validateAuth');
|
|
});
|
|
|
|
// Test 3: /refs has no wildcard CORS header
|
|
test('/refs has no wildcard CORS header', () => {
|
|
const refsBlock = sliceBetween(SERVER_SRC, "url.pathname === '/refs'", "url.pathname === '/activity/stream'");
|
|
expect(refsBlock).not.toContain("'*'");
|
|
});
|
|
|
|
// Test 4: /activity/history requires auth via validateAuth
|
|
test('/activity/history requires authentication', () => {
|
|
const historyBlock = sliceBetween(SERVER_SRC, "url.pathname === '/activity/history'", 'Batch endpoint');
|
|
expect(historyBlock).toContain('validateAuth');
|
|
});
|
|
|
|
// Test 5: /activity/history has no wildcard CORS header
|
|
test('/activity/history has no wildcard CORS header', () => {
|
|
const historyBlock = sliceBetween(SERVER_SRC, "url.pathname === '/activity/history'", 'Batch endpoint');
|
|
expect(historyBlock).not.toContain("'*'");
|
|
});
|
|
|
|
// Test 6: /activity/stream requires auth via Bearer OR view-only session cookie
|
|
// (N1: ?token= query param was dropped in v1.6.0.0 — URLs leak to logs/referer)
|
|
test('/activity/stream requires authentication with inline token check', () => {
|
|
const streamBlock = sliceBetween(SERVER_SRC, "url.pathname === '/activity/stream'", "url.pathname === '/activity/history'");
|
|
expect(streamBlock).toContain('validateAuth');
|
|
expect(streamBlock).toContain('validateSseSessionToken');
|
|
// Should not have wildcard CORS for the SSE stream
|
|
expect(streamBlock).not.toContain("Access-Control-Allow-Origin': '*'");
|
|
// ?token= query param must NOT be accepted anymore
|
|
expect(streamBlock).not.toContain("searchParams.get('token')");
|
|
});
|
|
|
|
// Test 7: /command accepts scoped tokens (not just root)
|
|
// This was the Wintermute bug — /command was BELOW the blanket validateAuth gate
|
|
// which only accepts root tokens. Scoped tokens got 401'd before reaching getTokenInfo.
|
|
test('/command endpoint sits ABOVE the blanket root-only auth gate', () => {
|
|
const commandIdx = SERVER_SRC.indexOf("url.pathname === '/command'");
|
|
const blanketGateIdx = SERVER_SRC.indexOf("Auth-required endpoints (root token only)");
|
|
// /command must appear BEFORE the blanket gate in source order
|
|
expect(commandIdx).toBeGreaterThan(0);
|
|
expect(blanketGateIdx).toBeGreaterThan(0);
|
|
expect(commandIdx).toBeLessThan(blanketGateIdx);
|
|
});
|
|
|
|
// Test 7b: /command uses getTokenInfo (accepts scoped tokens), not validateAuth (root-only)
|
|
test('/command uses getTokenInfo for auth, not validateAuth', () => {
|
|
const commandBlock = sliceBetween(SERVER_SRC, "url.pathname === '/command'", "Auth-required endpoints");
|
|
expect(commandBlock).toContain('getTokenInfo');
|
|
expect(commandBlock).not.toContain('validateAuth');
|
|
});
|
|
|
|
// Test 8: /tunnel/start requires root token
|
|
test('/tunnel/start requires root token', () => {
|
|
const tunnelBlock = sliceBetween(SERVER_SRC, "/tunnel/start", "Refs endpoint");
|
|
expect(tunnelBlock).toContain('isRootRequest');
|
|
expect(tunnelBlock).toContain('Root token required');
|
|
});
|
|
|
|
// Test 8b: /tunnel/start checks ngrok native config paths
|
|
test('/tunnel/start reads ngrok native config files', () => {
|
|
const tunnelBlock = sliceBetween(SERVER_SRC, "/tunnel/start", "Refs endpoint");
|
|
expect(tunnelBlock).toContain("'ngrok.yml'");
|
|
expect(tunnelBlock).toContain('authtoken');
|
|
});
|
|
|
|
// Test 8c: /tunnel/start returns already_active if tunnel is running
|
|
test('/tunnel/start returns already_active when tunnel exists', () => {
|
|
const tunnelBlock = sliceBetween(SERVER_SRC, "/tunnel/start", "Refs endpoint");
|
|
expect(tunnelBlock).toContain('already_active');
|
|
expect(tunnelBlock).toContain('tunnelActive');
|
|
});
|
|
|
|
// Test 9: /pair requires root token
|
|
test('/pair requires root token', () => {
|
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "/tunnel/start");
|
|
expect(pairBlock).toContain('isRootRequest');
|
|
expect(pairBlock).toContain('Root token required');
|
|
});
|
|
|
|
// Test 9b: /pair calls createSetupKey (not createToken)
|
|
test('/pair creates setup keys, not session tokens', () => {
|
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "/tunnel/start");
|
|
expect(pairBlock).toContain('createSetupKey');
|
|
expect(pairBlock).not.toContain('createToken');
|
|
});
|
|
|
|
// Test 10: tab ownership check happens before command dispatch
|
|
test('tab ownership check runs before command dispatch for scoped tokens', () => {
|
|
const handleBlock = sliceBetween(SERVER_SRC, "async function handleCommand", "Block mutation commands while watching");
|
|
expect(handleBlock).toContain('checkTabAccess');
|
|
expect(handleBlock).toContain('Tab not owned by your agent');
|
|
});
|
|
|
|
// Test 10a: tab gate is gated on own-only, not on isWrite
|
|
// Regression test for v1.20.0.0 footgun fix. Pre-fix the gate fired for
|
|
// any write command from any non-root token, which 403'd local skill
|
|
// spawns trying to drive the user's natural (unowned) tabs. The bundled
|
|
// hackernews-frontpage skill failed identically. The fix narrows the
|
|
// gate to `tabPolicy === 'own-only'` so pair-agent tunnel tokens stay
|
|
// strict while local shared-policy tokens (skill spawns) get unblocked.
|
|
test('tab gate predicate is own-only-scoped, not write-scoped', () => {
|
|
const handleBlock = sliceBetween(SERVER_SRC, "async function handleCommand", "Block mutation commands while watching");
|
|
// The gate condition must include the own-only check.
|
|
expect(handleBlock).toContain("tabPolicy === 'own-only'");
|
|
// It must NOT depend on WRITE_COMMANDS in the gate predicate (only inside
|
|
// the checkTabAccess call's isWrite arg, which is informational). The
|
|
// surrounding `if (...) {` for the gate must use `tabPolicy === 'own-only'`
|
|
// as the trigger, not `WRITE_COMMANDS.has(command) || ...`.
|
|
const gateLine = handleBlock.split('\n').find(l =>
|
|
l.includes("command !== 'newtab'") &&
|
|
l.includes('tokenInfo') &&
|
|
l.includes('tabPolicy')
|
|
);
|
|
expect(gateLine).toBeTruthy();
|
|
expect(gateLine).not.toMatch(/WRITE_COMMANDS\.has\(command\)\s*\|\|/);
|
|
});
|
|
|
|
// Test 10b: chain command pre-validates subcommand scopes
|
|
test('chain handler checks scope for each subcommand before dispatch', () => {
|
|
const metaSrc = fs.readFileSync(path.join(import.meta.dir, '../src/meta-commands.ts'), 'utf-8');
|
|
const chainBlock = metaSrc.slice(
|
|
metaSrc.indexOf("case 'chain':"),
|
|
metaSrc.indexOf("case 'diff':")
|
|
);
|
|
expect(chainBlock).toContain('checkScope');
|
|
expect(chainBlock).toContain('Chain rejected');
|
|
expect(chainBlock).toContain('tokenInfo');
|
|
});
|
|
|
|
// Test 10c: handleMetaCommand accepts tokenInfo parameter
|
|
test('handleMetaCommand accepts tokenInfo for chain scope checking', () => {
|
|
const metaSrc = fs.readFileSync(path.join(import.meta.dir, '../src/meta-commands.ts'), 'utf-8');
|
|
const sig = metaSrc.slice(
|
|
metaSrc.indexOf('export async function handleMetaCommand'),
|
|
metaSrc.indexOf('): Promise<string>')
|
|
);
|
|
expect(sig).toContain('tokenInfo');
|
|
});
|
|
|
|
// Test 10d: server passes tokenInfo to handleMetaCommand
|
|
// v1.35.0.0: shutdown is now factory-scoped; the call site uses shutdownFn,
|
|
// a thin wrapper that delegates to activeShutdown (set by buildFetchHandler).
|
|
test('server passes tokenInfo to handleMetaCommand', () => {
|
|
expect(SERVER_SRC).toContain('handleMetaCommand(command, args, browserManager, shutdownFn, tokenInfo,');
|
|
});
|
|
|
|
// Test 10e: activity attribution includes clientId
|
|
test('activity events include clientId from token', () => {
|
|
const commandStartBlock = sliceBetween(SERVER_SRC, "Activity: emit command_start", "try {");
|
|
expect(commandStartBlock).toContain('clientId: tokenInfo?.clientId');
|
|
});
|
|
|
|
// ─── Tunnel liveness verification ─────────────────────────────
|
|
|
|
// Test 11a: /pair endpoint probes tunnel before returning tunnel_url
|
|
test('/pair verifies tunnel is alive before returning tunnel_url', () => {
|
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "url.pathname === '/tunnel/start'");
|
|
// Must probe the tunnel URL
|
|
expect(pairBlock).toContain('verifiedTunnelUrl');
|
|
expect(pairBlock).toContain('Tunnel probe failed');
|
|
expect(pairBlock).toContain('marking tunnel as dead');
|
|
// Must tear down tunnel state on failure (via closeTunnel helper — clears
|
|
// tunnelActive, tunnelUrl, tunnelListener, and the tunnel Bun.serve listener)
|
|
expect(pairBlock).toContain('closeTunnel()');
|
|
});
|
|
|
|
// Test 11b: /pair returns null tunnel_url when tunnel is dead
|
|
test('/pair returns verified tunnel URL, not raw tunnelActive flag', () => {
|
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "url.pathname === '/tunnel/start'");
|
|
// Should use verifiedTunnelUrl (probe result), not raw tunnelUrl
|
|
expect(pairBlock).toContain('tunnel_url: verifiedTunnelUrl');
|
|
// Must NOT use raw tunnelActive check for the response
|
|
expect(pairBlock).not.toContain('tunnel_url: tunnelActive ? tunnelUrl');
|
|
});
|
|
|
|
// Test 11c: /tunnel/start probes cached tunnel before returning already_active
|
|
test('/tunnel/start verifies cached tunnel is alive before returning already_active', () => {
|
|
const tunnelBlock = sliceBetween(SERVER_SRC, "url.pathname === '/tunnel/start'", "url.pathname === '/refs'");
|
|
// Must probe before returning cached URL
|
|
expect(tunnelBlock).toContain('Cached tunnel is dead');
|
|
// Must tear down tunnel state on stale detection (via closeTunnel helper)
|
|
expect(tunnelBlock).toContain('closeTunnel()');
|
|
// Must fall through to restart when dead
|
|
expect(tunnelBlock).toContain('restarting');
|
|
});
|
|
|
|
// Test 11d: CLI verifies tunnel_url from server before printing instruction block
|
|
test('CLI probes tunnel_url before using it in instruction block', () => {
|
|
const pairSection = sliceBetween(CLI_SRC, 'Determine the URL to use', 'local HOST: write config');
|
|
// Must probe the tunnel URL
|
|
expect(pairSection).toContain('cliProbe');
|
|
expect(pairSection).toContain('Tunnel unreachable from CLI');
|
|
// Must fall through to restart logic on failure
|
|
expect(pairSection).toContain('attempting restart');
|
|
});
|
|
|
|
// ─── Batch endpoint security ─────────────────────────────────
|
|
|
|
// Test 12a: /batch endpoint sits ABOVE the blanket root-only auth gate (same as /command)
|
|
test('/batch endpoint sits ABOVE the blanket root-only auth gate', () => {
|
|
const batchIdx = SERVER_SRC.indexOf("url.pathname === '/batch'");
|
|
const blanketGateIdx = SERVER_SRC.indexOf("Auth-required endpoints (root token only)");
|
|
expect(batchIdx).toBeGreaterThan(0);
|
|
expect(blanketGateIdx).toBeGreaterThan(0);
|
|
expect(batchIdx).toBeLessThan(blanketGateIdx);
|
|
});
|
|
|
|
// Test 12b: /batch uses getTokenInfo (accepts scoped tokens), not validateAuth (root-only)
|
|
test('/batch uses getTokenInfo for auth, not validateAuth', () => {
|
|
const batchBlock = sliceBetween(SERVER_SRC, "url.pathname === '/batch'", "url.pathname === '/command'");
|
|
expect(batchBlock).toContain('getTokenInfo');
|
|
expect(batchBlock).not.toContain('validateAuth');
|
|
});
|
|
|
|
// Test 12c: /batch enforces max command limit
|
|
test('/batch enforces max 50 commands per batch', () => {
|
|
const batchBlock = sliceBetween(SERVER_SRC, "url.pathname === '/batch'", "url.pathname === '/command'");
|
|
expect(batchBlock).toContain('commands.length > 50');
|
|
expect(batchBlock).toContain('Max 50 commands per batch');
|
|
});
|
|
|
|
// Test 12d: /batch rejects nested batches
|
|
test('/batch rejects nested batch commands', () => {
|
|
const batchBlock = sliceBetween(SERVER_SRC, "url.pathname === '/batch'", "url.pathname === '/command'");
|
|
expect(batchBlock).toContain("cmd.command === 'batch'");
|
|
expect(batchBlock).toContain('Nested batch commands are not allowed');
|
|
});
|
|
|
|
// Test 12e: /batch skips per-command rate limiting (batch counts as 1 request)
|
|
test('/batch skips per-command rate limiting', () => {
|
|
const batchBlock = sliceBetween(SERVER_SRC, "url.pathname === '/batch'", "url.pathname === '/command'");
|
|
expect(batchBlock).toContain('skipRateCheck: true');
|
|
});
|
|
|
|
// Test 12f: /batch skips per-command activity events (emits batch-level events)
|
|
test('/batch emits batch-level activity, not per-command', () => {
|
|
const batchBlock = sliceBetween(SERVER_SRC, "url.pathname === '/batch'", "url.pathname === '/command'");
|
|
expect(batchBlock).toContain('skipActivity: true');
|
|
// Should emit batch-level start and end events
|
|
expect(batchBlock).toContain("command: 'batch'");
|
|
});
|
|
|
|
// Test 12g: /batch validates command field in each command
|
|
test('/batch validates each command has a command field', () => {
|
|
const batchBlock = sliceBetween(SERVER_SRC, "url.pathname === '/batch'", "url.pathname === '/command'");
|
|
expect(batchBlock).toContain("typeof cmd.command !== 'string'");
|
|
expect(batchBlock).toContain('Missing "command" field');
|
|
});
|
|
|
|
// Test 12h: /batch passes tabId through to handleCommandInternal
|
|
test('/batch passes tabId to handleCommandInternal for multi-tab support', () => {
|
|
const batchBlock = sliceBetween(SERVER_SRC, "url.pathname === '/batch'", "url.pathname === '/command'");
|
|
expect(batchBlock).toContain('tabId: cmd.tabId');
|
|
expect(batchBlock).toContain('handleCommandInternal');
|
|
});
|
|
|
|
// ─── Pair-agent regression tests ──────────────────────────
|
|
|
|
// Regression: connect command crashed with "domains is not defined" because
|
|
// a stray `domains,` variable was in the status fetch body (cli.ts:852).
|
|
test('connect command status fetch body has no undefined variable references', () => {
|
|
const connectBlock = sliceBetween(CLI_SRC, 'Launching headed Chromium', 'Terminal agent started');
|
|
// The status fetch should use a clean JSON body
|
|
expect(connectBlock).toContain("command: 'status'");
|
|
// Must NOT contain a bare `domains` reference in the fetch body
|
|
// (it would be `domains,` on its own line, not part of a key like `domains:`)
|
|
const bodyMatch = connectBlock.match(/body:\s*JSON\.stringify\(\{([^}]+)\}\)/);
|
|
expect(bodyMatch).not.toBeNull();
|
|
if (bodyMatch) {
|
|
// The body should only contain command and args, no stray variables
|
|
expect(bodyMatch[1]).not.toMatch(/\bdomains\b/);
|
|
}
|
|
});
|
|
|
|
// Regression: pair-agent server died 15s after CLI exited because the server
|
|
// monitored the connect subprocess PID. pair-agent must set BROWSE_PARENT_PID=0
|
|
// to disable self-termination.
|
|
test('pair-agent disables parent PID monitoring via BROWSE_PARENT_PID=0', () => {
|
|
const pairBlock = sliceBetween(CLI_SRC, 'Ensure headed mode', 'handlePairAgent');
|
|
// The connect subprocess env must override BROWSE_PARENT_PID
|
|
expect(pairBlock).toContain("BROWSE_PARENT_PID");
|
|
expect(pairBlock).toContain("'0'");
|
|
// The connect command must propagate BROWSE_PARENT_PID=0 via the
|
|
// serverEnv object literal passed to startServer. The literal text
|
|
// `serverEnv.BROWSE_PARENT_PID` is NOT in source — the value is
|
|
// assigned via object-literal syntax (`BROWSE_PARENT_PID: '0'`)
|
|
// inside the `const serverEnv: Record<string, string> = { ... }`
|
|
// declaration. Assert both pieces appear in the connect block.
|
|
const connectBlock = sliceBetween(CLI_SRC, 'Launching headed Chromium', 'Terminal agent started');
|
|
expect(connectBlock).toContain("const serverEnv");
|
|
expect(connectBlock).toContain("BROWSE_PARENT_PID: '0'");
|
|
});
|
|
|
|
// Regression: newtab returned 403 for scoped tokens because the tab ownership
|
|
// check ran before the newtab handler, checking the active tab (owned by root).
|
|
test('newtab is excluded from tab ownership check', () => {
|
|
const ownershipBlock = sliceBetween(SERVER_SRC, 'Tab ownership check (own-only tokens / pair-agent isolation)', 'newtab with ownership for scoped tokens');
|
|
// The ownership check condition must exclude newtab
|
|
expect(ownershipBlock).toContain("command !== 'newtab'");
|
|
});
|
|
|
|
// CVE fix: cookie-picker HTML must NOT inline the auth token.
|
|
// getCookiePickerHTML() must not accept an authToken parameter.
|
|
test('cookie-picker UI does not accept or inline auth token', () => {
|
|
const uiSrc = fs.readFileSync(path.join(import.meta.dir, '../src/cookie-picker-ui.ts'), 'utf-8');
|
|
// Function signature must not include authToken
|
|
expect(uiSrc).not.toMatch(/getCookiePickerHTML\([^)]*authToken/);
|
|
// No AUTH_TOKEN interpolation in template
|
|
expect(uiSrc).not.toContain("AUTH_TOKEN = '${authToken");
|
|
expect(uiSrc).not.toContain("AUTH_TOKEN = '${auth");
|
|
});
|
|
|
|
// CVE fix: cookie-picker route handler uses one-time code exchange, not open access.
|
|
test('cookie-picker HTML route requires code or session cookie', () => {
|
|
const routeSrc = fs.readFileSync(path.join(import.meta.dir, '../src/cookie-picker-routes.ts'), 'utf-8');
|
|
// Must have code validation
|
|
expect(routeSrc).toContain('pendingCodes');
|
|
expect(routeSrc).toContain('validSessions');
|
|
// Must NOT pass authToken to getCookiePickerHTML
|
|
expect(routeSrc).not.toMatch(/getCookiePickerHTML\([^)]*authToken/);
|
|
// Must set HttpOnly session cookie
|
|
expect(routeSrc).toContain('HttpOnly');
|
|
expect(routeSrc).toContain('SameSite=Strict');
|
|
});
|
|
});
|
|
|
|
describe('Pair scope defaults and revocation surface', () => {
|
|
// Regression: the CLI only sent scopes when --restrict was passed, so the
|
|
// effective pairing default lived in two places (CLI omission + server
|
|
// fallback) and could silently drift. Both sides must reference the shared
|
|
// DEFAULT_PAIR_SCOPES constant, and the CLI must send scopes
|
|
// unconditionally (the old conditional-spread shape is banned).
|
|
test('/pair default and CLI pairing body share DEFAULT_PAIR_SCOPES', () => {
|
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "url.pathname === '/tunnel/start'");
|
|
expect(pairBlock).toContain('DEFAULT_PAIR_SCOPES');
|
|
const cliBlock = sliceBetween(CLI_SRC, 'async function handlePairAgent', 'Determine the URL to use');
|
|
// Match the CODE shape, not a comment: a bare toContain('DEFAULT_PAIR_SCOPES')
|
|
// is satisfied by the explanatory comment and passes vacuously on a revert.
|
|
expect(cliBlock).toMatch(/scopes:\s*restrict\s*\?[\s\S]{0,200}?:\s*\[\.\.\.DEFAULT_PAIR_SCOPES\]/);
|
|
expect(cliBlock).not.toMatch(/\.\.\.\(restrict\s*\?/);
|
|
});
|
|
|
|
// control is the only scope behind an explicit flag; a scopes list must
|
|
// not be able to smuggle it into a pairing grant.
|
|
test('/pair rejects control inside a scopes list without the control flag', () => {
|
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "url.pathname === '/tunnel/start'");
|
|
expect(pairBlock).toContain("pairBody.scopes.includes('control')");
|
|
});
|
|
|
|
// CLI-encoded clientIds (spaces, UTF-8) must round-trip through the revoke
|
|
// route; slicing the raw pathname 404s on every encoded name.
|
|
test('DELETE /token decodes the clientId path segment', () => {
|
|
const revokeBlock = sliceBetween(SERVER_SRC, "url.pathname.startsWith('/token/')", "url.pathname === '/agents'");
|
|
expect(revokeBlock).toContain('decodeURIComponent');
|
|
});
|
|
});
|