mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
v0.41.3.0 fix(security/mcp): OAuth CORS lockdown + pre-register without DCR + validator surface (#1403)
* v0.41.3.0 fix(security/mcp): OAuth CORS lockdown, pre-register without DCR, validator surface
Three expanded cherry-picks plus codex-surfaced live-CORS fix, parser
rewrite, atomicity fix, DCR validator gate, SECURITY.md reconciliation.
What ships
- gbrain auth register-client gets --redirect-uri (repeatable) and
--token-endpoint-auth-method flags so the SECURITY.md-recommended
"pre-register without --enable-dcr" path actually works for claude.ai
and ChatGPT custom connectors.
- ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS = {client_secret_post,
client_secret_basic, none} validator gates all three registration
entry points (CLI, admin endpoint, DCR /register) so --enable-dcr is
no longer the looser path.
- Live Express OAuth server (/mcp, /token, /authorize, /register,
/revoke) was using default-wide-open cors() middleware — every
origin could complete a token exchange from a logged-in operator's
browser. Now default-deny; allowlist via GBRAIN_HTTP_CORS_ORIGIN.
- GBRAIN_HTTP_TRUST_PROXY env var on Express server with the same
semantics as the legacy bearer transport already had. Default
'loopback' preserved. SECURITY.md doc rewritten to match reality
(was lying that trust proxy was "disabled by default" while code
hardcoded 'loopback').
- Admin endpoint registration now atomic — INSERT-then-UPDATE for
public clients replaced with single INSERT via the new
registerClientManual(..., tokenEndpointAuthMethod) parameter (codex
outside-voice F4 catch).
- Legacy transport corsHeaders + corsPreflightHeaders consolidated
into one function gated on the allowlist for BOTH Allow-Origin and
Allow-Methods/Headers (codex F1; #983 thematically).
Surfaced by D7 codex outside-voice review on the v0.41.3 plan:
F1 (live Express CORS wide-open), F2 (indexOf parser couldn't do
repeatable flags), F3 (client_secret_basic missing from validator),
F4 (admin endpoint INSERT-then-UPDATE atomicity), F5 (DCR path
bypassed validator), F6 (env var already existed on legacy transport),
F7 (SECURITY.md vs impl doc disagreement).
Tests: 183 directly-touched cases green. Three new test files
(test/serve-http-trust-proxy.test.ts, test/serve-http-cors.test.ts,
test/auth-register-client-args.test.ts) + 18 new oauth.test.ts cases
+ 4 IRON RULE CORS preflight regressions.
Plan: ~/.claude/plans/system-instruction-you-are-working-wise-piglet.md
(D1-D11 captured, codex outside-voice integrated, GSTACK REVIEW REPORT
verdict CLEARED).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(test): audit-writer readRecent calendar-boundary flake
writer.log() uses real `new Date()` for filename computation, but the
test mocked `now` to 2026-05-22. When CI runs on a date in a different
ISO week (e.g. 2026-05-25 W22 vs the mocked W21), log() writes to one
file but readRecent(now) reads a different one — zero events overlap,
expect(2).toBe(0) fails.
Fix: write events directly to the file matching the test's mocked
`now` via writer.computeFilename(now), same pattern the cross-week
straddle test (line 234+) already used for the previous-week event.
Pre-existing test bug, surfaced when CI rolled past the week boundary
the original author wrote against. Not introduced by v0.41.3.0; fix
included here because /ship found it.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
ca68633faa
commit
6af0c91e53
+114
@@ -2,6 +2,120 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.41.3.0] - 2026-05-24
|
||||
|
||||
**Pre-register Claude and ChatGPT clients without `--enable-dcr` — the SECURITY.md-recommended setup actually works now.**
|
||||
|
||||
If you run `gbrain serve --http` to expose your brain over the network, the safe shape SECURITY.md recommends has always been "leave Dynamic Client Registration off, pre-register every browser-based client by hand." Before this release that path was broken at step one: `gbrain auth register-client` hard-coded no redirect URIs and no auth method, so claude.ai's first probe returned `Unregistered redirect_uri` and the only fix was hand-editing `oauth_clients` rows in psql. v0.41.3 makes the documented path actually usable.
|
||||
|
||||
While the SECURITY.md-recommended pre-registration shape was being plumbed, an independent code review found that the live Express OAuth server (`/mcp`, `/token`, `/authorize`, `/register`, `/revoke`) was using default-wide-open `cors()` middleware — every web origin could complete a token exchange from a logged-in operator's browser. That's now closed by default; OAuth endpoints reject all cross-origin requests unless `GBRAIN_HTTP_CORS_ORIGIN` lists the origin explicitly.
|
||||
|
||||
### How to use the new shape
|
||||
|
||||
Pre-register a confidential client with one paste-ready command:
|
||||
|
||||
```bash
|
||||
gbrain auth register-client claude-ai \
|
||||
--scopes "read write" \
|
||||
--redirect-uri https://claude.ai/api/mcp/auth_callback \
|
||||
--redirect-uri https://claude.com/api/mcp/auth_callback
|
||||
# --grant-types auto-set to authorization_code,refresh_token because --redirect-uri was passed
|
||||
```
|
||||
|
||||
Pre-register a public PKCE client (ChatGPT custom connector, Claude Code, Cursor) — no client secret minted:
|
||||
|
||||
```bash
|
||||
gbrain auth register-client chatgpt \
|
||||
--scopes "read write" \
|
||||
--redirect-uri https://chatgpt.com/connector/oauth/<HASH> \
|
||||
--token-endpoint-auth-method none
|
||||
```
|
||||
|
||||
Then start the server with the CORS allowlist set:
|
||||
|
||||
```bash
|
||||
GBRAIN_HTTP_CORS_ORIGIN=https://claude.ai gbrain serve --http --port 8787
|
||||
# Default reverse-proxy trust is "loopback" (Caddy/Tailscale on same host).
|
||||
# Behind Fly.io / Render / Vercel / nginx? Set GBRAIN_HTTP_TRUST_PROXY=1.
|
||||
GBRAIN_HTTP_TRUST_PROXY=1 gbrain serve --http --port 8787 --bind 0.0.0.0
|
||||
```
|
||||
|
||||
### What you get
|
||||
|
||||
| Surface | Before | After |
|
||||
|---|---|---|
|
||||
| `gbrain auth register-client` | hard-coded empty `redirect_uris`, NULL auth method, no public-client option | `--redirect-uri` (repeatable), `--token-endpoint-auth-method`, auto-set `authorization_code,refresh_token` when `--redirect-uri` is passed |
|
||||
| Express OAuth endpoints CORS | `cors()` default → `Access-Control-Allow-Origin: *` on /mcp, /token, /authorize, /register, /revoke | default-deny; allowlist via `GBRAIN_HTTP_CORS_ORIGIN`; startup WARN when `--bind 0.0.0.0` is set with no allowlist |
|
||||
| Legacy transport CORS preflight | leaked `Allow-Methods` + `Allow-Headers` to every Origin on OPTIONS regardless of allowlist | consolidated `corsHeaders(origin, {preflight})` — both Allow-Origin and Allow-Methods/Headers gated together |
|
||||
| Admin endpoint registering public client | INSERT confidential → UPDATE to NULL out `client_secret_hash` (non-atomic; UPDATE failure stranded a confidential row) | single atomic INSERT via `registerClientManual(..., tokenEndpointAuthMethod)` |
|
||||
| `token_endpoint_auth_method` validation | accepted any string at admin endpoint; CLI didn't even take the field | `ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS = {client_secret_post, client_secret_basic, none}` enforced at all three registration entry points (CLI, admin, DCR) |
|
||||
| Reverse-proxy trust env | hardcoded `'loopback'` in Express; docs claimed "disabled by default" (lie) | `GBRAIN_HTTP_TRUST_PROXY` env: `'loopback'` default, `'1'` for one-hop proxies, `'0'` to disable, numeric for N hops, named modes pass through |
|
||||
|
||||
### Things to watch after upgrade
|
||||
|
||||
- If you ran with `gbrain serve --http` behind a reverse proxy and a browser-based client (claude.ai, ChatGPT) at the same domain, you'll now need to add that origin to `GBRAIN_HTTP_CORS_ORIGIN`. Same-origin requests (no Origin header) are unaffected.
|
||||
- If you had hand-edited `oauth_clients` rows to set `token_endpoint_auth_method = 'frobnicate'` or any other non-allowlist value, those rows continue to function — the validator only gates new writes. To clean up, re-register the client via the CLI (which now mints the right shape atomically).
|
||||
- If you set `GBRAIN_HTTP_TRUST_PROXY=1` previously on the legacy bearer transport, that env var now also drives the Express OAuth server. Same value, same semantics — but the doc disagreement is gone.
|
||||
|
||||
### Itemized changes
|
||||
|
||||
#### Added
|
||||
|
||||
- `gbrain auth register-client --redirect-uri <uri>` (repeatable) — pre-register a client with one or more callback URLs. When passed without `--grant-types`, defaults to `authorization_code,refresh_token`.
|
||||
- `gbrain auth register-client --token-endpoint-auth-method <method>` — `client_secret_post` (default), `client_secret_basic`, or `none` (public PKCE-only client; no client secret minted).
|
||||
- `ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS` constant + `validateTokenEndpointAuthMethod()` validator exported from `src/core/oauth-provider.ts`. Single source of truth gated at all three registration entry points (CLI, admin endpoint, DCR `/register`). Closes the `--enable-dcr` loose-path hole where DCR previously skipped allowlist validation.
|
||||
- `GBRAIN_HTTP_TRUST_PROXY` env var on the Express OAuth server (`src/commands/serve-http.ts`). Maps `'loopback'` (default), `'0'`/`'false'` (trust nothing), `'1'`/`'true'` (one hop), other numeric (N hops), other strings pass-through to Express named modes / CIDR lists. Pure `resolveTrustProxy()` helper exported for testability.
|
||||
- `parseCorsAllowlistOAuth()` + `resolveCorsOrigin()` helpers in `src/commands/serve-http.ts`. The cors middleware on OAuth endpoints now uses `cors({ origin: resolveCorsOrigin(allowlist) })` — default-deny when `GBRAIN_HTTP_CORS_ORIGIN` is unset, function-form check when set.
|
||||
- Startup stderr WARN when `--bind 0.0.0.0` is set without `GBRAIN_HTTP_CORS_ORIGIN`. Surfaces the default-deny posture before the first cross-origin request.
|
||||
- 48-case `test/serve-http-trust-proxy.test.ts` + `test/serve-http-cors.test.ts` + `test/auth-register-client-args.test.ts` (new) plus 18 new cases in `test/oauth.test.ts` and 4 new IRON RULE CORS preflight regressions in `test/http-transport.test.ts`. 183 directly-touched tests, all green.
|
||||
|
||||
#### Changed
|
||||
|
||||
- `registerClientManual()` signature extends with `tokenEndpointAuthMethod?: string` parameter. Return type widens from `{clientId, clientSecret}` to `{clientId, clientSecret?}` because public clients (`'none'`) don't mint a secret. Atomic single INSERT for the public-client case — no more INSERT-then-UPDATE race.
|
||||
- `corsHeaders()` and `corsPreflightHeaders()` in `src/mcp/http-transport.ts` consolidated into one `corsHeaders(origin, {preflight: boolean})`. Methods/Headers only emit when `preflight === true AND origin in allowlist`. Closes the asymmetry where the OPTIONS handler leaked surface to non-allowlisted origins.
|
||||
- Admin endpoint `POST /admin/api/register-client` validates `tokenEndpointAuthMethod` via shared validator before calling `registerClientManual`. The post-insert UPDATE block that NULL'd `client_secret_hash` for `'none'` clients is gone; the atomic INSERT does it directly.
|
||||
- CLI argv parser at `src/commands/auth.ts:registerClient` rewritten from `indexOf`-based lookahead to a proper loop. Pre-fix the parser only honored the FIRST occurrence of any flag, so `--redirect-uri A --redirect-uri B` silently dropped B.
|
||||
- `SECURITY.md` "If you must use a custom HTTP wrapper" section gains a "Pre-registering claude.ai / ChatGPT clients without DCR" subsection with paste-ready commands. "CORS" section documents the v0.41.3 OAuth endpoint lockdown. "Reverse-proxy trust" rewritten to match reality — was claiming "disabled by default" while Express hardcoded `'loopback'`; now documents the `GBRAIN_HTTP_TRUST_PROXY` env contract honestly.
|
||||
|
||||
#### Fixed
|
||||
|
||||
- Admin endpoint atomicity bug (codex F4): pre-v0.41.3 the registration handler did `INSERT (confidential) → UPDATE (NULL out secret_hash)` for `tokenEndpointAuthMethod === 'none'`. If the UPDATE failed mid-flight (timeout, network), a confidential row with a real client_secret stranded — the agent thought it was registering a public client, but operators ended up with a confidential one. Single atomic INSERT now.
|
||||
- DCR validator gate (codex F5): pre-v0.41.3 the `--enable-dcr` `/register` path defaulted unknown `token_endpoint_auth_method` values to `'client_secret_post'`, silently swallowing typos. The shared validator now fires on the DCR path too — closes the "DCR was the loosest entry point" hole.
|
||||
- `client_secret_basic` admitted in the allowed set (codex F3): server supports HTTP Basic confidential client auth at `/token` (`src/commands/serve-http.ts:468`) but a narrower allowlist would have rejected it. The allowlist is exactly `{client_secret_post, client_secret_basic, none}` — the three methods the SDK's `mcpAuthRouter` advertises.
|
||||
- Wide-open `cors()` on every OAuth endpoint (codex F1, the biggest finding): pre-v0.41.3 the live Express server at `src/commands/serve-http.ts:400-404` ran `app.use('/mcp', cors())` (and same for /token, /authorize, /register, /revoke) with no allowlist. `cors()` defaults to `Access-Control-Allow-Origin: *`. Any web origin could complete a full OAuth flow from a logged-in operator's browser. Closed by default; explicit allowlist required.
|
||||
- Reverse-proxy doc disagreement (codex F7): docs at `SECURITY.md:127` said "Disabled by default" while `src/commands/serve-http.ts:390` hardcoded `app.set('trust proxy', 'loopback')`. Docs now match implementation.
|
||||
|
||||
### For contributors
|
||||
|
||||
- Three new TODOS filed in `TODOS.md` under "v0.41.3 security/MCP fix wave follow-ups": T13a (extract deny-by-default fine-grained scope wiring from PR #1316), T13b (extract real operation names in mcp_request_log from #1316), T13c (extract `access_tokens.last_used_at` LRU debounce from #1316). PR #1316's RLS posture rewrite is deliberately not filed — it changes the v0.26.7 auto-RLS event trigger that `gbrain doctor`'s `rls_event_trigger` check treats as load-bearing and needs its own plan-eng-review.
|
||||
- Community PRs closed as superseded (work either already in master or covered by this wave): #685 (chipoto69), #876 (toilalesondev), #1076 (lukejduncan), #1077 (lukejduncan), #620 (ArshyaAI). Status comment left on open PR #1316 (chipoto69) pointing at the three TODOS.
|
||||
- 4 IRON RULE regression tests added at `test/http-transport.test.ts` pin the consolidated `corsHeaders` matrix (preflight × allowlisted/non-allowlisted) so the CORS asymmetry bug class can't return silently. The fix-wave-structural assertion was updated to assert the NEW atomic admin endpoint shape; a regression guard pins that the post-insert UPDATE pattern is gone.
|
||||
|
||||
## To take advantage of v0.41.3.0
|
||||
|
||||
`gbrain upgrade` should do this automatically. There is no schema migration in this release; the changes are all in code + docs.
|
||||
|
||||
1. **Re-register browser-based clients with the new CLI flags:**
|
||||
```bash
|
||||
gbrain auth register-client claude-ai \
|
||||
--scopes "read write" \
|
||||
--redirect-uri https://claude.ai/api/mcp/auth_callback \
|
||||
--redirect-uri https://claude.com/api/mcp/auth_callback
|
||||
```
|
||||
(You can leave existing manually-edited `oauth_clients` rows in place; the validator only gates new writes.)
|
||||
|
||||
2. **Set `GBRAIN_HTTP_CORS_ORIGIN` if browser clients hit OAuth endpoints from a different origin.** Most setups (Claude Desktop, Cursor) don't need this; ChatGPT custom connector + claude.ai web flows do.
|
||||
|
||||
3. **Verify the new posture:**
|
||||
```bash
|
||||
curl -i -H "Origin: https://evil.example" -X OPTIONS http://localhost:8787/mcp
|
||||
# Expected: NO Access-Control-Allow-Methods header (was leaking pre-v0.41.3)
|
||||
curl -i -H "Origin: https://claude.ai" -X OPTIONS http://localhost:8787/mcp
|
||||
# Expected: has Access-Control-Allow-Methods + Access-Control-Allow-Origin (when allowlisted)
|
||||
```
|
||||
|
||||
4. **If something looks off,** please file an issue: https://github.com/garrytan/gbrain/issues with the output of `gbrain doctor` and which OAuth client + flow broke.
|
||||
|
||||
## [0.41.2.0] - 2026-05-24
|
||||
|
||||
**Your brain can now hold three lenses on the same data at once: creator, investor, engineer.** If you write content AND evaluate deals AND ship code, your old setup probably had three different agents pulling from three different mental models. This release ships four bundled "schema packs" that turn one brain into a multi-lens substrate: atoms + concepts join facts + takes as first-class units; gstack's typed learnings flow into the brain as first-class pages; the calibration profile that tracks how often you're wrong widens past its `{}` placeholder to score multiple domains side by side; and a one-shot migration importer lifts your OpenClaw's 13K atoms + 11K concepts into gbrain with permanent slug-keyed idempotency. Activate `gbrain-everything` and the dream cycle runs every pack-declared phase, the calibration profile produces all 7 domain scorecards in one query, and your gstack engineering learnings start appearing as queryable brain pages within seconds of being written.
|
||||
|
||||
+78
-9
@@ -50,6 +50,43 @@ exclusively via `gbrain auth create/list/revoke`.
|
||||
4. **Log all token issuance** — alert on unexpected registrations
|
||||
5. **Rate-limit registration and token endpoints**
|
||||
|
||||
### Pre-registering claude.ai / ChatGPT clients without DCR (v0.41.3+)
|
||||
|
||||
The recommended hardening posture above is: ship `gbrain serve --http`
|
||||
**without** `--enable-dcr` and pre-register every client manually. As of
|
||||
v0.41.3, `gbrain auth register-client` accepts the OAuth fields
|
||||
browser-based clients need:
|
||||
|
||||
```bash
|
||||
# Pre-register claude.ai (confidential client; two redirect URIs)
|
||||
gbrain auth register-client claude-ai \
|
||||
--scopes "read write" \
|
||||
--redirect-uri https://claude.ai/api/mcp/auth_callback \
|
||||
--redirect-uri https://claude.com/api/mcp/auth_callback
|
||||
# --grant-types is auto-set to authorization_code,refresh_token when
|
||||
# --redirect-uri is passed; pass --grant-types explicitly to override.
|
||||
|
||||
# Pre-register ChatGPT (public PKCE client; no client_secret minted)
|
||||
gbrain auth register-client chatgpt \
|
||||
--scopes "read write" \
|
||||
--redirect-uri https://chatgpt.com/connector/oauth/<HASH> \
|
||||
--token-endpoint-auth-method none
|
||||
```
|
||||
|
||||
Auth methods (`--token-endpoint-auth-method`):
|
||||
|
||||
- `client_secret_post` (default) — confidential client, secret in body
|
||||
- `client_secret_basic` — confidential client, secret in `Authorization` header
|
||||
- `none` — public PKCE-only client (no secret minted; ChatGPT custom
|
||||
connector, Claude Code, Cursor)
|
||||
|
||||
The validator rejects unknown methods at the registration boundary, and
|
||||
the same gate applies to the admin endpoint `POST /admin/api/register-client`
|
||||
and the DCR `POST /register` path. Pre-v0.41.3 the CLI hard-coded
|
||||
`redirect_uris = []` and `token_endpoint_auth_method = NULL`, forcing
|
||||
operators to UPDATE `oauth_clients` rows by hand to make claude.ai work
|
||||
without `--enable-dcr`. That footgun is gone.
|
||||
|
||||
### Token Management
|
||||
|
||||
```bash
|
||||
@@ -101,6 +138,19 @@ When the request `Origin` matches the allowlist, the server echoes it
|
||||
back in `Access-Control-Allow-Origin` (with `Vary: Origin`). Otherwise no
|
||||
CORS header is sent and the browser blocks the request.
|
||||
|
||||
**v0.41.3:** the same allowlist now gates every OAuth endpoint (`/mcp`,
|
||||
`/token`, `/authorize`, `/register`, `/revoke`). Pre-v0.41.3 these used
|
||||
default-wide-open `cors()` middleware, leaking
|
||||
`Access-Control-Allow-Origin: *` on every response — any web origin could
|
||||
complete a token exchange from a logged-in operator's browser. The CORS
|
||||
preflight handler in the legacy bearer transport was also asymmetric
|
||||
(actual-request path correctly default-deny, but OPTIONS preflight leaked
|
||||
`Access-Control-Allow-Methods` + `Access-Control-Allow-Headers` to every
|
||||
Origin); both are now consolidated through a single allowlist-gated path.
|
||||
A startup stderr WARN fires when `--bind 0.0.0.0` is set without
|
||||
`GBRAIN_HTTP_CORS_ORIGIN`, surfacing the default-deny posture before the
|
||||
first request.
|
||||
|
||||
### Rate limiting
|
||||
|
||||
Two buckets, both stored in a bounded LRU map (default 10K keys, evicts
|
||||
@@ -124,15 +174,34 @@ deployments.
|
||||
|
||||
### Reverse-proxy trust
|
||||
|
||||
Disabled by default. To honor `X-Forwarded-For` (or `X-Real-IP`) when
|
||||
gbrain runs behind a trusted reverse proxy:
|
||||
**Loopback-only by default** (v0.41.3+ Express server agrees with the
|
||||
legacy transport; pre-v0.41.3 the Express server hardcoded `'loopback'`
|
||||
while docs claimed "disabled by default" — that disagreement is gone).
|
||||
The default trusts only same-host proxies (127.0.0.1, ::1, fc00::/7);
|
||||
external forwarded-for headers are ignored regardless. To widen or
|
||||
narrow trust:
|
||||
|
||||
```bash
|
||||
# Trust exactly one hop — Fly.io, Render, Vercel, single-layer nginx
|
||||
GBRAIN_HTTP_TRUST_PROXY=1 gbrain serve --http --port 8787
|
||||
|
||||
# Trust N hops — Cloudflare → nginx → gbrain
|
||||
GBRAIN_HTTP_TRUST_PROXY=2 gbrain serve --http --port 8787
|
||||
|
||||
# Disable entirely — direct-exposure deployment with no proxy
|
||||
GBRAIN_HTTP_TRUST_PROXY=0 gbrain serve --http --port 8787
|
||||
|
||||
# Named Express modes (uniquelocal, linklocal) or CIDR lists pass through
|
||||
GBRAIN_HTTP_TRUST_PROXY=uniquelocal gbrain serve --http --port 8787
|
||||
GBRAIN_HTTP_TRUST_PROXY="10.0.0.0/8,192.168.1.0/24" gbrain serve --http --port 8787
|
||||
```
|
||||
|
||||
**Critical safety contract:** only set `GBRAIN_HTTP_TRUST_PROXY=1` when
|
||||
**both** of these are true:
|
||||
Both transports (Express OAuth server in `src/commands/serve-http.ts` and
|
||||
the legacy bearer transport in `src/mcp/http-transport.ts`) read the same
|
||||
env var, so single source of truth.
|
||||
|
||||
**Critical safety contract:** only widen past `'loopback'` when **both**
|
||||
of these are true:
|
||||
|
||||
1. gbrain is reachable only via a trusted reverse proxy (not directly
|
||||
exposed to the internet on the configured port). As of v0.34
|
||||
@@ -145,11 +214,11 @@ GBRAIN_HTTP_TRUST_PROXY=1 gbrain serve --http --port 8787
|
||||
X-Forwarded-For $remote_addr` does this; Cloudflare and most cloud
|
||||
load balancers handle it automatically.)
|
||||
|
||||
If gbrain is reachable directly AND `GBRAIN_HTTP_TRUST_PROXY=1` is set,
|
||||
clients can spoof their IP by sending arbitrary `X-Forwarded-For`
|
||||
headers, defeating the pre-auth IP rate limit. Without the flag, gbrain
|
||||
ignores all forwarded-for headers and uses the socket peer address,
|
||||
which is the safe default for direct-exposure deployments.
|
||||
If gbrain is reachable directly AND `GBRAIN_HTTP_TRUST_PROXY=1` (or any
|
||||
non-loopback value) is set, clients can spoof their IP by sending
|
||||
arbitrary `X-Forwarded-For` headers, defeating the pre-auth IP rate
|
||||
limit. The `'loopback'` default protects against this by ignoring all
|
||||
forwarded-for headers and using the socket peer address.
|
||||
|
||||
### Body size cap
|
||||
|
||||
|
||||
@@ -1,5 +1,58 @@
|
||||
# TODOS
|
||||
|
||||
## v0.41.3 security/MCP fix wave follow-ups (filed during ship of `garrytan/security-mcp-fix-wave`)
|
||||
|
||||
Source: codex outside-voice review on the v0.41.3 wave (D7) identified
|
||||
three real wins in PR #1316 (`chipoto69` — "Phase 4 multi-agent hardening")
|
||||
that did NOT land in v0.41.3. PR #1316 was bundled with RLS posture
|
||||
changes that conflict with v0.26.7's auto-RLS event trigger; the v0.41.3
|
||||
plan unbundled #1316 deliberately so its RLS posture rewrite gets its own
|
||||
architectural review. These three are the deferred standalone wins —
|
||||
each can ship as its own wave without touching RLS.
|
||||
|
||||
- [ ] **T13a (P1) — Extract deny-by-default fine-grained scope wiring
|
||||
from #1316.** Today the OAuth scope string (e.g. `read write`) is
|
||||
validated at registration via `ALLOWED_SCOPES_LIST` but does NOT
|
||||
constrain which MCP operations a token can call at dispatch time.
|
||||
Every op currently runs if the bearer is valid. #1316 adds per-op
|
||||
`requiredScope` metadata and a dispatch-time gate that returns 403
|
||||
when the bearer's scope set doesn't satisfy the op's requirement.
|
||||
Real security win: a `read`-scoped token can't call `put_page` or
|
||||
`submit_job`. Requires per-op annotation review (which ops need
|
||||
`write` vs `admin`) + scope-grammar decision (is `read` a strict
|
||||
subset of `write`, or are they orthogonal categories?). NOT in
|
||||
v0.41.3 because the per-op review is its own design exercise.
|
||||
Cherry-pick starter: PR #1316 diff against `src/core/operations.ts`
|
||||
and `src/mcp/dispatch.ts`. Effort: human ~2 days / CC ~3 hours.
|
||||
|
||||
- [ ] **T13b (P2) — Extract real operation names in mcp_request_log
|
||||
from #1316.** Pre-fix audit log records generic `tools/call` for
|
||||
every MCP request. #1316 carries the real op name (`get_page`,
|
||||
`put_page`, `submit_job`, etc.) into the `operation` column.
|
||||
Standalone win — no architectural risk, no schema change (column
|
||||
already exists), just dispatch-time wiring. Candidate for next
|
||||
minor (v0.41.4 or v0.42.x). Cherry-pick starter: #1316 diff
|
||||
against `src/mcp/dispatch.ts` audit-log insertion site.
|
||||
Effort: human ~1h / CC ~10min.
|
||||
|
||||
- [ ] **T13c (P2) — Extract `access_tokens.last_used_at` LRU debounce
|
||||
from #1316.** Today `last_used_at` is updated on every bearer
|
||||
request via the legacy transport's SQL-level WHERE-clause throttle
|
||||
(60s minimum gap). On high-traffic deployments the hot-row writes
|
||||
still hit Postgres for every request. #1316 adds an in-process LRU
|
||||
cache so the SQL UPDATE only fires once per token per cooldown
|
||||
window. Useful on multi-agent fleets sharing tokens at high rate;
|
||||
no value for personal-laptop installs. NOT a blocker. Cherry-pick
|
||||
starter: #1316's `src/core/token-last-used.ts` + the wiring in
|
||||
`src/mcp/http-transport.ts:validateToken`. Effort: human ~2h /
|
||||
CC ~20min.
|
||||
|
||||
**NOT filed:** the RLS posture rewrite from #1316. That changes the
|
||||
v0.26.7 auto-RLS event trigger that `gbrain doctor`'s
|
||||
`rls_event_trigger` check treats as load-bearing; it deserves its own
|
||||
plan-eng-review + doctor-check rewrite + breaking-change CHANGELOG
|
||||
note. Filing it as a TODO would imply it's ready to pull; it isn't.
|
||||
|
||||
## v0.41.0.0 follow-ups (v0.41.1+)
|
||||
|
||||
- [ ] **v0.41+: per-key rate-lease caps (`openai:responses`, `google:gemini`, etc.).**
|
||||
@@ -53,6 +106,7 @@
|
||||
current behavior (truncate-then-fail) is safe — no infinite loops,
|
||||
depth-cap prevents chains — but full semantic reduction unlocks higher
|
||||
self-fix success rates on legitimately-long prompts.
|
||||
|
||||
## v0.41 content-sanity follow-ups (filed during ship of `garrytan/lint-page-size-gate`)
|
||||
|
||||
Source: CEO + Eng review on the content-sanity defense plan. Both reviews
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gbrain",
|
||||
"version": "0.41.2.0",
|
||||
"version": "0.41.3.0",
|
||||
"description": "Postgres-native personal knowledge brain with hybrid RAG search",
|
||||
"type": "module",
|
||||
"main": "src/core/index.ts",
|
||||
|
||||
+114
-28
@@ -328,46 +328,125 @@ async function revokeClient(clientId: string) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse `gbrain auth register-client` argv. Walks the array once instead of
|
||||
* the prior `indexOf`-based pattern which (a) silently took only the FIRST
|
||||
* occurrence of a repeatable flag (defeated `--redirect-uri https://a
|
||||
* --redirect-uri https://b` — only `https://a` made it through), and (b)
|
||||
* accepted bare values via lookahead even when adjacent to another flag.
|
||||
*
|
||||
* v0.41.3 (T3): proper loop-based parser so `--redirect-uri` is repeatable,
|
||||
* and `--token-endpoint-auth-method` is recognized. Repeatable flags
|
||||
* accumulate into arrays. Unknown flags throw a usage error.
|
||||
*/
|
||||
interface RegisterClientArgs {
|
||||
grantTypes: string[];
|
||||
scopes: string;
|
||||
sourceId: string;
|
||||
federatedRead: string[] | undefined;
|
||||
redirectUris: string[];
|
||||
tokenEndpointAuthMethod: string | undefined;
|
||||
}
|
||||
|
||||
export function parseRegisterClientArgs(args: string[]): RegisterClientArgs {
|
||||
const out: RegisterClientArgs = {
|
||||
grantTypes: ['client_credentials'],
|
||||
scopes: 'read',
|
||||
sourceId: 'default',
|
||||
federatedRead: undefined,
|
||||
redirectUris: [],
|
||||
tokenEndpointAuthMethod: undefined,
|
||||
};
|
||||
let i = 0;
|
||||
let grantTypesSet = false;
|
||||
while (i < args.length) {
|
||||
const flag = args[i];
|
||||
const value = args[i + 1];
|
||||
const requireValue = () => {
|
||||
if (value === undefined || value.startsWith('--')) {
|
||||
throw new Error(`${flag} requires a value`);
|
||||
}
|
||||
return value;
|
||||
};
|
||||
switch (flag) {
|
||||
case '--grant-types': {
|
||||
const v = requireValue();
|
||||
out.grantTypes = v.split(',').map(s => s.trim()).filter(Boolean);
|
||||
grantTypesSet = out.grantTypes.length > 0;
|
||||
i += 2;
|
||||
break;
|
||||
}
|
||||
case '--scopes': out.scopes = requireValue(); i += 2; break;
|
||||
case '--source': out.sourceId = requireValue(); i += 2; break;
|
||||
case '--federated-read': {
|
||||
const v = requireValue();
|
||||
out.federatedRead = v.split(',').map(s => s.trim()).filter(Boolean);
|
||||
i += 2; break;
|
||||
}
|
||||
case '--redirect-uri':
|
||||
out.redirectUris.push(requireValue());
|
||||
i += 2; break;
|
||||
case '--token-endpoint-auth-method':
|
||||
out.tokenEndpointAuthMethod = requireValue();
|
||||
i += 2; break;
|
||||
default:
|
||||
throw new Error(`Unknown flag: ${flag}`);
|
||||
}
|
||||
}
|
||||
// v0.41.3: if --grant-types not explicitly set and any --redirect-uri was
|
||||
// passed, infer authorization_code + refresh_token. The single-flag path
|
||||
// (just --redirect-uri ...) is the SECURITY.md-recommended pre-registration
|
||||
// pattern; making operators redundantly pass `--grant-types` is footgun.
|
||||
if (!grantTypesSet && out.redirectUris.length > 0) {
|
||||
out.grantTypes = ['authorization_code', 'refresh_token'];
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
async function registerClient(name: string, args: string[]) {
|
||||
if (!name) {
|
||||
console.error('Usage: auth register-client <name> [--grant-types G] [--scopes S] [--source SOURCE] [--federated-read SRC1,SRC2,...]');
|
||||
console.error('Usage: auth register-client <name> [--grant-types G] [--scopes S] [--source SOURCE] [--federated-read SRC1,SRC2,...] [--redirect-uri URI ...] [--token-endpoint-auth-method client_secret_post|client_secret_basic|none]');
|
||||
process.exit(1);
|
||||
}
|
||||
const grantsIdx = args.indexOf('--grant-types');
|
||||
const scopesIdx = args.indexOf('--scopes');
|
||||
const sourceIdx = args.indexOf('--source');
|
||||
const federatedIdx = args.indexOf('--federated-read');
|
||||
const grantTypes = grantsIdx >= 0 && args[grantsIdx + 1]
|
||||
? args[grantsIdx + 1].split(',').map(s => s.trim()).filter(Boolean)
|
||||
: ['client_credentials'];
|
||||
const scopes = scopesIdx >= 0 && args[scopesIdx + 1] ? args[scopesIdx + 1] : 'read';
|
||||
// v0.34.1 (#861, D2): --source flag scopes the OAuth client to a single
|
||||
// source. Defaults to 'default' to match migration v60's backfill so
|
||||
// operators upgrading without changing flags see no behavior change.
|
||||
const sourceId = sourceIdx >= 0 && args[sourceIdx + 1] ? args[sourceIdx + 1] : 'default';
|
||||
// v0.34.1 (#876): --federated-read accepts a comma-separated source list
|
||||
// for federated read scope. When omitted, federated_read defaults to
|
||||
// [sourceId] (read scope == write scope, the v0.33 default).
|
||||
const federatedRead = federatedIdx >= 0 && args[federatedIdx + 1]
|
||||
? args[federatedIdx + 1].split(',').map(s => s.trim()).filter(Boolean)
|
||||
: undefined;
|
||||
let parsed: RegisterClientArgs;
|
||||
try {
|
||||
parsed = parseRegisterClientArgs(args);
|
||||
} catch (e: any) {
|
||||
console.error(`Error: ${e.message}`);
|
||||
console.error('Usage: auth register-client <name> [--grant-types G] [--scopes S] [--source SOURCE] [--federated-read SRC1,SRC2,...] [--redirect-uri URI ...] [--token-endpoint-auth-method client_secret_post|client_secret_basic|none]');
|
||||
process.exit(1);
|
||||
}
|
||||
const { grantTypes, scopes, sourceId, federatedRead, redirectUris, tokenEndpointAuthMethod } = parsed;
|
||||
|
||||
try {
|
||||
await withConfiguredSql(async (sql) => {
|
||||
const { GBrainOAuthProvider } = await import('../core/oauth-provider.ts');
|
||||
const provider = new GBrainOAuthProvider({ sql });
|
||||
const { clientId, clientSecret } = await provider.registerClientManual(
|
||||
name, grantTypes, scopes, [], sourceId, federatedRead,
|
||||
name, grantTypes, scopes, redirectUris, sourceId, federatedRead, tokenEndpointAuthMethod,
|
||||
);
|
||||
const effectiveFederated = federatedRead && federatedRead.length > 0 ? federatedRead : [sourceId];
|
||||
const effectiveAuthMethod = tokenEndpointAuthMethod || 'client_secret_post';
|
||||
console.log(`OAuth client registered: "${name}"\n`);
|
||||
console.log(` Client ID: ${clientId}`);
|
||||
console.log(` Client Secret: ${clientSecret}\n`);
|
||||
console.log(` Grant types: ${grantTypes.join(', ')}`);
|
||||
console.log(` Scopes: ${scopes}`);
|
||||
console.log(` Write source: ${sourceId}`);
|
||||
console.log(` Federated reads: ${effectiveFederated.join(', ')}\n`);
|
||||
console.log('Save the client secret — it will not be shown again.');
|
||||
console.log(` Client ID: ${clientId}`);
|
||||
if (clientSecret) {
|
||||
console.log(` Client Secret: ${clientSecret}\n`);
|
||||
} else {
|
||||
console.log(` Client Secret: <public client — none issued>\n`);
|
||||
}
|
||||
console.log(` Grant types: ${grantTypes.join(', ')}`);
|
||||
console.log(` Scopes: ${scopes}`);
|
||||
console.log(` Token auth method: ${effectiveAuthMethod}`);
|
||||
if (redirectUris.length > 0) {
|
||||
console.log(` Redirect URIs: ${redirectUris.join(', ')}`);
|
||||
}
|
||||
console.log(` Write source: ${sourceId}`);
|
||||
console.log(` Federated reads: ${effectiveFederated.join(', ')}\n`);
|
||||
if (clientSecret) {
|
||||
console.log('Save the client secret — it will not be shown again.');
|
||||
} else {
|
||||
console.log('Public client (PKCE-only) — no secret needed.');
|
||||
}
|
||||
console.log(`Revoke with: gbrain auth revoke-client "${clientId}"`);
|
||||
});
|
||||
} catch (e: any) {
|
||||
@@ -424,8 +503,15 @@ Usage:
|
||||
gbrain auth permissions <name> set-takes-holders <h1,h2,h3>
|
||||
Update visibility for an existing token
|
||||
gbrain auth register-client <name> [options] Register an OAuth 2.1 client (v0.26+)
|
||||
--grant-types <client_credentials,authorization_code> (default: client_credentials)
|
||||
--grant-types <client_credentials,authorization_code> (default: client_credentials;
|
||||
auto-set to authorization_code,refresh_token
|
||||
when --redirect-uri is passed)
|
||||
--scopes "<read write admin>" (default: read)
|
||||
--source <id> (default: default)
|
||||
--federated-read <id1,id2,...> (default: [source])
|
||||
--redirect-uri <https://...> (v0.41.3+; repeatable; required for authorization_code)
|
||||
--token-endpoint-auth-method <method> (v0.41.3+; client_secret_post | client_secret_basic | none;
|
||||
'none' = public PKCE-only client, no secret minted)
|
||||
gbrain auth revoke-client <client_id> Hard-delete an OAuth 2.1 client (cascades to tokens + codes)
|
||||
gbrain auth test <url> --token <token> Smoke-test a remote MCP server
|
||||
`);
|
||||
|
||||
+126
-19
@@ -25,7 +25,7 @@ import { requireBearerAuth } from '@modelcontextprotocol/sdk/server/auth/middlew
|
||||
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 { GBrainOAuthProvider, validateTokenEndpointAuthMethod } from '../core/oauth-provider.ts';
|
||||
import type { SqlQuery } from '../core/oauth-provider.ts';
|
||||
import { hasScope, ALLOWED_SCOPES_LIST, normalizeScopesInput } from '../core/scope.ts';
|
||||
import { summarizeMcpParams, dispatchToolCall } from '../mcp/dispatch.ts';
|
||||
@@ -187,6 +187,74 @@ export async function probeLiveness(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve `GBRAIN_HTTP_TRUST_PROXY` into a value Express's `app.set('trust
|
||||
* proxy', ...)` accepts. Pure function so the test surface is one place,
|
||||
* not the whole Express stack.
|
||||
*
|
||||
* Mapping:
|
||||
* - unset / empty → 'loopback' (pre-v0.41.3 default; trusts only
|
||||
* 127.0.0.1, ::1, ::ffff:127.0.0.1, fc00::/7)
|
||||
* - '0' / 'false' → false (trust nothing; req.ip is socket peer regardless
|
||||
* of X-Forwarded-For)
|
||||
* - '1' / 'true' → 1 (trust exactly one hop; safe for Fly.io / Render /
|
||||
* single-layer reverse proxy; matches the legacy transport's '==1' check)
|
||||
* - other numeric → parseInt (trust N hops)
|
||||
* - any other string → pass through verbatim (Express accepts named modes
|
||||
* like 'uniquelocal', 'linklocal', and CIDR/IP lists)
|
||||
*
|
||||
* SECURITY: only set GBRAIN_HTTP_TRUST_PROXY when BOTH (a) gbrain is
|
||||
* reachable only via a trusted reverse proxy, AND (b) the proxy strips
|
||||
* client-supplied X-Forwarded-For headers before re-emitting its own.
|
||||
* Otherwise clients can spoof their IP and defeat the pre-auth IP rate
|
||||
* limit. See SECURITY.md "Reverse-proxy trust" for the full contract.
|
||||
*/
|
||||
export function resolveTrustProxy(env: string | undefined): string | number | boolean {
|
||||
if (env === undefined || env === '') return 'loopback';
|
||||
if (env === '0' || env === 'false') return false;
|
||||
if (env === '1' || env === 'true') return 1;
|
||||
if (/^\d+$/.test(env)) return parseInt(env, 10);
|
||||
return env;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse `GBRAIN_HTTP_CORS_ORIGIN` into a Set of allowed origins for OAuth
|
||||
* endpoints. Mirrors `src/mcp/http-transport.ts:parseCorsAllowlist`. Single
|
||||
* env var so operators don't need to maintain two allowlists.
|
||||
*
|
||||
* Returns null when unset, empty, or whitespace-only — caller MUST treat
|
||||
* null as "deny all cross-origin" (the same posture the legacy transport
|
||||
* already takes).
|
||||
*/
|
||||
export function parseCorsAllowlistOAuth(): Set<string> | null {
|
||||
const v = process.env.GBRAIN_HTTP_CORS_ORIGIN;
|
||||
if (!v) return null;
|
||||
const origins = v.split(',').map(s => s.trim()).filter(Boolean);
|
||||
return origins.length === 0 ? null : new Set(origins);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a `cors.CorsOptions['origin']` value from the allowlist. The cors
|
||||
* package accepts:
|
||||
* - `false` → reject everything (no Allow-Origin header sent)
|
||||
* - `(origin, cb) => cb(null, boolean)` → dynamic per-request check
|
||||
* We use the function form when an allowlist is set so the value of the
|
||||
* Allow-Origin header echoes the request Origin (RFC 6454) instead of a
|
||||
* hardcoded string, and so the same options object covers all listed
|
||||
* origins without enumeration in the response.
|
||||
*
|
||||
* Same-origin requests (no Origin header) get `cb(null, true)` which the
|
||||
* cors package translates to "no CORS headers needed" — they're not
|
||||
* cross-origin so they don't trigger the gate.
|
||||
*/
|
||||
export function resolveCorsOrigin(allowlist: Set<string> | null): cors.CorsOptions['origin'] {
|
||||
if (allowlist === null) return false;
|
||||
return (origin: string | undefined, cb: (err: Error | null, allow?: boolean) => void) => {
|
||||
if (!origin) return cb(null, true);
|
||||
cb(null, allowlist.has(origin));
|
||||
};
|
||||
}
|
||||
|
||||
interface ServeHttpOptions {
|
||||
port: number;
|
||||
tokenTtl: number;
|
||||
@@ -387,7 +455,14 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
|
||||
// Express 5 app
|
||||
const app = express();
|
||||
app.set('trust proxy', 'loopback'); // Caddy/Tailscale reverse proxy on localhost
|
||||
// v0.41.3 (T8): configurable trust-proxy via GBRAIN_HTTP_TRUST_PROXY env.
|
||||
// Default 'loopback' (trust Caddy/Tailscale on the same host) preserves
|
||||
// pre-v0.41.3 behavior. Operators behind Fly.io / Render / Vercel / nginx
|
||||
// set GBRAIN_HTTP_TRUST_PROXY=1 (one hop) so X-Forwarded-For lands as the
|
||||
// real client IP for rate-limiting and req.secure detection. The legacy
|
||||
// transport already reads this env var (src/mcp/http-transport.ts:111)
|
||||
// for the same purpose; T8 makes the Express path agree.
|
||||
app.set('trust proxy', resolveTrustProxy(process.env.GBRAIN_HTTP_TRUST_PROXY));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Cookie parsing — required for /admin auth (express 5 has no built-in)
|
||||
@@ -395,13 +470,37 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
app.use(cookieParser());
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// CORS
|
||||
// CORS (v0.41.3, T7 — default-deny on every OAuth endpoint)
|
||||
// ---------------------------------------------------------------------------
|
||||
app.use('/mcp', cors());
|
||||
app.use('/token', cors());
|
||||
app.use('/authorize', cors());
|
||||
app.use('/register', cors());
|
||||
app.use('/revoke', cors());
|
||||
// Pre-v0.41.3 every OAuth endpoint used bare `cors()` which defaults to
|
||||
// `Access-Control-Allow-Origin: *` — any web origin could complete a token
|
||||
// exchange from a logged-in operator's browser. The fix parses
|
||||
// GBRAIN_HTTP_CORS_ORIGIN the same way the legacy transport already does
|
||||
// (src/mcp/http-transport.ts:parseCorsAllowlist) and gates every OAuth
|
||||
// surface behind the allowlist. When the env var is unset the OAuth
|
||||
// endpoints reject all cross-origin requests (default deny). Same-origin
|
||||
// requests are unaffected because browsers send no Origin header for them.
|
||||
//
|
||||
// The /admin SPA is the one cross-origin caller we expect on a personal
|
||||
// laptop install; it ships co-located with the brain and uses
|
||||
// same-origin XHR, so the lockdown doesn't break it.
|
||||
const corsAllowlistOAuth = parseCorsAllowlistOAuth();
|
||||
if (!corsAllowlistOAuth && bind === '0.0.0.0') {
|
||||
console.error(
|
||||
'[serve-http] WARNING: --bind 0.0.0.0 is set but GBRAIN_HTTP_CORS_ORIGIN is unset. OAuth endpoints will reject ALL cross-origin requests until you set the env var (comma-separated origins).',
|
||||
);
|
||||
}
|
||||
const corsOAuthOptions: cors.CorsOptions = {
|
||||
origin: resolveCorsOrigin(corsAllowlistOAuth),
|
||||
credentials: false,
|
||||
methods: ['GET', 'POST', 'OPTIONS'],
|
||||
allowedHeaders: ['Content-Type', 'Authorization', 'Accept'],
|
||||
};
|
||||
app.use('/mcp', cors(corsOAuthOptions));
|
||||
app.use('/token', cors(corsOAuthOptions));
|
||||
app.use('/authorize', cors(corsOAuthOptions));
|
||||
app.use('/register', cors(corsOAuthOptions));
|
||||
app.use('/revoke', cors(corsOAuthOptions));
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Custom client_credentials handler (before mcpAuthRouter)
|
||||
@@ -1121,18 +1220,26 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
|
||||
}
|
||||
const grants = Array.isArray(grantTypes) && grantTypes.length > 0 ? grantTypes : ['client_credentials'];
|
||||
const uris = Array.isArray(redirectUris) ? redirectUris : [];
|
||||
const result = await oauthProvider.registerClientManual(
|
||||
name, grants, scopeString, uris,
|
||||
);
|
||||
// Public client (PKCE-only, no secret): NULL out client_secret_hash and
|
||||
// set auth method so the SDK's clientAuth middleware skips the hash-vs-
|
||||
// plaintext comparison that would otherwise reject the request. This is
|
||||
// the supported pattern for browser-based OAuth (e.g. claude.ai's
|
||||
// Custom Connector flow, which uses authorization_code + PKCE).
|
||||
if (tokenEndpointAuthMethod === 'none') {
|
||||
await sql`UPDATE oauth_clients SET client_secret_hash = NULL, token_endpoint_auth_method = 'none' WHERE client_id = ${result.clientId}`;
|
||||
delete (result as any).clientSecret;
|
||||
// v0.41.3 (T1+T4): validate token_endpoint_auth_method via shared
|
||||
// ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS before reaching the provider.
|
||||
// Pre-v0.41.3 this endpoint did INSERT (confidential) → UPDATE (NULL
|
||||
// out secret_hash) for the 'none' case, which left a confidential
|
||||
// row stranded if the UPDATE failed (codex F4). Atomic now: pass the
|
||||
// method to registerClientManual and let it INSERT the correct row
|
||||
// in a single statement.
|
||||
let validatedAuthMethod: string | undefined;
|
||||
try {
|
||||
validatedAuthMethod = validateTokenEndpointAuthMethod(tokenEndpointAuthMethod);
|
||||
} catch (e) {
|
||||
res.status(400).json({
|
||||
error: 'invalid_token_endpoint_auth_method',
|
||||
message: e instanceof Error ? e.message : String(e),
|
||||
});
|
||||
return;
|
||||
}
|
||||
const result = await oauthProvider.registerClientManual(
|
||||
name, grants, scopeString, uris, 'default', undefined, validatedAuthMethod,
|
||||
);
|
||||
// Set per-client TTL if specified
|
||||
if (tokenTtl && Number(tokenTtl) > 0) {
|
||||
await sql`UPDATE oauth_clients SET token_ttl = ${Number(tokenTtl)} WHERE client_id = ${result.clientId}`;
|
||||
|
||||
+94
-12
@@ -50,6 +50,65 @@ function pgArray(arr: string[]): string {
|
||||
return `{${escaped.join(',')}}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Allow-list of RFC 7591 §2 `token_endpoint_auth_method` values gbrain
|
||||
* accepts at registration. Three values, chosen because the SDK's
|
||||
* `mcpAuthRouter` advertises exactly these three in
|
||||
* `token_endpoint_auth_methods_supported`:
|
||||
*
|
||||
* - `client_secret_post` — confidential client; secret in body (default)
|
||||
* - `client_secret_basic` — confidential client; secret in Authorization header
|
||||
* - `none` — public PKCE-only client (Claude Code, Cursor, ChatGPT custom connector)
|
||||
*
|
||||
* Three call sites enforce this set:
|
||||
* 1. CLI `gbrain auth register-client` (src/commands/auth.ts)
|
||||
* 2. Admin `POST /admin/api/register-client` (src/commands/serve-http.ts)
|
||||
* 3. DCR `POST /register` (this file, GBrainClientsStore.registerClient)
|
||||
*
|
||||
* **Read-tolerant by design.** `getClient` returns whatever is stored
|
||||
* verbatim — legacy rows with non-allowlist values (e.g. pre-v0.41.3
|
||||
* direct UPDATEs) continue to function. The validator gates new writes
|
||||
* ONLY; we don't break operators with hand-edited rows on upgrade.
|
||||
*/
|
||||
export type TokenEndpointAuthMethod = 'client_secret_post' | 'client_secret_basic' | 'none';
|
||||
|
||||
export const ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS = new Set<TokenEndpointAuthMethod>([
|
||||
'client_secret_post',
|
||||
'client_secret_basic',
|
||||
'none',
|
||||
]);
|
||||
|
||||
export class InvalidTokenEndpointAuthMethodError extends Error {
|
||||
readonly code = 'invalid_token_endpoint_auth_method';
|
||||
constructor(value: unknown) {
|
||||
super(
|
||||
`Invalid token_endpoint_auth_method: ${JSON.stringify(value)}. ` +
|
||||
`Expected one of: ${Array.from(ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS).join(', ')}. ` +
|
||||
`RFC 7591 §2 — see https://datatracker.ietf.org/doc/html/rfc7591#section-2.`,
|
||||
);
|
||||
this.name = 'InvalidTokenEndpointAuthMethodError';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a token_endpoint_auth_method value at the registration boundary.
|
||||
* Throws `InvalidTokenEndpointAuthMethodError` on rejection; returns the
|
||||
* typed value on success. Returns `'client_secret_post'` for undefined input
|
||||
* (RFC 7591 default).
|
||||
*
|
||||
* Apply at every registration entry point (CLI, admin endpoint, DCR). Do
|
||||
* NOT apply on read — legacy oauth_clients rows with non-allowlist values
|
||||
* must continue to function unchanged.
|
||||
*/
|
||||
export function validateTokenEndpointAuthMethod(value: unknown): TokenEndpointAuthMethod {
|
||||
if (value === undefined || value === null || value === '') return 'client_secret_post';
|
||||
if (typeof value !== 'string') throw new InvalidTokenEndpointAuthMethodError(value);
|
||||
if (!ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS.has(value as TokenEndpointAuthMethod)) {
|
||||
throw new InvalidTokenEndpointAuthMethodError(value);
|
||||
}
|
||||
return value as TokenEndpointAuthMethod;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a redirect_uri per RFC 6749 §3.1.2.1.
|
||||
*
|
||||
@@ -176,6 +235,12 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
// is operator-trusted).
|
||||
assertAllowedScopes(parseScopeString(client.scope));
|
||||
|
||||
// v0.41.3 (T5): validate token_endpoint_auth_method on the DCR path so
|
||||
// `--enable-dcr` is not the looser entry point. CLI and admin paths gate
|
||||
// through the same `validateTokenEndpointAuthMethod` helper — all three
|
||||
// registration entry points share one allow-list.
|
||||
const authMethod = validateTokenEndpointAuthMethod(client.token_endpoint_auth_method);
|
||||
|
||||
const clientId = generateToken('gbrain_cl_');
|
||||
// v0.34.1 (#909): RFC 7591 §2 — clients that authenticate at the token
|
||||
// endpoint via PKCE alone declare `token_endpoint_auth_method: "none"`.
|
||||
@@ -189,7 +254,6 @@ class GBrainClientsStore implements OAuthRegisteredClientsStore {
|
||||
// NULL` and skip the secret comparison. Confidential clients (default
|
||||
// `client_secret_post` and explicit `client_secret_basic`) still mint
|
||||
// a secret as before.
|
||||
const authMethod = client.token_endpoint_auth_method || 'client_secret_post';
|
||||
const isPublicClient = authMethod === 'none';
|
||||
const clientSecret = isPublicClient ? undefined : generateToken('gbrain_cs_');
|
||||
const secretHash = clientSecret ? hashToken(clientSecret) : null;
|
||||
@@ -755,16 +819,30 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
redirectUris: string[] = [],
|
||||
sourceId: string = 'default',
|
||||
federatedRead?: string[],
|
||||
): Promise<{ clientId: string; clientSecret: string }> {
|
||||
tokenEndpointAuthMethod?: string,
|
||||
): Promise<{ clientId: string; clientSecret?: string }> {
|
||||
// v0.28: ALLOWED_SCOPES allowlist. Reject `--scopes "read flying-unicorn"`
|
||||
// at registration so meaningless scope strings can't pile up in the DB.
|
||||
// Pre-allowlist clients keep working (allowlist is registration-time;
|
||||
// existing rows aren't re-validated).
|
||||
assertAllowedScopes(parseScopeString(scopes));
|
||||
|
||||
// v0.41.3 (T1+T2): validate token_endpoint_auth_method at the registration
|
||||
// boundary. Throws InvalidTokenEndpointAuthMethodError on bad input.
|
||||
// Default is `client_secret_post` (RFC 7591 §2).
|
||||
const authMethod = validateTokenEndpointAuthMethod(tokenEndpointAuthMethod);
|
||||
|
||||
const clientId = generateToken('gbrain_cl_');
|
||||
const clientSecret = generateToken('gbrain_cs_');
|
||||
const secretHash = hashToken(clientSecret);
|
||||
// v0.41.3 (T2): atomic public-client INSERT. When the caller declares
|
||||
// `tokenEndpointAuthMethod: 'none'` we mint NO secret and INSERT with
|
||||
// client_secret_hash = NULL in a single statement. Pre-fix, the admin
|
||||
// endpoint did INSERT-then-UPDATE which left a confidential row stranded
|
||||
// if the UPDATE failed mid-flight (codex F4). Confidential clients
|
||||
// (`client_secret_post` / `client_secret_basic`) get the secret minted
|
||||
// and hashed as before.
|
||||
const isPublicClient = authMethod === 'none';
|
||||
const clientSecret = isPublicClient ? undefined : generateToken('gbrain_cs_');
|
||||
const secretHash = clientSecret ? hashToken(clientSecret) : null;
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
|
||||
// v0.34.1 (#861 + #876): persist source_id AND federated_read so
|
||||
@@ -776,10 +854,11 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
try {
|
||||
await this.sql`
|
||||
INSERT INTO oauth_clients (client_id, client_secret_hash, client_name, redirect_uris,
|
||||
grant_types, scope, client_id_issued_at,
|
||||
grant_types, scope, token_endpoint_auth_method,
|
||||
client_id_issued_at,
|
||||
source_id, federated_read)
|
||||
VALUES (${clientId}, ${secretHash}, ${name},
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${now},
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${authMethod}, ${now},
|
||||
${sourceId}, ${pgArray(federated)})
|
||||
`;
|
||||
} catch (err) {
|
||||
@@ -790,17 +869,19 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
try {
|
||||
await this.sql`
|
||||
INSERT INTO oauth_clients (client_id, client_secret_hash, client_name, redirect_uris,
|
||||
grant_types, scope, client_id_issued_at, source_id)
|
||||
grant_types, scope, token_endpoint_auth_method,
|
||||
client_id_issued_at, source_id)
|
||||
VALUES (${clientId}, ${secretHash}, ${name},
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${now}, ${sourceId})
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${authMethod}, ${now}, ${sourceId})
|
||||
`;
|
||||
} catch (err2) {
|
||||
if (isUndefinedColumnError(err2, 'source_id')) {
|
||||
await this.sql`
|
||||
INSERT INTO oauth_clients (client_id, client_secret_hash, client_name, redirect_uris,
|
||||
grant_types, scope, client_id_issued_at)
|
||||
grant_types, scope, token_endpoint_auth_method,
|
||||
client_id_issued_at)
|
||||
VALUES (${clientId}, ${secretHash}, ${name},
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${now})
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${authMethod}, ${now})
|
||||
`;
|
||||
} else {
|
||||
throw err2;
|
||||
@@ -809,9 +890,10 @@ export class GBrainOAuthProvider implements OAuthServerProvider {
|
||||
} else if (isUndefinedColumnError(err, 'source_id')) {
|
||||
await this.sql`
|
||||
INSERT INTO oauth_clients (client_id, client_secret_hash, client_name, redirect_uris,
|
||||
grant_types, scope, client_id_issued_at)
|
||||
grant_types, scope, token_endpoint_auth_method,
|
||||
client_id_issued_at)
|
||||
VALUES (${clientId}, ${secretHash}, ${name},
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${now})
|
||||
${pgArray(redirectUris)}, ${pgArray(grantTypes)}, ${scopes}, ${authMethod}, ${now})
|
||||
`;
|
||||
} else {
|
||||
throw err;
|
||||
|
||||
+31
-17
@@ -137,23 +137,37 @@ export async function startHttpTransport(opts: HttpTransportOptions) {
|
||||
const corsAllowlist = parseCorsAllowlist();
|
||||
const tools = buildToolDefs(operations);
|
||||
|
||||
function corsHeaders(origin: string | null, extra: Record<string, string> = {}): Record<string, string> {
|
||||
const headers: Record<string, string> = { ...extra };
|
||||
if (corsAllowlist && origin && corsAllowlist.has(origin)) {
|
||||
headers['Access-Control-Allow-Origin'] = origin;
|
||||
headers['Vary'] = 'Origin';
|
||||
}
|
||||
return headers;
|
||||
/**
|
||||
* v0.41.3 (T6): single consolidated CORS header builder. Pre-fix there were
|
||||
* two parallel functions (`corsHeaders` for actual requests, `corsPreflightHeaders`
|
||||
* for OPTIONS) — the preflight variant unconditionally emitted
|
||||
* `Access-Control-Allow-Methods` + `Access-Control-Allow-Headers` to EVERY
|
||||
* Origin, leaking the API surface to attackers probing the preflight. The
|
||||
* actual-request path was correctly default-deny.
|
||||
*
|
||||
* One function, one allowlist gate. Methods/Headers only emit when
|
||||
* preflight=true AND origin is allowlisted. Allow-Origin emits only when
|
||||
* origin is allowlisted (unchanged). `Vary: Origin` pairs with Allow-Origin
|
||||
* so caches don't serve allowlisted responses to non-allowlisted requests.
|
||||
*
|
||||
* `extra` is for response-specific headers (Retry-After, etc.) and is
|
||||
* never gated by the allowlist.
|
||||
*/
|
||||
interface CorsHeaderOpts {
|
||||
preflight?: boolean;
|
||||
extra?: Record<string, string>;
|
||||
}
|
||||
|
||||
function corsPreflightHeaders(origin: string | null): Record<string, string> {
|
||||
const headers: Record<string, string> = {
|
||||
'Access-Control-Allow-Methods': 'POST, GET, OPTIONS',
|
||||
'Access-Control-Allow-Headers': 'Content-Type, Authorization, Accept',
|
||||
};
|
||||
if (corsAllowlist && origin && corsAllowlist.has(origin)) {
|
||||
function corsHeaders(origin: string | null, opts: CorsHeaderOpts = {}): Record<string, string> {
|
||||
const { preflight = false, extra = {} } = opts;
|
||||
const headers: Record<string, string> = { ...extra };
|
||||
const allowed = corsAllowlist && origin && corsAllowlist.has(origin);
|
||||
if (allowed) {
|
||||
headers['Access-Control-Allow-Origin'] = origin;
|
||||
headers['Vary'] = 'Origin';
|
||||
if (preflight) {
|
||||
headers['Access-Control-Allow-Methods'] = 'POST, OPTIONS';
|
||||
headers['Access-Control-Allow-Headers'] = 'Content-Type, Authorization, Accept';
|
||||
}
|
||||
}
|
||||
return headers;
|
||||
}
|
||||
@@ -216,7 +230,7 @@ export async function startHttpTransport(opts: HttpTransportOptions) {
|
||||
|
||||
// CORS preflight
|
||||
if (req.method === 'OPTIONS') {
|
||||
return new Response(null, { headers: corsPreflightHeaders(origin) });
|
||||
return new Response(null, { headers: corsHeaders(origin, { preflight: true }) });
|
||||
}
|
||||
|
||||
// Health check — no auth, no rate limit. Probes the DB so orchestration
|
||||
@@ -253,7 +267,7 @@ export async function startHttpTransport(opts: HttpTransportOptions) {
|
||||
{ error: 'rate_limited', message: 'Too many requests' },
|
||||
{
|
||||
status: 429,
|
||||
headers: corsHeaders(origin, { 'Retry-After': String(ipCheck.retryAfter ?? 60) }),
|
||||
headers: corsHeaders(origin, { extra: { 'Retry-After': String(ipCheck.retryAfter ?? 60) } }),
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -286,7 +300,7 @@ export async function startHttpTransport(opts: HttpTransportOptions) {
|
||||
{ error: 'rate_limited', message: 'Too many requests for this token' },
|
||||
{
|
||||
status: 429,
|
||||
headers: corsHeaders(origin, { 'Retry-After': String(tokCheck.retryAfter ?? 60) }),
|
||||
headers: corsHeaders(origin, { extra: { 'Retry-After': String(tokCheck.retryAfter ?? 60) } }),
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -222,11 +222,22 @@ describe('createAuditWriter — readRecent()', () => {
|
||||
const inWin2 = new Date(now.getTime() - 6 * 86400000).toISOString();
|
||||
const outOfWin = new Date(now.getTime() - 8 * 86400000).toISOString();
|
||||
|
||||
// All written to current-week file for simplicity (the readRecent
|
||||
// window filter is what we're testing, not the cross-week walk).
|
||||
writer.log({ ts: inWin1, message: 'in window 1' });
|
||||
writer.log({ ts: inWin2, message: 'in window 2' });
|
||||
writer.log({ ts: outOfWin, message: 'out of window' });
|
||||
// Write events DIRECTLY to the file matching `now` (not via
|
||||
// writer.log() which uses real `new Date()` for the filename).
|
||||
// Pre-fix: writer.log() wrote to real-clock current-week file, but
|
||||
// readRecent(now) read the test's mocked now's current/previous-week
|
||||
// files — when real clock and mocked `now` were in different ISO
|
||||
// weeks (which always happens at week boundaries), zero events
|
||||
// overlapped and the test flaked. The second test in this describe
|
||||
// (cross-week straddle) already used direct file writes for the
|
||||
// previous-week event for the same reason.
|
||||
const currentFile = path.join(dir, writer.computeFilename(now));
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
fs.appendFileSync(currentFile,
|
||||
JSON.stringify({ ts: inWin1, message: 'in window 1' }) + '\n' +
|
||||
JSON.stringify({ ts: inWin2, message: 'in window 2' }) + '\n' +
|
||||
JSON.stringify({ ts: outOfWin, message: 'out of window' }) + '\n',
|
||||
);
|
||||
|
||||
const recent = writer.readRecent(7, now);
|
||||
expect(recent.length).toBe(2);
|
||||
|
||||
@@ -0,0 +1,163 @@
|
||||
/**
|
||||
* Tests for parseRegisterClientArgs() in src/commands/auth.ts.
|
||||
*
|
||||
* v0.41.3 (T3): the pre-fix CLI parser used `args.indexOf('--flag')` which
|
||||
* silently took only the FIRST occurrence of a flag. That broke
|
||||
* `--redirect-uri A --redirect-uri B` (only A made it through). The rewrite
|
||||
* loops over argv and accumulates repeatable flags into arrays.
|
||||
*
|
||||
* Pure function — no DB, no fetch. The full register-client flow against
|
||||
* a live PGLite OAuth provider is covered in test/oauth.test.ts.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { parseRegisterClientArgs } from '../src/commands/auth.ts';
|
||||
|
||||
describe('parseRegisterClientArgs', () => {
|
||||
test('empty args → all defaults', () => {
|
||||
const out = parseRegisterClientArgs([]);
|
||||
expect(out.grantTypes).toEqual(['client_credentials']);
|
||||
expect(out.scopes).toBe('read');
|
||||
expect(out.sourceId).toBe('default');
|
||||
expect(out.federatedRead).toBeUndefined();
|
||||
expect(out.redirectUris).toEqual([]);
|
||||
expect(out.tokenEndpointAuthMethod).toBeUndefined();
|
||||
});
|
||||
|
||||
test('--grant-types comma-separated → array', () => {
|
||||
const out = parseRegisterClientArgs(['--grant-types', 'authorization_code,refresh_token']);
|
||||
expect(out.grantTypes).toEqual(['authorization_code', 'refresh_token']);
|
||||
});
|
||||
|
||||
test('--scopes preserves the whitespace-joined string', () => {
|
||||
const out = parseRegisterClientArgs(['--scopes', 'read write']);
|
||||
expect(out.scopes).toBe('read write');
|
||||
});
|
||||
|
||||
test('--source scopes the OAuth client', () => {
|
||||
const out = parseRegisterClientArgs(['--source', 'dept-x']);
|
||||
expect(out.sourceId).toBe('dept-x');
|
||||
});
|
||||
|
||||
test('--federated-read comma-separated → array', () => {
|
||||
const out = parseRegisterClientArgs(['--federated-read', 'dept-x,wecare,shared']);
|
||||
expect(out.federatedRead).toEqual(['dept-x', 'wecare', 'shared']);
|
||||
});
|
||||
|
||||
// T3 REGRESSION: pre-fix indexOf parser only took the first --redirect-uri
|
||||
describe('--redirect-uri (REPEATABLE — T3 regression)', () => {
|
||||
test('single --redirect-uri → single-element array', () => {
|
||||
const out = parseRegisterClientArgs(['--redirect-uri', 'https://claude.ai/api/mcp/auth_callback']);
|
||||
expect(out.redirectUris).toEqual(['https://claude.ai/api/mcp/auth_callback']);
|
||||
});
|
||||
|
||||
test('two --redirect-uri → both preserved', () => {
|
||||
// THE REGRESSION: pre-fix this returned only the first URI.
|
||||
const out = parseRegisterClientArgs([
|
||||
'--redirect-uri', 'https://claude.ai/api/mcp/auth_callback',
|
||||
'--redirect-uri', 'https://claude.com/api/mcp/auth_callback',
|
||||
]);
|
||||
expect(out.redirectUris).toEqual([
|
||||
'https://claude.ai/api/mcp/auth_callback',
|
||||
'https://claude.com/api/mcp/auth_callback',
|
||||
]);
|
||||
});
|
||||
|
||||
test('three --redirect-uri → all three preserved', () => {
|
||||
const out = parseRegisterClientArgs([
|
||||
'--redirect-uri', 'https://a.example/cb',
|
||||
'--redirect-uri', 'https://b.example/cb',
|
||||
'--redirect-uri', 'https://c.example/cb',
|
||||
]);
|
||||
expect(out.redirectUris).toHaveLength(3);
|
||||
});
|
||||
});
|
||||
|
||||
describe('--token-endpoint-auth-method', () => {
|
||||
test('omitted → undefined (provider applies RFC 7591 default)', () => {
|
||||
const out = parseRegisterClientArgs([]);
|
||||
expect(out.tokenEndpointAuthMethod).toBeUndefined();
|
||||
});
|
||||
|
||||
test('"none" → "none" (public PKCE client)', () => {
|
||||
const out = parseRegisterClientArgs(['--token-endpoint-auth-method', 'none']);
|
||||
expect(out.tokenEndpointAuthMethod).toBe('none');
|
||||
});
|
||||
|
||||
test('"client_secret_post" → "client_secret_post"', () => {
|
||||
const out = parseRegisterClientArgs(['--token-endpoint-auth-method', 'client_secret_post']);
|
||||
expect(out.tokenEndpointAuthMethod).toBe('client_secret_post');
|
||||
});
|
||||
|
||||
test('"client_secret_basic" → "client_secret_basic"', () => {
|
||||
const out = parseRegisterClientArgs(['--token-endpoint-auth-method', 'client_secret_basic']);
|
||||
expect(out.tokenEndpointAuthMethod).toBe('client_secret_basic');
|
||||
});
|
||||
|
||||
test('CLI parser does NOT validate the value — validator is on registerClientManual', () => {
|
||||
// Parser is shape-only. The validator runs at the registration boundary
|
||||
// so the same gate applies to CLI / admin / DCR. Putting validation in
|
||||
// the parser would mean DCR'd ApiClient strings bypass the same gate.
|
||||
const out = parseRegisterClientArgs(['--token-endpoint-auth-method', 'frobnicate']);
|
||||
expect(out.tokenEndpointAuthMethod).toBe('frobnicate');
|
||||
});
|
||||
});
|
||||
|
||||
describe('combination flows (worked examples from SECURITY.md)', () => {
|
||||
test('claude.ai pre-registration (confidential, two redirect URIs)', () => {
|
||||
const out = parseRegisterClientArgs([
|
||||
'--grant-types', 'authorization_code,refresh_token',
|
||||
'--scopes', 'read write',
|
||||
'--redirect-uri', 'https://claude.ai/api/mcp/auth_callback',
|
||||
'--redirect-uri', 'https://claude.com/api/mcp/auth_callback',
|
||||
]);
|
||||
expect(out.grantTypes).toEqual(['authorization_code', 'refresh_token']);
|
||||
expect(out.scopes).toBe('read write');
|
||||
expect(out.redirectUris).toHaveLength(2);
|
||||
expect(out.tokenEndpointAuthMethod).toBeUndefined();
|
||||
});
|
||||
|
||||
test('ChatGPT pre-registration (PKCE public client)', () => {
|
||||
const out = parseRegisterClientArgs([
|
||||
'--grant-types', 'authorization_code,refresh_token',
|
||||
'--scopes', 'read write',
|
||||
'--redirect-uri', 'https://chatgpt.com/connector/oauth/HASH',
|
||||
'--token-endpoint-auth-method', 'none',
|
||||
]);
|
||||
expect(out.grantTypes).toEqual(['authorization_code', 'refresh_token']);
|
||||
expect(out.redirectUris).toEqual(['https://chatgpt.com/connector/oauth/HASH']);
|
||||
expect(out.tokenEndpointAuthMethod).toBe('none');
|
||||
});
|
||||
|
||||
test('--redirect-uri without --grant-types → auto-infers authorization_code,refresh_token', () => {
|
||||
// Operator ergonomics: --redirect-uri without grant_types implies the
|
||||
// browser-OAuth flow; redundantly passing --grant-types is footgun.
|
||||
const out = parseRegisterClientArgs([
|
||||
'--redirect-uri', 'https://claude.ai/api/mcp/auth_callback',
|
||||
]);
|
||||
expect(out.grantTypes).toEqual(['authorization_code', 'refresh_token']);
|
||||
});
|
||||
|
||||
test('--redirect-uri + explicit --grant-types keeps the explicit set', () => {
|
||||
const out = parseRegisterClientArgs([
|
||||
'--grant-types', 'authorization_code', // no refresh
|
||||
'--redirect-uri', 'https://example.test/cb',
|
||||
]);
|
||||
expect(out.grantTypes).toEqual(['authorization_code']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('error cases', () => {
|
||||
test('--redirect-uri without value → throws', () => {
|
||||
expect(() => parseRegisterClientArgs(['--redirect-uri'])).toThrow(/requires a value/);
|
||||
});
|
||||
|
||||
test('--redirect-uri followed by another flag → throws (no greedy consume)', () => {
|
||||
expect(() => parseRegisterClientArgs(['--redirect-uri', '--scopes', 'read'])).toThrow(/requires a value/);
|
||||
});
|
||||
|
||||
test('unknown --flag throws', () => {
|
||||
expect(() => parseRegisterClientArgs(['--frobnicate', 'value'])).toThrow(/Unknown flag/);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -78,9 +78,20 @@ describe('v0.36.1.x #1077 — admin register-client supports PKCE public clients
|
||||
// (under either name) from req.body. Pin the fallback pattern so the
|
||||
// PKCE-fix regression contract stays load-bearing.
|
||||
expect(src).toMatch(/req\.body[^;]*scopes\s*\?\?\s*[^;]*scope\b/);
|
||||
// PKCE branch NULLs client_secret_hash + sets auth method to 'none'
|
||||
expect(src).toMatch(/tokenEndpointAuthMethod\s*===\s*'none'/);
|
||||
expect(src).toMatch(/client_secret_hash\s*=\s*NULL,\s*token_endpoint_auth_method\s*=\s*'none'/);
|
||||
// v0.41.3 (T4 atomicity fix, codex F4): admin endpoint now validates
|
||||
// tokenEndpointAuthMethod via the shared validator and passes it to
|
||||
// registerClientManual as a positional arg. Pre-v0.41.3 the route did
|
||||
// INSERT (confidential) → UPDATE (NULL out secret_hash) for the 'none'
|
||||
// case, which left a confidential row stranded if the UPDATE failed.
|
||||
// Atomic now: one INSERT writes the correct shape; no post-insert
|
||||
// UPDATE block (the regex deliberately asserts the post-insert UPDATE
|
||||
// is GONE).
|
||||
expect(src).toMatch(/validateTokenEndpointAuthMethod\(tokenEndpointAuthMethod\)/);
|
||||
expect(src).toMatch(/registerClientManual\([^)]*validatedAuthMethod[^)]*\)/);
|
||||
// Regression guard: post-insert UPDATE flipping client_secret_hash to
|
||||
// NULL based on a runtime check is exactly the non-atomic pattern T4
|
||||
// killed. Re-introducing it brings back codex F4.
|
||||
expect(src).not.toMatch(/UPDATE oauth_clients SET client_secret_hash = NULL, token_endpoint_auth_method = 'none'/);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -371,6 +371,66 @@ describe('http-transport: CORS', () => {
|
||||
expect(r.headers.get('access-control-allow-origin')).toBeNull();
|
||||
} finally { srv.stop(); }
|
||||
});
|
||||
|
||||
// v0.41.3 (T6 IRON RULE regression): pre-fix corsPreflightHeaders emitted
|
||||
// Access-Control-Allow-Methods + Access-Control-Allow-Headers to every
|
||||
// Origin unconditionally, leaking the API surface to attackers probing
|
||||
// OPTIONS. The fix consolidates corsHeaders + corsPreflightHeaders into
|
||||
// one function gated on the allowlist. These 4 cases pin the matrix.
|
||||
describe('CORS preflight (T6 — consolidated corsHeaders)', () => {
|
||||
test('preflight + no allowlist + any Origin → NO Allow-Methods/Headers', async () => {
|
||||
const srv = await startTest({});
|
||||
try {
|
||||
const r = await fetch(`${srv.url}/mcp`, {
|
||||
method: 'OPTIONS',
|
||||
headers: { 'Origin': 'https://evil.example', 'Access-Control-Request-Method': 'POST' },
|
||||
});
|
||||
expect(r.headers.get('access-control-allow-origin')).toBeNull();
|
||||
expect(r.headers.get('access-control-allow-methods')).toBeNull();
|
||||
expect(r.headers.get('access-control-allow-headers')).toBeNull();
|
||||
} finally { srv.stop(); }
|
||||
});
|
||||
|
||||
test('preflight + allowlist set + matching Origin → echoes ACAO + Methods + Headers', async () => {
|
||||
const srv = await startTest({ corsOrigin: 'https://claude.ai' });
|
||||
try {
|
||||
const r = await fetch(`${srv.url}/mcp`, {
|
||||
method: 'OPTIONS',
|
||||
headers: { 'Origin': 'https://claude.ai', 'Access-Control-Request-Method': 'POST' },
|
||||
});
|
||||
expect(r.headers.get('access-control-allow-origin')).toBe('https://claude.ai');
|
||||
expect(r.headers.get('access-control-allow-methods')).toContain('POST');
|
||||
expect(r.headers.get('access-control-allow-headers')).toContain('Authorization');
|
||||
expect(r.headers.get('vary')).toBe('Origin');
|
||||
} finally { srv.stop(); }
|
||||
});
|
||||
|
||||
test('preflight + allowlist set + NON-matching Origin → NO Allow-Methods/Headers (the regression)', async () => {
|
||||
const srv = await startTest({ corsOrigin: 'https://claude.ai' });
|
||||
try {
|
||||
const r = await fetch(`${srv.url}/mcp`, {
|
||||
method: 'OPTIONS',
|
||||
headers: { 'Origin': 'https://evil.example', 'Access-Control-Request-Method': 'POST' },
|
||||
});
|
||||
expect(r.headers.get('access-control-allow-origin')).toBeNull();
|
||||
// THE BUG: pre-fix these two headers leaked unconditionally
|
||||
expect(r.headers.get('access-control-allow-methods')).toBeNull();
|
||||
expect(r.headers.get('access-control-allow-headers')).toBeNull();
|
||||
} finally { srv.stop(); }
|
||||
});
|
||||
|
||||
test('actual POST + allowlist set + NON-matching Origin → NO ACAO (browser blocks downstream)', async () => {
|
||||
const srv = await startTest({ corsOrigin: 'https://claude.ai' });
|
||||
try {
|
||||
const r = await fetch(`${srv.url}/mcp`, {
|
||||
method: 'POST',
|
||||
headers: { 'Origin': 'https://evil.example', 'Authorization': 'Bearer x', 'Content-Type': 'application/json' },
|
||||
body: '{}',
|
||||
});
|
||||
expect(r.headers.get('access-control-allow-origin')).toBeNull();
|
||||
} finally { srv.stop(); }
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// --------------------------------------------------------------------------
|
||||
|
||||
@@ -44,9 +44,9 @@ describe('verifyConfidentialClientSecret (#1166)', () => {
|
||||
test('confidential client_secret_post: returns client on correct secret', async () => {
|
||||
const reg = await provider.registerClientManual('test-conf', ['authorization_code'], 'read write', ['https://example.test/cb']);
|
||||
expect(reg.clientId).toBeTruthy();
|
||||
expect(reg.clientSecret).toBeTruthy();
|
||||
expect(reg.clientSecret!).toBeTruthy();
|
||||
|
||||
const client = await provider.verifyConfidentialClientSecret(reg.clientId, reg.clientSecret);
|
||||
const client = await provider.verifyConfidentialClientSecret(reg.clientId, reg.clientSecret!);
|
||||
expect(client.client_id).toBe(reg.clientId);
|
||||
});
|
||||
|
||||
@@ -86,8 +86,8 @@ describe('verifyConfidentialClientSecret (#1166)', () => {
|
||||
|
||||
test('case-insensitive secret? NO — must be exact match', async () => {
|
||||
const reg = await provider.registerClientManual('test-case', ['authorization_code'], 'read', ['https://example.test/cb']);
|
||||
const wrongCase = reg.clientSecret.toUpperCase();
|
||||
if (wrongCase !== reg.clientSecret) {
|
||||
const wrongCase = reg.clientSecret!.toUpperCase();
|
||||
if (wrongCase !== reg.clientSecret!) {
|
||||
await expect(
|
||||
provider.verifyConfidentialClientSecret(reg.clientId, wrongCase),
|
||||
).rejects.toThrow(/Invalid client/);
|
||||
@@ -101,7 +101,7 @@ describe('verifyConfidentialClientSecret (#1166)', () => {
|
||||
[reg.clientId],
|
||||
);
|
||||
await expect(
|
||||
provider.verifyConfidentialClientSecret(reg.clientId, reg.clientSecret),
|
||||
provider.verifyConfidentialClientSecret(reg.clientId, reg.clientSecret!),
|
||||
).rejects.toThrow(/revoked/);
|
||||
});
|
||||
});
|
||||
@@ -116,7 +116,7 @@ describe('confidential-client full flow #1166', () => {
|
||||
`UPDATE oauth_clients SET grant_types = $1 WHERE client_id = $2`,
|
||||
[['client_credentials', 'refresh_token'], reg.clientId],
|
||||
);
|
||||
const initial = await provider.exchangeClientCredentials(reg.clientId, reg.clientSecret, 'read');
|
||||
const initial = await provider.exchangeClientCredentials(reg.clientId, reg.clientSecret!, 'read');
|
||||
// client_credentials grants don't issue refresh tokens (RFC 6749
|
||||
// 4.4.3), so we manually insert a refresh token to test the
|
||||
// verify-then-rotate path.
|
||||
@@ -129,7 +129,7 @@ describe('confidential-client full flow #1166', () => {
|
||||
);
|
||||
|
||||
// verify → exchange round-trip with the correct secret
|
||||
const client = await provider.verifyConfidentialClientSecret(reg.clientId, reg.clientSecret);
|
||||
const client = await provider.verifyConfidentialClientSecret(reg.clientId, reg.clientSecret!);
|
||||
const rotated = await provider.exchangeRefreshToken(client, refreshToken);
|
||||
expect(rotated.access_token).toBeTruthy();
|
||||
expect(rotated.refresh_token).toBeTruthy();
|
||||
|
||||
+193
-6
@@ -2,7 +2,13 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGlite } from '@electric-sql/pglite';
|
||||
import { vector } from '@electric-sql/pglite/vector';
|
||||
import { pg_trgm } from '@electric-sql/pglite/contrib/pg_trgm';
|
||||
import { GBrainOAuthProvider, coerceTimestamp } from '../src/core/oauth-provider.ts';
|
||||
import {
|
||||
GBrainOAuthProvider,
|
||||
coerceTimestamp,
|
||||
ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS,
|
||||
validateTokenEndpointAuthMethod,
|
||||
InvalidTokenEndpointAuthMethodError,
|
||||
} from '../src/core/oauth-provider.ts';
|
||||
import { hashToken, generateToken } from '../src/core/utils.ts';
|
||||
import { PGLITE_SCHEMA_SQL } from '../src/core/pglite-schema.ts';
|
||||
import { InvalidTokenError } from '@modelcontextprotocol/sdk/server/auth/errors.js';
|
||||
@@ -147,6 +153,7 @@ describe('client credentials', () => {
|
||||
'cc-test-agent', ['client_credentials'], 'read write',
|
||||
);
|
||||
clientId = result.clientId;
|
||||
if (!result.clientSecret) throw new Error('test bug: expected confidential client to have secret');
|
||||
clientSecret = result.clientSecret;
|
||||
});
|
||||
|
||||
@@ -194,7 +201,7 @@ describe('verifyAccessToken', () => {
|
||||
const { clientId, clientSecret } = await provider.registerClientManual(
|
||||
'verify-test', ['client_credentials'], 'read write',
|
||||
);
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret, 'read');
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret!, 'read');
|
||||
const authInfo = await provider.verifyAccessToken(tokens.access_token);
|
||||
|
||||
expect(authInfo.clientId).toBe(clientId);
|
||||
@@ -270,7 +277,7 @@ describe('verifyAccessToken', () => {
|
||||
const { clientId, clientSecret } = await provider.registerClientManual(
|
||||
'cascade-test', ['client_credentials'], 'read',
|
||||
);
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret, 'read');
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret!, 'read');
|
||||
await sql`DELETE FROM oauth_clients WHERE client_id = ${clientId}`;
|
||||
await expect(provider.verifyAccessToken(tokens.access_token)).rejects.toThrow('Invalid token');
|
||||
});
|
||||
@@ -282,7 +289,7 @@ describe('verifyAccessToken', () => {
|
||||
const { clientId, clientSecret } = await provider.registerClientManual(
|
||||
'typeof-test', ['client_credentials'], 'read',
|
||||
);
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret, 'read');
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret!, 'read');
|
||||
const authInfo = await provider.verifyAccessToken(tokens.access_token);
|
||||
|
||||
expect(typeof authInfo.expiresAt).toBe('number');
|
||||
@@ -314,7 +321,7 @@ describe('revokeToken', () => {
|
||||
const { clientId, clientSecret } = await provider.registerClientManual(
|
||||
'revoke-test', ['client_credentials'], 'read',
|
||||
);
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret, 'read');
|
||||
const tokens = await provider.exchangeClientCredentials(clientId, clientSecret!, 'read');
|
||||
|
||||
// Verify token works
|
||||
const authInfo = await provider.verifyAccessToken(tokens.access_token);
|
||||
@@ -788,7 +795,7 @@ describe('F1/F4 cross-client isolation', () => {
|
||||
const { clientId: attackerId } = await provider.registerClientManual(
|
||||
'revoke-attacker-test', ['client_credentials'], 'read',
|
||||
);
|
||||
const tokens = await provider.exchangeClientCredentials(ownerId, ownerSecret, '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
|
||||
@@ -1283,3 +1290,183 @@ describe('PKCE DCR public-client gate (#909)', () => {
|
||||
expect(String(tokens.token_type).toLowerCase()).toBe('bearer');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// v0.41.3 — T1: ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS + validator
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('v0.41.3 ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS', () => {
|
||||
test('Set contains exactly the three SDK-advertised methods', () => {
|
||||
expect(ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS.size).toBe(3);
|
||||
expect(ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS.has('client_secret_post')).toBe(true);
|
||||
expect(ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS.has('client_secret_basic')).toBe(true);
|
||||
expect(ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS.has('none')).toBe(true);
|
||||
});
|
||||
|
||||
test('client_secret_basic is included — codex F3 regression', () => {
|
||||
// The codex outside-voice review caught that omitting client_secret_basic
|
||||
// would break operators using HTTP Basic for confidential client auth at
|
||||
// the /token endpoint (server already supports it at serve-http.ts:468).
|
||||
expect(ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS.has('client_secret_basic')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('v0.41.3 validateTokenEndpointAuthMethod', () => {
|
||||
test('undefined → "client_secret_post" (RFC 7591 default)', () => {
|
||||
expect(validateTokenEndpointAuthMethod(undefined)).toBe('client_secret_post');
|
||||
});
|
||||
|
||||
test('null → "client_secret_post"', () => {
|
||||
expect(validateTokenEndpointAuthMethod(null)).toBe('client_secret_post');
|
||||
});
|
||||
|
||||
test('empty string → "client_secret_post"', () => {
|
||||
expect(validateTokenEndpointAuthMethod('')).toBe('client_secret_post');
|
||||
});
|
||||
|
||||
test('"client_secret_post" → "client_secret_post"', () => {
|
||||
expect(validateTokenEndpointAuthMethod('client_secret_post')).toBe('client_secret_post');
|
||||
});
|
||||
|
||||
test('"client_secret_basic" → "client_secret_basic"', () => {
|
||||
expect(validateTokenEndpointAuthMethod('client_secret_basic')).toBe('client_secret_basic');
|
||||
});
|
||||
|
||||
test('"none" → "none" (public PKCE client)', () => {
|
||||
expect(validateTokenEndpointAuthMethod('none')).toBe('none');
|
||||
});
|
||||
|
||||
test('unknown method throws InvalidTokenEndpointAuthMethodError', () => {
|
||||
expect(() => validateTokenEndpointAuthMethod('frobnicate')).toThrow(InvalidTokenEndpointAuthMethodError);
|
||||
});
|
||||
|
||||
test('error message names the bad value + all allowed methods', () => {
|
||||
try {
|
||||
validateTokenEndpointAuthMethod('frobnicate');
|
||||
throw new Error('should have thrown');
|
||||
} catch (e: any) {
|
||||
expect(e.message).toContain('frobnicate');
|
||||
expect(e.message).toContain('client_secret_post');
|
||||
expect(e.message).toContain('client_secret_basic');
|
||||
expect(e.message).toContain('none');
|
||||
}
|
||||
});
|
||||
|
||||
test('non-string input throws', () => {
|
||||
expect(() => validateTokenEndpointAuthMethod(123 as any)).toThrow(InvalidTokenEndpointAuthMethodError);
|
||||
expect(() => validateTokenEndpointAuthMethod({} as any)).toThrow(InvalidTokenEndpointAuthMethodError);
|
||||
expect(() => validateTokenEndpointAuthMethod([] as any)).toThrow(InvalidTokenEndpointAuthMethodError);
|
||||
});
|
||||
|
||||
test('InvalidTokenEndpointAuthMethodError has stable error code', () => {
|
||||
try {
|
||||
validateTokenEndpointAuthMethod('xyz');
|
||||
throw new Error('should have thrown');
|
||||
} catch (e: any) {
|
||||
expect(e.code).toBe('invalid_token_endpoint_auth_method');
|
||||
expect(e.name).toBe('InvalidTokenEndpointAuthMethodError');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// v0.41.3 — T2: registerClientManual tokenEndpointAuthMethod parameter
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('v0.41.3 registerClientManual tokenEndpointAuthMethod', () => {
|
||||
test('omitted → confidential client with secret (back-compat)', async () => {
|
||||
const result = await provider.registerClientManual(
|
||||
'v413-default-test', ['client_credentials'], 'read',
|
||||
);
|
||||
expect(result.clientId).toStartWith('gbrain_cl_');
|
||||
expect(result.clientSecret).toBeDefined();
|
||||
expect(result.clientSecret!).toStartWith('gbrain_cs_');
|
||||
});
|
||||
|
||||
test('explicit client_secret_post → confidential client with secret', async () => {
|
||||
const result = await provider.registerClientManual(
|
||||
'v413-csp-test', ['client_credentials'], 'read', [], 'default', undefined, 'client_secret_post',
|
||||
);
|
||||
expect(result.clientSecret).toBeDefined();
|
||||
});
|
||||
|
||||
test('explicit client_secret_basic → confidential client with secret', async () => {
|
||||
const result = await provider.registerClientManual(
|
||||
'v413-csb-test', ['client_credentials'], 'read', [], 'default', undefined, 'client_secret_basic',
|
||||
);
|
||||
expect(result.clientSecret).toBeDefined();
|
||||
});
|
||||
|
||||
test('"none" → public client with NO secret (T2 atomic INSERT)', async () => {
|
||||
// The pre-v0.41.3 admin endpoint did INSERT (confidential) → UPDATE
|
||||
// (NULL out secret_hash) for the 'none' case, leaving a confidential
|
||||
// row stranded if the UPDATE failed (codex F4). T2 moves this into
|
||||
// registerClientManual itself as a single atomic INSERT.
|
||||
const result = await provider.registerClientManual(
|
||||
'v413-public-test', ['authorization_code'], 'read',
|
||||
['https://example.test/cb'], 'default', undefined, 'none',
|
||||
);
|
||||
expect(result.clientId).toStartWith('gbrain_cl_');
|
||||
expect(result.clientSecret).toBeUndefined();
|
||||
|
||||
// Verify the stored row has client_secret_hash = NULL (public client shape)
|
||||
const client = await provider.clientsStore.getClient(result.clientId);
|
||||
expect(client).toBeDefined();
|
||||
expect(client!.client_secret).toBeUndefined();
|
||||
expect(client!.token_endpoint_auth_method).toBe('none');
|
||||
});
|
||||
|
||||
test('unknown auth method throws InvalidTokenEndpointAuthMethodError at registration boundary', async () => {
|
||||
await expect(
|
||||
provider.registerClientManual(
|
||||
'v413-bad-test', ['client_credentials'], 'read', [], 'default', undefined, 'frobnicate',
|
||||
),
|
||||
).rejects.toThrow(InvalidTokenEndpointAuthMethodError);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// v0.41.3 — T5: DCR /register handler applies the same validator
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('v0.41.3 DCR validator (T5)', () => {
|
||||
test('DCR rejects unknown token_endpoint_auth_method — closes --enable-dcr loose path', async () => {
|
||||
// Pre-v0.41.3 the DCR registration handler defaulted to 'client_secret_post'
|
||||
// for any unknown value, silently swallowing typos. T5 throws so the bad
|
||||
// input fails loud — same gate as CLI + admin paths.
|
||||
await expect(
|
||||
provider.clientsStore.registerClient!({
|
||||
client_name: 'dcr-bad-test',
|
||||
grant_types: ['authorization_code'],
|
||||
scope: 'read',
|
||||
redirect_uris: ['https://example.test/cb'],
|
||||
token_endpoint_auth_method: 'frobnicate',
|
||||
} as any),
|
||||
).rejects.toThrow(InvalidTokenEndpointAuthMethodError);
|
||||
});
|
||||
|
||||
test('DCR accepts "none" → public PKCE client', async () => {
|
||||
const reg = await provider.clientsStore.registerClient!({
|
||||
client_name: 'dcr-public-test',
|
||||
grant_types: ['authorization_code'],
|
||||
scope: 'read',
|
||||
redirect_uris: ['https://example.test/cb'],
|
||||
token_endpoint_auth_method: 'none',
|
||||
} as any);
|
||||
expect(reg.client_id).toStartWith('gbrain_cl_');
|
||||
// RFC 7591 §3.2.1: public clients MUST NOT receive a client_secret
|
||||
expect(reg.client_secret).toBeUndefined();
|
||||
});
|
||||
|
||||
test('DCR accepts "client_secret_basic" — codex F3 regression', async () => {
|
||||
const reg = await provider.clientsStore.registerClient!({
|
||||
client_name: 'dcr-basic-test',
|
||||
grant_types: ['client_credentials'],
|
||||
scope: 'read',
|
||||
redirect_uris: [],
|
||||
token_endpoint_auth_method: 'client_secret_basic',
|
||||
} as any);
|
||||
expect(reg.client_id).toStartWith('gbrain_cl_');
|
||||
expect(reg.client_secret).toStartWith('gbrain_cs_');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
/**
|
||||
* Tests for parseCorsAllowlistOAuth() and resolveCorsOrigin() in
|
||||
* src/commands/serve-http.ts.
|
||||
*
|
||||
* v0.41.3 (T7): pre-fix every OAuth endpoint (/mcp, /token, /authorize,
|
||||
* /register, /revoke) used bare `cors()` which defaults to
|
||||
* Access-Control-Allow-Origin: * — any web origin could complete a token
|
||||
* exchange from a logged-in operator's browser. The fix gates every OAuth
|
||||
* surface behind GBRAIN_HTTP_CORS_ORIGIN with default-deny.
|
||||
*
|
||||
* Two pure functions, no Express integration needed for the unit shape.
|
||||
* The end-to-end Express-router behavior (cors middleware + browser
|
||||
* preflight) is verified by test/e2e/serve-http-oauth.test.ts.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { parseCorsAllowlistOAuth, resolveCorsOrigin } from '../src/commands/serve-http.ts';
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
|
||||
describe('parseCorsAllowlistOAuth', () => {
|
||||
test('unset → null (default-deny posture)', async () => {
|
||||
await withEnv({ GBRAIN_HTTP_CORS_ORIGIN: undefined }, async () => {
|
||||
expect(parseCorsAllowlistOAuth()).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
test('empty string → null', async () => {
|
||||
await withEnv({ GBRAIN_HTTP_CORS_ORIGIN: '' }, async () => {
|
||||
expect(parseCorsAllowlistOAuth()).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
test('whitespace-only → null (no usable origins)', async () => {
|
||||
await withEnv({ GBRAIN_HTTP_CORS_ORIGIN: ' , ,' }, async () => {
|
||||
expect(parseCorsAllowlistOAuth()).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
test('single origin → Set of one', async () => {
|
||||
await withEnv({ GBRAIN_HTTP_CORS_ORIGIN: 'https://claude.ai' }, async () => {
|
||||
const set = parseCorsAllowlistOAuth();
|
||||
expect(set).not.toBeNull();
|
||||
expect(set!.size).toBe(1);
|
||||
expect(set!.has('https://claude.ai')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('comma-separated origins → Set of N', async () => {
|
||||
await withEnv({ GBRAIN_HTTP_CORS_ORIGIN: 'https://claude.ai,https://chatgpt.com,https://my.app' }, async () => {
|
||||
const set = parseCorsAllowlistOAuth();
|
||||
expect(set!.size).toBe(3);
|
||||
expect(set!.has('https://claude.ai')).toBe(true);
|
||||
expect(set!.has('https://chatgpt.com')).toBe(true);
|
||||
expect(set!.has('https://my.app')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('whitespace around values is trimmed', async () => {
|
||||
await withEnv({ GBRAIN_HTTP_CORS_ORIGIN: ' https://a.app , https://b.app ' }, async () => {
|
||||
const set = parseCorsAllowlistOAuth();
|
||||
expect(set!.has('https://a.app')).toBe(true);
|
||||
expect(set!.has('https://b.app')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('case-sensitive match (Origin headers are case-sensitive per RFC 6454)', async () => {
|
||||
await withEnv({ GBRAIN_HTTP_CORS_ORIGIN: 'https://Claude.AI' }, async () => {
|
||||
const set = parseCorsAllowlistOAuth();
|
||||
expect(set!.has('https://Claude.AI')).toBe(true);
|
||||
expect(set!.has('https://claude.ai')).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveCorsOrigin', () => {
|
||||
test('null allowlist → false (cors middleware sends no Allow-Origin)', () => {
|
||||
expect(resolveCorsOrigin(null)).toBe(false);
|
||||
});
|
||||
|
||||
test('allowlist + missing Origin → cb(null, true) (same-origin requests aren\'t cross-origin)', () => {
|
||||
const fn = resolveCorsOrigin(new Set(['https://claude.ai']));
|
||||
expect(typeof fn).toBe('function');
|
||||
const calls: Array<{err: Error | null; allow?: boolean}> = [];
|
||||
(fn as Function)(undefined, (err: Error | null, allow?: boolean) => calls.push({err, allow}));
|
||||
expect(calls).toHaveLength(1);
|
||||
expect(calls[0].err).toBeNull();
|
||||
expect(calls[0].allow).toBe(true);
|
||||
});
|
||||
|
||||
test('allowlist + matching Origin → cb(null, true)', () => {
|
||||
const fn = resolveCorsOrigin(new Set(['https://claude.ai']));
|
||||
const calls: Array<{err: Error | null; allow?: boolean}> = [];
|
||||
(fn as Function)('https://claude.ai', (err: Error | null, allow?: boolean) => calls.push({err, allow}));
|
||||
expect(calls[0].allow).toBe(true);
|
||||
});
|
||||
|
||||
test('allowlist + NON-matching Origin → cb(null, false) — the regression', () => {
|
||||
const fn = resolveCorsOrigin(new Set(['https://claude.ai']));
|
||||
const calls: Array<{err: Error | null; allow?: boolean}> = [];
|
||||
(fn as Function)('https://evil.example', (err: Error | null, allow?: boolean) => calls.push({err, allow}));
|
||||
expect(calls[0].err).toBeNull();
|
||||
expect(calls[0].allow).toBe(false);
|
||||
});
|
||||
|
||||
test('multi-origin allowlist + match → true', () => {
|
||||
const fn = resolveCorsOrigin(new Set(['https://claude.ai', 'https://chatgpt.com']));
|
||||
const calls: Array<boolean | undefined> = [];
|
||||
(fn as Function)('https://chatgpt.com', (_err: unknown, allow?: boolean) => calls.push(allow));
|
||||
expect(calls[0]).toBe(true);
|
||||
});
|
||||
|
||||
test('case-sensitive — "https://Claude.AI" does NOT match "https://claude.ai"', () => {
|
||||
const fn = resolveCorsOrigin(new Set(['https://claude.ai']));
|
||||
const calls: Array<boolean | undefined> = [];
|
||||
(fn as Function)('https://Claude.AI', (_err: unknown, allow?: boolean) => calls.push(allow));
|
||||
expect(calls[0]).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,83 @@
|
||||
/**
|
||||
* Tests for resolveTrustProxy() in src/commands/serve-http.ts.
|
||||
*
|
||||
* v0.41.3 (T8): GBRAIN_HTTP_TRUST_PROXY env var replaces the pre-fix hardcoded
|
||||
* `app.set('trust proxy', 'loopback')`. The Express trust-proxy value
|
||||
* determines whether X-Forwarded-For is honored (rate limit IP correctness)
|
||||
* and whether req.secure detects HTTPS termination at a proxy.
|
||||
*
|
||||
* Pure function — no Express, no fetch, no env mutation. Each case calls
|
||||
* resolveTrustProxy directly with the env string it would have read.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { resolveTrustProxy } from '../src/commands/serve-http.ts';
|
||||
|
||||
describe('resolveTrustProxy', () => {
|
||||
test('unset → "loopback" (pre-v0.41.3 default)', () => {
|
||||
expect(resolveTrustProxy(undefined)).toBe('loopback');
|
||||
});
|
||||
|
||||
test('empty string → "loopback" (env was set but blank, treat as unset)', () => {
|
||||
expect(resolveTrustProxy('')).toBe('loopback');
|
||||
});
|
||||
|
||||
test('"0" → false (trust nothing — defeat X-Forwarded-For spoofing)', () => {
|
||||
expect(resolveTrustProxy('0')).toBe(false);
|
||||
});
|
||||
|
||||
test('"false" → false', () => {
|
||||
expect(resolveTrustProxy('false')).toBe(false);
|
||||
});
|
||||
|
||||
test('"1" → 1 (trust exactly one hop — Fly.io / Render / single-layer proxy)', () => {
|
||||
expect(resolveTrustProxy('1')).toBe(1);
|
||||
});
|
||||
|
||||
test('"true" → 1', () => {
|
||||
expect(resolveTrustProxy('true')).toBe(1);
|
||||
});
|
||||
|
||||
test('"2" → 2 (trust two hops — Cloudflare → nginx → gbrain)', () => {
|
||||
expect(resolveTrustProxy('2')).toBe(2);
|
||||
});
|
||||
|
||||
test('"10" → 10 (deep proxy chain)', () => {
|
||||
expect(resolveTrustProxy('10')).toBe(10);
|
||||
});
|
||||
|
||||
test('"loopback" → "loopback" (explicit pass-through)', () => {
|
||||
expect(resolveTrustProxy('loopback')).toBe('loopback');
|
||||
});
|
||||
|
||||
test('"uniquelocal" → "uniquelocal" (Express named mode)', () => {
|
||||
expect(resolveTrustProxy('uniquelocal')).toBe('uniquelocal');
|
||||
});
|
||||
|
||||
test('"linklocal" → "linklocal" (Express named mode)', () => {
|
||||
expect(resolveTrustProxy('linklocal')).toBe('linklocal');
|
||||
});
|
||||
|
||||
test('CIDR list passes through verbatim (Express parses it)', () => {
|
||||
expect(resolveTrustProxy('10.0.0.0/8,192.168.1.0/24')).toBe('10.0.0.0/8,192.168.1.0/24');
|
||||
});
|
||||
|
||||
test('garbage string passes through (Express will reject at startup if invalid)', () => {
|
||||
// Fail-loud strategy: don't silently fall back to a default on garbage.
|
||||
// Express's IP filter will throw at boot, surfacing the typo immediately
|
||||
// rather than silently producing an unexpected security posture.
|
||||
expect(resolveTrustProxy('frobnicate')).toBe('frobnicate');
|
||||
});
|
||||
|
||||
test('numeric string with leading zero ("007") parses as 7', () => {
|
||||
// /^\d+$/ matches; parseInt accepts.
|
||||
expect(resolveTrustProxy('007')).toBe(7);
|
||||
});
|
||||
|
||||
test('"-1" passes through as string (not numeric — Express rejects)', () => {
|
||||
// The /^\d+$/ regex deliberately excludes negative numbers; pass-through
|
||||
// means Express sees an invalid value and throws at boot rather than
|
||||
// silently treating it as 1 or false.
|
||||
expect(resolveTrustProxy('-1')).toBe('-1');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user