Repository navigation
feat(mcp): await access hook answers without holding the tool mutex - #322
Open
ArnauvGilotra wants to merge 31 commits into
Open
ArnauvGilotra wants to merge 31 commits into
ArnauvGilotra wants to merge 31 commits into
Conversation
Add an opt-in `collections` allowlist to search, vsearch and query. It compiles to `d.collection IN (...)` in the shared eligibility SQL, so other collections are removed before the candidate limit for BM25, vector, hybrid fusion and graph expansion. Surfaces: repeatable `-c` on the CLI (one `-c` is unchanged), `collections` on gno_search/gno_vsearch/gno_query, REST /api/search and /api/query, and the SDK. Responses echo it as `meta.collections`. With the key unset, requests and outputs are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a request set both `collection` and `collections`, graph expansion used the plural allowlist alone, so neighbours from other allowlisted collections reached the reranker. Intersect the scopes before graph expansion and drop out-of-scope candidates before reranking. Regression test asserts the reranker never receives the excluded collection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One `collections` list was capped at 100 names over MCP, and REST had no bound at all. Deployments that keep one collection per access boundary need several hundred names in one request. Add one shared `MAX_REQUEST_COLLECTIONS = 1024` (the same value as the collection egress bound) and apply it to the MCP `collections` schema on gno_search, gno_vsearch and gno_query and to REST /api/search and /api/query, which now return a VALIDATION 400 above the bound. The MCP legacy golden is regenerated (only those three `maxItems` changed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gno_query, gno_search and gno_vsearch called with `collections` and no `collection` were judged over HTTP against every configured collection, so any stricter collection denied a request that only named remote ones. The gateway now uses the served scope: the list, or `collection` when the list holds it. An empty intersection is judged against both, never as an unscoped call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gno_query_diagnose omits the collections allowlist, and tool inputs are now closed, so sending collections there is a validation error rather than being ignored. Say so in the spec. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add opt-in `gateway.principals`: named bearer tokens for the resident MCP gateway. Only a `mayAssertSubject` principal may send `GNO-Subject` / `GNO-Context`; any other caller that sends them, or a malformed value, gets the redacted 403. The principal and subject reach the tool context snapshot, and an asserted subject keys the request-receipt namespace by its SHA-256 hash. Plumbing only: no tool output changes, and without principals the gateway behaves exactly as before (headers are stripped). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a principal token was rotated to equal the plain `tokenFile`, the collision check refused the principal but still accepted the plain token as the unscoped caller, so the clash failed open. Report every digest shared by two credentials and refuse all of them, the plain token included, until one is rotated again. Regression test rotates a principal onto the plain token, then back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
authorize() read the plain token once but checked principal clashes against the shared field, which a concurrent request could replace in between. A principal token equal to the plain token could then pass as the unnamed caller with no principal restrictions. Pass the request's snapshot into the clash check and add a concurrent-rotation test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lection-access-scoped-profile
Collections can declare `access` ({public}, {owner}, or {accessRef, firmwide}).
A non-admin gateway principal reads only collections its subject and context
admit, before the limit; accessRef collections that are not firm-wide stay
denied until an access hook exists. Missing, unreadable and malformed targets
give one byte-identical NOT_FOUND. Tools outside the scoped set (including
gno_changes, links, status) and gno:// resources are refused for such
principals; a new `scoped` profile advertises exactly that set. Served
documents need a valid `derived_from` typed-metadata list, and the new audit
rule provenance.required-metadata reports documents that lack one. Nothing
changes without gateway.principals.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… retrieval Every accessRef collection, firmwide included, stays denied until an access hook exists. gno_search, gno_query and gno_context now exclude documents with a malformed derived_from before candidate selection, graph expansion, reranking and the limit, through the internal mirror-hash allowlist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In access mode gno_context fills an omitted collections filter with every readable collection, so a principal that can read more than 128 collections got invalid_filter. Use the shared request bound (1024) for the input check, the capsule scope schema and its JSON schema, matching search and query.
Upstream supersession links (supersededBy on search, query, get, multi-get and Context Capsule evidence) name the end of a chain built from any document's relations, without checking whether the caller may read it. In access mode that named documents in unreadable collections. getSupersedingUris takes an optional canFollow predicate: the chain stops before the first successor the caller may not read, so a hidden document is never named and never bridges two readable ones. The MCP access paths pass the same servability check used for get, and the Context Capsule runtime threads it into retrieval. Unscoped callers are unchanged. Also covers gno_list_metadata in the non-admin refusal test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…callers A result's egressLineage lists every active collection that holds its content. In access mode, a readable document whose bytes also sit in a collection the caller may not read named that collection in search and query results, Context Capsule evidence and aggregate, and gno_recall. Scoped surfaces now return the lineage projected to readable collections (search and query in finishScopedRetrieval, the Context Capsule runtime through an egressLineageView applied after retrieval, recall in the MCP handler). The complete lineage is still computed by the pipeline and kept by retrieval traces, which record it before the projection. Unscoped callers are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Typed-metadata coverage warnings count the documents in the query scope whose metadata is pending or invalid, with the metadata predicate removed. Under collection access that predicate is what keeps documents without a valid derived_from out, so the counts included documents the caller may not read: adding one changed the warning a scoped search or query returned. Only the access hook path suppressed them. Retrieval now carries an internal accessScoped flag, set by the MCP access options for search, query and Context Capsules, and BM25, vector and hybrid retrieval omit the coverage warnings when it is set. A readable document always has valid typed metadata, so a count over readable documents alone has nothing to report. Unscoped callers are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Graph expansion metadata (meta.graphExpansion on gno_query, Context Capsule capability states) counted edges before neighbours were filtered, and its fallback reason told "no link" (graph_empty) apart from "a link to a document that was filtered out" (graph_neighbor_lookup_empty). Under collection access without an access hook, a link to a note the caller may not read therefore showed up as an explicit edge. Seeds were also looked up by content across every collection, so a hidden copy of a seed could take its slot and its count. With accessScoped set, expansion now seeds only from the allowlist, recounts edgeConfidence over the neighbours that survived every filter, and reports graph_no_new_candidates whenever no readable neighbour is added. Unscoped expansion is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A scoped supersededBy lookup first discovered the whole reachable chain (one successor query per hop), then asked canFollow about each successor of each input, one URI at a time, under the tool mutex. A long chain whose first successor the caller may not read cost a query per document before the rejection, and every distinct successor cost a serial check. The scoped walk now advances every input's chain together, one hop at a time: a hop runs one successor query for the chains' current notes and one canFollow call over the new successors they reach, and nothing past a rejected successor is read or asked about. canFollow takes a batch of URIs. A chain still advancing after 32 hops names no successor (fail closed), so one lookup costs at most 32 queries and 32 checks however many inputs it has. gno_multi_get resolves supersededBy for all its documents in one lookup instead of one per document. The unscoped walk is upstream's, unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…upstream The rewritten supersession walk gets explicit cycle cases (a self-loop and a 2-cycle name nothing, a tail cycle stops at the first repeated note, a rejection inside a cycle stops before the rejected note, the start of a cycle is never asked about) and a throwing canFollow, which fails the lookup so no successor is named. A differential test reads a corpus with supersession cycles, a chain through a collection with an access rule, a chain longer than the scoped hop limit, notes without provenance and a graph link through gno_get, gno_multi_get, gno_search and gno_query with no principal, and compares every serialized result with a frozen capture of the same calls on upstream main. Any byte of difference with access unset fails it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… may read Graph expansion for scoped callers recounted readable neighbours, but link resolution still ran over every document. A note the caller may not read (missing or invalid provenance) that shares a link target's title turned an explicit edge into an ambiguous one, and one that outranked the target took the link, dropping the readable neighbour. Confidence feeds candidate scoring, so this changed ranking as well as the reported counts. Scoped retrieval now passes a `readable` check to graph expansion. The seed-scoped neighbour lookup takes `hiddenDocumentIds`, which every resolver path (SQL, bulk and link workspace) treats as absent, and reports the documents that decided each resolution. Expansion checks those, hides the unreadable ones and resolves again until nothing new is hidden, so targets, match counts, confidence and scores never depend on hidden documents. Unscoped expansion is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since upstream's index-safety change, a server whose index holds a collection its config omits refuses every tool call (and the daemon's resident status) with an error naming those collections and their document counts. A non-admin principal reads only configured collections its access rule admits, so the refusal told it about collections it may not read, and whether one exists at all. In access mode the stale-collection check is skipped for MCP tool calls. Such a collection also keeps the resident from adopting an edited config file, which would leave the principal reading under access rules the file has since narrowed or removed; while an edit is pending its tool calls fail with UNAVAILABLE, naming nothing, until the edit is adopted. The daemon's GET /api/resident/status answers a non-admin principal 404 before any refresh or egress check, as gno_status answers NOT_FOUND: its answer and egress audit cover every collection. Unrestricted and admin callers keep the named refusal. Tests check that search, query, get and context are byte-identical with and without such a collection, including a hidden successor in it, that a pending access edit refuses a scoped caller over the resident HTTP transport, and that resident status is the same empty 404 for a scoped caller whatever the index holds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rojection gno_context now hands the model the gno-context-agent-v2 projection, which carries each evidence item's supersededBy. It is built from the already-scoped Capsule evidence, so a successor the caller may not read is null there; say so next to the other scoped supersededBy surfaces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…only A scoped caller's Context Capsule carried the config fingerprint of the whole config, an index fingerprint over every document of its collections (and the index-wide FTS state and link workspaces), and a retrieval fingerprint over every configured context. Adding, changing or removing a collection, document or context the caller may not read changed its Capsule. For access-scoped callers the config fingerprint now covers the config entries of the Capsule's collections (without their access rules) and the contexts that apply to them, the retrieval fingerprint uses those same contexts, and the index snapshot fingerprint covers the Capsule's collections and its evidence identities. The complete snapshots still guard the build against drift. Unrestricted callers are unchanged. The new test builds the same scoped Capsule over an index and config with and without hidden state and requires byte-identical output.
Opt-in `gateway.accessHook`: a fail-closed per-request hook for non-admin
gateway principals. GNO POSTs {subject, ctx, request_id, refs:[{sys, ref}]}
and serves only what `{allow:[i]}` lists; unlisted, late, non-200,
malformed or unreachable denies. Each distinct ref is asked once per
request.
- accessRef collections become readable when the hook allows their ref
(any in 1:1, firmwide only in shared, none in public_only).
- Every served document needs all refs under its collection's
`access.provenanceKey` (new; default derived_from) allowed. Checked on
BM25 and vector rows, on fused candidates before graph seeds, on graph
neighbours, again after graph fusion before rerank, and in
get/multi_get (a multi_get pattern never lists a denied document);
capsules inherit it through retrieval. Candidates are overfetched (x3),
counted in explain.
- Graph neighbours the hook denies are dropped before the scoped edge
count, so edge counts cover only allowed neighbours.
- The hook call is registered in the network boundary inventory as a
transport that sends no collection data.
- evals/acceptance/access-leak.ts: pinned leak gate at 100/1k/10k with an
exhaustive oracle, fake unix-socket hook and --negative-control.
Without `accessHook` or `provenanceKey`, behaviour and output are
unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The access hook got a fresh UUID as request_id, so a hook that matches request_id to its own turn state could not find the turn and denied every collection and document. A mayAssertSubject principal can now send GNO-Request-Id with its subject; every hook call of the request reuses it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Upstream supersession links name the newer document that replaces a result. Under collection access they already stop at successors the caller may not read; with gateway.accessHook set, a successor in a readable collection whose sources the hook denies was still named. The supersededBy predicate now asks the same per-document check as get and retrieval, so a hook-denied document is never named and never bridges a chain. The scoped walk hands it every new successor of a hop at once, so the hook gets at most one batched call per hop (and at most 32 hops), not one serial call per successor; refs already answered in the request are not asked again. accessRetrievalOptions also carries the hook check, so search, query and Context Capsule take one set of access options. The access-leak gate links documents into supersession chains and puts each marker in the document path, so a leaked supersededBy URI fails it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three chains reached in one gno_multi_get cost a single hook call for their three successors, and a chain whose second note the hook denies is never asked about the note past it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Graph link resolution for scoped callers left out documents they may not serve, but not documents the access hook denies: a hook-denied note sharing a link target's title still made the edge ambiguous, and one that outranked the target still took the link. The readable check behind resolution now asks the hook too (one batch per resolution round, cached for the request). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ection gno_context now returns the gno-context-agent-v2 projection, which carries each evidence item's supersededBy to the model. Check that a successor the access hook denies changes no byte of gno_context, gno_search or gno_get (linked versus unlinked), and that an allowed successor does appear in the projection. The fixture now uses canonical content hashes and the default index name so Context Capsule evidence can be captured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MCP tool calls run one at a time behind one process-wide mutex, and the access hook was called inside it. Every caller therefore waited for every other caller's hook answers: with a 250 ms hook, 20 concurrent users saw a p95 of 10.8 s. runTool and the gno_context tool now hold the mutex through a lease (src/mcp/tool-lease.ts). yieldToolMutex releases it for the duration of an external wait and takes it back before the tool continues; parallel waits of one call release it once and all resume only after it is held again. The access hook session awaits each hook call that way, unless a store transaction is open (other tools would join it). Tool code of different calls still never overlaps; only hook waits do. New gateway.accessHook.maxConcurrent (1..256, default 16) bounds hook calls in flight per hook, first in, first out. The timeoutMs deadline starts when a call is sent, so queueing is never a timeout denial. Verdicts stay per call and subject; every failure mode still denies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The timeoutMs timer started only once a call got a maxConcurrent slot, so a queued call could be answered allow well past its deadline. Set one absolute deadline before the call queues, drop a call still queued at it unsent, and deny any answer that arrives after it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@ArnauvGilotra is attempting to deploy a commit to the gmickel's projects Team on Vercel. A member of the Team first needs to authorize it. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #312. Depends on #321 (and through it #319, #313, #317). Based on
mainat3813e841(v2.12.0).Checklist
bun test(Bun 1.4.2; the one failure,test/soak/harness.test.ts, fails the same way on clean v2.11.0 here, see below)bun run lint:checkWhat changed
MCP tool calls run one at a time behind one process-wide mutex, and the access hook (#321) was called inside it, so every caller waited for every other caller's hook answers.
runTooland thegno_contexttool hold the mutex through a lease (newsrc/mcp/tool-lease.ts).yieldToolMutexreleases it for the duration of an external wait and takes it back before the tool continues; parallel waits of one call release it once and all resume only after it is held again.gateway.accessHook.maxConcurrent(1..256, default 16) bounds hook calls in flight per hook, first in, first out.timeoutMsis one absolute deadline set before a call queues: a call still queued at it is dropped unsent and denies, as does any answer after it.Commits to review
7efff5dafeat(mcp): await access hook answers without holding the tool mutex0bee31e0fix(mcp): count access-hook queue time toward the deadlineWhy
With a 250 ms hook and 20 concurrent users, p95 was 10.8 s; with this change it is 782 ms (local measurement).
How to test
bun test test/core/access-hook.test.ts test/mcp/access-hook-concurrency.test.ts test/mcp/tool-lease.test.ts bun evals/acceptance/access-leak.ts --output /tmp/access-leak.jsonTests run
bun test(Bun 1.4.2) on0bee31e0: 6375 pass, 7 skip, 1 fail. The failure istest/soak/harness.test.ts(sandbox teardown), which fails the same way on clean v2.11.0 here.bun run typecheckandbun run lint:check: clean.Docs, spec, changelog
spec/mcp.md,docs/CONFIGURATION.md,src/mcp/CLAUDE.md(module map), CHANGELOG[Unreleased] / Changed.Compatibility
Without
accessHook, nothing changes: no hook calls are made, and tool calls are still serialized one at a time.