diff --git a/CHANGELOG.md b/CHANGELOG.md index f849225e0..543ac849c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ All notable changes to GBrain will be documented in this file. +## [0.42.73.2] - 2026-08-05 + +**A write that deduplication redirects onto an existing page is now checked against the write scope of whoever asked for it.** When the same content arrives under a new slug, gbrain recognises it and points the write at the page that already holds it. That redirected target is now tested against the caller's own scope — under whichever mechanism confines that caller. One of the two mechanisms was consulted at that point; both are now. + +Nothing changes for local CLI use, or for clients that hold unrestricted write access — neither was ever scope-confined. A confined caller whose write dedups onto a page **inside** its own scope keeps working exactly as before; that redirect is a feature and it is preserved, with a regression test to keep it that way. A confined caller whose write dedups onto a page **outside** its scope now gets `permission_denied`, with the remedy in the message: drop the `id:` frontmatter field, or change the content, to write a new page under your own prefix. The denial does not name the page the write resolved to. + +Recommended for any brain served over HTTP to scope-restricted clients. + +### To take advantage of v0.42.73.2 + +```bash +gbrain upgrade +``` + +Nothing to configure. Existing clients keep their scopes unchanged, and no re-registration is needed. + +### For contributors + +Reported privately by an external security researcher, who supplied a fix and a regression test with it. The version that shipped composes the two existing scope-matching rules into a single predicate rather than restating either one, so the check at the door and the check after a redirect cannot drift apart; the audit the report prompted closed the same gap on one further caller path. + ## [0.42.73.1] - 2026-08-05 **Removes the PR gate that v0.42.73.0 added, and reverts the v0.42.72.1 contribution-policy change it enforced.** The gate cannot function on this repository, and it caused a real incident before that was understood. diff --git a/VERSION b/VERSION index 44577710e..541b32657 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.73.1 \ No newline at end of file +0.42.73.2 diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 0afb03990..37d1244ca 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -12,7 +12,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/commands/serve-http.ts` confidential revoke extension — a pre-router `/revoke` handler validates the RFC 7009 body, verifies hash-only secrets for both `client_secret_post` and `client_secret_basic`, rejects mixed authentication, preserves the SDK path for public clients, and separates opaque client-auth failures from retryable/backend failures. OAuth metadata advertises both confidential methods. Pinned by `test/e2e/serve-http-oauth.test.ts`. -- `src/core/operations.ts` — Contract-first operation definitions (the foundation). Exports upload validators `validateUploadPath`, `validatePageSlug`, `validateFilename`, plus `matchesSlugAllowList(slug, prefixes)` (glob matcher: `/*` matches recursive children; bare `` matches exact only). `OperationContext.remote` is a REQUIRED field flagging untrusted callers; `OperationContext.allowedSlugPrefixes` is the trusted-workspace allow-list set by the dream cycle; `OperationContext.auth?: AuthInfo` is threaded through HTTP dispatch for scope enforcement in `serve-http.ts` before the op runs. OAuth `whoami` exposes the authenticated `AuthInfo.sourceId` and `AuthInfo.allowedSources` grants as `source_id` and `federated_read`; absent grants serialize fail-closed as `null` and `[]`, while local, legacy, and stdio response shapes stay unchanged. `enforceSubagentSlugFence(ctx, slug, opName)` is the shared fail-closed subagent write fence: when `viaSubagent` and `allowedSlugPrefixes` is set, the slug must match the allow-list; else the legacy `wiki/agents//...` namespace check applies. Both `put_page` and `add_timeline_entry` (subagent-allowlisted) route through it. Auto-link skipped only when `remote=true && !trustedWorkspace`. `enforceClientSlugFence(ctx, slug, opName)` is the OAuth-client write fence: when `ctx.auth.boundSlugPrefixes` is present (threaded from `oauth_clients.bound_slug_prefixes` at token-verification time), every direct slug-mutating write op — `put_page`, `delete_page`, `restore_page`, `add_tag`, `remove_tag`, `add_link`/`remove_link` (`from` endpoint only; linking TO a readable page is a reference), `add_timeline_entry`, `revert_version`, `put_raw_data` — rejects out-of-prefix slugs with `permission_denied`, BEFORE each op's dry-run short-circuit. Plain-startsWith semantics matching `submit_agent`'s check for the same column (NOT the glob grammar of the subagent allow-list); empty-array binding is deny-all (fail-closed); no auth / no binding = no fence. The match rule itself lives in the exported `slugUnderBoundPrefixes(prefixes, slug)` so non-op write surfaces reuse it verbatim. It is BOUNDARY-AWARE (a prefix matches whole segments, so `emp-alice` does not admit `emp-alice-2/…`), lowercases both sides (stored slugs are lowercased by `validateSlug`, so comparing the caller's raw string let a mixed-case slug commit and only then trip the resolved-slug re-check), accepts BOTH the trailing-slash and the v85 `/*` glob spelling via `normalizeSlugPrefix` (the column predates this fence as submit_agent's binding, so one stored value must mean one span of slugs on both paths), and ignores empty-string prefixes. `assertValidSlugPrefixes` (`oauth-provider.ts`) rejects empty, whitespace-bearing, non-lowercase, and boundary-less entries at registration and rescope. `submit_agent` applies the same boundary-aware rule when validating a requested prefix against the binding, normalizes trailing-slash prefixes to the glob form `matchesSlugAllowList` expects before handing them to the child job, and collapses an EXPLICIT empty `allowed_tools`/`allowed_slug_prefixes` to the binding (the worker reads empty as "full registry" / "legacy `wiki/agents//` namespace", so `??` — which only substitutes null/undefined — left a vacuous-subset bypass). `put_page` additionally fences the RESOLVED slug when importFromContent's dedup pre-check redirects the write to a different page (same content_hash / `frontmatter.id`), since the disk write-through runs against that slug. `CLIENT_FENCED_WRITE_OPS` + `enforceBoundClientOpAllowList(auth, op)` are the fail-closed companion, applied once in `src/mcp/dispatch.ts` (the choke point both MCP transports share): a slug-bound client calling ANY `write`/`admin` op not on the allow-list gets `permission_denied`. This covers the ops that write by a key other than a slug and therefore cannot be fenced — `extract_entities`/`extract_facts` (mutate `people/*`, `companies/*`), `forget_fact` (numeric fact id, crosses sources), `ontology_propose` — and makes a write op added later denied-by-default instead of silently unfenced. `think` is on the allow-list because remote callers cannot persist from it. Pinned by `test/client-slug-fence.test.ts` and over-the-wire by `test/e2e/qm-provisioning.test.ts`. Every `Operation` carries `scope?: 'read' | 'write' | 'admin'` + `localOnly?: boolean`; `sync_brain`, `file_upload`, `file_list`, `file_url` are `admin + localOnly` (rejected over HTTP). Four trust-boundary call sites (`put_page` allowlist, file_upload trust-narrowing, submit_job protected-name guard, auto-link skip) use FAIL-CLOSED semantics: `ctx.remote === false` for trusted-only sites, `ctx.remote !== false` for "untrust unless explicit-false" — anything not strictly `false` is treated as remote (closes the HTTP MCP shell-job RCE where a read+write OAuth token could submit `shell` jobs). `sourceScopeOpts(ctx)` encodes the source-scoped read precedence ladder — federated array (`ctx.auth.allowedSources`) wins over scalar (`ctx.sourceId`/`ctx.auth.sourceId`) over nothing; every read-side op handler routes through it so a source-bound OAuth client can't see neighboring sources via `search`/`query`/`list_pages`/`get_page`/`find_experts`/query's image path, plus the by-slug reads `get_tags`/`get_links`/`get_backlinks`/`get_timeline` (and `get_page`'s tag fetch, which resolves against the concrete page's own `source_id`). `linkReadScopeOpts(ctx)` is the link-read sibling for `get_links`/`get_backlinks`: a link row references three pages (from/to/origin), and the engine's federated (`sourceIds[]`) branch scopes ALL THREE while its scalar (`sourceId`) branch scopes only the near endpoint (by design — trusted internal callers like `reconcileLinks` and back-link validators call the engine with a scalar scope and need the cross-source view). For an UNTRUSTED remote caller (`ctx.remote !== false`) carrying only a scalar scope it promotes that scope to a single-element `sourceIds:[id]`, routing them through the all-endpoint branch so a far/origin slug from outside the grant isn't disclosed; a federated array passes through unchanged and trusted local CLI (`ctx.remote === false`) keeps the scalar cross-source view. `thinkSourceScopeOpts(ctx)` maps the same precedence ladder onto `runThink`'s public options (`allowedSources`/`sourceId`) so the `think` op's gather and trajectory stages inherit the caller's source grant. `put_page`'s inline disk write-through is the shared `writePageThrough` helper (`src/core/write-through.ts`), ATOMIC via temp-sibling + rename so a crash or concurrent `gbrain sync` can't read a half-written `.md`; same helper backs `gbrain brainstorm/lsd --save`. Link provenance surface (#1941): `add_link` (`gbrain link`/`link-add`) + `remove_link` (`gbrain unlink`/`link-rm`) expose `link_source`/`link_type`; `add_link` rejects the reconciliation-managed built-ins via `MANAGED_LINK_SOURCES` (`markdown`/`frontmatter`/`mentions`/`wikilink-resolved`) and defaults omitted provenance to `'manual'` (the engine's own default stays `'markdown'` for internal callers); `list_link_sources` (`gbrain link-sources`, read) lists provenances via `sourceScopeOpts`. CLI aliases register through `cliHints.aliases` (collision-guarded in `src/cli.ts`). +- `src/core/operations.ts` — Contract-first operation definitions (the foundation). Exports upload validators `validateUploadPath`, `validatePageSlug`, `validateFilename`, plus `matchesSlugAllowList(slug, prefixes)` (glob matcher: `/*` matches recursive children; bare `` matches exact only). `OperationContext.remote` is a REQUIRED field flagging untrusted callers; `OperationContext.allowedSlugPrefixes` is the trusted-workspace allow-list set by the dream cycle; `OperationContext.auth?: AuthInfo` is threaded through HTTP dispatch for scope enforcement in `serve-http.ts` before the op runs. OAuth `whoami` exposes the authenticated `AuthInfo.sourceId` and `AuthInfo.allowedSources` grants as `source_id` and `federated_read`; absent grants serialize fail-closed as `null` and `[]`, while local, legacy, and stdio response shapes stay unchanged. `enforceSubagentSlugFence(ctx, slug, opName)` is the shared fail-closed subagent write fence: when `viaSubagent` and `allowedSlugPrefixes` is set, the slug must match the allow-list; else the legacy `wiki/agents//...` namespace check applies. Both `put_page` and `add_timeline_entry` (subagent-allowlisted) route through it. Auto-link skipped only when `remote=true && !trustedWorkspace`. `enforceClientSlugFence(ctx, slug, opName)` is the OAuth-client write fence: when `ctx.auth.boundSlugPrefixes` is present (threaded from `oauth_clients.bound_slug_prefixes` at token-verification time), every direct slug-mutating write op — `put_page`, `delete_page`, `restore_page`, `add_tag`, `remove_tag`, `add_link`/`remove_link` (`from` endpoint only; linking TO a readable page is a reference), `add_timeline_entry`, `revert_version`, `put_raw_data` — rejects out-of-prefix slugs with `permission_denied`, BEFORE each op's dry-run short-circuit. Plain-startsWith semantics matching `submit_agent`'s check for the same column (NOT the glob grammar of the subagent allow-list); empty-array binding is deny-all (fail-closed); no auth / no binding = no fence. The match rule itself lives in the exported `slugUnderBoundPrefixes(prefixes, slug)` so non-op write surfaces reuse it verbatim. It is BOUNDARY-AWARE (a prefix matches whole segments, so `emp-alice` does not admit `emp-alice-2/…`), lowercases both sides (stored slugs are lowercased by `validateSlug`, so comparing the caller's raw string let a mixed-case slug commit and only then trip the resolved-slug re-check), accepts BOTH the trailing-slash and the v85 `/*` glob spelling via `normalizeSlugPrefix` (the column predates this fence as submit_agent's binding, so one stored value must mean one span of slugs on both paths), and ignores empty-string prefixes. `assertValidSlugPrefixes` (`oauth-provider.ts`) rejects empty, whitespace-bearing, non-lowercase, and boundary-less entries at registration and rescope. `submit_agent` applies the same boundary-aware rule when validating a requested prefix against the binding, normalizes trailing-slash prefixes to the glob form `matchesSlugAllowList` expects before handing them to the child job, and collapses an EXPLICIT empty `allowed_tools`/`allowed_slug_prefixes` to the binding (the worker reads empty as "full registry" / "legacy `wiki/agents//` namespace", so `??` — which only substitutes null/undefined — left a vacuous-subset bypass). `put_page` additionally fences the RESOLVED slug when importFromContent's dedup pre-check redirects the write to a different page (same content_hash / `frontmatter.id`), since the disk write-through runs against that slug. That re-check applies whichever confinement the CALLER is under — OAuth binding and/or subagent allow-list/legacy namespace — via `slugOutsideCallerFence(ctx, slug)`, which composes `slugUnderBoundPrefixes` with the subagent fence's own match rule: the delegated `submit_agent` → subagent context carries `viaSubagent` + `allowedSlugPrefixes` but NO `auth`, so an auth-only test let a slug-bound client holding `agent` scope reach an out-of-fence page simply by delegating the write. Denials never name the resolved slug (it would be a slug-enumeration oracle). Pinned by `test/put-page-dedup-fence.test.ts`. `CLIENT_FENCED_WRITE_OPS` + `enforceBoundClientOpAllowList(auth, op)` are the fail-closed companion, applied once in `src/mcp/dispatch.ts` (the choke point both MCP transports share): a slug-bound client calling ANY `write`/`admin` op not on the allow-list gets `permission_denied`. This covers the ops that write by a key other than a slug and therefore cannot be fenced — `extract_entities`/`extract_facts` (mutate `people/*`, `companies/*`), `forget_fact` (numeric fact id, crosses sources), `ontology_propose` — and makes a write op added later denied-by-default instead of silently unfenced. `think` is on the allow-list because remote callers cannot persist from it. Pinned by `test/client-slug-fence.test.ts` and over-the-wire by `test/e2e/qm-provisioning.test.ts`. Every `Operation` carries `scope?: 'read' | 'write' | 'admin'` + `localOnly?: boolean`; `sync_brain`, `file_upload`, `file_list`, `file_url` are `admin + localOnly` (rejected over HTTP). Four trust-boundary call sites (`put_page` allowlist, file_upload trust-narrowing, submit_job protected-name guard, auto-link skip) use FAIL-CLOSED semantics: `ctx.remote === false` for trusted-only sites, `ctx.remote !== false` for "untrust unless explicit-false" — anything not strictly `false` is treated as remote (closes the HTTP MCP shell-job RCE where a read+write OAuth token could submit `shell` jobs). `sourceScopeOpts(ctx)` encodes the source-scoped read precedence ladder — federated array (`ctx.auth.allowedSources`) wins over scalar (`ctx.sourceId`/`ctx.auth.sourceId`) over nothing; every read-side op handler routes through it so a source-bound OAuth client can't see neighboring sources via `search`/`query`/`list_pages`/`get_page`/`find_experts`/query's image path, plus the by-slug reads `get_tags`/`get_links`/`get_backlinks`/`get_timeline` (and `get_page`'s tag fetch, which resolves against the concrete page's own `source_id`). `linkReadScopeOpts(ctx)` is the link-read sibling for `get_links`/`get_backlinks`: a link row references three pages (from/to/origin), and the engine's federated (`sourceIds[]`) branch scopes ALL THREE while its scalar (`sourceId`) branch scopes only the near endpoint (by design — trusted internal callers like `reconcileLinks` and back-link validators call the engine with a scalar scope and need the cross-source view). For an UNTRUSTED remote caller (`ctx.remote !== false`) carrying only a scalar scope it promotes that scope to a single-element `sourceIds:[id]`, routing them through the all-endpoint branch so a far/origin slug from outside the grant isn't disclosed; a federated array passes through unchanged and trusted local CLI (`ctx.remote === false`) keeps the scalar cross-source view. `thinkSourceScopeOpts(ctx)` maps the same precedence ladder onto `runThink`'s public options (`allowedSources`/`sourceId`) so the `think` op's gather and trajectory stages inherit the caller's source grant. `put_page`'s inline disk write-through is the shared `writePageThrough` helper (`src/core/write-through.ts`), ATOMIC via temp-sibling + rename so a crash or concurrent `gbrain sync` can't read a half-written `.md`; same helper backs `gbrain brainstorm/lsd --save`. Link provenance surface (#1941): `add_link` (`gbrain link`/`link-add`) + `remove_link` (`gbrain unlink`/`link-rm`) expose `link_source`/`link_type`; `add_link` rejects the reconciliation-managed built-ins via `MANAGED_LINK_SOURCES` (`markdown`/`frontmatter`/`mentions`/`wikilink-resolved`) and defaults omitted provenance to `'manual'` (the engine's own default stays `'markdown'` for internal callers); `list_link_sources` (`gbrain link-sources`, read) lists provenances via `sourceScopeOpts`. CLI aliases register through `cliHints.aliases` (collision-guarded in `src/cli.ts`). - `src/core/engine.ts` — Pluggable engine interface (BrainEngine). `clampSearchLimit(limit, default, cap)` takes an explicit cap so per-operation caps can be tighter than `MAX_SEARCH_LIMIT`. Exports `LinkBatchInput`/`TimelineBatchInput` for the bulk-insert API (`addLinksBatch`/`addTimelineEntriesBatch`). `readonly kind: 'postgres' | 'pglite'` discriminator lets `src/core/migrate.ts` and others branch without `instanceof` + dynamic imports. Methods: `batchLoadEmotionalInputs(slugs?)` (CTE-shaped read with per-table aggregates so page × N tags × M takes never produces N×M rows), `setEmotionalWeightBatch(rows)` (`UPDATE FROM unnest($1::text[],$2::text[],$3::real[])` composite-keyed on `(slug, source_id)`), `getRecentSalience(opts)`, `findAnomalies(opts)`. `PageFilters` has `sort?: 'updated_desc'|'updated_asc'|'created_desc'|'slug'` + `PAGE_SORT_SQL` whitelist consumed by both engines. `listAllPageRefs(): Promise>` ordered by `(source_id, slug)` — cheap cross-source enumeration replacing the `getAllSlugs()→getPage(slug)` N+1 (which silently defaulted to `source_id='default'`); parity across postgres-engine.ts + pglite-engine.ts; Pinned by `test/e2e/multi-source-bug-class.test.ts`. `SearchOpts`+`PageFilters` add `sourceIds?: string[]` (federated read axis; both engines apply `WHERE source_id = ANY($N::text[])` when set, preserve scalar `sourceId` fast path when unset); `traverseGraph(slug, depth, opts?)` and `traversePaths(slug, opts?)` accept `opts.sourceId`/`opts.sourceIds`. The by-slug read methods carry the same federated axis: `getTags`/`getLinks`/`getBacklinks` opts and `TimelineOpts` (consumed by `getTimeline`) accept `sourceIds?: string[]` taking precedence over the scalar `sourceId` (`source_id = ANY($::text[])` scoping the slug→page-id lookup); the link reads (`getLinks`/`getBacklinks`) scope ALL THREE endpoints (from/to/origin) on the federated branch while the scalar branch scopes only the near endpoint for trusted internal cross-source callers. `traverseGraph` opts has `frontierCap?: number` (per-iteration recursive-CTE cap, approx per-BFS-layer); return type `Promise` for MCP wire stability; export `TraverseGraphOpts`; Postgres uses parenthesized `LIMIT N ORDER BY (slug, id)` inside the recursive term, PGLite mirrors with positional params; Pinned by `test/regressions/v0_36_frontier_cap.test.ts`. Phantom-redirect methods: `refreshPageBody(slug, sourceId, compiled_truth, timeline, content_hash)` narrow-UPDATEs three columns + updated_at, skipping soft-deleted rows (content_hash refresh required so `gbrain sync` sees the canonical as unchanged after fence merge); `migrateFactsToCanonical(phantomSlug, canonicalSlug, sourceId)` UPDATEs `entity_slug`+`source_markdown_slug` on every active fact row keyed on the phantom, preserving embedding/validUntil/kind/status/source_session/confidence; parity at `test/phantom-redirect-engine-parity.test.ts`. `getAdjacencyBoosts(pageIds): Promise>` powers the per-query graph-signals stage — one SQL query returning inbound-link counts among top-K plus a cross-source count (links from differing `source_id`); `COALESCE(p.source_id,'default')` null safety, `HAVING >= 1`, cross-source CASE-WHEN excludes the target's own source; parity SQL across both engines; `SearchResult` gains optional `base_score`, `backlink_boost`, `salience_boost`, `recency_boost`, `exact_match_boost`, `graph_adjacency_boost`, `graph_cross_source_boost`, `session_demote_factor`, `reranker_delta` + internal staging fields; Pinned by `test/e2e/graph-signals-engine.test.ts`. Two REQUIRED methods: `deletePages(slugs, {sourceId}): Promise` (single-batch primitive returning slugs actually deleted) and `resolveSlugsByPaths(paths, {sourceId}): Promise>` (batch path→slug lookup); `sourceId` REQUIRED on both at the type level (asymmetric with single-row `deletePage` which keeps optional/'default'); both short-circuit on empty input and throw when `> DELETE_BATCH_SIZE`. Embedding-signature stale-detection quartet: `countStaleChunks(opts?)` gains optional `signature?: string` widening the stale predicate from `embedding IS NULL` to ALSO include chunks whose JOINed page `embedding_signature IS NOT NULL AND <> $signature` (NULL signature is GRANDFATHERED, never counted; omit `signature` for the legacy NULL-only count); `sumStaleChunkChars(opts?: {sourceId?, signature?}): Promise` = `SUM(LENGTH(chunk_text))` over stale chunks (same predicate + embed_skip filter + optional sourceId scope), used by `gbrain sync --all` cost preview via `estimateCostFromChars`; `setPageEmbeddingSignature(slug, {sourceId?, signature})` stamps `pages.embedding_signature` after a page's chunks (re)embed, idempotent no-op when page absent; `invalidateStaleSignatureEmbeddings({signature, sourceId?}): Promise` NULLs `embedding`+`embedded_at` on every chunk whose page signature is set AND differs, returning the count, called BEFORE `listStaleChunks` so signature-drift pages flow through the NULL-embedding keyset cursor unchanged (NULL never invalidated). Widens `findOrphanPages(opts?: {sourceId?, sourceIds?})` (candidate-side scoping only; inbound links counted from any source). Pinned by `test/sum-stale-chunk-chars.test.ts`, `test/embedding-signature-stale.test.ts`, `test/e2e/engine-parity.test.ts`. Free-text alias layer: `resolveAliases(aliasNorms, opts?): Promise>>` (READ; maps each normalized alias to declaring `(slug, source_id)` pairs, source-scoped) and `setPageAliases(slug, sourceId, aliasNorms)` (WRITE; replaces the full alias set, delete-then-insert, empty clears, idempotent on the unique triple), called by the `importFromContent` ingest projection and the `reindex --aliases` backfill; parity across both engines, Pinned by `test/search/page-aliases-engine.test.ts`. `searchVector` in both engines injects the shared `buildBestPerPagePoolCte` per-page max-pool so a page surfaces on its strongest chunk. `executeRawDirect(sql, params?, opts?)` is the lock-hot-path sibling of `executeRaw`: same single-statement contract, but routes to the direct session-mode pool when dual-pool is active (Postgres/Supabase port 5432) so a long-held lock heartbeat survives the transaction pooler's per-transaction connection recycling; PGLite delegates straight to `executeRaw` (no pooler). Both engines implement it; the Minion lock path (`claim`/`renewLock`) is the consumer. `reconnect(ctx?: {error?})` is a REQUIRED lifecycle method on BOTH engines: it recovers a dropped connection using the config captured at the last `connect()`, so callers (autopilot health probe, `batchRetry`) never `disconnect()` + bare `connect()` (which loses the config and throws `database_url undefined` forever, and opens a null-connection window). PostgresEngine rebuilds its pool with a `_reconnecting` reentrancy guard and emits pool-recovery audit; PGLite is single-writer in-process so it just restores the saved data dir for parity. - `src/core/engine-constants.ts` — single source of truth for engine batch-sizing constants. Exports `DELETE_BATCH_SIZE = 500` consumed by both engines' `deletePages` + `resolveSlugsByPaths` and by the sync delete + rename loops. Lives outside `engine.ts` (the interface module) to avoid circular-import worry — bounded per-statement work for predictable lock hold time + write amplification. - `src/core/background-work.ts` (#1762/#1745/#1775) — process background-work registry: the single owner of "drain every fire-and-forget DB-write sink before the CLI disconnects." `registerBackgroundWorkDrainer({name, order, drain(timeoutMs), abort?})` + `drainAllBackgroundWorkForCliExit({timeoutMs})` over a `Map` (idempotent registration by name; `__registerDrainerForTest` returns an unregister handle). Drains in explicit `(order, name)` order — facts FIRST (order 0) so its abort-path DB `logIngest` runs against the freshest live engine — and AWAITS `abort()` only when `drain()` reports `unfinished>0`. Best-effort per drainer: one sink's failure never blocks the others or the disconnect. FIVE sinks register at module import: `facts/queue.ts` (order 0; `abort`=`shutdown()` cancels a hung facts:absorb Haiku via internalAbort), `last-retrieved.ts` (order 1), `search/hybrid.ts` (order 2; `awaitPendingSearchCacheWrites` bounded via `Promise.race`), `eval-capture.ts` (order 3; `captureEvalCandidate` self-tracks its promise via `awaitPendingEvalCaptures`), `context/volunteer-events.ts` (order 4, #2095 — batched volunteer-event INSERTs, drained like the rest). Every cli.ts teardown site reaches it through `finishCliTeardown` (`src/core/cli-force-exit.ts`), which drains the registry before `engine.disconnect()` — closing the PGLite busy-loop where `db.close()` raced an in-flight job and pinned the single-writer lock (#1762). Exports `backgroundWorkSinkCount()` so the teardown helper computes its backstop deadline from the registered sink count. CLI-EXIT-ONLY: the facts `shutdown()` abort is permanent process state, never call in a long-lived `gbrain serve`. Companion changes: `src/core/ai/gateway.ts` `withDefaultTimeout(caller, ms)` bounds every outbound AI call (chat 300s, embed+multimodal 60s; env `GBRAIN_AI_{CHAT,EMBED,MULTIMODAL}_TIMEOUT_MS`; composed with caller signals via `AbortSignal.any`) and the teardown backstop honors an errored op's exit code so a hung disconnect can't mask failure as success (see `cli-force-exit.ts`); `src/core/postgres-engine.ts` `reconnect()` module-mode branch re-establishes via idempotent `db.connect()` + `connectionManager.setReadPool` refresh instead of `db.disconnect()` (no null window for concurrent ops; fail-loud on real connect failure — #1745); `src/core/search/hybrid.ts` `embedQueryBounded` + a shared `QueryEmbedDeadline` (6s, floored 2s per embed via `MIN_QUERY_EMBED_BUDGET_MS`; env `GBRAIN_QUERY_EMBED_TIMEOUT_MS`) bounds the cache-lookup AND inner query embeds so a stalled provider falls back to keyword instead of stalling the whole op (#1775). Incorporates + hardens PR #1763 (@ElliotDrel). Pinned by `test/core/background-work.test.ts`, `test/search/query-embed-deadline.test.ts`, `test/eval-capture-drain.test.ts`, `test/e2e/postgres-reconnect-singleton.test.ts`, `test/e2e/pglite-cli-exit.serial.test.ts`, `test/fix-wave-structural.test.ts`. diff --git a/docs/integrations/qm-harness.md b/docs/integrations/qm-harness.md index cdf785b0d..14c2165ef 100644 --- a/docs/integrations/qm-harness.md +++ b/docs/integrations/qm-harness.md @@ -60,7 +60,15 @@ Isolation model: The write fence is a **write** boundary within a source. It is not a privacy boundary, and it does not make every side effect prefix-clean. As of -v0.42.72.0: +v0.42.73.2: + +- **The fence follows a delegated write.** When a client with `agent` scope + hands work to a subagent via `submit_agent`, that subagent runs under its own + slug confinement rather than the parent's OAuth binding. Both confinements are + enforced, including on the path where deduplication redirects a write onto an + existing page: the redirected target is checked against whichever confinement + the calling context actually carries, so delegation does not widen what a + client can write. - **`add_link`/`remove_link` fence the `from` endpoint only.** A bound client can create an edge pointing AT a page it cannot write; the edge's `context` diff --git a/package.json b/package.json index 351ef72de..6fe3d3b43 100644 --- a/package.json +++ b/package.json @@ -148,7 +148,7 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.73.1", + "version": "0.42.73.2", "overrides": { "@hono/node-server": "^2.0.5", "fast-uri": "^3.1.5", diff --git a/src/core/operations.ts b/src/core/operations.ts index 46fdfb3ad..0b21f539e 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -213,20 +213,49 @@ function enforceSubagentSlugFence(ctx: OperationContext, slug: string, opName: s if (typeof ctx.subagentId !== 'number' || Number.isNaN(ctx.subagentId)) { throw new OperationError('permission_denied', `${opName} via subagent requires ctx.subagentId`); } + if (slugUnderSubagentFence(ctx, slug)) return; const allowList = ctx.allowedSlugPrefixes; - if (allowList && allowList.length > 0) { - if (!matchesSlugAllowList(slug, allowList)) { - throw new OperationError( - 'permission_denied', - `${opName} slug '${slug}' is not within the trusted-workspace allow-list (${allowList.join(', ')})` - ); - } - } else { - const prefix = `wiki/agents/${ctx.subagentId}/`; - if (!slug.startsWith(prefix) || slug.length === prefix.length) { - throw new OperationError('permission_denied', `${opName} via subagent must write under '${prefix}...'`); - } - } + throw new OperationError( + 'permission_denied', + allowList && allowList.length > 0 + ? `${opName} slug '${slug}' is not within the trusted-workspace allow-list (${allowList.join(', ')})` + : `${opName} via subagent must write under 'wiki/agents/${ctx.subagentId}/...'`, + ); +} + +/** + * The subagent fence's MATCH RULE, without the throwing. Split out so the + * resolved-slug re-check in put_page can ask the same question the entry + * fence asks, instead of re-deriving the namespace literal and drifting. + * Callers must have already established `ctx.viaSubagent === true`. + */ +function slugUnderSubagentFence(ctx: OperationContext, slug: string): boolean { + const allowList = ctx.allowedSlugPrefixes; + if (allowList && allowList.length > 0) return matchesSlugAllowList(slug, allowList); + const prefix = `wiki/agents/${ctx.subagentId}/`; + return slug.startsWith(prefix) && slug.length > prefix.length; +} + +/** + * Is `slug` outside whatever slug confinement THIS caller is under? + * + * A caller can be confined by EITHER mechanism, and the two arrive on + * different context fields: an OAuth binding lands on `ctx.auth + * .boundSlugPrefixes` (plain-prefix grammar), while a delegated subagent + * lands on `ctx.viaSubagent` + `ctx.allowedSlugPrefixes` (glob grammar) and + * carries NO `ctx.auth` at all. Testing only the OAuth field therefore lets + * a bound client that also holds `agent` scope re-open the path it is fenced + * out of simply by delegating the write through submit_agent — the same + * bypass shape the facts-backstop gate below is keyed against. + * + * Unconfined callers (local CLI, unbound client) match neither arm and are + * never fenced. + */ +function slugOutsideCallerFence(ctx: OperationContext, slug: string): boolean { + const bound = ctx.auth?.boundSlugPrefixes; + if (bound && !slugUnderBoundPrefixes(bound, slug)) return true; + if (ctx.viaSubagent === true && !slugUnderSubagentFence(ctx, slug)) return true; + return false; } /** @@ -1156,14 +1185,13 @@ const put_page: Operation = { // touching the DB, so throwing here leaves nothing to roll back. if (result.slug && result.slug !== slug) { // Deliberately does NOT name the resolved slug: it belongs to a page - // outside the binding, and echoing it would turn frontmatter-id guessing + // outside the fence, and echoing it would turn frontmatter-id guessing // into a slug-enumeration oracle. - if (!slugUnderBoundPrefixes(ctx.auth?.boundSlugPrefixes ?? [], result.slug) - && ctx.auth?.boundSlugPrefixes) { - ctx.logger.warn(`[put_page] dedup resolved '${slug}' to an out-of-fence page; refusing (client ${ctx.auth.clientId ?? 'unknown'})`); + if (slugOutsideCallerFence(ctx, result.slug)) { + ctx.logger.warn(`[put_page] dedup resolved '${slug}' to an out-of-fence page; refusing (client ${ctx.auth?.clientId ?? 'unknown'}, subagent ${ctx.subagentId ?? 'none'})`); throw new OperationError( 'permission_denied', - `put_page: this content already exists on a page outside your bound_slug_prefixes, so the write would have modified that page instead.`, + `put_page: this content already exists on a page outside your write scope, so the write would have modified that page instead.`, 'Remove the `id:` frontmatter field (or change the content) to write a new page under your own prefix.', ); } diff --git a/test/put-page-dedup-fence.test.ts b/test/put-page-dedup-fence.test.ts new file mode 100644 index 000000000..86bbb14b2 --- /dev/null +++ b/test/put-page-dedup-fence.test.ts @@ -0,0 +1,188 @@ +/** + * put_page dedup resolved-slug fence (v0.42.73.1). + * + * importFromContent's dedup pre-check can resolve a write to a DIFFERENT page + * than the caller named (same `frontmatter.id`), and the disk write-through + * runs against that RESOLVED slug. The re-check that fences it shipped in + * v0.42.72.0 testing `ctx.auth.boundSlugPrefixes` only — so a slug-bound + * OAuth client holding `agent` scope could delegate the write through + * submit_agent, whose subagent context carries `viaSubagent` + + * `allowedSlugPrefixes` but NO `auth`, and land the rewrite on a page outside + * its grant. + * + * Pins: every confinement a caller can be under fences the RESOLVED slug + * (OAuth binding, trusted-workspace allow-list, legacy subagent namespace), + * unconfined callers keep the dedup redirect, an in-fence redirect still + * works, and the denial never names the resolved slug (it would be a + * slug-enumeration oracle). + * + * PGLite hermetic. Every case resolves at the dedup pre-check, which returns + * before any chunk/embed work. + */ + +import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { importFromContent } from '../src/core/import-file.ts'; +import { operations, OperationError } from '../src/core/operations.ts'; +import type { OperationContext, Operation, AuthInfo } from '../src/core/operations.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; + +let engine: PGLiteEngine; + +const VICTIM_SLUG = 'people/alice-example'; +const VICTIM_ID = 'external-uuid-victim'; + +function put(): Operation { + const found = operations.find(o => o.name === 'put_page'); + if (!found) throw new Error('put_page op missing'); + return found; +} + +function page(id: string, body: string): string { + return ['---', 'type: concept', 'title: Notes', `id: ${id}`, '---', '', body].join('\n'); +} + +function makeCtx(overrides: Partial = {}): OperationContext { + return { + engine, + config: { engine: 'pglite' } as any, + logger: { info: () => {}, warn: () => {}, error: () => {} }, + dryRun: false, + remote: true, + sourceId: 'default', + ...overrides, + }; +} + +function boundAuth(prefixes: string[]): AuthInfo { + return { + token: 'test-token', + clientId: 'gbrain_cl_dedup_fence', + scopes: ['read', 'write', 'agent'], + sourceId: 'default', + boundSlugPrefixes: prefixes, + }; +} + +/** The attacker's move: echo the victim's frontmatter id under an in-fence slug. */ +async function putEchoingVictimId(ctx: OperationContext, slug: string, id = VICTIM_ID) { + return put().handler(ctx, { slug, content: page(id, 'Attacker body, different text.') }); +} + +async function expectFenced(p: Promise): Promise { + try { + await p; + throw new Error('should have thrown'); + } catch (e) { + expect(e).toBeInstanceOf(OperationError); + expect((e as OperationError).code).toBe('permission_denied'); + expect((e as Error).message).toContain('write scope'); + // The oracle guard: the resolved slug belongs to a page the caller may + // not see, so it must never appear in the denial. + expect((e as Error).message).not.toContain('alice-example'); + } +} + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}, 60_000); + +afterAll(async () => { + if (engine) await engine.disconnect(); +}, 60_000); + +beforeEach(async () => { + await resetPgliteState(engine); + const victim = await importFromContent(engine, VICTIM_SLUG, page(VICTIM_ID, 'Confidential.'), { + noEmbed: true, + sourceId: 'default', + }); + expect(victim.status).toBe('imported'); +}); + +describe('put_page: dedup-resolved slug is fenced by the caller\'s own confinement', () => { + test('delegated subagent (allow-list, NO auth) cannot rewrite an out-of-fence page', async () => { + // The bypass: submit_agent's subagent context carries allowedSlugPrefixes + // but no auth, so an auth-only re-check skipped exactly this caller. + const ctx = makeCtx({ + viaSubagent: true, + subagentId: 7, + allowedSlugPrefixes: ['wiki/agents/7/*'], + }); + await expectFenced(putEchoingVictimId(ctx, 'wiki/agents/7/notes')); + }); + + test('legacy sandbox subagent (namespace fence, no allow-list) cannot either', async () => { + const ctx = makeCtx({ viaSubagent: true, subagentId: 7 }); + await expectFenced(putEchoingVictimId(ctx, 'wiki/agents/7/notes')); + }); + + test('slug-bound OAuth client cannot (the v0.42.72.0 case, still fenced)', async () => { + const ctx = makeCtx({ auth: boundAuth(['emp-bob/']) }); + await expectFenced(putEchoingVictimId(ctx, 'emp-bob/notes')); + }); + + test('a caller under BOTH confinements is fenced (requested slug satisfies both)', async () => { + const ctx = makeCtx({ + auth: boundAuth(['emp-bob/']), + viaSubagent: true, + subagentId: 7, + allowedSlugPrefixes: ['emp-bob/*'], + }); + await expectFenced(putEchoingVictimId(ctx, 'emp-bob/notes')); + }); + + test('feature preserved: a redirect INSIDE the fence still dedups', async () => { + const inFence = await importFromContent(engine, 'wiki/agents/7/first', page('in-fence-id', 'Body.'), { + noEmbed: true, + sourceId: 'default', + }); + expect(inFence.status).toBe('imported'); + + const ctx = makeCtx({ + viaSubagent: true, + subagentId: 7, + allowedSlugPrefixes: ['wiki/agents/7/*'], + }); + const r = await putEchoingVictimId(ctx, 'wiki/agents/7/second', 'in-fence-id') as { + slug: string; status: string; + }; + expect(r.status).toBe('skipped'); + expect(r.slug).toBe('wiki/agents/7/first'); + }); + + test('feature preserved: a bound client\'s in-fence redirect still dedups', async () => { + // The OAuth mirror of the case above — the fence must not break the happy + // path it was already allowing before this change. + const inFence = await importFromContent(engine, 'emp-bob/first', page('bob-id', 'Body.'), { + noEmbed: true, + sourceId: 'default', + }); + expect(inFence.status).toBe('imported'); + + const ctx = makeCtx({ auth: boundAuth(['emp-bob/']) }); + const r = await putEchoingVictimId(ctx, 'emp-bob/second', 'bob-id') as { + slug: string; status: string; + }; + expect(r.status).toBe('skipped'); + expect(r.slug).toBe('emp-bob/first'); + }); + + test('fail-closed: viaSubagent without a subagentId is denied before any write', async () => { + // enforceSubagentSlugFence refuses rather than trusting a dispatcher that + // set viaSubagent but forgot the id — the branch slugUnderSubagentFence + // would otherwise evaluate against 'wiki/agents/undefined/'. + const ctx = makeCtx({ viaSubagent: true }); + const p = putEchoingVictimId(ctx, 'wiki/agents/7/notes'); + await expect(p).rejects.toBeInstanceOf(OperationError); + await expect(p).rejects.toThrow(/requires ctx\.subagentId/); + }); + + test('regression: an unconfined caller keeps the dedup redirect', async () => { + const r = await putEchoingVictimId(makeCtx(), 'anywhere/notes') as { slug: string; status: string }; + expect(r.status).toBe('skipped'); + expect(r.slug).toBe(VICTIM_SLUG); + }); +});