From a948dfd6e2dcd863325212245343ca66a088f8f2 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 7 Aug 2026 15:31:53 -0700 Subject: [PATCH] v0.42.74.0 fix(security): honor takes_holders over serve --http + agent-voice default-deny CORS (#2529 #2477) (#3868) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(auth): honor permissions.takes_holders for legacy bearer tokens over serve --http (#2529) The OAuth provider's legacy access_tokens branch parsed permissions.source_id but never read permissions.takes_holders, so the /mcp dispatch site's fail-closed default pinned every remote caller to world-only takes visibility — set-takes-holders was a silent no-op over serve --http, in both directions (grants above world never applied; restrictions below world didn't either). - src/core/legacy-token-scope.ts: new parseTakesHoldersAllowList shared by BOTH transports (the drift between the legacy HTTP transport's correct inline parse and the OAuth provider is how this bug shipped). [] preserved as explicit deny-all; non-array → undefined → consumer defaults ['world']. - src/core/operations.ts: AuthInfo.takesHoldersAllowList typed field (same ride-along as sourceId/allowedSources). - src/core/oauth-provider.ts: legacy branch threads the stored grant. OAuth-client tokens unchanged (no per-client storage — TODO filed). - src/mcp/http-transport.ts: converged on the shared helper (behavior no-op). - src/commands/serve-http.ts: sidecar cast replaced by the typed field. - src/core/facts/meta-hook.ts: hashAllowList gives [] its own cache key (cache identity only — payload filtering stays visibility-based). Tests: 7 verifyAccessToken cases (grant/absent/garbage/deny-all/mixed/ oauth-client/column-default), pure-helper describe, meta-hook cache-key pin, and a Postgres e2e (test/e2e/serve-http-takes-holders.test.ts) pinning the issue repro end-to-end over POST /mcp — the seam that had no coverage. Reported by @Derek95king. Co-Authored-By: Claude Fable 5 * fix(agent-voice): default-deny CORS + origin gate + loopback bind in reference server (#2477) The recipe reference server reflected any Origin into Access-Control-Allow-Origin on every route, including the unauthenticated side-effectful POSTs. Recipe is install_kind: copy-into-host-repo, so the reference is the source of truth for every future install. - Default-deny CORS: headers emitted only for exact matches against the new AGENT_VOICE_CORS_ORIGIN allowlist (comma-separated), with Vary: Origin; Allow-Credentials never set. Same-origin /call needs no configuration. - Origin gate on /session and /tool: CORS headers gate response reads, not request sends — a no-preflight "simple" cross-origin POST still executes. Disallowed Origins now 403 before any body read / upstream fetch / tool dispatch. No-Origin callers (curl, Twilio, native) and same-origin pages (Origin host == Host, tunnels included) pass. DNS rebinding stays a documented production-checklist item (TODO filed). - Loopback-default bind: HOST env, default 127.0.0.1 (mirrors gbrain serve --http --bind default); HOST=0.0.0.0 for containers/LAN. - Startup log prints the bind + CORS posture; recipe md + install manifest bumped to 0.1.1 with the production checklist rewritten to match. Tests: test/agent-voice-cors.serial.test.ts spawns the real server twice and pins default-deny, allowlist echo + trimming, preflight behavior, and the gate's ordering (evil-origin 403 vs no-origin reaching the handler). Reported by @sebastiondev. Co-Authored-By: Claude Fable 5 * test: agent-voice origin gate fails closed on malformed Origin (#2477) Coverage-audit follow-up: a cross-origin POST with an unparseable Origin header must 403 (new URL() throws → originAllowed returns false), never fall through to the handler. A bypass here would defeat the gate. Co-Authored-By: Claude Fable 5 * test: pin loopback-bind default + wire-level fail-closed takes default (#2529 #2477) Pre-landing review (testing specialist) flagged two revert-catching gaps: - #2477 HOST default (127.0.0.1) had no assertion — a regression to all-interfaces would pass every test. Capture the server's startup log and assert the loopback bind; add a HOST=0.0.0.0 override case. - #2529 serve-http `?? ['world']` default branch was only unit-covered. Add a 4th e2e case: a legacy token with no takes_holders key sees world-held takes but NOT brain-held ones over POST /mcp, pinning the fail-closed default end-to-end. Also assert the deny-all case returns a successful (non-error) tool result so the negatives can't pass vacuously. Verified: agent-voice 4/4, serve-http-takes-holders e2e 4/4 (real Postgres). Co-Authored-By: Claude Fable 5 * fix(auth): shared permissions decode + collision-free hot-memory cache key (#2529) Adversarial-review hardening on the #2529 fix: - The two transports shared parseTakesHoldersAllowList but still decoded the permissions column differently: the OAuth provider JSON.parse'd a string-typed value, the legacy HTTP transport didn't. On a double-encoded jsonb string scalar (#2339 class) a deny-all token would fail open to ['world'] on the HTTP transport while the provider honored it. Extract coerceLegacyPermissions into the shared module and route both through it, so "the two transports cannot drift" is literally true (shared decode + shared parse). Arrays/scalars/malformed strings → undefined (no grant). - hashAllowList used bare sentinels ('_' for undefined, '(empty)' for []), which collided with real holder values ['_'] and ['(empty)']. Encode collision-free (undefined → 'none', else JSON.stringify(sorted)) so the []-vs-undefined cache separation the #2529 change relies on holds for every holder value. Tests: coerceLegacyPermissions unit cases (object/JSON-string/malformed/ array/scalar), all existing takes-holders + meta-hook + e2e suites green. Co-Authored-By: Claude Fable 5 * v0.42.74.0 fix(security): honor takes_holders over serve --http + agent-voice default-deny CORS (#2529 #2477) Co-Authored-By: Claude Fable 5 * docs: document agent-voice HOST + AGENT_VOICE_CORS_ORIGIN env vars in install hint (v0.42.74.0) The post-install hint's env-var quick-start predated the #2477 hardening and listed neither the loopback-default HOST bind nor the default-deny AGENT_VOICE_CORS_ORIGIN allowlist. Add both as optional entries (safe by default) and refresh the stale startup-log line to match the server's actual loopback-bind output. Co-Authored-By: Claude Fable 5 * docs: correct CHANGELOG command guidance + TODOS filing version (#2529 #2477) Ship-stage document-release caught two wrong CLI invocations in the v0.42.74.0 CHANGELOG "To take advantage" block: `gbrain auth permissions ` has no read-only view form (that shape errors + exits 1 — set the scope directly with `set-takes-holders `), and `integrations install agent-voice --refresh` requires `--target `. Also correct the follow-ups TODO header from the plan's stale v0.42.56.0 guess to the actual ship version v0.42.74.0. Co-Authored-By: Claude Fable 5 * chore(deps): bump js-yaml to 3.15.1 — osv GHSA-5p4m-2wfm-xmqj (#2529 #2477) osv-scan flagged js-yaml@3.15.0 (High, CVSS 7.5), fixed in 3.15.1. The transitive copy (gray-matter → js-yaml) was pinned to ^3.15.0 by the package.json `overrides` block; bump both the direct dependency and the override to ^3.15.1 so the vulnerable version is gone from bun.lock entirely (gray-matter/js-yaml now resolves to 3.15.1). Patch bump, in-range, frontmatter parsing verified (markdown + frontmatter + import + oauth suites green). Co-Authored-By: Claude Fable 5 * chore(deps): bump admin nanoid to 3.3.18 — osv GHSA-2v37-7h3g-55p8 (#2529 #2477) osv-scan flagged nanoid@3.3.16 (High, CVSS 8.2) in admin/bun.lock, fixed in 3.3.17. nanoid is transitive (postcss → nanoid), so pin it in the admin overrides block; refresh resolves to 3.3.18 (latest patched 3.x). Admin SPA build verified green; both root and admin lockfiles now scan clean. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 20 ++ TODOS.md | 29 ++ VERSION | 2 +- admin/bun.lock | 3 +- admin/package.json | 3 +- bun.lock | 6 +- docs/architecture/KEY_FILES.md | 4 +- package.json | 6 +- recipes/agent-voice.md | 6 +- recipes/agent-voice/code/server.mjs | 106 +++++-- recipes/agent-voice/install/manifest.json | 2 +- .../agent-voice/install/post-install-hint.md | 8 +- src/commands/serve-http.ts | 9 +- src/core/facts/meta-hook.ts | 22 +- src/core/legacy-token-scope.ts | 41 +++ src/core/oauth-provider.ts | 25 +- src/core/operations.ts | 16 ++ src/mcp/http-transport.ts | 13 +- test/agent-voice-cors.serial.test.ts | 266 ++++++++++++++++++ test/e2e/serve-http-takes-holders.test.ts | 189 +++++++++++++ test/facts-meta-cache.test.ts | 29 ++ test/legacy-token-federated-scope.test.ts | 61 ++++ test/oauth.test.ts | 78 +++++ 23 files changed, 884 insertions(+), 60 deletions(-) create mode 100644 test/agent-voice-cors.serial.test.ts create mode 100644 test/e2e/serve-http-takes-holders.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 543ac849c..af63efcf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ All notable changes to GBrain will be documented in this file. +## [0.42.74.0] - 2026-08-07 + +**Two fixes for agents that reach a brain over the network: takes-holder visibility now works the way you set it, and the voice recipe is safe by default.** + +Legacy bearer tokens served over `gbrain serve --http` now honor the takes-holder allow-list you set with `gbrain auth permissions set-takes-holders`. Before, that setting was read on one serving path but silently ignored on the other, so a remote agent saw only world-held takes no matter what you granted — a token you widened to see brain-held takes saw none of them, and a token you narrowed still saw the public ones. Both directions now behave as configured, an empty grant means "no takes" (not "the default set"), and the two serving paths decode and apply the grant through one shared piece of code so they cannot drift apart again. Tokens with no grant continue to fall back to public-only, so nothing widens on upgrade. + +The bundled voice-agent recipe (`recipes/agent-voice`) ships secure by default. Its reference server now refuses cross-origin browser requests unless you name the origins in `AGENT_VOICE_CORS_ORIGIN`, gates the endpoints that spend your OpenAI key or read your brain so a stray web page can't trigger them, and listens on loopback only until you set `HOST` to expose it. The voice page you run locally is unaffected. Because this recipe is copied into your own repo at install time, `gbrain integrations install agent-voice --refresh` picks up the hardened version. + +### To take advantage of v0.42.74.0 + +```bash +gbrain upgrade +``` + +Then, if you serve a brain to remote agents, set each token's takes-holder scope with `gbrain auth permissions set-takes-holders world,brain` (or your desired holders). Voice-recipe operators run `gbrain integrations install agent-voice --refresh --target `, then set `AGENT_VOICE_CORS_ORIGIN` if a browser on another origin needs access and `HOST=0.0.0.0` only if the server must listen beyond loopback. + +### For contributors + +Both issues were reported by external security researchers who supplied fixes. Ship-stage adversarial review hardened two more spots: the two serving paths now share one permissions-decode helper (not just the allow-list parser) so a malformed double-encoded row can't make them disagree, and the hot-memory cache key encodes the allow-list collision-free so the empty-vs-absent distinction holds for every holder value. Credit @Derek95king (takes-holder threading) and @sebastiondev (voice-recipe CORS). + ## [0.42.73.2] - 2026-08-05 **A write that deduplication redirects onto an existing page is now checked against the write scope of whoever asked for it.** When the same content arrives under a new slug, gbrain recognises it and points the write at the page that already holds it. That redirected target is now tested against the caller's own scope — under whichever mechanism confines that caller. One of the two mechanisms was consulted at that point; both are now. diff --git a/TODOS.md b/TODOS.md index 06479393e..af31e9b0a 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1,5 +1,34 @@ # TODOS +## serve --http takes-holders + agent-voice hardening follow-ups (filed v0.42.74.0) + +Deferred from the #2529/#2477 security-fix wave (plan-eng-review + codex outside +voice CLEARED). None block the wave. + +- [ ] **P2 — Per-OAuth-client `takes_holders` storage (#2529 follow-up).** Legacy + bearer tokens honor `access_tokens.permissions.takes_holders` through + `verifyAccessToken`; OAuth clients have no equivalent column on `oauth_clients`, + so OAuth-minted tokens fail closed to `['world']`. Needs a schema migration + (`oauth_clients.takes_holders` JSONB or TEXT[]) + a `register-client` flag + + the `verifyAccessToken` JOIN projection. Include surfacing the EFFECTIVE + takes-holder scope in `whoami` output as part of this follow-up, so operators + can self-diagnose the legacy-vs-OAuth semantic split instead of reading docs. + Where: `src/schema.sql`, `src/core/migrate.ts`, `src/core/oauth-provider.ts`, + `src/commands/auth.ts`, `src/core/operations.ts` (whoami). +- [ ] **P3 — agent-voice Host-header allowlist (DNS-rebinding hardening).** The + #2477 fix ships default-deny CORS + an Origin gate on `/session`/`/tool`, but + the gate derives self-origin from the `Host` header, so a DNS-rebound page + (attacker origin whose host resolves to the operator's loopback) still passes. + Validate `Host` against `localhost`/`127.0.0.1`/operator-configured hosts and + 403 otherwise; slots beside `originAllowed()` in the router. Issue #2477 + explicitly deferred this. Where: `recipes/agent-voice/code/server.mjs`. +- [ ] **P3 — Debounce `last_used_at` in the oauth-provider legacy path.** The + legacy branch of `verifyAccessToken` fires an unconditional + `UPDATE access_tokens SET last_used_at = now()` on EVERY request, while the + legacy HTTP transport debounces the same write to once per 60s via a WHERE + clause (`src/mcp/http-transport.ts` validateToken). Apply the same pattern — + one fewer write per request on the `serve --http` hot path. + Where: `src/core/oauth-provider.ts`. ## v0.42.67.0 follow-ups (Windows build tooling) Filed as follow-ups from v0.42.67.0 (`.gitattributes` LF pin for `*.sh` + diff --git a/VERSION b/VERSION index 541b32657..f2d3e9ef8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.73.2 +0.42.74.0 diff --git a/admin/bun.lock b/admin/bun.lock index 18cf7221f..7d98e3c91 100644 --- a/admin/bun.lock +++ b/admin/bun.lock @@ -19,6 +19,7 @@ }, "overrides": { "@babel/core": "^7.29.6", + "nanoid": "^3.3.17", "postcss": "^8.5.23", }, "packages": { @@ -224,7 +225,7 @@ "ms": ["ms@2.1.3", "", {}, "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA=="], - "nanoid": ["nanoid@3.3.16", "", { "bin": { "nanoid": "bin/nanoid.cjs" } }, "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q=="], + "nanoid": ["nanoid@3.3.18", "", { "bin": { "nanoid": "bin/nanoid.cjs" } }, "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w=="], "node-releases": ["node-releases@2.0.37", "", {}, "sha512-1h5gKZCF+pO/o3Iqt5Jp7wc9rH3eJJ0+nh/CIoiRwjRxde/hAHyLPXYN4V3CqKAbiZPSeJFSWHmJsbkicta0Eg=="], diff --git a/admin/package.json b/admin/package.json index b81271ae5..5654a4722 100644 --- a/admin/package.json +++ b/admin/package.json @@ -20,6 +20,7 @@ }, "overrides": { "@babel/core": "^7.29.6", - "postcss": "^8.5.23" + "postcss": "^8.5.23", + "nanoid": "^3.3.17" } } diff --git a/bun.lock b/bun.lock index 1ebc01b26..0c0847632 100644 --- a/bun.lock +++ b/bun.lock @@ -26,7 +26,7 @@ "express-rate-limit": "^7.5.0", "gray-matter": "^4.0.3", "heic-decode": "^2.1.0", - "js-yaml": "^3.15.0", + "js-yaml": "^3.15.1", "marked": "^18.0.2", "openai": "^4.0.0", "pgvector": "^0.2.0", @@ -59,7 +59,7 @@ "form-data": "^4.0.6", "hono": "^4.12.34", "ip-address": "^10.3.1", - "js-yaml": "^3.15.0", + "js-yaml": "^3.15.1", "qs": "^6.15.2", }, "packages": { @@ -461,7 +461,7 @@ "jose": ["jose@6.2.2", "", {}, "sha512-d7kPDd34KO/YnzaDOlikGpOurfF0ByC2sEV4cANCtdqLlTfBlw2p14O/5d/zv40gJPbIQxfES3nSx1/oYNyuZQ=="], - "js-yaml": ["js-yaml@3.15.0", "", { "dependencies": { "argparse": "^1.0.7", "esprima": "^4.0.0" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-ttBQIIQPDeLjpPOohtUdXuXUVoA2uIB6fEH9HyJ7234s5mBJ5wTx20njxplLZQgLaOfpmPQA7X2t5AX6tIPbog=="], + "js-yaml": ["js-yaml@3.15.1", "", { "dependencies": { "argparse": "^1.0.7", "esprima": "^4.0.0" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-S99WuO3HlhO3XN41EtYUNl9zzXjoJx7QvmipxsJVxtCBT0YHEFy+iOJhjSvrmV12nYhWpZaM8lPHkJm0yUMbag=="], "json-schema": ["json-schema@0.4.0", "", {}, "sha512-es94M3nTIfsEPisRafak+HDLfHXnKBhV3vU5eqPcS3flIWqcxJWgXHXiey3YrpaNsanY5ei1VoYEbOzijuq9BA=="], diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 37d1244ca..5f086270c 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -279,10 +279,10 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/mcp/server.ts` — MCP stdio server (generated from operations). Tool-call handler delegates to `dispatchToolCall` from `src/mcp/dispatch.ts` so stdio + HTTP transports share one validation, context-build, and error-format path. Stdin `'end'` / `'close'` shutdown hooks are skipped when `process.env.MCP_STDIO === '1'` — gateway-piped stdio MCP wrappers (OpenClaw's `bundle-mcp`) pipe the handshake then close their stdin half, which would otherwise kill the server before the first tool call; signal handlers (SIGTERM/SIGINT/SIGHUP) + the parent-process watchdog still cover legitimate disconnects. `src/commands/serve.ts` exposes `ServeOptions.mcpStdio?: boolean` as a test seam so the guard is exercisable without process.env mutation. Pinned by `test/serve-stdio-lifecycle.test.ts`. - `src/mcp/dispatch.ts` — shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, `validateParams(op, params)`. Single source of truth for `(ctx, params)` handler arg order and the 5-field `OperationContext` shape (engine + config + logger + dryRun + remote). Defaults `remote: true` (untrusted); local CLI callers pass `remote: false`. Also exports `summarizeMcpParams(opName, params)` — privacy-preserving redactor for `mcp_request_log` and the admin SSE feed, returns `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`. Intersects submitted top-level keys against the operation's declared `params` allow-list (declared keys preserved sorted; unknown keys counted but never named, closing the attacker-controlled-key-name leak). Byte counts bucketed up to nearest 1KB so an attacker can't binary-search secret-content sizes by probing. Raw payload visibility is opt-in via `gbrain serve --http --log-full-params` (loud stderr warning). New logging paths route through this helper, not `JSON.stringify(params)`. - `src/mcp/rate-limit.ts` — Bounded-LRU token-bucket limiter. `buildDefaultLimiters()` returns the two-bucket pipeline: pre-auth IP (30/60s, fires BEFORE the DB lookup so brute-force load against `access_tokens` is capped) + post-auth token-id (60/60s). Tracks `lastTouchedMs` separately from `lastRefillMs` so an exhausted key can't be reset by hammering past the TTL. LRU cap bounds memory under attacker-controlled key growth. -- `src/commands/serve-http.ts` — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--bind HOST] [--log-full-params]`. Combines MCP SDK's `mcpAuthRouter` (authorize/token/register/revoke), a custom `client_credentials` handler running BEFORE the router (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; custom handler falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement + `localOnly` rejection before op dispatch, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE broadcasts every MCP request. `cookie-parser` wired (Express 5 has no built-in). Startup logging prints port, engine, issuer URL (honors `--public-url`), client count, DCR status, and the admin bootstrap token line — but the generated token's raw value only prints when stderr is an interactive TTY (`shouldSuppressBootstrapPrint`): a non-TTY (containerized/piped) start hides it so the secret never lands in centralized log storage, env-sourced tokens (`$GBRAIN_ADMIN_BOOTSTRAP_TOKEN`) are always hidden, `--print-admin-token` forces the raw value on a trusted terminal, and `--suppress-bootstrap-token` hides everything. The `/mcp` request handler's OperationContext literal sets `remote: true` explicitly (without it `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and a `read+write`-scoped OAuth token could submit `shell` jobs — RCE). `summarizeMcpParams` from `src/mcp/dispatch.ts` feeds both `mcp_request_log` writes and the SSE feed by default (raw via `--log-full-params`). Cookie `Secure` flag set behind HTTPS or a public-URL proxy; magic-link nonce store LRU-bounded; DCR disable routes through the `GBrainOAuthProvider` `dcrDisabled` constructor option (not a router monkey-patch); `transport.handleRequest` wrapped in try/catch to return a JSON-RPC 500 envelope; OperationError + unexpected exceptions unified through `buildError` / `serializeError` so `/mcp` always returns the same envelope. `/health` is liveness-only via `probeLiveness(sql, engineName, version, timeoutMs)` racing `sql\`SELECT 1\`` against the exported `HEALTH_TIMEOUT_MS = 3000` (returns the same `ProbeHealthResult` tagged-union as `probeHealth`, single timer-cleanup site, single 503 envelope); body shape `{status, version, engine}` only. Full stats moved to admin-only `/admin/api/full-stats` (gated by `requireAdmin`, calls `probeHealth(engine, ...)`) — keeps `getStats()`'s 6× count(*) off the public route so a saturated pool doesn't trigger orchestrator restart cascades. Every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so it works against PGLite; the four `mcp_request_log.params` INSERT sites (success / auth_failed / scope_denied / server-error) go through `executeRawJsonb(engine, ...)` so the column stores real objects (`params->>'op'` returns `search`, not the quoted string). `--bind HOST` defaults `127.0.0.1` (self-hosters pass `--bind 0.0.0.0`); a stderr WARN fires when `--public-url` is set without `--bind`; the banner prints a `Bind:` line. `AuthInfo.sourceId` + `AuthInfo.allowedSources` are the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. The HTTP MCP `tools/list` handler at `:837-849` uses `paramDefToSchema(v)` from `src/mcp/tool-defs.ts` so array params keep `items` (strict-mode OAuth clients otherwise reject the whole tool list). `POST /ingest` enforces the slug-prefix write fence at the ROUTE, not the op layer: the route hands its payload to the `ingest_capture` minion handler, which deliberately bypasses `put_page`, so no `OperationContext` exists and `enforceClientSlugFence` never runs — a slug-bound client must therefore supply `X-Gbrain-Slug` and it must satisfy `slugUnderBoundPrefixes`, else 403 (without the check a bound client could overwrite any page, in the `default` source, since untrusted payloads carry no source grant). +- `src/commands/serve-http.ts` — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--bind HOST] [--log-full-params]`. Combines MCP SDK's `mcpAuthRouter` (authorize/token/register/revoke), a custom `client_credentials` handler running BEFORE the router (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; custom handler falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement + `localOnly` rejection before op dispatch, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE broadcasts every MCP request. `cookie-parser` wired (Express 5 has no built-in). Startup logging prints port, engine, issuer URL (honors `--public-url`), client count, DCR status, and the admin bootstrap token line — but the generated token's raw value only prints when stderr is an interactive TTY (`shouldSuppressBootstrapPrint`): a non-TTY (containerized/piped) start hides it so the secret never lands in centralized log storage, env-sourced tokens (`$GBRAIN_ADMIN_BOOTSTRAP_TOKEN`) are always hidden, `--print-admin-token` forces the raw value on a trusted terminal, and `--suppress-bootstrap-token` hides everything. The `/mcp` request handler's OperationContext literal sets `remote: true` explicitly (without it `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and a `read+write`-scoped OAuth token could submit `shell` jobs — RCE). `summarizeMcpParams` from `src/mcp/dispatch.ts` feeds both `mcp_request_log` writes and the SSE feed by default (raw via `--log-full-params`). Cookie `Secure` flag set behind HTTPS or a public-URL proxy; magic-link nonce store LRU-bounded; DCR disable routes through the `GBrainOAuthProvider` `dcrDisabled` constructor option (not a router monkey-patch); `transport.handleRequest` wrapped in try/catch to return a JSON-RPC 500 envelope; OperationError + unexpected exceptions unified through `buildError` / `serializeError` so `/mcp` always returns the same envelope. `/health` is liveness-only via `probeLiveness(sql, engineName, version, timeoutMs)` racing `sql\`SELECT 1\`` against the exported `HEALTH_TIMEOUT_MS = 3000` (returns the same `ProbeHealthResult` tagged-union as `probeHealth`, single timer-cleanup site, single 503 envelope); body shape `{status, version, engine}` only. Full stats moved to admin-only `/admin/api/full-stats` (gated by `requireAdmin`, calls `probeHealth(engine, ...)`) — keeps `getStats()`'s 6× count(*) off the public route so a saturated pool doesn't trigger orchestrator restart cascades. Every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so it works against PGLite; the four `mcp_request_log.params` INSERT sites (success / auth_failed / scope_denied / server-error) go through `executeRawJsonb(engine, ...)` so the column stores real objects (`params->>'op'` returns `search`, not the quoted string). `--bind HOST` defaults `127.0.0.1` (self-hosters pass `--bind 0.0.0.0`); a stderr WARN fires when `--public-url` is set without `--bind`; the banner prints a `Bind:` line. `AuthInfo.sourceId` + `AuthInfo.allowedSources` + `AuthInfo.takesHoldersAllowList` are the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` (source scope from the `oauth_clients` row; takes-holders from `access_tokens.permissions.takes_holders` for legacy bearer tokens). The `/mcp` dispatch site reads `authInfo.takesHoldersAllowList ?? ['world']` — absent grants (OAuth-client tokens, pre-v29 brains) fail closed to world-only takes visibility, while an explicit `[]` grant is preserved as deny-all; pinned end-to-end by `test/e2e/serve-http-takes-holders.test.ts`. The HTTP MCP `tools/list` handler at `:837-849` uses `paramDefToSchema(v)` from `src/mcp/tool-defs.ts` so array params keep `items` (strict-mode OAuth clients otherwise reject the whole tool list). `POST /ingest` enforces the slug-prefix write fence at the ROUTE, not the op layer: the route hands its payload to the `ingest_capture` minion handler, which deliberately bypasses `put_page`, so no `OperationContext` exists and `enforceClientSlugFence` never runs — a slug-bound client must therefore supply `X-Gbrain-Slug` and it must satisfy `slugUnderBoundPrefixes`, else 403 (without the check a bound client could overwrite any page, in the `default` source, since untrusted payloads carry no source grant). - `src/core/sql-query.ts` — engine-aware tagged-template SQL adapter for OAuth/admin/auth infrastructure. `sqlQueryForEngine(engine)` returns a `SqlQuery` (`(strings, ...values) => Promise`) that walks the template, builds `$N` positional SQL, asserts every value is a `SqlValue` (string | number | bigint | boolean | Date | null), and routes through `engine.executeRaw(sql, params)` (Postgres via postgres.js `unsafe(sql, params)`, PGLite via `db.query(sql, params)`). Deliberately narrower than postgres.js's `sql` tag: no nested fragments, `sql.json()`, `sql.unsafe()`, `sql.begin()`, or array binding — the narrow scalar-only surface is the feature (keeps it from drifting into a partial postgres.js clone). JSONB writes go through `executeRawJsonb(engine, sql, scalarParams, jsonbParams)` which composes positional `$N::jsonb` casts and passes JS **objects** through; an object reaches the wire with the correct type oid, so executeRawJsonb is safe (verified by `test/sql-query.test.ts` on PGLite, `test/e2e/auth-permissions.test.ts:67` on Postgres). Positional binding is NOT universally immune, though: binding a `JSON.stringify(x)` **string** to a bare `$N::jsonb` via `unsafe()` double-encodes it into a jsonb string scalar on real Postgres (the #2339 class; PGLite hides it). Fixes: pass a raw object (executeRawJsonb / `sql.json`), or cast through `$N::text::jsonb`. `scripts/check-jsonb-pattern.sh` (template grep) doesn't fire on `executeRawJsonb(...)` because it passes objects; the positional `$N::jsonb` + `JSON.stringify` form is caught by `scripts/check-jsonb-params.mjs`. Consumed by `src/commands/auth.ts`, `src/commands/serve-http.ts`, `src/core/oauth-provider.ts`, `src/commands/files.ts`, `src/mcp/http-transport.ts` so all five work uniformly against PGLite and Postgres. - `src/commands/serve.ts` — `gbrain serve` stdio MCP entrypoint with idempotent shutdown across every parent-disconnect signal. Stdio EOF, SIGTERM, SIGINT, SIGHUP, and parent-process death (every reparent case — PID 1, launchd subreaper, systemd, tmux, or a parent shell with `PR_SET_CHILD_SUBREAPER`) all funnel into one `cleanup(reason)` that releases the engine and the PGLite write-lock dir within 5 seconds (otherwise the lock is held indefinitely after Claude Desktop / Cursor / launchd-managed gateways disconnect, forcing a 5-minute stale-lock wait on next start). Watchdog reparent check is `getParentPid() !== initialParentPid` (the `=== 1` check missed the subreaper case under launchd/systemd). Bun's `process.ppid` cache is stale across reparenting ([oven-sh/bun#30305](https://github.com/oven-sh/bun/issues/30305)) so `getParentPid()` runs `spawnSync('ps', ['-o', 'ppid=', '-p', PID])` per tick. Startup probe verifies `ps` is on PATH; if not (stripped containers, busybox), the watchdog skips installing AND emits a loud `[gbrain serve] watchdog disabled: ps unavailable ...` stderr line so operators see the degraded mode. Pinned by `test/serve-stdio-lifecycle.test.ts` (22 cases). Credit @Aragorn2046 + @seungsu-kr. -- `src/core/oauth-provider.ts` — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore`. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1: `authorize` + `exchangeAuthorizationCode` with PKCE, `client_credentials`, `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR validates redirect_uri is `https://` or loopback per RFC 6749 §3.1.2.1). All tokens + client secrets SHA-256 hashed before storage. Auth codes single-use with 10-minute TTL via atomic `DELETE...RETURNING` (closes RFC 6749 §10.5 TOCTOU); refresh rotation also `DELETE...RETURNING` (§10.4 stolen-token detection). `pgArray()` escapes commas/quotes/braces so a comma-bearing redirect_uri can't smuggle a second array element. Legacy `access_tokens` fallback in `verifyAccessToken` grandfathers pre-v0.26 bearer tokens as `read+write+admin`. `sweepExpiredTokens()` runs on startup in try/catch and returns the count via `RETURNING 1` + array length. RFC hardening: `client_id` folded atomically into the `DELETE WHERE` for both auth-code exchange and refresh rotation (wrong-client paths don't burn the row); refresh-scope-subset enforced against the original grant on the row (RFC 6749 §6, so revoking a scope shrinks existing refresh tokens); `client_id` bound on `revokeToken` (RFC 7009 §2.1); `/token` `redirect_uri` validated against the `/authorize` value (RFC 6749 §4.1.3, empty-string treated as missing not wildcard); bare `catch {}` in `verifyAccessToken`/`getClient` replaced by `isUndefinedColumnError` from `src/core/utils.ts` (only SQLSTATE 42703 falls through to legacy; lock timeouts/network blips throw); `dcrDisabled` constructor option lets `serve-http.ts` disable `/register` without monkey-patching the router. Module-private `coerceTimestamp()` normalizes postgres-driver-as-string BIGINT columns to JS numbers at 5 read sites (`getClient` for RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` + `verifyAccessToken` for the SDK's `typeof === 'number'` check); throws on NaN/Infinity (fail loud at boundary), returns undefined for SQL NULL (callers treat NULL as expired). Not promoted to `utils.ts` — generic BIGINT precision-loss risk. `registerClient` honors `token_endpoint_auth_method: "none"` (RFC 7591 §3.2.1): public PKCE clients store `client_secret_hash = NULL` and the response omits `client_secret`; confidential clients (`client_secret_post` / `client_secret_basic`) keep their one-time-reveal shape; `getClient` normalizes NULL `client_secret_hash` to JS `undefined` so the SDK's clientAuth path accepts public clients. `verifyAccessToken` JOINs `oauth_clients.source_id` (write scope, scalar) + `oauth_clients.federated_read` (read scope, TEXT[]) + `oauth_clients.bound_slug_prefixes` (write fence, TEXT[] — consumed by `enforceClientSlugFence` in `operations.ts`) onto the returned `AuthInfo`; legacy brains degrade via `isUndefinedColumnError` fallback, dropping the newest projection first. `rescopeClient(clientId, {sourceId?, federatedRead?, boundSlugPrefixes?})` is the trusted-operator rescope (CLI `gbrain auth rescope-client`, admin `POST /admin/api/rescope-client`); `boundSlugPrefixes` is tri-state — undefined leaves the binding untouched, `null` clears it, a non-empty array replaces it (explicit empty array rejected as ambiguous deny-all) — so roster churn updates the write fence in place without rotating secrets. +- `src/core/oauth-provider.ts` — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore`. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1: `authorize` + `exchangeAuthorizationCode` with PKCE, `client_credentials`, `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR validates redirect_uri is `https://` or loopback per RFC 6749 §3.1.2.1). All tokens + client secrets SHA-256 hashed before storage. Auth codes single-use with 10-minute TTL via atomic `DELETE...RETURNING` (closes RFC 6749 §10.5 TOCTOU); refresh rotation also `DELETE...RETURNING` (§10.4 stolen-token detection). `pgArray()` escapes commas/quotes/braces so a comma-bearing redirect_uri can't smuggle a second array element. Legacy `access_tokens` fallback in `verifyAccessToken` grandfathers pre-v0.26 bearer tokens as `read+write+admin`, and threads BOTH stored grants off the token's `permissions` JSONB: `source_id` via `parseLegacyTokenScope` and `takes_holders` via `parseTakesHoldersAllowList` (both in `src/core/legacy-token-scope.ts`, shared with the legacy HTTP transport so the two transports cannot drift; `[]` takes-holders preserved as explicit deny-all, missing/non-array → undefined → the `/mcp` dispatch site's fail-closed `['world']`; OAuth-client tokens carry no takes-holders grant pending per-client storage — TODOS.md). `sweepExpiredTokens()` runs on startup in try/catch and returns the count via `RETURNING 1` + array length. RFC hardening: `client_id` folded atomically into the `DELETE WHERE` for both auth-code exchange and refresh rotation (wrong-client paths don't burn the row); refresh-scope-subset enforced against the original grant on the row (RFC 6749 §6, so revoking a scope shrinks existing refresh tokens); `client_id` bound on `revokeToken` (RFC 7009 §2.1); `/token` `redirect_uri` validated against the `/authorize` value (RFC 6749 §4.1.3, empty-string treated as missing not wildcard); bare `catch {}` in `verifyAccessToken`/`getClient` replaced by `isUndefinedColumnError` from `src/core/utils.ts` (only SQLSTATE 42703 falls through to legacy; lock timeouts/network blips throw); `dcrDisabled` constructor option lets `serve-http.ts` disable `/register` without monkey-patching the router. Module-private `coerceTimestamp()` normalizes postgres-driver-as-string BIGINT columns to JS numbers at 5 read sites (`getClient` for RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` + `verifyAccessToken` for the SDK's `typeof === 'number'` check); throws on NaN/Infinity (fail loud at boundary), returns undefined for SQL NULL (callers treat NULL as expired). Not promoted to `utils.ts` — generic BIGINT precision-loss risk. `registerClient` honors `token_endpoint_auth_method: "none"` (RFC 7591 §3.2.1): public PKCE clients store `client_secret_hash = NULL` and the response omits `client_secret`; confidential clients (`client_secret_post` / `client_secret_basic`) keep their one-time-reveal shape; `getClient` normalizes NULL `client_secret_hash` to JS `undefined` so the SDK's clientAuth path accepts public clients. `verifyAccessToken` JOINs `oauth_clients.source_id` (write scope, scalar) + `oauth_clients.federated_read` (read scope, TEXT[]) + `oauth_clients.bound_slug_prefixes` (write fence, TEXT[] — consumed by `enforceClientSlugFence` in `operations.ts`) onto the returned `AuthInfo`; legacy brains degrade via `isUndefinedColumnError` fallback, dropping the newest projection first. `rescopeClient(clientId, {sourceId?, federatedRead?, boundSlugPrefixes?})` is the trusted-operator rescope (CLI `gbrain auth rescope-client`, admin `POST /admin/api/rescope-client`); `boundSlugPrefixes` is tri-state — undefined leaves the binding untouched, `null` clears it, a non-empty array replaces it (explicit empty array rejected as ambiguous deny-all) — so roster churn updates the write fence in place without rotating secrets. - `admin/` — React 19 + Vite + TypeScript admin SPA embedded in the binary via `admin/dist/` served by `serve-http.ts`. 7 screens: Login (bootstrap token → session cookie), Dashboard (metrics + SSE feed + token health), Agents (sortable table + sparklines + Register), Register (modal with scope checkboxes + grant type selector), Credentials reveal (Copy + Download JSON + one-time-only warning), Request Log (filterable paginated), Agent Detail drawer (Details / Activity / Config Export tabs + Revoke). Design tokens: `#0a0a0f` bg, Inter for UI, JetBrains Mono for data, 4-32px spacing scale, rounded pill badges. HTTP-only SameSite=Strict cookie auth. 65KB gzip. Build: `cd admin && bun install && bun run build`; output at `admin/dist/` is committed for self-contained binaries. - `src/commands/auth.ts` — token management. `gbrain auth create/list/revoke/test` for legacy bearer tokens, plus `gbrain auth register-client` and `gbrain auth revoke-client ` for OAuth 2.1 client lifecycle. `revoke-client` runs an atomic `DELETE...RETURNING` on `oauth_clients`; FK `ON DELETE CASCADE` on `oauth_tokens.client_id` and `oauth_codes.client_id` purges every active token + auth code in one transaction; `process.exit(1)` on no-such-client (idempotent). Legacy tokens stored as SHA-256 hashes in `access_tokens`; OAuth clients in `oauth_clients`; legacy tokens grandfather to `read+write+admin` scopes on the OAuth HTTP server (no migration). Every SQL site routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` (and `executeRawJsonb` for the takes-holders `permissions` JSONB column) so `gbrain auth` works against PGLite; the takes-holders write goes through `executeRawJsonb(engine, sql, [name, hash], [{takes_holders:[...]}])` which round-trips with `jsonb_typeof = 'object'`. `register-client` accepts `--source ` (write authority, scalar) and `--federated-read ` (read scope, array) and prints the resolved `Write source` + `Federated reads`; pre-v0.34 clients backfill to `source_id='default'` via migration v60. The bare `gbrain auth create ` form (no `--takes-holders`) mints a token via the exported pure `parseAuthCreateArgs(rest)` (the inline version used `rest[takesIdx + 1]` resolving to `rest[0]` when `takesIdx === -1`, excluding the name from the positional search). Pinned by `test/auth-create-args.test.ts`. - `src/commands/connect.ts` + `src/core/connect-probe.ts` — `gbrain connect [--token ]` one-command coding-agent onboarding from a bearer token. Turns an MCP URL + token into a paste-ready `claude mcp add ... -H "Authorization: Bearer ..."` block (default) or, with `--install`, runs it directly and smoke-tests the token. Direct HTTP MCP — Claude Code talks straight to a remote `gbrain serve --http`, no local install needed. Token resolution: `--token` > `$GBRAIN_REMOTE_TOKEN` > placeholder (print) / error (install). The generated block tells the agent to call `get_brain_identity` + `list_skills` (the `LEARN_INSTRUCTION` export, which names `put_page` not `capture` since `capture` is CLI-only, not an MCP tool) with a core-tools fallback for hosts without skill publishing. URL normalization appends `/mcp` to a bare host but REJECTS a scheme-less host; pure helpers (`isLinkLocalOrMetadata`, URL parse, render) are unit-tested. Flags: `--token`, `--name ` (default `gbrain`, validated against `NAME_RE`), `--agent claude-code|codex|perplexity|generic`, `--install`, `--yes` (required for `--install` in non-TTY), `--force`, `--json` (token redacted unless `--show-token`), `--timeout-ms`. `connect` is in `CLI_ONLY` + `CLI_ONLY_SELF_HELP`; dispatched in `cli.ts:handleCliOnly` with no local DB connect. `AGENT_SPECS` drives per-agent rendering + `--install`: `claude-code` → `buildClaudeMcpAddArgv` (literal `-H "Authorization: Bearer "`); `codex` → `buildCodexMcpAddArgv` = `codex mcp add --url --bearer-token-env-var GBRAIN_REMOTE_TOKEN` (Codex reads the token from the env var at runtime, never written to config; `--install` runs it and prints an `export GBRAIN_REMOTE_TOKEN` hint when missing); `perplexity` + `generic` are `installable:false` and reject `--install`. `--oauth` (`supportsOAuth:true` = perplexity/generic only) emits an OAuth 2.1 client-credentials connector block (Issuer URL via `issuerFromMcpUrl` = mcp-url minus `/mcp`, Client ID, Client Secret) — least-privilege scopes + short-lived rotating tokens vs a long-lived full-access secret. Creds from `--client-id`/`--client-secret` (BYO) or `--register` (`deps.registerOAuthClient` shells `gbrain auth register-client --grant-types client_credentials --scopes --token-endpoint-auth-method client_secret_post` and parses `Client ID:`/`Client Secret:`); `--oauth` rejected for claude-code/codex and incompatible with `--install`. `buildJson` is a generic shape (`agent`, `command`/`command_argv` null for perplexity/generic, `header`, `env_var`, oauth fields with redaction); the codex `command` carries only the env-var name, never the token. `cmdString(binary, argv)` POSIX-single-quotes args. `ConnectDeps` = `{isTTY, promptYesNo, hasBinary(bin), runBinary(bin, argv), probe, env(name)}` — binary-generic so `claude` and `codex` share the path; `env` injectable for tests. Security: rendered command single-quotes the token so shell metacharacters can't run code when pasted; token validated before it lands in an HTTP header; link-local / cloud-metadata addresses (incl. IPv4-mapped IPv6 `::ffff:169.254.x.x` and AWS IMDSv2-over-IPv6 `fd00:ec2::254`) refused as a token-exfil guard while localhost/RFC1918/LAN stay allowed; token redacted from all error output. `src/core/connect-probe.ts` is the raw-bearer MCP smoke probe backing `--install`: connects the official MCP SDK `Client` over `StreamableHTTPClientTransport` with a STATIC `Authorization` header (no OAuth/discovery — distinct from `mcp-client.ts:callRemoteTool` which is OAuth-only and `remote-mcp-probe.ts:smokeTestMcp` which only sends `initialize`), runs the full `initialize` handshake via `client.connect()`, then calls `get_brain_identity` (read-scope, non-localOnly) to prove a tool call round-trips. Never throws — every failure maps to `{ ok: false, reason: 'auth' | 'unreachable' | 'timeout' | 'tool_error' | 'unknown', message }` so a wrong/expired token fails at setup, not on the agent's first request. `DEFAULT_PROBE_TIMEOUT_MS = 15_000` shared with `connect.ts`. `serve-http.ts` adds exported pure `skillPublishStatus(publishSkills)` for the startup banner `Skills: published / not published` line + a one-line `gbrain config set mcp.publish_skills true` stderr nudge when publishing is OFF. Docs: `docs/mcp/CODEX.md`, `docs/mcp/PERPLEXITY.md`, `docs/mcp/CLAUDE_CODE.md`, `docs/tutorials/connect-coding-agent.md`. Pinned by `test/connect.test.ts` (pure-helper + render, all four agents) + `test/e2e/connect-bearer.test.ts` (raw-bearer probe + full OAuth chain register→connect→discovery→`/token` mint→`get_brain_identity`, client registered in `beforeAll` before serve takes the PGLite single-writer lock; drives real `claude` + `codex` binaries through `connect --install` with sandboxed `HOME`/`CODEX_HOME`, asserts registration + token never in Codex config, skips when a binary is absent) + `test/e2e/serve-stdio-roundtrip.test.ts` (spawns real `gbrain serve` stdio against a fresh `init --pglite` brain, drives the SDK client through `initialize`→`tools/list`→`tools/call`, asserts the advertised core-tool set and that `capture` is NOT advertised) + `test/serve-skills-publish-nudge.test.ts` (the `test/audit/batch-retry-audit.test.ts` ENOENT case was made hermetic — it had read the real `~/.gbrain/audit`). diff --git a/package.json b/package.json index 6fe3d3b43..70a55c655 100644 --- a/package.json +++ b/package.json @@ -122,7 +122,7 @@ "express-rate-limit": "^7.5.0", "gray-matter": "^4.0.3", "heic-decode": "^2.1.0", - "js-yaml": "^3.15.0", + "js-yaml": "^3.15.1", "marked": "^18.0.2", "openai": "^4.0.0", "pgvector": "^0.2.0", @@ -148,7 +148,7 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.73.2", + "version": "0.42.74.0", "overrides": { "@hono/node-server": "^2.0.5", "fast-uri": "^3.1.5", @@ -159,6 +159,6 @@ "hono": "^4.12.34", "ip-address": "^10.3.1", "qs": "^6.15.2", - "js-yaml": "^3.15.0" + "js-yaml": "^3.15.1" } } diff --git a/recipes/agent-voice.md b/recipes/agent-voice.md index 734f4f208..ae90385d0 100644 --- a/recipes/agent-voice.md +++ b/recipes/agent-voice.md @@ -1,7 +1,7 @@ --- id: agent-voice name: Voice Personas (Mars + Venus) -version: 0.1.0 +version: 0.1.1 description: WebRTC-first voice agent reference (Mars + Venus personas, optional Twilio adapter). Skillpack-as-reference paradigm — the install-time agent COPIES code into your host agent repo where it becomes user-owned and mutable, NOT a runtime gbrain dependency. category: voice install_kind: copy-into-host-repo @@ -132,7 +132,9 @@ Reference code ships intentionally minimal. Before public deployment: - **Twilio signature validation** on `/voice` — currently absent; add `X-Twilio-Signature` header validation. - **Rate limiting** on `/session` and `/tool` — currently absent. -- **CORS allowlist** — currently `*`; restrict to your deployed origins. +- **CORS allowlist** — default-deny out of the box: no `Access-Control-Allow-Origin` header is emitted unless the request's Origin exactly matches `AGENT_VOICE_CORS_ORIGIN` (comma-separated origins, e.g. `AGENT_VOICE_CORS_ORIGIN=https://your.app,https://staging.your.app`). The served `/call` page is same-origin and needs no configuration. `/session` and `/tool` additionally reject cross-origin browser requests (403) unless allowlisted — CORS headers alone can't stop a no-preflight "simple" POST from executing. +- **Bind address** — the server listens on `127.0.0.1` by default; set `HOST=0.0.0.0` for containers or direct LAN exposure (and prefer a tunnel for anything public). +- **Host-header allowlist** — not shipped; the origin gate derives self-origin from the `Host` header, so DNS rebinding is not covered. Add a `Host` allowlist before exposing beyond loopback. - **Auth on /tool** — voice-side tool calls currently trust the in-process connection; if you expose `/tool` publicly, gate it behind a session token. - **HTTPS** — required for browser mic access in production. Use ngrok / Caddy / Cloudflare Tunnel. - **Twilio fallback URL** — `/fallback` is a TwiML stub; wire to your operator's cell for crash recovery. diff --git a/recipes/agent-voice/code/server.mjs b/recipes/agent-voice/code/server.mjs index f26c90cb7..5934c8269 100644 --- a/recipes/agent-voice/code/server.mjs +++ b/recipes/agent-voice/code/server.mjs @@ -19,16 +19,26 @@ * against `lib/twilio-bridge.mjs` (port-ready stubs included). * * Configuration via env: - * PORT default 8765 - * OPENAI_API_KEY required for /session - * OPENAI_REALTIME_MODEL default 'gpt-4o-realtime-preview' - * DEFAULT_PERSONA default 'venus' (one of 'mars' | 'venus') - * BRAIN_ROOT passed through to context-builder - * TIMEZONE passed through to context-builder + * PORT default 8765 + * HOST default '127.0.0.1' (loopback-only; set 0.0.0.0 + * for containers / direct LAN exposure) + * OPENAI_API_KEY required for /session + * OPENAI_REALTIME_MODEL default 'gpt-4o-realtime-preview' + * DEFAULT_PERSONA default 'venus' (one of 'mars' | 'venus') + * AGENT_VOICE_CORS_ORIGIN comma-separated exact origins allowed via CORS + * (default unset = default-deny; the served /call + * page is same-origin and needs nothing) + * BRAIN_ROOT passed through to context-builder + * TIMEZONE passed through to context-builder * - * Security posture: this is reference code. It does NOT ship hardening for - * production deployment (no rate limiting, no Twilio signature validation, - * no CORS allowlist). Operators add those at install time per the recipe's + * Security posture: CORS is default-deny (exact-origin allowlist via + * AGENT_VOICE_CORS_ORIGIN), the side-effectful POSTs (/session, /tool) are + * gated on the Origin header (CORS headers gate response READS, not request + * SENDS — a cross-origin "simple" POST skips preflight, so without the gate + * an attacker page could blind-fire /session and burn the OpenAI key), and + * the listener binds loopback by default. Still reference code: rate + * limiting, Twilio signature validation, and a Host-header allowlist + * (DNS-rebinding hardening) are operator-added per the recipe's * "production checklist." */ @@ -43,10 +53,60 @@ const __dirname = dirname(fileURLToPath(import.meta.url)); const PUBLIC_DIR = join(__dirname, 'public'); const PORT = parseInt(process.env.PORT || '8765', 10); +// Loopback by default (mirrors `gbrain serve --http --bind 127.0.0.1`). +// Tunnels (ngrok / Caddy / Cloudflare) target localhost, so the documented +// flows keep working; container/LAN deployments set HOST=0.0.0.0. +const HOST = process.env.HOST || '127.0.0.1'; const DEFAULT_PERSONA = (process.env.DEFAULT_PERSONA || 'venus').toLowerCase(); const OPENAI_REALTIME_MODEL = process.env.OPENAI_REALTIME_MODEL || 'gpt-4o-realtime-preview'; const OPENAI_REALTIME_URL = 'https://api.openai.com/v1/realtime/calls'; +// ── CORS + origin gate (default-deny) ───────────────────────────────── +// Mirrors the GBRAIN_HTTP_CORS_ORIGIN pattern in gbrain's own HTTP +// transport: exact-origin allowlist, no header emitted otherwise, and +// Access-Control-Allow-Credentials is never set. +function parseCorsAllowlist() { + const v = process.env.AGENT_VOICE_CORS_ORIGIN; + if (!v) return null; + const entries = v.split(',').map((s) => s.trim()).filter(Boolean); + return entries.length > 0 ? new Set(entries) : null; +} +const CORS_ALLOWLIST = parseCorsAllowlist(); + +function applyCors(req, res) { + const origin = req.headers.origin; + if (!(CORS_ALLOWLIST && origin && CORS_ALLOWLIST.has(origin))) return; + res.setHeader('Access-Control-Allow-Origin', origin); + res.setHeader('Vary', 'Origin'); + if (req.method === 'OPTIONS') { + res.setHeader('Access-Control-Allow-Methods', 'GET, POST, OPTIONS'); + res.setHeader('Access-Control-Allow-Headers', 'Content-Type, Authorization'); + } +} + +// CORS headers gate response READS, not request SENDS: a cross-origin +// "simple" POST (e.g. text/plain) skips preflight entirely, executes +// server-side, and only the response is withheld from the attacker's JS. +// For endpoints with side effects (/session spends OPENAI_API_KEY, /tool +// dispatches brain reads) that isn't enough — reject disallowed Origins +// BEFORE doing any work. Requests without an Origin header (curl, Twilio +// webhooks, native apps) pass; browser requests pass only when same-origin +// (Origin host matches the Host header — covers the served /call page, +// including through a tunnel) or explicitly allowlisted. Known limit: a +// DNS-rebound page's Origin host matches the rebound Host header, so this +// does not defend against DNS rebinding (Host-header allowlist is the +// production-checklist follow-up). +function originAllowed(req) { + const origin = req.headers.origin; + if (!origin) return true; + if (CORS_ALLOWLIST && CORS_ALLOWLIST.has(origin)) return true; + try { + return new URL(origin).host === req.headers.host; + } catch { + return false; + } +} + const MIME = { '.html': 'text/html; charset=utf-8', '.js': 'application/javascript; charset=utf-8', @@ -223,10 +283,9 @@ function handleVoiceTwiml(req, res) { const server = createServer(async (req, res) => { const url = new URL(req.url, `http://${req.headers.host}`); - // CORS: allow same-origin only by default. Operators relax in production. - res.setHeader('Access-Control-Allow-Origin', req.headers.origin || '*'); - res.setHeader('Access-Control-Allow-Methods', 'GET, POST, OPTIONS'); - res.setHeader('Access-Control-Allow-Headers', 'Content-Type, Authorization'); + // CORS: default-deny. Headers are emitted only for allowlisted origins + // (AGENT_VOICE_CORS_ORIGIN); a 204 without CORS headers is browser-blocked. + applyCors(req, res); if (req.method === 'OPTIONS') return send(res, 204, ''); try { @@ -239,11 +298,13 @@ const server = createServer(async (req, res) => { if (url.pathname === '/directory') { return serveStatic(res, 'directory.html'); } - if (url.pathname === '/session') { - return handleSession(req, res); - } - if (url.pathname === '/tool') { - return handleTool(req, res); + if (url.pathname === '/session' || url.pathname === '/tool') { + // Origin gate BEFORE any body read / upstream fetch / tool dispatch — + // blocks blind cross-origin "simple" POSTs that CORS headers can't. + if (!originAllowed(req)) { + return sendJson(res, 403, { error: 'origin not allowed' }); + } + return url.pathname === '/session' ? handleSession(req, res) : handleTool(req, res); } if (url.pathname === '/voice') { return handleVoiceTwiml(req, res); @@ -266,11 +327,16 @@ const server = createServer(async (req, res) => { } }); -server.listen(PORT, () => { +server.listen(PORT, HOST, () => { // eslint-disable-next-line no-console - console.log(`[agent-voice] listening on http://localhost:${PORT}`); + console.log(`[agent-voice] listening on http://${HOST}:${PORT} (bind: ${HOST}${HOST === '127.0.0.1' ? ' — set HOST=0.0.0.0 to expose beyond loopback' : ''})`); console.log(`[agent-voice] default persona: ${DEFAULT_PERSONA}`); console.log(`[agent-voice] read-only tools: ${getEffectiveAllowlist().join(', ')}`); + if (!CORS_ALLOWLIST) { + console.log('[agent-voice] CORS: default-deny. Set AGENT_VOICE_CORS_ORIGIN=https://your.app to allow cross-origin browser clients.'); + } else { + console.log(`[agent-voice] CORS allowlist: ${[...CORS_ALLOWLIST].join(', ')}`); + } }); process.on('SIGTERM', () => server.close(() => process.exit(0))); diff --git a/recipes/agent-voice/install/manifest.json b/recipes/agent-voice/install/manifest.json index c8b2a626d..96fb737a1 100644 --- a/recipes/agent-voice/install/manifest.json +++ b/recipes/agent-voice/install/manifest.json @@ -1,7 +1,7 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", "recipe": "agent-voice", - "version": "0.1.0", + "version": "0.1.1", "install_kind": "copy-into-host-repo", "description": "src → target mapping consumed by `gbrain integrations install agent-voice`. The install command reads this manifest, computes SHA-256 of each source file at copy time, writes the per-file hash to /services/voice-agent/.gbrain-source.json so --refresh can do three-way classification (unchanged-identical / unchanged-stale / locally-modified).", "target_root_relative_to_host_repo": "services/voice-agent", diff --git a/recipes/agent-voice/install/post-install-hint.md b/recipes/agent-voice/install/post-install-hint.md index c2f906d79..66f5d716c 100644 --- a/recipes/agent-voice/install/post-install-hint.md +++ b/recipes/agent-voice/install/post-install-hint.md @@ -15,8 +15,14 @@ OPENAI_API_KEY=sk-... # required (OpenAI Realtime API) DEFAULT_PERSONA=venus # optional (one of: venus, mars) BRAIN_ROOT=/path/to/your/brain # optional (enables live context) TIMEZONE=US/Pacific # optional +HOST=0.0.0.0 # optional — binds 127.0.0.1 (loopback) by default; set only to expose beyond localhost +AGENT_VOICE_CORS_ORIGIN=https://your.app # optional — CORS is default-deny; list exact origins (comma-separated) only if a browser on another origin needs access ``` +The two security env vars ship safe by default: the server listens on loopback +only and refuses cross-origin browser requests. The local `/call` flow below +needs neither. See the recipe's production checklist before exposing publicly. + Optional for inbound Twilio: ```bash TWILIO_ACCOUNT_SID=AC... @@ -56,7 +62,7 @@ If any prompt-shape test fails, the privacy guard has caught a name you'd want t ```bash cd /services/voice-agent bun run start # or `npm start` -# → listening on http://localhost:8765 +# → listening on http://127.0.0.1:8765 (bind: 127.0.0.1 — set HOST=0.0.0.0 to expose beyond loopback) ``` Open `http://localhost:8765/call` in a browser, click Connect, grant mic permission. You should be talking to Venus (or Mars if you set `DEFAULT_PERSONA=mars`). diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index 4940fbb88..cb4bad1c3 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -2004,8 +2004,13 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption // injection via the metaHook. HTTP-specific concerns (mcp_request_log // persistence + SSE broadcast) stay here; the dispatcher returns the // ToolResult and we read isError + _meta to pick the right branch. - const tokenAllowList = (authInfo as AuthInfo & { takesHoldersAllowList?: string[] }).takesHoldersAllowList - ?? ['world']; + // #2529: takesHoldersAllowList is a typed AuthInfo field populated by + // verifyAccessToken from access_tokens.permissions.takes_holders for + // legacy bearer tokens ([] preserved as deny-all). The fail-closed + // ['world'] default covers OAuth-client tokens (no per-client storage + // yet — see TODOS.md) and pre-v29 brains (no permissions column → + // isUndefinedColumnError fallback in verifyAccessToken). + const tokenAllowList = authInfo.takesHoldersAllowList ?? ['world']; // v0.34.1 (#861, D13): AuthInfo.sourceId is now a real typed field // populated from oauth_clients.source_id (migration v60 backfilled // NULL → 'default'). Pre-fix this site cast through AuthInfo and diff --git a/src/core/facts/meta-hook.ts b/src/core/facts/meta-hook.ts index de80ac330..2024eaefb 100644 --- a/src/core/facts/meta-hook.ts +++ b/src/core/facts/meta-hook.ts @@ -124,9 +124,23 @@ export function __resetHotMemoryCacheForTests(): void { _cache.clear(); } -/** Stable hash of the (sorted) allow-list. Mirrors the auth contract. */ +/** + * Stable hash of the (sorted) allow-list. Mirrors the auth contract. + * + * The allow-list affects cache IDENTITY only — the payload itself is filtered + * by fact visibility (see the `visibility` tier above), never by the takes + * allow-list. Keeping `[]` (explicit deny-all grant) distinct from undefined + * (no grant) is insurance so a future allowlist-dependent payload is born + * safe, not a claim that `[]` suppresses hot memory today. + * + * Encoding is collision-free: undefined maps to a token no JSON array can + * produce, and every concrete list serializes via JSON.stringify. A bare + * `sorted.join('|')` would have collided `['a|b']` with `['a','b']`, and bare + * sentinels would have collided `['(empty)']` with `[]` — so a holder value + * that happened to equal a sentinel could not share a cache bucket it + * shouldn't. + */ function hashAllowList(list: string[] | undefined): string { - if (!list || list.length === 0) return '_'; - const sorted = [...list].sort(); - return sorted.join('|'); + if (!list) return 'none'; + return JSON.stringify([...list].sort()); } diff --git a/src/core/legacy-token-scope.ts b/src/core/legacy-token-scope.ts index 5e616c280..be34780f7 100644 --- a/src/core/legacy-token-scope.ts +++ b/src/core/legacy-token-scope.ts @@ -20,3 +20,44 @@ export function parseLegacyTokenScope(rawSource: unknown): { sourceId: string; a } return { sourceId: 'default' }; } + +/** + * Parse a legacy bearer token's stored `access_tokens.permissions.takes_holders` + * grant. Shared by both HTTP transports (`src/mcp/http-transport.ts` and the + * OAuth provider behind `serve --http`) so the two cannot drift. + * + * ARRAY → filtered to string entries, with the empty array PRESERVED as an + * explicit deny-all grant (engines translate `[]` to `holder = ANY('{}')`, + * which matches nothing). Missing or non-array values → undefined; consumers + * apply their own fail-closed default (`['world']`). + * + * The filter is `typeof === 'string'` ONLY — no `length > 0` like + * `parseLegacyTokenScope` above — because the legacy HTTP transport has always + * kept empty-string entries and this helper must be a behavior no-op there. + */ +export function parseTakesHoldersAllowList(raw: unknown): string[] | undefined { + if (!Array.isArray(raw)) return undefined; + return (raw as unknown[]).filter((h): h is string => typeof h === 'string'); +} + +/** + * Coerce a legacy token's raw `access_tokens.permissions` column value to a + * plain object for grant extraction. A well-formed jsonb column reads back as + * an object on both drivers, but a double-encoded jsonb string scalar (the + * #2339 class) reads back as a JSON string — decode it here so BOTH the OAuth + * provider and the legacy HTTP transport interpret the row identically instead + * of one honoring the grant while the other silently fails open. Malformed + * strings and non-object/non-string values → undefined (no grant). + */ +export function coerceLegacyPermissions(raw: unknown): Record | undefined { + const asObject = (v: unknown): Record | undefined => + v !== null && typeof v === 'object' && !Array.isArray(v) ? (v as Record) : undefined; + if (typeof raw === 'string') { + try { + return asObject(JSON.parse(raw)); + } catch { + return undefined; + } + } + return asObject(raw); +} diff --git a/src/core/oauth-provider.ts b/src/core/oauth-provider.ts index 1979ff185..656946959 100644 --- a/src/core/oauth-provider.ts +++ b/src/core/oauth-provider.ts @@ -27,7 +27,7 @@ import { hashToken, generateToken, isUndefinedColumnError } from './utils.ts'; import { assertValidSourceId } from './source-id.ts'; import { hasScope, assertAllowedScopes, parseScopeString, InvalidScopeError } from './scope.ts'; import type { AuthInfo as CoreAuthInfo } from './operations.ts'; -import { parseLegacyTokenScope } from './legacy-token-scope.ts'; +import { parseLegacyTokenScope, parseTakesHoldersAllowList, coerceLegacyPermissions } from './legacy-token-scope.ts'; /** * A slug-prefix write binding is only meaningful if every entry actually @@ -795,19 +795,15 @@ export class GBrainOAuthProvider implements OAuthServerProvider { UPDATE access_tokens SET last_used_at = now() WHERE token_hash = ${tokenHash} `; const name = legacyRows[0].name as string; - const permissionsRaw = legacyRows[0].permissions; - let permissions: unknown = permissionsRaw; - if (typeof permissionsRaw === 'string') { - try { - permissions = JSON.parse(permissionsRaw); - } catch { - permissions = undefined; - } - } - const sourceGrant = permissions && typeof permissions === 'object' - ? (permissions as Record).source_id - : undefined; - const { sourceId, allowedSources } = parseLegacyTokenScope(sourceGrant); + const permissions = coerceLegacyPermissions(legacyRows[0].permissions); + const { sourceId, allowedSources } = parseLegacyTokenScope(permissions?.source_id); + // #2529: thread the stored takes-holders grant, mirroring the legacy + // HTTP transport's validateToken (both decode via coerceLegacyPermissions + // + parseTakesHoldersAllowList so they cannot drift). Undefined (no array + // grant, or the pre-v29 no-permissions-column fallback above) → the /mcp + // dispatch site defaults to the fail-closed ['world']. An explicit [] + // grant is preserved as deny-all. + const takesHoldersAllowList = parseTakesHoldersAllowList(permissions?.takes_holders); return { token, clientId: name, @@ -819,6 +815,7 @@ export class GBrainOAuthProvider implements OAuthServerProvider { // allowedSources for federated reads, matching legacy HTTP transport. sourceId, allowedSources, + takesHoldersAllowList, } as CoreAuthInfo as SdkAuthInfo; } diff --git a/src/core/operations.ts b/src/core/operations.ts index 0b21f539e..78f334cc5 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -479,6 +479,22 @@ export interface AuthInfo { * case (back-compat). */ allowedSources?: string[]; + /** + * Per-token allow-list for the holder field on `takes`, populated at + * token-verification time from `access_tokens.permissions.takes_holders` + * for legacy bearer tokens (via `parseTakesHoldersAllowList` in + * `src/core/legacy-token-scope.ts`). The HTTP transport threads this into + * `OperationContext.takesHoldersAllowList` (documented below). + * + * `[]` is an explicit deny-all grant and is PRESERVED (never collapsed). + * `undefined` means the token row carries no array grant — OAuth clients + * have no per-client storage yet (see TODOS.md) — and consumers apply the + * fail-closed `['world']` default at the dispatch site. + * + * Rides the same `as CoreAuthInfo as SdkAuthInfo` cast as `sourceId` / + * `allowedSources` above. + */ + takesHoldersAllowList?: string[]; /** * v0.42.72.0: slug-prefix WRITE binding from * `oauth_clients.bound_slug_prefixes`, threaded at token-verification diff --git a/src/mcp/http-transport.ts b/src/mcp/http-transport.ts index b1ee57a5a..bb11864f6 100644 --- a/src/mcp/http-transport.ts +++ b/src/mcp/http-transport.ts @@ -34,7 +34,7 @@ import { VERSION } from '../version.ts'; import { dispatchToolCall } from './dispatch.ts'; import { buildDefaultLimiters, type RateLimiter } from './rate-limit.ts'; import { sqlQueryForEngine } from '../core/sql-query.ts'; -import { parseLegacyTokenScope } from '../core/legacy-token-scope.ts'; +import { parseLegacyTokenScope, parseTakesHoldersAllowList, coerceLegacyPermissions } from '../core/legacy-token-scope.ts'; export { parseLegacyTokenScope }; const DEFAULT_BODY_CAP = 1024 * 1024; // 1 MiB @@ -214,10 +214,13 @@ export async function startHttpTransport(opts: HttpTransportOptions) { .catch(() => { /* fire-and-forget */ }); // v0.28: extract per-token takes-holder allow-list. Fail-safe default // is ['world'] — a token with no permissions row sees public claims only. - const perms = (row as { permissions?: { takes_holders?: unknown; source_id?: unknown } }).permissions; - const allowList = Array.isArray(perms?.takes_holders) - ? (perms!.takes_holders as unknown[]).filter(h => typeof h === 'string') as string[] - : ['world']; + // #2529: decode + parse via the shared core helpers so this transport and + // the OAuth provider behind `serve --http` cannot drift — including a + // double-encoded jsonb string scalar (#2339 class), which both now decode + // identically instead of one honoring the grant while the other fails + // open to ['world']. + const perms = coerceLegacyPermissions((row as { permissions?: unknown }).permissions); + const allowList = parseTakesHoldersAllowList(perms?.takes_holders) ?? ['world']; // #1336: honor the operator-set source grant stored on the token. const { sourceId, allowedSources } = parseLegacyTokenScope(perms?.source_id); const auth: AuthInfo = { diff --git a/test/agent-voice-cors.serial.test.ts b/test/agent-voice-cors.serial.test.ts new file mode 100644 index 000000000..974cd7eac --- /dev/null +++ b/test/agent-voice-cors.serial.test.ts @@ -0,0 +1,266 @@ +/** + * #2477 — agent-voice reference server ships default-deny CORS, an Origin + * gate on the side-effectful POSTs, and a loopback-default bind. + * + * Spawns the real server (recipes/agent-voice/code/server.mjs) and asserts: + * + * 1. Default posture (no env): no Access-Control-Allow-Origin header is + * ever emitted; cross-origin POSTs to /session and /tool are rejected + * 403 BEFORE any work (proven by ordering: a no-Origin POST /session + * reaches the handler and 500s on the missing OPENAI_API_KEY instead). + * Same-origin requests (Origin host == Host header) pass the gate. + * 2. Allowlist posture (AGENT_VOICE_CORS_ORIGIN): exact-match origins get + * ACAO echo + Vary: Origin (+ preflight headers on OPTIONS) and pass + * the gate; everything else stays default-deny. Spaces around commas + * are trimmed. + * + * `.serial` suffix: binds TCP ports, runs in the serial pass. + * Pattern per test/admin-embed-spawn.serial.test.ts (spawn + /health poll + + * SIGTERM→SIGKILL cleanup). The default HOST bind (127.0.0.1) is covered + * implicitly — every request here reaches the server via 127.0.0.1. + */ + +import { describe, test, expect } from 'bun:test'; +import type { Subprocess } from 'bun'; +import { join } from 'path'; + +const SERVER_SCRIPT = join(import.meta.dir, '..', 'recipes', 'agent-voice', 'code', 'server.mjs'); +const EVIL = 'https://evil.example'; + +function pickPort(): number { + return 31000 + Math.floor(Math.random() * 4000); +} + +interface VoiceServer { + port: number; + base: string; + proc: Subprocess<'ignore', 'pipe', 'pipe'>; + /** Accumulated stdout — carries the startup bind + CORS posture log lines. */ + stdout: () => string; +} + +async function spawnVoice(extraEnv: Record = {}): Promise { + const port = pickPort(); + const env: Record = { + ...(process.env as Record), + PORT: String(port), + // The server starts without a key; /session 500s lazily — which the + // ordering assertions below rely on. + OPENAI_API_KEY: '', + }; + // A developer's shell must not leak an allowlist or bind override into the + // default-deny spawn. + delete env.AGENT_VOICE_CORS_ORIGIN; + delete env.HOST; + Object.assign(env, extraEnv); + + const proc = Bun.spawn([process.execPath, SERVER_SCRIPT], { + env, + stdin: 'ignore', + stdout: 'pipe', + stderr: 'pipe', + }); + + // Drain stdout in the background so the startup log lines are assertable. + let out = ''; + (async () => { + try { + for await (const chunk of proc.stdout as ReadableStream) { + out += new TextDecoder().decode(chunk); + } + } catch { /* stream closed on shutdown */ } + })(); + + const base = `http://127.0.0.1:${port}`; + const deadline = Date.now() + 30_000; + while (Date.now() < deadline) { + try { + const res = await fetch(`${base}/health`, { signal: AbortSignal.timeout(2000) }); + if (res.ok) return { port, base, proc, stdout: () => out }; + } catch { /* not up yet */ } + await new Promise(r => setTimeout(r, 250)); + } + proc.kill(); + const stderr = await new Response(proc.stderr).text(); + throw new Error(`agent-voice server did not become healthy in 30s\nstderr: ${stderr.slice(-800)}`); +} + +async function stopVoice(server: VoiceServer): Promise { + server.proc.kill('SIGTERM'); + const graceful = await Promise.race([ + server.proc.exited.then(() => true), + new Promise(r => setTimeout(() => r(false), 2000)), + ]); + if (!graceful) { + server.proc.kill('SIGKILL'); + await server.proc.exited.catch(() => {}); + } +} + +describe('agent-voice CORS default-deny + origin gate (#2477)', () => { + test('default posture: no CORS headers, cross-origin side-effect POSTs 403 before any work', async () => { + const server = await spawnVoice(); + try { + // No ACAO on a cross-origin read — the browser blocks the response. + const health = await fetch(`${server.base}/health`, { headers: { Origin: EVIL } }); + expect(health.status).toBe(200); + expect(health.headers.get('access-control-allow-origin')).toBeNull(); + expect(health.headers.get('vary') ?? '').not.toContain('Origin'); + + // Preflight still answers 204, but with no CORS headers it fails in + // the browser — the actual cross-origin request never fires. + const preflight = await fetch(`${server.base}/session`, { + method: 'OPTIONS', + headers: { Origin: EVIL, 'Access-Control-Request-Method': 'POST' }, + }); + expect(preflight.status).toBe(204); + expect(preflight.headers.get('access-control-allow-origin')).toBeNull(); + expect(preflight.headers.get('access-control-allow-methods')).toBeNull(); + + // Origin gate: a "simple" cross-origin POST (no preflight in real + // browsers) is rejected before the handler runs. + const gated = await fetch(`${server.base}/session`, { + method: 'POST', + headers: { Origin: EVIL, 'Content-Type': 'text/plain' }, + body: 'v=0', + }); + expect(gated.status).toBe(403); + expect(await gated.text()).toContain('origin not allowed'); + + // Ordering proof: without an Origin header (curl / Twilio / native) + // the same request PASSES the gate and reaches the handler, which + // 500s on the missing OPENAI_API_KEY. 403 above therefore came from + // the gate, before any upstream spend could happen. + const noOrigin = await fetch(`${server.base}/session`, { + method: 'POST', + headers: { 'Content-Type': 'text/plain' }, + body: 'v=0', + }); + expect(noOrigin.status).toBe(500); + expect(await noOrigin.text()).toContain('OPENAI_API_KEY not set'); + + // Same-origin browser requests (Origin host == Host header) pass the + // gate — the served /call page keeps working with zero config. + const sameOrigin = await fetch(`${server.base}/session`, { + method: 'POST', + headers: { Origin: server.base, 'Content-Type': 'text/plain' }, + body: 'v=0', + }); + expect(sameOrigin.status).toBe(500); // handler reached, not 403 + + // Malformed Origin (unparseable by new URL()) must fail closed → 403, + // never fall through to the handler. A bypass here would be a real hole. + const malformed = await fetch(`${server.base}/session`, { + method: 'POST', + headers: { Origin: 'http://[::bogus', 'Content-Type': 'text/plain' }, + body: 'v=0', + }); + expect(malformed.status).toBe(403); + + // /tool: gated identically (before the body is even read)… + const toolGated = await fetch(`${server.base}/tool`, { + method: 'POST', + headers: { Origin: EVIL, 'Content-Type': 'text/plain' }, + body: 'not json', + }); + expect(toolGated.status).toBe(403); + + // …while a no-Origin caller reaches the handler (400 invalid_json). + const toolNoOrigin = await fetch(`${server.base}/tool`, { + method: 'POST', + headers: { 'Content-Type': 'text/plain' }, + body: 'not json', + }); + expect(toolNoOrigin.status).toBe(400); + } finally { + await stopVoice(server); + } + }, 90_000); + + test('allowlist posture: exact-match origins get CORS headers and pass the gate; others stay denied', async () => { + // Space after the comma pins trimming. + const server = await spawnVoice({ AGENT_VOICE_CORS_ORIGIN: 'https://ok.example, https://two.example' }); + try { + // Allowlisted origin: ACAO echoes the exact origin + Vary: Origin. + const ok = await fetch(`${server.base}/health`, { headers: { Origin: 'https://ok.example' } }); + expect(ok.headers.get('access-control-allow-origin')).toBe('https://ok.example'); + expect(ok.headers.get('vary') ?? '').toContain('Origin'); + + // Second (space-padded) entry works too — trimming pinned. + const two = await fetch(`${server.base}/health`, { headers: { Origin: 'https://two.example' } }); + expect(two.headers.get('access-control-allow-origin')).toBe('https://two.example'); + + // Preflight from an allowlisted origin carries the method/header grants. + const preflight = await fetch(`${server.base}/session`, { + method: 'OPTIONS', + headers: { Origin: 'https://ok.example', 'Access-Control-Request-Method': 'POST' }, + }); + expect(preflight.status).toBe(204); + expect(preflight.headers.get('access-control-allow-origin')).toBe('https://ok.example'); + expect(preflight.headers.get('access-control-allow-methods')).toContain('POST'); + // Never credentialed, even when allowlisted. + expect(preflight.headers.get('access-control-allow-credentials')).toBeNull(); + + // Allowlisted origin passes the side-effect gate (handler reached). + const gatePass = await fetch(`${server.base}/session`, { + method: 'POST', + headers: { Origin: 'https://ok.example', 'Content-Type': 'text/plain' }, + body: 'v=0', + }); + expect(gatePass.status).toBe(500); + expect(await gatePass.text()).toContain('OPENAI_API_KEY not set'); + + // Exact match, not wildcard: unlisted origins stay fully denied. + const evil = await fetch(`${server.base}/health`, { headers: { Origin: EVIL } }); + expect(evil.headers.get('access-control-allow-origin')).toBeNull(); + const evilPost = await fetch(`${server.base}/session`, { + method: 'POST', + headers: { Origin: EVIL, 'Content-Type': 'text/plain' }, + body: 'v=0', + }); + expect(evilPost.status).toBe(403); + } finally { + await stopVoice(server); + } + }, 90_000); + + // The loopback-default bind is the security-relevant half of #2477. A raw + // socket probe of a non-loopback interface is flaky in CI (no LAN IP, + // firewalls), so pin the HOST knob via the startup log the listen callback + // emits: default → 127.0.0.1, HOST override → honored. + async function readStartupLog(server: VoiceServer): Promise { + const deadline = Date.now() + 3000; + while (Date.now() < deadline) { + if (server.stdout().includes('listening on')) break; + await new Promise(r => setTimeout(r, 50)); + } + return server.stdout(); + } + + test('HOST default binds loopback (127.0.0.1)', async () => { + const server = await spawnVoice(); // HOST scrubbed from env → default + try { + const log = await readStartupLog(server); + expect(log).toContain(`listening on http://127.0.0.1:${server.port}`); + expect(log).toContain('bind: 127.0.0.1'); + // The default log carries a "set HOST=0.0.0.0 to expose" hint, so the + // bind proof is the loopback URL above, not mere absence of "0.0.0.0". + expect(log).not.toContain('http://0.0.0.0'); + } finally { + await stopVoice(server); + } + }, 90_000); + + test('HOST override is honored (0.0.0.0 for containers/LAN)', async () => { + const server = await spawnVoice({ HOST: '0.0.0.0' }); + try { + const log = await readStartupLog(server); + expect(log).toContain(`listening on http://0.0.0.0:${server.port}`); + // Still reachable via loopback when bound to all interfaces. + const health = await fetch(`${server.base}/health`); + expect(health.status).toBe(200); + } finally { + await stopVoice(server); + } + }, 90_000); +}); diff --git a/test/e2e/serve-http-takes-holders.test.ts b/test/e2e/serve-http-takes-holders.test.ts new file mode 100644 index 000000000..72395ff0d --- /dev/null +++ b/test/e2e/serve-http-takes-holders.test.ts @@ -0,0 +1,189 @@ +/** + * E2E regression for #2529: `serve --http` honors a legacy bearer token's + * stored `access_tokens.permissions.takes_holders` grant end-to-end. + * + * The seam under test — verifyAccessToken (oauth-provider.ts legacy branch) + * → AuthInfo.takesHoldersAllowList → /mcp dispatch → engine holder filter — + * had NO end-to-end pin before this file; that absence is how #2529 shipped + * (the OAuth provider never read the grant, so every remote caller was + * pinned to ['world'] regardless of what the operator configured). + * + * Spins up a real `gbrain serve --http` against real Postgres, seeds a page + * with a brain-held and a world-held take, inserts legacy tokens with three + * different grants directly (the same rows `gbrain auth create/permissions` + * writes), and asserts `takes_list` over POST /mcp filters per grant: + * + * ['world','brain'] → sees both takes + * ['world'] → world take only (brain take invisible) + * [] → explicit deny-all: sees neither + * + * Run: DATABASE_URL=... bun test test/e2e/serve-http-takes-holders.test.ts + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { setupDB, teardownDB, getConn, hasDatabase } from './helpers.ts'; +import { importFromContent } from '../../src/core/import-file.ts'; +import { hashToken, generateToken } from '../../src/core/utils.ts'; + +const skip = !hasDatabase(); +const describeE2E = skip ? describe.skip : describe; + +if (skip) { + console.log('Skipping E2E serve-http-takes-holders tests (DATABASE_URL not set)'); +} + +const PORT = 19141; // unique per e2e file — avoid collision with serve-http-oauth (19131) +const BASE = `http://localhost:${PORT}`; + +const BRAIN_CLAIM = 'brain-held regression claim for issue 2529'; +const WORLD_CLAIM = 'world-held regression claim for issue 2529'; + +describeE2E('serve --http honors permissions.takes_holders for legacy bearer tokens (#2529)', () => { + let serverProcess: ReturnType | null = null; + let fullToken: string; + let worldToken: string; + let denyAllToken: string; + let noGrantToken: string; + + async function insertLegacyToken(name: string, takesHolders: string[]): Promise { + const token = generateToken('gbrain_'); + const conn = getConn(); + await conn.unsafe( + `INSERT INTO access_tokens (id, name, token_hash, permissions) + VALUES (gen_random_uuid(), $1, $2, $3::text::jsonb)`, + [name, hashToken(token), JSON.stringify({ takes_holders: takesHolders })], + ); + return token; + } + + // A token whose permissions carry NO takes_holders key — the shape an + // OAuth-client token or a pre-`set-takes-holders` legacy token has. The + // dispatch site must coalesce the undefined grant to the fail-closed + // ['world'] default over the wire (serve-http.ts `?? ['world']`). + async function insertNoGrantToken(name: string): Promise { + const token = generateToken('gbrain_'); + await getConn().unsafe( + `INSERT INTO access_tokens (id, name, token_hash, permissions) + VALUES (gen_random_uuid(), $1, $2, '{}'::jsonb)`, + [name, hashToken(token)], + ); + return token; + } + + beforeAll(async () => { + const engine = await setupDB(); + const conn = getConn(); + + // access_tokens is not in helpers' truncate list — clear prior runs' rows + // so the unique name constraint can't collide. + await conn.unsafe(`DELETE FROM access_tokens WHERE name LIKE 'takes-e2e-%'`); + + // Seed a page + two takes (one per holder tier). + await importFromContent(engine, 'e2e/takes-regression', '# Takes Regression\n\nSeed page for #2529.', { noEmbed: true }); + const [page] = await conn.unsafe(`SELECT id FROM pages WHERE slug = 'e2e/takes-regression'`); + await engine.addTakesBatch([ + { page_id: page.id as number, row_num: 1, claim: BRAIN_CLAIM, kind: 'take', holder: 'brain', weight: 0.75, active: true, superseded_by: null }, + { page_id: page.id as number, row_num: 2, claim: WORLD_CLAIM, kind: 'take', holder: 'world', weight: 0.5, active: true, superseded_by: null }, + ]); + + // Legacy bearer tokens with three grant shapes — the same permissions + // rows `gbrain auth create --takes-holders` / `set-takes-holders` write. + fullToken = await insertLegacyToken('takes-e2e-full', ['world', 'brain']); + worldToken = await insertLegacyToken('takes-e2e-world', ['world']); + denyAllToken = await insertLegacyToken('takes-e2e-denyall', []); + noGrantToken = await insertNoGrantToken('takes-e2e-nogrant'); + + // Start the HTTP server (same pattern as serve-http-oauth.test.ts). + const { spawn } = await import('child_process'); + serverProcess = spawn('bun', [ + 'run', 'src/cli.ts', 'serve', '--http', + '--port', String(PORT), + '--public-url', `http://localhost:${PORT}`, + ], { + cwd: process.cwd(), + env: process.env, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + let stderr = ''; + serverProcess.stderr?.on('data', (d: Buffer) => { stderr += d.toString(); }); + + let ready = false; + for (let i = 0; i < 30; i++) { + try { + const res = await fetch(`${BASE}/health`); + if (res.ok) { ready = true; break; } + } catch {} + await new Promise(r => setTimeout(r, 500)); + } + if (!ready) throw new Error('Server failed to start within 15s.\nstderr: ' + stderr.slice(-500)); + }, 60_000); + + afterAll(async () => { + if (serverProcess) { + serverProcess.kill('SIGTERM'); + await new Promise(r => setTimeout(r, 1000)); + if (!serverProcess.killed) serverProcess.kill('SIGKILL'); + } + try { + await getConn().unsafe(`DELETE FROM access_tokens WHERE name LIKE 'takes-e2e-%'`); + } catch (e: any) { + // eslint-disable-next-line no-console + console.error(`[afterAll] token cleanup failed: ${e.message}`); + } + await teardownDB(); + }, 30_000); + + async function takesListBody(token: string): Promise { + const res = await fetch(`${BASE}/mcp`, { + method: 'POST', + headers: { + 'Authorization': `Bearer ${token}`, + 'Content-Type': 'application/json', + 'Accept': 'application/json, text/event-stream', + }, + body: JSON.stringify({ + jsonrpc: '2.0', id: 1, method: 'tools/call', + params: { name: 'takes_list', arguments: {} }, + }), + }); + expect(res.status).not.toBe(401); + return res.text(); + } + + // The MCP transport can answer as JSON or SSE (text/event-stream). Assert + // the body carries a successful tool result, not a JSON-RPC error envelope, + // so a negative-only content check can't pass vacuously on a broken pipeline. + function expectToolResultOk(body: string): void { + expect(body).toContain('"result"'); + expect(body).not.toContain('"error"'); + } + + test("['world','brain'] grant sees the brain-held take (the #2529 repro)", async () => { + const body = await takesListBody(fullToken); + expect(body).toContain(BRAIN_CLAIM); + expect(body).toContain(WORLD_CLAIM); + }, 15_000); + + test("['world'] grant sees only world-held takes — brain take stays invisible", async () => { + const body = await takesListBody(worldToken); + expect(body).toContain(WORLD_CLAIM); + expect(body).not.toContain(BRAIN_CLAIM); + }, 15_000); + + test('[] grant is explicit deny-all — sees neither take (not silently world)', async () => { + const body = await takesListBody(denyAllToken); + expectToolResultOk(body); // a successful empty result, not an error envelope + expect(body).not.toContain(BRAIN_CLAIM); + expect(body).not.toContain(WORLD_CLAIM); + }, 15_000); + + test('no takes_holders grant → fail-closed to world-only over the wire (?? default)', async () => { + // Pins serve-http.ts `authInfo.takesHoldersAllowList ?? ['world']`: an + // undefined grant (OAuth clients, pre-set-takes-holders legacy tokens) + // must see world-held takes and NOT brain-held ones through /mcp dispatch. + const body = await takesListBody(noGrantToken); + expect(body).toContain(WORLD_CLAIM); + expect(body).not.toContain(BRAIN_CLAIM); + }, 15_000); +}); diff --git a/test/facts-meta-cache.test.ts b/test/facts-meta-cache.test.ts index 7b9415fab..d832cbf2c 100644 --- a/test/facts-meta-cache.test.ts +++ b/test/facts-meta-cache.test.ts @@ -154,4 +154,33 @@ describe('meta-hook cache', () => { expect(r1?.brain_hot_memory).toBeDefined(); expect(r2?.brain_hot_memory).toBeDefined(); }); + + test('[] (explicit deny-all) and undefined allow-lists do NOT share a cache entry (#2529)', async () => { + // Isolated source so topK saturation from other tests can't mask the + // count difference this test keys on. + const src = 'deny-key-src'; + await engine.executeRaw( + `INSERT INTO sources (id, name, config) VALUES ($1, $1, '{}'::jsonb) ON CONFLICT (id) DO NOTHING`, + [src], + ); + await engine.insertFact( + { fact: 'deny-key seed fact', kind: 'fact', entity_slug: 'deny-key', visibility: 'world', source: 'test' }, + { source_id: src }, + ); + // Warm the cache under the UNSET allow-list key. + const unset = await getBrainHotMemoryMeta('get_stats', ctx({ sourceId: src })); + const unsetCount = (unset?.brain_hot_memory as { facts: unknown[] } | undefined)?.facts.length ?? 0; + expect(unsetCount).toBeGreaterThan(0); + + // New fact lands AFTER the warm — a shared cache key would serve the + // stale (pre-insert) payload to the [] caller. Pre-fix, hashAllowList + // collapsed both to '_' and this returned unsetCount. + await engine.insertFact( + { fact: 'deny-key post-warm fact', kind: 'fact', entity_slug: 'deny-key', visibility: 'world', source: 'test' }, + { source_id: src }, + ); + const emptyList = await getBrainHotMemoryMeta('get_stats', ctx({ sourceId: src, takesHoldersAllowList: [] })); + const emptyCount = (emptyList?.brain_hot_memory as { facts: unknown[] } | undefined)?.facts.length ?? 0; + expect(emptyCount).toBeGreaterThan(unsetCount); + }); }); diff --git a/test/legacy-token-federated-scope.test.ts b/test/legacy-token-federated-scope.test.ts index 3cf1547b9..0b75be09a 100644 --- a/test/legacy-token-federated-scope.test.ts +++ b/test/legacy-token-federated-scope.test.ts @@ -11,6 +11,7 @@ */ import { describe, test, expect } from 'bun:test'; import { parseLegacyTokenScope } from '../src/mcp/http-transport.ts'; +import { parseTakesHoldersAllowList, coerceLegacyPermissions } from '../src/core/legacy-token-scope.ts'; describe('parseLegacyTokenScope', () => { test('array grant → allowedSources (federated read) with first as scalar floor', () => { @@ -43,3 +44,63 @@ describe('parseLegacyTokenScope', () => { expect(parseLegacyTokenScope(['a', 5, '', 'b'])).toEqual({ sourceId: 'a', allowedSources: ['a', 'b'] }); }); }); + +// #2529 — permissions.takes_holders parse contract, shared by the legacy HTTP +// transport and the OAuth provider behind `serve --http` so the two cannot +// drift. Imported from the owner module (src/core/legacy-token-scope.ts), not +// re-exported through a transport. +describe('parseTakesHoldersAllowList', () => { + test('string array passes through unchanged', () => { + expect(parseTakesHoldersAllowList(['world', 'brain'])).toEqual(['world', 'brain']); + }); + + test('empty array is PRESERVED as explicit deny-all (not collapsed)', () => { + const result = parseTakesHoldersAllowList([]); + expect(result).toBeDefined(); + expect(result).toEqual([]); + }); + + test('non-string entries are filtered; empty strings KEPT (deliberate divergence from parseLegacyTokenScope)', () => { + expect(parseTakesHoldersAllowList(['world', 42, null, '', 'brain'])).toEqual(['world', '', 'brain']); + }); + + test('non-array values → undefined (consumer applies fail-closed default)', () => { + expect(parseTakesHoldersAllowList('world')).toBeUndefined(); + expect(parseTakesHoldersAllowList(123)).toBeUndefined(); + expect(parseTakesHoldersAllowList(null)).toBeUndefined(); + expect(parseTakesHoldersAllowList(undefined)).toBeUndefined(); + expect(parseTakesHoldersAllowList({ takes_holders: ['world'] })).toBeUndefined(); + }); +}); + +// #2529 (ship adversarial review): both transports decode a legacy token's +// permissions column through this shared coercer so a double-encoded jsonb +// string scalar (#2339 class) is interpreted identically — the OAuth provider +// can't honor a grant the legacy HTTP transport silently fails open on. +describe('coerceLegacyPermissions', () => { + test('plain object passes through', () => { + expect(coerceLegacyPermissions({ takes_holders: ['world', 'brain'] })).toEqual({ takes_holders: ['world', 'brain'] }); + }); + + test('JSON string (double-encoded jsonb scalar) is decoded to its object', () => { + const decoded = coerceLegacyPermissions(JSON.stringify({ takes_holders: [], source_id: 'x' })); + expect(decoded).toEqual({ takes_holders: [], source_id: 'x' }); + // Downstream parse then honors the [] deny-all grant — not fail-open world. + expect(parseTakesHoldersAllowList(decoded?.takes_holders)).toEqual([]); + }); + + test('malformed JSON string → undefined (no grant)', () => { + expect(coerceLegacyPermissions('{not valid json')).toBeUndefined(); + }); + + test('JSON string that decodes to a non-object (array/scalar) → undefined', () => { + expect(coerceLegacyPermissions('"world"')).toBeUndefined(); + expect(coerceLegacyPermissions('[1,2]')).toBeUndefined(); + }); + + test('null / undefined / scalar → undefined', () => { + expect(coerceLegacyPermissions(null)).toBeUndefined(); + expect(coerceLegacyPermissions(undefined)).toBeUndefined(); + expect(coerceLegacyPermissions(42)).toBeUndefined(); + }); +}); diff --git a/test/oauth.test.ts b/test/oauth.test.ts index cd223ae87..91f32ad95 100644 --- a/test/oauth.test.ts +++ b/test/oauth.test.ts @@ -509,6 +509,84 @@ describe('verifyAccessToken', () => { expect(authInfo.sourceId).toBe('default'); expect(authInfo.allowedSources).toEqual(['default', 'src-a', 'src-b']); }); + + // ------------------------------------------------------------------------- + // #2529 — legacy access_tokens fallback threads permissions.takes_holders + // into AuthInfo.takesHoldersAllowList. Each test adds the v29 permissions + // column idempotently and inserts `permissions` EXPLICITLY: the column's + // NOT NULL DEFAULT is '{"takes_holders":["world"]}', so relying on the + // default would silently turn an "absent key" case into a ['world'] case. + // ------------------------------------------------------------------------- + + async function insertLegacyTokenWithPermissions( + name: string, + permissions: Record | undefined, + ): Promise { + await sql` + ALTER TABLE access_tokens + ADD COLUMN IF NOT EXISTS permissions JSONB NOT NULL DEFAULT '{"takes_holders":["world"]}'::jsonb + `; + const token = generateToken('gbrain_'); + const hash = hashToken(token); + if (permissions === undefined) { + await sql` + INSERT INTO access_tokens (id, name, token_hash) + VALUES (${crypto.randomUUID()}, ${name}, ${hash}) + `; + } else { + await sql` + INSERT INTO access_tokens (id, name, token_hash, permissions) + VALUES (${crypto.randomUUID()}, ${name}, ${hash}, ${JSON.stringify(permissions)}::jsonb) + `; + } + return token; + } + + test('legacy token with takes_holders grant → takesHoldersAllowList threaded (#2529)', async () => { + const token = await insertLegacyTokenWithPermissions('takes-grant-agent', { takes_holders: ['world', 'brain'] }); + const authInfo = await provider.verifyAccessToken(token) as CoreAuthInfo; + expect(authInfo.takesHoldersAllowList).toEqual(['world', 'brain']); + }); + + test('legacy token with no takes_holders key → undefined (consumer defaults to world)', async () => { + const token = await insertLegacyTokenWithPermissions('takes-absent-agent', {}); + const authInfo = await provider.verifyAccessToken(token) as CoreAuthInfo; + expect(authInfo.takesHoldersAllowList).toBeUndefined(); + }); + + test('legacy token with non-array takes_holders → undefined (fail-closed at consumer)', async () => { + const token = await insertLegacyTokenWithPermissions('takes-garbage-agent', { takes_holders: 'world' }); + const authInfo = await provider.verifyAccessToken(token) as CoreAuthInfo; + expect(authInfo.takesHoldersAllowList).toBeUndefined(); + }); + + test('legacy token with empty-array takes_holders → [] preserved as explicit deny-all', async () => { + const token = await insertLegacyTokenWithPermissions('takes-denyall-agent', { takes_holders: [] }); + const authInfo = await provider.verifyAccessToken(token) as CoreAuthInfo; + expect(authInfo.takesHoldersAllowList).toBeDefined(); + expect(authInfo.takesHoldersAllowList).toEqual([]); + }); + + test('legacy token with mixed-type takes_holders → non-string entries filtered', async () => { + const token = await insertLegacyTokenWithPermissions('takes-mixed-agent', { takes_holders: ['world', 42, null] }); + const authInfo = await provider.verifyAccessToken(token) as CoreAuthInfo; + expect(authInfo.takesHoldersAllowList).toEqual(['world']); + }); + + test('OAuth-client token → takesHoldersAllowList undefined (no per-client storage; fail-closed)', async () => { + const { clientId, clientSecret } = await provider.registerClientManual( + 'takes-oauth-client', ['client_credentials'], 'read', + ); + const tokens = await provider.exchangeClientCredentials(clientId, clientSecret!, 'read'); + const authInfo = await provider.verifyAccessToken(tokens.access_token) as CoreAuthInfo; + expect(authInfo.takesHoldersAllowList).toBeUndefined(); + }); + + test('legacy token relying on the v29 column default → ["world"] (fix invisible to unrestricted tokens)', async () => { + const token = await insertLegacyTokenWithPermissions('takes-default-agent', undefined); + const authInfo = await provider.verifyAccessToken(token) as CoreAuthInfo; + expect(authInfo.takesHoldersAllowList).toEqual(['world']); + }); }); // ---------------------------------------------------------------------------