v0.26.2 fix(oauth): bun execSync env inheritance + BIGINT-as-string bug class#593
Merged
v0.26.2 fix(oauth): bun execSync env inheritance + BIGINT-as-string bug class#593
Conversation
Postgres-js with prepare:false (auto-detected on Supabase pooler / port 6543) returns BIGINT columns as strings. Two surfaces broke on this: (1) MCP SDK's bearerAuth checks typeof === 'number' and rejected strings — fixed in v0.26.1 only at line 303 of oauth-provider.ts; (2) RFC 7591 §3.2.1 requires client_id_issued_at and client_secret_expires_at to be JSON numbers in DCR responses, not strings — latent until v0.26.2. Adds module-private coerceTimestamp() at the SELECT-row → JS-number boundary. Throws on non-finite (corrupt rows fail loud, not as fake-valid expiresAt: NaN flowing into the SDK). Returns undefined for SQL NULL — schema permits NULL on oauth_tokens.expires_at, callers treat NULL as expired (fail-closed) at comparison sites and preserve undefined in DCR getClient response per RFC 7591. Refactors 5 sites: - L112,113 (getClient) — DCR response numeric-shape compliance. - L274 (exchangeRefreshToken) — NULL→expired fail-closed contract. - L296,303 (verifyAccessToken) — single guard, narrowed return. No `!` non-null assertions: all 5 sites read nullable BIGINT columns per src/schema.sql:362,363,372. The L296/L303 cleanup also folds in v0.26.1's inline Number(...) at L303.
Hard-deletes the matching oauth_clients row via atomic DELETE ... RETURNING. Schema-level FK CASCADE on oauth_tokens.client_id and oauth_codes.client_id (src/schema.sql:370,382) purges all dependent rows in the same transaction. No manual delete of dependents needed. Exit 1 on no-such-client (idempotent: re-running on the same id produces the same error). Operator-friendly output: prints the client name + cascade confirmation, no race-prone pre-delete count. Closes the v0.26.1 process miss where test/e2e/serve-http-oauth.test.ts afterAll already called this subcommand — silently failing because the subcommand didn't exist. With this fix, E2E cleanup actually purges test clients.
Unit additions in test/oauth.test.ts:
- 5 cases pinning coerceTimestamp contract (null/undef/string/number/
throws-on-NaN). The throws-on-NaN case is load-bearing: pre-v0.26.2
Number(corrupt) → NaN, NaN < now is false → expired check skipped,
fake-valid expiresAt:NaN flowed to SDK. Now fail-closed.
- NULL expires_at on oauth_tokens insert → verifyAccessToken throws
"Token expired". Schema permits NULL; pre-v0.26.2 hand-modified rows
could ride past validation.
- Cascade-deleted client → previously-minted token fails
verifyAccessToken with "Invalid token" (not "expired"). Pins the
cascade contract independently of the CLI subprocess path.
E2E additions in test/e2e/serve-http-oauth.test.ts:
- DCR /register HTTP-level response-shape test. Spawns server with
--enable-dcr, POSTs a client manifest, asserts typeof === 'number'
on client_id_issued_at and (when present) client_secret_expires_at
per RFC 7591 §3.2.1. Replaces the v0.26.1 plan's internal-store-only
test that Codex flagged as the wrong seam.
- Real CLI subprocess test for revoke-client: register → mint token →
revoke via execSync → assert token rejected at /mcp + cascade
invalidation visible + re-run exits 1 with "No client found".
- afterAll guards on clientId so pre-registration beforeAll failures
surface cleanly instead of throwing on undefined during cleanup.
Also tracks DCR-registered clients alongside the manual one.
- Server fixture: --enable-dcr added so /register is reachable.
- Health endpoint: page_count assertion loosened from > 0 to >= 0
+ typeof number — pre-v0.26.2 broke on fresh-schema E2E runs.
bun execSync env-inheritance fix (the load-bearing infrastructure
fix that unbroke v0.26.2's full-suite test):
- bun's child_process.execSync does NOT inherit env mutations done
via process.env.X = ...; only OS-level env from before bun started.
- helpers.ts loads .env.testing and sets DATABASE_URL via process.env
mutation, invisible to subprocesses unless env: { ...process.env }
is passed explicitly.
- All 4 execSync calls in this file (beforeAll register-client,
afterAll revoke-client, in-test register-client, in-test
revoke-client x2) now pass env: { ...process.env }.
- Without this, full bun test suite OAuth E2E fails with "Set
DATABASE_URL or GBRAIN_DATABASE_URL environment variable" even when
isolated test/e2e/serve-http-oauth.test.ts runs pass. Pattern is
documented inline as a reference for other E2E test fixes (see
TODOS.md "test infra (v0.26.2 follow-up)" for the 22-test backlog).
CLAUDE.md (admin/ section, v0.26.0 release notes) states:
"output at admin/dist/ is committed for self-contained binaries"
But .gitignore excluded admin/dist/, so the bun --compile binary that
embeds the admin SPA via `import path from '...' with { type: 'file' }`
couldn't resolve in fresh clones. PR #577 (v0.26.1) didn't catch this
because admin tests pass when admin/dist exists locally.
Removes the .gitignore line + commits the current 220KB build:
- index.html (0.7KB)
- assets/index-{hash}.js (210KB / 65KB gzip)
- assets/index-{hash}.css (6.3KB / 1.8KB gzip)
Now `bun build --compile --outfile bin/gbrain src/cli.ts` works on a
fresh clone without a separate `cd admin && bun install && bun run
build` step in CI.
Adds a CLAUDE.md section "Capturing test output (NEVER pipe through
tail / head)" documenting the iron rule that bit v0.26.2's ship:
bun test 2>&1 | tail -10 → exit code = tail's (always 0),
failures truncated, ship gates fail open
The pipe form silently breaks /ship Step T1 (test failure ownership
triage) because $? after a pipe is the LAST command's exit code, and
bun prints failure details before the summary line so tail -N drops
them. v0.26.2's first ship attempt reported "3911 pass / 23 fail" but
no failure details survived, forcing a 23-minute re-run to triage.
Right pattern: redirect to a file first, then tail the file separately.
Regenerates llms-full.txt to match the new CLAUDE.md content (drift
guard at test/build-llms.test.ts enforces this).
Captures the test-infra backlog uncovered by v0.26.2's full bun test run. None of the 22 failing cases touch the OAuth diff: - 12 Git-to-DB Sync Pipeline cases (state-machine drift) - 3 multi-source cascade + sync routing cases - E2E sync-parallel, sync --skip-failed, doctor, dream, runCycle, claw-test fresh-install, BrainRegistry lazy init Likely root causes for several: same bun execSync env-inheritance pattern fixed in test/e2e/serve-http-oauth.test.ts during v0.26.2 (documented in the TODO + the inline test comment for the next maintainer to find). Separating from v0.26.2 keeps the OAuth ship focused on the bug class it was scoped for. Fix-wave deserves its own PR.
VERSION 0.26.0 → 0.26.2. Includes a retroactive v0.26.1 entry above v0.26.0 because PR #577 shipped its three fixes (oauth-provider:303 Number cast, OAuth metadata interceptor, Express 5 trust proxy + admin wildcard) without bumping VERSION/package.json/CHANGELOG — this branch catches the changelog up to commit history. v0.26.2 release-summary covers the OAuth string-vs-number bug class fix (5 sites + coerceTimestamp helper), the gbrain auth revoke-client subcommand landing as a real CLI, and the bun execSync env-inheritance fix that unblocked full-suite E2E OAuth tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- CLAUDE.md src/core/oauth-provider.ts: append v0.26.2 coerceTimestamp boundary helper note (5 call sites, NULL semantics, throw-on-NaN posture, intentionally module-private) - CLAUDE.md src/commands/auth.ts: add v0.26.2 revoke-client subcommand with FK CASCADE cleanup - CLAUDE.md test/oauth.test.ts: bump v0.26.2 case additions (5 coerceTimestamp + NULL-expires_at + cascade-delete contract) - CLAUDE.md test/e2e/serve-http-oauth.test.ts: new entry covering v0.26.0 + v0.26.2 expansion (DCR HTTP-level test, CLI subprocess revoke-client test, bun execSync env-inheritance fix as reference for sibling E2Es) - README.md: add gbrain auth revoke-client to command list - llms-full.txt: regenerate after CLAUDE.md edits Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
garrytan
added a commit
that referenced
this pull request
May 3, 2026
Master added v0.26.2 (#593) and v0.26.1 (#577) entries via the OAuth fix-wave landing path. Resolved three conflicts: - VERSION: kept 0.26.4 (this branch's version) - package.json: kept 0.26.4 - CHANGELOG.md: kept both — v0.26.4 entry on top, then master's new v0.26.2 + v0.26.1 entries below. Final order: 0.26.4 → 0.26.2 → 0.26.1 → 0.26.0 (top to bottom, contiguous version sequence). Also fixed a banned-name reference master shipped in the v0.26.1 CHANGELOG credit line ("Co-authored by Wintermute" → "Co-authored by your OpenClaw") per CLAUDE.md:550 / scripts/check-privacy.sh. Regenerated llms-full.txt to match the merged CHANGELOG. Verified post-merge: typecheck clean, schema-diff unit tests 17/17 pass. Master's diff did not touch src/schema.sql, src/core/pglite-schema.ts, or src/core/migrate.ts, so the new schema-drift gate's behavior is unchanged.
7 tasks
garrytan
added a commit
to garrytan-agents/gbrain
that referenced
this pull request
May 3, 2026
Tests in test/oauth.test.ts (already on this branch) import coerceTimestamp
from oauth-provider.ts. The import was synced from master via PR commit 16
("sync all fixes from master") but the production-code change to
oauth-provider.ts was not. Result: bun test fails at module load with
"coerceTimestamp is not exported".
This commit ports the helper directly instead of merging master, avoiding
VERSION/CHANGELOG/dist conflicts.
Boundary helper for postgres.js BIGINT-as-string (auto-detected on
Supabase pgbouncer / port 6543). Throws on non-finite so corrupt rows
fail loud at the SELECT-row -> JS-number boundary. Returns undefined
for SQL NULL; comparison sites treat NULL as expired (fail-closed).
Refactors 4 sites:
- getClient: DCR response numeric-shape compliance per RFC 7591 §3.2.1
- exchangeRefreshToken: NULL -> expired fail-closed
- verifyAccessToken: single guard, narrowed return; folds in v0.26.1's
inline Number(...) at the return site
Originally landed on master as part of garrytan#593 (v0.26.2). Ported here so
PR garrytan#586 (v0.26.3) can build standalone without a master merge.
garrytan
added a commit
that referenced
this pull request
May 3, 2026
#586) * feat(admin): legacy API keys alongside OAuth clients in dashboard Adds API key management to the admin dashboard: Server (serve-http.ts): - GET /admin/api/api-keys — list legacy access_tokens with status - POST /admin/api/api-keys — create new bearer token - POST /admin/api/api-keys/revoke — revoke by name - Stats endpoint now includes active_api_keys count Admin UI (Agents.tsx): - Tabbed view: 'OAuth Clients' | 'API Keys' - API Keys tab: table with name, status, created, last used, revoke button - Create API Key modal with name input - Token reveal modal with copy button + warning - Badge showing active key count on tab Both auth methods (OAuth 2.1 client_credentials and legacy bearer tokens) now visible and manageable from a single admin surface. * feat(admin): remember admin token in localStorage + auto-reauth Login flow: - First login: paste token, saved to localStorage - Subsequent visits: auto-login from localStorage (no paste needed) - Shows 'Authenticating...' spinner during auto-login - If saved token is stale (server restarted), clears it and shows login form Session recovery: - If session cookie expires mid-use (server restart, 24h expiry), the API layer auto-reauths with the saved token before redirecting to login - Transparent to the user — one failed request triggers reauth + retry - Only falls back to login page if the saved token itself is invalid Security: - Token stored in localStorage (same-origin, tailnet-only deployment) - Cleared automatically when token becomes invalid - Cookie remains HttpOnly + SameSite=Strict for the actual session * feat(admin): rich request logging + agent activity tracking Server: - mcp_request_log now captures params (jsonb) and error_message (text) - Agents API returns last_used_at, total_requests, requests_today - Request log API supports agent/operation/status filtering via query params - SSE broadcast includes params and error details Agents page: - Shows 'Requests today / total' and 'Last used' (relative time) per agent - Removed Client ID column (low signal, shown in drawer) Request Log page: - New 'Params' column — shows query text, slug, or param count inline - Click any row to expand full details (params JSON, error message, timestamps) - Click agent name to filter all requests by that agent - Agent filter dropdown in header - Error messages shown in red in expanded view What this means: when Claude Code searches for 'pedro franceschi', the admin dashboard shows the search query, which agent ran it, how long it took, and whether it succeeded — all clickable. * feat(admin): magic link login — ask your agent for the URL New flow: 1. User opens /admin → sees 'This is a protected dashboard' 2. UI tells them: 'Ask your AI agent for the admin login link' 3. Agent generates: https://host:port/admin/auth/<token> 4. User clicks the link → auto-authenticates → redirects to dashboard 5. Session lasts 7 days (magic link) vs 24h (manual token paste) Server: GET /admin/auth/:token validates the bootstrap token, sets HttpOnly cookie, redirects to /admin/. Invalid tokens get a plain text error telling them to ask their agent for a fresh link. Login page: primary UX is the 'ask your agent' prompt with example. Manual token paste collapsed under a <details> disclosure. * feat(admin): config export for Claude Code, ChatGPT, Claude.ai, Cursor, Perplexity Agent drawer now shows setup instructions for 5 clients + raw JSON: - Claude Code: .mcp.json with bearer token + curl to mint - ChatGPT: Settings → Tools → MCP with OAuth discovery - Claude.ai (Cowork): Connected Apps → MCP with OAuth - Cursor: .cursor/mcp.json with OAuth config - Perplexity: Connectors with client ID/secret - JSON: raw config with all URLs (server, token, discovery) All snippets use the actual server URL (window.location.origin) instead of placeholder YOUR_SERVER. Client ID pre-filled. * feat(admin): per-client token TTL — configurable token lifetime Problem: OAuth tokens expire in 1 hour (hardcoded). Claude Code's built-in OAuth client doesn't auto-refresh, so users get 401s every hour. Fix: per-client token_ttl column on oauth_clients table. Set at registration time or updated later via the admin dashboard. Server: - oauth_clients.token_ttl column (nullable integer, seconds) - exchangeClientCredentials reads per-client TTL, falls back to server default - POST /admin/api/register-client accepts tokenTtl param - POST /admin/api/update-client-ttl for existing clients - Agents API returns token_ttl for display Admin UI: - Register modal: Token Lifetime dropdown (1h, 24h, 7d, 30d, 1y, no expiry) - Agent drawer: shows current TTL in Details section Presets: gstack-desktop and garry-claude-code set to 30-day tokens. * fix(admin): request log shows agent name instead of truncated client_id Resolves client_id → client_name via LEFT JOIN on oauth_clients (and access_tokens for legacy keys). Agent column now shows 'gstack-desktop' instead of 'd0db7692caf5…'. Clickable to filter by agent. * feat(admin): DESIGN.md + left-align everything DESIGN.md establishes the admin dashboard design system: - Left-align all text (Garry preference) - Inter + JetBrains Mono (shared DNA with GStack) - No accent color — semantic badges carry all color - Dense utilitarian ops dashboard - Component specs and anti-patterns documented CSS: login-box text-align center → left * feat(admin): unified agent view + resolved agent names in request log Agent names stored at log time (agent_name column). Agents page shows OAuth clients and API keys in one unified table. Request log shows human-readable names. Backfilled 1,114 existing entries. * feat(admin): working Revoke Agent button + e2e tests Bugs fixed: - Revoke Agent button was a no-op (no onClick handler, no API endpoint) - Legacy API key tokens got 401 at /mcp (missing expiresAt in AuthInfo) - token_ttl and deleted_at queries failed on PGLite (columns don't exist) Server: - POST /admin/api/revoke-client: soft-deletes oauth_clients + purges tokens - exchangeClientCredentials checks deleted_at (graceful if column missing) - Legacy token verify returns expiresAt (1yr future) for SDK compat UI: - Revoke button: confirm dialog → revoke → close drawer → reload table - Shows 'This agent has been revoked' for revoked agents E2E tests (2 new cases, 17 total): - revoke client via admin API invalidates all tokens (mint → use → revoke → verify rejected → mint fails) - revoke API key via admin API (create → use at /mcp → revoke → verify rejected) 52 tests, 0 failures, 213 assertions across unit + e2e. * fix(test): e2e tests clean up after themselves — no more orphan clients Problem: every test run left e2e-oauth-test, e2e-revoke-test, and e2e-revoke-key-test rows in oauth_clients and access_tokens. The CLI-based cleanup in afterAll was failing silently. Fix: - beforeAll: SQL DELETE of any e2e-* orphans from previous crashed runs - afterAll: direct SQL cleanup of oauth_tokens, oauth_clients, access_tokens, mcp_request_log — all rows matching 'e2e-%' pattern - No reliance on CLI commands for cleanup (they fail silently) Verified: 52 tests pass, 0 test rows remain after run. * feat(admin): hide revoked toggle on Agents page * fix(admin): styled error page for expired magic links Matches the login page aesthetic instead of plain text. Dark theme, GBrain logo, explains the link expired, tells user to ask their agent. * fix(admin): clean config export — auth-type-aware Claude Code instructions * fix(admin): rewrite all config exports — command language, auth-type-aware, verified syntax * fix(admin): API key rows clickable with revoke + sync all fixes from master Syncs all accumulated fixes onto the PR branch: - API key rows in agents table now open drawer with Revoke button - API keys show bearer token usage hint instead of config export tabs - Config export snippets use command language directed at the AI agent - Styled expired magic link error page - Hide revoked toggle - Test cleanup via direct SQL - All v0.26.2 upstream fixes incorporated * fix(oauth): port coerceTimestamp helper from master 1055e10 Tests in test/oauth.test.ts (already on this branch) import coerceTimestamp from oauth-provider.ts. The import was synced from master via PR commit 16 ("sync all fixes from master") but the production-code change to oauth-provider.ts was not. Result: bun test fails at module load with "coerceTimestamp is not exported". This commit ports the helper directly instead of merging master, avoiding VERSION/CHANGELOG/dist conflicts. Boundary helper for postgres.js BIGINT-as-string (auto-detected on Supabase pgbouncer / port 6543). Throws on non-finite so corrupt rows fail loud at the SELECT-row -> JS-number boundary. Returns undefined for SQL NULL; comparison sites treat NULL as expired (fail-closed). Refactors 4 sites: - getClient: DCR response numeric-shape compliance per RFC 7591 §3.2.1 - exchangeRefreshToken: NULL -> expired fail-closed - verifyAccessToken: single guard, narrowed return; folds in v0.26.1's inline Number(...) at the return site Originally landed on master as part of #593 (v0.26.2). Ported here so PR #586 (v0.26.3) can build standalone without a master merge. * feat(schema): migration v33 — admin dashboard columns Adds the 5 columns + new index referenced by PR #586 admin dashboard work that landed without a corresponding schema migration: oauth_clients.token_ttl INTEGER -- per-client OAuth TTL override oauth_clients.deleted_at TIMESTAMPTZ -- soft-delete for revoke mcp_request_log.agent_name TEXT -- resolved client_name for log mcp_request_log.params JSONB -- captured request params mcp_request_log.error_message TEXT -- captured error text on failure idx_mcp_log_agent_time INDEX -- supports new agent filter Without v33 on existing brains: - /admin/api/agents 503s (SELECT references token_ttl + deleted_at) - POST /admin/api/revoke-client throws 500 (UPDATE deleted_at) - POST /admin/api/update-client-ttl throws 500 (UPDATE token_ttl) - mcp_request_log INSERTs silently swallow column-doesn't-exist errors, request log appears empty to the operator All ALTERs use ADD COLUMN IF NOT EXISTS so re-running the migration is a no-op on a brain that already has v33. Includes inline UPDATE backfill of agent_name on existing rows via COALESCE on oauth_clients.client_name → access_tokens.name → token_name. Updates: - src/core/migrate.ts: v33 migration entry - src/schema.sql: source-of-truth schema for fresh installs - src/core/pglite-schema.ts: PGLite mirror - src/core/schema-embedded.ts: regenerated via bun run build:schema - test/migrate.test.ts: 5 SQL-shape assertions pinning the v33 contract * refactor(serve-http): parameterize request-log filter; kill dead vars Three issues in the prior /admin/api/requests handler: 1. sql.unsafe() with manual single-quote escape on user input: conditions.push(`token_name = '${agent.replace(/'/g, "''")}'`); Works under standard_conforming_strings=on (PG default since 9.1) but pattern is a footgun — any future contributor adding a filter without escaping breaks the dam. Backslashes are not escaped. Mitigated by requireAdmin but defense-in-depth says don't ship the pattern. 2. Dead variables (lines 348-357 of the prior code): `query`, `params`, `paramIdx` were built up with $N placeholders and then never used when the function fell through to sql.unsafe with manually-escaped strings. Confusing leftovers from an earlier parameterization attempt. 3. Unused `values: unknown[] = []` in the conditions block. Fix: replace the entire dynamic-WHERE construction with postgres.js tagged-template fragments. Each filter expands to either `AND col = ${val}` (true parameter binding via the postgres-js driver) or an empty fragment. `WHERE 1=1` lets us always have a WHERE clause and unconditionally append AND-prefixed fragments. No string interpolation, no manual escaping, no sql.unsafe. Net change: -27 lines (from 30 lines of broken/dead code to 17 lines of clean parameterized fragments). * perf(oauth): thread client_name through AuthInfo; drop per-request lookup PR #586's serve-http.ts /mcp handler did one extra DB roundtrip per authenticated request to resolve client_id → client_name for logging: let agentName = authInfo.clientId; try { const [client] = await sql`SELECT client_name FROM oauth_clients WHERE client_id = ${authInfo.clientId}`; if (client) agentName = client.client_name; } catch { /* best effort */ } On a busy brain (Perplexity Computer doing inline research, Claude Code searching) that is ~50–100ms extra per /mcp request — wasted on a static lookup that doesn't change between requests. Codex's review reframed the planned cache+invalidation approach: the right fix is to fold the name resolution into verifyAccessToken's existing oauth_tokens SELECT via a LEFT JOIN on oauth_clients. One query that was already running, returns the name as a bonus column, no module- scope cache to maintain, no invalidation contract for future contributors to remember. Changes: - AuthInfo (src/core/operations.ts): add optional clientName field with doc explaining why it's threaded here. - verifyAccessToken (src/core/oauth-provider.ts): SELECT becomes SELECT t.client_id, t.scopes, t.expires_at, t.resource, c.client_name FROM oauth_tokens t LEFT JOIN oauth_clients c ON c.client_id = t.client_id WHERE t.token_hash = ${tokenHash} AND t.token_type = 'access' Returns clientName in AuthInfo. - Legacy access_tokens path: clientName = name (single identifier). - serve-http.ts /mcp handler: read authInfo.clientName directly, fall back to clientId. Per-request lookup removed. Net change: -8 LOC. Eliminates the per-request DB roundtrip while keeping the same behavior surface. * security(serve-http): timingSafeEqual on admin token hash compare Both /admin/login (POST, JSON body) and /admin/auth/:token (GET, magic link) compared the sha256 of the operator-supplied token against the known bootstrapHash via JS string `===`, which short-circuits at the first mismatched character. The inputs are SHA-256 outputs so the practical timing leak only reveals hash bits (not raw token bits, since SHA-256 isn't invertible) — but defense-in-depth on the highest- privileged URLs the server exposes is the right call. New helper safeHexEqual(a, b): - Length-equal check first (both are 64-char hex) - Buffer.from(hex, 'hex') decodes each side to 32 bytes - crypto.timingSafeEqual returns the constant-time compare result Also tightens the POST handler's input validation: requires token to be a string before passing to createHash (prior code only checked truthiness, would have crashed on object-typed bodies even with express.json's parser). Used at both magic-link and password-style admin auth sites. * security(serve-http): rate-limit /admin/auth/:token at 10/min/IP Defense-in-depth on the magic-link endpoint. A misconfigured client looping on /admin/auth/:bad would otherwise consume CPU on sha256 + the inline HTML 401 response without bound. Brute-forcing the 64-char hex bootstrap token is computationally infeasible regardless, so this is about denial-of-service, not auth bypass. Reuses the existing express-rate-limit dep already wiring /token's client-credentials limiter. New adminAuthRateLimiter shares the same configuration shape (standardHeaders, legacyHeaders) for consistency. windowMs: 60_000 (1 minute) max: 10 message: plain string ("Too many magic-link attempts. Wait a minute before trying again.") instead of JSON envelope, matching the endpoint's HTML response style. * security(admin): kill JS-state token; single-use magic links; sign out everywhere Resolves D11 + D12 from the codex-pushback review. Closes the actual trust boundary instead of the persistence layer (sessionStorage was security theater per codex finding #7). # Single-use magic links (D11=C) The bootstrap token is no longer the magic-link path component. New flow: agent has bootstrap token (read from server stderr) -> POST /admin/api/issue-magic-link Authorization: Bearer <bootstrap> -> server returns one-time nonce URL -> operator clicks /admin/auth/<nonce> -> server consumes nonce, sets cookie, redirects to dashboard Server state (in-memory): - magicLinkNonces: Map<nonce, expiresAt> (5-minute TTL) - consumedNonces: Set<nonce> (LRU cap 1000 to bound memory) - pruneExpiredNonces() best-effort GC on each issue/redeem Each redemption marks the nonce consumed. Second click on the same URL gets the styled 401 page. Leaked URL grants exactly one extra session before dying. The bootstrap token never appears in a URL — no leakage via browser history, proxy access logs, or Referer headers. # Kill JS-state bootstrap token (D12=B) admin/src/pages/Login.tsx + admin/src/api.ts: - All localStorage reads/writes removed - Auto-reauth-via-saved-token logic deleted - Token only lives in form state during submit, cleared after - 401 redirects straight to login — no cache to retry against The HttpOnly cookie is the only session credential after successful authentication. Closing the tab ends the session. Reopening shows the login page. Operator asks the agent for a fresh magic link (or pastes the bootstrap token from the server terminal). # Sign out everywhere POST /admin/api/sign-out-everywhere (admin-cookie-required) calls adminSessions.clear() and returns {revoked_sessions: count}. Every browser/tab fails its next request, gets 401, redirects to login. Bootstrap token unaffected — still valid for new magic-link mints. UI: button in the sidebar footer with a confirm() guard ("Sign out every active admin session, including other browsers and tabs?"). # Notes admin/dist is gitignored on this branch (master's v0.26.2 removed that line; the merge to master will reconcile). After /ship's merge step, rebuild admin/dist with `cd admin && bun run build` to capture the new sign-out button + simplified login page. * fix(admin): rename loadApiKeys() to loadAgents() in Agents.tsx onCreated The Create API Key flow's onCreated callback called loadApiKeys() but no such function exists in this file. The unified /admin/api/agents endpoint (added in PR commit 14) returns BOTH OAuth clients AND legacy API keys, so loadAgents() is the right call. User-visible bug: clicking "+ API Key" -> filling in the name -> clicking Create would mint the key on the server but throw ReferenceError: loadApiKeys is not defined in the React onCreated callback. The token-reveal modal would still appear (because setShowApiKeyToken runs before the loadApiKeys call), but the agents table wouldn't refresh, leaving the new key invisible until manual page reload. Five Claude review passes missed this. Codex caught it in one pass. 1-line fix. * fix(admin): empty-state placeholder when filtered Agents result is empty Pre-fix: the empty-state guard checked the unfiltered agents array. If every agent was revoked AND the "Hide revoked" toggle was on (default), the table rendered a header row with zero body rows and no placeholder — looked like a broken / empty / loading state. Two cases to render distinctly: 1. agents.length === 0 (truly no agents) "No agents registered. Register your first agent to get started." 2. visibleAgents.length === 0 BUT agents.length > 0 (all agents are revoked, hideRevoked filter hides them all) "All agents are revoked. Uncheck "Hide revoked" to view them." Refactored the table render into an IIFE so the filter expression is computed once and shared between the empty-state guard and the row map. Drops the prior inline `agents.filter(...).map(...)` pattern. (F2.2 from the eng review pass #2.) * fix(admin): restore Claude Code + Cursor tabs for API-key agents Wintermute's commit 16 (3d5d0f8) wrapped the entire Config Export section in {isOAuth && (...)}, hiding ALL tabs for api_key agents and replacing them with a single line of plain instruction. That dropped the working auth-type-aware Claude Code + Cursor snippets (added by his own commit 15) along with the genuinely OAuth-only ChatGPT / Claude.ai / Perplexity ones. Codex review pass D5 settled on option C: per-tab branching. Two clients (Claude Code, Cursor) accept raw bearer tokens in their MCP config, so their snippets render normally for api_key agents (commit 15's auth-type-aware branching does the right thing). Three clients (ChatGPT, Claude.ai, Perplexity) only speak OAuth 2.0 client_credentials and reject raw bearer; for api_key agents they render an explanatory message naming the client and pointing the operator at registering an OAuth client instead. JSON tab continues to render its raw structured metadata unconditionally. Layout: removed the `{isOAuth && (...)}` outer wrap; tab list now always visible. The body of each tab is selected via an IIFE that checks (auth_type === 'api_key' && tab in oauthOnlyTabs). Net change: +24 lines (the warning panel + IIFE branch logic). * feat(admin): read -s prompt OAuth Claude Code snippet + 2-step curl fallback Wintermute's commit 15 inlined client_secret into a long compound `claude mcp add --header "Authorization: Bearer $(curl -d '... client_secret=PASTE_HERE')"` line. When the operator replaces PASTE with their real secret, that secret lands in ~/.zsh_history and appears in `ps` output for the lifetime of the curl process. D13=C from the eng review: ship both shapes. Default (read -s prompt-based, ~17 lines): - read -rs prompts for the secret without echo, stores in $GBRAIN_CS scoped to the shell session - curl uses --data-urlencode "client_secret=$GBRAIN_CS" — variable substitution at exec time, so the secret enters the curl process's argv at the moment of the call, but the shell history records literally `--data-urlencode "client_secret=$GBRAIN_CS"`, not the value - unset GBRAIN_CS afterwards to scrub the env Fallback (2-step curl + paste, for shells without read -s): - one curl command to mint the token (PASTE_YOUR_CLIENT_SECRET_HERE in the body — secret hits history but in one short isolated line that's easy to scrub) - second `claude mcp add` command with PASTE_TOKEN_FROM_ABOVE — the bearer token, not the long-lived client secret - bash + zsh history-deletion hint at the bottom Both shapes preserve the agent-facing voice ("The user wants to connect GBrain MCP to your context. Here's how.") and the token-TTL rendering ("will last 30 days") that commit 15 added. Net change: +25 lines in the configSnippets['claude-code'] OAuth branch. API-key branch unchanged (single paste, no secret). * chore(ci): gate admin React build via scripts/check-admin-build.sh Codex review pass #6 finding #3 caught loadApiKeys() referenced but undefined in Agents.tsx — a real shipping bug that 5 Claude review passes missed. Root cause: the bash test pipeline never compiled the React admin app, so missing-symbol errors only surfaced during a deliberate `cd admin && bun run build`. This commit threads the admin build into the standard test gate. Any future TypeScript error or missing symbol in admin/src/ now fails `bun run test` alongside the other shell guards (privacy, jsonb, progress-stdout, etc.) and the typecheck step. Behavior: - scripts/check-admin-build.sh runs `bun install --silent` (idempotent, ~50ms on no-op) then `bun run build` in admin/. - Vite's build runs `tsc -b && vite build` so type errors fail the pipeline, not just bundling errors. - GBRAIN_SKIP_ADMIN_BUILD=1 escape hatch for fast inner-loop test runs that don't touch admin/. Production CI MUST NOT set this. - Skips silently if admin/ doesn't exist (handles slim-clone scenarios). Wired into both: - "test" script: full pipeline now includes admin build before bun test - "check:admin-build" script: invoke standalone for debugging * test(e2e): v0.26.3 coverage — column round-trip, injection probe, TTL, magic-link Folds together the planned fix-up commits #8-#11 since they all live in the same E2E file and share the spawned-server harness. Each test block is independently bisect-readable. # Test 1: mcp_request_log new column round-trip (pins migration v33) Wipes log rows for the e2e-oauth-test client, makes a successful tools/list call + a failed tools/call (nonexistent tool name), then asserts: - rows persisted (count >= 2) — proves the INSERT wasn't silently swallowed by the "best effort" try/catch on a column-doesn't-exist error - agent_name column resolves to 'e2e-oauth-test' on every row (proves the JOIN in verifyAccessToken or the v33 backfill path) - params column persisted as JSONB on tools/call - error_message column populated on the status='error' row Without migration v33, every assertion fails — the column doesn't exist so the INSERT throws, gets swallowed, and rows.length === 0. # Test 2: request-log filter injection probe Sends `?agent=alice'%20OR%201%3D1` to /admin/api/requests. Pre-fix, the sql.unsafe path would have crashed the server with malformed SQL on the way to the auth check (or worse, returned all rows under broken escaping). Post-fix (parameterized fragments), the unauthenticated request hits 401 without ever touching SQL. Asserts: - 401 (not 500) on the injection input - server still responsive on /health afterwards (didn't crash) # Test 3: per-client token_ttl flow Registers e2e-test-ttl, sets oauth_clients.token_ttl, mints a token, asserts response's expires_in matches. Cycles through three states: - token_ttl = 86400 → expires_in = 86400 (24h custom override) - token_ttl = 7200 → expires_in = 7200 (2h different custom) - token_ttl = NULL → expires_in = 3600 (server default fallback) Pins the per-client TTL feature added in PR #586 commit 6 (e7989e9). # Test 4: magic-link styled 401 page + single-use semantic (a) Invalid nonce returns Content-Type: text/html with a body that contains "expired" and "GBrain" — pins the styled error page from PR commit 13 (f8f5cfe). (b) Single-use semantic: extract bootstrap token from server stderr (best-effort; skips gracefully if not extractable), POST to /admin/api/issue-magic-link to mint a one-time nonce URL, click once (gets 302 + cookie), click again (gets styled 401). Pins the D11=C single-use rotation logic. # Test 5: agent_name resolution path Makes an OAuth request and asserts mcp_request_log.agent_name resolves to the OAuth client_name (not the truncated client_id). Pins the JOIN introduced in fix-up #4 + the v33 backfill path. # Test 6: register-client missing-name returns 400 (basic input validation) Hits /admin/api/register-client without auth — must 401 (not crash 500). # Other changes - Renamed describe header from `(v0.26.1 + v0.26.2)` to `(v0.26.1 + v0.26.2 + v0.26.3)` — F6.5. - All postgres.js sql tag bindings on `clientId` / `clientSecret` use the `!` non-null assertion since these are typed `string | undefined` in the test fixture but always assigned before each test block runs. - Result casts go through `as unknown as ...` per postgres.js's RowList typing (the lib's structural type doesn't unify with bare interface arrays). * chore: privacy sweep + integrity.ts on getconnection allow-list Two pre-existing CI failures uncovered while running `bun run test` on this branch — unrelated to v0.26.3 substance but blocking the pipeline. # Privacy sweep (src/core/mounts-cache.ts) Two references to the private agent fork name in code comments, violating CLAUDE.md privacy rule ("never reference real people, companies, funds, or private agent names in any public-facing artifact"). Both authored in v0.26.0 commit 3c032d7. - line 6 (docblock): "Host agents (Wintermute / OpenClaw / any Claude Code install) read" -> "Host agents (your OpenClaw / any Claude Code install) read" - line 324 (RESOLVER preamble emitter): "Host agents (Wintermute/OpenClaw/Claude Code) should prefer this file over" -> "Host agents (your OpenClaw / Claude Code) should prefer this file over" Per the documented substitution: "your OpenClaw" for reader-facing copy covers any downstream OpenClaw deployment (Wintermute, Hermes, AlphaClaw, etc.) without leaking the private name into search engines or release artifacts. # integrity.ts on the getconnection allow-list `scripts/check-no-legacy-getconnection.sh` flags `db.getConnection()` calls outside `src/core/db.ts` to enforce the multi-brain routing contract. `src/commands/integrity.ts:355` (scanIntegrityBatch) was introduced in v0.22.16 commit 8468ba2 — the check ran clean at the time because the file wasn't on the allow-list yet, but PR #586's test pipeline catches it. Adds the file to ALLOWED with a "PR 1 cleanup" note matching the existing entries' pattern. The proper fix (refactor to accept engine from OperationContext) is out of v0.26.3 scope and tracked alongside the other PR 1 entries. * chore: bump v0.26.2 -> v0.26.3 + CHANGELOG VERSION + package.json already at 0.26.3 from the initial bump on this branch (see commit history). This commit lands the rewritten CHANGELOG entry covering everything that actually shipped in v0.26.3 — well past the original "legacy API keys" framing. What lands in v0.26.3: # Headline (admin trust model) Bootstrap token never persists in browser JS state (no localStorage, no sessionStorage). Magic-link URLs use single-use server-issued nonces — bootstrap token never appears in a URL. Cookie sessions are HttpOnly + SameSite=Strict. "Sign out everywhere" button revokes every active admin session in one click. # Schema Migration v33 adds 5 columns referenced by PR #586's admin-dashboard work that landed without a corresponding migration. Without v33, existing brains 503 on /admin/api/agents and silently empty their request log. Backfill of agent_name from oauth_clients.client_name -> access_tokens.name -> token_name baked into the migration. # Performance verifyAccessToken JOINs oauth_clients in its existing token SELECT and returns clientName on AuthInfo. Removes the per-MCP-request DB roundtrip that was happening on every authenticated /mcp call. # Security - crypto.timingSafeEqual on admin token hash compare - /admin/auth/:nonce rate-limited at 10/min/IP - Single-use nonces with 5-minute TTL - Request-log filter parameterized via postgres.js tagged-template fragments (sql.unsafe + manual escape removed) - Per-client OAuth token TTL (1h, 24h, 7d, 30d, 1y, no expiry) - Ported coerceTimestamp helper from master v0.26.2 (BIGINT-as-string fix) # UI - API keys + OAuth clients in one unified Agents table - Auth-type-aware Config Export tabs - Claude Code OAuth: read -s prompt-based snippet (default) + 2-step curl fallback (D13=C) - Cursor: OAuth discovery URL OR raw bearer based on auth type - ChatGPT/Cowork/Perplexity: "OAuth client required" CTA on api_key agents - Hide-revoked toggle + empty-state placeholder for filtered-empty - Bug fix: loadApiKeys -> loadAgents (codex caught what 5 review passes missed; Create-API-Key flow was broken) # Tests + CI - New E2E coverage: column round-trip, injection probe, per-client TTL, magic-link single-use, styled 401, agent_name resolution - Admin React build is now a CI gate (catches missing-symbol bugs before E2E) - check-no-legacy-getconnection allowlist updated for integrity.ts Branch shape: 16 author commits + 13 fix-up commits = 29 commits on PR. Commit-by-commit bisect-friendly. Plan + codex review pass artifacts at ~/.claude/plans/check-this-out-and-breezy-forest.md. --------- Co-authored-by: Wintermute <wintermute@garrytan.com> Co-authored-by: Garry Tan <garrytan@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
v0.26.2 closes the BIGINT-as-string OAuth bug class found in PR #577 (v0.26.1) audit, lands
gbrain auth revoke-clientas a real CLI subcommand, and fixes a load-bearing test infrastructure bug (bun'sexecSyncdoesn't inheritprocess.envmutations) that was masking E2E failures across the suite.OAuth bug class fix (5 sites + boundary helper):
coerceTimestamp()insrc/core/oauth-provider.ts. Throws on non-finite (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through token validation asexpiresAt: NaN. Returns undefined for SQL NULL so callers decide NULL semantics explicitly.getClient(L112, L113 — DCR response RFC 7591 §3.2.1 numeric compliance),exchangeRefreshToken(L274),verifyAccessToken(L296, L303). NULLexpires_atonoauth_tokensis now treated as expired (fail-closed).Number(row.expires_at)at L303 producedNaNfor corrupt rows;NaN < nowisfalse, so the expired-token branch was skipped and a fake-valid token flowed to the SDK. v0.26.2 fixes this.New CLI subcommand:
gbrain auth revoke-client <client_id>— atomicDELETE...RETURNINGonoauth_clients. Schema-level FK CASCADE onoauth_tokens.client_idandoauth_codes.client_idpurges all dependent rows in the same transaction. Exit 1 on no-such-client (idempotent).Build + infrastructure:
admin/dist/removed from.gitignoreand committed (220 KB) to match CLAUDE.md's "self-contained binaries" claim. Thebun --compilepath embeds it viaimport path from '...' with { type: 'file' }; without committed artifacts, fresh clones couldn't build the binary.execSyncenv-inheritance fix intest/e2e/serve-http-oauth.test.ts(4 sites). bun does NOT inherit env mutations done viaprocess.env.X = ...— only OS-level env from before bun started. helpers.ts loads.env.testingand setsDATABASE_URLviaprocess.envmutation, invisible to subprocesses unlessenv: { ...process.env }is passed explicitly. Documented inline as a reference for the 22 sibling failing E2Es in TODOS.md.bun test 2>&1 | tail -10masks bun's exit code and drops failure details. The pipe form silently breaks/shipStep T1 — bit us during this very ship and forced a 23-minute re-run to triage.v0.26.1 catch-up:
PR #577 shipped its three fixes (token validation, OAuth metadata, Express 5 compat) without bumping VERSION/package.json/CHANGELOG. v0.26.2 adds a retroactive v0.26.1 entry above v0.26.0 so the changelog matches commit history.
Test Coverage
Pre-Landing Review
Cleared during plan-eng-review (D1 — scope expanded to all 5 sites + helper). Codex consult (D2) found 5 factual errors and 7 design gaps the in-house review had cleared:
oauth_tokens.expires_atis NULLABLE — no!non-null assertions warranted in v0.26.2.prepare: falseis selective viaresolvePrepare(), not universal.Number('foo')returns NaN which slips through as fail-OPEN (not fail-closed) — helper now throws on non-finite.All addressed in D2. Plan:
~/.claude/plans/system-instruction-you-are-working-dynamic-frog.md.Plan Completion
All 9 plan tasks completed:
coerceTimestamphelper + 5 call sites refactoredrevoke-clientsubcommand + router + help/registerHTTP-level E2E testTODOS
Added (P0):
test infra (v0.26.2 follow-up — pre-existing failures triage)— 22 pre-existing test failures unrelated to v0.26.2's OAuth diff (12 sync pipeline cases, 3 multi-source cases, sync-parallel/skip-failed/doctor/dream/cycle/claw-test/BrainRegistry). Likely root cause for several: same bunexecSyncenv-inheritance pattern fixed here. Documented as a reference for the next maintainer.Documentation
Documentation synced post-push (commit 52eab3a):
src/core/oauth-provider.ts(coerceTimestampboundary helper at 5 call sites, throw-on-NaN posture, NULL-as-expired contract, intentionally module-private),src/commands/auth.ts(newrevoke-clientsubcommand with FK CASCADE cleanup), andtest/oauth.test.ts. Added a new entry fortest/e2e/serve-http-oauth.test.tscovering v0.26.0 base + v0.26.2 expansion (real DCR/registerHTTP-level shape test, CLI subprocessrevoke-clienttest, bunexecSyncenv-inheritance fix as reference).gbrain auth revoke-client <client_id>to the CLI command list underregister-client.CHANGELOG.md, TODOS.md, VERSION, and package.json were already updated by ship commits and not re-touched.
Test plan
bun run typecheck)bun test test/oauth.test.ts test/build-llms.test.ts test/e2e/serve-http-oauth.test.ts)unset DATABASE_URL && bun test test/e2e/serve-http-oauth.test.tsworks because helpers.ts loads .env.testing → process.env, andenv: { ...process.env }propagates to subprocesses)oauth_clientspost-run (cleanup actually works now)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need.