mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
v0.26.9 fix(oauth): RFC 6749 hardening + close HTTP MCP shell-job RCE (#628)
* fix(mcp): close HTTP MCP shell-job RCE + tighten remote contract
The HTTP MCP transport in serve-http.ts inlined its own OperationContext
literal and forgot to set `remote: true`. With the field undefined at the
operations.ts protected-job-name guard (line 1391), an HTTP MCP caller
holding a write-scoped OAuth token could submit `submit_job {name: "shell"}`
and execute arbitrary commands on the gbrain host (RCE-class).
Two-layer fix:
1. F7 — explicit `remote: true` on the inlined /mcp OperationContext.
Stdio MCP at src/mcp/dispatch.ts:61 already set this; the HTTP path
was the regression.
2. F7b — fail-closed contract on the four ctx.remote consumer sites in
operations.ts (auto-link skip, telemetry x2, protected-job guard).
The protected-job guard flips from `if (ctx.remote && ...)` to
`if (ctx.remote !== false && ...)` and the trusted-marker site flips
from `!ctx.remote && ...` to `ctx.remote === false && ...`. Anything
that isn't strictly `false` now treats the caller as remote/untrusted.
3. D12 — `OperationContext.remote` becomes REQUIRED in the TypeScript
type. The compiler now catches future transports that forget the field.
The runtime fail-closed defaults are belt+suspenders for any caller
that bypasses the type via `as` cast or `Partial<>` spread.
Tests:
- New `test/trust-boundary-contract.test.ts` (4 cases) pins the
fail-closed semantics: undefined-via-cast rejects, remote=true rejects,
remote=false allowed (only path that escalates protected-name jobs).
- `test/e2e/serve-http-oauth.test.ts` adds 2 cases asserting HTTP MCP
cannot submit `shell` or `subagent` jobs even with read+write scope.
- `test/e2e/graph-quality.test.ts` adds the now-required `remote: false`
to its fixture (e2e graph quality simulates local-CLI writes).
Verification: bun test -> 3742 pass / 0 fail. typecheck clean.
Thanks to @ElectricSheepIO on X for the security review that surfaced
this trust-boundary regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(oauth): RFC 6749 hardening + serve-http defense in depth
OAuth provider hardening pass that brings the provider into RFC compliance
on auth code, refresh token, and revocation flows, and tightens the
serve-http surface around request logging and admin cookies.
Provider (src/core/oauth-provider.ts):
- F1: bind client_id atomically into the auth code DELETE WHERE clause for
exchangeAuthorizationCode + challengeForAuthorizationCode. Previous
pattern (DELETE...RETURNING then post-hoc client compare) burned codes
on the wrong-client path so the legitimate client could not retry.
RFC 6749 §10.5.
- F2: same atomic predicate on exchangeRefreshToken. The pre-fix shape
defeated RFC 6749 §10.4's stolen-token detection by letting attacker +
victim both succeed.
- F3: refresh token rejects requested scopes that are not a subset of the
ORIGINAL grant on the row. Codex C9: subset is checked against the
recorded grant, not the client's currently-allowed scopes (which can
expand later); omitted scope inherits the original verbatim and stays
distinct from explicit-empty. RFC 6749 §6.
- F4: revokeToken adds AND client_id to the DELETE so a client cannot
revoke another client's tokens by guessing the hash. RFC 7009 §2.1.
- F5: deleted_at and token_ttl column probes use a new
isUndefinedColumnError helper (extracted to src/core/utils.ts per D14)
that matches SQLSTATE 42703 or column-name-in-message. Bare catch{}
used to swallow lock timeouts, network blips, and auth failures as
"column missing" — fail-open posture in a security path.
- F6: sweepExpiredTokens uses RETURNING 1 + array length. Pre-fix
(result as any).count returned 0 on at least one engine even when
rows were deleted, and codes were never counted.
- F7c: NEW finding eva-brain missed. exchangeAuthorizationCode now folds
redirect_uri into the atomic DELETE predicate when the parameter is
provided. Stored on /authorize, never compared on /token before this
commit. RFC 6749 §4.1.3 violation. Back-compat: when caller omits the
parameter the predicate is skipped, preserving SDK consumers that
haven't adopted the parameter yet.
- F12 (cleanup, not security): dcrDisabled constructor option replaces
the prior monkey-patch of _clientsStore in serve-http.ts. The SDK's
mcpAuthRouter only wires up /register when the store exposes
registerClient, so omitting the method via the constructor is
sufficient. Reframed as cleanup per codex C10 — the monkey-patch
happened before mcpAuthRouter ran, so the prior shape did not have
a real security regression to claim.
Dispatch (src/mcp/dispatch.ts):
- F8: new summarizeMcpParams(opName, params) intersects submitted keys
against the operation's declared params allow-list. Returns
{redacted, kind, declared_keys, unknown_key_count, approx_bytes}.
Closes the codex C8 leak: a naive "dump all submitted keys" summary
still echoed attacker-controlled key names like
put_page {"wiki/people/sensitive_name": "..."} into mcp_request_log
+ the SSE feed. Allow-list pattern keeps debug visibility on declared
keys while counting unknowns without naming them.
Serve-http (src/commands/serve-http.ts) + serve (src/commands/serve.ts):
- F8 wiring: mcp_request_log + SSE broadcast routed through
summarizeMcpParams by default. New --log-full-params flag bypasses
redaction with a loud stderr warning at startup. Default privacy-
positive; flag is the documented escape hatch for self-hosted
operators debugging on their own laptop.
- F9: admin cookies set Secure when req.secure OR issuerUrl.protocol
is https. Cloudflare-tunnel + reverse-proxy deployments where the
inside-tunnel hop looks like http but the public URL is https now
tag cookies correctly.
- F10: bound magicLinkNonces with NONCE_LRU_CAP. Previously only the
consumed-nonces map was capped; an attacker (or misbehaving agent)
with the bootstrap token could mint nonces faster than they expired
and grow the live store unbounded.
- F12: dcrDisabled flows through to the provider constructor instead of
monkey-patching _clientsStore after construction.
- F14: try/catch wraps StreamableHTTPServerTransport setup +
handleRequest. SDK-level throws no longer fall through to express's
default HTML error page; clients expecting JSON-RPC envelopes get a
JSON 500 instead.
- F15: error envelope unified via buildError + serializeError from
src/core/errors.ts. OperationError and unexpected exceptions both
emit the same {class, code, message, hint} shape so clients can
pattern-match a single envelope.
Tests:
- test/oauth.test.ts adds 11 cases:
* F1+F2 wrong-client cannot consume / read PKCE / burn refresh,
paired with owner-still-redeems atomically afterward (codex D6 —
proves the predicate doesn't burn the row on attacker attempts).
* F3 refresh scope subset enforced.
* F4 wrong-client cannot revoke.
* F5 non-schema SQL not swallowed by client_credentials soft-delete probe.
* F6 sweepExpiredTokens returns count > 0 after deleting rows.
* F7c redirect_uri match succeeds, mismatch rejects, omitted preserves
back-compat for callers that don't pass the parameter.
* F12 dcrDisabled constructor option exposes only getClient,
registerClientManual still works.
- test/mcp-dispatch-summarize.test.ts (NEW, 6 cases): pins the F8
privacy invariants. The codex-C8 attacker-key-name probe asserts that
a sensitive name submitted as a key never appears anywhere in the
redactor's output.
Verification: bun run typecheck clean. test/oauth.test.ts 55/55,
test/mcp-dispatch-summarize.test.ts 6/6,
test/trust-boundary-contract.test.ts 4/4 from commit A. The one
unrelated unit failure surfaces on master too — environment-sensitive
test that expects ~/.gbrain/config.json to be absent in the test env.
Out of scope: F11 (auth register-client --redirect-uri flag) and F13
(serve --http argv positive-int validator) per codex C11 — operator
UX gaps, not trust-boundary fixes. Filed as follow-up TODOs.
Thanks to @ElectricSheepIO on X for the security review that surfaced
this hardening pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: file F11 + F13 as OAuth hardening follow-up TODOs
Codex C11 flagged these as scope creep on the v0.26.7 OAuth hardening
PR (operator UX, not trust-boundary). Capturing them here so the
context survives — eva-brain has both implementations and the lift is
mechanical when we want to do them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(oauth): close adversarial-review findings on F7c + F8
Two bugs surfaced by an adversarial subagent during /ship's pre-landing
review pass that the codex + plan-eng-review didn't catch.
D15 / F7c: `exchangeAuthorizationCode` used `redirectUri ? ...` ternary
to choose the with-redirect vs no-redirect SQL. Empty string fell
through to the no-redirect branch, so a caller submitting
`redirect_uri=""` at /token bypassed the binding entirely. RFC 6749
§4.1.3 spec violation. Switch to `redirectUri !== undefined`. Test:
empty-string redirect_uri must reject when /authorize stored a real URI.
D16 / F8: `summarizeMcpParams` published exact byte length via
`approx_bytes = JSON.stringify(params).length`. Submitting put_page with
a known prefix and observing the resulting log entry across repeated
probes lets an attacker binary-search the size of secret suffix content.
Bucket to 1KB resolution. The redacted summary keeps a coarse
"roughly how big" signal for operators while making size-based
side-channel attacks useless.
Test count: 65 → 67 across the three new test files.
Typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: bump version and changelog (v0.26.9)
OAuth 2.1 hardening + HTTP MCP shell-job RCE fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: update project documentation for v0.26.9
Annotate CLAUDE.md key-files entries with v0.26.9 OAuth/MCP hardening pass:
- src/core/operations.ts: D12 (OperationContext.remote required) + F7b
(4-site fail-closed flip), HTTP MCP shell-job RCE close
- src/core/utils.ts: D14 isUndefinedColumnError extracted helper
- src/mcp/dispatch.ts: F8 summarizeMcpParams privacy redactor with
declared-keys allow-list + 1KB byte bucketing
- src/commands/serve-http.ts: F7+F8+F9+F10+F12+F14+F15 hardening
- src/core/oauth-provider.ts: F1+F2+F3+F4+F5+F6+F7c+F12 RFC 6749/7009
hardening pass
Add new test-file entries for test/mcp-dispatch-summarize.test.ts
(7 cases) and test/trust-boundary-contract.test.ts (4 cases). Extend
test/oauth.test.ts (+14 cases) and test/e2e/serve-http-oauth.test.ts
(+2 RCE-close regressions) entries with v0.26.9 case counts.
README.md: added --log-full-params to gbrain serve --http surface.
SECURITY.md: documented mcp_request_log.params redaction default
({redacted, kind, declared_keys, unknown_key_count, approx_bytes}) +
--log-full-params opt-in.
docs/mcp/DEPLOY.md: operator-facing note on SSE feed + audit log
redaction default and when to flip --log-full-params on.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
9c2dc4cd54
commit
cb02932388
@@ -2,6 +2,57 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.26.9] - 2026-05-04
|
||||
|
||||
## **OAuth 2.1 hardening + closes an HTTP MCP shell-job RCE.**
|
||||
## **Anyone running `gbrain serve --http` should upgrade. The same write-scoped token your agent uses to call `put_page` could submit `shell` jobs and execute commands on the gbrain host.**
|
||||
|
||||
The HTTP MCP transport's request-handler context literal was missing one field — `remote: true` — and that one field was the difference between a trusted local CLI write and an untrusted agent-facing write. With it absent, `operations.ts:1391`'s protected-job-name guard saw a falsy undefined and skipped. An HTTP MCP caller with a `read+write` OAuth token could submit `submit_job {name: "shell", params: {cmd: "id"}}` and own the host. Stdio MCP set the field correctly via `src/mcp/dispatch.ts:61`; HTTP inlined a parallel context-builder for several releases and lost it.
|
||||
|
||||
Fix is two-layered. **F7** (`serve-http.ts`) sets `remote: true` explicitly. **F7b** flips the four trust-boundary call sites in `operations.ts` from falsy-default to `ctx.remote !== false` — anything that isn't strictly `false` now treats the caller as remote/untrusted, so a future transport that forgets the field fails closed instead of fails open. **D12** makes `OperationContext.remote` REQUIRED in the TypeScript type so the compiler is the first line of defense; the runtime fail-closed defaults are belt+suspenders for `as` casts and `Partial<>` spreads.
|
||||
|
||||
The OAuth provider in `src/core/oauth-provider.ts` got a parallel hardening pass on the same release. **F1+F2** fold `client_id` atomically into the auth-code and refresh-token DELETE WHERE clauses (RFC 6749 §10.5 + §10.4 — pre-fix the post-hoc client compare burned the row on wrong-client paths so the legitimate client couldn't retry). **F3** enforces the refresh-scope-subset rule against the original grant on the row, not the client's currently-allowed scopes (RFC 6749 §6). **F4** binds `client_id` on `revokeToken` so a client can only revoke its own tokens (RFC 7009 §2.1). **F7c** validates `redirect_uri` on `/token` against the value stored at `/authorize` (RFC 6749 §4.1.3 — eva-brain missed this; codex caught it). **F5** swaps bare `catch {}` for SQLSTATE 42703 column-existence probes so lock timeouts and network blips no longer ride through as "column missing." **F6** fixes `sweepExpiredTokens` to actually return a count.
|
||||
|
||||
`mcp_request_log` and the admin SSE feed now redact request payloads by default. The new `summarizeMcpParams` helper at `src/mcp/dispatch.ts` intersects submitted keys against the operation's declared `params` allow-list — declared keys preserve for debug visibility, unknown keys are counted but never named (closes the attacker-controlled-key-name leak surface). Byte counts get bucketed to 1KB so attackers can't binary-search secret-content sizes via repeated probes. Operators on their own laptops who want full payload visibility can flip on `--log-full-params` with a loud stderr warning.
|
||||
|
||||
Smaller hardening: admin cookies set `Secure` when behind HTTPS or a public-URL proxy (F9), magic-link nonces are bounded by an LRU cap (F10), `/mcp` wraps `transport.handleRequest` in try/catch so SDK throws hit a JSON-RPC 500 instead of express's default HTML error page (F14), and OperationError + unexpected exceptions both route through the unified `buildError`/`serializeError` envelope (F15). DCR disable became a constructor option on the provider rather than a serve-http monkey-patch (F12 — cleanup, not security).
|
||||
|
||||
To take advantage of v0.26.9
|
||||
============================
|
||||
|
||||
`gbrain upgrade` is a one-step upgrade. There is no migration; all changes are application-layer.
|
||||
|
||||
1. **Upgrade.** `gbrain upgrade`. Confirm `gbrain --version` shows `0.26.9`.
|
||||
|
||||
2. **If you run `gbrain serve --http`,** restart the process. The trust-boundary fix is in the request handler, so it takes effect on the next listen.
|
||||
|
||||
3. **If you run a multi-tenant deployment** (operators other than you have admin / register-client access), audit existing OAuth clients for unexpected redirect_uris with `gbrain auth list-clients` (when you wire the helper) or by inspecting `oauth_clients.redirect_uris` directly. The hardening doesn't touch existing clients; it only constrains future redemptions.
|
||||
|
||||
4. **If your dashboard or scripts read `mcp_request_log.params`,** note the schema shift. The default shape is now `{redacted: true, kind, declared_keys, unknown_key_count, approx_bytes}` (declared_keys is sorted; approx_bytes rounds up to nearest KB). Pass `--log-full-params` to `gbrain serve --http` to opt back into raw payloads with a startup warning.
|
||||
|
||||
5. **If anything breaks,** please file an issue: https://github.com/garrytan/gbrain/issues
|
||||
|
||||
Thanks to @ElectricSheepIO on X for the security review that surfaced this hardening pass.
|
||||
|
||||
### Itemized changes
|
||||
|
||||
- `src/core/oauth-provider.ts` — atomic client_id binding (F1, F2, F4), redirect_uri validation on exchange (F7c), refresh scope subset (F3), `isUndefinedColumnError` column probes (F5), `sweepExpiredTokens` correct count via `RETURNING 1` (F6), `dcrDisabled` constructor option (F12).
|
||||
- `src/core/operations.ts` — `OperationContext.remote` becomes required (D12). Four sites flipped to `ctx.remote !== false` / `=== false` for fail-closed semantics (F7b).
|
||||
- `src/commands/serve-http.ts` — explicit `remote: true` on /mcp context (F7), `summarizeMcpParams` wired into `mcp_request_log` + SSE feed by default (F8), `--log-full-params` opt-in, cookie `secure` flag (F9), bounded magic-link nonce LRU (F10), try/catch wrap on transport.handleRequest (F14), unified error envelope via `buildError`/`serializeError` (F15), DCR disable via provider constructor (F12).
|
||||
- `src/commands/serve.ts` — `--log-full-params` argv flag with stderr warning at startup.
|
||||
- `src/mcp/dispatch.ts` — new `summarizeMcpParams(opName, params)` helper. Returns `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}` with allow-list intersection and 1KB bucketing.
|
||||
- `src/core/utils.ts` — extracted `isUndefinedColumnError` predicate (D14, reusable).
|
||||
- `test/oauth.test.ts` — 14 new cases pinning F1/F2/F3/F4/F5/F6/F7c/F12 invariants, including the empty-string redirect_uri bypass test from the adversarial-review pass.
|
||||
- `test/mcp-dispatch-summarize.test.ts` — 7 cases pinning F8 redaction invariants including the attacker-key-name probe and the 1KB bucket assertion.
|
||||
- `test/trust-boundary-contract.test.ts` — 4 cases pinning F7b fail-closed semantics under cast bypass.
|
||||
- `test/e2e/serve-http-oauth.test.ts` — 2 new E2E regressions for shell-job and subagent-job submission rejection over HTTP MCP.
|
||||
- `test/e2e/graph-quality.test.ts` — adds explicit `remote: false` to the test fixture (D13 audit follow-through).
|
||||
|
||||
### For contributors
|
||||
|
||||
- `test/oauth.test.ts` now uses the F1/F4 cross-client isolation pattern: a wrong-client attempt must reject AND the rightful owner must still succeed atomically afterward. Apply the same shape when adding tests around the OAuth provider's predicate-bound DELETEs.
|
||||
- `summarizeMcpParams` is the canonical privacy-preserving redactor. New code paths that log MCP request shapes should route through it rather than inlining `JSON.stringify(params)`.
|
||||
|
||||
## [0.26.8] - 2026-05-04
|
||||
|
||||
## **Every gbrain brain becomes secure by default on upgrade. No public table without RLS, ever.**
|
||||
|
||||
@@ -40,13 +40,13 @@ strict behavior when unset.
|
||||
|
||||
## Key files
|
||||
|
||||
- `src/core/operations.ts` — Contract-first operation definitions (the foundation). Also exports upload validators: `validateUploadPath`, `validatePageSlug`, `validateFilename`, plus `matchesSlugAllowList(slug, prefixes)` (v0.23 glob matcher: `<prefix>/*` matches recursive children; bare `<prefix>` matches exact only). `OperationContext.remote` flags untrusted callers; `OperationContext.allowedSlugPrefixes` (v0.23) is the trusted-workspace allow-list set by the dream cycle. `put_page` enforces: when `viaSubagent` and `allowedSlugPrefixes` is set, slug must match the allow-list; else the legacy `wiki/agents/<id>/...` namespace check applies. Auto-link enabled for trusted-workspace writes (skipped only when `remote=true && !trustedWorkspace`). As of v0.26.0, every `Operation` also carries `scope?: 'read' | 'write' | 'admin'` + `localOnly?: boolean`. All ops are annotated; `sync_brain`, `file_upload`, `file_list`, and `file_url` are `admin + localOnly` (rejected over HTTP). `OperationContext.auth?: AuthInfo` is threaded through HTTP dispatch for scope enforcement in `serve-http.ts` before the op runs.
|
||||
- `src/core/operations.ts` — Contract-first operation definitions (the foundation). Also exports upload validators: `validateUploadPath`, `validatePageSlug`, `validateFilename`, plus `matchesSlugAllowList(slug, prefixes)` (v0.23 glob matcher: `<prefix>/*` matches recursive children; bare `<prefix>` matches exact only). `OperationContext.remote` flags untrusted callers; `OperationContext.allowedSlugPrefixes` (v0.23) is the trusted-workspace allow-list set by the dream cycle. `put_page` enforces: when `viaSubagent` and `allowedSlugPrefixes` is set, slug must match the allow-list; else the legacy `wiki/agents/<id>/...` namespace check applies. Auto-link enabled for trusted-workspace writes (skipped only when `remote=true && !trustedWorkspace`). As of v0.26.0, every `Operation` also carries `scope?: 'read' | 'write' | 'admin'` + `localOnly?: boolean`. All ops are annotated; `sync_brain`, `file_upload`, `file_list`, and `file_url` are `admin + localOnly` (rejected over HTTP). `OperationContext.auth?: AuthInfo` is threaded through HTTP dispatch for scope enforcement in `serve-http.ts` before the op runs. **v0.26.9 (D12 + F7b):** `OperationContext.remote` is now a REQUIRED field in the TypeScript type — the compiler is the first defense against transports that forget to set it. Four trust-boundary call sites (`put_page` allowlist, file_upload trust-narrowing, submit_job protected-name guard, auto-link skip) flipped from falsy-default (`!ctx.remote`) to fail-closed semantics (`ctx.remote === false` for "trusted-only" sites and `ctx.remote !== false` for "untrust unless explicit-false"). Anything that isn't strictly `false` is now treated as remote. Closed an HTTP MCP shell-job RCE: a `read+write`-scoped OAuth token could submit `shell` jobs because the HTTP request handler's literal context skipped `remote: true` and `submit_job`'s protected-name guard saw a falsy undefined. Stdio MCP set the field correctly via dispatch.ts; HTTP inlined a parallel context-builder for several releases and lost it.
|
||||
- `src/core/engine.ts` — Pluggable engine interface (BrainEngine). `clampSearchLimit(limit, default, cap)` takes an explicit cap so per-operation caps can be tighter than `MAX_SEARCH_LIMIT`. Exports `LinkBatchInput` / `TimelineBatchInput` for the v0.12.1 bulk-insert API (`addLinksBatch` / `addTimelineEntriesBatch`). As of v0.13.1, `BrainEngine` has a `readonly kind: 'postgres' | 'pglite'` discriminator so migrations (`src/core/migrate.ts`) and other consumers can branch on engine without `instanceof` + dynamic imports.
|
||||
- `src/core/engine-factory.ts` — Engine factory with dynamic imports (`'pglite'` | `'postgres'`)
|
||||
- `src/core/pglite-engine.ts` — PGLite (embedded Postgres 17.5 via WASM) implementation, all 40 BrainEngine methods. `addLinksBatch` / `addTimelineEntriesBatch` use multi-row `unnest()` with manual `$N` placeholders. As of v0.13.1, `connect()` wraps `PGlite.create()` in a try/catch that emits an actionable error naming the macOS 26.3 WASM bug (#223) and pointing at `gbrain doctor`; the lock is released on failure so the next process can retry cleanly. v0.22.0: `searchKeyword` and `searchKeywordChunks` multiply `ts_rank` by the source-factor CASE expression at the chunk-grain level; `searchVector` becomes a two-stage CTE — inner CTE keeps `ORDER BY cc.embedding <=> vec` so HNSW stays usable, outer SELECT re-ranks by `raw_score * source_factor`. Inner LIMIT scales with offset to preserve pagination contract. As of v0.22.6.1, `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL — probes for the specific forward-referenced state the embedded schema blob needs (`pages.source_id`, `links.link_source`, `links.origin_page_id`, `content_chunks.symbol_name`, `content_chunks.language`, `sources` FK target table) and adds only what's missing. Closes the upgrade-wedge bug class that bit users 10+ times across 6 schema versions over 2 years (#239/#243/#266/#357/#366/#374/#375/#378/#395/#396). No-op on fresh installs and modern brains.
|
||||
- `src/core/pglite-schema.ts` — PGLite-specific DDL (pgvector, pg_trgm, triggers)
|
||||
- `src/core/postgres-engine.ts` — Postgres + pgvector implementation (Supabase / self-hosted). `addLinksBatch` / `addTimelineEntriesBatch` use `INSERT ... SELECT FROM unnest($1::text[], ...) JOIN pages ON CONFLICT DO NOTHING RETURNING 1` — 4-5 array params regardless of batch size, sidesteps the 65535-parameter cap. As of v0.12.3, `searchKeyword` / `searchVector` scope `statement_timeout` via `sql.begin` + `SET LOCAL` so the GUC dies with the transaction instead of leaking across the pooled postgres.js connection (contributed by @garagon). `getEmbeddingsByChunkIds` uses `tryParseEmbedding` so one corrupt row skips+warns instead of killing the query. v0.22.0: `searchKeyword`, `searchKeywordChunks`, and `searchVector` apply source-aware ranking by inlining the source-factor CASE and `NOT (col LIKE …)` hard-exclude clause from `src/core/search/sql-ranking.ts`. `searchVector` switches to a two-stage CTE (HNSW-safe inner ORDER BY, source-boost re-rank in the outer SELECT) and carries `p.source_id` through inner→outer for v0.18 multi-source callers. v0.22.1 (#406): `_savedConfig` retains the connect config; `reconnect()` tears down + recreates the pool from saved config (called by supervisor watchdog after 3 consecutive health-check failures). `executeRaw` is a single-statement passthrough — no per-call retry (D3 dropped that as unsound for non-idempotent statements; recovery is supervisor-driven). v0.22.1 (#363, contributed by @orendi84): `connect()` applies `resolveSessionTimeouts()` from `db.ts` as connection-time startup parameters (`statement_timeout`, `idle_in_transaction_session_timeout`) so orphan pgbouncer backends can't hold locks for hours. v0.22.1 (#409, contributed by @atrevino47): `countStaleChunks()` + `listStaleChunks()` server-side-filter on `embedding IS NULL` for `embed --stale`, eliminating ~76 MB/call client-side pull on a fully-embedded brain; `upsertChunks()` resets both `embedding` AND `embedded_at` to NULL when chunk_text changes without a new embedding (consistency). As of v0.22.6.1, `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL on the same forward-reference probe set as the PGLite engine, so old Postgres brains pinned at v0.13/v0.18/v0.19 walk forward cleanly instead of wedging on `column "..." does not exist`.
|
||||
- `src/core/utils.ts` — Shared SQL utilities extracted from postgres-engine.ts. Exports `parseEmbedding(value)` (throws on unknown input, used by migration + ingest paths where data integrity matters) and as of v0.12.3 `tryParseEmbedding(value)` (returns `null` + warns once per process, used by search/rescore paths where availability matters more than strictness).
|
||||
- `src/core/utils.ts` — Shared SQL utilities extracted from postgres-engine.ts. Exports `parseEmbedding(value)` (throws on unknown input, used by migration + ingest paths where data integrity matters) and as of v0.12.3 `tryParseEmbedding(value)` (returns `null` + warns once per process, used by search/rescore paths where availability matters more than strictness). **v0.26.9 (D14):** adds `isUndefinedColumnError(err)` predicate — pattern-matches Postgres SQLSTATE 42703 / "column ... does not exist" with engine-driver shape variation tolerated. Replaces bare `catch {}` blocks in `oauth-provider.ts` so genuine errors (lock timeout, network blip, permission denied) propagate while column-missing falls through to the legacy fallback path. Reusable from any future code that needs the same column-existence probe semantics.
|
||||
- `src/core/db.ts` — Connection management, schema initialization. v0.22.1 (#363, contributed by @orendi84): `resolveSessionTimeouts()` returns `statement_timeout` + `idle_in_transaction_session_timeout` (defaults: 5min each, env-overridable via `GBRAIN_STATEMENT_TIMEOUT` / `GBRAIN_IDLE_TX_TIMEOUT` / `GBRAIN_CLIENT_CHECK_INTERVAL`). Both `connect()` (module singleton) and `PostgresEngine.connect()` (worker pool) consume the result via postgres.js's `connection` option, sending GUCs as startup parameters that survive PgBouncer transaction mode (unlike the prior `setSessionDefaults` post-pool SET, kept as a back-compat no-op shim).
|
||||
- `src/commands/migrate-engine.ts` — Bidirectional engine migration (`gbrain migrate --to supabase/pglite`)
|
||||
- `src/core/import-file.ts` — importFromFile + importFromContent (chunk + embed + tags)
|
||||
@@ -126,10 +126,10 @@ strict behavior when unset.
|
||||
- `src/commands/features.ts` — `gbrain features --json --auto-fix`: usage scan + feature adoption salesman
|
||||
- `src/commands/autopilot.ts` — `gbrain autopilot --install`: self-maintaining brain daemon (sync+extract+embed)
|
||||
- `src/mcp/server.ts` — MCP stdio server (generated from operations). v0.22.7: tool-call handler delegates to `dispatchToolCall` from `src/mcp/dispatch.ts` so stdio + HTTP transports share one validation, context-build, and error-format path.
|
||||
- `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `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 to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport.
|
||||
- `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `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 to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport. **v0.26.9 (F8):** adds `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 as a sorted array for debug visibility; 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 via repeated probes. Operators on a personal laptop who want raw payload visibility opt back in with `gbrain serve --http --log-full-params` (loud stderr warning at startup). Canonical helper — new logging code paths route through it rather than `JSON.stringify(params)`.
|
||||
- `src/mcp/rate-limit.ts` (v0.22.7) — 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 actually 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` (v0.26.0) — 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]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, 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 endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token.
|
||||
- `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `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 race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements 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 wrapped in try/catch. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper.
|
||||
- `src/commands/serve-http.ts` (v0.26.0) — 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] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, 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 endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape.
|
||||
- `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `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 race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements 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 wrapped in try/catch. **v0.26.9 RFC 6749/7009 hardening pass:** F1+F2 fold `client_id` atomically into the `DELETE WHERE` clauses for both auth-code exchange and refresh rotation — pre-fix the post-hoc client compare burned the row on wrong-client paths so the legitimate client couldn't retry. F3 enforces refresh-scope-subset against the original grant on the row (RFC 6749 §6), not the client's currently-allowed scopes — fixes the case where revoking a scope from a client wouldn't shrink the agent's existing refresh tokens. F4 binds `client_id` on `revokeToken` so a client can only revoke its own tokens (RFC 7009 §2.1). F7c validates the `/token` request's `redirect_uri` against the value stored at `/authorize` (RFC 6749 §4.1.3) — empty-string treated as missing rather than wildcard match (adversarial-review fix). F5 swaps bare `catch {}` blocks in `verifyAccessToken` and `getClient` for `isUndefinedColumnError` from `src/core/utils.ts` — only SQLSTATE 42703 falls through to legacy fallback; lock timeouts and network blips throw and surface. F6 makes `sweepExpiredTokens()` actually return the count via `RETURNING 1` + array length, not a fire-and-forget zero. F12 adds `dcrDisabled` constructor option so `serve-http.ts` can disable the `/register` endpoint without monkey-patching the router. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper.
|
||||
- `admin/` (v0.26.0) — 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 button), Register (modal with scope checkboxes + grant type selector), Credentials reveal (full-screen modal with Copy + Download JSON + yellow 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 (v0.22.7 wired as a first-class CLI subcommand) plus `gbrain auth register-client` (v0.26.0) and `gbrain auth revoke-client <client_id>` (v0.26.2) 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 + authorization code in a single transaction. `process.exit(1)` on no-such-client (idempotent — re-running on the same id produces the same exit-1 message). Legacy tokens stored as SHA-256 hashes in `access_tokens`; OAuth clients in `oauth_clients`. As of v0.26.0, legacy tokens grandfather to `read+write+admin` scopes on the OAuth HTTP server, so pre-v0.26 deployments keep working with no migration.
|
||||
- `src/commands/upgrade.ts` — Self-update CLI. `runPostUpgrade()` enumerates migrations from the TS registry (src/commands/migrations/index.ts) and tail-calls `runApplyMigrations(['--yes', '--non-interactive'])` so the mechanical side of every outstanding migration runs unconditionally.
|
||||
@@ -282,7 +282,7 @@ Key commands added in v0.14.2:
|
||||
- `gbrain doctor` gains two new checks: `sync_failures` (surfaces unacknowledged parse failures with exact paths + fix hints) and `brain_score` (renders the 5-component breakdown when score < 100: embed coverage / 35, link density / 25, timeline coverage / 15, orphans / 15, dead links / 10 — sum equals total).
|
||||
|
||||
Key commands added in v0.26.0 (OAuth 2.1 + HTTP server + admin dashboard):
|
||||
- `gbrain serve --http [--port 3131] [--token-ttl 3600] [--enable-dcr]` — HTTP MCP server with OAuth 2.1, admin dashboard at `/admin`, SSE activity feed at `/admin/events`, health check at `/health`. Prints admin bootstrap token on first start. Alongside (not replacing) stdio `gbrain serve`.
|
||||
- `gbrain serve --http [--port 3131] [--token-ttl 3600] [--enable-dcr] [--log-full-params]` — HTTP MCP server with OAuth 2.1, admin dashboard at `/admin`, SSE activity feed at `/admin/events`, health check at `/health`. Prints admin bootstrap token on first start. Alongside (not replacing) stdio `gbrain serve`. As of v0.26.9, `mcp_request_log.params` and the SSE feed default to a redacted summary (`{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`); pass `--log-full-params` to log raw payloads on a personal laptop with a startup warning.
|
||||
- **OAuth client registration** — three paths:
|
||||
1. CLI: `gbrain auth register-client <name> --grant-types <types> --scopes <scopes>` (wired into `src/commands/auth.ts` as a thin wrapper over `GBrainOAuthProvider.registerClientManual`). Default grant types: `client_credentials`. Default scopes: `read`.
|
||||
2. Admin dashboard: Register client modal → credential reveal with Copy + Download JSON.
|
||||
@@ -484,7 +484,9 @@ parity), `test/cli.test.ts` (CLI structure), `test/config.test.ts` (config redac
|
||||
`test/doctor.test.ts` (doctor command + v0.12.3 assertions that `jsonb_integrity` scans the four v0.12.0 write sites and `markdown_body_completeness` is present),
|
||||
`test/utils.test.ts` (shared SQL utilities + `tryParseEmbedding` null-return and single-warn semantics),
|
||||
`test/build-llms.test.ts` (llms.txt/llms-full.txt generator: path resolution, idempotence, spec shape, regen-drift guard, content contract, AGENTS.md install-path mirror, size-budget enforcement — 7 cases),
|
||||
`test/oauth.test.ts` (v0.26.0 OAuth 2.1 provider — 27 cases: register, getClient, `client_credentials` grant exchange, `authorization_code` flow with PKCE challenge / verifier, refresh token rotation, `verifyAccessToken` with both OAuth + legacy `access_tokens` fallback, `revokeToken`, `sweepExpiredTokens`, and a contract test asserting `scope` + `localOnly` annotations are set correctly on all 30 operations; **v0.26.2** adds 5 `coerceTimestamp` unit cases (null/undefined/string/number/throw-on-NaN), NULL-`expires_at`-as-expired contract tests for both refresh + access token paths, and a cascade-delete contract test asserting `revoke-client` purges `oauth_tokens` + `oauth_codes` rows via FK CASCADE),
|
||||
`test/oauth.test.ts` (v0.26.0 OAuth 2.1 provider — 27 cases: register, getClient, `client_credentials` grant exchange, `authorization_code` flow with PKCE challenge / verifier, refresh token rotation, `verifyAccessToken` with both OAuth + legacy `access_tokens` fallback, `revokeToken`, `sweepExpiredTokens`, and a contract test asserting `scope` + `localOnly` annotations are set correctly on all 30 operations; **v0.26.2** adds 5 `coerceTimestamp` unit cases (null/undefined/string/number/throw-on-NaN), NULL-`expires_at`-as-expired contract tests for both refresh + access token paths, and a cascade-delete contract test asserting `revoke-client` purges `oauth_tokens` + `oauth_codes` rows via FK CASCADE; **v0.26.9** adds 14 cases pinning the F1/F2/F3/F4/F5/F6/F7c/F12 invariants, including the F1/F4 cross-client isolation pattern (wrong-client attempt MUST reject AND rightful owner MUST still succeed atomically afterward) and the empty-string `redirect_uri` bypass guard surfaced during adversarial review),
|
||||
`test/mcp-dispatch-summarize.test.ts` (v0.26.9 — 7 cases pinning F8 `summarizeMcpParams` invariants: declared-keys allow-list intersection, attacker-key-name leak guard (unknown keys counted not named), 1KB byte bucketing for size-probe defense, missing op falls through to fully-redacted shape, declared-keys sorted for deterministic output),
|
||||
`test/trust-boundary-contract.test.ts` (v0.26.9 — 4 cases pinning F7b fail-closed semantics under cast bypass: `ctx.remote === undefined` treated as remote/untrusted at every flipped call site, `as any` and `Partial<>` spreads can't downgrade trust by accident),
|
||||
`test/check-resolvable-cli.test.ts` (v0.19 CLI wrapper: exit codes, JSON envelope shape, AGENTS.md fallback chain),
|
||||
`test/regression-v0_16_4.test.ts` (findRepoRoot regression guard — hermetic startDir parameterization),
|
||||
`test/filing-audit.test.ts` (v0.19 Check 6: `writes_pages` / `writes_to` frontmatter, filing-rules JSON validation),
|
||||
@@ -511,7 +513,7 @@ E2E tests (`test/e2e/`): Run against real Postgres+pgvector. Require `DATABASE_U
|
||||
- `test/e2e/engine-parity.test.ts` (v0.22.0) — Postgres ↔ PGLite top-result and result-set parity for `searchKeyword` + `searchVector`. Codex flagged that Postgres ranks pages then picks best chunk while PGLite returns chunks directly — without parity coverage the source-boost fix could pass on PGLite and fail on Postgres. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/postgres-bootstrap.test.ts` (v0.22.6.1) — exercises `PostgresEngine.initSchema()` directly against a fresh real Postgres database. Asserts the bootstrap path is no-op on fresh installs and that SCHEMA_SQL replays cleanly through the engine path (not via the standalone `db.initSchema` from `src/core/db.ts`, which would have produced false-positive coverage). Codex caught the E2E-shape gap during plan review.
|
||||
- `test/e2e/http-transport.test.ts` (v0.22.7) — 8 cases against real Postgres covering `gbrain serve --http` end-to-end: bearer auth round-trip, `last_used_at` SQL-level debounce semantics, `mcp_request_log` row insertion on success and auth_failed paths, `/health` DB-down → 503 (DB-probing health check), and the F1+F2+F3 dispatch round-trip with a real operation. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/serve-http-oauth.test.ts` (v0.26.0, expanded v0.26.2) — real-Postgres E2E against `gbrain serve --http` with full OAuth 2.1. Spawns a subprocess server, registers a client via the CLI, mints `client_credentials` tokens, exercises the `/mcp` JSON-RPC pipeline. **v0.26.2 adds:** real DCR `/register` HTTP-level response-shape test (asserts `typeof body.client_id_issued_at === 'number'` over the wire — RFC 7591 §3.2.1 spec compliance, not just internal-store shape); real CLI subprocess test for `revoke-client` (registers → mints token → revokes via `execSync` → asserts token rejected at `/mcp` → asserts re-run exits 1); server fixture flips on `--enable-dcr` so `/register` is reachable. **bun execSync env-inheritance fix:** bun's `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, which is invisible to subprocesses unless `env: { ...process.env }` is passed explicitly — every subprocess call in this file passes `env: { ...process.env }` for that reason. Reference fix for the next maintainer hitting the same failure mode in sibling sync/cycle/dream/claw-test E2Es. `afterAll` cleanup is guarded on `clientId` (won't throw if `beforeAll` failed before registration); cleanup errors surface to stderr without throwing so real test failures aren't masked. Tracks DCR-registered clients alongside the manual one. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/serve-http-oauth.test.ts` (v0.26.0, expanded v0.26.2, expanded v0.26.9) — real-Postgres E2E against `gbrain serve --http` with full OAuth 2.1. Spawns a subprocess server, registers a client via the CLI, mints `client_credentials` tokens, exercises the `/mcp` JSON-RPC pipeline. **v0.26.2 adds:** real DCR `/register` HTTP-level response-shape test (asserts `typeof body.client_id_issued_at === 'number'` over the wire — RFC 7591 §3.2.1 spec compliance, not just internal-store shape); real CLI subprocess test for `revoke-client` (registers → mints token → revokes via `execSync` → asserts token rejected at `/mcp` → asserts re-run exits 1); server fixture flips on `--enable-dcr` so `/register` is reachable. **bun execSync env-inheritance fix:** bun's `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, which is invisible to subprocesses unless `env: { ...process.env }` is passed explicitly — every subprocess call in this file passes `env: { ...process.env }` for that reason. Reference fix for the next maintainer hitting the same failure mode in sibling sync/cycle/dream/claw-test E2Es. `afterAll` cleanup is guarded on `clientId` (won't throw if `beforeAll` failed before registration); cleanup errors surface to stderr without throwing so real test failures aren't masked. Tracks DCR-registered clients alongside the manual one. **v0.26.9** adds 2 regressions for the F7 trust-boundary fix: an HTTP MCP `submit_job` for `name: "shell"` MUST reject with a permission error (proving the request handler now sets `remote: true` and `submit_job`'s protected-name guard fires), and the same guard rejects subagent submission. Closes the OAuth-token-to-RCE escalation path. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/sync-parallel.test.ts` (v0.22.13 PR #490) — DATABASE_URL-gated. T2: 60-file Postgres sync at concurrency=4 imports all + no connection leak (probes `pg_stat_activity` before/after to confirm worker engines disconnected). P4: 120-file serial-vs-parallel benchmark prints `SYNC_PARALLEL_BENCH N files | serial=Xms | parallel(4)=Yms | speedup=Zx` for CHANGELOG quoting. Asserts parallel ≤ serial × 1.5 (CI-noise tolerant; not a strict speedup gate).
|
||||
- Tier 2 (`skills.test.ts`) requires OpenClaw + API keys, runs nightly in CI
|
||||
- If `.env.testing` doesn't exist in this directory, check sibling worktrees for one:
|
||||
|
||||
@@ -729,7 +729,7 @@ ADMIN
|
||||
gbrain serve MCP server (stdio)
|
||||
gbrain serve --http [--port 3131] HTTP MCP server with OAuth 2.1 + admin dashboard
|
||||
[--token-ttl 3600] [--enable-dcr]
|
||||
[--public-url URL]
|
||||
[--public-url URL] [--log-full-params]
|
||||
gbrain auth create|list|revoke|test Legacy bearer token management
|
||||
gbrain auth register-client <name> Register an OAuth 2.1 client
|
||||
--grant-types client_credentials,authorization_code
|
||||
|
||||
+11
@@ -166,3 +166,14 @@ psql "$DATABASE_URL" -c \
|
||||
`body_too_large`, `parse_error`, `unknown_method`. Failed-auth rows have
|
||||
`token_name = NULL`. Inserts are fire-and-forget so audit failures
|
||||
never block requests.
|
||||
|
||||
**v0.26.9 redaction default.** The `params` column now stores
|
||||
`{redacted, kind, declared_keys, unknown_key_count, approx_bytes}` instead
|
||||
of raw JSON-RPC payloads. Declared keys (intersected against the operation's
|
||||
spec) preserve for debug visibility; unknown keys are counted but never
|
||||
named so attackers can't probe key existence; byte sizes bucket to 1KB so
|
||||
content sizes can't be binary-searched. The same shape is broadcast on the
|
||||
admin SSE feed at `/admin/events`. Operators on a personal laptop who want
|
||||
raw payloads back can pass `gbrain serve --http --log-full-params` (loud
|
||||
stderr warning at startup). Multi-tenant deployments should leave it
|
||||
on the redacted default.
|
||||
|
||||
@@ -1,5 +1,31 @@
|
||||
# TODOS
|
||||
|
||||
## OAuth/MCP hardening (v0.26.7 follow-up)
|
||||
|
||||
### F11 — `auth register-client --redirect-uri` flag
|
||||
**Priority:** P3
|
||||
|
||||
**What:** `gbrain auth register-client` always passes `[]` for redirect URIs; there is no CLI flag to set them. Operators who want to register an `authorization_code` client without DCR have to hand-edit the database.
|
||||
|
||||
**Why:** Operator UX gap, not a trust-boundary issue. Codex C11 correctly flagged it as scope creep on the v0.26.7 hardening pass — kept out of that PR but worth doing.
|
||||
|
||||
**Pros:** Closes the operator-experience gap. Validates `https://` or loopback per RFC 6749 §3.1.2.1 at registration time. Repeatable flag.
|
||||
**Cons:** ~30 lines of argv parsing + URL validation. Adds one more flag to the `auth register-client` surface. Low value relative to the OAuth provider hardening that already shipped.
|
||||
**Context:** Eva-brain has the implementation under `src/commands/auth.ts:registerClient`. Lift verbatim — the `localhost`/`127.0.0.1`/`::1` exact-match validation is correct; codex spot-check confirmed it does NOT match `localhost.evil.com`. v0.27 candidate.
|
||||
**Depends on:** Nothing.
|
||||
|
||||
### F13 — `gbrain serve --http` argv positive-int validator
|
||||
**Priority:** P3
|
||||
|
||||
**What:** `parseInt(args[idx + 1])` on `--port` and `--token-ttl` accepts the next flag as the value if the argument is missing (e.g., `--port --token-ttl 100` parses port as NaN → fallback 3131). Negative integers like `--port -1` parse to -1, server fails to bind with a confusing error.
|
||||
|
||||
**Why:** Hygiene, not security. Codex C11 flagged as scope creep. Cheap to do later.
|
||||
|
||||
**Pros:** Replaces `parseInt(...) || fallback` with a `parsePositiveIntOption(args, flag, fallback, {max?})` helper that validates the next arg isn't a flag, matches `^[1-9]\d*$`, and clamps to a max. Exits 2 with a clear error.
|
||||
**Cons:** ~20 lines of helper + threading through `serve.ts`. Behavior change: previously-silent bad input now exits loud. Probably fine; no consumer relies on the silent fallback.
|
||||
**Context:** Eva-brain has the helper at `src/commands/serve.ts`. v0.27 candidate.
|
||||
**Depends on:** Nothing.
|
||||
|
||||
## destructive-guard (v0.26.5 follow-up)
|
||||
|
||||
### Adjacent 2 — Storage objects orphan on hard purge
|
||||
|
||||
@@ -85,6 +85,15 @@ Save this token. Open `http://localhost:3131/admin` and paste it to access the
|
||||
dashboard. The dashboard shows live activity, registered clients, request logs,
|
||||
and per-client config export.
|
||||
|
||||
> **v0.26.9+:** `mcp_request_log.params` and the live SSE activity feed default
|
||||
> to a redacted summary `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`.
|
||||
> Declared param keys are kept (intersected against the operation's spec); unknown
|
||||
> keys are counted but never named, and byte sizes round up to 1KB so size-probe
|
||||
> attacks can't binary-search secret content. Operators on a personal laptop who
|
||||
> want raw payloads back can pass `gbrain serve --http --log-full-params` (loud
|
||||
> stderr warning fires at startup). Multi-tenant deployments should leave it on
|
||||
> the redacted default.
|
||||
|
||||
### 2. Register OAuth clients
|
||||
|
||||
Register clients from the **`/admin` dashboard**:
|
||||
|
||||
+20
-9
@@ -137,13 +137,13 @@ strict behavior when unset.
|
||||
|
||||
## Key files
|
||||
|
||||
- `src/core/operations.ts` — Contract-first operation definitions (the foundation). Also exports upload validators: `validateUploadPath`, `validatePageSlug`, `validateFilename`, plus `matchesSlugAllowList(slug, prefixes)` (v0.23 glob matcher: `<prefix>/*` matches recursive children; bare `<prefix>` matches exact only). `OperationContext.remote` flags untrusted callers; `OperationContext.allowedSlugPrefixes` (v0.23) is the trusted-workspace allow-list set by the dream cycle. `put_page` enforces: when `viaSubagent` and `allowedSlugPrefixes` is set, slug must match the allow-list; else the legacy `wiki/agents/<id>/...` namespace check applies. Auto-link enabled for trusted-workspace writes (skipped only when `remote=true && !trustedWorkspace`). As of v0.26.0, every `Operation` also carries `scope?: 'read' | 'write' | 'admin'` + `localOnly?: boolean`. All ops are annotated; `sync_brain`, `file_upload`, `file_list`, and `file_url` are `admin + localOnly` (rejected over HTTP). `OperationContext.auth?: AuthInfo` is threaded through HTTP dispatch for scope enforcement in `serve-http.ts` before the op runs.
|
||||
- `src/core/operations.ts` — Contract-first operation definitions (the foundation). Also exports upload validators: `validateUploadPath`, `validatePageSlug`, `validateFilename`, plus `matchesSlugAllowList(slug, prefixes)` (v0.23 glob matcher: `<prefix>/*` matches recursive children; bare `<prefix>` matches exact only). `OperationContext.remote` flags untrusted callers; `OperationContext.allowedSlugPrefixes` (v0.23) is the trusted-workspace allow-list set by the dream cycle. `put_page` enforces: when `viaSubagent` and `allowedSlugPrefixes` is set, slug must match the allow-list; else the legacy `wiki/agents/<id>/...` namespace check applies. Auto-link enabled for trusted-workspace writes (skipped only when `remote=true && !trustedWorkspace`). As of v0.26.0, every `Operation` also carries `scope?: 'read' | 'write' | 'admin'` + `localOnly?: boolean`. All ops are annotated; `sync_brain`, `file_upload`, `file_list`, and `file_url` are `admin + localOnly` (rejected over HTTP). `OperationContext.auth?: AuthInfo` is threaded through HTTP dispatch for scope enforcement in `serve-http.ts` before the op runs. **v0.26.9 (D12 + F7b):** `OperationContext.remote` is now a REQUIRED field in the TypeScript type — the compiler is the first defense against transports that forget to set it. Four trust-boundary call sites (`put_page` allowlist, file_upload trust-narrowing, submit_job protected-name guard, auto-link skip) flipped from falsy-default (`!ctx.remote`) to fail-closed semantics (`ctx.remote === false` for "trusted-only" sites and `ctx.remote !== false` for "untrust unless explicit-false"). Anything that isn't strictly `false` is now treated as remote. Closed an HTTP MCP shell-job RCE: a `read+write`-scoped OAuth token could submit `shell` jobs because the HTTP request handler's literal context skipped `remote: true` and `submit_job`'s protected-name guard saw a falsy undefined. Stdio MCP set the field correctly via dispatch.ts; HTTP inlined a parallel context-builder for several releases and lost it.
|
||||
- `src/core/engine.ts` — Pluggable engine interface (BrainEngine). `clampSearchLimit(limit, default, cap)` takes an explicit cap so per-operation caps can be tighter than `MAX_SEARCH_LIMIT`. Exports `LinkBatchInput` / `TimelineBatchInput` for the v0.12.1 bulk-insert API (`addLinksBatch` / `addTimelineEntriesBatch`). As of v0.13.1, `BrainEngine` has a `readonly kind: 'postgres' | 'pglite'` discriminator so migrations (`src/core/migrate.ts`) and other consumers can branch on engine without `instanceof` + dynamic imports.
|
||||
- `src/core/engine-factory.ts` — Engine factory with dynamic imports (`'pglite'` | `'postgres'`)
|
||||
- `src/core/pglite-engine.ts` — PGLite (embedded Postgres 17.5 via WASM) implementation, all 40 BrainEngine methods. `addLinksBatch` / `addTimelineEntriesBatch` use multi-row `unnest()` with manual `$N` placeholders. As of v0.13.1, `connect()` wraps `PGlite.create()` in a try/catch that emits an actionable error naming the macOS 26.3 WASM bug (#223) and pointing at `gbrain doctor`; the lock is released on failure so the next process can retry cleanly. v0.22.0: `searchKeyword` and `searchKeywordChunks` multiply `ts_rank` by the source-factor CASE expression at the chunk-grain level; `searchVector` becomes a two-stage CTE — inner CTE keeps `ORDER BY cc.embedding <=> vec` so HNSW stays usable, outer SELECT re-ranks by `raw_score * source_factor`. Inner LIMIT scales with offset to preserve pagination contract. As of v0.22.6.1, `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL — probes for the specific forward-referenced state the embedded schema blob needs (`pages.source_id`, `links.link_source`, `links.origin_page_id`, `content_chunks.symbol_name`, `content_chunks.language`, `sources` FK target table) and adds only what's missing. Closes the upgrade-wedge bug class that bit users 10+ times across 6 schema versions over 2 years (#239/#243/#266/#357/#366/#374/#375/#378/#395/#396). No-op on fresh installs and modern brains.
|
||||
- `src/core/pglite-schema.ts` — PGLite-specific DDL (pgvector, pg_trgm, triggers)
|
||||
- `src/core/postgres-engine.ts` — Postgres + pgvector implementation (Supabase / self-hosted). `addLinksBatch` / `addTimelineEntriesBatch` use `INSERT ... SELECT FROM unnest($1::text[], ...) JOIN pages ON CONFLICT DO NOTHING RETURNING 1` — 4-5 array params regardless of batch size, sidesteps the 65535-parameter cap. As of v0.12.3, `searchKeyword` / `searchVector` scope `statement_timeout` via `sql.begin` + `SET LOCAL` so the GUC dies with the transaction instead of leaking across the pooled postgres.js connection (contributed by @garagon). `getEmbeddingsByChunkIds` uses `tryParseEmbedding` so one corrupt row skips+warns instead of killing the query. v0.22.0: `searchKeyword`, `searchKeywordChunks`, and `searchVector` apply source-aware ranking by inlining the source-factor CASE and `NOT (col LIKE …)` hard-exclude clause from `src/core/search/sql-ranking.ts`. `searchVector` switches to a two-stage CTE (HNSW-safe inner ORDER BY, source-boost re-rank in the outer SELECT) and carries `p.source_id` through inner→outer for v0.18 multi-source callers. v0.22.1 (#406): `_savedConfig` retains the connect config; `reconnect()` tears down + recreates the pool from saved config (called by supervisor watchdog after 3 consecutive health-check failures). `executeRaw` is a single-statement passthrough — no per-call retry (D3 dropped that as unsound for non-idempotent statements; recovery is supervisor-driven). v0.22.1 (#363, contributed by @orendi84): `connect()` applies `resolveSessionTimeouts()` from `db.ts` as connection-time startup parameters (`statement_timeout`, `idle_in_transaction_session_timeout`) so orphan pgbouncer backends can't hold locks for hours. v0.22.1 (#409, contributed by @atrevino47): `countStaleChunks()` + `listStaleChunks()` server-side-filter on `embedding IS NULL` for `embed --stale`, eliminating ~76 MB/call client-side pull on a fully-embedded brain; `upsertChunks()` resets both `embedding` AND `embedded_at` to NULL when chunk_text changes without a new embedding (consistency). As of v0.22.6.1, `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL on the same forward-reference probe set as the PGLite engine, so old Postgres brains pinned at v0.13/v0.18/v0.19 walk forward cleanly instead of wedging on `column "..." does not exist`.
|
||||
- `src/core/utils.ts` — Shared SQL utilities extracted from postgres-engine.ts. Exports `parseEmbedding(value)` (throws on unknown input, used by migration + ingest paths where data integrity matters) and as of v0.12.3 `tryParseEmbedding(value)` (returns `null` + warns once per process, used by search/rescore paths where availability matters more than strictness).
|
||||
- `src/core/utils.ts` — Shared SQL utilities extracted from postgres-engine.ts. Exports `parseEmbedding(value)` (throws on unknown input, used by migration + ingest paths where data integrity matters) and as of v0.12.3 `tryParseEmbedding(value)` (returns `null` + warns once per process, used by search/rescore paths where availability matters more than strictness). **v0.26.9 (D14):** adds `isUndefinedColumnError(err)` predicate — pattern-matches Postgres SQLSTATE 42703 / "column ... does not exist" with engine-driver shape variation tolerated. Replaces bare `catch {}` blocks in `oauth-provider.ts` so genuine errors (lock timeout, network blip, permission denied) propagate while column-missing falls through to the legacy fallback path. Reusable from any future code that needs the same column-existence probe semantics.
|
||||
- `src/core/db.ts` — Connection management, schema initialization. v0.22.1 (#363, contributed by @orendi84): `resolveSessionTimeouts()` returns `statement_timeout` + `idle_in_transaction_session_timeout` (defaults: 5min each, env-overridable via `GBRAIN_STATEMENT_TIMEOUT` / `GBRAIN_IDLE_TX_TIMEOUT` / `GBRAIN_CLIENT_CHECK_INTERVAL`). Both `connect()` (module singleton) and `PostgresEngine.connect()` (worker pool) consume the result via postgres.js's `connection` option, sending GUCs as startup parameters that survive PgBouncer transaction mode (unlike the prior `setSessionDefaults` post-pool SET, kept as a back-compat no-op shim).
|
||||
- `src/commands/migrate-engine.ts` — Bidirectional engine migration (`gbrain migrate --to supabase/pglite`)
|
||||
- `src/core/import-file.ts` — importFromFile + importFromContent (chunk + embed + tags)
|
||||
@@ -223,10 +223,10 @@ strict behavior when unset.
|
||||
- `src/commands/features.ts` — `gbrain features --json --auto-fix`: usage scan + feature adoption salesman
|
||||
- `src/commands/autopilot.ts` — `gbrain autopilot --install`: self-maintaining brain daemon (sync+extract+embed)
|
||||
- `src/mcp/server.ts` — MCP stdio server (generated from operations). v0.22.7: tool-call handler delegates to `dispatchToolCall` from `src/mcp/dispatch.ts` so stdio + HTTP transports share one validation, context-build, and error-format path.
|
||||
- `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `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 to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport.
|
||||
- `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `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 to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport. **v0.26.9 (F8):** adds `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 as a sorted array for debug visibility; 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 via repeated probes. Operators on a personal laptop who want raw payload visibility opt back in with `gbrain serve --http --log-full-params` (loud stderr warning at startup). Canonical helper — new logging code paths route through it rather than `JSON.stringify(params)`.
|
||||
- `src/mcp/rate-limit.ts` (v0.22.7) — 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 actually 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` (v0.26.0) — 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]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, 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 endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token.
|
||||
- `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `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 race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements 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 wrapped in try/catch. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper.
|
||||
- `src/commands/serve-http.ts` (v0.26.0) — 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] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, 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 endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape.
|
||||
- `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `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 race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements 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 wrapped in try/catch. **v0.26.9 RFC 6749/7009 hardening pass:** F1+F2 fold `client_id` atomically into the `DELETE WHERE` clauses for both auth-code exchange and refresh rotation — pre-fix the post-hoc client compare burned the row on wrong-client paths so the legitimate client couldn't retry. F3 enforces refresh-scope-subset against the original grant on the row (RFC 6749 §6), not the client's currently-allowed scopes — fixes the case where revoking a scope from a client wouldn't shrink the agent's existing refresh tokens. F4 binds `client_id` on `revokeToken` so a client can only revoke its own tokens (RFC 7009 §2.1). F7c validates the `/token` request's `redirect_uri` against the value stored at `/authorize` (RFC 6749 §4.1.3) — empty-string treated as missing rather than wildcard match (adversarial-review fix). F5 swaps bare `catch {}` blocks in `verifyAccessToken` and `getClient` for `isUndefinedColumnError` from `src/core/utils.ts` — only SQLSTATE 42703 falls through to legacy fallback; lock timeouts and network blips throw and surface. F6 makes `sweepExpiredTokens()` actually return the count via `RETURNING 1` + array length, not a fire-and-forget zero. F12 adds `dcrDisabled` constructor option so `serve-http.ts` can disable the `/register` endpoint without monkey-patching the router. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper.
|
||||
- `admin/` (v0.26.0) — 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 button), Register (modal with scope checkboxes + grant type selector), Credentials reveal (full-screen modal with Copy + Download JSON + yellow 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 (v0.22.7 wired as a first-class CLI subcommand) plus `gbrain auth register-client` (v0.26.0) and `gbrain auth revoke-client <client_id>` (v0.26.2) 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 + authorization code in a single transaction. `process.exit(1)` on no-such-client (idempotent — re-running on the same id produces the same exit-1 message). Legacy tokens stored as SHA-256 hashes in `access_tokens`; OAuth clients in `oauth_clients`. As of v0.26.0, legacy tokens grandfather to `read+write+admin` scopes on the OAuth HTTP server, so pre-v0.26 deployments keep working with no migration.
|
||||
- `src/commands/upgrade.ts` — Self-update CLI. `runPostUpgrade()` enumerates migrations from the TS registry (src/commands/migrations/index.ts) and tail-calls `runApplyMigrations(['--yes', '--non-interactive'])` so the mechanical side of every outstanding migration runs unconditionally.
|
||||
@@ -379,7 +379,7 @@ Key commands added in v0.14.2:
|
||||
- `gbrain doctor` gains two new checks: `sync_failures` (surfaces unacknowledged parse failures with exact paths + fix hints) and `brain_score` (renders the 5-component breakdown when score < 100: embed coverage / 35, link density / 25, timeline coverage / 15, orphans / 15, dead links / 10 — sum equals total).
|
||||
|
||||
Key commands added in v0.26.0 (OAuth 2.1 + HTTP server + admin dashboard):
|
||||
- `gbrain serve --http [--port 3131] [--token-ttl 3600] [--enable-dcr]` — HTTP MCP server with OAuth 2.1, admin dashboard at `/admin`, SSE activity feed at `/admin/events`, health check at `/health`. Prints admin bootstrap token on first start. Alongside (not replacing) stdio `gbrain serve`.
|
||||
- `gbrain serve --http [--port 3131] [--token-ttl 3600] [--enable-dcr] [--log-full-params]` — HTTP MCP server with OAuth 2.1, admin dashboard at `/admin`, SSE activity feed at `/admin/events`, health check at `/health`. Prints admin bootstrap token on first start. Alongside (not replacing) stdio `gbrain serve`. As of v0.26.9, `mcp_request_log.params` and the SSE feed default to a redacted summary (`{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`); pass `--log-full-params` to log raw payloads on a personal laptop with a startup warning.
|
||||
- **OAuth client registration** — three paths:
|
||||
1. CLI: `gbrain auth register-client <name> --grant-types <types> --scopes <scopes>` (wired into `src/commands/auth.ts` as a thin wrapper over `GBrainOAuthProvider.registerClientManual`). Default grant types: `client_credentials`. Default scopes: `read`.
|
||||
2. Admin dashboard: Register client modal → credential reveal with Copy + Download JSON.
|
||||
@@ -581,7 +581,9 @@ parity), `test/cli.test.ts` (CLI structure), `test/config.test.ts` (config redac
|
||||
`test/doctor.test.ts` (doctor command + v0.12.3 assertions that `jsonb_integrity` scans the four v0.12.0 write sites and `markdown_body_completeness` is present),
|
||||
`test/utils.test.ts` (shared SQL utilities + `tryParseEmbedding` null-return and single-warn semantics),
|
||||
`test/build-llms.test.ts` (llms.txt/llms-full.txt generator: path resolution, idempotence, spec shape, regen-drift guard, content contract, AGENTS.md install-path mirror, size-budget enforcement — 7 cases),
|
||||
`test/oauth.test.ts` (v0.26.0 OAuth 2.1 provider — 27 cases: register, getClient, `client_credentials` grant exchange, `authorization_code` flow with PKCE challenge / verifier, refresh token rotation, `verifyAccessToken` with both OAuth + legacy `access_tokens` fallback, `revokeToken`, `sweepExpiredTokens`, and a contract test asserting `scope` + `localOnly` annotations are set correctly on all 30 operations; **v0.26.2** adds 5 `coerceTimestamp` unit cases (null/undefined/string/number/throw-on-NaN), NULL-`expires_at`-as-expired contract tests for both refresh + access token paths, and a cascade-delete contract test asserting `revoke-client` purges `oauth_tokens` + `oauth_codes` rows via FK CASCADE),
|
||||
`test/oauth.test.ts` (v0.26.0 OAuth 2.1 provider — 27 cases: register, getClient, `client_credentials` grant exchange, `authorization_code` flow with PKCE challenge / verifier, refresh token rotation, `verifyAccessToken` with both OAuth + legacy `access_tokens` fallback, `revokeToken`, `sweepExpiredTokens`, and a contract test asserting `scope` + `localOnly` annotations are set correctly on all 30 operations; **v0.26.2** adds 5 `coerceTimestamp` unit cases (null/undefined/string/number/throw-on-NaN), NULL-`expires_at`-as-expired contract tests for both refresh + access token paths, and a cascade-delete contract test asserting `revoke-client` purges `oauth_tokens` + `oauth_codes` rows via FK CASCADE; **v0.26.9** adds 14 cases pinning the F1/F2/F3/F4/F5/F6/F7c/F12 invariants, including the F1/F4 cross-client isolation pattern (wrong-client attempt MUST reject AND rightful owner MUST still succeed atomically afterward) and the empty-string `redirect_uri` bypass guard surfaced during adversarial review),
|
||||
`test/mcp-dispatch-summarize.test.ts` (v0.26.9 — 7 cases pinning F8 `summarizeMcpParams` invariants: declared-keys allow-list intersection, attacker-key-name leak guard (unknown keys counted not named), 1KB byte bucketing for size-probe defense, missing op falls through to fully-redacted shape, declared-keys sorted for deterministic output),
|
||||
`test/trust-boundary-contract.test.ts` (v0.26.9 — 4 cases pinning F7b fail-closed semantics under cast bypass: `ctx.remote === undefined` treated as remote/untrusted at every flipped call site, `as any` and `Partial<>` spreads can't downgrade trust by accident),
|
||||
`test/check-resolvable-cli.test.ts` (v0.19 CLI wrapper: exit codes, JSON envelope shape, AGENTS.md fallback chain),
|
||||
`test/regression-v0_16_4.test.ts` (findRepoRoot regression guard — hermetic startDir parameterization),
|
||||
`test/filing-audit.test.ts` (v0.19 Check 6: `writes_pages` / `writes_to` frontmatter, filing-rules JSON validation),
|
||||
@@ -608,7 +610,7 @@ E2E tests (`test/e2e/`): Run against real Postgres+pgvector. Require `DATABASE_U
|
||||
- `test/e2e/engine-parity.test.ts` (v0.22.0) — Postgres ↔ PGLite top-result and result-set parity for `searchKeyword` + `searchVector`. Codex flagged that Postgres ranks pages then picks best chunk while PGLite returns chunks directly — without parity coverage the source-boost fix could pass on PGLite and fail on Postgres. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/postgres-bootstrap.test.ts` (v0.22.6.1) — exercises `PostgresEngine.initSchema()` directly against a fresh real Postgres database. Asserts the bootstrap path is no-op on fresh installs and that SCHEMA_SQL replays cleanly through the engine path (not via the standalone `db.initSchema` from `src/core/db.ts`, which would have produced false-positive coverage). Codex caught the E2E-shape gap during plan review.
|
||||
- `test/e2e/http-transport.test.ts` (v0.22.7) — 8 cases against real Postgres covering `gbrain serve --http` end-to-end: bearer auth round-trip, `last_used_at` SQL-level debounce semantics, `mcp_request_log` row insertion on success and auth_failed paths, `/health` DB-down → 503 (DB-probing health check), and the F1+F2+F3 dispatch round-trip with a real operation. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/serve-http-oauth.test.ts` (v0.26.0, expanded v0.26.2) — real-Postgres E2E against `gbrain serve --http` with full OAuth 2.1. Spawns a subprocess server, registers a client via the CLI, mints `client_credentials` tokens, exercises the `/mcp` JSON-RPC pipeline. **v0.26.2 adds:** real DCR `/register` HTTP-level response-shape test (asserts `typeof body.client_id_issued_at === 'number'` over the wire — RFC 7591 §3.2.1 spec compliance, not just internal-store shape); real CLI subprocess test for `revoke-client` (registers → mints token → revokes via `execSync` → asserts token rejected at `/mcp` → asserts re-run exits 1); server fixture flips on `--enable-dcr` so `/register` is reachable. **bun execSync env-inheritance fix:** bun's `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, which is invisible to subprocesses unless `env: { ...process.env }` is passed explicitly — every subprocess call in this file passes `env: { ...process.env }` for that reason. Reference fix for the next maintainer hitting the same failure mode in sibling sync/cycle/dream/claw-test E2Es. `afterAll` cleanup is guarded on `clientId` (won't throw if `beforeAll` failed before registration); cleanup errors surface to stderr without throwing so real test failures aren't masked. Tracks DCR-registered clients alongside the manual one. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/serve-http-oauth.test.ts` (v0.26.0, expanded v0.26.2, expanded v0.26.9) — real-Postgres E2E against `gbrain serve --http` with full OAuth 2.1. Spawns a subprocess server, registers a client via the CLI, mints `client_credentials` tokens, exercises the `/mcp` JSON-RPC pipeline. **v0.26.2 adds:** real DCR `/register` HTTP-level response-shape test (asserts `typeof body.client_id_issued_at === 'number'` over the wire — RFC 7591 §3.2.1 spec compliance, not just internal-store shape); real CLI subprocess test for `revoke-client` (registers → mints token → revokes via `execSync` → asserts token rejected at `/mcp` → asserts re-run exits 1); server fixture flips on `--enable-dcr` so `/register` is reachable. **bun execSync env-inheritance fix:** bun's `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, which is invisible to subprocesses unless `env: { ...process.env }` is passed explicitly — every subprocess call in this file passes `env: { ...process.env }` for that reason. Reference fix for the next maintainer hitting the same failure mode in sibling sync/cycle/dream/claw-test E2Es. `afterAll` cleanup is guarded on `clientId` (won't throw if `beforeAll` failed before registration); cleanup errors surface to stderr without throwing so real test failures aren't masked. Tracks DCR-registered clients alongside the manual one. **v0.26.9** adds 2 regressions for the F7 trust-boundary fix: an HTTP MCP `submit_job` for `name: "shell"` MUST reject with a permission error (proving the request handler now sets `remote: true` and `submit_job`'s protected-name guard fires), and the same guard rejects subagent submission. Closes the OAuth-token-to-RCE escalation path. Skips gracefully when `DATABASE_URL` is unset.
|
||||
- `test/e2e/sync-parallel.test.ts` (v0.22.13 PR #490) — DATABASE_URL-gated. T2: 60-file Postgres sync at concurrency=4 imports all + no connection leak (probes `pg_stat_activity` before/after to confirm worker engines disconnected). P4: 120-file serial-vs-parallel benchmark prints `SYNC_PARALLEL_BENCH N files | serial=Xms | parallel(4)=Yms | speedup=Zx` for CHANGELOG quoting. Asserts parallel ≤ serial × 1.5 (CI-noise tolerant; not a strict speedup gate).
|
||||
- Tier 2 (`skills.test.ts`) requires OpenClaw + API keys, runs nightly in CI
|
||||
- If `.env.testing` doesn't exist in this directory, check sibling worktrees for one:
|
||||
@@ -2330,7 +2332,7 @@ ADMIN
|
||||
gbrain serve MCP server (stdio)
|
||||
gbrain serve --http [--port 3131] HTTP MCP server with OAuth 2.1 + admin dashboard
|
||||
[--token-ttl 3600] [--enable-dcr]
|
||||
[--public-url URL]
|
||||
[--public-url URL] [--log-full-params]
|
||||
gbrain auth create|list|revoke|test Legacy bearer token management
|
||||
gbrain auth register-client <name> Register an OAuth 2.1 client
|
||||
--grant-types client_credentials,authorization_code
|
||||
@@ -4610,6 +4612,15 @@ Save this token. Open `http://localhost:3131/admin` and paste it to access the
|
||||
dashboard. The dashboard shows live activity, registered clients, request logs,
|
||||
and per-client config export.
|
||||
|
||||
> **v0.26.9+:** `mcp_request_log.params` and the live SSE activity feed default
|
||||
> to a redacted summary `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`.
|
||||
> Declared param keys are kept (intersected against the operation's spec); unknown
|
||||
> keys are counted but never named, and byte sizes round up to 1KB so size-probe
|
||||
> attacks can't binary-search secret content. Operators on a personal laptop who
|
||||
> want raw payloads back can pass `gbrain serve --http --log-full-params` (loud
|
||||
> stderr warning fires at startup). Multi-tenant deployments should leave it on
|
||||
> the redacted default.
|
||||
|
||||
### 2. Register OAuth clients
|
||||
|
||||
Register clients from the **`/admin` dashboard**:
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gbrain",
|
||||
"version": "0.26.8",
|
||||
"version": "0.26.9",
|
||||
"description": "Postgres-native personal knowledge brain with hybrid RAG search",
|
||||
"type": "module",
|
||||
"main": "src/core/index.ts",
|
||||
|
||||
+115
-40
@@ -25,7 +25,10 @@ import type { BrainEngine } from '../core/engine.ts';
|
||||
import { operations, OperationError } from '../core/operations.ts';
|
||||
import type { OperationContext, AuthInfo } from '../core/operations.ts';
|
||||
import { GBrainOAuthProvider } from '../core/oauth-provider.ts';
|
||||
import type { SqlQuery } from '../core/oauth-provider.ts';
|
||||
import { summarizeMcpParams } from '../mcp/dispatch.ts';
|
||||
import { loadConfig } from '../core/config.ts';
|
||||
import { buildError, serializeError } from '../core/errors.ts';
|
||||
import { VERSION } from '../version.ts';
|
||||
import * as db from '../core/db.ts';
|
||||
|
||||
@@ -41,19 +44,39 @@ interface ServeHttpOptions {
|
||||
* issuer claim in tokens MUST match the discovery URL clients hit.
|
||||
*/
|
||||
publicUrl?: string;
|
||||
/**
|
||||
* When true, write raw request payloads to mcp_request_log + the admin SSE
|
||||
* feed. Default false: payloads are summarized via dispatch.summarizeMcpParams
|
||||
* (declared keys only, no values, no attacker-controlled key names).
|
||||
*
|
||||
* Operators running gbrain on their own laptop and debugging agent behavior
|
||||
* can flip this on with `--log-full-params`. The flag prints a loud warning
|
||||
* at startup so the privacy posture change is visible.
|
||||
*/
|
||||
logFullParams?: boolean;
|
||||
}
|
||||
|
||||
export async function runServeHttp(engine: BrainEngine, options: ServeHttpOptions) {
|
||||
const { port, tokenTtl, enableDcr, publicUrl } = options;
|
||||
const { port, tokenTtl, enableDcr, publicUrl, logFullParams } = options;
|
||||
const config = loadConfig() || { engine: 'pglite' as const };
|
||||
|
||||
// Get raw SQL connection for OAuth provider
|
||||
const sql = db.getConnection();
|
||||
if (logFullParams) {
|
||||
console.error(
|
||||
'[serve-http] WARNING: --log-full-params writes raw request payloads to mcp_request_log + SSE feed. Disable for shared dashboards or production.',
|
||||
);
|
||||
}
|
||||
|
||||
// Initialize OAuth provider
|
||||
// Get raw SQL connection for OAuth provider
|
||||
const sql = db.getConnection() as SqlQuery;
|
||||
|
||||
// Initialize OAuth provider. F12 cleanup: DCR-disable now flips a
|
||||
// constructor option instead of monkey-patching `_clientsStore` after
|
||||
// construction. Same outcome (no /register endpoint when --enable-dcr
|
||||
// is not passed); cleaner shape for tests and future maintainers.
|
||||
const oauthProvider = new GBrainOAuthProvider({
|
||||
sql: sql as any,
|
||||
sql,
|
||||
tokenTtl,
|
||||
dcrDisabled: !enableDcr,
|
||||
});
|
||||
|
||||
// Sweep expired tokens on startup (non-blocking)
|
||||
@@ -152,6 +175,20 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
// reverse proxies / tunnels; default to localhost for dev.
|
||||
const issuerUrl = new URL(publicUrl || `http://localhost:${port}`);
|
||||
|
||||
// F9: cookie `secure` flag honors both the request's TLS state (req.secure
|
||||
// is set when express trust-proxy lands an X-Forwarded-Proto: https) AND
|
||||
// the operator's declared issuer protocol (so a Cloudflare-tunnel deploy
|
||||
// where the connection inside the tunnel looks like http but the public
|
||||
// URL is https still tags cookies Secure). Without this, an attacker on
|
||||
// the network path could MITM the admin cookie over plaintext.
|
||||
const adminCookie = (req: Request, maxAge: number) => ({
|
||||
httpOnly: true,
|
||||
sameSite: 'strict' as const,
|
||||
secure: req.secure || issuerUrl.protocol === 'https:',
|
||||
maxAge,
|
||||
path: '/admin',
|
||||
});
|
||||
|
||||
const authRouterOptions: any = {
|
||||
provider: oauthProvider,
|
||||
issuerUrl,
|
||||
@@ -159,15 +196,10 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
resourceName: 'GBrain MCP Server',
|
||||
};
|
||||
|
||||
// Disable DCR by removing registerClient from the clients store
|
||||
if (!enableDcr) {
|
||||
// Override the provider's clientsStore to remove registerClient
|
||||
const originalStore = oauthProvider.clientsStore;
|
||||
(oauthProvider as any)._clientsStore = {
|
||||
getClient: originalStore.getClient.bind(originalStore),
|
||||
// No registerClient = DCR disabled
|
||||
};
|
||||
}
|
||||
// F12: DCR disable lives on the provider's constructor option above. The
|
||||
// SDK's mcpAuthRouter reads provider.clientsStore once and only wires up
|
||||
// /register when the store exposes registerClient — so passing dcrDisabled
|
||||
// to the constructor is sufficient. No monkey-patching here.
|
||||
|
||||
const authRouter = mcpAuthRouter(authRouterOptions);
|
||||
|
||||
@@ -232,12 +264,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
const expiresAt = Date.now() + 24 * 60 * 60 * 1000; // 24 hours
|
||||
adminSessions.set(sessionId, expiresAt);
|
||||
|
||||
res.cookie('gbrain_admin', sessionId, {
|
||||
httpOnly: true,
|
||||
sameSite: 'strict',
|
||||
maxAge: 24 * 60 * 60 * 1000,
|
||||
path: '/admin',
|
||||
});
|
||||
res.cookie('gbrain_admin', sessionId, adminCookie(req, 24 * 60 * 60 * 1000));
|
||||
res.json({ status: 'authenticated' });
|
||||
});
|
||||
|
||||
@@ -271,6 +298,15 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
for (const [nonce, expiresAt] of magicLinkNonces) {
|
||||
if (expiresAt < now) magicLinkNonces.delete(nonce);
|
||||
}
|
||||
// F10: bound the live-nonce store too. An attacker with the bootstrap
|
||||
// token (or a misbehaving agent) could mint nonces faster than they
|
||||
// expire. Map iteration order is insertion order, so dropping from the
|
||||
// front gives a simple FIFO eviction matching the consumedNonces pattern.
|
||||
if (magicLinkNonces.size > NONCE_LRU_CAP) {
|
||||
const drop = magicLinkNonces.size - NONCE_LRU_CAP;
|
||||
const it = magicLinkNonces.keys();
|
||||
for (let i = 0; i < drop; i++) magicLinkNonces.delete(it.next().value as string);
|
||||
}
|
||||
// Cap consumedNonces growth — drop oldest entries past the LRU cap.
|
||||
if (consumedNonces.size > NONCE_LRU_CAP) {
|
||||
const drop = consumedNonces.size - NONCE_LRU_CAP;
|
||||
@@ -339,12 +375,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
const sessionExpiresAt = Date.now() + 7 * 24 * 60 * 60 * 1000; // 7 days for magic link
|
||||
adminSessions.set(sessionId, sessionExpiresAt);
|
||||
|
||||
res.cookie('gbrain_admin', sessionId, {
|
||||
httpOnly: true,
|
||||
sameSite: 'strict',
|
||||
maxAge: 7 * 24 * 60 * 60 * 1000,
|
||||
path: '/admin',
|
||||
});
|
||||
res.cookie('gbrain_admin', sessionId, adminCookie(req, 7 * 24 * 60 * 60 * 1000));
|
||||
res.redirect('/admin/');
|
||||
});
|
||||
|
||||
@@ -663,15 +694,31 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
error: (msg: string) => console.error(`[ERROR] ${msg}`),
|
||||
},
|
||||
dryRun: !!(params?.dry_run),
|
||||
// F7: HTTP MCP is the untrusted/agent-facing transport. Stdio MCP at
|
||||
// src/mcp/dispatch.ts:61 sets this; the inlined HTTP context-builder
|
||||
// forgot it for several releases, which let HTTP MCP callers with a
|
||||
// read+write token submit `shell` jobs and execute arbitrary commands
|
||||
// on the host (RCE). The fail-closed contract in operations.ts is the
|
||||
// belt; this is the suspenders.
|
||||
remote: true,
|
||||
auth: authInfo,
|
||||
};
|
||||
|
||||
// F8: redact request payload by default (declared keys only via the
|
||||
// op's `params` allow-list; values + attacker-controlled key names
|
||||
// never written to mcp_request_log or the SSE feed). --log-full-params
|
||||
// bypasses this for operators debugging on their own laptop, with the
|
||||
// startup warning printed earlier.
|
||||
const safeParamsSummary = summarizeMcpParams(name, params);
|
||||
const logParams = logFullParams
|
||||
? (params ? JSON.stringify(params) : null)
|
||||
: (safeParamsSummary ? JSON.stringify(safeParamsSummary) : null);
|
||||
const broadcastParams = logFullParams ? (params || {}) : safeParamsSummary;
|
||||
|
||||
try {
|
||||
const result = await op.handler(ctx, (params || {}) as Record<string, unknown>);
|
||||
const latency = Date.now() - startTime;
|
||||
|
||||
// Log request + broadcast to SSE
|
||||
const logParams = params ? JSON.stringify(params) : null;
|
||||
try {
|
||||
await sql`INSERT INTO mcp_request_log (token_name, agent_name, operation, latency_ms, status, params)
|
||||
VALUES (${authInfo.clientId}, ${agentName}, ${name}, ${latency}, ${'success'}, ${logParams})`;
|
||||
@@ -680,7 +727,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
broadcastEvent({
|
||||
agent: agentName,
|
||||
operation: name,
|
||||
params: params || {},
|
||||
params: broadcastParams,
|
||||
scopes: authInfo.scopes.join(','),
|
||||
latency_ms: latency,
|
||||
status: 'success',
|
||||
@@ -690,10 +737,22 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
return { content: [{ type: 'text', text: JSON.stringify(result) }] };
|
||||
} catch (e) {
|
||||
const latency = Date.now() - startTime;
|
||||
const error = e instanceof OperationError ? e.toJSON() : { error: 'internal_error', message: e instanceof Error ? e.message : 'Unknown error' };
|
||||
|
||||
const errMsg = e instanceof Error ? e.message : 'Unknown error';
|
||||
const logParams = params ? JSON.stringify(params) : null;
|
||||
// F15: unify error envelope. Both OperationError and unexpected
|
||||
// exceptions go through src/core/errors.ts so clients see a single
|
||||
// shape ({class, code, message, hint}). Pre-fix, OperationError
|
||||
// serialized via e.toJSON() and other exceptions used a hand-rolled
|
||||
// {error, message} envelope — a client couldn't pattern-match
|
||||
// reliably across the two.
|
||||
const errorPayload = e instanceof OperationError
|
||||
? buildError({
|
||||
class: 'OperationError',
|
||||
code: e.code,
|
||||
message: e.message,
|
||||
hint: e.suggestion,
|
||||
docs_url: e.docs,
|
||||
})
|
||||
: serializeError(e);
|
||||
const errMsg = errorPayload.message;
|
||||
try {
|
||||
await sql`INSERT INTO mcp_request_log (token_name, agent_name, operation, latency_ms, status, params, error_message)
|
||||
VALUES (${authInfo.clientId}, ${agentName}, ${name}, ${latency}, ${'error'}, ${logParams}, ${errMsg})`;
|
||||
@@ -702,21 +761,37 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
broadcastEvent({
|
||||
agent: agentName,
|
||||
operation: name,
|
||||
params: params || {},
|
||||
params: broadcastParams,
|
||||
scopes: authInfo.scopes.join(','),
|
||||
latency_ms: latency,
|
||||
status: 'error',
|
||||
error: errMsg,
|
||||
error: errorPayload,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
|
||||
return { content: [{ type: 'text', text: JSON.stringify(error) }], isError: true };
|
||||
return { content: [{ type: 'text', text: JSON.stringify({ error: errorPayload }) }], isError: true };
|
||||
}
|
||||
});
|
||||
|
||||
// Use StreamableHTTPServerTransport for stateless request handling
|
||||
const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined as any });
|
||||
await server.connect(transport);
|
||||
await transport.handleRequest(req, res, req.body);
|
||||
// F14: wrap transport setup + handleRequest in try/catch. Without this,
|
||||
// an SDK-level throw (e.g., schema parse failure on a malformed request)
|
||||
// propagates to express's default error handler, which renders an HTML
|
||||
// error page — clients expecting JSON-RPC envelopes break. On
|
||||
// !res.headersSent we emit a minimal JSON 500 so the client at least
|
||||
// gets parseable JSON back.
|
||||
try {
|
||||
const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined as any });
|
||||
await server.connect(transport);
|
||||
await transport.handleRequest(req, res, req.body);
|
||||
} catch (e) {
|
||||
console.error('MCP request handler error:', e instanceof Error ? e.message : e);
|
||||
if (!res.headersSent) {
|
||||
res.status(500).json({
|
||||
error: 'internal_error',
|
||||
message: e instanceof Error ? e.message : 'Unknown error',
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -22,8 +22,15 @@ export async function runServe(engine: BrainEngine, args: string[] = []) {
|
||||
const publicUrlIdx = args.indexOf('--public-url');
|
||||
const publicUrl = publicUrlIdx >= 0 ? args[publicUrlIdx + 1] : undefined;
|
||||
|
||||
// F8 escape hatch: --log-full-params writes raw payloads to mcp_request_log
|
||||
// and the admin SSE feed instead of redacted summaries. Off by default
|
||||
// (privacy-first); operators running gbrain on their own laptop can flip
|
||||
// it on for debug visibility. Loud startup warning fires in serve-http.ts
|
||||
// when set so the posture change is visible in stderr.
|
||||
const logFullParams = args.includes('--log-full-params');
|
||||
|
||||
const { runServeHttp } = await import('./serve-http.ts');
|
||||
await runServeHttp(engine, { port, tokenTtl, enableDcr, publicUrl });
|
||||
await runServeHttp(engine, { port, tokenTtl, enableDcr, publicUrl, logFullParams });
|
||||
} else {
|
||||
console.error('Starting GBrain MCP server (stdio)...');
|
||||
await startMcpServer(engine);
|
||||
|
||||
+116
-32
@@ -22,14 +22,14 @@ import type {
|
||||
import type { OAuthServerProvider, AuthorizationParams } from '@modelcontextprotocol/sdk/server/auth/provider.js';
|
||||
import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/server/auth/clients.js';
|
||||
import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js';
|
||||
import { hashToken, generateToken } from './utils.ts';
|
||||
import { hashToken, generateToken, isUndefinedColumnError } from './utils.ts';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Types
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Raw SQL query function — works with both PGLite and postgres tagged templates */
|
||||
type SqlQuery = (strings: TemplateStringsArray, ...values: unknown[]) => Promise<Record<string, unknown>[]>;
|
||||
export type SqlQuery = (strings: TemplateStringsArray, ...values: unknown[]) => Promise<Record<string, unknown>[]>;
|
||||
|
||||
/**
|
||||
* Convert a JS array to a PostgreSQL array literal for PGLite compat.
|
||||
@@ -111,6 +111,16 @@ interface GBrainOAuthProviderOptions {
|
||||
tokenTtl?: number;
|
||||
/** Default refresh token TTL in seconds (default: 30 days) */
|
||||
refreshTtl?: number;
|
||||
/**
|
||||
* Disable Dynamic Client Registration (RFC 7591) while keeping the rest of
|
||||
* the OAuth surface intact. When true, `clientsStore.registerClient` is not
|
||||
* surfaced to the SDK router, so POST `/register` returns 404 even though
|
||||
* the underlying provider can still register clients programmatically via
|
||||
* `registerClientManual`. Replaces the previous monkey-patching pattern in
|
||||
* serve-http.ts (cleanup, not a security fix — DCR was never reachable
|
||||
* before mcpAuthRouter ran).
|
||||
*/
|
||||
dcrDisabled?: boolean;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -185,17 +195,29 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
private sql: SqlQuery;
|
||||
private _clientsStore: GBrainClientsStore;
|
||||
private readonly dcrDisabled: boolean;
|
||||
private tokenTtl: number;
|
||||
private refreshTtl: number;
|
||||
|
||||
constructor(options: GBrainOAuthProviderOptions) {
|
||||
this.sql = options.sql;
|
||||
this._clientsStore = new GBrainClientsStore(this.sql);
|
||||
this.dcrDisabled = options.dcrDisabled === true;
|
||||
this.tokenTtl = options.tokenTtl || 3600;
|
||||
this.refreshTtl = options.refreshTtl || 30 * 24 * 3600;
|
||||
}
|
||||
|
||||
get clientsStore(): OAuthRegisteredClientsStore {
|
||||
if (this.dcrDisabled) {
|
||||
// Surface getClient only — without registerClient the SDK's mcpAuthRouter
|
||||
// does not wire up the /register DCR endpoint. Replaces the prior
|
||||
// monkey-patch in serve-http.ts; the outcome is identical (DCR off-by-
|
||||
// default), but the API expresses intent on the constructor instead of
|
||||
// requiring callers to mutate `_clientsStore` after construction.
|
||||
return {
|
||||
getClient: this._clientsStore.getClient.bind(this._clientsStore),
|
||||
} as OAuthRegisteredClientsStore;
|
||||
}
|
||||
return this._clientsStore;
|
||||
}
|
||||
|
||||
@@ -230,13 +252,18 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
}
|
||||
|
||||
async challengeForAuthorizationCode(
|
||||
_client: OAuthClientInformationFull,
|
||||
client: OAuthClientInformationFull,
|
||||
authorizationCode: string,
|
||||
): Promise<string> {
|
||||
const codeHash = hashToken(authorizationCode);
|
||||
// F1 hardening: bind client_id atomically so a wrong client cannot read
|
||||
// another client's PKCE challenge. Pre-fix the SELECT didn't filter on
|
||||
// client_id at all.
|
||||
const rows = await this.sql`
|
||||
SELECT code_challenge FROM oauth_codes
|
||||
WHERE code_hash = ${codeHash} AND expires_at > ${Math.floor(Date.now() / 1000)}
|
||||
WHERE code_hash = ${codeHash}
|
||||
AND client_id = ${client.client_id}
|
||||
AND expires_at > ${Math.floor(Date.now() / 1000)}
|
||||
`;
|
||||
if (rows.length === 0) throw new Error('Authorization code not found or expired');
|
||||
return rows[0].code_challenge as string;
|
||||
@@ -246,27 +273,44 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
client: OAuthClientInformationFull,
|
||||
authorizationCode: string,
|
||||
_codeVerifier?: string,
|
||||
_redirectUri?: string,
|
||||
redirectUri?: string,
|
||||
resource?: URL,
|
||||
): Promise<OAuthTokens> {
|
||||
const codeHash = hashToken(authorizationCode);
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
|
||||
// Atomic single-use: DELETE...RETURNING in one statement closes the
|
||||
// TOCTOU window. RFC 6749 §10.5 requires auth codes be single-use; the
|
||||
// earlier SELECT-then-DELETE pattern let two concurrent token requests
|
||||
// both pass the SELECT before either ran the DELETE, issuing two valid
|
||||
// token pairs from one code. With RETURNING, the second request gets
|
||||
// zero rows back and fails cleanly. See CSO finding #2.
|
||||
const rows = await this.sql`
|
||||
DELETE FROM oauth_codes
|
||||
WHERE code_hash = ${codeHash} AND expires_at > ${now}
|
||||
RETURNING client_id, scopes, resource
|
||||
`;
|
||||
// F1 + F7c hardening: bind client_id AND redirect_uri atomically into the
|
||||
// DELETE WHERE clause. RFC 6749 §10.5 requires auth codes be single-use;
|
||||
// RFC 6749 §4.1.3 requires the token endpoint validate redirect_uri
|
||||
// matches the value sent at /authorize. The previous SELECT-then-compare
|
||||
// pattern (a) burned the code on the wrong-client path so the legitimate
|
||||
// client could not retry, and (b) ignored redirect_uri on exchange
|
||||
// entirely. With RETURNING, the second request — or any wrong-client /
|
||||
// wrong-redirect-uri attempt — gets zero rows back and fails cleanly.
|
||||
// The legitimate client's code stays available for one valid redemption.
|
||||
//
|
||||
// Use `redirectUri !== undefined` rather than truthy — an attacker
|
||||
// submitting `redirect_uri=""` (empty string) at /token would otherwise
|
||||
// hit the falsy branch and bypass the binding entirely.
|
||||
const rows = redirectUri !== undefined
|
||||
? await this.sql`
|
||||
DELETE FROM oauth_codes
|
||||
WHERE code_hash = ${codeHash}
|
||||
AND client_id = ${client.client_id}
|
||||
AND redirect_uri = ${redirectUri}
|
||||
AND expires_at > ${now}
|
||||
RETURNING client_id, scopes, resource
|
||||
`
|
||||
: await this.sql`
|
||||
DELETE FROM oauth_codes
|
||||
WHERE code_hash = ${codeHash}
|
||||
AND client_id = ${client.client_id}
|
||||
AND expires_at > ${now}
|
||||
RETURNING client_id, scopes, resource
|
||||
`;
|
||||
if (rows.length === 0) throw new Error('Authorization code not found or expired');
|
||||
|
||||
const codeRow = rows[0];
|
||||
if (codeRow.client_id !== client.client_id) throw new Error('Client mismatch');
|
||||
|
||||
// Issue tokens
|
||||
const scopes = (codeRow.scopes as string[]) || [];
|
||||
@@ -286,26 +330,42 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
const tokenHash = hashToken(refreshToken);
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
|
||||
// Atomic rotation: DELETE...RETURNING closes the TOCTOU window. RFC 6749
|
||||
// §10.4 detection of stolen refresh tokens depends on second-use failure;
|
||||
// the earlier SELECT-then-DELETE pattern let attacker + victim both
|
||||
// succeed, defeating that signal. See CSO finding #3.
|
||||
// F2 hardening: bind client_id atomically into the DELETE WHERE clause.
|
||||
// RFC 6749 §10.4 detection of stolen refresh tokens depends on second-use
|
||||
// failure. The previous SELECT-then-DELETE pattern + post-hoc client
|
||||
// compare let an attacker who guessed/stole a refresh token burn it on
|
||||
// the wrong-client path, defeating the stolen-token signal for the
|
||||
// legitimate client. With the predicate in the DELETE, wrong-client
|
||||
// attempts get zero rows back; the legitimate client retains the row
|
||||
// for one valid rotation.
|
||||
const rows = await this.sql`
|
||||
DELETE FROM oauth_tokens
|
||||
WHERE token_hash = ${tokenHash} AND token_type = 'refresh'
|
||||
WHERE token_hash = ${tokenHash}
|
||||
AND token_type = 'refresh'
|
||||
AND client_id = ${client.client_id}
|
||||
RETURNING client_id, scopes, expires_at
|
||||
`;
|
||||
if (rows.length === 0) throw new Error('Refresh token not found');
|
||||
|
||||
const row = rows[0];
|
||||
if (row.client_id !== client.client_id) throw new Error('Client mismatch');
|
||||
// NULL expires_at is treated as expired (fail-closed). Schema permits NULL
|
||||
// even though issueTokens always sets it, so a corrupt or hand-modified row
|
||||
// can't ride past validation.
|
||||
const expiresAt = coerceTimestamp(row.expires_at);
|
||||
if (expiresAt === undefined || expiresAt < now) throw new Error('Refresh token expired');
|
||||
|
||||
const tokenScopes = scopes || (row.scopes as string[]) || [];
|
||||
// F3 hardening: requested scopes on refresh MUST be a subset of the
|
||||
// original grant on this refresh token's row. RFC 6749 §6: "the scope of
|
||||
// the access token … MUST NOT include any scope not originally granted by
|
||||
// the resource owner." Scope is checked against the row's scopes (the
|
||||
// grant), NOT against the client's currently-allowed scopes (which can
|
||||
// expand later). Omitted scope (`undefined`) inherits the original grant
|
||||
// verbatim and stays distinct from an explicit empty array.
|
||||
const grantedScopes = (row.scopes as string[]) || [];
|
||||
if (scopes && scopes.some(s => !grantedScopes.includes(s))) {
|
||||
throw new Error('Requested scope exceeds refresh token grant');
|
||||
}
|
||||
const tokenScopes = scopes ?? grantedScopes;
|
||||
return this.issueTokens(client.client_id, tokenScopes, resource, true);
|
||||
}
|
||||
|
||||
@@ -378,11 +438,21 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
async revokeToken(
|
||||
_client: OAuthClientInformationFull,
|
||||
client: OAuthClientInformationFull,
|
||||
request: OAuthTokenRevocationRequest,
|
||||
): Promise<void> {
|
||||
const tokenHash = hashToken(request.token);
|
||||
await this.sql`DELETE FROM oauth_tokens WHERE token_hash = ${tokenHash}`;
|
||||
// F4 hardening: bind client_id so a client can only revoke its own
|
||||
// tokens. RFC 7009 §2.1: "The authorization server first validates the
|
||||
// client credentials … and then verifies whether the token was issued
|
||||
// to the client making the revocation request." Pre-fix, any
|
||||
// authenticated client that knew (or guessed) another client's token
|
||||
// hash could revoke it.
|
||||
await this.sql`
|
||||
DELETE FROM oauth_tokens
|
||||
WHERE token_hash = ${tokenHash}
|
||||
AND client_id = ${client.client_id}
|
||||
`;
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -397,13 +467,19 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
const client = await this._clientsStore.getClient(clientId);
|
||||
if (!client) throw new Error('Client not found');
|
||||
|
||||
// Check if client has been revoked (soft-deleted)
|
||||
// Check if client has been revoked (soft-deleted). The deleted_at column
|
||||
// is recent — pre-migration brains don't have it, so the probe must
|
||||
// tolerate that one specific failure mode without swallowing real errors
|
||||
// (lock timeouts, network blips, auth failures).
|
||||
try {
|
||||
const [revoked] = await this.sql`SELECT deleted_at FROM oauth_clients WHERE client_id = ${clientId} AND deleted_at IS NOT NULL`;
|
||||
if (revoked) throw new Error('Client has been revoked');
|
||||
} catch (e) {
|
||||
// deleted_at column may not exist on PGLite/older schemas — skip check
|
||||
// F5 hardening: surface anything that ISN'T a missing-column error.
|
||||
// Bare `catch {}` masked DB outages as "client not revoked" — fail-open
|
||||
// posture in a security-sensitive code path.
|
||||
if (e instanceof Error && e.message === 'Client has been revoked') throw e;
|
||||
if (!isUndefinedColumnError(e, 'deleted_at')) throw e;
|
||||
}
|
||||
|
||||
// Check grant type first (before verifying secret)
|
||||
@@ -427,7 +503,11 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
try {
|
||||
const ttlRows = await this.sql`SELECT token_ttl FROM oauth_clients WHERE client_id = ${clientId}`;
|
||||
if (ttlRows.length > 0 && ttlRows[0].token_ttl) clientTtl = Number(ttlRows[0].token_ttl);
|
||||
} catch { /* token_ttl column doesn't exist — use server default */ }
|
||||
} catch (e) {
|
||||
// F5 hardening: same posture as the deleted_at probe above. Only the
|
||||
// "column doesn't exist" path is a non-fatal fall-through.
|
||||
if (!isUndefinedColumnError(e, 'token_ttl')) throw e;
|
||||
}
|
||||
|
||||
// Client credentials: access token only, NO refresh token (RFC 6749 4.4.3)
|
||||
return this.issueTokens(clientId, grantedScopes, undefined, false, clientTtl);
|
||||
@@ -439,13 +519,17 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
|
||||
async sweepExpiredTokens(): Promise<number> {
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
// F6 hardening: postgres.js and PGLite expose deleted-row count on
|
||||
// different shapes; `(result as any).count` returned 0 on at least one
|
||||
// engine even when rows were deleted, and codes were never counted at
|
||||
// all. RETURNING 1 + array length is portable across both engines.
|
||||
const result = await this.sql`
|
||||
DELETE FROM oauth_tokens WHERE expires_at < ${now}
|
||||
DELETE FROM oauth_tokens WHERE expires_at < ${now} RETURNING 1
|
||||
`;
|
||||
const deletedCodes = await this.sql`
|
||||
DELETE FROM oauth_codes WHERE expires_at < ${now}
|
||||
DELETE FROM oauth_codes WHERE expires_at < ${now} RETURNING 1
|
||||
`;
|
||||
return (result as any).count || 0;
|
||||
return result.length + deletedCodes.length;
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
+14
-8
@@ -214,9 +214,12 @@ export interface OperationContext {
|
||||
* confinement when remote=true and allow unrestricted local-filesystem access
|
||||
* when remote=false.
|
||||
*
|
||||
* When unset, operations MUST default to the stricter (remote=true) behavior.
|
||||
* REQUIRED as of the F7b hardening — the type system is the first line of defense.
|
||||
* Every transport (CLI / stdio MCP / HTTP MCP / subagent dispatcher) sets this
|
||||
* explicitly. Consumers still treat anything that isn't strictly `false` as
|
||||
* remote/untrusted (defense in depth in case the type is bypassed via cast).
|
||||
*/
|
||||
remote?: boolean;
|
||||
remote: boolean;
|
||||
/**
|
||||
* Subagent runtime context (v0.16+). Set by the subagent tool dispatcher when
|
||||
* dispatching an op as a tool call from an LLM loop. Used to enforce per-op
|
||||
@@ -407,7 +410,7 @@ const put_page: Operation = {
|
||||
const trustedWorkspace = ctx.viaSubagent === true
|
||||
&& Array.isArray(ctx.allowedSlugPrefixes)
|
||||
&& ctx.allowedSlugPrefixes.length > 0;
|
||||
if (ctx.remote === true && !trustedWorkspace) {
|
||||
if (ctx.remote !== false && !trustedWorkspace) {
|
||||
autoLinks = { skipped: 'remote' };
|
||||
autoTimeline = { skipped: 'remote' };
|
||||
} else if (result.parsedPage) {
|
||||
@@ -1388,16 +1391,19 @@ const submit_job: Operation = {
|
||||
// GBRAIN_ALLOW_SHELL_JOBS env flag — even if that flag is on, MCP callers
|
||||
// cannot submit protected-type jobs.
|
||||
const { isProtectedJobName } = await import('./minions/protected-names.ts');
|
||||
if (ctx.remote && isProtectedJobName(name)) {
|
||||
// F7b fail-closed: anything that is not strictly false (i.e., remote=true OR
|
||||
// the field somehow leaks in undefined despite the required type) rejects
|
||||
// protected job submissions. Closes the HTTP MCP shell-job RCE that surfaced
|
||||
// when the HTTP transport's OperationContext literal forgot to set remote.
|
||||
if (ctx.remote !== false && isProtectedJobName(name)) {
|
||||
throw new OperationError('permission_denied', `'${name}' jobs cannot be submitted over MCP (CLI-only for security)`);
|
||||
}
|
||||
|
||||
const { MinionQueue } = await import('./minions/queue.ts');
|
||||
const queue = new MinionQueue(ctx.engine);
|
||||
// Trusted flag set only when this is a local (non-remote) submission. When
|
||||
// remote=true, the guard above has already thrown for protected names, so
|
||||
// passing undefined here is safe for any non-protected name that slips by.
|
||||
const trusted = !ctx.remote && isProtectedJobName(name) ? { allowProtectedSubmit: true } : undefined;
|
||||
// Trusted flag fires ONLY for an explicit local CLI submission of a protected
|
||||
// name. Strict `=== false` so an untyped/cast context can't escalate.
|
||||
const trusted = ctx.remote === false && isProtectedJobName(name) ? { allowProtectedSubmit: true } : undefined;
|
||||
return queue.add(name, (p.data as Record<string, unknown>) || {}, {
|
||||
queue: (p.queue as string) || 'default',
|
||||
priority: (p.priority as number) || 0,
|
||||
|
||||
@@ -109,6 +109,33 @@ export function parseEmbedding(value: unknown): Float32Array | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect a Postgres "undefined column" error (SQLSTATE 42703) without depending
|
||||
* on the postgres.js driver-specific error class.
|
||||
*
|
||||
* Used for forward-compat probes — code that does `SELECT foo FROM bar` against
|
||||
* schemas where `foo` may not exist yet on legacy installs (column was added in
|
||||
* a later migration). Bare `try { ... } catch {}` swallows EVERY error
|
||||
* (network blips, lock timeouts, auth failures) which masks real bugs as
|
||||
* "column missing." This predicate keeps the probe narrow.
|
||||
*
|
||||
* Matches on either:
|
||||
* - SQLSTATE code `42703` (postgres.js sets this on the error)
|
||||
* - the column name appearing in the message alongside a "does not exist" /
|
||||
* "no such column" / "undefined column" clause (PGLite + various driver
|
||||
* wraps)
|
||||
*
|
||||
* Anything else falls through and the caller MUST re-throw.
|
||||
*/
|
||||
export function isUndefinedColumnError(error: unknown, column: string): boolean {
|
||||
const code = typeof error === 'object' && error !== null && 'code' in error
|
||||
? String((error as { code?: unknown }).code)
|
||||
: '';
|
||||
if (code === '42703') return true;
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
return message.includes(column) && /does not exist|no such column|undefined column/i.test(message);
|
||||
}
|
||||
|
||||
let _tryParseEmbeddingWarned = false;
|
||||
|
||||
/**
|
||||
|
||||
@@ -23,6 +23,101 @@ export interface DispatchOpts {
|
||||
logger?: OperationContext['logger'];
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a privacy-safe summary of MCP request params for logging + the admin
|
||||
* SSE feed.
|
||||
*
|
||||
* The previous default of `JSON.stringify(params)` wrote raw payloads —
|
||||
* page bodies, search queries, file paths — into `mcp_request_log` and
|
||||
* broadcast them to every connected admin browser. For a personal-knowledge
|
||||
* brain those payloads include private notes about real people / deals /
|
||||
* companies, retained indefinitely.
|
||||
*
|
||||
* The redactor returns the SHAPE of the request (what op was called, which
|
||||
* declared params were passed, approximate size) without any of the values.
|
||||
*
|
||||
* Hardening note (codex C8): a naive "dump all submitted keys" summary still
|
||||
* leaks via attacker-controlled key names — a caller can submit
|
||||
* `put_page {"wiki/people/sensitive_name": "..."}` and the key becomes a
|
||||
* persistent log entry. To prevent this, we intersect submitted keys
|
||||
* against the operation's declared `params` allow-list (the same definition
|
||||
* `validateParams` reads). Anything outside the allow-list is counted but
|
||||
* not named.
|
||||
*
|
||||
* Operators who want full payloads for debugging set `--log-full-params` on
|
||||
* `gbrain serve --http`; that path bypasses this helper and writes the raw
|
||||
* JSON, with a loud startup warning.
|
||||
*/
|
||||
export interface ParamSummary {
|
||||
redacted: true;
|
||||
kind: 'array' | 'object' | string;
|
||||
declared_keys?: string[];
|
||||
unknown_key_count?: number;
|
||||
length?: number;
|
||||
approx_bytes?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Round a byte count UP to the nearest 1KB so the redacted summary keeps a
|
||||
* coarse size signal without enabling a size-based side channel.
|
||||
*
|
||||
* Why bucketing matters: the previous shape published `approx_bytes` as the
|
||||
* exact JSON.stringify(params).length. An attacker who can submit
|
||||
* `put_page` with a known prefix and observe the resulting log entry
|
||||
* could binary-search the byte length of secret content (the body the
|
||||
* legitimate user just wrote) via repeated probes. Bucketing to 1KB
|
||||
* resolution destroys that channel while preserving the operator-useful
|
||||
* "roughly how large was the request" signal.
|
||||
*/
|
||||
function bucketBytes(n: number | undefined): number | undefined {
|
||||
if (n === undefined || !Number.isFinite(n)) return undefined;
|
||||
if (n <= 0) return 0;
|
||||
const KB = 1024;
|
||||
return Math.ceil(n / KB) * KB;
|
||||
}
|
||||
|
||||
export function summarizeMcpParams(opName: string, params: unknown): ParamSummary | null {
|
||||
if (params == null) return null;
|
||||
|
||||
let approxBytes: number | undefined;
|
||||
try { approxBytes = bucketBytes(JSON.stringify(params).length); } catch { approxBytes = undefined; }
|
||||
|
||||
if (Array.isArray(params)) {
|
||||
return {
|
||||
redacted: true,
|
||||
kind: 'array',
|
||||
length: params.length,
|
||||
...(approxBytes !== undefined ? { approx_bytes: approxBytes } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
if (typeof params === 'object') {
|
||||
const submittedKeys = Object.keys(params as Record<string, unknown>);
|
||||
const op = operations.find(o => o.name === opName);
|
||||
const allowList = op ? new Set(Object.keys(op.params)) : new Set<string>();
|
||||
const declared: string[] = [];
|
||||
let unknown = 0;
|
||||
for (const k of submittedKeys) {
|
||||
if (allowList.has(k)) declared.push(k);
|
||||
else unknown += 1;
|
||||
}
|
||||
declared.sort();
|
||||
return {
|
||||
redacted: true,
|
||||
kind: 'object',
|
||||
declared_keys: declared,
|
||||
unknown_key_count: unknown,
|
||||
...(approxBytes !== undefined ? { approx_bytes: approxBytes } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
redacted: true,
|
||||
kind: typeof params,
|
||||
...(approxBytes !== undefined ? { approx_bytes: approxBytes } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
/** Validate required params exist and have the expected type. Returns null on success, error message on failure. */
|
||||
export function validateParams(op: Operation, params: Record<string, unknown>): string | null {
|
||||
for (const [key, def] of Object.entries(op.params)) {
|
||||
|
||||
@@ -40,6 +40,9 @@ function makeContext(): OperationContext {
|
||||
config: { engine: 'pglite' } as any,
|
||||
logger: { info: () => {}, warn: () => {}, error: () => {} },
|
||||
dryRun: false,
|
||||
// E2E graph quality simulates local-CLI writes (auto-link / timeline run).
|
||||
// After F7b made `remote` required this needs to be explicit.
|
||||
remote: false,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -722,4 +722,62 @@ describeE2E('serve-http OAuth 2.1 E2E (v0.26.1 + v0.26.2 + v0.26.3)', () => {
|
||||
});
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
// F7 + F7b: HTTP MCP shell-job RCE regression
|
||||
// =========================================================================
|
||||
//
|
||||
// The headline trust-boundary fix. Pre-fix, the inlined OperationContext
|
||||
// literal in serve-http.ts forgot to set `remote: true`, which meant
|
||||
// operations.ts:1391's protected-job-name guard (`if (ctx.remote && ...)`)
|
||||
// saw a falsy undefined and skipped. An HTTP MCP caller with a write-scoped
|
||||
// token could then submit `{name: "shell", params: {cmd: "id"}}` over /mcp
|
||||
// and execute arbitrary commands on the gbrain host.
|
||||
//
|
||||
// The fix is two-layered:
|
||||
// 1) F7 — serve-http.ts sets `remote: true` explicitly.
|
||||
// 2) F7b — operations.ts:1391 + :1400 use `ctx.remote !== false` /
|
||||
// `ctx.remote === false` so undefined fails closed even if a
|
||||
// future transport bypasses the type via cast.
|
||||
//
|
||||
// Together they close the path even if either layer regresses alone.
|
||||
|
||||
test('F7: HTTP MCP cannot submit shell jobs (RCE regression)', async () => {
|
||||
const { access_token } = await mintToken('read write');
|
||||
const res = await mcpCall(access_token, 'tools/call', {
|
||||
name: 'submit_job',
|
||||
arguments: { name: 'shell', data: { cmd: 'id' } },
|
||||
});
|
||||
|
||||
const body = await res.text();
|
||||
// Must reject. Either HTTP 4xx, or a JSON-RPC envelope carrying an
|
||||
// OperationError with code permission_denied. The exact wire shape
|
||||
// depends on SDK error mapping — assert the negative invariant
|
||||
// (no command executed) and the positive invariant (rejection signal).
|
||||
const rejected =
|
||||
res.status >= 400 ||
|
||||
body.includes('permission_denied') ||
|
||||
body.includes('cannot be submitted over MCP');
|
||||
expect(rejected).toBe(true);
|
||||
|
||||
// Negative: response must NOT contain a successful submit_job result
|
||||
// (which would surface a job_id field). If a job ID came back the
|
||||
// privesc landed.
|
||||
expect(body).not.toMatch(/"job_id"\s*:\s*"?\d+/);
|
||||
}, 15_000);
|
||||
|
||||
test('F7: HTTP MCP cannot submit subagent jobs (protected name)', async () => {
|
||||
const { access_token } = await mintToken('read write');
|
||||
const res = await mcpCall(access_token, 'tools/call', {
|
||||
name: 'submit_job',
|
||||
arguments: { name: 'subagent', data: { prompt: 'noop' } },
|
||||
});
|
||||
const body = await res.text();
|
||||
const rejected =
|
||||
res.status >= 400 ||
|
||||
body.includes('permission_denied') ||
|
||||
body.includes('cannot be submitted over MCP');
|
||||
expect(rejected).toBe(true);
|
||||
expect(body).not.toMatch(/"job_id"\s*:\s*"?\d+/);
|
||||
}, 15_000);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
/**
|
||||
* Unit tests for `summarizeMcpParams` — the F8/codex-C8 redactor that strips
|
||||
* raw values AND attacker-controlled key names from `mcp_request_log` + the
|
||||
* admin SSE feed.
|
||||
*
|
||||
* Three invariants pinned:
|
||||
* 1. Declared keys (the ones the operation accepts per its `params`
|
||||
* definition) survive into `declared_keys` for debug visibility.
|
||||
* 2. Unknown keys are counted but NOT named. A caller submitting a key
|
||||
* like `wiki/people/sensitive_name` cannot leak that string into the
|
||||
* log via the redactor's output.
|
||||
* 3. The redactor never echoes raw values for any key. Privacy-positive
|
||||
* default; --log-full-params is the documented escape hatch and lives
|
||||
* in serve-http.ts, not here.
|
||||
*/
|
||||
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { summarizeMcpParams, type ParamSummary } from '../src/mcp/dispatch.ts';
|
||||
|
||||
describe('summarizeMcpParams — declared-keys allow-list', () => {
|
||||
test('declared keys are preserved alphabetically', () => {
|
||||
// put_page declares params: slug, content (and a few others). The summary
|
||||
// should list both, sorted, without any value bytes.
|
||||
const summary = summarizeMcpParams('put_page', {
|
||||
slug: 'people/alice',
|
||||
content: '# Alice\n\nA private note.',
|
||||
}) as ParamSummary;
|
||||
|
||||
expect(summary).not.toBeNull();
|
||||
expect(summary.redacted).toBe(true);
|
||||
expect(summary.kind).toBe('object');
|
||||
expect(summary.declared_keys).toEqual(expect.arrayContaining(['slug', 'content']));
|
||||
// Sorted property — fixed order across runs makes log diffs reviewable.
|
||||
const sorted = [...(summary.declared_keys ?? [])].sort();
|
||||
expect(summary.declared_keys).toEqual(sorted);
|
||||
expect(summary.unknown_key_count).toBe(0);
|
||||
expect(summary.approx_bytes).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('unknown keys are counted but never named', () => {
|
||||
// Codex C8 attacker scenario: attacker controls key names and stuffs
|
||||
// sensitive data into them. Privacy posture requires we count them
|
||||
// without echoing the names anywhere in the summary.
|
||||
const sensitiveKey = 'wiki/people/SENSITIVE_TARGET_NAME';
|
||||
const summary = summarizeMcpParams('put_page', {
|
||||
slug: 'people/alice',
|
||||
[sensitiveKey]: 'attacker-controlled value',
|
||||
another_unknown: 'whatever',
|
||||
}) as ParamSummary;
|
||||
|
||||
// Hard invariant: the sensitive name MUST NOT appear in any field of
|
||||
// the summary.
|
||||
const serialized = JSON.stringify(summary);
|
||||
expect(serialized).not.toContain('SENSITIVE_TARGET_NAME');
|
||||
expect(serialized).not.toContain('attacker-controlled value');
|
||||
|
||||
// unknown_key_count counts the keys we couldn't validate.
|
||||
expect(summary.unknown_key_count).toBe(2);
|
||||
// Declared keys still surface — slug is part of put_page's allow-list.
|
||||
expect(summary.declared_keys).toContain('slug');
|
||||
});
|
||||
|
||||
test('unknown op name produces all-unknown summary (zero declared keys)', () => {
|
||||
// If the operation name doesn't resolve, the allow-list is empty and
|
||||
// every submitted key is unknown. Privacy stays intact: no key names
|
||||
// surface in the output.
|
||||
const summary = summarizeMcpParams('this_op_does_not_exist', {
|
||||
foo: 'a',
|
||||
bar: 'b',
|
||||
}) as ParamSummary;
|
||||
expect(summary.declared_keys).toEqual([]);
|
||||
expect(summary.unknown_key_count).toBe(2);
|
||||
const serialized = JSON.stringify(summary);
|
||||
expect(serialized).not.toContain('foo');
|
||||
expect(serialized).not.toContain('bar');
|
||||
});
|
||||
|
||||
test('null/undefined params return null (caller writes SQL NULL)', () => {
|
||||
expect(summarizeMcpParams('put_page', null)).toBeNull();
|
||||
expect(summarizeMcpParams('put_page', undefined)).toBeNull();
|
||||
});
|
||||
|
||||
test('array params summarize length without elements', () => {
|
||||
const summary = summarizeMcpParams('put_page', [1, 2, 3, 'sensitive']) as ParamSummary;
|
||||
expect(summary.kind).toBe('array');
|
||||
expect(summary.length).toBe(4);
|
||||
const serialized = JSON.stringify(summary);
|
||||
expect(serialized).not.toContain('sensitive');
|
||||
});
|
||||
|
||||
test('primitive params summarize kind without value', () => {
|
||||
const summary = summarizeMcpParams('put_page', 'a sensitive string') as ParamSummary;
|
||||
expect(summary.kind).toBe('string');
|
||||
const serialized = JSON.stringify(summary);
|
||||
expect(serialized).not.toContain('sensitive');
|
||||
});
|
||||
|
||||
test('approx_bytes is bucketed to 1KB to defeat size-based side-channels', () => {
|
||||
// D16 / adversarial-review fix: the previous shape exposed exact byte
|
||||
// length of every request, enabling an attacker to binary-search the
|
||||
// size of secret content via repeated probes (submit put_page with a
|
||||
// known prefix, observe approx_bytes, narrow the unknown-suffix size).
|
||||
// Bucketing to 1KB resolution destroys the side-channel while keeping
|
||||
// the operator-useful "roughly how big" signal.
|
||||
const tiny = summarizeMcpParams('put_page', { slug: 'a' }) as ParamSummary;
|
||||
// Tiny payload (~14 bytes) rounds up to the first 1KB bucket.
|
||||
expect(tiny.approx_bytes).toBe(1024);
|
||||
|
||||
// 2KB payload should fall in either the 2KB or 3KB bucket depending on
|
||||
// exact serialization length — the invariant is that it's a multiple of
|
||||
// 1024, NOT the literal byte count.
|
||||
const medium = summarizeMcpParams('put_page', {
|
||||
slug: 'people/test',
|
||||
content: 'x'.repeat(2000),
|
||||
}) as ParamSummary;
|
||||
expect(medium.approx_bytes).toBeDefined();
|
||||
expect(medium.approx_bytes! % 1024).toBe(0);
|
||||
// Bucket cannot be less than the actual size and must round UP, so
|
||||
// a ~2KB payload lands in the 2KB or 3KB bucket.
|
||||
expect(medium.approx_bytes!).toBeGreaterThanOrEqual(2048);
|
||||
});
|
||||
});
|
||||
@@ -614,3 +614,350 @@ describe('redirect_uri validation (DCR)', () => {
|
||||
expect(stored!.redirect_uris[0]).toBe(trickyUri);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// F1 / F4 — Wrong-client cross-tenant attempts
|
||||
// ---------------------------------------------------------------------------
|
||||
//
|
||||
// The atomic client_id binding lives in the DELETE WHERE clause for auth
|
||||
// codes (exchange + challenge), refresh tokens (rotate), and revocations.
|
||||
// Without it, any authenticated client that knew/guessed another client's
|
||||
// hash could (a) consume the code/refresh on the wrong-client path,
|
||||
// burning it for the legitimate client, or (b) revoke another client's
|
||||
// tokens. These tests pin the negative invariant — wrong client fails —
|
||||
// AND the positive invariant — owner still succeeds atomically afterward.
|
||||
|
||||
describe('F1/F4 cross-client isolation', () => {
|
||||
test('wrong client cannot consume another client authorization code', async () => {
|
||||
const { clientId: ownerId } = await provider.registerClientManual(
|
||||
'authcode-owner-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const { clientId: attackerId } = await provider.registerClientManual(
|
||||
'authcode-attacker-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const owner = (await provider.clientsStore.getClient(ownerId))!;
|
||||
const attacker = (await provider.clientsStore.getClient(attackerId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(owner, {
|
||||
codeChallenge: 'challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
|
||||
// Attacker holding the same code MUST be rejected.
|
||||
await expect(provider.exchangeAuthorizationCode(attacker, code)).rejects.toThrow();
|
||||
|
||||
// The atomic predicate's payoff: the legitimate owner can STILL redeem
|
||||
// the code afterward. Without it, the attacker would have burned the
|
||||
// row in the DELETE and the owner's redemption would 404.
|
||||
const tokens = await provider.exchangeAuthorizationCode(owner, code);
|
||||
expect(tokens.access_token).toStartWith('gbrain_at_');
|
||||
});
|
||||
|
||||
test('wrong client cannot read another client PKCE challenge', async () => {
|
||||
const { clientId: ownerId } = await provider.registerClientManual(
|
||||
'challenge-owner-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const { clientId: attackerId } = await provider.registerClientManual(
|
||||
'challenge-attacker-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const owner = (await provider.clientsStore.getClient(ownerId))!;
|
||||
const attacker = (await provider.clientsStore.getClient(attackerId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(owner, {
|
||||
codeChallenge: 'owner-challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
|
||||
await expect(provider.challengeForAuthorizationCode!(attacker, code)).rejects.toThrow();
|
||||
await expect(provider.challengeForAuthorizationCode!(owner, code)).resolves.toBe('owner-challenge');
|
||||
});
|
||||
|
||||
test('wrong client cannot revoke another client token', async () => {
|
||||
const { clientId: ownerId, clientSecret: ownerSecret } = await provider.registerClientManual(
|
||||
'revoke-owner-test', ['client_credentials'], 'read',
|
||||
);
|
||||
const { clientId: attackerId } = await provider.registerClientManual(
|
||||
'revoke-attacker-test', ['client_credentials'], 'read',
|
||||
);
|
||||
const tokens = await provider.exchangeClientCredentials(ownerId, ownerSecret, 'read');
|
||||
const attacker = (await provider.clientsStore.getClient(attackerId))!;
|
||||
|
||||
// Attacker tries to revoke owner's token. revokeToken returns void
|
||||
// (silent on no-op), so we assert the token still verifies after.
|
||||
await provider.revokeToken!(attacker, { token: tokens.access_token });
|
||||
const authInfo = await provider.verifyAccessToken(tokens.access_token);
|
||||
expect(authInfo.clientId).toBe(ownerId);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// F2 + F3 — Refresh-token cross-client isolation + scope subset
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('F2/F3 refresh hardening', () => {
|
||||
test('wrong client cannot burn another client refresh token', async () => {
|
||||
const { clientId: ownerId } = await provider.registerClientManual(
|
||||
'refresh-owner-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const { clientId: attackerId } = await provider.registerClientManual(
|
||||
'refresh-attacker-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const owner = (await provider.clientsStore.getClient(ownerId))!;
|
||||
const attacker = (await provider.clientsStore.getClient(attackerId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(owner, {
|
||||
codeChallenge: 'challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
const tokens = await provider.exchangeAuthorizationCode(owner, code);
|
||||
|
||||
// Attacker rejected.
|
||||
await expect(provider.exchangeRefreshToken(attacker, tokens.refresh_token!)).rejects.toThrow();
|
||||
|
||||
// Owner still redeems atomically — the row was not burned by the
|
||||
// attacker's attempt.
|
||||
const rotated = await provider.exchangeRefreshToken(owner, tokens.refresh_token!);
|
||||
expect(rotated.access_token).toStartWith('gbrain_at_');
|
||||
expect(rotated.refresh_token).toBeDefined();
|
||||
expect(rotated.refresh_token).not.toBe(tokens.refresh_token);
|
||||
});
|
||||
|
||||
test('refresh cannot request scopes outside the original grant (F3)', async () => {
|
||||
// Client allowed scopes 'read write', but the user only authorized 'read'.
|
||||
// The refresh token row carries the granted scope, NOT the client's
|
||||
// currently-allowed scopes (codex C9). Requesting 'write' on refresh
|
||||
// must fail even though the client could mint a fresh write-scoped
|
||||
// token via a new authorize round trip.
|
||||
const { clientId } = await provider.registerClientManual(
|
||||
'refresh-scope-test', ['authorization_code'], 'read write',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const client = (await provider.clientsStore.getClient(clientId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(client, {
|
||||
codeChallenge: 'challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
const tokens = await provider.exchangeAuthorizationCode(client, code);
|
||||
|
||||
// Attempt to escalate to write — must reject.
|
||||
await expect(
|
||||
provider.exchangeRefreshToken(client, tokens.refresh_token!, ['read', 'write']),
|
||||
).rejects.toThrow(/scope/i);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// F5 — fail-loud column probes (was: bare catch{})
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('F5 verifyAccessToken / client_credentials column probes', () => {
|
||||
test('non-schema SQL failures are not swallowed by client credentials soft-delete probe', async () => {
|
||||
// Synthesize a non-schema error (SQLSTATE 57P01 = admin_shutdown) and
|
||||
// make sure the catch block re-throws instead of silently treating
|
||||
// the client as not-revoked. Without the predicate this throw used to
|
||||
// disappear into the void.
|
||||
const sqlFailure = Object.assign(new Error('database session failed'), { code: '57P01' });
|
||||
const fakeSql = async (strings: TemplateStringsArray): Promise<Record<string, unknown>[]> => {
|
||||
const query = strings.join('$');
|
||||
if (query.includes('SELECT client_id, client_secret_hash')) {
|
||||
return [{
|
||||
client_id: 'gbrain_cl_fake',
|
||||
client_secret_hash: hashToken('secret'),
|
||||
client_name: 'fake',
|
||||
redirect_uris: [],
|
||||
grant_types: ['client_credentials'],
|
||||
scope: 'read',
|
||||
client_id_issued_at: 1,
|
||||
}];
|
||||
}
|
||||
if (query.includes('SELECT deleted_at')) throw sqlFailure;
|
||||
return [];
|
||||
};
|
||||
const failingProvider = new GBrainOAuthProvider({ sql: fakeSql as any });
|
||||
|
||||
await expect(
|
||||
failingProvider.exchangeClientCredentials('gbrain_cl_fake', 'secret', 'read'),
|
||||
).rejects.toThrow('database session failed');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// F6 — sweepExpiredTokens returns a meaningful count across both engines
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('F6 sweepExpiredTokens count', () => {
|
||||
test('returns count > 0 after deleting expired rows', async () => {
|
||||
const firstClient = (await sql`SELECT client_id FROM oauth_clients LIMIT 1`)[0];
|
||||
const t1 = hashToken(generateToken('sweep_count_'));
|
||||
const t2 = hashToken(generateToken('sweep_count_'));
|
||||
await sql`INSERT INTO oauth_tokens (token_hash, token_type, client_id, scopes, expires_at)
|
||||
VALUES (${t1}, ${'access'}, ${firstClient.client_id as string}, ${'{read}'}, ${1})`;
|
||||
await sql`INSERT INTO oauth_tokens (token_hash, token_type, client_id, scopes, expires_at)
|
||||
VALUES (${t2}, ${'access'}, ${firstClient.client_id as string}, ${'{read}'}, ${2})`;
|
||||
|
||||
const swept = await provider.sweepExpiredTokens();
|
||||
|
||||
// Pre-fix: returned 0 on PGLite/postgres.js even when rows were deleted
|
||||
// because (result as any).count was unset on at least one path. With
|
||||
// RETURNING 1 + result.length, the actual row count flows back.
|
||||
expect(swept).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// F7c — auth code redirect_uri validated on /token (RFC 6749 §4.1.3)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('F7c redirect_uri binding on auth code exchange', () => {
|
||||
test('matching redirect_uri succeeds', async () => {
|
||||
const { clientId } = await provider.registerClientManual(
|
||||
'redir-match-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const client = (await provider.clientsStore.getClient(clientId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(client, {
|
||||
codeChallenge: 'challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
|
||||
const tokens = await provider.exchangeAuthorizationCode(
|
||||
client, code, undefined, 'http://localhost:3000/callback',
|
||||
);
|
||||
expect(tokens.access_token).toStartWith('gbrain_at_');
|
||||
});
|
||||
|
||||
test('mismatched redirect_uri rejects', async () => {
|
||||
const { clientId } = await provider.registerClientManual(
|
||||
'redir-mismatch-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const client = (await provider.clientsStore.getClient(clientId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(client, {
|
||||
codeChallenge: 'challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
|
||||
// Attacker submitting the auth code with a different redirect_uri (e.g.,
|
||||
// an attacker-controlled callback URL) MUST be rejected. RFC 6749 §4.1.3.
|
||||
await expect(
|
||||
provider.exchangeAuthorizationCode(
|
||||
client, code, undefined, 'https://attacker.example/cb',
|
||||
),
|
||||
).rejects.toThrow();
|
||||
});
|
||||
|
||||
test('empty-string redirect_uri does NOT bypass the binding', async () => {
|
||||
// D15 / adversarial-review fix: `redirectUri ? ...` would treat empty string
|
||||
// as falsy and silently fall through to the no-redirect-uri branch,
|
||||
// letting an attacker submit `redirect_uri=""` to bypass the predicate.
|
||||
// The fix uses `redirectUri !== undefined`. This test asserts the bypass
|
||||
// is closed: an empty-string redirect_uri must reject (zero-row DELETE
|
||||
// since stored value is the original non-empty URI), not slip through.
|
||||
const { clientId } = await provider.registerClientManual(
|
||||
'redir-empty-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const client = (await provider.clientsStore.getClient(clientId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(client, {
|
||||
codeChallenge: 'challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
|
||||
await expect(
|
||||
provider.exchangeAuthorizationCode(client, code, undefined, ''),
|
||||
).rejects.toThrow();
|
||||
});
|
||||
|
||||
test('omitted redirect_uri (back-compat) still succeeds', async () => {
|
||||
// Existing callers that don't pass redirectUri keep working — the
|
||||
// predicate only fires when redirectUri is provided. This protects
|
||||
// against breaking SDK consumers that haven't adopted the parameter
|
||||
// yet, while still hardening the path for those that have.
|
||||
const { clientId } = await provider.registerClientManual(
|
||||
'redir-omitted-test', ['authorization_code'], 'read',
|
||||
['http://localhost:3000/callback'],
|
||||
);
|
||||
const client = (await provider.clientsStore.getClient(clientId))!;
|
||||
|
||||
let redirectUrl = '';
|
||||
const mockRes = { redirect: (url: string) => { redirectUrl = url; } } as any;
|
||||
await provider.authorize(client, {
|
||||
codeChallenge: 'challenge',
|
||||
redirectUri: 'http://localhost:3000/callback',
|
||||
scopes: ['read'],
|
||||
}, mockRes);
|
||||
const code = new URL(redirectUrl).searchParams.get('code')!;
|
||||
|
||||
const tokens = await provider.exchangeAuthorizationCode(client, code);
|
||||
expect(tokens.access_token).toStartWith('gbrain_at_');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// F12 — DCR disable via constructor option (cleanup, not security)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('F12 dcrDisabled constructor option', () => {
|
||||
test('clientsStore omits registerClient when dcrDisabled=true', () => {
|
||||
const dcrOff = new GBrainOAuthProvider({ sql, dcrDisabled: true });
|
||||
const store = dcrOff.clientsStore;
|
||||
expect(typeof store.getClient).toBe('function');
|
||||
// SDK's mcpAuthRouter checks for registerClient before wiring up the
|
||||
// /register endpoint. Absence of the method == DCR endpoint not exposed.
|
||||
expect((store as any).registerClient).toBeUndefined();
|
||||
});
|
||||
|
||||
test('clientsStore exposes registerClient when dcrDisabled is false/unset', () => {
|
||||
const dcrOn = new GBrainOAuthProvider({ sql });
|
||||
expect(typeof dcrOn.clientsStore.registerClient).toBe('function');
|
||||
});
|
||||
|
||||
test('registerClientManual still works on dcrDisabled providers (CLI path)', async () => {
|
||||
// The CLI code path uses registerClientManual, which is independent of
|
||||
// the DCR /register endpoint. dcrDisabled must NOT break it.
|
||||
const dcrOff = new GBrainOAuthProvider({ sql, dcrDisabled: true });
|
||||
const result = await dcrOff.registerClientManual(
|
||||
'dcr-disabled-cli-test', ['client_credentials'], 'read',
|
||||
);
|
||||
expect(result.clientId).toStartWith('gbrain_cl_');
|
||||
expect(result.clientSecret).toStartWith('gbrain_cs_');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
/**
|
||||
* Trust-boundary contract regression tests (F7b).
|
||||
*
|
||||
* Pins the fail-closed semantics on `ctx.remote`. After v0.27 the field is
|
||||
* REQUIRED in the TypeScript type, but consumer code MUST still treat any
|
||||
* value that isn't strictly `false` as remote/untrusted. This is the runtime
|
||||
* defense-in-depth for the case where a context is constructed via `as` cast
|
||||
* or `Partial<>` spread and `remote` ends up undefined despite the type.
|
||||
*
|
||||
* The bug class this guards against: a future transport (HTTP/2, WebSocket,
|
||||
* a third-party plugin) inlines its own OperationContext literal and forgets
|
||||
* to set `remote`. Without these tests, the type system's compile-time check
|
||||
* is the only line of defense, and any `as OperationContext` cast bypasses
|
||||
* it.
|
||||
*/
|
||||
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import {
|
||||
operations,
|
||||
type Operation,
|
||||
type OperationContext,
|
||||
} from '../src/core/operations.ts';
|
||||
import type { BrainEngine } from '../src/core/engine.ts';
|
||||
|
||||
const submit_job = operations.find(o => o.name === 'submit_job') as Operation;
|
||||
if (!submit_job) throw new Error('submit_job operation missing');
|
||||
|
||||
// Stub engine — submit_job's protected-name guard fires before any DB call,
|
||||
// so the engine handle is never read on the rejection path.
|
||||
const stubEngine = {} as BrainEngine;
|
||||
const stubLogger = { info: () => {}, warn: () => {}, error: () => {} };
|
||||
|
||||
/**
|
||||
* Construct an OperationContext with `remote` deliberately undefined despite
|
||||
* the type system saying it's required. Mimics what would happen if a future
|
||||
* transport inlines its own context literal and forgets the field.
|
||||
*/
|
||||
function castUndefinedRemoteCtx(): OperationContext {
|
||||
return {
|
||||
engine: stubEngine,
|
||||
config: { engine: 'pglite' } as any,
|
||||
logger: stubLogger,
|
||||
dryRun: false,
|
||||
// remote intentionally omitted; cast through unknown to bypass the type
|
||||
} as unknown as OperationContext;
|
||||
}
|
||||
|
||||
describe('F7b — trust-boundary contract fail-closed semantics', () => {
|
||||
test('protected job submission rejected when remote is undefined (cast bypass)', async () => {
|
||||
const ctx = castUndefinedRemoteCtx();
|
||||
await expect(
|
||||
submit_job.handler(ctx, { name: 'shell', data: { cmd: 'id' } })
|
||||
).rejects.toMatchObject({ code: 'permission_denied' });
|
||||
});
|
||||
|
||||
test('protected job submission rejected when remote is true', async () => {
|
||||
const ctx = { ...castUndefinedRemoteCtx(), remote: true } as OperationContext;
|
||||
await expect(
|
||||
submit_job.handler(ctx, { name: 'shell', data: { cmd: 'id' } })
|
||||
).rejects.toMatchObject({ code: 'permission_denied' });
|
||||
});
|
||||
|
||||
test('protected job submission ALLOWED only when remote is strictly false', async () => {
|
||||
const ctx = { ...castUndefinedRemoteCtx(), remote: false } as OperationContext;
|
||||
// The handler now passes the protected-name guard and continues into the
|
||||
// queue. We don't actually want to enqueue anything in a unit test, so
|
||||
// we expect the call to fail at a LATER point (engine.executeRaw on the
|
||||
// stub). That's fine — what we're proving is that the guard does NOT
|
||||
// throw `permission_denied`, which is the failure mode we'd see if F7b
|
||||
// regressed back to a falsy check that treated remote=false as remote.
|
||||
await expect(
|
||||
submit_job.handler(ctx, { name: 'shell', data: { cmd: 'id' } })
|
||||
).rejects.not.toMatchObject({ code: 'permission_denied' });
|
||||
});
|
||||
|
||||
test('non-protected job names always allowed regardless of remote', async () => {
|
||||
// 'default-noop' is not in PROTECTED_JOB_NAMES. The protected-name guard
|
||||
// skips entirely, so we again get a downstream stub-engine error.
|
||||
const cases: OperationContext[] = [
|
||||
castUndefinedRemoteCtx(),
|
||||
{ ...castUndefinedRemoteCtx(), remote: true } as OperationContext,
|
||||
{ ...castUndefinedRemoteCtx(), remote: false } as OperationContext,
|
||||
];
|
||||
for (const ctx of cases) {
|
||||
await expect(
|
||||
submit_job.handler(ctx, { name: 'noop-job', data: {} })
|
||||
).rejects.not.toMatchObject({ code: 'permission_denied' });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user