diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f5e7626d..7d8bd94a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,50 @@ All notable changes to GBrain will be documented in this file. +## [0.35.3.0] - 2026-05-15 + +**Fix wave: 19 stale community PRs land as one bisect-friendly PR with the architectural fix none of them surfaced.** + +Two real bugs, both in shipping code on master since v0.28+, both fixed at the architecture level instead of one site at a time. The first: `extract_facts.entity_hints` and the X-API resolver's `candidates` output schema both declared `type: 'array'` without `items`, so strict-mode validators (Gemini Pro structured outputs, OpenAI strict tool defs) rejected the entire tool schema. Twelve community PRs converged on the entity_hints fix; only one (#910 from @DmitryBMsk) caught the candidates side. The second: every remote-source `git clone` and `git pull` invocation has been broken for ~7 months because `--no-recurse-submodules` was spliced BEFORE the subcommand verb in `GIT_SSRF_FLAGS`. Real git rejects this with exit 129 ("unknown option"); the fake-git test harness exits 0 regardless of argv shape, so CI never caught it. Seven community PRs flagged this. Codex outside-voice review on top of all 19 PRs surfaced the structural finding none of them saw: three duplicate `ParamDef→JSON Schema` mappers existed across the MCP surface, not one. The live HTTP MCP `tools/list` path (`serve-http.ts:837`) and the subagent brain-tool registry (`brain-allowlist.ts:84`) would both have stayed broken after a `buildToolDefs`-only patch. + +### What you can now do + +**Use `extract_facts` from strict-mode agents again.** Gemini Pro structured outputs and OpenAI strict tool definitions both reject `type: 'array'` without `items`. The `entity_hints` param now declares `items: { type: 'string' }` and the resolver `candidates` output declares the full `XTweetCandidate` interface with `required: [tweet_id, text, created_at, score, url]` and `additionalProperties: false`. The schema matches the TypeScript interface byte-for-byte — single source of truth. + +**Clone and pull remote sources over real git again.** `cloneRepo` and `pullRepo` now spread `--no-recurse-submodules` AFTER the verb where it belongs. Real git stops rejecting the call with exit 129. Constant naming signals the position rule so future flag additions land in the right place: `GIT_SSRF_FLAGS` is global config (spread before the verb), `GIT_SSRF_SUBCOMMAND_FLAGS` is subcommand-scoped (spread after). + +**Get one canonical ParamDef→schema mapper.** New `paramDefToSchema(p: ParamDef)` helper exported from `src/mcp/tool-defs.ts`. Three consumers now share one source of truth: `buildToolDefs` (stdio MCP), `serve-http.ts:837` (HTTP MCP `tools/list`), and `brain-allowlist.ts:84` (subagent tool registry). Recursive on `items` so nested array-of-arrays preserves inner shape on the wire — closes the same bug class one layer deeper. + +**Trust structural guards in CI.** Three new tests fail loudly with property paths if any future array drift reintroduces the bug class. `test/mcp-tool-defs.test.ts` walks every operation's `inputSchema` recursively. `test/git-remote.test.ts` asserts `--no-recurse-submodules` indexOf > verb indexOf — position-anchored, not just inclusion. `test/resolvers.test.ts` explicitly imports `xHandleToTweetResolver` + `urlReachableResolver` and walks both `inputSchema` AND `outputSchema` (the previously planned `getDefaultRegistry()` walk would have silently passed against zero resolvers — codex catch). + +### Itemized changes + +- `src/mcp/tool-defs.ts` exports new recursive `paramDefToSchema(p: ParamDef)` helper. Key ordering (type, description, enum, default, items) is intentional — matches the pre-v0.35.3 inline mappers so JSON.stringify output stays byte-stable for every operation that doesn't use nested arrays. +- `src/commands/serve-http.ts:837-849` swaps the inline ParamDef destructure for `paramDefToSchema(v)`. Closes the HTTP MCP `tools/list` bug. OAuth-authenticated remote agents (Claude Desktop, ChatGPT, Perplexity over HTTP MCP) now see `items` on every array param. +- `src/core/minions/tools/brain-allowlist.ts:84-97` swaps the inline destructure inside `paramsToInputSchema()` for `paramDefToSchema(v)`. Preserves the `required:` aggregation at `:95` (that's at the tool-def level, not the param level — codex explicitly out-of-scope). +- `src/core/operations.ts:2699` adds `items: { type: 'string' }` to `entity_hints`. The handler at `:2733` already coerced with `Array.isArray(...)` so runtime shape is unchanged; only the schema declaration was broken. +- `src/core/resolvers/builtin/x-api/handle-to-tweet.ts:102` replaces `candidates: { type: 'array' }` with full-spec items matching the `XTweetCandidate` interface 10 lines above, including `required: [all 5 fields]` and `additionalProperties: false` (D5 decision: without `required`, schema permits `{}`). +- `src/core/git-remote.ts:29-44` splits `GIT_SSRF_FLAGS` (3 `-c` config flags, spread BEFORE the verb) from new exported `GIT_SSRF_SUBCOMMAND_FLAGS = ['--no-recurse-submodules']` (spread AFTER). `cloneRepo:156` and `pullRepo:182` now spread the new constant in subcommand position. +- `test/mcp-tool-defs.test.ts` adds: (a) explicit fixture for `extract_facts.entity_hints.items.type === 'string'`, (b) synthetic nested-array ParamDef pinning `items.items.type` recursion, (c) `findArrayWithoutItems` walker that fails the suite with a property path on any `type: 'array'` lacking `items.type`. `legacyInlineMap` reference mirrors the new recursive helper. +- `test/git-remote.test.ts` snapshot split: `GIT_SSRF_FLAGS` pins to 3 elements (no submodules), new `GIT_SSRF_SUBCOMMAND_FLAGS` snapshot pins to 1. `cloneRepo` + `pullRepo` argv tests assert `indexOf(--no-recurse-submodules) > indexOf(verb)` — position-anchored regression guard. Pre-v0.35.3 the existing test at `:233` baked the bug in via `argv.slice(0, GIT_SSRF_FLAGS.length)`. +- `test/resolvers.test.ts` (existing file) gets a new `describe` block. Explicitly imports `xHandleToTweetResolver` + `urlReachableResolver` and walks both `inputSchema` AND `outputSchema` recursively. Negative coverage guard asserts `builtins.length >= 2` so a future autoformatter dropping the array can't silently turn the walk into a no-op. +- `test/e2e/zeroentropy-live.test.ts` raises rerank test timeouts to handle ZeroEntropy's cold-start latency (observed ~5-6s on Tier 2 runners; subsequent calls < 500ms). Passes explicit `timeoutMs: 25000` to each rerank() call and a 30s bun:test per-test timeout. Production `DEFAULT_RERANK_TIMEOUT_MS = 5000` in `gateway.ts` stays put for the search hot path. +- Contributed by: @DmitryBMsk (PR #910 — deepest variant of the entity_hints + candidates double-fix); cleanest naming from PR #846 (`GIT_SSRF_SUBCOMMAND_FLAGS`). 17 superseded PRs being closed with thank-you notes after this merge: #1028, #1023, #1020, #999, #985, #980, #979, #963, #904, #863, #862, #847, #846, #842, #832, #812. + +## To take advantage of v0.35.3.0 + +`gbrain upgrade` is all you need. There's no schema migration, no config change, no manual action. + +1. Run `gbrain upgrade` — the new tool schemas reach your MCP clients on next restart. +2. Restart your MCP clients (Claude Code, Claude Desktop, ChatGPT, Cursor, etc.) so they re-fetch `tools/list`. +3. Verify `extract_facts` works from strict-mode agents: + ```bash + gbrain --tools-json | jq '.tools[] | select(.name == "extract_facts") | .inputSchema.properties.entity_hints' + ``` + Should show `{"type":"array","items":{"type":"string"},...}` — pre-fix, `items` was missing. +4. If you use remote-source git clones (`gbrain config get sources` shows any with a remote URL), they'll start working again on the next `gbrain sync`. Pre-fix, every clone of a remote-source repository was silently failing with git exit 129. + +If `gbrain doctor` flags anything after upgrade, please file an issue with the doctor output. This was a 7-month silent bug — the doctor's structural guards should catch the next one before it ships. ## [0.35.1.1] - 2026-05-16 **Fix wave: `gbrain eval longmemeval` actually runs against the public _s split.** diff --git a/CLAUDE.md b/CLAUDE.md index 2be8b7ed6..8e1e02c12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,7 @@ strict behavior when unset. - `src/core/storage.ts` — Pluggable storage interface (S3, Supabase Storage, local) - `src/core/storage-config.ts` (v0.22.11) — Storage tiering: `loadStorageConfig` reads `gbrain.yml`, normalizes deprecated keys (`git_tracked` / `supabase_only`) to canonical (`db_tracked` / `db_only`) with once-per-process deprecation warning, and runs `normalizeAndValidateStorageConfig` (auto-fixes missing trailing `/`, throws `StorageConfigError` on tier overlap). Path-segment matcher: `media/x/` does NOT match `media/xerox/foo`. Replaces gray-matter (broken on delimiter-less YAML) with a dedicated parser for the `gbrain.yml` shape. - `src/core/disk-walk.ts` (v0.22.11) — `walkBrainRepo(repoPath)` returns `Map` from one recursive `readdirSync`. Skips dot-dirs, `node_modules`, non-`.md` files. Used by `gbrain storage status` to replace per-page `existsSync + statSync` (~400K syscalls on 200K-page brains → tens). +- `src/core/git-remote.ts` (v0.35.3.0) — SSRF-hardened git invocations for remote-source `cloneRepo` and `pullRepo`. Exports two distinct flag constants because `git`'s argv grammar treats them differently: `GIT_SSRF_FLAGS` (3 `-c` config flags — `protocol.allow=user`, `protocol.file.allow=never`, `http.allowRedirects=false`) is **global config**, spread BEFORE the subcommand verb. New `GIT_SSRF_SUBCOMMAND_FLAGS = ['--no-recurse-submodules']` is **subcommand-scoped**, spread AFTER the verb. Pre-v0.35.3 a single combined `GIT_SSRF_FLAGS` array spread `--no-recurse-submodules` before the verb where real git rejects it with exit 129 ("unknown option"); the fake-git test harness exited 0 regardless of argv shape, so CI missed it for ~7 months and every remote-source clone/pull was silently broken. `cloneRepo` argv: `git clone --depth=1 [--branch X] -- `. `pullRepo` argv: `git -C pull --ff-only`. Pinned by `test/git-remote.test.ts` position-anchored regression guard (`argv.indexOf('--no-recurse-submodules') > argv.indexOf(verb)`). - `src/commands/storage.ts` (v0.22.11) — `gbrain storage status [--repo P] [--json]`. Split into pure data (`getStorageStatus`) + JSON formatter + human formatter (ASCII-only per D10) matching the `orphans.ts` pattern. `PageCountsByTier` and `DiskUsageByTier` are distinct nominal types so swaps fail at compile time. - `gbrain.yml` (brain repo root, v0.22.11) — Optional storage tiering config. Top-level `storage:` section with `db_tracked:` and `db_only:` array-valued keys. `gbrain sync` auto-manages `.gitignore` for `db_only` paths on successful sync (skips on dry-run, blocked-by-failures, submodule context, or `GBRAIN_NO_GITIGNORE=1`). `gbrain export --restore-only [--repo P] [--type T] [--slug-prefix S]` repopulates missing `db_only` files from the database. - `src/core/supabase-admin.ts` — Supabase admin API (project discovery, pgvector check) @@ -149,8 +150,8 @@ strict behavior when unset. - `src/core/minions/wait-for-completion.ts` (v0.15) — poll-until-terminal helper for CLI callers. `TimeoutError` does NOT cancel the job; `AbortSignal` exits without throwing. Default `pollMs`: 1000 on Postgres, 250 on PGLite inline. - `src/core/minions/transcript.ts` (v0.15) — renders `subagent_messages` + `subagent_tool_executions` to markdown. Tool rows splice under their owning assistant `tool_use` by `tool_use_id`. UTF-8-safe truncation; unknown block types fall through to fenced JSON. - `src/core/minions/plugin-loader.ts` (v0.15) — `GBRAIN_PLUGIN_PATH` discovery. Absolute paths only, left-wins collision, `gbrain.plugin.json` with `plugin_version: "gbrain-plugin-v1"`, plugins ship DEFS only (no new tools), `allowed_tools:` validated at load time against the derived registry. -- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. -- `src/mcp/tool-defs.ts` (v0.15) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. +- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29, v0.35.3.0) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. **v0.35.3.0:** `paramsToInputSchema()` now consumes `paramDefToSchema` from `src/mcp/tool-defs.ts` instead of its own inline destructure. Required-aggregation at the tool-def level stays here (out of scope for the shared helper, which is per-param). Closes the third drift site in the ParamDef→JSON Schema bug class. +- `src/mcp/tool-defs.ts` (v0.15, v0.35.3.0) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. **v0.35.3.0:** exports the new recursive `paramDefToSchema(p: ParamDef)` helper — single source of truth for ParamDef→JSON Schema mapping. Three consumers now share one mapper: `buildToolDefs` (stdio MCP), `src/commands/serve-http.ts:837` (HTTP MCP `tools/list`), and `src/core/minions/tools/brain-allowlist.ts:84` (subagent tool registry). Pre-v0.35.3, three inline destructures had drifted across the surface — the live HTTP MCP path dropped `items` on every array param after a v0.32 review caught only the stdio side. Recursive on `items` so nested array-of-arrays preserves inner shape on the wire. Key ordering (type, description, enum, default, items) is intentional — matches the pre-v0.35.3 inline mappers so JSON.stringify output stays byte-stable. `test/mcp-tool-defs.test.ts` adds a `findArrayWithoutItems` walker that fails the suite with a property path on any future `type: 'array'` lacking `items.type`. - `src/core/minions/attachments.ts` — Attachment validation (path traversal, null byte, oversize, base64, duplicate detection) - `src/commands/agent.ts` (v0.16) — `gbrain agent run [flags]` CLI. Submits `subagent` (or N children + 1 aggregator) under `{allowProtectedSubmit: true}`. Single-entry `--fanout-manifest` short-circuits. Children get `on_child_fail: 'continue'` + `max_stalled: 3`. `--follow` is the default on TTY; streams logs + polls `waitForCompletion` in parallel. Ctrl-C detaches, does not cancel. - `src/commands/agent-logs.ts` (v0.16) — `gbrain agent logs [--follow] [--since]`. Merges JSONL heartbeat audit + `subagent_messages` into a chronological timeline. `parseSince` accepts ISO-8601 or relative (`5m`, `1h`, `2d`). Transcript tail renders only for terminal jobs. @@ -160,7 +161,7 @@ strict behavior when unset. - `src/mcp/server.ts` — MCP stdio server (generated from operations). v0.22.7: tool-call handler delegates to `dispatchToolCall` from `src/mcp/dispatch.ts` so stdio + HTTP transports share one validation, context-build, and error-format path. **v0.34.1.0 (#870):** stdin `'end'` / `'close'` shutdown hooks are skipped when `process.env.MCP_STDIO === '1'`. Gateway-piped stdio MCP wrappers (OpenClaw's `bundle-mcp`, similar) pipe the JSON-RPC handshake then close their stdin half; pre-fix this killed the server before the first tool call landed. Signal handlers (SIGTERM / SIGINT / SIGHUP) and the parent-process watchdog still cover legitimate disconnects. `src/commands/serve.ts` exposes `ServeOptions.mcpStdio?: boolean` as a test seam so the runtime guard is exercisable without process.env mutation. Pinned by `test/serve-stdio-lifecycle.test.ts`. - `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `validateParams(op, params)`. Single source of truth for `(ctx, params)` handler arg order and the 5-field `OperationContext` shape (engine + config + logger + dryRun + remote). Defaults to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport. **v0.26.9 (F8):** adds `summarizeMcpParams(opName, params)` — privacy-preserving redactor for `mcp_request_log` and the admin SSE feed. Returns `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`. Intersects submitted top-level keys against the operation's declared `params` allow-list (declared keys preserved as a sorted array for debug visibility; unknown keys counted but never named, closing the attacker-controlled-key-name leak). Byte counts bucketed up to nearest 1KB so an attacker can't binary-search secret-content sizes via repeated probes. Operators on a personal laptop who want raw payload visibility opt back in with `gbrain serve --http --log-full-params` (loud stderr warning at startup). Canonical helper — new logging code paths route through it rather than `JSON.stringify(params)`. - `src/mcp/rate-limit.ts` (v0.22.7) — Bounded-LRU token-bucket limiter. `buildDefaultLimiters()` returns the two-bucket pipeline: pre-auth IP (30/60s, fires BEFORE the DB lookup so brute-force load against `access_tokens` is actually capped) + post-auth token-id (60/60s). Tracks `lastTouchedMs` separately from `lastRefillMs` so an exhausted key can't be reset by hammering past the TTL. LRU cap bounds memory under attacker-controlled key growth. -- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. +- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. **v0.35.3.0:** the inline ParamDef→schema mapper at `:837-849` (HTTP MCP `tools/list` handler) is replaced with `paramDefToSchema(v)` from `src/mcp/tool-defs.ts`. Pre-fix this site silently dropped `items` on every array param so strict-mode OAuth clients (Gemini Pro structured outputs, OpenAI strict tool defs) rejected the whole tool list. Single mapper now serves stdio MCP, HTTP MCP `tools/list`, and the subagent registry. - `src/core/sql-query.ts` (v0.31.3) — Engine-aware tagged-template SQL adapter for OAuth/admin/auth infrastructure. `sqlQueryForEngine(engine)` returns a `SqlQuery` (`(strings, ...values) => Promise`) that walks the template, builds `$N` positional SQL, asserts every value is a `SqlValue` (string | number | bigint | boolean | Date | null), and routes through `engine.executeRaw(sql, params)` so Postgres goes via postgres.js's `unsafe(sql, params)` path and PGLite via its embedded `db.query(sql, params)`. Deliberately narrower than postgres.js's `sql` tag: no nested fragments, no `sql.json()`, no `sql.unsafe()`, no `sql.begin()`, no array binding. The narrow surface is the feature — codex finding #7 from the v0.31 plan review argued the adapter should stay scalar-only or it drifts into a partial postgres.js clone. JSONB writes go through the separate `executeRawJsonb(engine, sql, scalarParams, jsonbParams)` helper that composes positional `$N::jsonb` casts and passes JS objects through; the v0.12.0 double-encode bug class doesn't apply because positional binding through `unsafe()` reaches the wire protocol with the correct type oid (verified by `test/sql-query.test.ts` on PGLite and `test/e2e/auth-permissions.test.ts:67` on Postgres). `scripts/check-jsonb-pattern.sh` doesn't fire because `executeRawJsonb(...)` is a method call, not the banned literal-template-tag interpolation pattern. Consumed by `src/commands/auth.ts`, `src/commands/serve-http.ts`, `src/core/oauth-provider.ts`, `src/commands/files.ts`, and `src/mcp/http-transport.ts` so all five sites work against PGLite and Postgres uniformly. Closes the bug where `gbrain auth` + `gbrain serve --http` were silently Postgres-only because they routed every SQL through the postgres.js singleton (community PR #681). - `src/commands/serve.ts` (v0.31.3) — `gbrain serve` stdio MCP entrypoint with idempotent shutdown across every parent-disconnect signal. Stdio EOF, SIGTERM, SIGINT, SIGHUP, and parent-process death (every reparent case — PID 1, launchd subreaper, systemd, tmux, or a parent shell with `PR_SET_CHILD_SUBREAPER`) all funnel into one `cleanup(reason)` path that releases the engine and the PGLite write-lock dir within 5 seconds. Pre-v0.31.3 the stdio MCP server held the lock indefinitely after Claude Desktop / Cursor / launchd-managed gateways disconnected, forcing a 5-minute stale-lock wait on the next start. Watchdog reparent check is `getParentPid() !== initialParentPid` (capturing the initial ppid once at install time and firing on any change); the previous `=== 1` check missed the subreaper case under launchd / systemd. Bun's `process.ppid` cache is stale across reparenting (see [oven-sh/bun#30305](https://github.com/oven-sh/bun/issues/30305)) so `getParentPid()` runs `spawnSync('ps', ['-o', 'ppid=', '-p', PID])` per tick to read the live kernel PPID. Startup probe verifies `ps` is on PATH; if not (stripped containers, busybox without procps), the watchdog skips installing AND emits a loud `[gbrain serve] watchdog disabled: ps unavailable, parent-death detection unavailable — child will rely on stdin EOF / signals only` stderr line so operators see the degraded mode at boot. Pinned by `test/serve-stdio-lifecycle.test.ts` (22 cases). Closes #413, #446. Credit @Aragorn2046 (origin features in #591) and @seungsu-kr (rebased submitter, Bun ppid workaround). - `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `https://` or loopback per RFC 6749 §3.1.2.1). All tokens + client secrets SHA-256 hashed before storage. Auth codes single-use with 10-minute TTL via atomic `DELETE...RETURNING` (closes RFC 6749 §10.5 TOCTOU race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements so a comma-bearing redirect_uri can't smuggle a second array element. Legacy `access_tokens` fallback in `verifyAccessToken` grandfathers pre-v0.26 bearer tokens as `read+write+admin`. `sweepExpiredTokens()` runs on startup wrapped in try/catch. **v0.26.9 RFC 6749/7009 hardening pass:** F1+F2 fold `client_id` atomically into the `DELETE WHERE` clauses for both auth-code exchange and refresh rotation — pre-fix the post-hoc client compare burned the row on wrong-client paths so the legitimate client couldn't retry. F3 enforces refresh-scope-subset against the original grant on the row (RFC 6749 §6), not the client's currently-allowed scopes — fixes the case where revoking a scope from a client wouldn't shrink the agent's existing refresh tokens. F4 binds `client_id` on `revokeToken` so a client can only revoke its own tokens (RFC 7009 §2.1). F7c validates the `/token` request's `redirect_uri` against the value stored at `/authorize` (RFC 6749 §4.1.3) — empty-string treated as missing rather than wildcard match (adversarial-review fix). F5 swaps bare `catch {}` blocks in `verifyAccessToken` and `getClient` for `isUndefinedColumnError` from `src/core/utils.ts` — only SQLSTATE 42703 falls through to legacy fallback; lock timeouts and network blips throw and surface. F6 makes `sweepExpiredTokens()` actually return the count via `RETURNING 1` + array length, not a fire-and-forget zero. F12 adds `dcrDisabled` constructor option so `serve-http.ts` can disable the `/register` endpoint without monkey-patching the router. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper. **v0.34.1.0 (#909):** `registerClient` honors `token_endpoint_auth_method: "none"` (RFC 7591 §3.2.1) — public PKCE clients (Claude Code, Cursor, every other PKCE-first MCP client) store `client_secret_hash = NULL` and the response payload omits `client_secret` entirely. Confidential clients (default `client_secret_post` and explicit `client_secret_basic`) keep their one-time-reveal shape. `getClient` correctly normalizes a NULL `client_secret_hash` to JS `undefined` so the SDK's clientAuth path accepts the public client at `/token`. **v0.34.1.0 (#861 + #876):** `verifyAccessToken` JOINs `oauth_clients.source_id` (write scope, scalar) + `oauth_clients.federated_read` (read scope, TEXT[]) and surfaces both on the returned `AuthInfo`. Pre-v60 / pre-v61 brains degrade gracefully via `isUndefinedColumnError` fallback so the upgrade chain is non-blocking on legacy DBs. diff --git a/VERSION b/VERSION index f4b4f188c..1de48e265 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.35.1.1 \ No newline at end of file +0.35.3.0 \ No newline at end of file diff --git a/llms-full.txt b/llms-full.txt index 7c0641d5e..0d7198fe6 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -168,6 +168,7 @@ strict behavior when unset. - `src/core/storage.ts` — Pluggable storage interface (S3, Supabase Storage, local) - `src/core/storage-config.ts` (v0.22.11) — Storage tiering: `loadStorageConfig` reads `gbrain.yml`, normalizes deprecated keys (`git_tracked` / `supabase_only`) to canonical (`db_tracked` / `db_only`) with once-per-process deprecation warning, and runs `normalizeAndValidateStorageConfig` (auto-fixes missing trailing `/`, throws `StorageConfigError` on tier overlap). Path-segment matcher: `media/x/` does NOT match `media/xerox/foo`. Replaces gray-matter (broken on delimiter-less YAML) with a dedicated parser for the `gbrain.yml` shape. - `src/core/disk-walk.ts` (v0.22.11) — `walkBrainRepo(repoPath)` returns `Map` from one recursive `readdirSync`. Skips dot-dirs, `node_modules`, non-`.md` files. Used by `gbrain storage status` to replace per-page `existsSync + statSync` (~400K syscalls on 200K-page brains → tens). +- `src/core/git-remote.ts` (v0.35.3.0) — SSRF-hardened git invocations for remote-source `cloneRepo` and `pullRepo`. Exports two distinct flag constants because `git`'s argv grammar treats them differently: `GIT_SSRF_FLAGS` (3 `-c` config flags — `protocol.allow=user`, `protocol.file.allow=never`, `http.allowRedirects=false`) is **global config**, spread BEFORE the subcommand verb. New `GIT_SSRF_SUBCOMMAND_FLAGS = ['--no-recurse-submodules']` is **subcommand-scoped**, spread AFTER the verb. Pre-v0.35.3 a single combined `GIT_SSRF_FLAGS` array spread `--no-recurse-submodules` before the verb where real git rejects it with exit 129 ("unknown option"); the fake-git test harness exited 0 regardless of argv shape, so CI missed it for ~7 months and every remote-source clone/pull was silently broken. `cloneRepo` argv: `git clone --depth=1 [--branch X] -- `. `pullRepo` argv: `git -C pull --ff-only`. Pinned by `test/git-remote.test.ts` position-anchored regression guard (`argv.indexOf('--no-recurse-submodules') > argv.indexOf(verb)`). - `src/commands/storage.ts` (v0.22.11) — `gbrain storage status [--repo P] [--json]`. Split into pure data (`getStorageStatus`) + JSON formatter + human formatter (ASCII-only per D10) matching the `orphans.ts` pattern. `PageCountsByTier` and `DiskUsageByTier` are distinct nominal types so swaps fail at compile time. - `gbrain.yml` (brain repo root, v0.22.11) — Optional storage tiering config. Top-level `storage:` section with `db_tracked:` and `db_only:` array-valued keys. `gbrain sync` auto-manages `.gitignore` for `db_only` paths on successful sync (skips on dry-run, blocked-by-failures, submodule context, or `GBRAIN_NO_GITIGNORE=1`). `gbrain export --restore-only [--repo P] [--type T] [--slug-prefix S]` repopulates missing `db_only` files from the database. - `src/core/supabase-admin.ts` — Supabase admin API (project discovery, pgvector check) @@ -257,8 +258,8 @@ strict behavior when unset. - `src/core/minions/wait-for-completion.ts` (v0.15) — poll-until-terminal helper for CLI callers. `TimeoutError` does NOT cancel the job; `AbortSignal` exits without throwing. Default `pollMs`: 1000 on Postgres, 250 on PGLite inline. - `src/core/minions/transcript.ts` (v0.15) — renders `subagent_messages` + `subagent_tool_executions` to markdown. Tool rows splice under their owning assistant `tool_use` by `tool_use_id`. UTF-8-safe truncation; unknown block types fall through to fenced JSON. - `src/core/minions/plugin-loader.ts` (v0.15) — `GBRAIN_PLUGIN_PATH` discovery. Absolute paths only, left-wins collision, `gbrain.plugin.json` with `plugin_version: "gbrain-plugin-v1"`, plugins ship DEFS only (no new tools), `allowed_tools:` validated at load time against the derived registry. -- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. -- `src/mcp/tool-defs.ts` (v0.15) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. +- `src/core/minions/tools/brain-allowlist.ts` (v0.15, extended v0.23, v0.29, v0.35.3.0) — derives subagent tool registry from `src/core/operations.ts`. 13-name allow-list as of v0.29 (was 11). By default `put_page` schema is namespace-wrapped per subagent (`^wiki/agents//.+`). **v0.23 trusted-workspace path:** when `BuildBrainToolsOpts.allowedSlugPrefixes` is set, the put_page schema instead describes the prefix list to the model and the OperationContext is threaded with `allowedSlugPrefixes`. Trust comes from `PROTECTED_JOB_NAMES` gating subagent submission — MCP cannot reach this field. Only cycle.ts (synthesize/patterns) and direct CLI submitters set it. **v0.29:** `get_recent_salience` + `find_anomalies` added to the allow-list. `get_recent_transcripts` deliberately NOT added — all subagent calls run with `ctx.remote === true`, and the v0.29 trust gate rejects remote callers, so adding it would always reject (footgun). The cycle synthesize phase already calls `discoverTranscripts` directly. **v0.35.3.0:** `paramsToInputSchema()` now consumes `paramDefToSchema` from `src/mcp/tool-defs.ts` instead of its own inline destructure. Required-aggregation at the tool-def level stays here (out of scope for the shared helper, which is per-param). Closes the third drift site in the ParamDef→JSON Schema bug class. +- `src/mcp/tool-defs.ts` (v0.15, v0.35.3.0) — extracted `buildToolDefs(ops)` helper. MCP server + subagent tool registry both call it; byte-for-byte equivalence pinned by `test/mcp-tool-defs.test.ts`. **v0.35.3.0:** exports the new recursive `paramDefToSchema(p: ParamDef)` helper — single source of truth for ParamDef→JSON Schema mapping. Three consumers now share one mapper: `buildToolDefs` (stdio MCP), `src/commands/serve-http.ts:837` (HTTP MCP `tools/list`), and `src/core/minions/tools/brain-allowlist.ts:84` (subagent tool registry). Pre-v0.35.3, three inline destructures had drifted across the surface — the live HTTP MCP path dropped `items` on every array param after a v0.32 review caught only the stdio side. Recursive on `items` so nested array-of-arrays preserves inner shape on the wire. Key ordering (type, description, enum, default, items) is intentional — matches the pre-v0.35.3 inline mappers so JSON.stringify output stays byte-stable. `test/mcp-tool-defs.test.ts` adds a `findArrayWithoutItems` walker that fails the suite with a property path on any future `type: 'array'` lacking `items.type`. - `src/core/minions/attachments.ts` — Attachment validation (path traversal, null byte, oversize, base64, duplicate detection) - `src/commands/agent.ts` (v0.16) — `gbrain agent run [flags]` CLI. Submits `subagent` (or N children + 1 aggregator) under `{allowProtectedSubmit: true}`. Single-entry `--fanout-manifest` short-circuits. Children get `on_child_fail: 'continue'` + `max_stalled: 3`. `--follow` is the default on TTY; streams logs + polls `waitForCompletion` in parallel. Ctrl-C detaches, does not cancel. - `src/commands/agent-logs.ts` (v0.16) — `gbrain agent logs [--follow] [--since]`. Merges JSONL heartbeat audit + `subagent_messages` into a chronological timeline. `parseSince` accepts ISO-8601 or relative (`5m`, `1h`, `2d`). Transcript tail renders only for terminal jobs. @@ -268,7 +269,7 @@ strict behavior when unset. - `src/mcp/server.ts` — MCP stdio server (generated from operations). v0.22.7: tool-call handler delegates to `dispatchToolCall` from `src/mcp/dispatch.ts` so stdio + HTTP transports share one validation, context-build, and error-format path. **v0.34.1.0 (#870):** stdin `'end'` / `'close'` shutdown hooks are skipped when `process.env.MCP_STDIO === '1'`. Gateway-piped stdio MCP wrappers (OpenClaw's `bundle-mcp`, similar) pipe the JSON-RPC handshake then close their stdin half; pre-fix this killed the server before the first tool call landed. Signal handlers (SIGTERM / SIGINT / SIGHUP) and the parent-process watchdog still cover legitimate disconnects. `src/commands/serve.ts` exposes `ServeOptions.mcpStdio?: boolean` as a test seam so the runtime guard is exercisable without process.env mutation. Pinned by `test/serve-stdio-lifecycle.test.ts`. - `src/mcp/dispatch.ts` (v0.22.7) — Shared tool-call dispatch consumed by both stdio (`server.ts`) and HTTP transports. Exports `dispatchToolCall(engine, name, params, opts)`, `buildOperationContext(engine, params, opts)`, and `validateParams(op, params)`. Single source of truth for `(ctx, params)` handler arg order and the 5-field `OperationContext` shape (engine + config + logger + dryRun + remote). Defaults to `remote: true` (untrusted); local CLI callers pass `remote: false`. Closed F1/F2/F3 drift bugs in the original v0.22.5 HTTP transport. **v0.26.9 (F8):** adds `summarizeMcpParams(opName, params)` — privacy-preserving redactor for `mcp_request_log` and the admin SSE feed. Returns `{redacted, kind, declared_keys, unknown_key_count, approx_bytes}`. Intersects submitted top-level keys against the operation's declared `params` allow-list (declared keys preserved as a sorted array for debug visibility; unknown keys counted but never named, closing the attacker-controlled-key-name leak). Byte counts bucketed up to nearest 1KB so an attacker can't binary-search secret-content sizes via repeated probes. Operators on a personal laptop who want raw payload visibility opt back in with `gbrain serve --http --log-full-params` (loud stderr warning at startup). Canonical helper — new logging code paths route through it rather than `JSON.stringify(params)`. - `src/mcp/rate-limit.ts` (v0.22.7) — Bounded-LRU token-bucket limiter. `buildDefaultLimiters()` returns the two-bucket pipeline: pre-auth IP (30/60s, fires BEFORE the DB lookup so brute-force load against `access_tokens` is actually capped) + post-auth token-id (60/60s). Tracks `lastTouchedMs` separately from `lastRefillMs` so an exhausted key can't be reset by hammering past the TTL. LRU cap bounds memory under attacker-controlled key growth. -- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. +- `src/commands/serve-http.ts` (v0.26.0) — Express 5 HTTP MCP server with OAuth 2.1, admin dashboard, and SSE live activity feed. Started via `gbrain serve --http [--port N] [--token-ttl N] [--enable-dcr] [--public-url URL] [--log-full-params]`. Supersedes the v0.22.7 `src/mcp/http-transport.ts` simple bearer-auth path. Combines MCP SDK's `mcpAuthRouter` (authorize / token / register / revoke endpoints), a custom `client_credentials` handler (SDK's token endpoint throws `UnsupportedGrantTypeError` for CC; the custom handler runs BEFORE the router and falls through for `auth_code` / `refresh_token`), `requireBearerAuth` middleware for `/mcp` with scope enforcement before op dispatch, `localOnly` rejection, and `express-rate-limit` at 50 req / 15 min on `/token`. Serves the built admin SPA from `admin/dist/` with SPA fallback. `/admin/events` SSE endpoint broadcasts every MCP request to connected admin browsers. `cookie-parser` middleware wired (Express 5 has no built-in). Startup logging prints port, engine, configured issuer URL (honors `--public-url`), registered-client count, DCR status, and admin bootstrap token. **v0.26.9 hardening pass:** F7 sets `remote: true` explicitly on the `/mcp` request handler's OperationContext literal (closes the HTTP shell-job RCE — without this, `submit_job`'s protected-name guard at `operations.ts:1391` saw a falsy undefined and skipped, letting a `read+write`-scoped OAuth token submit `shell` jobs). F8 wires `summarizeMcpParams` from `src/mcp/dispatch.ts` into both `mcp_request_log` writes and the admin SSE feed by default (raw payloads opt-in via `--log-full-params` with stderr warning). F9 sets cookie `Secure` flag when behind HTTPS or a public-URL proxy. F10 caps the magic-link nonce store with an LRU bound. F12 routes DCR disable through the `GBrainOAuthProvider` constructor's `dcrDisabled` option instead of the prior monkey-patch on the express router. F14 wraps `transport.handleRequest` in try/catch so SDK throws return a JSON-RPC 500 envelope instead of express's default HTML error page. F15 unifies OperationError + unexpected exceptions through `buildError` / `serializeError` so `/mcp` always returns the same envelope shape. **v0.28.1:** `/health` endpoint extracted into pure `probeHealth(engine)` async function with `HEALTH_TIMEOUT_MS = 3000` exported constant — drops the timeout from 5s to 3s so Fly.io's 5s health-check deadline gets 2s of headroom for TCP, response framing, and clock skew. Races `engine.getStats()` against the timeout via `Promise.race`; saturated pool returns 503 with `Health check timed out (database pool may be saturated)` instead of hanging. `clearTimeout` in finally block prevents pending-timer pile-up under high probe rates (race-leak fix from adversarial review). **v0.28.10:** `/health` is now liveness-only via the new `probeLiveness(sql, engineName, version, timeoutMs)` helper that races `sql\`SELECT 1\`` against `HEALTH_TIMEOUT_MS` and returns the same `ProbeHealthResult` tagged-union as `probeHealth` (single timer-cleanup site, single 503 envelope). Body shape: `{status, version, engine}` only — engine stats are no longer spread on the public route. Full stats moved to a new admin endpoint `/admin/api/full-stats` (sibling to `/admin/api/stats` and `/admin/api/health-indicators`) gated by the existing `requireAdmin` middleware; that route calls `probeHealth(engine, ...)` and returns the original spread-stats body. `?full=true` query param removed entirely. Closes the original DoS surface where `getStats()`'s 6× count(*) on 96K-page brains through PgBouncer exceeded `HEALTH_TIMEOUT_MS` and triggered orchestrator restart cascades (Fly.io / k8s seeing 503 → restart loop → advisory-lock pile-up on the migration lock). Outside-voice review (Codex) caught that `/admin/api/health-indicators` is NOT a full-stats endpoint (returns only `{expiring_soon, error_rate}`), and that an alternative loopback-IP gate would have depended on `app.set('trust proxy', 'loopback')` semantics holding under proxy/XFF misconfiguration; the shipped admin-cookie design avoids both. **v0.31.3 (#681):** every OAuth/admin/audit SQL call routes through `sqlQueryForEngine(engine)` from `src/core/sql-query.ts` so `gbrain serve --http` works against PGLite brains. The four `mcp_request_log.params` INSERT sites (success path, auth_failed path, scope_denied path, server-error path) all go through `executeRawJsonb(engine, ...)` so the JSONB column stores real objects, not JSON-encoded strings — closes the bug where `params->>'op'` returned the encoded string `"search"` (with quotes) instead of `search`. Migration v46 normalizes any pre-v0.31.3 string-shaped backlog rows on first start. **v0.34.1.0 (#864):** new `--bind HOST` CLI flag with default `127.0.0.1`. Personal-laptop installs no longer publish the brain to the LAN by accident. Self-hosted operators pass `--bind 0.0.0.0` (or a specific interface IP) once to accept remote connections. A stderr WARN fires when `--public-url` is set without `--bind` so the operator sees the binding before the first request (common cause of "ngrok forwards to me but the agent can't reach the upstream" misconfigurations). The startup banner prints a `Bind:` line. **v0.34.1.0 (#861):** drops the `(authInfo as AuthInfo & {sourceId?: string}).sourceId ?? env ?? 'default'` cast chain — `AuthInfo.sourceId` and `AuthInfo.allowedSources` are now the typed source of truth, populated by `oauth-provider.ts:verifyAccessToken` from the `oauth_clients` row. **v0.35.3.0:** the inline ParamDef→schema mapper at `:837-849` (HTTP MCP `tools/list` handler) is replaced with `paramDefToSchema(v)` from `src/mcp/tool-defs.ts`. Pre-fix this site silently dropped `items` on every array param so strict-mode OAuth clients (Gemini Pro structured outputs, OpenAI strict tool defs) rejected the whole tool list. Single mapper now serves stdio MCP, HTTP MCP `tools/list`, and the subagent registry. - `src/core/sql-query.ts` (v0.31.3) — Engine-aware tagged-template SQL adapter for OAuth/admin/auth infrastructure. `sqlQueryForEngine(engine)` returns a `SqlQuery` (`(strings, ...values) => Promise`) that walks the template, builds `$N` positional SQL, asserts every value is a `SqlValue` (string | number | bigint | boolean | Date | null), and routes through `engine.executeRaw(sql, params)` so Postgres goes via postgres.js's `unsafe(sql, params)` path and PGLite via its embedded `db.query(sql, params)`. Deliberately narrower than postgres.js's `sql` tag: no nested fragments, no `sql.json()`, no `sql.unsafe()`, no `sql.begin()`, no array binding. The narrow surface is the feature — codex finding #7 from the v0.31 plan review argued the adapter should stay scalar-only or it drifts into a partial postgres.js clone. JSONB writes go through the separate `executeRawJsonb(engine, sql, scalarParams, jsonbParams)` helper that composes positional `$N::jsonb` casts and passes JS objects through; the v0.12.0 double-encode bug class doesn't apply because positional binding through `unsafe()` reaches the wire protocol with the correct type oid (verified by `test/sql-query.test.ts` on PGLite and `test/e2e/auth-permissions.test.ts:67` on Postgres). `scripts/check-jsonb-pattern.sh` doesn't fire because `executeRawJsonb(...)` is a method call, not the banned literal-template-tag interpolation pattern. Consumed by `src/commands/auth.ts`, `src/commands/serve-http.ts`, `src/core/oauth-provider.ts`, `src/commands/files.ts`, and `src/mcp/http-transport.ts` so all five sites work against PGLite and Postgres uniformly. Closes the bug where `gbrain auth` + `gbrain serve --http` were silently Postgres-only because they routed every SQL through the postgres.js singleton (community PR #681). - `src/commands/serve.ts` (v0.31.3) — `gbrain serve` stdio MCP entrypoint with idempotent shutdown across every parent-disconnect signal. Stdio EOF, SIGTERM, SIGINT, SIGHUP, and parent-process death (every reparent case — PID 1, launchd subreaper, systemd, tmux, or a parent shell with `PR_SET_CHILD_SUBREAPER`) all funnel into one `cleanup(reason)` path that releases the engine and the PGLite write-lock dir within 5 seconds. Pre-v0.31.3 the stdio MCP server held the lock indefinitely after Claude Desktop / Cursor / launchd-managed gateways disconnected, forcing a 5-minute stale-lock wait on the next start. Watchdog reparent check is `getParentPid() !== initialParentPid` (capturing the initial ppid once at install time and firing on any change); the previous `=== 1` check missed the subreaper case under launchd / systemd. Bun's `process.ppid` cache is stale across reparenting (see [oven-sh/bun#30305](https://github.com/oven-sh/bun/issues/30305)) so `getParentPid()` runs `spawnSync('ps', ['-o', 'ppid=', '-p', PID])` per tick to read the live kernel PPID. Startup probe verifies `ps` is on PATH; if not (stripped containers, busybox without procps), the watchdog skips installing AND emits a loud `[gbrain serve] watchdog disabled: ps unavailable, parent-death detection unavailable — child will rely on stdin EOF / signals only` stderr line so operators see the degraded mode at boot. Pinned by `test/serve-stdio-lifecycle.test.ts` (22 cases). Closes #413, #446. Credit @Aragorn2046 (origin features in #591) and @seungsu-kr (rebased submitter, Bun ppid workaround). - `src/core/oauth-provider.ts` (v0.26.0) — `GBrainOAuthProvider` implementing the MCP SDK's `OAuthServerProvider` + `OAuthRegisteredClientsStore` interfaces. Backed by raw SQL (works on both PGLite and Postgres — OAuth is infrastructure, not a BrainEngine concern). Full OAuth 2.1 spec: `authorize` + `exchangeAuthorizationCode` with PKCE (for ChatGPT), `client_credentials` (for Perplexity / Claude), `refresh_token` with rotation, `revokeToken`, `registerClient` (DCR path validates redirect_uri must be `https://` or loopback per RFC 6749 §3.1.2.1). All tokens + client secrets SHA-256 hashed before storage. Auth codes single-use with 10-minute TTL via atomic `DELETE...RETURNING` (closes RFC 6749 §10.5 TOCTOU race). Refresh rotation also `DELETE...RETURNING` (closes §10.4 stolen-token detection bypass). `pgArray()` escapes commas/quotes/braces in elements so a comma-bearing redirect_uri can't smuggle a second array element. Legacy `access_tokens` fallback in `verifyAccessToken` grandfathers pre-v0.26 bearer tokens as `read+write+admin`. `sweepExpiredTokens()` runs on startup wrapped in try/catch. **v0.26.9 RFC 6749/7009 hardening pass:** F1+F2 fold `client_id` atomically into the `DELETE WHERE` clauses for both auth-code exchange and refresh rotation — pre-fix the post-hoc client compare burned the row on wrong-client paths so the legitimate client couldn't retry. F3 enforces refresh-scope-subset against the original grant on the row (RFC 6749 §6), not the client's currently-allowed scopes — fixes the case where revoking a scope from a client wouldn't shrink the agent's existing refresh tokens. F4 binds `client_id` on `revokeToken` so a client can only revoke its own tokens (RFC 7009 §2.1). F7c validates the `/token` request's `redirect_uri` against the value stored at `/authorize` (RFC 6749 §4.1.3) — empty-string treated as missing rather than wildcard match (adversarial-review fix). F5 swaps bare `catch {}` blocks in `verifyAccessToken` and `getClient` for `isUndefinedColumnError` from `src/core/utils.ts` — only SQLSTATE 42703 falls through to legacy fallback; lock timeouts and network blips throw and surface. F6 makes `sweepExpiredTokens()` actually return the count via `RETURNING 1` + array length, not a fire-and-forget zero. F12 adds `dcrDisabled` constructor option so `serve-http.ts` can disable the `/register` endpoint without monkey-patching the router. **v0.26.2:** module-private `coerceTimestamp()` boundary helper at the top of the file normalizes postgres-driver-as-string BIGINT columns to JS numbers at every read site (5 call sites: `getClient` L112+L113 for DCR `/register` RFC 7591 §3.2.1 numeric timestamps, `exchangeRefreshToken` L274 + `verifyAccessToken` L296+L303 for the SDK's `typeof === 'number'` bearerAuth check). Throws on non-finite input (NaN/Infinity) so corrupt rows fail loud at the boundary instead of riding through as `expiresAt: NaN`; returns undefined for SQL NULL so callers decide NULL semantics explicitly (refresh + access token paths treat NULL as expired). Helper intentionally NOT promoted to `src/core/utils.ts` — codex review flagged repo-wide BIGINT precision-loss risk for a generic helper. **v0.34.1.0 (#909):** `registerClient` honors `token_endpoint_auth_method: "none"` (RFC 7591 §3.2.1) — public PKCE clients (Claude Code, Cursor, every other PKCE-first MCP client) store `client_secret_hash = NULL` and the response payload omits `client_secret` entirely. Confidential clients (default `client_secret_post` and explicit `client_secret_basic`) keep their one-time-reveal shape. `getClient` correctly normalizes a NULL `client_secret_hash` to JS `undefined` so the SDK's clientAuth path accepts the public client at `/token`. **v0.34.1.0 (#861 + #876):** `verifyAccessToken` JOINs `oauth_clients.source_id` (write scope, scalar) + `oauth_clients.federated_read` (read scope, TEXT[]) and surfaces both on the returned `AuthInfo`. Pre-v60 / pre-v61 brains degrade gracefully via `isUndefinedColumnError` fallback so the upgrade chain is non-blocking on legacy DBs. diff --git a/package.json b/package.json index 49d0b9839..470821e09 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.35.1.1", + "version": "0.35.3.0", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index 752324b3c..db00d3540 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -28,6 +28,7 @@ import { GBrainOAuthProvider } from '../core/oauth-provider.ts'; import type { SqlQuery } from '../core/oauth-provider.ts'; import { hasScope, ALLOWED_SCOPES_LIST } from '../core/scope.ts'; import { summarizeMcpParams, dispatchToolCall } from '../mcp/dispatch.ts'; +import { paramDefToSchema } from '../mcp/tool-defs.ts'; import { getBrainHotMemoryMeta } from '../core/facts/meta-hook.ts'; import { loadConfig } from '../core/config.ts'; import { buildError, serializeError } from '../core/errors.ts'; @@ -840,12 +841,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption inputSchema: { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type, - description: v.description, - ...(v.enum ? { enum: v.enum } : {}), - ...(v.default !== undefined ? { default: v.default } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, paramDefToSchema(v)]), ), required: Object.entries(op.params).filter(([, v]) => v.required).map(([k]) => k), }, diff --git a/src/core/git-remote.ts b/src/core/git-remote.ts index 348488fd0..dbbc339d1 100644 --- a/src/core/git-remote.ts +++ b/src/core/git-remote.ts @@ -20,16 +20,38 @@ import { join } from 'path'; import { isInternalUrl } from './url-safety.ts'; /** - * SSRF-defensive flag set. Used by both cloneRepo and pullRepo. + * Git CLI accepts two flag positions: + * git [global -c flags] [subcommand flags] [args] + * + * Global flags (the `-c key=value` config overrides) MUST come before the + * subcommand. Subcommand-specific flags (like `--no-recurse-submodules`) + * MUST come after the subcommand. Mixing the two positions makes git fail + * with `unknown option` (exit 129). Pre-v0.34 the single GIT_SSRF_FLAGS + * constant spliced both positions before the verb; real git rejected the + * subcommand flag but the test harness used a fake-git script that didn't + * validate, so every remote-source clone/pull broke silently in production. + * + * Split into two constants so the call-site spread is unambiguous and the + * type/name signal the position rule. + */ + +/** + * Global git config flags. Spread BEFORE the subcommand verb. * - http.followRedirects=false: closes DNS rebinding via redirect chains * - protocol.file.allow=never: no local-file URLs (defense in depth) * - protocol.ext.allow=never: no external helpers (`git-remote-foo`) - * - --no-recurse-submodules: .gitmodules cannot become a second fetch surface */ export const GIT_SSRF_FLAGS = [ '-c', 'http.followRedirects=false', '-c', 'protocol.file.allow=never', '-c', 'protocol.ext.allow=never', +] as const; + +/** + * Subcommand-level flags. Spread AFTER the subcommand verb (clone/pull). + * - --no-recurse-submodules: .gitmodules cannot become a second fetch surface + */ +export const GIT_SSRF_SUBCOMMAND_FLAGS = [ '--no-recurse-submodules', ] as const; @@ -153,7 +175,7 @@ export function cloneRepo(url: string, destDir: string, opts: CloneOpts = {}): v } } - const args: string[] = [...GIT_SSRF_FLAGS, 'clone']; + const args: string[] = [...GIT_SSRF_FLAGS, 'clone', ...GIT_SSRF_SUBCOMMAND_FLAGS]; if (opts.depth !== 0) { args.push(`--depth=${opts.depth ?? 1}`); } @@ -179,7 +201,7 @@ export function cloneRepo(url: string, destDir: string, opts: CloneOpts = {}): v /** Pull a repo with --ff-only and the same SSRF-defensive flags as cloneRepo. */ export function pullRepo(repoPath: string, opts: { timeoutMs?: number } = {}): void { - const args: string[] = ['-C', repoPath, ...GIT_SSRF_FLAGS, 'pull', '--ff-only']; + const args: string[] = ['-C', repoPath, ...GIT_SSRF_FLAGS, 'pull', ...GIT_SSRF_SUBCOMMAND_FLAGS, '--ff-only']; try { execFileSync('git', args, { stdio: ['ignore', 'pipe', 'pipe'], diff --git a/src/core/minions/tools/brain-allowlist.ts b/src/core/minions/tools/brain-allowlist.ts index 8fbe32d23..cedd4f46e 100644 --- a/src/core/minions/tools/brain-allowlist.ts +++ b/src/core/minions/tools/brain-allowlist.ts @@ -26,6 +26,7 @@ import type { BrainEngine } from '../../engine.ts'; import type { GBrainConfig } from '../../config.ts'; import { operations } from '../../operations.ts'; import type { Operation, OperationContext } from '../../operations.ts'; +import { paramDefToSchema } from '../../../mcp/tool-defs.ts'; import type { ToolCtx, ToolDef } from '../types.ts'; /** @@ -85,12 +86,7 @@ function paramsToInputSchema(op: Operation): Record { return { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type === 'array' ? 'array' : v.type, - ...(v.description ? { description: v.description } : {}), - ...(v.enum ? { enum: v.enum } : {}), - ...(v.items ? { items: { type: v.items.type } } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, paramDefToSchema(v)]), ), required: Object.entries(op.params).filter(([, v]) => v.required).map(([k]) => k), }; diff --git a/src/core/operations.ts b/src/core/operations.ts index 2d763ef9b..49ed17700 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -2696,7 +2696,7 @@ const extract_facts: Operation = { params: { turn_text: { type: 'string', required: true, description: 'The user message or page body to extract facts from. Sanitized via INJECTION_PATTERNS before the LLM call.' }, session_id: { type: 'string', description: 'Opaque session id (e.g. topic-id from MCP _meta.session_id, or CLI --session). Stored on each fact for the recall --session filter. Not an auth surface.' }, - entity_hints: { type: 'array', description: 'Existing canonical entity slugs the agent has already resolved. Helps the extractor pick the right slug.' }, + entity_hints: { type: 'array', items: { type: 'string' }, description: 'Existing canonical entity slugs the agent has already resolved. Helps the extractor pick the right slug.' }, is_dream_generated: { type: 'boolean', description: 'When true, extraction is skipped (anti-loop). Caller flips this on for pages with dream_generated:true frontmatter.' }, visibility: { type: 'string', description: 'Default visibility for extracted facts. private (default) | world.' }, }, diff --git a/src/core/resolvers/builtin/x-api/handle-to-tweet.ts b/src/core/resolvers/builtin/x-api/handle-to-tweet.ts index 9c0cea8ec..ffaefbbfa 100644 --- a/src/core/resolvers/builtin/x-api/handle-to-tweet.ts +++ b/src/core/resolvers/builtin/x-api/handle-to-tweet.ts @@ -99,7 +99,21 @@ export const xHandleToTweetResolver: Resolver { + return { + type: p.type === 'array' ? 'array' : p.type, + ...(p.description ? { description: p.description } : {}), + ...(p.enum ? { enum: p.enum } : {}), + ...(p.default !== undefined ? { default: p.default } : {}), + ...(p.items ? { items: paramDefToSchema(p.items) } : {}), + }; +} + export function buildToolDefs(ops: Operation[]): McpToolDef[] { return ops.map(op => ({ name: op.name, @@ -17,12 +44,7 @@ export function buildToolDefs(ops: Operation[]): McpToolDef[] { inputSchema: { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type === 'array' ? 'array' : v.type, - ...(v.description ? { description: v.description } : {}), - ...(v.enum ? { enum: v.enum } : {}), - ...(v.items ? { items: { type: v.items.type } } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, paramDefToSchema(v)]), ), required: Object.entries(op.params) .filter(([, v]) => v.required) diff --git a/test/e2e/zeroentropy-live.test.ts b/test/e2e/zeroentropy-live.test.ts index 852801143..772c7b1c1 100644 --- a/test/e2e/zeroentropy-live.test.ts +++ b/test/e2e/zeroentropy-live.test.ts @@ -99,6 +99,14 @@ describe('ZE live — embed round-trip', () => { }); }); +// ZE rerank API has multi-second cold-start latency on the first request +// of a CI run (observed ~5-6s on Tier 2 runners; subsequent calls < 500ms). +// The production DEFAULT_RERANK_TIMEOUT_MS in gateway.ts stays at 5s for the +// search hot path; these E2E tests pass an explicit input.timeoutMs and a +// longer bun:test per-test timeout so cold-start doesn't flake CI. +const ZE_RERANK_TIMEOUT_MS = 25_000; +const ZE_TEST_TIMEOUT_MS = 30_000; + describe('ZE live — rerank round-trip', () => { test('rerank({query, documents}) returns sorted RerankResult[]', async () => { if (skipAll) { @@ -112,6 +120,7 @@ describe('ZE live — rerank round-trip', () => { 'My cat likes to eat tuna fish.', 'Chlorophyll absorbs red and blue light during photosynthesis.', ], + timeoutMs: ZE_RERANK_TIMEOUT_MS, }); expect(out.length).toBe(3); for (const r of out) { @@ -128,7 +137,7 @@ describe('ZE live — rerank round-trip', () => { // the cat doc must NOT be at the top. const topIndex = out[0]!.index; expect(topIndex).not.toBe(1); // index 1 is the cat doc - }); + }, ZE_TEST_TIMEOUT_MS); test('rerank with top_n=2 returns at most 2 results', async () => { if (skipAll) { @@ -139,9 +148,10 @@ describe('ZE live — rerank round-trip', () => { query: 'photosynthesis', documents: ['photosynthesis a', 'cats b', 'photosynthesis c'], topN: 2, + timeoutMs: ZE_RERANK_TIMEOUT_MS, }); expect(out.length).toBeLessThanOrEqual(2); - }); + }, ZE_TEST_TIMEOUT_MS); }); describe('ZE live — flexible dims', () => { diff --git a/test/git-remote.test.ts b/test/git-remote.test.ts index a8c744d6d..6d4a60d05 100644 --- a/test/git-remote.test.ts +++ b/test/git-remote.test.ts @@ -4,6 +4,7 @@ import { join } from 'path'; import { tmpdir } from 'os'; import { GIT_SSRF_FLAGS, + GIT_SSRF_SUBCOMMAND_FLAGS, parseRemoteUrl, RemoteUrlError, cloneRepo, @@ -81,11 +82,22 @@ const fakePath = (): string => `${FAKE_GIT_DIR}:${process.env.PATH ?? ''}`; // --------------------------------------------------------------------------- describe('GIT_SSRF_FLAGS', () => { - test('exact shape — codex SSRF lockdown', () => { + test('exact shape — global -c config flags only (spread BEFORE the verb)', () => { expect([...GIT_SSRF_FLAGS]).toEqual([ '-c', 'http.followRedirects=false', '-c', 'protocol.file.allow=never', '-c', 'protocol.ext.allow=never', + ]); + }); +}); + +describe('GIT_SSRF_SUBCOMMAND_FLAGS', () => { + test('exact shape — subcommand-level flags only (spread AFTER the verb)', () => { + // v0.34 fix wave: --no-recurse-submodules is a clone/pull subcommand + // flag, not a global flag. Real git exits 129 with "unknown option" + // when it appears before the verb. The pre-v0.34 single-constant + // spread baked the bug in. + expect([...GIT_SSRF_SUBCOMMAND_FLAGS]).toEqual([ '--no-recurse-submodules', ]); }); @@ -229,12 +241,22 @@ describe('cloneRepo', () => { const calls = readArgvLog(); expect(calls.length).toBe(1); const argv = calls[0]; - // Pin the SSRF flags before the 'clone' verb (codex Q2 invariant). + // Global -c config flags must appear BEFORE the 'clone' verb. expect(argv.slice(0, GIT_SSRF_FLAGS.length)).toEqual([...GIT_SSRF_FLAGS]); expect(argv).toContain('clone'); expect(argv).toContain('--depth=1'); expect(argv).toContain('https://example.com/repo'); expect(argv[argv.length - 1]).toBe(dest); + // v0.34 fix wave: subcommand flags MUST appear after the verb. Real + // git rejects `git --no-recurse-submodules clone ...` with exit 129. + // The fake-git harness returned 0 for any argv shape, so this + // position-anchored assertion is the structural regression test. + const cloneIdx = argv.indexOf('clone'); + expect(cloneIdx).toBeGreaterThan(-1); + for (const subFlag of GIT_SSRF_SUBCOMMAND_FLAGS) { + const flagIdx = argv.indexOf(subFlag); + expect(flagIdx).toBeGreaterThan(cloneIdx); + } }); test('depth=0 means no --depth flag (full clone)', async () => { @@ -309,6 +331,13 @@ describe('pullRepo', () => { expect(argv.slice(2, 2 + GIT_SSRF_FLAGS.length)).toEqual([...GIT_SSRF_FLAGS]); expect(argv).toContain('pull'); expect(argv).toContain('--ff-only'); + // v0.34 fix wave: subcommand flag position assertion. + const pullIdx = argv.indexOf('pull'); + expect(pullIdx).toBeGreaterThan(-1); + for (const subFlag of GIT_SSRF_SUBCOMMAND_FLAGS) { + const flagIdx = argv.indexOf(subFlag); + expect(flagIdx).toBeGreaterThan(pullIdx); + } rmSync(repo, { recursive: true, force: true }); }); diff --git a/test/mcp-tool-defs.test.ts b/test/mcp-tool-defs.test.ts index 5309ce39d..7ee4dd015 100644 --- a/test/mcp-tool-defs.test.ts +++ b/test/mcp-tool-defs.test.ts @@ -10,10 +10,39 @@ import { describe, test, expect } from 'bun:test'; import { operations } from '../src/core/operations.ts'; -import { buildToolDefs } from '../src/mcp/tool-defs.ts'; +import { buildToolDefs, paramDefToSchema } from '../src/mcp/tool-defs.ts'; +import type { ParamDef } from '../src/core/operations.ts'; -// Pre-extraction inline shape — lifted verbatim from the original -// src/mcp/server.ts block so any future drift fails this test loudly. +// Reference shape — mirrors the canonical `paramDefToSchema` helper from +// src/mcp/tool-defs.ts. Drift between the helper and this reference fails +// the byte-equality test loudly. +// +// v0.34 update: paramDefToSchema is recursive on `items` so nested +// array-of-arrays preserves the inner shape on the MCP wire. The reference +// below mirrors that recursion. The previous shallow `{ items: { type: +// v.items.type } }` (legacy buildToolDefs) silently dropped nested items +// — explicit fixture assertions below catch the drift class. +// +// `default` is included to match paramDefToSchema; no current op uses +// `default:` at the ParamDef level so the round-trip is unchanged for +// every existing operation, but new ops that add a default get it on the +// wire automatically. +type ParamDefLike = { + type: 'string' | 'number' | 'boolean' | 'object' | 'array'; + description?: string; + enum?: string[]; + default?: unknown; + items?: ParamDefLike; +}; +function referenceParamDefToSchema(p: ParamDefLike): Record { + return { + type: p.type === 'array' ? 'array' : p.type, + ...(p.description ? { description: p.description } : {}), + ...(p.enum ? { enum: p.enum } : {}), + ...(p.default !== undefined ? { default: p.default } : {}), + ...(p.items ? { items: referenceParamDefToSchema(p.items) } : {}), + }; +} function legacyInlineMap(ops: typeof operations) { return ops.map(op => ({ name: op.name, @@ -21,12 +50,7 @@ function legacyInlineMap(ops: typeof operations) { inputSchema: { type: 'object' as const, properties: Object.fromEntries( - Object.entries(op.params).map(([k, v]) => [k, { - type: v.type === 'array' ? 'array' : v.type, - ...(v.description ? { description: v.description } : {}), - ...(v.enum ? { enum: v.enum } : {}), - ...(v.items ? { items: { type: v.items.type } } : {}), - }]), + Object.entries(op.params).map(([k, v]) => [k, referenceParamDefToSchema(v)]), ), required: Object.entries(op.params) .filter(([, v]) => v.required) @@ -65,3 +89,98 @@ describe('buildToolDefs', () => { } }); }); + +// --------------------------------------------------------------------------- +// Structural array-items guard (v0.34 fix wave). +// +// JSON Schema strict-mode validators (Gemini Pro strict, OpenAI structured +// outputs) reject `type: 'array'` without `items`. Pre-v0.34 this happened +// in production: `extract_facts.entity_hints` and `handle_to_tweet`'s +// `candidates` both shipped as bare arrays. +// +// This recursive guard walks every tool def's inputSchema and fails the +// suite with a property path if any array lacks `items.type`. Drift-proof +// against future ops adding bare arrays. +// --------------------------------------------------------------------------- + +interface SchemaNode { + type?: unknown; + properties?: Record; + items?: SchemaNode; + [k: string]: unknown; +} + +function findArrayWithoutItems(node: SchemaNode, path: string[]): string[] { + const violations: string[] = []; + if (node && typeof node === 'object') { + if (node.type === 'array') { + if (!node.items || typeof node.items !== 'object') { + violations.push(`${path.join('.') || ''} (array missing items)`); + } else if (!('type' in node.items)) { + violations.push(`${path.join('.') || ''}.items (items missing type)`); + } else { + violations.push(...findArrayWithoutItems(node.items, [...path, 'items'])); + } + } + if (node.properties && typeof node.properties === 'object') { + for (const [k, child] of Object.entries(node.properties)) { + violations.push(...findArrayWithoutItems(child as SchemaNode, [...path, k])); + } + } + if (node.items && typeof node.items === 'object' && node.type !== 'array') { + violations.push(...findArrayWithoutItems(node.items, [...path, 'items'])); + } + } + return violations; +} + +describe('paramDefToSchema structural guard', () => { + test('every operation inputSchema array has items.type set (no bare arrays)', () => { + const allViolations: string[] = []; + for (const def of buildToolDefs(operations)) { + const v = findArrayWithoutItems(def.inputSchema as SchemaNode, [def.name]); + allViolations.push(...v); + } + expect(allViolations).toEqual([]); + }); + + test('extract_facts.entity_hints declares items.type as string', () => { + const def = buildToolDefs(operations).find(d => d.name === 'extract_facts'); + expect(def).toBeDefined(); + const eh = (def!.inputSchema.properties as Record).entity_hints; + expect(eh.type).toBe('array'); + expect(eh.items).toBeDefined(); + expect((eh.items as SchemaNode).type).toBe('string'); + }); + + test('paramDefToSchema recursively propagates nested items.items.type', () => { + // Synthetic ParamDef: array-of-arrays-of-strings. No current op uses + // this shape, so this test pins the contract for future ops and proves + // the helper recurses (closes the v0.32 nested-drop bug class). + const nested: ParamDef = { + type: 'array', + items: { + type: 'array', + items: { type: 'string' }, + }, + }; + const schema = paramDefToSchema(nested) as SchemaNode; + expect(schema.type).toBe('array'); + expect((schema.items as SchemaNode).type).toBe('array'); + expect(((schema.items as SchemaNode).items as SchemaNode).type).toBe('string'); + }); + + test('paramDefToSchema preserves description on nested items', () => { + const p: ParamDef = { + type: 'array', + description: 'outer', + items: { + type: 'string', + description: 'inner', + }, + }; + const schema = paramDefToSchema(p) as SchemaNode; + expect(schema.description).toBe('outer'); + expect((schema.items as SchemaNode).description).toBe('inner'); + }); +}); diff --git a/test/resolvers.test.ts b/test/resolvers.test.ts index 4ca2d98f1..de9482c3c 100644 --- a/test/resolvers.test.ts +++ b/test/resolvers.test.ts @@ -620,3 +620,98 @@ describe('x_handle_to_tweet resolver', () => { expect(query).toContain('words'); }); }); + +// --------------------------------------------------------------------------- +// Structural array-items guard for resolver inputSchema + outputSchema. +// +// v0.34 fix wave: handle_to_tweet's `candidates` field shipped as bare +// `type: 'array'` without `items` (caught by community PR #910). Same +// bug class as extract_facts.entity_hints, different surface. Resolvers +// don't ride through buildToolDefs so the MCP-side structural guard +// doesn't see them. +// +// This guard walks the builtin resolvers explicitly (NOT via +// getDefaultRegistry(), which starts empty until commands/resolvers.ts +// registers — codex catch). Both inputSchema AND outputSchema are +// checked because resolvers are bidirectional. +// --------------------------------------------------------------------------- + +interface SchemaNode { + type?: unknown; + properties?: Record; + items?: SchemaNode; + [k: string]: unknown; +} + +function findArrayWithoutItems(node: SchemaNode | undefined, path: string[]): string[] { + const violations: string[] = []; + if (!node || typeof node !== 'object') return violations; + if (node.type === 'array') { + if (!node.items || typeof node.items !== 'object') { + violations.push(`${path.join('.') || ''} (array missing items)`); + } else if (!('type' in node.items)) { + violations.push(`${path.join('.') || ''}.items (items missing type)`); + } else { + violations.push(...findArrayWithoutItems(node.items, [...path, 'items'])); + } + } + if (node.properties && typeof node.properties === 'object') { + for (const [k, child] of Object.entries(node.properties)) { + violations.push(...findArrayWithoutItems(child as SchemaNode, [...path, k])); + } + } + return violations; +} + +describe('builtin resolver schemas — structural array-items guard', () => { + // Explicit-import list. getDefaultRegistry() returns an empty registry + // until commands/resolvers.ts registers, so walking it would silently + // pass with zero resolvers visited (codex finding). + const builtins = [ + { name: 'url_reachable', resolver: urlReachableResolver }, + { name: 'x_handle_to_tweet', resolver: xHandleToTweetResolver }, + ]; + + for (const { name, resolver } of builtins) { + test(`${name}: inputSchema has no bare arrays`, () => { + const violations = findArrayWithoutItems( + resolver.inputSchema as SchemaNode | undefined, + [name, 'inputSchema'], + ); + expect(violations).toEqual([]); + }); + + test(`${name}: outputSchema has no bare arrays`, () => { + const violations = findArrayWithoutItems( + resolver.outputSchema as SchemaNode | undefined, + [name, 'outputSchema'], + ); + expect(violations).toEqual([]); + }); + } + + test('x_handle_to_tweet candidates declares full item shape with required + additionalProperties:false', () => { + const out = xHandleToTweetResolver.outputSchema as SchemaNode; + const candidates = (out.properties as Record).candidates; + expect(candidates.type).toBe('array'); + expect(candidates.items).toBeDefined(); + const items = candidates.items as SchemaNode; + expect(items.type).toBe('object'); + expect(items.properties).toBeDefined(); + const props = items.properties as Record; + expect(Object.keys(props).sort()).toEqual( + ['created_at', 'score', 'text', 'tweet_id', 'url'], + ); + expect((items as { required?: unknown }).required).toEqual( + ['tweet_id', 'text', 'created_at', 'score', 'url'], + ); + expect((items as { additionalProperties?: unknown }).additionalProperties).toBe(false); + }); + + test('coverage: at least 2 builtins walked (catches future regression where registry import drops)', () => { + // Negative guard against codex's "underspecified registry walk" finding: + // if this list ever empties (e.g. an autoformatter drops the array), + // the rest of the suite would silently pass. + expect(builtins.length).toBeGreaterThanOrEqual(2); + }); +});