diff --git a/ROADMAP.md b/ROADMAP.md index 91b2833c4..def7bba25 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -862,7 +862,7 @@ graph LR | Web UI + macOS app UX audit | In progress | P0 | — | | | | Release qualification gate (auto-QA matrix blocks the tag) | In progress | P0 | — | [081-release-qa-gate](./specs/081-release-qa-gate/) | | | Action log / transparency — info at a glance | In progress | P1 | — | | | -| Agent-token scope hardening: every MCP request authorized by its own scope (spec 105) | In progress | P1 | 0/109 (0%) | [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | | +| Agent-token scope hardening: every MCP request authorized by its own scope (spec 105) | In progress | P1 | 12/109 (11%) | [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | | | Token-efficiency benchmark: measured savings, published results | In progress | P1 | 62/64 (97%) | [103-token-bench](./specs/103-token-bench/) | | | Telemetry identity & data quality (machine_id + CI-filter hardening) | In progress | P1 | — | | | | Telemetry v7: honest funnel + churn instrumentation | In progress | P1 | — | [080-telemetry-v7-churn](./specs/080-telemetry-v7-churn/) | | @@ -1008,5 +1008,5 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—` | [102-schema-deferred](./specs/102-schema-deferred/) | `shipped` | 89/89 (100%) | | [103-token-bench](./specs/103-token-bench/) | `shipped` | 62/64 (97%) | | [104-auto-routing-mode](./specs/104-auto-routing-mode/) | — | — | -| [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | `drafted` | 0/109 (0%) | +| [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | `in-flight` | 12/109 (11%) | | [106-security-residual-fixes](./specs/106-security-residual-fixes/) | `shipped` | 18/19 (95%) | diff --git a/docs/features/agent-tokens.md b/docs/features/agent-tokens.md index a8e7e889c..ca7b727c9 100644 --- a/docs/features/agent-tokens.md +++ b/docs/features/agent-tokens.md @@ -268,17 +268,77 @@ Server scoping is enforced at three levels: receive every event unchanged; the stream is rendered per connection. 4. **Cached responses** (`read_cache`) — a truncated response is parked behind a cache key, and the key is a hash, not a credential. Every entry is stamped - with the authorization that produced it (server scope, permission tier, - profile pin, effective profile, caller kind). `read_cache` refuses, on every - page, any request whose own authorization could not have produced the entry, - so a narrower token sharing the same MCP session cannot page a broader - token's response. An unrestricted admin may read any entry; a token may read - its own entries and those of tokens at least as narrow as itself. Profile - scope is compared as a server set, so deleting or narrowing a profile after - the entry was produced revokes cached access as well (a stale pin resolves to - a deny-all scope and reads nothing). An unauthenticated `/mcp` caller ranks - below an authenticated admin: it cannot page an entry an API-key admin - produced. + with the authorization snapshot that authorized producing it (server scope, + permission tier, profile pin, effective profile, caller kind), captured when + the call was authorized — a profile narrowed while the call was in flight + does not re-stamp the response. `read_cache` — on every MCP surface and on + the REST direct call path (`POST /api/v1/tools/call`) — admits, on every + page, exactly three kinds of request and refuses every other, so a + narrower token sharing the same MCP session cannot page a broader token's + response. Ordered by **caller kind first**: an administrator may read any + entry regardless of its own profile binding (an unauthenticated `/mcp` + caller ranks below an authenticated admin and cannot page an entry an + API-key admin produced); an agent token never reads an administrator's + entry. Between agent entries a reader is admitted when it presents the + **same effective authorization** the entry was produced under — the same + token, server grant, permission tiers, pin and effective profile server + set (compared as sets, so list order and the profile's name do not + matter) — or when it is **unrestricted**: a `*` server grant, no pin, no + effective profile, and every permission tier the entry's producer held. A + token that is wider than the producer but still bounded (an `{a,b}` grant + over an `{a}` entry, a session that left the profile it produced under) + is refused: it re-runs the call under its own credential instead. Profile + scope is compared as a server set, so deleting or narrowing a profile + after the entry was produced revokes cached access as well (a stale pin + resolves to a deny-all scope and reads nothing). + + A page that `read_cache` itself has to truncate again is stamped with its + *parent's* snapshot, never the redeemer's, so provenance is monotone down + the chain. For a scoped caller every refusal — an entry it may not read, an + expired entry, an internal entry, a key that never existed — answers with + the same `cache key not found` body, status and timing (a refusal commits + the same stats write a miss does), so a key cannot be probed for + existence. A refusal also never decodes the entry's payload: the gate + reads a small header stored in front of each record, so a multi-megabyte + entry is refused as quickly as a one-line one. An expired entry is refused + like a miss and left for the periodic cleanup sweep to evict, so the + refusing read writes exactly what a miss writes. The header and the record + behind it are two encodings of the same stamp; an entry on which they + disagree (a corrupt or hand-edited database) is treated as unreadable — + refused for every caller, invalidated, never served. The header is + fixed-size and decides the whole verdict by itself: it carries the caller + kind, the permission tiers and a digest of the producer's effective + authorization, and the reader's own digest is compared against it — so a + refusal never loads the producer's snapshot, a pre-upgrade entry is + invalidated without being decoded, and a probe costs what a miss costs on + the first request after a restart as much as on the thousandth, however + many servers the producer's authorization names. Each distinct snapshot + is still stored once, under that digest, for administrator diagnostics; + nothing reads it to decide. The size statistics are reconciled from the + store by the periodic cleanup sweep, which is why an invalidated + pre-upgrade entry can leave `total_size_bytes` over-counting for at most + one sweep interval. + + Server-edition OAuth **users** are bounded by the same dispatch gates as + agent tokens (server allowlist, permission tier, effective profile), so a + user's cached entry is stamped with those dimensions as well as the user + id, and redemption requires the same user *with the same* authorization: + a grant changed or a profile changed since the entry was produced revokes + cached access exactly as it does for an agent token. + + **Upgrading.** Entries written by any release before this one — including + the immediately preceding one, which stamped a producer but no schema + version — are refused for **every** caller, administrators included, and + are invalidated on the first attempt to read them (a one-time + `cache key not found` on keys minted before the upgrade; re-run the + original tool call). The registry and repository-metadata caches mcpproxy + keeps for itself are stamped internal from this release on: never readable + through `read_cache`, and kept rather than evicted when refused. Registry + and repository-metadata entries persisted *before* the upgrade carry no + stamp, so the first `read_cache` probe of such a key after upgrading + invalidates it once — the next registry search or repository lookup + re-fetches and re-stamps it. The no-eviction guarantee applies to entries + written after the upgrade. ## Administrative Operations Are Admin-Only diff --git a/docs/features/routing-modes.md b/docs/features/routing-modes.md index 4a32a63ae..f5e023576 100644 --- a/docs/features/routing-modes.md +++ b/docs/features/routing-modes.md @@ -63,7 +63,7 @@ The default mode uses BM25 full-text search to help AI agents discover relevant - `call_tool_read` — Execute read-only tool calls - `call_tool_write` — Execute write tool calls - `call_tool_destructive` — Execute destructive tool calls -- `read_cache` — Access paginated responses. Each cached page is stamped with the authorization that produced it (agent-token server scope, permission tier, profile pin, effective profile, caller kind); a request whose own authorization could not have produced the entry is refused on every page, so a narrower token sharing an MCP session cannot read a broader token's response. +- `read_cache` — Access paginated responses. Each cached page is stamped with the authorization that produced it (agent-token server scope, permission tier, profile pin, effective profile, caller kind); a request whose own authorization could not have produced the entry is refused on every page — with the same `cache key not found` body a missing key produces, for scoped callers — so a narrower token sharing an MCP session cannot read a broader token's response or probe for its existence. Administrators read any entry (caller kind first); entries written by any release before this one and mcpproxy's own registry/repository-metadata cache entries are readable by no one (the former are invalidated on first read). See [Agent Tokens](./agent-tokens.md). **How it works:** 1. AI agent calls `retrieve_tools` with a natural language query diff --git a/internal/appctx/adapters.go b/internal/appctx/adapters.go index 21d12c7dd..4611d3c7c 100644 --- a/internal/appctx/adapters.go +++ b/internal/appctx/adapters.go @@ -279,11 +279,16 @@ func (c *CacheManagerAdapter) Get(key string) (interface{}, bool) { return record.FullContent, true } -// Set adapts the cache manager to implement our interface +// Set adapts the cache manager to implement our interface. The entry is +// stamped internal (Spec 105 FR-002): it is written on the proxy's own behalf +// and read back through the ungated Get above, so read_cache refuses it for +// every caller without evicting it. An unstamped Store would be legacy +// provenance, invalidated by the first read_cache probe of the key. func (c *CacheManagerAdapter) Set(key string, value interface{}, _ time.Duration) error { // The cache manager has a different Store signature, so we adapt it valueStr := fmt.Sprintf("%v", value) - return c.Store(key, "generic_tool", map[string]interface{}{}, valueStr, "", 0) + return c.StoreAs(key, "generic_tool", map[string]interface{}{}, valueStr, "", 0, + cache.Authorization{CallerKind: cache.CallerKindInternal}) } // Delete removes a cache entry diff --git a/internal/appctx/cache_adapter_test.go b/internal/appctx/cache_adapter_test.go new file mode 100644 index 000000000..de17e6181 --- /dev/null +++ b/internal/appctx/cache_adapter_test.go @@ -0,0 +1,49 @@ +package appctx + +import ( + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.etcd.io/bbolt" + "go.uber.org/zap" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/cache" +) + +// Spec 105 FR-002 (critique round 2, finding 7): CacheManagerAdapter.Set was +// the last production writer still going through the unstamped Store, so +// anything written through the appctx CacheManager interface landed as +// LEGACY provenance — refused for every read_cache caller and invalidated by +// the first probe of its key. The adapter writes on the proxy's own behalf +// and reads back through the ungated Get, which is the internal kind: kept, +// never redeemable through read_cache. +func TestCacheManagerAdapter_SetStampsInternal(t *testing.T) { + db, err := bbolt.Open(filepath.Join(t.TempDir(), "cache.db"), 0644, &bbolt.Options{Timeout: time.Second}) + require.NoError(t, err) + defer db.Close() + base, err := cache.NewManager(db, zap.NewNop()) + require.NoError(t, err) + defer base.Close() + adapter := &CacheManagerAdapter{Manager: base} + + require.NoError(t, adapter.Set("generic:key", "SENTINEL_VALUE", time.Hour)) + + rec, ok := base.Peek("generic:key") + require.True(t, ok) + require.NotNil(t, rec.Producer, "an unstamped entry is legacy provenance and would be invalidated on first gated read") + assert.Equal(t, cache.CallerKindInternal, rec.Producer.CallerKind) + assert.True(t, rec.HasCurrentProvenance()) + + // The adapter's own reader still serves it ... + got, ok := adapter.Get("generic:key") + require.True(t, ok) + assert.Equal(t, "SENTINEL_VALUE", got) + // ... and the gated door refuses it for an administrator WITHOUT evicting. + _, err = base.GetRecordsAs("generic:key", 0, 10, cache.Authorization{CallerKind: cache.CallerKindAdmin}) + assert.ErrorIs(t, err, cache.ErrInternalEntry) + _, ok = base.Peek("generic:key") + assert.True(t, ok, "refusing an internal entry must not evict it") +} diff --git a/internal/cache/authorization.go b/internal/cache/authorization.go index b0f61543e..1758d7892 100644 --- a/internal/cache/authorization.go +++ b/internal/cache/authorization.go @@ -1,22 +1,100 @@ package cache -import "errors" +import ( + "crypto/sha256" + "encoding/json" + "errors" + "fmt" + "slices" +) // Caller kinds recorded on a cache entry. They mirror the auth context types // the MCP layer hands out; the cache package keeps its own copy so it does not // depend on internal/auth. const ( - CallerKindAdmin = "admin" // API-key admin: unrestricted - CallerKindAdminUser = "admin_user" // OAuth admin (server edition): unrestricted - CallerKindAnonymous = "anonymous" // unauthenticated /mcp caller (back-compat admin): unrestricted + CallerKindAdmin = "admin" // API-key admin: administrator + CallerKindAdminUser = "admin_user" // OAuth admin (server edition): administrator + CallerKindAnonymous = "anonymous" // unauthenticated /mcp caller (back-compat admin): administrator-shaped CallerKindAgent = "agent" // agent token: bounded by AllowedServers/Permissions/ProfilePin CallerKindUser = "user" // OAuth user (server edition): bounded to its own identity + // CallerKindInternal marks an entry the proxy wrote for ITSELF — the + // registry search cache and the repository guesser cache. No request + // produces such an entry, so no read_cache caller can redeem it, the + // administrator included (Spec 105 FR-002, SC-005 named exception). Its + // legitimate readers are the ungated Peek/Get paths of its writers. + CallerKindInternal = "internal" ) +// callerKindCodes is the one-byte encoding of each kind in the fixed frame +// header a stored record carries (see recordHeader). Code 0 is reserved for +// "no producer" (an unstamped record); a code this table does not name is +// provenance this binary does not recognise. Never renumber: the codes are +// persisted. +var callerKindCodes = map[string]uint8{ + CallerKindAdmin: 1, + CallerKindAdminUser: 2, + CallerKindAnonymous: 3, + CallerKindAgent: 4, + CallerKindUser: 5, + CallerKindInternal: 6, +} + +var callerKindNames = func() map[uint8]string { + names := make(map[uint8]string, len(callerKindCodes)) + for kind, code := range callerKindCodes { + names[code] = kind + } + return names +}() + +// IsKnownCallerKind reports whether kind is one this binary stamps and +// gates on. A record carrying any other kind has provenance this binary does +// not recognise (see Record.HasCurrentProvenance). +func IsKnownCallerKind(kind string) bool { + _, ok := callerKindCodes[kind] + return ok +} + +// callerKindCode is the frame-header code for kind; 0 for the empty kind +// (an unstamped record), which HasCurrentProvenance refuses. +func callerKindCode(kind string) uint8 { + return callerKindCodes[kind] +} + +// callerKindFromCode is the inverse of callerKindCode; "" for a code this +// binary does not know (0 included). +func callerKindFromCode(code uint8) string { + return callerKindNames[code] +} + // ErrUnauthorizedRead is returned when a reader's authorization could not have -// produced the entry it asks for (Spec 104 FR-016a). +// produced the entry it asks for (Spec 104 FR-016a), and for the entries no +// request could have produced: legacy provenance and internal entries (Spec +// 105 FR-002). The handler surfaces one non-disclosing body for scoped callers. var ErrUnauthorizedRead = errors.New("cache entry was produced under an authorization this request does not hold") +// ErrLegacyProvenance is the ErrUnauthorizedRead a gated read returns for an +// entry with absent, legacy or unrecognised provenance (Spec 105 FR-002). The +// entry has been invalidated by the time the caller sees it. errors.Is(err, +// ErrUnauthorizedRead) holds. +var ErrLegacyProvenance = fmt.Errorf("%w: entry predates provenance stamping and has been invalidated", ErrUnauthorizedRead) + +// ErrEntryUnreadable is returned by a gated read to a reader the header +// ADMITTED whose entry then proved unreadable: a body this binary cannot +// decode, or one that disagrees with the header it sits behind. It is not an +// ErrUnauthorizedRead — the reader was entitled to the entry, and what it +// learns (its own entry is corrupt) discloses nothing about another subject +// — and it is not a miss: the refusal shape is decided on the fixed header +// only, so an admitted reader never receives it (codex round 6). The entry +// has been invalidated by the time the caller sees the error. +var ErrEntryUnreadable = errors.New("cache entry is unreadable and has been invalidated") + +// ErrInternalEntry is the ErrUnauthorizedRead a gated read returns for an +// internal (registry/guesser) entry. The entry is kept: its keys are +// guessable, and evicting on refusal would let any caller purge what the +// proxy's own readers depend on. errors.Is(err, ErrUnauthorizedRead) holds. +var ErrInternalEntry = fmt.Errorf("%w: entry is internal to the proxy", ErrUnauthorizedRead) + // Authorization is the authorization a cache entry was produced under, and the // authorization a read_cache request presents. A cache key is a hash, not a // credential: without this record a narrower token on the same MCP session @@ -24,13 +102,15 @@ var ErrUnauthorizedRead = errors.New("cache entry was produced under an authoriz type Authorization struct { CallerKind string `json:"caller_kind"` // Principal identifies the caller within its kind: agent name for agent - // tokens, user id for OAuth users. Empty for unrestricted kinds. + // tokens, user id for OAuth users. Empty for administrator kinds. Principal string `json:"principal,omitempty"` // AllowedServers is the agent token's server scope ("*" = every server). - // nil means unrestricted (admin kinds). + // nil means unrestricted for administrator kinds; for an agent it is an + // empty grant, which the dispatch gates (auth.CanAccessServer) and the + // read gate alike treat as deny-all. AllowedServers []string `json:"allowed_servers,omitempty"` // Permissions is the agent token's permission tier list. nil means - // unrestricted (admin kinds). + // unrestricted (administrator kinds). Permissions []string `json:"permissions,omitempty"` // ProfilePin is the agent token's pinned profile ("" = unpinned). ProfilePin string `json:"profile_pin,omitempty"` @@ -49,101 +129,252 @@ type Authorization struct { ProfileServers []string `json:"profile_servers,omitempty"` } -// Unrestricted reports whether the caller kind carries no server/permission -// bound of its own. -func (a Authorization) Unrestricted() bool { - switch a.CallerKind { - case CallerKindAdmin, CallerKindAdminUser, CallerKindAnonymous: - return true +// IsAdministrator reports whether the caller kind is an administrator kind: +// the API-key admin, the OAuth admin of the server edition, and the +// administrator-shaped anonymous /mcp caller. The name is deliberately about +// KIND, not reach — an administrator request can still be bounded to a +// profile, and the read gate ignores that binding (Spec 105 FR-001, D5). +func (a Authorization) IsAdministrator() bool { + return kindIsAdministrator(a.CallerKind) +} + +// IsScoped reports whether the caller kind is bounded by the dispatch gates +// — auth.CanAccessServer, HasPermission and the effective profile — rather +// than admitted as an administrator: agent tokens and server-edition OAuth +// users. A user is allowlist-scoped exactly like an agent at every dispatch +// gate (call_tool_*, direct dispatch, retrieve_tools/describe_tool +// visibility, set_profile), so the read gate bounds it the same way +// (codex round 4). +func (a Authorization) IsScoped() bool { + return kindIsScoped(a.CallerKind) +} + +// DenyAll reports whether a SCOPED snapshot could have authorized no tool +// call at all: an empty server grant (deny-all on every dispatch gate — an +// agent or user AuthContext with no AllowedServers reaches nothing), or a +// binding to an empty effective profile (an empty profile, or the scope a +// stale pin resolves to). As a producer snapshot it could not have authorized +// the entry it is stamped on, so no scoped reader qualifies for it however +// broad; as a reader it could not have produced ANY entry, its own +// deny-all-stamped one included. Administrator kinds are never deny-all here: +// the read gate admits them on kind alone. The frame header records this bit +// so the gated read can refuse a scoped reader without loading the snapshot. +func (a Authorization) DenyAll() bool { + if !a.IsScoped() { + return false } - return false + return len(a.AllowedServers) == 0 || (a.ProfileScoped && len(a.ProfileServers) == 0) } -// CouldHaveProduced reports whether reader is at least as broad as the -// producing authorization a in every dimension — i.e. whether the reader could -// have generated the entry itself. That is the read gate for read_cache: a -// reader never sees a payload it could not have obtained by calling the tool. -// -// Profile scope is compared first and for every kind: a request bounded to a -// profile (by pin, URL or set_profile) is narrower than an unscoped one, and a -// scoped reader must currently cover every server the producer's profile -// exposed — compared as server sets, so a profile that was deleted (stale pin: -// same name, deny-all scope) or narrowed since the entry was produced no -// longer reads it. -// -// The anonymous kind is unrestricted for tool calls but is not an identity -// (auth.AnonymousContext), so it ranks below an authenticated admin: it may -// read anonymous, agent and user entries, never an authenticated admin's. -func (a Authorization) CouldHaveProduced(reader Authorization) bool { - if reader.ProfileScoped { - // A deny-all scope (empty profile, or a stale pin) can call no tool, - // so it could not have produced ANY entry — including one that was - // stamped deny-all because the profile vanished while the upstream - // call was in flight. - if len(reader.ProfileServers) == 0 { - return false - } - if !a.ProfileScoped || !coversAll(reader.ProfileServers, a.ProfileServers) { - return false +// Permission tiers, mirrored from internal/auth (which this package does not +// import). The frame header records a producer's tier set as bits so an +// unrestricted reader's coverage of it is decided on the header alone; a +// tier this table does not name sets permBitOther, which no header-only +// coverage check can cover — only digest equality (or an administrator) +// admits such a producer. +const ( + permRead = "read" + permWrite = "write" + permDestructive = "destructive" +) + +const ( + permBitRead uint8 = 1 << iota + permBitWrite + permBitDestructive + permBitOther uint8 = 1 << 7 + // permBitKnown is every bit permissionBits can set; a header tier byte + // with any other bit was not written by this binary (decodeHeaderBytes). + permBitKnown = permBitRead | permBitWrite | permBitDestructive | permBitOther +) + +// permissionBits encodes a permission tier list as header bits. +func permissionBits(perms []string) uint8 { + var bits uint8 + for _, p := range perms { + switch p { + case permRead: + bits |= permBitRead + case permWrite: + bits |= permBitWrite + case permDestructive: + bits |= permBitDestructive + default: + bits |= permBitOther } } - if reader.Unrestricted() { - if reader.CallerKind == CallerKindAnonymous { - return a.CallerKind != CallerKindAdmin && a.CallerKind != CallerKindAdminUser - } - return true + return bits +} + +// canonicalAuthorization is the shape the digest hashes: the effective +// authorization with every list sorted and deduplicated, and WITHOUT the +// profile name — the gate compares server sets, not names (a renamed profile +// with the same servers is the same authorization; a stale pin keeps its +// name while resolving to deny-all). Principal is included: for a user it is +// the identity the gate requires, and for an agent it makes "digest-equal" +// mean the same credential. +type canonicalAuthorization struct { + CallerKind string `json:"k"` + Principal string `json:"p,omitempty"` + AllowedServers []string `json:"s,omitempty"` + Permissions []string `json:"t,omitempty"` + ProfilePin string `json:"pin,omitempty"` + ProfileScoped bool `json:"ps,omitempty"` + ProfileServers []string `json:"pss,omitempty"` +} + +func sortedSet(in []string) []string { + if len(in) == 0 { + return nil } - if a.Unrestricted() { - return false + out := slices.Clone(in) + slices.Sort(out) + return slices.Compact(out) +} + +// digest is the content address of an effective authorization: the SHA-256 +// of its canonical encoding. It is what the frame header stores for the +// producer and what a reader is compared by, so the gate's same-kind verdict +// is one 32-byte comparison whatever the snapshot names (research D16). +func (a Authorization) digest() [sha256.Size]byte { + data, err := json.Marshal(canonicalAuthorization{ + CallerKind: a.CallerKind, + Principal: a.Principal, + AllowedServers: sortedSet(a.AllowedServers), + Permissions: sortedSet(a.Permissions), + ProfilePin: a.ProfilePin, + ProfileScoped: a.ProfileScoped, + ProfileServers: sortedSet(a.ProfileServers), + }) + if err != nil { + // Strings, string slices and a bool: json.Marshal cannot fail. + panic(fmt.Sprintf("cache: marshal canonical authorization: %v", err)) } - if reader.CallerKind != a.CallerKind { - return false + return sha256.Sum256(data) +} + +// unrestricted reports whether an AGENT reader is a superset of every agent +// snapshot on the server and profile dimensions: a wildcard grant, no pin +// and no effective profile. Tier coverage is checked separately, on the +// header's bits. A user is never unrestricted here: the header carries no +// identity, and a user reader must be the same user. +func (a Authorization) unrestricted() bool { + return a.CallerKind == CallerKindAgent && !a.ProfileScoped && a.ProfilePin == "" && + slices.Contains(a.AllowedServers, "*") +} + +// producerFacts is everything the gate knows about a producer: exactly the +// fields the fixed frame header carries. Derived from a stored header on the +// gated door and from the Authorization itself in CouldHaveProduced, so the +// two cannot disagree. +type producerFacts struct { + Kind string + DenyAll bool + Perms uint8 + Digest [sha256.Size]byte +} + +func (a Authorization) facts() producerFacts { + return producerFacts{Kind: a.CallerKind, DenyAll: a.DenyAll(), Perms: permissionBits(a.Permissions), Digest: a.digest()} +} + +// readerFacts is the reader's side of the verdict, computed ONCE per request +// from the reader's own authorization — before the transaction, so a miss +// and a refusal do the same work — and compared against any number of +// headers in O(1). +type readerFacts struct { + Kind string + DenyAll bool + Unrestricted bool + Perms uint8 + Digest [sha256.Size]byte +} + +func newReaderFacts(reader Authorization) readerFacts { + return readerFacts{ + Kind: reader.CallerKind, + DenyAll: reader.DenyAll(), + Unrestricted: reader.unrestricted(), + Perms: permissionBits(reader.Permissions), + Digest: reader.digest(), } - switch a.CallerKind { - case CallerKindUser: - return reader.Principal != "" && reader.Principal == a.Principal - case CallerKindAgent: - if reader.ProfilePin != "" && reader.ProfilePin != a.ProfilePin { - return false - } - return coversServers(reader.AllowedServers, a.AllowedServers) && - coversAll(reader.Permissions, a.Permissions) +} + +func kindIsAdministrator(kind string) bool { + switch kind { + case CallerKindAdmin, CallerKindAdminUser, CallerKindAnonymous: + return true } return false } -// coversServers reports whether the reader's server scope includes every -// server in the producer's scope. A "*" wildcard covers everything; only a -// wildcard covers a wildcard. -func coversServers(reader, producer []string) bool { - readerAll := false - for _, s := range reader { - if s == "*" { - readerAll = true - break - } - } - if readerAll { - return true +func kindIsScoped(kind string) bool { + return kind == CallerKindAgent || kind == CallerKindUser +} + +// kindVerdict is the CALLER-KIND-FIRST part of the read gate, decided on +// the two kinds alone (Spec 105 FR-001, research D5). decided is false only +// when both sides are the same scoped kind, where the header's remaining +// facts decide. +func kindVerdict(producerKind, readerKind string) (admit, decided bool) { + if producerKind == CallerKindInternal { + return false, true } - for _, s := range producer { - if s == "*" { - return false + if kindIsAdministrator(readerKind) { + if readerKind == CallerKindAnonymous { + return producerKind != CallerKindAdmin && producerKind != CallerKindAdminUser, true } + return true, true + } + if !kindIsScoped(readerKind) || readerKind != producerKind { + return false, true } - return coversAll(reader, producer) + return false, false } -// coversAll reports whether every element of want is present in have. -func coversAll(have, want []string) bool { - set := make(map[string]struct{}, len(have)) - for _, s := range have { - set[s] = struct{}{} +// admits is THE read gate (research D16): a verdict from the producer's +// fixed header facts and the reader's precomputed facts, in O(1) — it never +// loads a producer snapshot, so a refusal does no work proportional to what +// it refuses, and a nonexistent key is indistinguishable from it in timing +// class (Spec 105 Definitions, non-disclosing refusal). In order: +// +// - caller kind first (FR-001, D5): an administrator reader is admitted to +// any request-kind snapshot (the anonymous kind never to an authenticated +// administrator's); a reader of another kind, or an internal producer, +// is refused; +// - the deny-all bits, on both sides: a scoped snapshot that could have +// authorized no call is no producer, and a deny-all reader could have +// produced nothing; +// - digest equality: the reader IS the effective authorization the entry +// was produced under — kind, identity, server grant, tier set, pin and +// profile server set, compared as sets; +// - else, for an agent snapshot only, an UNRESTRICTED agent reader +// (wildcard grant, no pin, no profile) whose tier set covers the +// producer's — a superset of anything an agent could have produced. +// +// A reader that is strictly wider than the producer but bounded (a {a,b} +// grant over an {a} entry, an unscoped session over its own profiled entry, +// a user whose grant grew) is REFUSED: FR-001 obliges the door to refuse +// non-supersets; it does not oblige it to admit every superset, and deciding +// that shape would mean loading the snapshot. Fail-closed by design. +func admits(p producerFacts, r readerFacts) bool { + if admit, decided := kindVerdict(p.Kind, r.Kind); decided { + return admit } - for _, s := range want { - if _, ok := set[s]; !ok { - return false - } + if p.DenyAll || r.DenyAll { + return false } - return true + if p.Digest == r.Digest { + return true + } + return p.Kind == CallerKindAgent && r.Unrestricted && + p.Perms&permBitOther == 0 && p.Perms&^r.Perms == 0 +} + +// CouldHaveProduced reports whether reader may redeem an entry produced +// under a: it is admits over the facts a stored header carries for a, so the +// predicate and the gated door (Manager.GetRecordsAs) return one verdict. +// See admits for the ordering. +func (a Authorization) CouldHaveProduced(reader Authorization) bool { + return admits(a.facts(), newReaderFacts(reader)) } diff --git a/internal/cache/authorization_test.go b/internal/cache/authorization_test.go index df77a4172..2b0a07153 100644 --- a/internal/cache/authorization_test.go +++ b/internal/cache/authorization_test.go @@ -3,6 +3,8 @@ package cache import ( "encoding/json" "errors" + "fmt" + "path/filepath" "testing" "go.uber.org/zap" @@ -14,6 +16,28 @@ import ( // unrestricted admin could have produced anything, a weather-only token could // not have produced a github listing. func TestAuthorization_CouldHaveProduced(t *testing.T) { + for _, tc := range couldHaveProducedCases() { + t.Run(tc.name, func(t *testing.T) { + if got := tc.producer.CouldHaveProduced(tc.reader); got != tc.want { + t.Fatalf("producer=%+v reader=%+v: got %v want %v", tc.producer, tc.reader, got, tc.want) + } + }) + } +} + +// authorizationCase is one (producer, reader) cell of the read-gate matrix. +type authorizationCase struct { + name string + producer Authorization + reader Authorization + want bool +} + +// couldHaveProducedCases is the read-gate matrix, shared by the predicate +// test and by TestGetRecordsAs_DoorAgreesWithPredicate, which drives every +// cell through the gated door (fixed header short-cuts, snapshot cache and +// all) and requires the same verdict. +func couldHaveProducedCases() []authorizationCase { admin := Authorization{CallerKind: CallerKindAdmin} anonymous := Authorization{CallerKind: CallerKindAnonymous} broad := Authorization{CallerKind: CallerKindAgent, Principal: "broad", @@ -35,20 +59,38 @@ func TestAuthorization_CouldHaveProduced(t *testing.T) { ProfileScoped: true, ProfileServers: []string{"github", "weather"}} adminInDenyAll := Authorization{CallerKind: CallerKindAdmin, Profile: "research", ProfileScoped: true, ProfileServers: []string{}} - alice := Authorization{CallerKind: CallerKindUser, Principal: "user-alice"} - bob := Authorization{CallerKind: CallerKindUser, Principal: "user-bob"} + alice := Authorization{CallerKind: CallerKindUser, Principal: "user-alice", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read", "write"}} + bob := Authorization{CallerKind: CallerKindUser, Principal: "user-bob", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read", "write"}} - cases := []struct { - name string - producer Authorization - reader Authorization - want bool - }{ + cases := []authorizationCase{ {"same agent token", broad, broad, true}, {"narrower server scope", broad, narrow, false}, {"narrower permission tier", broad, Authorization{CallerKind: CallerKindAgent, AllowedServers: []string{"github", "weather"}, Permissions: []string{"read"}}, false}, - {"broader agent may read narrower", narrow, broad, true}, + // Research D16 (codex round 5): the door decides on the fixed header + // alone, so a scoped reader is admitted only when its effective + // authorization DIGEST equals the producer's, or it is unrestricted + // (wildcard grant, no pin, no profile, every tier the producer + // held). A strictly wider but bounded reader is refused — FR-001 + // obliges refusing non-supersets, never admitting supersets, so the + // rare shape is fail-closed. Before D16 these cells admitted. + {"strictly wider bounded agent is refused (D16: digest-equal or unrestricted only)", narrow, broad, false}, + {"same effective authorization, server list in another order: digest-equal", broad, Authorization{CallerKind: CallerKindAgent, Principal: "broad", + AllowedServers: []string{"weather", "github", "github"}, Permissions: []string{"write", "read"}}, true}, + {"same scope under another agent principal is refused (D16: identity is in the digest)", broad, Authorization{CallerKind: CallerKindAgent, Principal: "twin", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read", "write"}}, false}, + {"same scope under another profile NAME is digest-equal (sets, not names)", pinnedByName("research"), pinnedByName("renamed"), true}, + {"unrestricted agent with a narrower tier set cannot read a write-tier entry", broad, Authorization{CallerKind: CallerKindAgent, Principal: "star-ro", + AllowedServers: []string{"*"}, Permissions: []string{"read"}}, false}, + {"unrestricted agent whose tier set covers the entry's reads it", narrow, Authorization{CallerKind: CallerKindAgent, Principal: "star-rw", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write"}}, true}, + {"unrestricted agent cannot read a producer holding a tier this binary does not name", Authorization{CallerKind: CallerKindAgent, Principal: "odd", + AllowedServers: []string{"github"}, Permissions: []string{"read", "audit"}}, wildcard, false}, + {"digest equality admits a producer holding a tier this binary does not name", Authorization{CallerKind: CallerKindAgent, Principal: "odd", + AllowedServers: []string{"github"}, Permissions: []string{"read", "audit"}}, Authorization{CallerKind: CallerKindAgent, Principal: "odd", + AllowedServers: []string{"github"}, Permissions: []string{"audit", "read"}}, true}, {"wildcard server scope covers everything", broad, wildcard, true}, {"explicit list does not cover wildcard", wildcard, broad, false}, {"admin reads agent entry", broad, admin, true}, @@ -60,16 +102,20 @@ func TestAuthorization_CouldHaveProduced(t *testing.T) { {"anonymous reads agent entry", broad, anonymous, true}, {"same profile pin", pinned, pinned, true}, {"different profile pin", pinned, otherPin, false}, - {"unpinned reader is broader than pinned producer", pinned, broad, true}, + {"unpinned bounded reader of a pinned producer is refused (D16: not unrestricted, not digest-equal)", pinned, broad, false}, {"pinned reader is narrower than unpinned producer", broad, pinned, false}, - {"admin bound to a URL profile cannot read an unscoped admin entry", admin, adminInProfile, false}, + // Spec 105 FR-001 (D5, task T031): superset is ordered by caller kind + // first, so an administrator's own profile binding never narrows what + // it may redeem. Before this feature the profile comparison ran first + // and these four cells were false (#1226 R1-F2). + {"admin bound to a URL profile reads an unscoped admin entry (kind first)", admin, adminInProfile, true}, {"unscoped admin reads profile-bound admin entry", adminInProfile, admin, true}, {"profile whose servers cover the producer's profile", adminInProfile, adminInWiderProfile, true}, - {"profile whose servers do not cover the producer's profile", adminInWiderProfile, adminInProfile, false}, - {"deleted profile: same name, deny-all scope, cannot read", adminInProfile, adminInDenyAll, false}, + {"admin profile that does not cover the producer's profile still reads (kind first)", adminInWiderProfile, adminInProfile, true}, + {"deleted profile: same name, deny-all scope, admin still reads (kind first)", adminInProfile, adminInDenyAll, true}, {"stale pin (profile deleted) cannot read its own earlier entry", pinned, stalePin, false}, {"deny-all reader matches nothing, not even a deny-all-stamped entry", stalePin, stalePin, false}, - {"deny-all admin reader matches nothing", adminInDenyAll, adminInDenyAll, false}, + {"deny-all admin reader is not guarded: the deny-all guard is for agents only", adminInDenyAll, adminInDenyAll, true}, {"anonymous reads a user's entry", alice, anonymous, true}, {"anonymous cannot read an admin_user entry", Authorization{CallerKind: CallerKindAdminUser, Principal: "u9"}, anonymous, false}, {"same user", alice, alice, true}, @@ -77,19 +123,72 @@ func TestAuthorization_CouldHaveProduced(t *testing.T) { {"agent cannot read a user's entry", alice, wildcard, false}, {"admin reads a user's entry", alice, admin, true}, } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if got := tc.producer.CouldHaveProduced(tc.reader); got != tc.want { - t.Fatalf("producer=%+v reader=%+v: got %v want %v", tc.producer, tc.reader, got, tc.want) - } - }) + return append(cases, userContainmentCases(alice, bob)...) +} + +// pinnedByName is a pinned agent whose effective profile carries name; the +// server set is the same whatever the name. +func pinnedByName(name string) Authorization { + return Authorization{CallerKind: CallerKindAgent, Principal: "pinned", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read", "write"}, + ProfilePin: "research", Profile: name, ProfileScoped: true, ProfileServers: []string{"github"}} +} + +// The gated door decides every verdict on the fixed frame header (caller +// kind first, the deny-all bits, the producer digest and tier bits — research +// D16) and CouldHaveProduced is the same function over the same facts. Every +// cell of the matrix is stored and probed through GetRecordsAs — an internal +// producer aside, which the door refuses before the guard — twice: through +// the manager that stored it, and through a REOPENED manager with nothing in +// memory, so the verdict is proven to come from the header on disk and a +// short-cut can never admit what the predicate refuses or refuse what it +// admits. +func TestGetRecordsAs_DoorAgreesWithPredicate(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + cases := couldHaveProducedCases() + for i, tc := range cases { + if err := m.StoreAs(fmt.Sprintf("cell-%d", i), "t", nil, `[{"v":1}]`, "", 1, tc.producer); err != nil { + t.Fatal(err) + } + } + probe := func(t *testing.T, m *Manager, pass string, i int, tc authorizationCase) { + t.Helper() + key := fmt.Sprintf("cell-%d", i) + resp, err := m.GetRecordsAs(key, 0, 10, tc.reader) + admitted := err == nil && resp != nil && len(resp.Records) == 1 + if admitted != tc.want { + t.Fatalf("%s: door admitted=%v (err=%v), predicate says %v", pass, admitted, err, tc.want) + } + if !admitted && !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("%s: refusal must be ErrUnauthorizedRead, got %v", pass, err) + } + if _, ok := m.Peek(key); !ok { + t.Fatalf("%s: the entry must survive the read", pass) + } + } + for i, tc := range cases { + t.Run(tc.name+"/same manager", func(t *testing.T) { probe(t, m, "same manager", i, tc) }) + } + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m, db = openManagerAt(t, path) + defer db.Close() + defer m.Close() + for i, tc := range cases { + t.Run(tc.name+"/reopened", func(t *testing.T) { probe(t, m, "reopened", i, tc) }) } } // Entries persisted before producer stamping existed carry no authorization. -// They are treated as produced by an unrestricted caller with no identity: -// any unrestricted reader (anonymous /mcp included) may page them, no agent -// or user may. +// Spec 105 FR-002 (task T031 inversion): they are legacy provenance — refused +// for EVERY caller kind, administrators and the anonymous /mcp caller +// included, and invalidated by the first refused redemption. Before this +// feature they were treated as produced by an unrestricted caller with no +// identity and any unrestricted reader could page them; the full matrix and +// the durability proof live in manager_legacy_test.go. func TestGetRecordsAs_LegacyEntryWithoutProducer(t *testing.T) { db := setupTestDB(t) defer db.Close() @@ -99,27 +198,45 @@ func TestGetRecordsAs_LegacyEntryWithoutProducer(t *testing.T) { } defer m.Close() - if err := m.Store("legacy", "github:list", nil, `{"items":[1,2,3]}`, "items", 3); err != nil { - t.Fatal(err) + seed := func() { + t.Helper() + if err := m.Store("legacy", "github:list", nil, `{"items":[1,2,3]}`, "items", 3); err != nil { + t.Fatal(err) + } } + seed() agent := Authorization{CallerKind: CallerKindAgent, AllowedServers: []string{"*"}, Permissions: []string{"read"}} if _, err := m.GetRecordsAs("legacy", 0, 10, agent); !errors.Is(err, ErrUnauthorizedRead) { t.Fatalf("agent reading a legacy entry: got %v, want ErrUnauthorizedRead", err) } - if _, err := m.GetRecordsAs("legacy", 0, 10, Authorization{CallerKind: CallerKindAdmin}); err != nil { - t.Fatalf("admin reading a legacy entry: %v", err) + if _, ok := m.Peek("legacy"); ok { + t.Fatal("the refused redemption must invalidate the legacy entry") + } + seed() + if _, err := m.GetRecordsAs("legacy", 0, 10, Authorization{CallerKind: CallerKindAdmin}); !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("admin reading a legacy entry: got %v, want ErrUnauthorizedRead (FR-002: every caller)", err) } - if _, err := m.GetRecordsAs("legacy", 0, 10, Authorization{CallerKind: CallerKindAnonymous}); err != nil { - t.Fatalf("anonymous reading a legacy entry: %v", err) + seed() + if _, err := m.GetRecordsAs("legacy", 0, 10, Authorization{CallerKind: CallerKindAnonymous}); !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("anonymous reading a legacy entry: got %v, want ErrUnauthorizedRead (FR-002: every caller)", err) } + seed() user := Authorization{CallerKind: CallerKindUser, Principal: "u1"} if _, err := m.GetRecordsAs("legacy", 0, 10, user); !errors.Is(err, ErrUnauthorizedRead) { t.Fatalf("user reading a legacy entry: got %v, want ErrUnauthorizedRead", err) } + // Once invalidated the key is a plain miss for every caller. + if _, err := m.GetRecordsAs("legacy", 0, 10, Authorization{CallerKind: CallerKindAdmin}); !errors.Is(err, ErrKeyNotFound) { + t.Fatalf("after invalidation: got %v, want ErrKeyNotFound", err) + } } // A refused read must not count as a hit or bump the entry's access stats — -// otherwise the refusal is visible as "someone read this" in the stats. +// otherwise the refusal is visible as "someone read this" in the stats. It +// DOES count as a miss (critique round 1, finding 1 — inverted from "stats +// byte-identical"): a miss is the one signal an absent key leaves, and taking +// the same committing branch is what puts the refusal in the miss's timing +// class; a refusal that committed nothing was a ~3000x timing oracle. func TestGetRecordsAs_RefusedReadLeavesStatsUntouched(t *testing.T) { db := setupTestDB(t) defer db.Close() @@ -139,8 +256,12 @@ func TestGetRecordsAs_RefusedReadLeavesStatsUntouched(t *testing.T) { t.Fatalf("got %v, want ErrUnauthorizedRead", err) } after := *m.GetStats() + if after.MissCount != before.MissCount+1 { + t.Fatalf("a refused read must count as exactly one miss: before=%+v after=%+v", before, after) + } + after.MissCount = before.MissCount if before != after { - t.Fatalf("refused read changed stats: before=%+v after=%+v", before, after) + t.Fatalf("refused read changed stats beyond the miss: before=%+v after=%+v", before, after) } rec, ok := m.Peek("k") if !ok || rec.AccessCount != 0 { @@ -184,3 +305,190 @@ func TestGetRecordsAs_SameAuthorizationIsByteIdentical(t *testing.T) { } } } + +// Spec 105 FR-001 (`spec.md:124`, research D5, gap FR001-G6): superset is +// ordered by CALLER KIND FIRST. An administrator reader qualifies for any +// snapshot regardless of its own profile binding — unscoped, narrower, wider, +// empty, or a profile deleted since — where before this feature the profile +// comparison ran first and refused a profile-bound administrator every +// unscoped or wider entry (#1226 R1-F2). An agent reader never qualifies for +// an administrator-produced snapshot, however broad the agent. Between agent +// snapshots nothing changes: the deny-all guard (an empty effective profile +// reads nothing, not even its own deny-all-stamped entry) applies to agent +// readers only, and pin equality, server set and permission set must each +// contain the snapshot's. +// +// The pre-D5 table above (TestAuthorization_CouldHaveProduced) pins the +// profile-first order for administrators; task T031 inverts those cells. +func TestAuthorization_CallerKindFirst(t *testing.T) { + admin := Authorization{CallerKind: CallerKindAdmin} + adminUser := Authorization{CallerKind: CallerKindAdminUser, Principal: "u9"} + adminInProfile := Authorization{CallerKind: CallerKindAdmin, Profile: "research", + ProfileScoped: true, ProfileServers: []string{"github"}} + adminInWiderProfile := Authorization{CallerKind: CallerKindAdmin, Profile: "everything", + ProfileScoped: true, ProfileServers: []string{"github", "weather"}} + adminInEmptyProfile := Authorization{CallerKind: CallerKindAdmin, Profile: "empty", + ProfileScoped: true, ProfileServers: []string{}} + adminInDeletedProfile := Authorization{CallerKind: CallerKindAdmin, Profile: "research", + ProfileScoped: true, ProfileServers: nil} // the URL/session profile vanished: same name, deny-all scope + adminUserInProfile := Authorization{CallerKind: CallerKindAdminUser, Principal: "u9", Profile: "research", + ProfileScoped: true, ProfileServers: []string{"github"}} + broad := Authorization{CallerKind: CallerKindAgent, Principal: "broad", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read", "write"}} + wildcard := Authorization{CallerKind: CallerKindAgent, Principal: "star", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}} + pinnedWildcard := Authorization{CallerKind: CallerKindAgent, Principal: "star-pinned", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}, + ProfilePin: "research", Profile: "research", ProfileScoped: true, ProfileServers: []string{"github"}} + agentInSessionProfile := Authorization{CallerKind: CallerKindAgent, Principal: "broad", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read", "write"}, + Profile: "research", ProfileScoped: true, ProfileServers: []string{"github"}} + agentInEmptyProfile := Authorization{CallerKind: CallerKindAgent, Principal: "star", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}, + Profile: "empty", ProfileScoped: true, ProfileServers: []string{}} + stalePin := pinnedWildcard + stalePin.ProfileServers = []string{} // pinned profile deleted: deny-all, same name + // An agent whose token carries NO server grant. Token creation normalises + // an empty list to ["*"], but a server-edition rotation that narrows the + // grant persists nil (intersectAllowedServers), and every dispatch gate + // (auth.CanAccessServer, serverInScope) treats that as deny-all. + emptyGrant := Authorization{CallerKind: CallerKindAgent, Principal: "empty", + AllowedServers: nil, Permissions: []string{"read"}} + emptyGrantList := emptyGrant + emptyGrantList.Principal, emptyGrantList.AllowedServers = "empty-list", []string{} + emptyGrantWider := Authorization{CallerKind: CallerKindAgent, Principal: "empty-wider", + AllowedServers: nil, Permissions: []string{"read", "write", "destructive"}} + + cases := []struct { + name string + producer Authorization + reader Authorization + want bool + }{ + // Administrator reader qualifies for ANY snapshot, whatever its profile. + {"profile-bound admin reads an unscoped admin entry", admin, adminInProfile, true}, + {"narrower-profile admin reads a wider-profile admin entry", adminInWiderProfile, adminInProfile, true}, + {"wider-profile admin reads a narrower-profile admin entry", adminInProfile, adminInWiderProfile, true}, + {"empty-profile admin reads an unscoped admin entry", admin, adminInEmptyProfile, true}, + {"empty-profile admin reads a profile-bound admin entry", adminInProfile, adminInEmptyProfile, true}, + {"deleted-profile admin reads an unscoped admin entry", admin, adminInDeletedProfile, true}, + {"deleted-profile admin reads its own earlier profile-bound entry", adminInProfile, adminInDeletedProfile, true}, + {"empty-profile admin reads its own deny-all-stamped entry", adminInEmptyProfile, adminInEmptyProfile, true}, + {"profile-bound admin reads an unscoped agent entry", broad, adminInProfile, true}, + {"profile-bound admin reads a wildcard agent entry", wildcard, adminInProfile, true}, + {"empty-profile admin reads a pinned agent entry", pinnedWildcard, adminInEmptyProfile, true}, + {"profile-bound admin_user reads an unscoped admin entry", admin, adminUserInProfile, true}, + {"profile-bound admin_user reads an unscoped admin_user entry", adminUser, adminUserInProfile, true}, + {"unscoped admin reads a profile-bound admin entry (unchanged)", adminInProfile, admin, true}, + + // Agent reader never qualifies for an administrator snapshot. + {"wildcard full-permission agent cannot read an unscoped admin entry", admin, wildcard, false}, + {"wildcard agent cannot read a profile-bound admin entry", adminInProfile, wildcard, false}, + {"pinned wildcard agent cannot read an admin entry bound to the same profile", adminInProfile, pinnedWildcard, false}, + {"wildcard agent cannot read an admin_user entry", adminUser, wildcard, false}, + {"wildcard agent cannot read an empty-profile admin entry", adminInEmptyProfile, wildcard, false}, + + // Between agent snapshots the dimension checks are unchanged. + {"pinned wildcard agent cannot read an unpinned agent entry", broad, pinnedWildcard, false}, + {"pinned wildcard agent cannot read an unpinned wildcard entry", wildcard, pinnedWildcard, false}, + {"session-profiled agent cannot read an unscoped agent entry", broad, agentInSessionProfile, false}, + {"unscoped bounded agent is refused its own session-profiled entry (D16: the digest differs, the grant is not unrestricted)", agentInSessionProfile, broad, false}, + {"wildcard agent reads a narrower agent entry", broad, wildcard, true}, + + // Deny-all guard applies to AGENT readers only. + {"empty-profile agent reads nothing: unscoped agent entry", broad, agentInEmptyProfile, false}, + {"empty-profile agent reads nothing: its own deny-all-stamped entry", agentInEmptyProfile, agentInEmptyProfile, false}, + {"stale pin reads nothing: its own earlier entry", pinnedWildcard, stalePin, false}, + {"stale pin reads nothing: its own deny-all-stamped entry", stalePin, stalePin, false}, + + // Codex round 1: an empty server grant is deny-all everywhere else + // (CanAccessServer, serverInScope), so an empty-grant agent could not + // have produced ANY entry — coversServers([], []) must not let it + // redeem an identically-stamped one. + {"empty-grant agent reads nothing: its own identically-stamped entry (nil)", emptyGrant, emptyGrant, false}, + {"empty-grant agent reads nothing: its own identically-stamped entry ([])", emptyGrantList, emptyGrantList, false}, + {"empty-grant agent reads nothing: nil vs [] are the same deny-all", emptyGrantList, emptyGrant, false}, + {"empty-grant agent reads nothing: another empty-grant agent's entry", emptyGrant, emptyGrantWider, false}, + {"empty-grant agent reads nothing: unscoped agent entry", broad, emptyGrant, false}, + {"empty-grant agent reads nothing: admin entry", admin, emptyGrant, false}, + {"admin reads an empty-grant agent's entry (kind first)", emptyGrant, admin, true}, + {"profile-bound admin reads an empty-grant agent's entry (kind first)", emptyGrant, adminInProfile, true}, + + // Codex round 2: the deny-all rule is symmetric. A current-version + // AGENT snapshot with an empty server grant, or bounded to an empty + // effective profile, could not have authorized any tool, so no entry + // legitimately carries it as a PRODUCER — such a stamp is provenance + // the gate does not recognise, and containment against the empty set + // (coversServers(x, []) is vacuously true) must not let a broader + // agent redeem it. Administrator readers qualify for any snapshot + // (FR-001, kind first); the anonymous administrator-shaped kind too. + {"broad agent cannot read an empty-grant (nil) producer snapshot", emptyGrant, broad, false}, + {"broad agent cannot read an empty-grant ([]) producer snapshot", emptyGrantList, broad, false}, + {"wildcard agent cannot read an empty-grant producer snapshot", emptyGrant, wildcard, false}, + {"unscoped agent cannot read an empty-profile producer snapshot", agentInEmptyProfile, wildcard, false}, + {"unscoped agent cannot read a stale-pin producer snapshot", stalePin, wildcard, false}, + {"scoped agent cannot read an empty-profile producer snapshot", agentInEmptyProfile, pinnedWildcard, false}, + {"pinned agent cannot read a stale-pin producer snapshot", stalePin, pinnedWildcard, false}, + {"admin reads an empty-profile agent's entry (kind first)", agentInEmptyProfile, admin, true}, + {"admin reads a stale-pin agent's entry (kind first)", stalePin, admin, true}, + {"anonymous reads an empty-grant agent's entry (kind first)", emptyGrant, Authorization{CallerKind: CallerKindAnonymous}, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.producer.CouldHaveProduced(tc.reader); got != tc.want { + t.Fatalf("producer=%+v reader=%+v: got %v want %v", tc.producer, tc.reader, got, tc.want) + } + }) + } +} + +// userContainmentCases: a server-edition user is allowlist-scoped at every +// dispatch gate exactly like an agent token (auth.CanAccessServer, +// HasPermission and the effective profile bound AuthTypeUser too), so the +// read gate bounds a user snapshot the same way. Identity equality is +// NECESSARY, never sufficient: a user allowed only {a} produces an `a` +// entry, is later narrowed to {b}, or enters a disjoint or deny-all profile, +// and must not redeem the old entry on the strength of the user id alone +// (codex round 4, finding 1). +func userContainmentCases(alice, bob Authorization) []authorizationCase { + narrowed := alice + narrowed.AllowedServers = []string{"weather"} + reassigned := alice + reassigned.AllowedServers = []string{"deploy"} + readOnly := alice + readOnly.Permissions = []string{"read"} + wider := alice + wider.AllowedServers = []string{"*"} + wider.Permissions = []string{"read", "write", "destructive"} + inProfile := alice + inProfile.Profile, inProfile.ProfileScoped, inProfile.ProfileServers = "research", true, []string{"github"} + inDisjointProfile := alice + inDisjointProfile.Profile, inDisjointProfile.ProfileScoped, inDisjointProfile.ProfileServers = "deploy", true, []string{"weather"} + inDenyAllProfile := alice + inDenyAllProfile.Profile, inDenyAllProfile.ProfileScoped, inDenyAllProfile.ProfileServers = "research", true, []string{} + // The production shape of a plain OAuth user context (auth.UserContext): + // no server grant at all — deny-all on every dispatch gate. + noGrant := Authorization{CallerKind: CallerKindUser, Principal: alice.Principal} + bobWider := bob + bobWider.AllowedServers = []string{"*"} + bobWider.Permissions = []string{"read", "write", "destructive"} + return []authorizationCase{ + {"user: same id, grant narrowed since", alice, narrowed, false}, + {"user: same id, grant reassigned to a disjoint server", alice, reassigned, false}, + {"user: same id, permission tier dropped", alice, readOnly, false}, + // Research D16: a user is admitted on digest equality only — the + // header cannot carry the identity an unrestricted-user rule would + // need, so a wider grant for the same user is refused, fail-closed. + {"user: same id, wider grant is refused (D16: digest-equal only)", alice, wider, false}, + {"user: same id, own narrower entry with the wider grant is refused (D16)", narrowed, alice, false}, + {"user: same id, now bound to a profile that does not cover the entry", alice, inProfile, false}, + {"user: same id, profile entry read unscoped is refused (D16: wider, not digest-equal)", inProfile, alice, false}, + {"user: same id, disjoint profile", inProfile, inDisjointProfile, false}, + {"user: same id, deny-all profile (deleted since)", alice, inDenyAllProfile, false}, + {"user: same id, no grant (production user context) reads nothing", alice, noGrant, false}, + {"user: no-grant producer snapshot is deny-all, its own id included", noGrant, noGrant, false}, + {"user: no-grant producer snapshot, wider same user still refused", noGrant, wider, false}, + {"user: no-grant producer snapshot, administrator reads (kind first)", noGrant, Authorization{CallerKind: CallerKindAdmin}, true}, + {"user: other id with a wider grant is still another identity", alice, bobWider, false}, + } +} diff --git a/internal/cache/manager.go b/internal/cache/manager.go index 90a43ba02..64523df31 100644 --- a/internal/cache/manager.go +++ b/internal/cache/manager.go @@ -4,9 +4,11 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "strconv" "strings" + "sync" "sync/atomic" "time" @@ -17,8 +19,29 @@ import ( const ( CacheBucket = "cache" CacheStatsBucket = "cache_stats" - DefaultTTL = 2 * time.Hour - CleanupInterval = 10 * time.Minute + // CacheSnapshotBucket holds each distinct producer authorization snapshot + // ONCE, keyed by the digest its records' frame headers carry, for + // ADMINISTRATOR DIAGNOSTICS only: no read path consults it — the gate + // decides on the header alone (research D16). Written in the same + // transaction as the record that first references it; unreferenced + // snapshots are dropped by the cleanup sweep. + CacheSnapshotBucket = "cache_snapshots" + DefaultTTL = 2 * time.Hour + CleanupInterval = 10 * time.Minute +) + +// Read outcomes a caller can act on with errors.Is. The messages are part of +// the agent-facing contract: the read_cache banner and its consumers key on +// "cache key not found", and the non-disclosing refusal for scoped callers +// (Spec 105 FR-001) reuses ErrKeyNotFound verbatim. +var ( + // ErrKeyNotFound: no entry under the key (or nothing left after an + // invalidation). + ErrKeyNotFound = errors.New("cache key not found") + // ErrKeyExpired: the entry had passed its TTL. The ungated Get evicts it; + // the gated read refuses it like a miss and leaves it to the cleanup + // sweep (see getGuarded). + ErrKeyExpired = errors.New("cache key expired") ) // Manager handles cached tool responses @@ -27,15 +50,24 @@ type Manager struct { logger *zap.Logger stats *Stats stopCh chan struct{} + // writeMu serialises update: the in-memory stats snapshot it takes must + // be the state the transaction started from, and bbolt's own writer lock + // is acquired inside db.Update, after that snapshot. + writeMu sync.Mutex + // dbUpdate runs a write transaction; db.Update in production. It is a + // seam so a test can make the COMMIT fail after the closure succeeded + // (disk full at fsync), a fault no in-process bbolt setup produces. + dbUpdate func(fn func(tx *bbolt.Tx) error) error } // NewManager creates a new cache manager func NewManager(db *bbolt.DB, logger *zap.Logger) (*Manager, error) { manager := &Manager{ - db: db, - logger: logger, - stats: &Stats{}, - stopCh: make(chan struct{}), + db: db, + logger: logger, + stats: &Stats{}, + stopCh: make(chan struct{}), + dbUpdate: db.Update, } // Initialize buckets @@ -46,6 +78,9 @@ func NewManager(db *bbolt.DB, logger *zap.Logger) (*Manager, error) { if _, err := tx.CreateBucketIfNotExists([]byte(CacheStatsBucket)); err != nil { return fmt.Errorf("create cache stats bucket: %w", err) } + if _, err := tx.CreateBucketIfNotExists([]byte(CacheSnapshotBucket)); err != nil { + return fmt.Errorf("create cache snapshots bucket: %w", err) + } return nil }) if err != nil { @@ -106,14 +141,17 @@ func NextUniqueTimestamp() time.Time { var lastKeyNano atomic.Int64 // Store saves a tool response to cache with no producer authorization. Such an -// entry is readable only by unrestricted callers; production callers stamp the -// producer via StoreAs. +// entry has legacy provenance (Spec 105 FR-002): the gated read refuses it for +// every caller and invalidates it. It exists for the ungated readers and for +// tests that seed pre-feature records; production callers stamp the producer +// via StoreAs. func (m *Manager) Store(key, toolName string, args map[string]interface{}, content, recordPath string, totalRecords int) error { return m.storeRecord(key, toolName, args, content, recordPath, totalRecords, nil) } // StoreAs saves a tool response to cache stamped with the authorization it was -// produced under. GetRecordsAs refuses readers that could not have produced it. +// produced under and the current RecordVersion. GetRecordsAs refuses readers +// that could not have produced it. func (m *Manager) StoreAs(key, toolName string, args map[string]interface{}, content, recordPath string, totalRecords int, producer Authorization) error { return m.storeRecord(key, toolName, args, content, recordPath, totalRecords, &producer) } @@ -121,6 +159,7 @@ func (m *Manager) StoreAs(key, toolName string, args map[string]interface{}, con func (m *Manager) storeRecord(key, toolName string, args map[string]interface{}, content, recordPath string, totalRecords int, producer *Authorization) error { record := &Record{ Producer: producer, + Version: RecordVersion, Key: key, ToolName: toolName, Args: args, @@ -135,12 +174,20 @@ func (m *Manager) storeRecord(key, toolName string, args map[string]interface{}, CreatedAt: time.Now(), } - return m.db.Update(func(tx *bbolt.Tx) error { + return m.update(func(tx *bbolt.Tx) error { bucket := tx.Bucket([]byte(CacheBucket)) - data, err := record.MarshalBinary() + data, snapshot, err := record.marshalFrame() if err != nil { return fmt.Errorf("marshal cache record: %w", err) } + // The diagnostic snapshot is persisted in THIS transaction, once per + // distinct producer digest, so the snapshots bucket never describes + // an entry the cache bucket does not hold. + if producer != nil { + if err := m.putSnapshot(tx, producer.digest(), snapshot); err != nil { + return err + } + } if err := bucket.Put([]byte(key), data); err != nil { return fmt.Errorf("store cache record: %w", err) @@ -154,46 +201,153 @@ func (m *Manager) storeRecord(key, toolName string, args map[string]interface{}, }) } -// Get retrieves a cached tool response +// Get retrieves a cached tool response without a read gate. It is the read +// path of the proxy's own writers (the repository guesser); credentialed +// requests go through GetRecordsAs. func (m *Manager) Get(key string) (*Record, error) { return m.getGuarded(key, nil) } -// getGuarded is Get with an optional read gate. The gate runs after the -// expiry check and BEFORE the access-stats update, so a refused read neither -// counts as a hit nor marks the entry as accessed. -func (m *Manager) getGuarded(key string, guard func(*Record) error) (*Record, error) { - var record *Record - - err := m.db.Update(func(tx *bbolt.Tx) error { +// getGuarded is Get with an optional read gate. A non-nil guard marks the +// GATED door (read_cache). On that door EVERY verdict is decided on the +// record's FIXED-SIZE FRAME HEADER (decodeRecordHeader: version, caller kind, +// deny-all bit, tier bits, expiry, size, producer digest — 52 bytes) and +// never on the payload nor on the producer snapshot: a refusal that decoded +// a multi-megabyte FullContent, or a snapshot naming thousands of servers, +// first would take a timing class a nonexistent key does not, and the spec's +// non-disclosing refusal is indistinguishable in status, body AND timing +// class (Spec 105 Definitions; codex rounds 2, 4 and 5; research D16). The +// order is: provenance class first (Spec 105 FR-002) — a value with no +// frame, a frame this binary cannot decode, or a header with legacy or +// unrecognised provenance is refused for every caller and invalidated; then +// an internal entry is refused for every caller WITHOUT eviction, even when +// it has expired (its writers' ungated readers serve expired entries as +// stale until cleanup, and a guessable key must not let a probe evict them +// early); then an expired entry is refused like a miss and left for the +// cleanup sweep; then the guard runs on the header's facts (admits). Only an +// admitted read decodes the record — and only then are the access stats +// updated, so a refused read never counts as a hit or marks the entry as +// accessed. +// +// An admitted read whose body then proves undecodable, or disagrees with the +// header it was admitted on, is invalidated too — but reported as +// ErrEntryUnreadable, an admitted-class outcome, never as a refusal: the +// refusal shape is decided on the header only (codex round 6). +// +// Every refusal COMMITS, as a miss. A refusal that returned its error from the +// Update closure made bbolt roll the transaction back without a disk write, +// while a miss committed a stats write: ~5 µs against ~10 ms, a timing class +// a single probe could read as "a live entry sits behind this key". So the +// guard verdict, like every other outcome, is handed out through `verdict` +// after a committed stats write; the closure returns an error only for a +// storage fault, and m.update then restores the in-memory stats to the +// rolled-back state. The invalidating outcomes (the legacy-provenance +// refusal, the admitted unreadable body) additionally delete the key — +// FR-002 requires the legacy entry durably invalidated by the refusal +// itself, not by a later sweep — and that delete is bounded: bbolt rewrites +// the leaf minus the entry and frees the value's pages by id range, never +// reading the payload (pinned by +// TestGetRecordsAs_EvictingRefusalWritesArePayloadIndependent). It is also +// one-shot per key: the entry is gone, so the second probe is a plain miss. +func (m *Manager) getGuarded(key string, guard func(header recordHeader) error) (*Record, error) { + var ( + record *Record + verdict error + ) + + err := m.update(func(tx *bbolt.Tx) error { bucket := tx.Bucket([]byte(CacheBucket)) + // bbolt hands back a view into its page memory: no copy, whatever + // the value's size. data := bucket.Get([]byte(key)) if data == nil { - m.stats.MissCount++ - _ = m.saveStats(tx) - return fmt.Errorf("cache key not found") + verdict = ErrKeyNotFound + return m.commitMiss(tx) } + var header recordHeader + if guard != nil { + var err error + header, err = decodeRecordHeader(data) + if err != nil || !header.HasCurrentProvenance() { + // Legacy or unrecognised provenance: refuse every caller and + // invalidate on this first redemption, committed (FR-002), + // WITHOUT decoding the value. The size folded out of the + // stats is the header's TotalSize — what the store folded + // in — and 0 for a pre-frame bare-JSON value or a corrupt + // frame, whose size is unknown without a payload-sized + // decode the refusal must not do (codex round 5). Until + // the next cleanup sweep TotalSizeBytes then over-counts + // by that entry's payload; the sweep recomputes + // TotalEntries and TotalSizeBytes exactly from the bucket + // it walks anyway, so the statistics are eventually + // consistent while the refusal stays O(1) (research D16). + m.logger.Info("Invalidated cache entry with legacy provenance on first redemption", + zap.String("key", key), + zap.Uint8("version", header.Version), + zap.Bool("has_producer", header.KindCode != 0), + zap.String("caller_kind", header.Kind), + zap.NamedError("frame", err)) + verdict = ErrLegacyProvenance + return m.evict(tx, bucket, key, header.TotalSize, "invalidate legacy cache record") + } + // Internal entry: refused for every caller, kept — expired or not. + if header.Kind == CallerKindInternal { + verdict = ErrInternalEntry + return m.commitMiss(tx) + } + // Expired: refused exactly as a miss and LEFT for the cleanup + // sweep (CleanupInterval), which evicts expired entries anyway. + // Deleting here would rewrite the entry's leaf — work a miss + // never does, and proportional to whatever the leaf's other + // values hold — for no gain: nothing requires the gated door + // to evict, and the ungated Get keeps doing so for its own + // readers. + if header.expired() { + verdict = ErrKeyExpired + return m.commitMiss(tx) + } + // The guard sees the header and nothing else: kind, deny-all + // bit, tier bits and the producer digest decide (admits). + if err := guard(header); err != nil { + verdict = err + return m.commitMiss(tx) + } + } + + // Admitted (or the ungated door): the payload is decoded from here on. record = &Record{} if err := record.UnmarshalBinary(data); err != nil { - return fmt.Errorf("unmarshal cache record: %w", err) + record = nil + if guard == nil { + return fmt.Errorf("unmarshal cache record: %w", err) + } + // A frame the gate admitted around a body this binary cannot + // decode — or one that DISAGREES with the header the gate + // admitted on (UnmarshalBinary checks the two agree exactly): + // invalidate, the way cleanup drops undecodable records, and + // never return the body. The reader was ADMITTED — entitled to + // the entry — so the decode it paid for is not a refusal + // oracle, and its outcome is not a refusal either: the refusal + // shape is decided on the fixed header only, and an admitted + // reader gets the admitted-class ErrEntryUnreadable, which the + // handler renders distinctly for every caller kind (codex round + // 6). The size folded out is the header's, the one the stats + // were told at store time. + m.logger.Info("Invalidated unreadable cache entry on gated read", + zap.String("key", key), + zap.Error(err)) + verdict = ErrEntryUnreadable + return m.evict(tx, bucket, key, header.TotalSize, "invalidate unreadable cache record") } - // Check if expired + // Expired on the ungated door (the gated door already refused it on + // the header): evict in this transaction and COMMIT the eviction. if record.IsExpired() { - _ = bucket.Delete([]byte(key)) - m.stats.EvictedCount++ - m.stats.TotalEntries-- - m.stats.TotalSizeBytes -= record.TotalSize - _ = m.saveStats(tx) - return fmt.Errorf("cache key expired") - } - - if guard != nil { - if err := guard(record); err != nil { - record = nil - return err - } + size := record.TotalSize + record = nil + verdict = ErrKeyExpired + return m.evict(tx, bucket, key, size, "evict expired cache record") } // Update access stats @@ -212,8 +366,74 @@ func (m *Manager) getGuarded(key string, guard func(*Record) error) (*Record, er m.stats.HitCount++ return m.saveStats(tx) }) + if err != nil { + return nil, err + } + if verdict != nil { + return nil, verdict + } + return record, nil +} - return record, err +// update runs fn inside a bbolt write transaction. The in-memory stats are +// mutated inside fn but only STAY mutated once the transaction has committed: +// whether fn returned an error or the commit itself failed afterwards (page +// write, file grow, fsync — disk full), bbolt rolled the transaction back, and +// the counters are restored to what they were when it began. So the counters +// never record a mutation the bucket did not commit, and GetStats agrees with +// the bucket on every path, not only the happy one. writeMu serialises the +// snapshot with the transaction: it is taken before bbolt's writer lock is +// acquired, so without it a concurrent writer's committed delta could be +// snapshotted away by this one's restore. +func (m *Manager) update(fn func(tx *bbolt.Tx) error) error { + m.writeMu.Lock() + defer m.writeMu.Unlock() + prev := *m.stats + if err := m.dbUpdate(fn); err != nil { + *m.stats = prev + return err + } + return nil +} + +// commitMiss records a miss and persists the stats — the one committing +// branch every refusal shares with an absent key. +func (m *Manager) commitMiss(tx *bbolt.Tx) error { + m.stats.MissCount++ + return m.saveStats(tx) +} + +// evict deletes key inside tx, folds the eviction into the stats and persists +// them. size is what the store folded in for the entry — the header's +// TotalSize — or 0 when the value has no decodable header (a pre-frame or +// corrupt record): never an estimate read from the payload. The cleanup +// sweep reconciles TotalSizeBytes from the bucket. what names the operation +// in the storage error. +func (m *Manager) evict(tx *bbolt.Tx, bucket *bbolt.Bucket, key string, size int, what string) error { + if err := bucket.Delete([]byte(key)); err != nil { + return fmt.Errorf("%s: %w", what, err) + } + m.stats.EvictedCount++ + m.stats.TotalEntries-- + m.stats.TotalSizeBytes -= size + return m.saveStats(tx) +} + +// putSnapshot persists the diagnostic snapshot under the producer's digest +// unless the bucket already holds one (first writer wins; every snapshot +// under a digest is the same effective authorization). +func (m *Manager) putSnapshot(tx *bbolt.Tx, digest [sha256.Size]byte, snapshot []byte) error { + bucket := tx.Bucket([]byte(CacheSnapshotBucket)) + if bucket == nil { + return fmt.Errorf("cache snapshots bucket %q is missing", CacheSnapshotBucket) + } + if bucket.Get(digest[:]) != nil { + return nil + } + if err := bucket.Put(digest[:], snapshot); err != nil { + return fmt.Errorf("store producer snapshot: %w", err) + } + return nil } // GetRecords retrieves paginated records from a cached response without a @@ -224,24 +444,43 @@ func (m *Manager) GetRecords(key string, offset, limit int) (*ReadCacheResponse, // GetRecordsAs retrieves paginated records from a cached response, refusing // with ErrUnauthorizedRead when reader could not have produced the entry -// (Spec 104 FR-016a). The gate runs on every page. An entry with no recorded -// producer (persisted before stamping existed, or written through Store by an -// internal caller) is treated as produced by an unrestricted caller with no -// identity — readable by any unrestricted kind, never by an agent or user. +// (Spec 104 FR-016a). The gate runs on every page. +// +// Two classes of entry are refused for EVERY caller kind, administrators +// included (Spec 105 FR-002): +// - legacy provenance (no producer, no version, or a version this binary +// does not recognise — every entry persisted before stamping existed): +// refused with ErrLegacyProvenance and durably invalidated by the refusal; +// - internal entries (CallerKindInternal — the registry and guesser caches): +// refused with ErrInternalEntry WITHOUT eviction, since their keys are +// guessable and their writers' ungated readers depend on them. +// +// Every refusal is decided on the record's fixed header. A reader the header +// admits whose entry then proves unreadable (undecodable body, or one that +// disagrees with the header) gets ErrEntryUnreadable — not an +// ErrUnauthorizedRead — and the entry is invalidated. func (m *Manager) GetRecordsAs(key string, offset, limit int, reader Authorization) (*ReadCacheResponse, error) { - return m.getRecords(key, offset, limit, func(r *Record) error { - producer := Authorization{CallerKind: CallerKindAnonymous} - if r.Producer != nil { - producer = *r.Producer - } - if !producer.CouldHaveProduced(reader) { + // The reader's side of the verdict is computed ONCE, here, before the + // transaction: its digest, deny-all and tier bits are a function of the + // caller's own credential, and computing them outside the door means a + // miss and a refusal do the same work (research D16). + r := newReaderFacts(reader) + return m.getRecords(key, offset, limit, func(header recordHeader) error { + // getGuarded has already refused legacy provenance (invalidated) + // and internal entries (kept), so the producer is of a request + // kind here. The verdict is admits over the header's facts — kind + // first (FR-001), the deny-all bits, digest equality, then the + // unrestricted-agent rule — in O(1), with no snapshot loaded and + // no payload seen. CouldHaveProduced is the same function over the + // same facts, so the door and the predicate cannot disagree. + if !admits(header.facts(), r) { return ErrUnauthorizedRead } return nil }) } -func (m *Manager) getRecords(key string, offset, limit int, guard func(*Record) error) (*ReadCacheResponse, error) { +func (m *Manager) getRecords(key string, offset, limit int, guard func(header recordHeader) error) (*ReadCacheResponse, error) { record, err := m.getGuarded(key, guard) if err != nil { return nil, err @@ -278,7 +517,8 @@ func (m *Manager) getRecords(key string, offset, limit int, guard func(*Record) } response := &ReadCacheResponse{ - Records: paginatedRecords, + Records: paginatedRecords, + Producer: record.Producer, Meta: Meta{ Key: key, TotalRecords: totalRecords, @@ -301,7 +541,7 @@ func (m *Manager) GetStats() *Stats { // It is a no-op (nil error) if the key is absent. Used by the registry refresh // path (FR-007) to drop cached server lists on demand. func (m *Manager) Invalidate(key string) error { - return m.db.Update(func(tx *bbolt.Tx) error { + return m.update(func(tx *bbolt.Tx) error { bucket := tx.Bucket([]byte(CacheBucket)) data := bucket.Get([]byte(key)) if data == nil { @@ -333,7 +573,7 @@ func (m *Manager) Refresh(key string) error { // a registry's cached results regardless of tag/query/limit (FR-007). func (m *Manager) InvalidatePrefix(prefix string) (int, error) { deleted := 0 - err := m.db.Update(func(tx *bbolt.Tx) error { + err := m.update(func(tx *bbolt.Tx) error { bucket := tx.Bucket([]byte(CacheBucket)) cursor := bucket.Cursor() @@ -405,17 +645,28 @@ func (m *Manager) startCleanup() { } } -// cleanup removes expired cache entries +// cleanup removes expired cache entries (and records it cannot decode), prunes +// the snapshots no surviving entry references, and RECOMPUTES TotalEntries +// and TotalSizeBytes from the entries that survive — the sweep walks and +// decodes every record anyway, so the exact figures are free here, and the +// gated read's legacy invalidation, which must not decode the value it +// deletes, leaves the size to this sweep (research D16). Between an +// invalidation and the next sweep the statistics may over-count by that +// entry's payload; they are never wrong for longer than CleanupInterval. func (m *Manager) cleanup() error { now := time.Now() cleanupCount := 0 totalSizeReduced := 0 - err := m.db.Update(func(tx *bbolt.Tx) error { + err := m.update(func(tx *bbolt.Tx) error { bucket := tx.Bucket([]byte(CacheBucket)) cursor := bucket.Cursor() var keysToDelete [][]byte + // Snapshot digests the surviving entries reference; the rest of the + // snapshots bucket is garbage once the expired entries are gone. + referenced := map[[sha256.Size]byte]struct{}{} + survivors, survivingSize := 0, 0 for key, value := cursor.First(); key != nil; key, value = cursor.Next() { var record Record @@ -430,6 +681,12 @@ func (m *Manager) cleanup() error { keysToDelete = append(keysToDelete, key) cleanupCount++ totalSizeReduced += record.TotalSize + continue + } + survivors++ + survivingSize += record.TotalSize + if header, err := decodeRecordHeader(value); err == nil && header.KindCode != 0 { + referenced[header.Digest] = struct{}{} } } @@ -440,10 +697,16 @@ func (m *Manager) cleanup() error { } } - // Update stats + if err := m.pruneSnapshots(tx, referenced); err != nil { + return err + } + + // Update stats: the counters are set from the bucket, not + // decremented, so any drift a header-only eviction left behind is + // reconciled here. m.stats.CleanupCount += cleanupCount - m.stats.TotalEntries -= cleanupCount - m.stats.TotalSizeBytes -= totalSizeReduced + m.stats.TotalEntries = survivors + m.stats.TotalSizeBytes = survivingSize return m.saveStats(tx) }) @@ -461,6 +724,34 @@ func (m *Manager) cleanup() error { return nil } +// pruneSnapshots deletes every snapshot no surviving entry references; a +// later entry under the same digest re-persists it. +func (m *Manager) pruneSnapshots(tx *bbolt.Tx, referenced map[[sha256.Size]byte]struct{}) error { + bucket := tx.Bucket([]byte(CacheSnapshotBucket)) + if bucket == nil { + return nil + } + var stale [][]byte + cursor := bucket.Cursor() + for key, _ := cursor.First(); key != nil; key, _ = cursor.Next() { + var hash [sha256.Size]byte + if len(key) != sha256.Size { + stale = append(stale, append([]byte(nil), key...)) + continue + } + copy(hash[:], key) + if _, ok := referenced[hash]; !ok { + stale = append(stale, append([]byte(nil), key...)) + } + } + for _, key := range stale { + if err := bucket.Delete(key); err != nil { + return fmt.Errorf("prune producer snapshot: %w", err) + } + } + return nil +} + // loadStats loads cache statistics from database func (m *Manager) loadStats() error { return m.db.View(func(tx *bbolt.Tx) error { @@ -477,6 +768,9 @@ func (m *Manager) loadStats() error { // saveStats saves cache statistics to database func (m *Manager) saveStats(tx *bbolt.Tx) error { bucket := tx.Bucket([]byte(CacheStatsBucket)) + if bucket == nil { + return fmt.Errorf("cache stats bucket %q is missing", CacheStatsBucket) + } data, err := m.stats.MarshalBinary() if err != nil { return fmt.Errorf("marshal stats: %w", err) diff --git a/internal/cache/manager_frame_integrity_test.go b/internal/cache/manager_frame_integrity_test.go new file mode 100644 index 000000000..af6bc908f --- /dev/null +++ b/internal/cache/manager_frame_integrity_test.go @@ -0,0 +1,563 @@ +package cache + +import ( + "encoding/json" + "errors" + "fmt" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "go.etcd.io/bbolt" +) + +// Codex round 3. The gated read decides admission on the frame HEADER and +// then decodes the BODY — two encodings of the same record that a binary this +// repository ships never lets disagree (MarshalBinary derives one from the +// other). A value that does disagree was not written by such a binary: it is +// corrupt or crafted, and must be treated like any other undecodable frame — +// invalidated, and never a source of content. In particular the header must +// not admit a reader to a body stamped under a BROADER authorization +// (finding 1: an `a`-only header in front of an administrator body handed +// the body to an `a`-only agent), and a future-expiry header must not admit +// a reader to an expired body (the admitted decode then evicted the entry on +// the gated door, work the header verdict never does). Every reader here was +// ADMITTED on the header, so the outcome is the admitted-class +// ErrEntryUnreadable, not a refusal (codex round 6): the refusal shape is +// decided on the header only. +func TestGetRecordsAs_FrameHeaderBodyDisagreementIsUnreadable(t *testing.T) { + now := time.Now().Round(0) + aOnly := &Authorization{CallerKind: CallerKindAgent, Principal: "narrow", AllowedServers: []string{"a"}, Permissions: []string{"read"}} + broad := &Authorization{CallerKind: CallerKindAgent, Principal: "broad", AllowedServers: []string{"a", "b"}, Permissions: []string{"read", "write"}} + admin := &Authorization{CallerKind: CallerKindAdmin} + const content = `[{"name":"SENTINEL-DISAGREE"}]` + + record := func(mutate func(r *Record)) *Record { + r := &Record{Key: "k", ToolName: "t", FullContent: content, TotalSize: len(content), + Timestamp: now, ExpiresAt: now.Add(time.Hour), CreatedAt: now, LastAccessed: now, + Version: RecordVersion, Producer: aOnly} + if mutate != nil { + mutate(r) + } + return r + } + encode := func(t *testing.T, v interface{}) []byte { + t.Helper() + data, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + return data + } + const seedContent = `[{"name":"SEED"}]` + + fixtures := []struct { + name string + header recordHeader + body *Record + reader Authorization + }{ + {name: "a-only header, administrator body, a-only reader", + header: record(nil).header(), body: record(func(r *Record) { r.Producer = admin }), reader: *aOnly}, + {name: "a-only header, broader agent body, a-only reader", + header: record(nil).header(), body: record(func(r *Record) { r.Producer = broad }), reader: *aOnly}, + {name: "a-only header, legacy (unstamped) body, a-only reader", + header: record(nil).header(), body: record(func(r *Record) { r.Producer = nil; r.Version = 0 }), reader: *aOnly}, + {name: "future-expiry header, expired body, administrator reader", + header: record(nil).header(), body: record(func(r *Record) { r.ExpiresAt = now.Add(-time.Hour) }), reader: *admin}, + {name: "header size differs from body size, administrator reader", + header: record(nil).header(), body: record(func(r *Record) { r.TotalSize = len(content) + 1 }), reader: *admin}, + {name: "current-version header, unversioned body, administrator reader", + header: record(nil).header(), body: record(func(r *Record) { r.Version = 0 }), reader: *admin}, + } + for _, fx := range fixtures { + t.Run(fx.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + const key = "disagree" + // A genuine a-only entry beside the crafted one: it persists the + // a-only snapshot the crafted header references, so what refuses + // the crafted value is the header/body disagreement — not a + // snapshot the bucket lacks — and it is the live neighbour whose + // accounting the invalidation must leave exact. + if err := m.StoreAs("seed", "t", nil, seedContent, "", 1, *aOnly); err != nil { + t.Fatal(err) + } + putFramedRecord(t, db, key, fx.header.encode(), encode(t, fx.body)) + // Fold the entry into the stats the way a Store would have, so + // the invalidation's accounting is observable. + if err := m.update(func(tx *bbolt.Tx) error { + m.stats.TotalEntries++ + m.stats.TotalSizeBytes += fx.header.TotalSize + return m.saveStats(tx) + }); err != nil { + t.Fatal(err) + } + + resp, err := m.GetRecordsAs(key, 0, 10, fx.reader) + if !errors.Is(err, ErrEntryUnreadable) || errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("got err=%v resp=%v, want ErrEntryUnreadable (admitted on the header, unreadable behind it)", err, resp) + } + if resp != nil { + t.Fatalf("disagreeing frame returned content: %+v", resp) + } + if _, ok := m.Peek(key); ok { + t.Fatal("disagreeing frame still present after the refused redemption") + } + if got := m.GetStats(); got.TotalEntries != 1 || got.TotalSizeBytes != len(seedContent) || got.EvictedCount != 1 { + t.Fatalf("stats after invalidation = %+v, want the crafted entry and its header size folded out and the seed (%d bytes) untouched", *got, len(seedContent)) + } + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m2, db2 := openManagerAt(t, path) + defer db2.Close() + defer m2.Close() + if got := onDiskEntryCount(t, db2); got != 1 { + t.Fatalf("on-disk count after restart = %d, want 1 (the seed)", got) + } + }) + } + + // Control: a frame MarshalBinary wrote round-trips through every door. + t.Run("control: self-written frame agrees", func(t *testing.T) { + m, db := openManagerAt(t, filepath.Join(t.TempDir(), "cache.db")) + defer db.Close() + defer m.Close() + if err := m.StoreAs("k", "t", nil, content, "", 1, *aOnly); err != nil { + t.Fatal(err) + } + if _, err := m.GetRecordsAs("k", 0, 10, *aOnly); err != nil { + t.Fatalf("same-authorization redemption: %v", err) + } + }) +} + +// Codex round 4, finding 2 (cache) / finding 1 (server): the round-3 frame +// carried the whole producer snapshot in a JSON header bounded at 1 MiB, so +// (a) a legitimate snapshot over the bound — configuration bounds neither +// the server count nor the name length — could not be cached at all, and +// (b) every live-key refusal JSON-decoded the whole header while a +// nonexistent key decoded nothing: a fleet-sized timing oracle. The frame +// header is now FIXED-SIZE (version, kind, deny-all bit, tier bits, expiry, +// size, producer DIGEST) and the snapshot is kept once, under its digest, in +// the snapshots bucket for diagnostics. This pins (a): a snapshot naming +// 5,000 servers in both its grant and its profile round-trips, is redeemed +// by its producer (FR-001: same authorization, same entry) — through the +// storing manager and after a restart, where nothing but the header on disk +// can decide (research D16) — by a reader presenting the same authorization +// with its lists in another order (the digest is canonical) and by an +// unrestricted agent covering its tier (the header's bits), is stored once +// across entries, and is refused to a narrower agent on a plain scope +// refusal. The timing half is TestGetRecordsAs_RefusalIsSnapshotSizeIndependent. +func TestStoreAs_LargeSnapshotRoundTrips(t *testing.T) { + const content = `[{"name":"SENTINEL-LARGE"}]` + producer := fleetSnapshot(5000, 112) + if n := len(snapshotBytes(producer)); n <= 1<<20 { + t.Fatalf("fixture snapshot is %d bytes; want over the former 1 MiB header bound, which refused it at write time", n) + } + narrow := Authorization{CallerKind: CallerKindAgent, Principal: "narrow", AllowedServers: []string{"srv-000001"}, Permissions: []string{"read"}} + reordered := producer + reordered.AllowedServers = append([]string(nil), producer.AllowedServers...) + reordered.ProfileServers = append([]string(nil), producer.ProfileServers...) + slices.Reverse(reordered.AllowedServers) + slices.Reverse(reordered.ProfileServers) + reordered.Profile = "fleet-renamed" // the name is not part of the digest + // Unrestricted (wildcard grant, no pin, no profile) with the producer's + // one tier: admitted on the header's tier bits, without the snapshot. + star := Authorization{CallerKind: CallerKindAgent, Principal: "star", AllowedServers: []string{"*"}, Permissions: []string{"read"}} + + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + for _, key := range []string{"large-1", "large-2"} { + if err := m.StoreAs(key, "t", nil, content, "", 1, producer); err != nil { + t.Fatalf("store under a %d-byte snapshot: %v", len(snapshotBytes(producer)), err) + } + } + if got := onDiskSnapshotCount(t, db); got != 1 { + t.Fatalf("snapshots on disk = %d, want 1: two entries under one authorization must share one snapshot", got) + } + redeem := func(t *testing.T, m *Manager, key string, reader Authorization) { + t.Helper() + resp, err := m.GetRecordsAs(key, 0, 10, reader) + if err != nil { + t.Fatalf("producer's own redemption refused: %v", err) + } + if len(resp.Records) != 1 { + t.Fatalf("records = %+v, want the one sentinel", resp.Records) + } + if resp.Producer == nil || len(resp.Producer.AllowedServers) != len(producer.AllowedServers) { + t.Fatalf("the admitted page must carry the full producer snapshot for child stamping; got %v", resp.Producer != nil) + } + if _, ok := m.Peek(key); !ok { + t.Fatal("entry deleted by its producer's redemption") + } + } + redeem(t, m, "large-1", producer) + redeem(t, m, "large-1", reordered) + redeem(t, m, "large-1", star) + if _, err := m.GetRecordsAs("large-1", 0, 10, narrow); !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("narrow agent: err = %v, want ErrUnauthorizedRead", err) + } + if _, ok := m.Peek("large-1"); !ok { + t.Fatal("a scope refusal must not evict") + } + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + + // After a restart nothing is in memory: the header on disk decides. + m2, db2 := openManagerAt(t, path) + defer db2.Close() + defer m2.Close() + redeem(t, m2, "large-2", producer) + redeem(t, m2, "large-2", reordered) + redeem(t, m2, "large-2", star) + if _, err := m2.GetRecordsAs("large-2", 0, 10, narrow); !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("narrow agent after restart: err = %v, want ErrUnauthorizedRead", err) + } +} + +// Research D16: the snapshots bucket is administrator diagnostics, not an +// input to the verdict. Deleting a producer's snapshot from it changes +// nothing on the gated door: the producer still redeems (digest-equal on the +// header), an unrestricted agent still redeems, a narrower agent is still +// refused without eviction, and the next store re-persists the snapshot. +func TestGetRecordsAs_SnapshotBucketIsDiagnosticsOnly(t *testing.T) { + m, db := openManagerAt(t, filepath.Join(t.TempDir(), "cache.db")) + defer db.Close() + defer m.Close() + producer := Authorization{CallerKind: CallerKindAgent, Principal: "p", AllowedServers: []string{"a", "b"}, Permissions: []string{"read"}} + narrow := Authorization{CallerKind: CallerKindAgent, Principal: "n", AllowedServers: []string{"a"}, Permissions: []string{"read"}} + star := Authorization{CallerKind: CallerKindAgent, Principal: "star", AllowedServers: []string{"*"}, Permissions: []string{"read"}} + if err := m.StoreAs("k", "t", nil, `[1]`, "", 1, producer); err != nil { + t.Fatal(err) + } + if got := onDiskSnapshotCount(t, db); got != 1 { + t.Fatalf("premise: snapshots = %d, want 1", got) + } + digest := producer.digest() + if err := db.Update(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheSnapshotBucket)).Delete(digest[:]) + }); err != nil { + t.Fatal(err) + } + if got := onDiskSnapshotCount(t, db); got != 0 { + t.Fatalf("premise: snapshots = %d, want 0", got) + } + for name, reader := range map[string]Authorization{"producer": producer, "unrestricted agent": star, "administrator": {CallerKind: CallerKindAdmin}} { + if resp, err := m.GetRecordsAs("k", 0, 10, reader); err != nil || len(resp.Records) != 1 { + t.Fatalf("%s: resp=%v err=%v, want the entry served without its diagnostic snapshot", name, resp, err) + } + } + if _, err := m.GetRecordsAs("k", 0, 10, narrow); !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("narrow: err = %v, want ErrUnauthorizedRead", err) + } + if _, ok := m.Peek("k"); !ok { + t.Fatal("the entry must survive") + } + if err := m.StoreAs("k2", "t", nil, `[1]`, "", 1, producer); err != nil { + t.Fatal(err) + } + if got := onDiskSnapshotCount(t, db); got != 1 { + t.Fatalf("snapshots after re-store = %d, want 1", got) + } +} + +// fleetSnapshot returns an agent authorization naming n servers of nameLen +// characters in both its grant and its effective profile, the way a real +// snapshot on a large fleet does. +func fleetSnapshot(n, nameLen int) Authorization { + a := Authorization{CallerKind: CallerKindAgent, Principal: "wide", Permissions: []string{"read"}, + ProfilePin: "fleet", Profile: "fleet", ProfileScoped: true} + pad := strings.Repeat("x", nameLen-len("srv-000000-")) + for i := 0; i < n; i++ { + name := fmt.Sprintf("srv-%06d-%s", i, pad) + a.AllowedServers = append(a.AllowedServers, name) + a.ProfileServers = append(a.ProfileServers, name) + } + return a +} + +// onDiskSnapshotCount counts the snapshots bucket directly. +func onDiskSnapshotCount(t *testing.T, db *bbolt.DB) int { + t.Helper() + n := 0 + if err := db.View(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheSnapshotBucket)).ForEach(func(_, _ []byte) error { + n++ + return nil + }) + }); err != nil { + t.Fatal(err) + } + return n +} + +// The snapshots bucket is content-addressed and shared, so the cleanup sweep +// must drop a snapshot only once NO surviving entry references it, and keep +// one that a live entry still does. +func TestCleanup_PrunesUnreferencedSnapshots(t *testing.T) { + m, db := openManagerAt(t, filepath.Join(t.TempDir(), "cache.db")) + defer db.Close() + defer m.Close() + a := Authorization{CallerKind: CallerKindAgent, Principal: "a", AllowedServers: []string{"a"}, Permissions: []string{"read"}} + b := Authorization{CallerKind: CallerKindAgent, Principal: "b", AllowedServers: []string{"b"}, Permissions: []string{"read"}} + for key, p := range map[string]Authorization{"a-live": a, "a-expiring": a, "b-expiring": b} { + if err := m.StoreAs(key, "t", nil, `[1]`, "", 1, p); err != nil { + t.Fatal(err) + } + } + if got := onDiskSnapshotCount(t, db); got != 2 { + t.Fatalf("snapshots = %d, want 2", got) + } + expireEntry(t, db, "a-expiring") + expireEntry(t, db, "b-expiring") + if err := m.cleanup(); err != nil { + t.Fatal(err) + } + if got := onDiskEntryCount(t, db); got != 1 { + t.Fatalf("entries after cleanup = %d, want 1", got) + } + if got := onDiskSnapshotCount(t, db); got != 1 { + t.Fatalf("snapshots after cleanup = %d, want 1: a's is still referenced, b's is not", got) + } + if _, err := m.GetRecordsAs("a-live", 0, 10, a); err != nil { + t.Fatalf("the surviving entry must still redeem after the prune: %v", err) + } + // A later entry under b re-persists the pruned snapshot. + if err := m.StoreAs("b-again", "t", nil, `[1]`, "", 1, b); err != nil { + t.Fatal(err) + } + if got := onDiskSnapshotCount(t, db); got != 2 { + t.Fatalf("snapshots after re-store = %d, want 2", got) + } +} + +// Codex rounds 3, 4 and 5 on the pre-frame (bare JSON) record's size. Round 3 +// folded the value's length out (an over-count of the escaped body, clamped +// against unrelated entries); round 4 decoded the value to fold out exactly +// len(FullContent) — a payload-sized decode on a refusal path, the one +// documented exception to payload independence — and round 5 pointed out +// that the exception IS the timing oracle FR-001 forbids. Research D16: the +// invalidation folds out NOTHING for a value without a decodable header (the +// entry count is exact without decoding, the size is not), and the cleanup +// sweep, which walks and decodes every record anyway, recomputes +// TotalEntries and TotalSizeBytes from the bucket. So the statistics are +// eventually consistent: over-counting by exactly the legacy payload until +// the sweep, exact after it — the live neighbour's, and nothing clamped — in +// memory and after a restart. +func TestGetRecordsAs_PreFrameInvalidationLeavesSizeToTheSweep(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + broad := Authorization{CallerKind: CallerKindAgent, Principal: "b", AllowedServers: []string{"a"}, Permissions: []string{"read"}} + live := `[{"v":"` + strings.Repeat("x", 100) + `"}]` + if err := m.StoreAs("live", "t", nil, live, "", 1, broad); err != nil { + t.Fatal(err) + } + // Content whose JSON encoding is far longer than the content itself, so + // any estimate from the value's length would be visibly wrong. + legacy := `["` + strings.Repeat(`\"`, 200) + `"]` + if err := m.Store("legacy", "t", nil, legacy, "", 1); err != nil { + t.Fatal(err) + } + if err := db.Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(CacheBucket)) + var rec Record + if err := rec.UnmarshalBinary(bucket.Get([]byte("legacy"))); err != nil { + return err + } + raw, err := json.Marshal(&rec) + if err != nil { + return err + } + return bucket.Put([]byte("legacy"), raw) + }); err != nil { + t.Fatal(err) + } + if got := m.GetStats().TotalSizeBytes; got != len(live)+len(legacy) { + t.Fatalf("seed: TotalSizeBytes = %d, want %d", got, len(live)+len(legacy)) + } + + if _, err := m.GetRecordsAs("legacy", 0, 10, Authorization{CallerKind: CallerKindAdmin}); !errors.Is(err, ErrLegacyProvenance) { + t.Fatalf("err = %v, want ErrLegacyProvenance", err) + } + if _, ok := m.Peek("legacy"); ok { + t.Fatal("pre-frame record still present") + } + // Documented drift: the entry is gone and counted out, the size waits + // for the sweep — nothing was read from the payload to find it. + if got := m.GetStats(); got.TotalEntries != 1 || got.TotalSizeBytes != len(live)+len(legacy) || got.EvictedCount != 1 { + t.Fatalf("stats after invalidation = %+v, want entries 1, evicted 1, size still %d (the legacy payload waits for the sweep)", *got, len(live)+len(legacy)) + } + if err := m.cleanup(); err != nil { + t.Fatal(err) + } + if got := m.GetStats(); got.TotalEntries != 1 || got.TotalSizeBytes != len(live) { + t.Fatalf("stats after the sweep = %+v, want entries 1, size %d (the live neighbour's, exactly)", *got, len(live)) + } + if _, err := m.GetRecordsAs("live", 0, 10, broad); err != nil { + t.Fatalf("live neighbour: %v", err) + } + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m2, db2 := openManagerAt(t, path) + defer db2.Close() + defer m2.Close() + if got := m2.GetStats(); got.TotalEntries != 1 || got.TotalSizeBytes != len(live) { + t.Fatalf("stats after restart = %+v, want entries 1, size %d", *got, len(live)) + } +} + +// The sweep's recomputation is from the bucket, whatever the counters held: +// a 1 MiB pre-frame entry invalidated on the gated door leaves TotalSizeBytes +// inflated by 1 MiB (round 3's original complaint) for at most one +// CleanupInterval, and drift of any other origin is reconciled the same way. +func TestCleanup_RecomputesEntriesAndSizeFromTheBucket(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + defer db.Close() + defer m.Close() + content := `[{"v":"` + strings.Repeat("x", 1<<20) + `"}]` + if err := m.Store("pre-frame", "t", nil, content, "", 1); err != nil { + t.Fatal(err) + } + if err := db.Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(CacheBucket)) + var rec Record + if err := rec.UnmarshalBinary(bucket.Get([]byte("pre-frame"))); err != nil { + return err + } + raw, err := json.Marshal(&rec) + if err != nil { + return err + } + return bucket.Put([]byte("pre-frame"), raw) + }); err != nil { + t.Fatal(err) + } + if _, err := m.GetRecordsAs("pre-frame", 0, 10, Authorization{CallerKind: CallerKindAdmin}); !errors.Is(err, ErrLegacyProvenance) { + t.Fatalf("err = %v, want ErrLegacyProvenance", err) + } + if got := m.GetStats(); got.TotalEntries != 0 || got.TotalSizeBytes != len(content) { + t.Fatalf("stats after invalidation = %+v, want entries 0 and the 1 MiB still counted until the sweep", *got) + } + // Arbitrary drift on top, as a crashed transaction or an older binary + // could have left. + if err := m.update(func(tx *bbolt.Tx) error { + m.stats.TotalEntries = 7 + m.stats.TotalSizeBytes += 12345 + return m.saveStats(tx) + }); err != nil { + t.Fatal(err) + } + if err := m.cleanup(); err != nil { + t.Fatal(err) + } + if got := m.GetStats(); got.TotalEntries != 0 || got.TotalSizeBytes != 0 { + t.Fatalf("stats after the sweep = %+v, want entries 0, size 0", *got) + } +} + +// Codex round 6, cache finding 1. The header's flag byte and tier byte each +// have bits no binary of this repository emits (only recordFlagDenyAll; only +// permBitRead|Write|Destructive|Other). A frame carrying one is not a frame +// this binary wrote, and the gate must classify it as unrecognised +// provenance ON THE HEADER — the O(1) refuse-and-invalidate path — never +// after a body decode: with the digest preserved, an unknown tier bit used +// to be admitted on digest equality and then caught by the header/body +// agreement check, a payload-sized decode on a refusal (the timing class +// round 5 removed), and an unknown flag bit was ignored outright, so the +// record was served. Pinned through the allocation meter on a 4 MB body: +// the refusal must sit in the miss's class, for the digest-equal producer +// and for an administrator alike. +func TestGetRecordsAs_UnknownHeaderBitsAreUnrecognisedProvenance(t *testing.T) { + producer := Authorization{CallerKind: CallerKindAgent, Principal: "p", AllowedServers: []string{"a"}, Permissions: []string{"read"}} + admin := Authorization{CallerKind: CallerKindAdmin} + const bigPayload = 4 << 20 + content := `[{"v":"` + strings.Repeat("x", bigPayload) + `"}]` + + tampers := []struct { + name string + offset int + mutate func(b byte) byte + }{ + {"unknown flag bit", recordHeaderOffFlg, func(b byte) byte { return b | 1<<1 }}, + {"unknown permission bit", recordHeaderOffPrm, func(byte) byte { return 1 << 6 }}, + {"every reserved flag bit", recordHeaderOffFlg, func(b byte) byte { return b | ^byte(recordFlagDenyAll) }}, + {"every reserved permission bit", recordHeaderOffPrm, func(b byte) byte { return b | ^permBitKnown }}, + } + for _, tp := range tampers { + for _, rd := range []struct { + name string + reader Authorization + }{{"digest-equal producer", producer}, {"administrator", admin}} { + t.Run(tp.name+"/"+rd.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + const key = "target" + if err := m.StoreAs(key, "t", nil, content, "", 1, producer); err != nil { + t.Fatal(err) + } + // Flip the one byte in place; the digest, size, expiry and body + // are the genuine entry's. + if err := db.Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(CacheBucket)) + value := append([]byte(nil), bucket.Get([]byte(key))...) + at := len(recordFrameMagic) + tp.offset + value[at] = tp.mutate(value[at]) + return bucket.Put([]byte(key), value) + }); err != nil { + t.Fatal(err) + } + // Probed cold: only the header on disk can decide. + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m, db = openManagerAt(t, path) + defer db.Close() + defer m.Close() + + allocated, resp, err := allocatedBy(func() (*ReadCacheResponse, error) { + return m.GetRecordsAs(key, 0, 10, rd.reader) + }) + if !errors.Is(err, ErrLegacyProvenance) || resp != nil { + t.Fatalf("got err=%v resp=%v, want ErrLegacyProvenance decided on the header", err, resp != nil) + } + if allocated > refusalAllocBudget { + t.Fatalf("refusing the tampered 4 MB entry allocated %d bytes (budget %d): the body was decoded before the refusal", allocated, refusalAllocBudget) + } + if got := onDiskEntryCount(t, db); got != 0 { + t.Fatalf("on-disk count after the refusal = %d, want 0 (invalidated)", got) + } + }) + } + } + + // The decoder itself: every emitted combination round-trips, every + // reserved bit is corrupt. + t.Run("decoder", func(t *testing.T) { + base := (&Record{Version: RecordVersion, Producer: &producer, ExpiresAt: time.Now().Add(time.Hour), TotalSize: 1}).header() + for perms := 0; perms <= 0xff; perms++ { + for flags := 0; flags <= 0xff; flags++ { + raw := base.encode() + raw[recordHeaderOffFlg], raw[recordHeaderOffPrm] = byte(flags), byte(perms) + _, err := decodeHeaderBytes(raw) + emitted := byte(flags)&^recordFlagDenyAll == 0 && byte(perms)&^permBitKnown == 0 + if emitted && err != nil { + t.Fatalf("flags %#x perms %#x: %v, want decodable", flags, perms, err) + } + if !emitted && !errors.Is(err, errRecordFrameCorrupt) { + t.Fatalf("flags %#x perms %#x: err=%v, want errRecordFrameCorrupt", flags, perms, err) + } + } + } + }) +} diff --git a/internal/cache/manager_internal_test.go b/internal/cache/manager_internal_test.go new file mode 100644 index 000000000..d9d7196e3 --- /dev/null +++ b/internal/cache/manager_internal_test.go @@ -0,0 +1,166 @@ +package cache + +import ( + "errors" + "testing" + "time" + + "go.etcd.io/bbolt" + "go.uber.org/zap" +) + +// Spec 105 FR-002 (gap FR001-G2): entries the proxy writes for ITSELF — the +// registry search cache (`registry-servers::::`) and +// the repository guesser cache (`npm:`) — are stamped with the internal +// caller kind by their writers and are non-redeemable through the gated read +// for every caller, administrators included (SC-005 names this exception). +// Unlike legacy entries they are refused WITHOUT eviction: their keys are +// guessable, and evicting on refusal would let any caller purge the entries +// the registry (Peek) and guesser (Get) readers depend on. +// +// The kind is spelled as its wire value here so this file compiles against +// the pre-feature package; the constant the writers use is asserted by the +// writer tests in internal/runtime and internal/experiments. +const internalCallerKindWire = "internal" + +func TestGetRecordsAs_InternalEntryRefusedForEveryCallerWithoutEviction(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + m, err := NewManager(db, zap.NewNop()) + if err != nil { + t.Fatal(err) + } + defer m.Close() + + internal := Authorization{CallerKind: internalCallerKindWire} + const registryKey = "registry-servers:official:::10" + const npmKey = "npm:@acme/mcp-server" + if err := m.StoreAs(registryKey, "registry-servers", nil, `[{"id":"srv-1","name":"SENTINEL-REGISTRY"}]`, "", 1, internal); err != nil { + t.Fatal(err) + } + if err := m.StoreAs(npmKey, "repo_guess", map[string]interface{}{"package_name": "@acme/mcp-server"}, `{"package_name":"@acme/mcp-server","exists":true}`, "", 1, internal); err != nil { + t.Fatal(err) + } + before := *m.GetStats() + + readers := []struct { + name string + reader Authorization + }{ + {"admin", Authorization{CallerKind: CallerKindAdmin}}, + {"admin_user", Authorization{CallerKind: CallerKindAdminUser, Principal: "u9"}}, + {"anonymous", Authorization{CallerKind: CallerKindAnonymous}}, + {"wildcard agent", Authorization{CallerKind: CallerKindAgent, Principal: "star", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}}}, + {"user", Authorization{CallerKind: CallerKindUser, Principal: "u1"}}, + // Even a reader presenting the internal kind itself is refused: the + // gated read is the agent-facing door, and no request comes through it + // as the proxy's own writer. + {"internal-shaped reader", internal}, + } + for _, key := range []string{registryKey, npmKey} { + for _, rd := range readers { + t.Run(key+"/"+rd.name, func(t *testing.T) { + resp, err := m.GetRecordsAs(key, 0, 10, rd.reader) + if !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("%s reading internal entry %q: got err=%v resp=%v, want ErrUnauthorizedRead", rd.name, key, err, resp) + } + if !errors.Is(err, ErrInternalEntry) { + t.Fatalf("%s: got %v, want the ErrInternalEntry sentinel the handler renders for administrators", rd.name, err) + } + if resp != nil { + t.Fatalf("refused internal read returned content: %+v", resp) + } + // No eviction: the internal readers still find their entry. + rec, ok := m.Peek(key) + if !ok { + t.Fatalf("internal entry %q was evicted by a refused redemption (eviction DoS on a guessable key)", key) + } + if rec.Producer == nil || rec.Producer.CallerKind != internalCallerKindWire { + t.Fatalf("internal stamp lost: %+v", rec.Producer) + } + if got, err := m.Get(key); err != nil || got == nil { + t.Fatalf("the ungated internal reader (Get) must still serve %q: %v", key, err) + } + }) + } + } + + // Refusals are neither hits nor evictions. Each refusal counts as a MISS + // — the same stats signal an absent key leaves, so the refusal commits + // like a miss and shares its timing class (critique round 1, finding 1); + // the Get control calls above are the only hits. + refusals := len(readers) * 2 + after := *m.GetStats() + if got, want := after.MissCount, before.MissCount+refusals; got != want { + t.Fatalf("MissCount = %d, want %d: every refused internal read counts as a miss", got, want) + } + after.HitCount = before.HitCount + after.MissCount = before.MissCount + if before != after { + t.Fatalf("refused internal reads changed stats beyond misses and the control Gets: before=%+v after=%+v", before, after) + } +} + +// Critique round 2, finding 4: on the gated door the expiry check ran BEFORE +// the internal-kind check, so a read_cache probe of a guessable internal key +// whose entry had passed its TTL committed the eviction — exactly the +// eviction-on-refusal FR-002 forbids for internal entries, bounded only by +// the cleanup interval. The registry reader (runtime.SearchRegistryServers) +// deliberately serves an expired entry through Peek as Stale until cleanup +// runs; the gated door must leave it there. +func TestGetRecordsAs_ExpiredInternalEntryRefusedWithoutEviction(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + m, err := NewManager(db, zap.NewNop()) + if err != nil { + t.Fatal(err) + } + defer m.Close() + + const key = "registry-servers:official:::10" + if err := m.StoreAs(key, "registry-servers", nil, `[{"id":"srv-1"}]`, "", 1, Authorization{CallerKind: internalCallerKindWire}); err != nil { + t.Fatal(err) + } + if err := db.Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(CacheBucket)) + var rec Record + if err := rec.UnmarshalBinary(bucket.Get([]byte(key))); err != nil { + return err + } + rec.ExpiresAt = time.Now().Add(-time.Hour) + data, err := rec.MarshalBinary() + if err != nil { + return err + } + return bucket.Put([]byte(key), data) + }); err != nil { + t.Fatal(err) + } + before := *m.GetStats() + + for _, rd := range []struct { + name string + reader Authorization + }{ + {"admin", Authorization{CallerKind: CallerKindAdmin}}, + {"wildcard agent", Authorization{CallerKind: CallerKindAgent, Principal: "star", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}}}, + } { + resp, err := m.GetRecordsAs(key, 0, 10, rd.reader) + if !errors.Is(err, ErrInternalEntry) { + t.Fatalf("%s reading an EXPIRED internal entry: got err=%v resp=%v, want ErrInternalEntry (the internal refusal, not the expiry eviction)", rd.name, err, resp) + } + rec, ok := m.Peek(key) + if !ok { + t.Fatalf("%s: the expired internal entry was evicted by a gated probe; the registry reader serves it as Stale until cleanup", rd.name) + } + if !rec.IsExpired() { + t.Fatalf("premise lost: %+v", rec) + } + } + after := *m.GetStats() + if after.EvictedCount != before.EvictedCount || after.TotalEntries != before.TotalEntries { + t.Fatalf("a gated probe of an expired internal key must not evict: before=%+v after=%+v", before, after) + } +} diff --git a/internal/cache/manager_legacy_test.go b/internal/cache/manager_legacy_test.go new file mode 100644 index 000000000..241fac796 --- /dev/null +++ b/internal/cache/manager_legacy_test.go @@ -0,0 +1,611 @@ +package cache + +import ( + "encoding/json" + "errors" + "path/filepath" + "slices" + "testing" + "time" + + "go.etcd.io/bbolt" + "go.uber.org/zap" +) + +// Spec 105 FR-002 (gap FR001-G1): an entry with absent, legacy or unrecognised +// provenance — every entry persisted before producer stamping existed, and +// anything still written through Store — is refused for EVERY caller kind, +// administrators and the administrator-shaped anonymous caller included +// (research D2, SC-004), and is durably invalidated on that first refused +// redemption: gone from Peek, gone after the bbolt file is closed and +// reopened. Before this feature such an entry was readable by any +// unrestricted kind and survived every refusal. + +// openManagerAt opens (or reopens) a cache manager on the bbolt file at path. +// Tests that prove durability close the first handle and reopen the same file +// — the only way to tell a committed delete from one bbolt rolled back. +func openManagerAt(t *testing.T, path string) (*Manager, *bbolt.DB) { + t.Helper() + db, err := bbolt.Open(path, 0644, &bbolt.Options{Timeout: time.Second}) + if err != nil { + t.Fatalf("open bbolt %s: %v", path, err) + } + m, err := NewManager(db, zap.NewNop()) + if err != nil { + t.Fatalf("new manager: %v", err) + } + return m, db +} + +// onDiskEntryCount counts the records physically present in the cache bucket +// in a fresh read transaction — the committed state, independent of whatever +// the in-memory stats believe. +func onDiskEntryCount(t *testing.T, db *bbolt.DB) int { + t.Helper() + n := 0 + err := db.View(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheBucket)).ForEach(func(_, _ []byte) error { + n++ + return nil + }) + }) + if err != nil { + t.Fatalf("count cache bucket: %v", err) + } + return n +} + +// putRawRecord writes an arbitrary JSON document under key straight into the +// cache bucket, bypassing every Store path — the shape a pre-feature binary +// left on disk (no producer field, no version field) or a record a later +// schema wrote (a version this binary does not recognise). +func putRawRecord(t *testing.T, db *bbolt.DB, key string, doc map[string]interface{}) { + t.Helper() + data, err := json.Marshal(doc) + if err != nil { + t.Fatal(err) + } + if err := db.Update(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheBucket)).Put([]byte(key), data) + }); err != nil { + t.Fatalf("put raw record: %v", err) + } +} + +// legacyReaders enumerates every caller kind the gate can see. FR-002 names +// them all: "every caller" includes administrators and anonymous callers. +func legacyReaders() []struct { + name string + reader Authorization +} { + return []struct { + name string + reader Authorization + }{ + {"admin", Authorization{CallerKind: CallerKindAdmin}}, + {"admin_user", Authorization{CallerKind: CallerKindAdminUser, Principal: "u9"}}, + {"anonymous", Authorization{CallerKind: CallerKindAnonymous}}, + {"wildcard agent", Authorization{CallerKind: CallerKindAgent, Principal: "star", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}}}, + {"user", Authorization{CallerKind: CallerKindUser, Principal: "u1"}}, + } +} + +func TestGetRecordsAs_LegacyEntryRefusedForEveryCallerAndInvalidated(t *testing.T) { + for _, tc := range legacyReaders() { + t.Run(tc.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + + // Store (no producer) is exactly what a pre-stamping binary wrote. + if err := m.Store("legacy", "github:list", nil, `{"items":[1,2,3]}`, "items", 3); err != nil { + t.Fatal(err) + } + if _, ok := m.Peek("legacy"); !ok { + t.Fatal("premise: the legacy entry is present before the first redemption") + } + + resp, err := m.GetRecordsAs("legacy", 0, 10, tc.reader) + if !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("%s reading a legacy (nil-producer) entry: got err=%v resp=%v, want ErrUnauthorizedRead", tc.name, err, resp) + } + if !errors.Is(err, ErrLegacyProvenance) { + t.Fatalf("%s: got %v, want the ErrLegacyProvenance sentinel the handler renders for administrators", tc.name, err) + } + if resp != nil { + t.Fatalf("%s: a refused legacy read must return no content, got %+v", tc.name, resp) + } + + // Invalidated on first redemption: the refusal evicts the entry. + if rec, ok := m.Peek("legacy"); ok { + t.Fatalf("%s: legacy entry still present after the refused redemption: %+v", tc.name, rec) + } + if got := onDiskEntryCount(t, db); got != 0 { + t.Fatalf("%s: legacy entry still on disk after refusal (count=%d); the delete was rolled back", tc.name, got) + } + // Stats mutate only on the committed path and must agree with the + // bucket: the eviction is one fewer entry, one more eviction. + stats := *m.GetStats() + if stats.TotalEntries != 0 || stats.EvictedCount != 1 { + t.Fatalf("%s: stats after committed eviction = %+v, want TotalEntries=0 EvictedCount=1", tc.name, stats) + } + + // Durably: still absent after the bbolt file is closed and reopened. + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m2, db2 := openManagerAt(t, path) + defer db2.Close() + defer m2.Close() + if rec, ok := m2.Peek("legacy"); ok { + t.Fatalf("%s: legacy entry survived a reopen: %+v", tc.name, rec) + } + if got := onDiskEntryCount(t, db2); got != 0 { + t.Fatalf("%s: on-disk count after reopen = %d, want 0", tc.name, got) + } + // A second redemption of the same key is a plain miss, for every + // caller — nothing left to disclose. + if _, err := m2.GetRecordsAs("legacy", 0, 10, tc.reader); err == nil || errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("%s: after invalidation the key must be a plain miss, got %v", tc.name, err) + } + }) + } +} + +// Upgrade fixture (FR-002, SC-004, gap FR001-G7a): a record exactly as a +// pre-feature binary serialised it — no producer field at all, no version +// field (or version 0) — is refused for administrator and agent callers, +// returns no content, and is absent after a restart of the store. A record +// carrying a version this binary does not recognise is treated the same way: +// unknown provenance is legacy provenance. +func TestGetRecordsAs_UpgradeFixturePreFeatureRecordRefusedAndAbsentAfterReopen(t *testing.T) { + now := time.Now() + preFeature := map[string]interface{}{ + "key": "pre-feature", + "tool_name": "github:list_issues", + "args": map[string]interface{}{"repo": "acme/widgets"}, + "timestamp": now, + "full_content": `{"issues":[{"id":1,"title":"SENTINEL-PRE-FEATURE"},{"id":2,"title":"second"}]}`, + "record_path": "issues", + "total_records": 2, + "total_size": 80, + "expires_at": now.Add(time.Hour), + "access_count": 0, + "last_accessed": now, + "created_at": now, + // no "producer", no "version": the pre-feature wire shape + } + variant := func(key string, extra map[string]interface{}) map[string]interface{} { + doc := map[string]interface{}{} + for k, v := range preFeature { + doc[k] = v + } + doc["key"] = key + for k, v := range extra { + doc[k] = v + } + return doc + } + + fixtures := []struct { + name string + doc map[string]interface{} + }{ + {"no producer, no version", preFeature}, + {"no producer, version 0", variant("version-zero", map[string]interface{}{"version": 0})}, + {"no producer, unrecognised version", variant("unknown-version", map[string]interface{}{"version": 99})}, + // Stamped with a producer but no version: a record from a binary that + // stamped producers before versions existed is still legacy provenance. + {"producer stamped, no version", variant("stamped-no-version", map[string]interface{}{ + "producer": map[string]interface{}{"caller_kind": CallerKindAdmin}})}, + // Critique round 1, finding 2: "unrecognised provenance" is decided on + // the CALLER KIND as well as the version. A kind this binary does not + // know — an empty stamp, or one a later binary added without bumping + // RecordVersion and a rollback left behind — is not one an + // administrator reader may be handed (CouldHaveProduced answers true + // for any non-internal kind once the reader is an administrator, so + // an unknown kind failed OPEN for the anonymous and admin readers). + {"current version, empty caller kind", variant("empty-kind", map[string]interface{}{ + "version": RecordVersion, "producer": map[string]interface{}{"caller_kind": ""}})}, + {"current version, unknown caller kind", variant("unknown-kind", map[string]interface{}{ + "version": RecordVersion, "producer": map[string]interface{}{"caller_kind": "superadmin"}})}, + {"current version, versioned-looking caller kind", variant("future-kind", map[string]interface{}{ + "version": RecordVersion, "producer": map[string]interface{}{"caller_kind": "agent-v2", + "allowed_servers": []string{"*"}}})}, + // Critique round 1 finding 6 / round 2 finding 6: a record this binary + // cannot even decode (here: a version that overflows the uint8 field — + // a downgrade after a future schema bump) is provenance it does not + // recognise. Before this round it surfaced as a distinct "unmarshal + // cache record" body for every caller and was never invalidated. + {"undecodable version", variant("undecodable", map[string]interface{}{"version": 300})}, + } + readers := []struct { + name string + reader Authorization + }{ + {"admin", Authorization{CallerKind: CallerKindAdmin}}, + {"anonymous", Authorization{CallerKind: CallerKindAnonymous}}, + {"agent", Authorization{CallerKind: CallerKindAgent, Principal: "bot", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}}}, + } + + for _, fx := range fixtures { + for _, rd := range readers { + t.Run(fx.name+"/"+rd.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + key := fx.doc["key"].(string) + putRawRecord(t, db, key, fx.doc) + if got, want := onDiskEntryCount(t, db), 1; got != want { + t.Fatalf("premise: the raw record is on disk (count=%d)", got) + } + + resp, err := m.GetRecordsAs(key, 0, 10, rd.reader) + if !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("%s reading a pre-feature record: got err=%v, want ErrUnauthorizedRead", rd.name, err) + } + // The specific sentinel, not just the parent: the handler keys + // the administrator's "predates provenance" body on it. + if !errors.Is(err, ErrLegacyProvenance) { + t.Fatalf("%s: got %v, want ErrLegacyProvenance", rd.name, err) + } + if resp != nil { + t.Fatalf("refused read returned content: %+v", resp) + } + if rec, ok := m.Peek(key); ok { + t.Fatalf("pre-feature record still present after the refused redemption: %+v", rec) + } + + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m2, db2 := openManagerAt(t, path) + defer db2.Close() + defer m2.Close() + if rec, ok := m2.Peek(key); ok { + t.Fatalf("pre-feature record survived a restart: %+v", rec) + } + if got := onDiskEntryCount(t, db2); got != 0 { + t.Fatalf("on-disk count after restart = %d, want 0", got) + } + }) + } + } +} + +// A refused legacy redemption must not disturb its neighbours: only the +// redeemed key is invalidated, and the stats stay consistent with the bucket. +func TestGetRecordsAs_LegacyInvalidationIsPerKey(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + defer db.Close() + defer m.Close() + + if err := m.Store("legacy-a", "t", nil, `[1]`, "", 1); err != nil { + t.Fatal(err) + } + if err := m.Store("legacy-b", "t", nil, `[2]`, "", 1); err != nil { + t.Fatal(err) + } + admin := Authorization{CallerKind: CallerKindAdmin} + if err := m.StoreAs("stamped", "t", nil, `[3]`, "", 1, admin); err != nil { + t.Fatal(err) + } + + if _, err := m.GetRecordsAs("legacy-a", 0, 10, admin); !errors.Is(err, ErrUnauthorizedRead) { + t.Fatalf("got %v, want ErrUnauthorizedRead", err) + } + if _, ok := m.Peek("legacy-a"); ok { + t.Fatal("redeemed legacy key must be gone") + } + if _, ok := m.Peek("legacy-b"); !ok { + t.Fatal("an unredeemed legacy key must be untouched") + } + if _, ok := m.Peek("stamped"); !ok { + t.Fatal("a stamped key must be untouched") + } + if got, want := onDiskEntryCount(t, db), 2; got != want { + t.Fatalf("on-disk count = %d, want %d", got, want) + } + if got := m.GetStats().TotalEntries; got != 2 { + t.Fatalf("stats.TotalEntries = %d, want 2 (must match the bucket)", got) + } + if _, err := m.GetRecordsAs("stamped", 0, 10, admin); err != nil { + t.Fatalf("the stamped neighbour must still read: %v", err) + } +} + +// Codex round 2, finding 1: a current-version agent snapshot that could have +// authorized no tool — an empty server grant, or an empty effective profile +// (an empty profile, or the scope a stale pin resolves to) — is not one any +// request legitimately produced an entry under. The reader-side deny-all +// guards (codex round 1) left the PRODUCER side open: containment against an +// empty set is vacuously true, so any broader agent redeemed such a record. +// At the door it must be refused for every agent reader with the +// non-disclosing verdict (a miss to the handler), kept (it is a stamped +// record, not legacy provenance), and still readable by an administrator +// (FR-001, kind first). +func TestGetRecordsAs_DenyAllProducerSnapshotRefusedForAgentReaders(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + m, err := NewManager(db, zap.NewNop()) + if err != nil { + t.Fatal(err) + } + defer m.Close() + + producers := map[string]Authorization{ + "nil-grant": {CallerKind: CallerKindAgent, Principal: "rotated", AllowedServers: nil, Permissions: []string{"read"}}, + "empty-grant": {CallerKind: CallerKindAgent, Principal: "rotated", AllowedServers: []string{}, Permissions: []string{"read"}}, + "empty-profile": {CallerKind: CallerKindAgent, Principal: "star", AllowedServers: []string{"*"}, + Permissions: []string{"read", "write", "destructive"}, Profile: "empty", ProfileScoped: true, ProfileServers: []string{}}, + "stale-pin": {CallerKind: CallerKindAgent, Principal: "star-pinned", AllowedServers: []string{"*"}, + Permissions: []string{"read", "write", "destructive"}, ProfilePin: "research", Profile: "research", + ProfileScoped: true, ProfileServers: nil}, + } + readers := []struct { + name string + reader Authorization + }{ + {"broad agent", Authorization{CallerKind: CallerKindAgent, Principal: "broad", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read", "write"}}}, + {"wildcard agent", Authorization{CallerKind: CallerKindAgent, Principal: "star", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}}}, + {"pinned wildcard agent", Authorization{CallerKind: CallerKindAgent, Principal: "star-pinned", + AllowedServers: []string{"*"}, Permissions: []string{"read", "write", "destructive"}, + ProfilePin: "research", Profile: "research", ProfileScoped: true, ProfileServers: []string{"github"}}}, + } + for name, producer := range producers { + if err := m.StoreAs(name, "t", nil, `[{"name":"SENTINEL_DENY_ALL_PRODUCER"}]`, "", 1, producer); err != nil { + t.Fatal(err) + } + for _, rd := range readers { + t.Run(name+"/"+rd.name, func(t *testing.T) { + resp, err := m.GetRecordsAs(name, 0, 10, rd.reader) + if !errors.Is(err, ErrUnauthorizedRead) || resp != nil { + t.Fatalf("%s redeemed a record stamped with a deny-all producer %+v: resp=%+v err=%v", rd.name, producer, resp, err) + } + if _, ok := m.Peek(name); !ok { + t.Fatal("a refused stamped record must not be evicted") + } + }) + } + t.Run(name+"/admin control", func(t *testing.T) { + resp, err := m.GetRecordsAs(name, 0, 10, Authorization{CallerKind: CallerKindAdmin}) + if err != nil || len(resp.Records) != 1 { + t.Fatalf("an administrator qualifies for any snapshot: resp=%+v err=%v", resp, err) + } + }) + } +} + +// putFramedRecord writes a value in the framed layout with an arbitrary +// header and body — the shapes a later or corrupted binary could leave: a +// header the gate must classify WITHOUT reading the body. +func putFramedRecord(t *testing.T, db *bbolt.DB, key string, header, body []byte) { + t.Helper() + if err := db.Update(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheBucket)).Put([]byte(key), encodeRecordFrame(header, body)) + }); err != nil { + t.Fatalf("put framed record: %v", err) + } +} + +// Codex round 2, finding 3: the gate now decides provenance on the frame +// header alone, so the unrecognised-provenance rules the raw-JSON fixtures +// above pin (unknown version, unknown or empty caller kind, undecodable) +// must hold on the HEADER too — a raw fixture is legacy simply for having no +// frame, which would let a header-level check go vacuous. Each shape is +// refused for every caller with ErrLegacyProvenance and durably invalidated; +// the body is never consulted (a body the gate would have admitted sits +// behind every bad header here). Round 4 made the header fixed-size with the +// producer named by digest, so the shapes are: no producer (kind code 0), +// unknown version, unknown kind code, a truncated frame, and an agent +// header in front of an administrator body (the digest the gate admits on +// and the body's producer disagree; the snapshots bucket is diagnostics +// only and its content never decides — research D16). +func TestGetRecordsAs_FramedRecordWithUnrecognisedHeaderIsLegacy(t *testing.T) { + now := time.Now() + adminProducer := &Authorization{CallerKind: CallerKindAdmin} + goodBody := func(key string) []byte { + rec := &Record{Key: key, ToolName: "t", FullContent: `[{"name":"SENTINEL-FRAMED"}]`, TotalSize: 27, + Timestamp: now, ExpiresAt: now.Add(time.Hour), CreatedAt: now, LastAccessed: now, + Version: RecordVersion, Producer: adminProducer} + body, err := json.Marshal(rec) + if err != nil { + t.Fatal(err) + } + return body + } + // current is the header goodBody's record would carry. + current := (&Record{Version: RecordVersion, Producer: adminProducer, ExpiresAt: now.Add(time.Hour), TotalSize: 27}).header() + with := func(mutate func(h *recordHeader)) []byte { + h := current + mutate(&h) + return h.encode() + } + aOnly := Authorization{CallerKind: CallerKindAgent, Principal: "a", AllowedServers: []string{"a"}, Permissions: []string{"read"}} + + type fixture struct { + name string + header []byte + body func(key string) []byte + // admittedOnly restricts the readers to the kinds the header admits + // (the body is reached only after admission). + admittedOnly bool + // skipKinds names reader kinds the header refuses on KIND alone — + // before the snapshot is ever needed — so they never reach the + // shape under test; asserted separately below. + skipKinds []string + // want is the verdict; ErrLegacyProvenance unless set. A shape the + // header ADMITS and only the body betrays is not a refusal + // (codex round 6): the reader is entitled to the entry, so it + // gets the admitted-class ErrEntryUnreadable — never the shape a + // header refusal shares with a miss. + want error + } + fixtures := []fixture{ + {name: "header: no producer (kind code 0)", header: with(func(h *recordHeader) { h.KindCode, h.Kind = 0, "" }), body: goodBody}, + {name: "header: unknown version", header: with(func(h *recordHeader) { h.Version = 99 }), body: goodBody}, + {name: "header: unknown caller kind code", header: with(func(h *recordHeader) { h.KindCode, h.Kind = 200, "" }), body: goodBody}, + {name: "header: truncated frame"}, + // A well-formed a-only agent header (its digest in the snapshots + // bucket or not — the gate never looks) in front of an + // administrator body. Every reader the header admits — an + // administrator on the kind, an unrestricted agent on the + // tier-covered digest mismatch rule — then finds a body that + // disagrees with the header's digest: unreadable, invalidated. A + // reader of another scoped kind (a user) is refused on the kind + // alone, non-disclosingly, and the entry is kept — see the user + // assertion below. + {name: "header: agent digest in front of an administrator body", + header: with(func(h *recordHeader) { + h.KindCode, h.Kind = callerKindCode(CallerKindAgent), CallerKindAgent + h.Perms = permissionBits(aOnly.Permissions) + h.Digest = aOnly.digest() + }), body: goodBody, skipKinds: []string{CallerKindUser}, want: ErrEntryUnreadable}, + // The one shape the gate admits on the header and only then finds + // undecodable: invalidated, and reported as unreadable (after + // admission, so the payload-sized decode is the admitted reader's, + // not a probe's). Readers the header does NOT admit are refused on + // the header, with the non-disclosing verdict, and never reach the + // body. + {name: "body: undecodable behind an admitted header", + header: current.encode(), + body: func(string) []byte { return []byte("{not json") }, + admittedOnly: true, want: ErrEntryUnreadable}, + } + for _, fx := range fixtures { + for _, rd := range legacyReaders() { + if fx.admittedOnly && !rd.reader.IsAdministrator() { + continue + } + if fx.admittedOnly && rd.reader.CallerKind == CallerKindAnonymous { + continue // ranks below an authenticated administrator's snapshot + } + if slices.Contains(fx.skipKinds, rd.reader.CallerKind) { + continue + } + t.Run(fx.name+"/"+rd.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + const key = "framed" + if fx.name == "header: truncated frame" { + // The magic followed by fewer bytes than a header. + if err := db.Update(func(tx *bbolt.Tx) error { + value := append(append([]byte(nil), recordFrameMagic...), current.encode()[:recordHeaderSize/2]...) + return tx.Bucket([]byte(CacheBucket)).Put([]byte(key), value) + }); err != nil { + t.Fatal(err) + } + } else { + putFramedRecord(t, db, key, fx.header, fx.body(key)) + } + + want := fx.want + if want == nil { + want = ErrLegacyProvenance + } + resp, err := m.GetRecordsAs(key, 0, 10, rd.reader) + if !errors.Is(err, want) { + t.Fatalf("%s: got err=%v resp=%v, want %v", rd.name, err, resp, want) + } + if resp != nil { + t.Fatalf("refused read returned content: %+v", resp) + } + if _, ok := m.Peek(key); ok { + t.Fatal("record still present after the refused redemption") + } + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m2, db2 := openManagerAt(t, path) + defer db2.Close() + defer m2.Close() + if got := onDiskEntryCount(t, db2); got != 0 { + t.Fatalf("on-disk count after restart = %d, want 0", got) + } + }) + } + } + + // Codex round 6, server finding 1: the admitted-then-undecodable shape + // for a SCOPED reader. An agent whose digest equals the header's is + // admitted on the header — it is entitled to the entry — and only then + // finds the body undecodable (or disagreeing with the header). What it + // learns is that its own entry is corrupt, which discloses nothing + // about any other subject; but the outcome must not wear the refusal + // shape, so that every response sharing the not-found shape is one the + // header decided: ErrEntryUnreadable, an admitted-class error that is + // NOT an ErrUnauthorizedRead (the handler renders it distinctly for + // every caller kind), with the entry invalidated all the same. + t.Run("body: undecodable behind an admitted header/digest-equal agent gets ErrEntryUnreadable", func(t *testing.T) { + for _, body := range []struct { + name string + body []byte + }{ + {"undecodable", []byte("{not json")}, + {"disagreeing (administrator body)", goodBody("framed")}, + } { + t.Run(body.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + const key = "framed" + putFramedRecord(t, db, key, with(func(h *recordHeader) { + h.KindCode, h.Kind = callerKindCode(CallerKindAgent), CallerKindAgent + h.Perms = permissionBits(aOnly.Permissions) + h.Digest = aOnly.digest() + }), body.body) + resp, err := m.GetRecordsAs(key, 0, 10, aOnly) + if !errors.Is(err, ErrEntryUnreadable) || resp != nil { + t.Fatalf("digest-equal agent: got err=%v resp=%v, want ErrEntryUnreadable", err, resp) + } + if errors.Is(err, ErrUnauthorizedRead) || errors.Is(err, ErrKeyNotFound) || errors.Is(err, ErrKeyExpired) { + t.Fatalf("err %v wears a refusal shape; an admitted reader's outcome must not", err) + } + if got := onDiskEntryCount(t, db); got != 0 { + t.Fatalf("on-disk count = %d, want 0: the unreadable entry is invalidated", got) + } + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m2, db2 := openManagerAt(t, path) + defer db2.Close() + defer m2.Close() + if got := onDiskEntryCount(t, db2); got != 0 { + t.Fatalf("on-disk count after restart = %d, want 0", got) + } + }) + } + }) + + // A user reader is refused on the header's kind alone (caller kind + // first: an agent snapshot is never a user's), with the ordinary + // non-disclosing verdict, and the entry is kept — the body is never + // decoded, so its disagreement is not observed. + t.Run("header: agent digest in front of an administrator body/user refused on kind, kept", func(t *testing.T) { + m, db := openManagerAt(t, filepath.Join(t.TempDir(), "cache.db")) + defer db.Close() + defer m.Close() + const key = "framed" + putFramedRecord(t, db, key, with(func(h *recordHeader) { + h.KindCode, h.Kind = callerKindCode(CallerKindAgent), CallerKindAgent + h.Perms = permissionBits(aOnly.Permissions) + h.Digest = aOnly.digest() + }), goodBody(key)) + user := Authorization{CallerKind: CallerKindUser, Principal: "u1", AllowedServers: []string{"*"}, Permissions: []string{"read"}} + resp, err := m.GetRecordsAs(key, 0, 10, user) + if !errors.Is(err, ErrUnauthorizedRead) || errors.Is(err, ErrLegacyProvenance) || resp != nil { + t.Fatalf("user: got err=%v resp=%v, want the plain ErrUnauthorizedRead", err, resp) + } + // Peek cannot decode the crafted value (its body disagrees with + // the header), so count the bucket directly. + if got := onDiskEntryCount(t, db); got != 1 { + t.Fatalf("on-disk count = %d, want 1: a kind refusal must not evict", got) + } + }) +} diff --git a/internal/cache/manager_payload_independent_test.go b/internal/cache/manager_payload_independent_test.go new file mode 100644 index 000000000..5e38057c7 --- /dev/null +++ b/internal/cache/manager_payload_independent_test.go @@ -0,0 +1,417 @@ +package cache + +import ( + "encoding/json" + "errors" + "fmt" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "go.etcd.io/bbolt" +) + +// refusalAllocBudget is a refusal's allocation budget: room for the frame +// header, the stats round-trip and bbolt's own bookkeeping for a commit (an +// eviction hands the payload's pages to the freelist by id, never by +// content), and an order of magnitude below the smallest payload that would +// betray a decode. The 1 KB and 4 MB legs must both fit — a refusal that +// scaled with the payload fails on the 4 MB leg alone. +const refusalAllocBudget = 256 << 10 + +// Codex round 2 (cache finding 3, server finding 1): spec Definitions make a +// non-disclosing refusal indistinguishable in status, body AND timing CLASS +// from a miss. Round 1 pinned the commit shape (every refusal commits a stats +// write like a miss); what it did not pin is the work done BEFORE the +// refusal. A miss went straight to the stats write, while every existing key +// was fully JSON-decoded — its multi-megabyte FullContent included — before +// the provenance class, expiry or the guard were looked at. A narrow token +// alternating a known key against a nonexistent one therefore measured an +// O(payload) decode on one and not the other: same body, different class. +// +// The gate now decides on a fixed-size frame header (version, producer, +// expiry, size) that MarshalBinary writes in front of the record; the record +// is decoded only after admission. This test pins that structurally, through +// the heap: the bytes a refused read allocates must not grow with the entry's +// payload, for every refusing path — the non-evicting ones (scope, admin +// snapshot, internal, expired) and the invalidating ones (legacy provenance, +// undecodable). A positive control proves the meter sees a decode: an +// ADMITTED read of the same big entry allocates at least the payload. +func TestGetRecordsAs_RefusalIsPayloadSizeIndependent(t *testing.T) { + broad := Authorization{CallerKind: CallerKindAgent, Principal: "broad", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read"}} + narrow := Authorization{CallerKind: CallerKindAgent, Principal: "narrow", + AllowedServers: []string{"weather"}, Permissions: []string{"read"}} + admin := Authorization{CallerKind: CallerKindAdmin} + + const bigPayload = 4 << 20 + payload := func(n int) string { return `[{"v":"` + strings.Repeat("x", n) + `"}]` } + sizes := []struct { + name string + n int + }{{"1KB", 1 << 10}, {"4MB", bigPayload}} + + // Every refusing path, seeded into a fresh database so the eviction of + // one entry never rewrites the payload of a sibling. + variants := []struct { + name string + seed func(t *testing.T, m *Manager, db *bbolt.DB, key, body string) + want error + }{ + {"scope refusal (broader agent snapshot)", func(t *testing.T, m *Manager, _ *bbolt.DB, key, body string) { + if err := m.StoreAs(key, "t", nil, body, "", 1, broad); err != nil { + t.Fatal(err) + } + }, ErrUnauthorizedRead}, + {"administrator snapshot", func(t *testing.T, m *Manager, _ *bbolt.DB, key, body string) { + if err := m.StoreAs(key, "t", nil, body, "", 1, admin); err != nil { + t.Fatal(err) + } + }, ErrUnauthorizedRead}, + {"internal entry", func(t *testing.T, m *Manager, _ *bbolt.DB, key, body string) { + if err := m.StoreAs(key, "t", nil, body, "", 1, Authorization{CallerKind: CallerKindInternal}); err != nil { + t.Fatal(err) + } + }, ErrInternalEntry}, + {"legacy provenance (framed, unstamped), evicted", func(t *testing.T, _ *Manager, db *bbolt.DB, key, body string) { + now := time.Now() + rec := &Record{Key: key, ToolName: "t", Timestamp: now, FullContent: body, TotalSize: len(body), + ExpiresAt: now.Add(time.Hour), CreatedAt: now, LastAccessed: now} + data, err := rec.MarshalBinary() + if err != nil { + t.Fatal(err) + } + if err := db.Update(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheBucket)).Put([]byte(key), data) + }); err != nil { + t.Fatal(err) + } + }, ErrLegacyProvenance}, + {"undecodable record, evicted", func(t *testing.T, _ *Manager, db *bbolt.DB, key, body string) { + if err := db.Update(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheBucket)).Put([]byte(key), []byte("\xff not a record "+body)) + }); err != nil { + t.Fatal(err) + } + }, ErrLegacyProvenance}, + {"expired entry, left for the sweep", func(t *testing.T, m *Manager, db *bbolt.DB, key, body string) { + if err := m.StoreAs(key, "t", nil, body, "", 1, broad); err != nil { + t.Fatal(err) + } + expireEntry(t, db, key) + }, ErrKeyExpired}, + // Codex round 5: the two refusals that still did work proportional + // to hidden state. A same-kind scope refusal against a producer + // naming 5,000 servers, probed COLD (the manager reopened, so + // nothing decoded at store time can help), and a pre-frame legacy + // entry, which used to be decoded to recover its size. Both must + // sit in the miss's class on the first probe — there is no + // warm-up and no one-shot exception (research D16). + {"scope refusal against a 5,000-server producer, cold manager", func(t *testing.T, m *Manager, _ *bbolt.DB, key, body string) { + if err := m.StoreAs(key, "t", nil, body, "", 1, fleetSnapshot(5000, 112)); err != nil { + t.Fatal(err) + } + }, ErrUnauthorizedRead}, + {"legacy provenance (pre-frame bare JSON), evicted", func(t *testing.T, _ *Manager, db *bbolt.DB, key, body string) { + now := time.Now() + putRawRecord(t, db, key, map[string]interface{}{ + "key": key, "tool_name": "t", "timestamp": now, "full_content": body, + "total_size": len(body), "expires_at": now.Add(time.Hour), "created_at": now, "last_accessed": now, + }) + }, ErrLegacyProvenance}, + } + + for _, v := range variants { + for _, sz := range sizes { + t.Run(v.name+"/"+sz.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + // A small live neighbour so the key is never alone in its leaf. + if err := m.StoreAs("neighbour", "t", nil, payload(64), "", 1, broad); err != nil { + t.Fatal(err) + } + v.seed(t, m, db, "target", payload(sz.n)) + // Every variant is probed through a REOPENED manager: the + // verdict must come from the fixed header on disk, never + // from anything the store path left in memory. + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + m, db = openManagerAt(t, path) + defer db.Close() + defer m.Close() + + allocated, resp, err := allocatedBy(func() (*ReadCacheResponse, error) { + return m.GetRecordsAs("target", 0, 10, narrow) + }) + if !errors.Is(err, v.want) || resp != nil { + t.Fatalf("premise: expected refusal %v, got resp=%v err=%v", v.want, resp != nil, err) + } + if allocated > refusalAllocBudget { + t.Fatalf("refusing a %s entry allocated %d bytes (budget %d): the payload was decoded before the refusal — a timing class a nonexistent key does not share", sz.name, allocated, refusalAllocBudget) + } + }) + } + } + + t.Run("miss", func(t *testing.T) { + m, db := openManagerAt(t, filepath.Join(t.TempDir(), "cache.db")) + defer db.Close() + defer m.Close() + if err := m.StoreAs("neighbour", "t", nil, payload(bigPayload), "", 1, broad); err != nil { + t.Fatal(err) + } + allocated, _, err := allocatedBy(func() (*ReadCacheResponse, error) { + return m.GetRecordsAs("absent", 0, 10, narrow) + }) + if !errors.Is(err, ErrKeyNotFound) { + t.Fatalf("premise: %v", err) + } + if allocated > refusalAllocBudget { + t.Fatalf("a miss next to a 4 MB neighbour allocated %d bytes (budget %d)", allocated, refusalAllocBudget) + } + }) + + t.Run("positive control: an admitted read decodes the payload", func(t *testing.T) { + m, db := openManagerAt(t, filepath.Join(t.TempDir(), "cache.db")) + defer db.Close() + defer m.Close() + if err := m.StoreAs("target", "t", nil, payload(bigPayload), "", 1, broad); err != nil { + t.Fatal(err) + } + allocated, resp, err := allocatedBy(func() (*ReadCacheResponse, error) { + return m.GetRecordsAs("target", 0, 10, broad) + }) + if err != nil || resp == nil || len(resp.Records) != 1 { + t.Fatalf("premise: the producer reads its own entry: resp=%+v err=%v", resp, err) + } + if allocated < bigPayload { + t.Fatalf("meter blind: an admitted read of a 4 MB entry allocated only %d bytes", allocated) + } + }) +} + +// allocatedBy returns the heap bytes fn allocated (runtime.MemStats.TotalAlloc +// is cumulative and never decremented by GC, so the delta is exactly the +// allocation volume of the call; nothing else allocates in this package's +// tests, which do not run in parallel). +func allocatedBy(fn func() (*ReadCacheResponse, error)) (uint64, *ReadCacheResponse, error) { + var before, after runtime.MemStats + runtime.GC() + runtime.ReadMemStats(&before) + resp, err := fn() + runtime.ReadMemStats(&after) + return after.TotalAlloc - before.TotalAlloc, resp, err +} + +// expireEntry rewrites the entry's expiry into the past in place, the way the +// package's expiry tests and the server's read_cache scope tests do. +func expireEntry(t *testing.T, db *bbolt.DB, key string) { + t.Helper() + if err := db.Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(CacheBucket)) + var rec Record + if err := rec.UnmarshalBinary(bucket.Get([]byte(key))); err != nil { + return err + } + rec.ExpiresAt = time.Now().Add(-time.Hour) + data, err := rec.MarshalBinary() + if err != nil { + return err + } + return bucket.Put([]byte(key), data) + }); err != nil { + t.Fatal(err) + } +} + +// The same finding, on the commit shape: TestGetRecordsAs_RefusalCommitsLikeAMiss +// pins the non-evicting refusals (scope, admin snapshot, internal, and — now +// that the gated door leaves expired entries to the cleanup sweep — expired) +// to a miss's bbolt write count. The two invalidating refusals (legacy +// provenance, undecodable) cannot write exactly what a miss writes: FR-002 +// requires the legacy entry durably invalidated by the refusal itself, not +// by a later sweep. Their extra work is pinned as BOUNDED instead: the +// delete rewrites the leaf node minus the entry and hands the value's pages +// to the freelist by id range, so the page-write count is the same for a +// 1 KB and a 4 MB payload and exceeds a miss's by a constant that does not +// depend on the entry (the cache leaf, and the freelist page it dirties). +// It is also one-shot: the entry is gone, so a second probe is a plain miss. +func TestGetRecordsAs_EvictingRefusalWritesArePayloadIndependent(t *testing.T) { + broad := Authorization{CallerKind: CallerKindAgent, Principal: "broad", + AllowedServers: []string{"github"}, Permissions: []string{"read"}} + narrow := Authorization{CallerKind: CallerKindAgent, Principal: "narrow", + AllowedServers: []string{"weather"}, Permissions: []string{"read"}} + payload := func(n int) string { return `[{"v":"` + strings.Repeat("x", n) + `"}]` } + + seeds := map[string]func(t *testing.T, m *Manager, db *bbolt.DB, key, body string){ + "legacy": func(t *testing.T, _ *Manager, db *bbolt.DB, key, body string) { + now := time.Now() + putRawRecord(t, db, key, map[string]interface{}{"key": key, "tool_name": "t", "timestamp": now, + "full_content": body, "total_size": len(body), "expires_at": now.Add(time.Hour), "created_at": now, "last_accessed": now}) + }, + "undecodable": func(t *testing.T, _ *Manager, db *bbolt.DB, key, body string) { + if err := db.Update(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheBucket)).Put([]byte(key), []byte("\xff"+body)) + }); err != nil { + t.Fatal(err) + } + }, + } + + // writesFor seeds one fresh database and returns the bbolt page writes + // the refused read performed, and those of a miss on the same database. + writesFor := func(t *testing.T, seed func(*testing.T, *Manager, *bbolt.DB, string, string), n int) (refusal, miss int64) { + t.Helper() + m, db := openManagerAt(t, filepath.Join(t.TempDir(), "cache.db")) + defer db.Close() + defer m.Close() + if err := m.StoreAs("neighbour", "t", nil, payload(64), "", 1, broad); err != nil { + t.Fatal(err) + } + seed(t, m, db, "target", payload(n)) + before := db.Stats().TxStats.Write + if _, err := m.GetRecordsAs("absent", 0, 10, narrow); !errors.Is(err, ErrKeyNotFound) { + t.Fatalf("premise: %v", err) + } + miss = db.Stats().TxStats.Write - before + before = db.Stats().TxStats.Write + if resp, err := m.GetRecordsAs("target", 0, 10, narrow); err == nil || resp != nil { + t.Fatalf("premise: expected a refusal, got resp=%+v err=%v", resp, err) + } + refusal = db.Stats().TxStats.Write - before + if _, ok := m.Peek("target"); ok { + t.Fatal("premise: an evicting refusal removes the entry") + } + return refusal, miss + } + + // One committed delete of a single-page leaf: the cache leaf page and + // the meta/freelist pages a miss already dirties. Anything beyond that + // is a rebalance or a payload-sized rewrite, neither of which a refusal + // may perform. + const evictionWriteBudget = 2 + for name, seed := range seeds { + t.Run(name, func(t *testing.T) { + smallRefusal, smallMiss := writesFor(t, seed, 1<<10) + bigRefusal, bigMiss := writesFor(t, seed, 4<<20) + if smallMiss != bigMiss { + t.Fatalf("premise: a miss writes the same pages whatever its neighbours hold: %d vs %d", smallMiss, bigMiss) + } + if smallRefusal != bigRefusal { + t.Fatalf("evicting a 1 KB entry wrote %d pages, a 4 MB entry %d: the eviction is payload-proportional", smallRefusal, bigRefusal) + } + if extra := bigRefusal - bigMiss; extra > evictionWriteBudget { + t.Fatalf("evicting refusal wrote %d pages, a miss %d: %d extra exceeds the one-leaf budget of %d (%s)", bigRefusal, bigMiss, extra, evictionWriteBudget, fmt.Sprint(name)) + } + }) + } +} + +// putRawRecord's JSON-only fixture and MarshalBinary's framed output must +// both decode to the same Record: the frame is invisible to every reader +// that goes through UnmarshalBinary (Peek, Get, cleanup, Invalidate), and a +// record a pre-feature binary wrote — raw JSON, no frame — still decodes. +func TestRecord_BinaryRoundTripAcceptsFramedAndRawJSON(t *testing.T) { + now := time.Now().Round(0) + rec := &Record{Key: "k", ToolName: "t", FullContent: `[1]`, TotalSize: 3, ExpiresAt: now.Add(time.Hour), + CreatedAt: now, LastAccessed: now, Timestamp: now, Version: RecordVersion, + Producer: &Authorization{CallerKind: CallerKindAgent, Principal: "p", AllowedServers: []string{"a"}, Permissions: []string{"read"}}} + framed, err := rec.MarshalBinary() + if err != nil { + t.Fatal(err) + } + raw, err := json.Marshal(rec) + if err != nil { + t.Fatal(err) + } + if string(framed) == string(raw) { + t.Fatal("MarshalBinary must frame the record so the gate can read its header without the payload") + } + for name, data := range map[string][]byte{"framed": framed, "raw": raw} { + var got Record + if err := got.UnmarshalBinary(data); err != nil { + t.Fatalf("%s: %v", name, err) + } + if got.Key != "k" || got.FullContent != `[1]` || !got.HasCurrentProvenance() || got.Producer.Principal != "p" { + t.Fatalf("%s: round trip lost fields: %+v", name, got) + } + } +} + +// Codex round 4 (cache finding 2, server finding 1) put the producer snapshot +// behind a content hash and an in-memory LRU, so a refusal was O(1) once the +// snapshot was WARM — and codex round 5 pointed at the word "warm": after a +// restart, or once 128 other snapshots had evicted the target, the first +// same-kind refusal of a key stamped with a 5,000-server snapshot read and +// JSON-decoded that snapshot from the bucket, while a nonexistent key +// decoded nothing. Snapshot size is unbounded, so the difference could be +// made arbitrarily large. The door now decides on the fixed header alone — +// the reader's effective-authorization digest against the producer's, plus +// the kind, deny-all and tier bits (research D16) — and never loads a +// snapshot, so the FIRST probe on a cold manager must already sit in the +// miss's allocation class, and stay there: no warm-up, no positive control +// for a cold load. +func TestGetRecordsAs_RefusalIsSnapshotSizeIndependent(t *testing.T) { + wide := fleetSnapshot(5000, 112) + small := Authorization{CallerKind: CallerKindAgent, Principal: "small", AllowedServers: []string{"a", "b"}, Permissions: []string{"read"}} + // Same kind, not deny-all, not unrestricted and not the producer: the + // refusal is the digest comparison, the shape that used to load. + narrow := Authorization{CallerKind: CallerKindAgent, Principal: "narrow", AllowedServers: []string{"zzz"}, Permissions: []string{"read"}} + + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + for key, p := range map[string]Authorization{"wide": wide, "small": small} { + if err := m.StoreAs(key, "t", nil, `[{"v":1}]`, "", 1, p); err != nil { + t.Fatal(err) + } + } + m.Close() + if err := db.Close(); err != nil { + t.Fatal(err) + } + // Reopen: whatever the store path held in memory is gone. + m, db = openManagerAt(t, path) + defer db.Close() + defer m.Close() + + probe := func(key string, want error) uint64 { + t.Helper() + allocated, resp, err := allocatedBy(func() (*ReadCacheResponse, error) { + return m.GetRecordsAs(key, 0, 10, narrow) + }) + if !errors.Is(err, want) || resp != nil { + t.Fatalf("%s: resp=%v err=%v, want %v", key, resp != nil, err, want) + } + return allocated + } + // Room for the reader's own digest, the guard, the error path and + // bbolt's commit noise (a freelist rewrite shows up as a ~16 KiB step); + // still ~18x below the snapshot a load would betray. + const constant = 64 << 10 + coldMiss := probe("absent", ErrKeyNotFound) + if cold := probe("wide", ErrUnauthorizedRead); cold > coldMiss+constant { + t.Fatalf("the COLD first refusal against a %d-byte snapshot allocated %d bytes, a miss %d: the snapshot was loaded before the refusal — a timing class a nonexistent key does not share", len(snapshotBytes(wide)), cold, coldMiss) + } + + // Interleaved rounds, minimum per key: bbolt's commit-time allocations + // vary by a few pages between commits and under the race detector, and + // the interleaving spreads that noise over all three keys alike. + const rounds = 12 + wideMin, smallMin, miss := ^uint64(0), ^uint64(0), ^uint64(0) + for i := 0; i < rounds; i++ { + wideMin = min(wideMin, probe("wide", ErrUnauthorizedRead)) + smallMin = min(smallMin, probe("small", ErrUnauthorizedRead)) + miss = min(miss, probe("absent", ErrKeyNotFound)) + } + t.Logf("refusal: wide=%d small=%d; miss=%d bytes", wideMin, smallMin, miss) + if wideMin > smallMin+constant { + t.Fatalf("a refusal against a 5,000-server snapshot allocated %d bytes, against a 2-server one %d: the refusal still scales with the snapshot", wideMin, smallMin) + } + if wideMin > miss+constant { + t.Fatalf("a refusal allocated %d bytes, a miss %d: not the same class", wideMin, miss) + } +} diff --git a/internal/cache/manager_refusal_commit_test.go b/internal/cache/manager_refusal_commit_test.go new file mode 100644 index 000000000..04b9e663f --- /dev/null +++ b/internal/cache/manager_refusal_commit_test.go @@ -0,0 +1,213 @@ +package cache + +import ( + "errors" + "path/filepath" + "testing" + "time" + + "go.etcd.io/bbolt" + "go.uber.org/zap" +) + +// Spec 105 FR-001 / Definitions "non-disclosing refusal" (critique round 1, +// finding 1): a scoped caller's refusal must match a miss in status, body AND +// timing class. Body parity was pinned by T025; the timing class was not. An +// absent key commits a stats write (one bbolt fsync, ~10 ms on a laptop) while +// a guard refusal returned an error from the Update closure — bbolt rolled +// the transaction back and wrote nothing (~5 µs). Three orders of magnitude, +// stable and repeatable, so one probe told a narrow token whether a live +// entry (a broader token's page, or a guessable internal registry/guesser +// key) sat behind the key. +// +// Timing itself is not asserted — that would flake in CI. The MECHANISM is: +// every refused path must take the same committing branch a miss takes, +// proven through bbolt's own write counter, and must count as a miss in the +// stats (the same signal an absent key leaves). +func TestGetRecordsAs_RefusalCommitsLikeAMiss(t *testing.T) { + db, err := bbolt.Open(filepath.Join(t.TempDir(), "cache.db"), 0644, &bbolt.Options{Timeout: time.Second}) + if err != nil { + t.Fatal(err) + } + defer db.Close() + m, err := NewManager(db, zap.NewNop()) + if err != nil { + t.Fatal(err) + } + defer m.Close() + + broad := Authorization{CallerKind: CallerKindAgent, Principal: "broad", + AllowedServers: []string{"github", "weather"}, Permissions: []string{"read"}} + narrow := Authorization{CallerKind: CallerKindAgent, Principal: "narrow", + AllowedServers: []string{"weather"}, Permissions: []string{"read"}} + if err := m.StoreAs("broad-entry", "t", nil, `[1]`, "", 1, broad); err != nil { + t.Fatal(err) + } + if err := m.StoreAs("admin-entry", "t", nil, `[1]`, "", 1, Authorization{CallerKind: CallerKindAdmin}); err != nil { + t.Fatal(err) + } + if err := m.StoreAs("npm:@acme/mcp-server", "repo_guess", nil, `[1]`, "", 1, Authorization{CallerKind: CallerKindInternal}); err != nil { + t.Fatal(err) + } + // Codex round 2, finding 3: an expired entry used to be evicted by the + // gated read that found it — a leaf rewrite a miss never performs. It is + // now refused like a miss and left to the cleanup sweep. + if err := m.StoreAs("expired-entry", "t", nil, `[1]`, "", 1, broad); err != nil { + t.Fatal(err) + } + expireEntry(t, db, "expired-entry") + + // writesFor returns how many bbolt page writes the read performed — + // zero means the transaction was rolled back, never fsynced. + writesFor := func(key string) int64 { + t.Helper() + before := db.Stats().TxStats.Write + resp, err := m.GetRecordsAs(key, 0, 10, narrow) + if err == nil || resp != nil { + t.Fatalf("%s: expected a refusal, got resp=%+v err=%v", key, resp, err) + } + return db.Stats().TxStats.Write - before + } + + missBefore := m.GetStats().MissCount + absent := writesFor("no-such-key") + if absent == 0 { + t.Fatal("premise: a miss commits a stats write") + } + for _, key := range []string{"broad-entry", "admin-entry", "npm:@acme/mcp-server", "expired-entry"} { + if got := writesFor(key); got != absent { + t.Errorf("%s: refusal performed %d bbolt writes, a miss performs %d — different timing class (a rolled-back refusal is a ~3000x timing oracle; an evicting one rewrites a leaf)", key, got, absent) + } + } + if got, want := m.GetStats().MissCount, missBefore+5; got != want { + t.Errorf("MissCount = %d, want %d: every refusal must count as a miss, exactly as an absent key does", got, want) + } + + // The refused entries are untouched: no hit, no access bump, no eviction + // (the expired one waits for the sweep). + for _, key := range []string{"broad-entry", "admin-entry", "npm:@acme/mcp-server", "expired-entry"} { + rec, ok := m.Peek(key) + if !ok { + t.Fatalf("%s: a refused read must not evict", key) + } + if rec.AccessCount != 0 { + t.Errorf("%s: a refused read must not count as an access: %+v", key, rec) + } + } + if m.GetStats().HitCount != 0 { + t.Errorf("HitCount = %d, want 0", m.GetStats().HitCount) + } +} + +// Critique round 1, finding 3: the doc comment on getGuarded promised that +// m.stats is mutated only on committing paths, but the expiry and legacy +// branches decremented the counters BEFORE saveStats ran. When the stats +// write fails bbolt rolls the delete back while the in-memory counters keep +// the decrement — the on-disk == in-memory invariant T023 pins held only on +// the happy path. A failed write must leave the counters agreeing with the +// bucket. +func TestGetRecordsAs_FailedCommitRestoresStats(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + defer db.Close() + defer m.Close() + + if err := m.Store("legacy", "t", nil, `[1]`, "", 1); err != nil { + t.Fatal(err) + } + before := *m.GetStats() + if before.TotalEntries != 1 { + t.Fatalf("premise: stats count the stored entry: %+v", before) + } + + // Make the stats write fail: drop the stats bucket. The read must then + // surface a storage error (not a verdict) and change nothing. + if err := db.Update(func(tx *bbolt.Tx) error { + return tx.DeleteBucket([]byte(CacheStatsBucket)) + }); err != nil { + t.Fatal(err) + } + + var readErr error + func() { + defer func() { + if r := recover(); r != nil { + readErr = errors.New("getGuarded panicked on a missing stats bucket") + } + }() + _, readErr = m.GetRecordsAs("legacy", 0, 10, Authorization{CallerKind: CallerKindAdmin}) + }() + if readErr == nil || errors.Is(readErr, ErrUnauthorizedRead) || errors.Is(readErr, ErrKeyNotFound) { + t.Fatalf("a failed stats write must surface as a storage error, got %v", readErr) + } + + onDisk := onDiskEntryCount(t, db) + if onDisk != 1 { + t.Fatalf("the rolled-back transaction must leave the entry on disk, count=%d", onDisk) + } + after := *m.GetStats() + if after != before { + t.Fatalf("in-memory stats mutated by a rolled-back transaction: before=%+v after=%+v (bucket holds %d)", before, after, onDisk) + } +} + +// Codex round 2, finding 2: the round-1 restore covered a closure that +// FAILED. bbolt can also fail AFTER the closure succeeded — at commit (page +// write, file grow, fsync: disk full), in which case db.Update rolls the +// transaction back and returns the commit error. The in-memory counters had +// already been mutated inside the closure and were never restored: the +// record stayed on disk while TotalEntries/TotalSizeBytes were decremented +// and EvictedCount incremented. The invariant is "stats mutate only once +// Update returned nil", whatever stage failed. +func TestGetRecordsAs_FailedCommitAfterCallbackRestoresStats(t *testing.T) { + path := filepath.Join(t.TempDir(), "cache.db") + m, db := openManagerAt(t, path) + defer db.Close() + defer m.Close() + + if err := m.Store("legacy", "t", nil, `[1]`, "", 1); err != nil { + t.Fatal(err) + } + if err := m.StoreAs("stamped", "t", nil, `[1]`, "", 1, Authorization{CallerKind: CallerKindAdmin}); err != nil { + t.Fatal(err) + } + before := *m.GetStats() + if before.TotalEntries != 2 { + t.Fatalf("premise: stats count the stored entries: %+v", before) + } + + // Commit failure: the closure runs to completion against a real + // transaction, then the transaction is rolled back and the commit error + // surfaces — exactly what bbolt does when the fsync fails. + errCommit := errors.New("injected: commit failed (disk full)") + m.dbUpdate = func(fn func(tx *bbolt.Tx) error) error { + return db.Update(func(tx *bbolt.Tx) error { + if err := fn(tx); err != nil { + return err + } + return errCommit + }) + } + + // Every gated outcome that mutates counters: the evicting legacy + // refusal, the miss on an absent key, and a hit. + for _, tc := range []struct { + name string + key string + }{ + {"evicting legacy refusal", "legacy"}, + {"miss", "absent"}, + {"hit", "stamped"}, + } { + _, err := m.GetRecordsAs(tc.key, 0, 10, Authorization{CallerKind: CallerKindAdmin}) + if !errors.Is(err, errCommit) { + t.Fatalf("%s: a failed commit must surface as the storage error, got %v", tc.name, err) + } + if got := onDiskEntryCount(t, db); got != 2 { + t.Fatalf("%s: the rolled-back transaction must leave both entries on disk, count=%d", tc.name, got) + } + if after := *m.GetStats(); after != before { + t.Fatalf("%s: in-memory stats mutated by a transaction whose commit failed: before=%+v after=%+v", tc.name, before, after) + } + } +} diff --git a/internal/cache/manager_test.go b/internal/cache/manager_test.go index 14d459662..49da45bcb 100644 --- a/internal/cache/manager_test.go +++ b/internal/cache/manager_test.go @@ -320,6 +320,40 @@ func TestExpiredRecords(t *testing.T) { if err == nil { t.Error("Expected error for expired record") } + + // Spec 105 FR-002 durable invalidation (gap FR001-G3): the expiry branch + // deletes the record inside the read transaction and then returns a + // non-nil error from the same Update closure, which makes bbolt ROLL BACK + // the delete — the entry stays on disk while the in-memory stats already + // count it as evicted. An expired entry must be absent after the miss and + // the stats must agree with the bucket. + if rec, ok := manager.Peek(key); ok { + t.Fatalf("expired record still on disk after the expiring Get (delete rolled back): %+v", rec) + } + onDisk := 0 + if err := db.View(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(CacheBucket)).ForEach(func(_, _ []byte) error { + onDisk++ + return nil + }) + }); err != nil { + t.Fatal(err) + } + stats := manager.GetStats() + if stats.TotalEntries != onDisk { + t.Fatalf("stats.TotalEntries=%d but the bucket holds %d entries: stats drifted from the committed state", stats.TotalEntries, onDisk) + } + if stats.EvictedCount != 1 { + t.Fatalf("stats.EvictedCount=%d, want 1 (the committed eviction)", stats.EvictedCount) + } + // The stored stats must reflect the same commit: reload them from disk. + reloaded := &Manager{db: db, logger: logger, stats: &Stats{}} + if err := reloaded.loadStats(); err != nil { + t.Fatal(err) + } + if reloaded.stats.TotalEntries != onDisk || reloaded.stats.EvictedCount != 1 { + t.Fatalf("persisted stats %+v disagree with the bucket (%d entries): the eviction was not committed", *reloaded.stats, onDisk) + } } func TestCleanup(t *testing.T) { diff --git a/internal/cache/models.go b/internal/cache/models.go index f78f33895..ca01b0106 100644 --- a/internal/cache/models.go +++ b/internal/cache/models.go @@ -1,7 +1,13 @@ package cache import ( + "bytes" + "crypto/sha256" + "encoding/binary" "encoding/json" + "errors" + "fmt" + "math" "time" ) @@ -21,9 +27,245 @@ type Record struct { CreatedAt time.Time `json:"created_at"` // Producer is the authorization the entry was produced under (Spec 104 // FR-016a). nil on entries persisted before stamping existed or written - // through Store by internal callers; those are readable only by - // unrestricted callers. + // through Store; together with Version it decides the entry's provenance + // class (see HasCurrentProvenance). Producer *Authorization `json:"producer,omitempty"` + // Version is the provenance schema the entry was written under (Spec 105 + // FR-002). 0/absent marks a record persisted before this field existed; + // any value other than RecordVersion is provenance this binary does not + // recognise. Both are legacy: refused for every caller and invalidated on + // the first gated read. + Version uint8 `json:"version,omitempty"` +} + +// RecordVersion is the provenance schema current binaries stamp on every +// record they write. Bump it only when the meaning of Producer changes in a +// way older readers must not trust — a bump makes every existing entry legacy. +const RecordVersion uint8 = 1 + +// HasCurrentProvenance reports whether the record carries a producer stamp +// written under the current provenance schema. Anything else — no producer, +// no version, a version this binary does not know, a caller kind it does not +// know — is legacy provenance (Spec 105 FR-002): the gated read refuses it +// for every caller kind and invalidates it. The kind is checked structurally +// rather than trusting the version alone: a later binary that adds a kind +// without bumping RecordVersion, followed by a rollback, must not leave an +// entry the administrator gate would wave through (it accepts any +// non-internal kind for an administrator reader). +func (c *Record) HasCurrentProvenance() bool { + return c.header().HasCurrentProvenance() +} + +// recordHeader is the FIXED-SIZE, payload-free part of a stored record: +// everything the gated read needs to decide — provenance class (version, +// caller kind), the deny-all bit, the producer's permission tier bits, expiry, +// the size the eviction stats fold out, and the DIGEST of the producer's +// effective authorization. MarshalBinary writes it in front of the record +// body so the gate can decode it alone (decodeRecordHeader) and decide every +// verdict from it in O(1) (research D16): a refusal must not do work +// proportional to the payload it refuses, nor to the producer snapshot (an +// authorization naming thousands of servers) — a nonexistent key does +// neither, and a non-disclosing refusal is indistinguishable from it in +// timing class (Spec 105 Definitions; codex rounds 2, 4 and 5). The snapshot +// itself is kept once per distinct authorization in the snapshots bucket, +// keyed by the same digest, for administrator diagnostics only: no read path +// consults it. The header is derived from the Record at marshal time, so the +// two never disagree on a record this binary wrote — and UnmarshalBinary +// refuses a value on which they do. +type recordHeader struct { + Version uint8 + KindCode uint8 + // Kind is the caller kind KindCode names; "" for code 0 (no producer) + // and for a code this binary does not know. + Kind string + // DenyAll is Producer.DenyAll() at marshal time: a scoped snapshot that + // could have authorized nothing, refused for scoped readers on the + // header alone. + DenyAll bool + // Perms is permissionBits(Producer.Permissions): the tier set an + // unrestricted reader must cover. + Perms uint8 + ExpiresAt time.Time + TotalSize int + // Digest is Producer.digest(): the content address of the producer's + // effective authorization; zero for an unstamped record. + Digest [sha256.Size]byte +} + +func (c *Record) header() recordHeader { + h := recordHeader{Version: c.Version, ExpiresAt: c.ExpiresAt, TotalSize: c.TotalSize} + if c.Producer != nil { + h.KindCode = callerKindCode(c.Producer.CallerKind) + h.Kind = callerKindFromCode(h.KindCode) + h.DenyAll = c.Producer.DenyAll() + h.Perms = permissionBits(c.Producer.Permissions) + h.Digest = c.Producer.digest() + } + return h +} + +// facts is the producer side of the read gate as this header carries it. +func (h recordHeader) facts() producerFacts { + return producerFacts{Kind: h.Kind, DenyAll: h.DenyAll, Perms: h.Perms, Digest: h.Digest} +} + +// snapshotBytes is the diagnostic encoding of a producer snapshot: the JSON +// of the Authorization as the request carried it (profile name and list +// order included). It is what the snapshots bucket stores under the +// producer's digest, for an administrator to inspect; the gate never reads +// it. It is never bounded: any authorization the proxy can mint fits. +func snapshotBytes(a Authorization) []byte { + data, err := json.Marshal(a) + if err != nil { + // Authorization is strings, string slices and a bool: json.Marshal + // cannot fail on it. + panic(fmt.Sprintf("cache: marshal authorization snapshot: %v", err)) + } + return data +} + +// HasCurrentProvenance is Record.HasCurrentProvenance decided on the header. +func (h recordHeader) HasCurrentProvenance() bool { + return h.KindCode != 0 && h.Version == RecordVersion && IsKnownCallerKind(h.Kind) +} + +func (h recordHeader) expired() bool { + return time.Now().After(h.ExpiresAt) +} + +// Stored value layout, written by MarshalBinary: +// +// recordFrameMagic | fixed header (recordHeaderSize bytes) | record JSON +// +// Fixed header, big-endian: +// +// [0] version +// [1] caller kind code (callerKindCodes; 0 = no producer) +// [2] flags (recordFlagDenyAll; every other bit reserved, must be 0) +// [3] producer permission tier bits (permBitKnown; other bits must be 0) +// [4:12] expires_at, Unix nanoseconds +// [12:20] total_size +// [20:52] producer effective-authorization digest (Authorization.digest) +// +// The magic starts with a NUL byte, which no JSON document does, so a value +// without it is a record a pre-frame binary wrote as bare JSON: UnmarshalBinary +// still decodes it (the ungated readers and the cleanup sweep keep working +// across the upgrade), while the gated read treats the missing header as the +// legacy provenance it is (Spec 105 FR-002). The magic's last byte is the +// frame layout version: a frame with another one is unrecognised provenance. +var recordFrameMagic = []byte("\x00mcpproxy-cache-record\x03") + +const ( + recordHeaderSize = 4 + 8 + 8 + sha256.Size + recordFlagDenyAll = 1 << 0 + recordHeaderOffVer = 0 + recordHeaderOffKnd = 1 + recordHeaderOffFlg = 2 + recordHeaderOffPrm = 3 + recordHeaderOffExp = 4 + recordHeaderOffSiz = 12 + recordHeaderOffDig = 20 +) + +var ( + errRecordUnframed = errors.New("cache record has no frame header (written before frame headers existed)") + errRecordFrameCorrupt = errors.New("cache record frame header is corrupt") + errRecordFrameMismatch = fmt.Errorf("%w: header disagrees with the record body", errRecordFrameCorrupt) +) + +// encodeExpiry is the header's encoding of an expiry instant. A zero time +// (never written by storeRecord) encodes as 0 — the Unix epoch, long expired +// — rather than the undefined UnixNano of the year 1. +func encodeExpiry(t time.Time) int64 { + if t.IsZero() { + return 0 + } + return t.UnixNano() +} + +func (h recordHeader) encode() []byte { + out := make([]byte, recordHeaderSize) + out[recordHeaderOffVer] = h.Version + out[recordHeaderOffKnd] = h.KindCode + if h.DenyAll { + out[recordHeaderOffFlg] |= recordFlagDenyAll + } + out[recordHeaderOffPrm] = h.Perms + binary.BigEndian.PutUint64(out[recordHeaderOffExp:], uint64(encodeExpiry(h.ExpiresAt))) + binary.BigEndian.PutUint64(out[recordHeaderOffSiz:], uint64(int64(h.TotalSize))) + copy(out[recordHeaderOffDig:], h.Digest[:]) + return out +} + +// decodeHeaderBytes decodes a fixed header. A flag or tier byte carrying a +// bit no MarshalBinary of this repository emits is a frame this binary did +// not write: corrupt, so the gate classifies it as unrecognised provenance +// here — before admission, in O(1) — rather than admitting on the digest +// and discovering the disagreement after a payload-sized body decode, or +// (an unknown flag bit) never at all (codex round 6). +func decodeHeaderBytes(raw []byte) (recordHeader, error) { + if len(raw) != recordHeaderSize { + return recordHeader{}, errRecordFrameCorrupt + } + if raw[recordHeaderOffFlg]&^recordFlagDenyAll != 0 || raw[recordHeaderOffPrm]&^permBitKnown != 0 { + return recordHeader{}, errRecordFrameCorrupt + } + h := recordHeader{ + Version: raw[recordHeaderOffVer], + KindCode: raw[recordHeaderOffKnd], + DenyAll: raw[recordHeaderOffFlg]&recordFlagDenyAll != 0, + Perms: raw[recordHeaderOffPrm], + } + h.Kind = callerKindFromCode(h.KindCode) + h.ExpiresAt = time.Unix(0, int64(binary.BigEndian.Uint64(raw[recordHeaderOffExp:]))) + size := int64(binary.BigEndian.Uint64(raw[recordHeaderOffSiz:])) + if size < 0 || size > math.MaxInt { + return recordHeader{}, errRecordFrameCorrupt + } + h.TotalSize = int(size) + copy(h.Digest[:], raw[recordHeaderOffDig:]) + return h, nil +} + +// encodeRecordFrame lays header and body out as MarshalBinary stores them. +func encodeRecordFrame(header, body []byte) []byte { + prefix := len(recordFrameMagic) + len(header) + capHint := prefix + if len(body) <= math.MaxInt-prefix { + capHint += len(body) + } + out := make([]byte, 0, capHint) + out = append(out, recordFrameMagic...) + out = append(out, header...) + return append(out, body...) +} + +// splitRecordFrame separates a stored value into its header and body bytes. +// It touches only the fixed-size prefix: the returned slices alias data. A +// value without the magic is unframed — the whole value is the body. +func splitRecordFrame(data []byte) (header, body []byte, err error) { + if !bytes.HasPrefix(data, recordFrameMagic) { + return nil, data, errRecordUnframed + } + rest := data[len(recordFrameMagic):] + if len(rest) < recordHeaderSize { + return nil, nil, errRecordFrameCorrupt + } + return rest[:recordHeaderSize], rest[recordHeaderSize:], nil +} + +// decodeRecordHeader decodes only the frame header of a stored value — a +// fixed number of bytes, never O(payload) and never O(snapshot). A value +// without a frame, or with a frame this binary cannot decode, is reported as +// an error with a zero header (TotalSize 0: the size of such a record is +// unknown without decoding it, which the gate must not do — the cleanup +// sweep reconciles the size statistics from the bucket instead). +func decodeRecordHeader(data []byte) (recordHeader, error) { + raw, _, err := splitRecordFrame(data) + if err != nil { + return recordHeader{}, err + } + return decodeHeaderBytes(raw) } // Stats represents cache statistics @@ -40,6 +282,13 @@ type Stats struct { type ReadCacheResponse struct { Records []interface{} `json:"records"` Meta Meta `json:"meta"` + // Producer is the authorization snapshot the paged entry was produced + // under. It never reaches the wire: the read_cache handler carries it to + // the store of a recursively re-truncated page so provenance stays + // monotone down the chain — a child page is stamped with its PARENT's + // snapshot, never with the (possibly broader) redeemer's (Spec 105 + // FR-001). + Producer *Authorization `json:"-"` } // Meta represents metadata about the cached response @@ -52,14 +301,79 @@ type Meta struct { RecordPath string `json:"record_path,omitempty"` } -// MarshalBinary implements encoding.BinaryMarshaler for Record +// MarshalBinary implements encoding.BinaryMarshaler for Record: the fixed +// frame header (derived from the record) followed by the record as JSON. +// There is no size bound: the header is fixed-size and names the producer +// by digest, so any authorization the proxy can mint fits (codex round 4). +// The diagnostic snapshot is persisted by the store path +// (Manager.storeRecord) in the same transaction — see marshalFrame. func (c *Record) MarshalBinary() ([]byte, error) { - return json.Marshal(c) + data, _, err := c.marshalFrame() + return data, err } -// UnmarshalBinary implements encoding.BinaryUnmarshaler for Record +// marshalFrame is MarshalBinary plus the diagnostic snapshot bytes for the +// producer the header names (nil for an unstamped record), so a store can +// persist both from one encoding. +func (c *Record) marshalFrame() (data, snapshot []byte, err error) { + body, err := json.Marshal(c) + if err != nil { + return nil, nil, err + } + if c.Producer != nil { + snapshot = snapshotBytes(*c.Producer) + } + return encodeRecordFrame(c.header().encode(), body), snapshot, nil +} + +// UnmarshalBinary implements encoding.BinaryUnmarshaler for Record. It +// accepts both the framed layout and the bare JSON a pre-frame binary wrote. +// The body carries every field; the header, when present, must AGREE with +// it — exactly, on every field it carries — or the value is corrupt. The +// gated read admits a reader on the header alone, so a header that promised +// a narrower producer, a later expiry or another size than the body it +// fronts would hand that reader a body the header never authorized (codex +// round 3, finding 1). No binary of this repository writes such a value +// (MarshalBinary derives the header from the record), so it is refused like +// any other undecodable frame: errRecordFrameMismatch, and the record is +// left zero — a caller never sees the body. func (c *Record) UnmarshalBinary(data []byte) error { - return json.Unmarshal(data, c) + header, body, err := splitRecordFrame(data) + if err != nil && !errors.Is(err, errRecordUnframed) { + return err + } + if err := json.Unmarshal(body, c); err != nil { + return err + } + if header == nil { + return nil + } + h, err := decodeHeaderBytes(header) + if err != nil { + *c = Record{} + return err + } + if !h.agreesWith(c.header()) { + *c = Record{} + return errRecordFrameMismatch + } + return nil +} + +// agreesWith reports whether two headers are equal field for field: the same +// version, kind, deny-all bit, tier bits, expiry instant and size, and the +// same producer digest — so every dimension the gate decides on (kind, +// principal, server grant, permissions, pin, profile scope and profile +// server set) must match; an unstamped record digests to zero and agrees +// only with an unstamped header. +func (h recordHeader) agreesWith(o recordHeader) bool { + return h.Version == o.Version && + h.KindCode == o.KindCode && + h.DenyAll == o.DenyAll && + h.Perms == o.Perms && + encodeExpiry(h.ExpiresAt) == encodeExpiry(o.ExpiresAt) && + h.TotalSize == o.TotalSize && + h.Digest == o.Digest } // MarshalBinary implements encoding.BinaryMarshaler for Stats diff --git a/internal/experiments/guesser.go b/internal/experiments/guesser.go index d04022f6d..e09a29f3d 100644 --- a/internal/experiments/guesser.go +++ b/internal/experiments/guesser.go @@ -345,11 +345,14 @@ func (g *Guesser) cacheInfo(cacheKey string, info *RepositoryInfo) { return } - // Cache for 6 hours - if err := g.cacheManager.Store(cacheKey, "repo_guess", map[string]interface{}{ + // Stamped internal (Spec 105 FR-002): read_cache refuses the entry for + // every caller without evicting it, and the ungated Get above keeps + // serving it. An unstamped entry would be legacy provenance — invalidated + // by the first read_cache probe of this guessable key. + if err := g.cacheManager.StoreAs(cacheKey, "repo_guess", map[string]interface{}{ "package_name": info.PackageName, "type": string(info.Type), - }, string(data), "", 1); err != nil { + }, string(data), "", 1, cache.Authorization{CallerKind: cache.CallerKindInternal}); err != nil { g.logger.Warn("Failed to cache repo info", zap.Error(err)) } } diff --git a/internal/experiments/guesser_cache_stamp_test.go b/internal/experiments/guesser_cache_stamp_test.go new file mode 100644 index 000000000..8928042c8 --- /dev/null +++ b/internal/experiments/guesser_cache_stamp_test.go @@ -0,0 +1,53 @@ +package experiments + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Spec 105 FR-002 (gap FR001-G2, task T029): the guesser is one of the two +// writers of INTERNAL cache entries (`npm:`). It must stamp every entry +// it writes with the internal caller kind so read_cache refuses the entry for +// every caller without evicting it — and its own read path, which goes +// through the ungated Get, must keep serving the stamped entry. +// +// The kind is spelled as its wire value so this test compiles against the +// pre-feature cache package; the constant lives in internal/cache. +const internalCallerKindWire = "internal" + +// failingTransport makes any real network attempt an error, so a cache hit is +// the only way checkNPMPackageWithClient can answer with Exists=true. +type failingTransport struct{} + +func (failingTransport) RoundTrip(*http.Request) (*http.Response, error) { + return nil, errors.New("network disabled in test") +} + +func TestGuesser_CacheEntriesAreStampedInternal(t *testing.T) { + guesser, db := setupTestGuesser(t) + defer db.Close() + + const pkg = "@acme/mcp-server-stamp-test" + const key = "npm:" + pkg + guesser.cacheInfo(key, &RepositoryInfo{ + Type: RepoTypeNPM, PackageName: pkg, Exists: true, Version: "1.2.3", + InstallCmd: "npm install " + pkg, + }) + + rec, ok := guesser.cacheManager.Peek(key) + require.True(t, ok, "the guesser writer must persist its entry") + require.NotNil(t, rec.Producer, "a guesser entry must carry a producer stamp: an unstamped entry is legacy provenance and would be evicted on the first read_cache probe") + assert.Equal(t, internalCallerKindWire, rec.Producer.CallerKind, "guesser entries are internal entries") + + // The internal reader is unaffected by the stamp: a cache hit answers + // without touching the network. + info := guesser.checkNPMPackageWithClient(context.Background(), pkg, &http.Client{Transport: failingTransport{}}) + require.NotNil(t, info) + assert.True(t, info.Exists, "the stamped entry must still be served to the guesser's own read path") + assert.Equal(t, "1.2.3", info.Version) +} diff --git a/internal/runtime/registry_cache_stamp_test.go b/internal/runtime/registry_cache_stamp_test.go new file mode 100644 index 000000000..eeeaedc1b --- /dev/null +++ b/internal/runtime/registry_cache_stamp_test.go @@ -0,0 +1,86 @@ +package runtime + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" +) + +// Spec 105 FR-002 (gap FR001-G2, task T029): the registry search path is one +// of the two writers of INTERNAL cache entries +// (`registry-servers::::`). It must stamp every entry +// it writes with the internal caller kind so read_cache refuses the entry for +// every caller — administrators included (SC-005 names this exception) — +// without evicting it; and its own reader (Peek, which serves a cached list +// while flagging its age) must keep finding the stamped entry. +// +// The kind is spelled as its wire value so this test compiles against the +// pre-feature cache package; the constant lives in internal/cache. +const internalCallerKindWire = "internal" + +func TestSearchRegistryServers_CacheEntryIsStampedInternal(t *testing.T) { + // A single-page official-protocol listing on loopback; the SSRF guard is + // relaxed through the same config flag an operator running a private + // mirror would set. + fetches := 0 + registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fetches++ + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "servers": []map[string]interface{}{{ + "server": map[string]interface{}{ + "name": "io.example/sentinel-registry-server", + "description": "SENTINEL-REGISTRY-ENTRY", + "remotes": []interface{}{ + map[string]interface{}{"type": "streamable-http", "url": "https://example.com/mcp"}, + }, + }, + }}, + "metadata": map[string]interface{}{}, + }) + })) + defer registry.Close() + + cfg := &config.Config{ + DataDir: t.TempDir(), + Listen: "127.0.0.1:0", + Registries: []config.RegistryEntry{{ + ID: "stamp-test", Name: "Stamp Test", URL: registry.URL, ServersURL: registry.URL, + Protocol: "modelcontextprotocol/registry", + }}, + AllowPrivateRegistryFetch: true, + } + rt, err := New(cfg, "", zap.NewNop()) + require.NoError(t, err) + t.Cleanup(func() { _ = rt.Close() }) + + servers, info, err := rt.SearchRegistryServers("stamp-test", "", "", 5) + require.NoError(t, err) + require.Len(t, servers, 1, "premise: the fake registry lists one server") + require.NotNil(t, info) + require.Zero(t, info.AgeSeconds, "premise: a fresh fetch reports age zero") + require.Equal(t, 1, fetches) + + key := fmt.Sprintf("registry-servers:%s:%s:%s:%d", "stamp-test", "", "", 5) + rec, ok := rt.CacheManager().Peek(key) + require.True(t, ok, "the registry writer must persist its list under %q", key) + require.NotNil(t, rec.Producer, "a registry entry must carry a producer stamp: an unstamped entry is legacy provenance and would be evicted on the first read_cache probe") + assert.Equal(t, internalCallerKindWire, rec.Producer.CallerKind, "registry entries are internal entries") + + // The internal reader is unaffected by the stamp: the second search is + // served from the stamped entry without a second fetch. + servers, info, err = rt.SearchRegistryServers("stamp-test", "", "", 5) + require.NoError(t, err) + require.Len(t, servers, 1) + require.NotNil(t, info, "the cached list must be served with its freshness info") + assert.False(t, info.Stale) + assert.Equal(t, 1, fetches, "the stamped entry must still be served to the registry's own read path") +} diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 3fe9d7e87..369d9df1a 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -2233,10 +2233,15 @@ func (r *Runtime) SearchRegistryServers(registryID, tag, query string, limit int } // Cache the freshly fetched list so subsequent searches surface its age. + // The entry is stamped internal (Spec 105 FR-002): read_cache refuses it + // for every caller without evicting it, and the Peek above keeps serving + // it. An unstamped entry would be legacy provenance — invalidated by the + // first read_cache probe of this guessable key. var cacheInfo *contracts.RegistryCacheInfo if r.cacheManager != nil { if data, mErr := json.Marshal(result); mErr == nil { - if sErr := r.cacheManager.Store(cacheKey, "registry-servers", nil, string(data), "", len(result)); sErr != nil { + if sErr := r.cacheManager.StoreAs(cacheKey, "registry-servers", nil, string(data), "", len(result), + cache.Authorization{CallerKind: cache.CallerKindInternal}); sErr != nil { r.logger.Warn("Failed to cache registry search", zap.Error(sErr)) } } diff --git a/internal/server/cache_authz.go b/internal/server/cache_authz.go index 4736c0e6b..326861e65 100644 --- a/internal/server/cache_authz.go +++ b/internal/server/cache_authz.go @@ -2,8 +2,12 @@ package server import ( "context" + "errors" + "fmt" "sort" + "github.com/mark3labs/mcp-go/mcp" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" "github.com/smart-mcp-proxy/mcpproxy-go/internal/cache" "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" @@ -41,8 +45,20 @@ func (p *MCPProxyServer) cacheAuthorizationWith(ctx context.Context, profileName a.Permissions = append([]string(nil), ac.Permissions...) a.ProfilePin = ac.ProfilePin case ac.Type == auth.AuthTypeUser: + // A server-edition user is bounded by the SAME dispatch gates + // as an agent token — CanAccessServer, HasPermission and the + // effective profile all apply to any non-admin context — so its + // snapshot carries the same dimensions and the read gate holds + // the user to them, identity included, through the header + // digest (codex round 4: a snapshot of the user id alone let a + // user narrowed to {b} redeem the {a} entry it produced + // earlier; research D16: digest equality is the only user + // admission). a.CallerKind = cache.CallerKindUser a.Principal = ac.UserID + a.AllowedServers = append([]string(nil), ac.AllowedServers...) + a.Permissions = append([]string(nil), ac.Permissions...) + a.ProfilePin = ac.ProfilePin case ac.Type == auth.AuthTypeAdminUser: a.CallerKind = cache.CallerKindAdminUser a.Principal = ac.UserID @@ -84,3 +100,53 @@ func (p *MCPProxyServer) cacheStoreAs(producer cache.Authorization) CacheStore { } return producerCacheStore{store: p.cacheManager, producer: producer} } + +// childPageProducer is the snapshot a recursively re-truncated read_cache +// page is stamped with: the parent entry's own producer (Spec 105 FR-001, +// monotone recursive provenance). GetRecordsAs refuses legacy provenance +// before a page exists, so a paged entry always carries a producer; the +// redeemer is the fallback only for a page that somehow arrives without one. +func childPageProducer(page *cache.ReadCacheResponse, redeemer cache.Authorization) cache.Authorization { + if page != nil && page.Producer != nil { + return *page.Producer + } + return redeemer +} + +// readCacheRefusal renders a failed gated read as the read_cache tool error. +// +// For a scoped caller — anything but an administrator kind — an entry that +// exists but was produced under an authorization the caller does not hold, +// an entry that expired, an internal entry and a key that never existed all +// answer with ONE body, the not-found one, so the refusal is not an existence +// oracle (Spec 105 FR-001 "refusal is non-disclosing", FR-010(1)). The body +// keeps the "cache key not found" substring agents already handle, and the +// cache commits every refusal the way it commits a miss, so the timing class +// matches too. Storage failures (a bbolt error) stay distinct: they are +// operational faults, not answers about the key. So is an entry the header +// ADMITTED the caller to and that then proved unreadable (an undecodable or +// header-disagreeing body): the caller was entitled to it, the refusal shape +// is decided on the fixed header only, and every caller kind is told the +// entry is unreadable and has been invalidated (cache.ErrEntryUnreadable; +// codex round 6). A frame the header itself cannot vouch for is unrecognised +// provenance (legacy: refused for every caller, invalidated). +// +// Administrators get the reason: legacy provenance (invalidated), an internal +// entry, or — for the anonymous /mcp caller — an authenticated +// administrator's entry. +func readCacheRefusal(err error, reader cache.Authorization) *mcp.CallToolResult { + if !reader.IsAdministrator() && (errors.Is(err, cache.ErrUnauthorizedRead) || errors.Is(err, cache.ErrKeyExpired)) { + err = cache.ErrKeyNotFound + } + switch { + case errors.Is(err, cache.ErrEntryUnreadable): + return mcp.NewToolResultError("Cache entry is unreadable: its stored record could not be decoded and it has been invalidated. Re-run the original tool call to obtain a new cache key.") + case errors.Is(err, cache.ErrLegacyProvenance): + return mcp.NewToolResultError("Cache entry is not readable: it predates provenance stamping and has been invalidated. Re-run the original tool call to obtain a new cache key.") + case errors.Is(err, cache.ErrInternalEntry): + return mcp.NewToolResultError("Cache entry is not readable: it is internal to mcpproxy (registry or repository metadata) and cannot be paged through read_cache.") + case errors.Is(err, cache.ErrUnauthorizedRead): + return mcp.NewToolResultError("Cache entry is not readable with this credential: it was produced under a broader authorization (server scope, permission tier or profile) than this request holds. Re-run the original tool call with this credential to obtain your own cache key.") + } + return mcp.NewToolResultError(fmt.Sprintf("Failed to retrieve cached data: %v", err)) +} diff --git a/internal/server/mcp.go b/internal/server/mcp.go index 0fc13e096..e55eaab1d 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -5807,11 +5807,9 @@ func (p *MCPProxyServer) handleReadCache(ctx context.Context, request mcp.CallTo reader := p.cacheAuthorization(ctx) response, err := p.cacheManager.GetRecordsAs(key, offset, limit, reader) if err != nil { + // The activity record keeps the real reason; the body below may not. p.emitActivityInternalToolCall("read_cache", "", "", "", sessionID, requestID, "error", err.Error(), time.Since(startTime).Milliseconds(), activityArgs, nil, nil, "") - if errors.Is(err, cache.ErrUnauthorizedRead) { - return mcp.NewToolResultError("Cache entry is not readable with this credential: it was produced under a broader authorization (server scope, permission tier or profile) than this request holds. Re-run the original tool call with this credential to obtain your own cache key."), nil - } - return mcp.NewToolResultError(fmt.Sprintf("Failed to retrieve cached data: %v", err)), nil + return readCacheRefusal(err, reader), nil } // Serialize response @@ -5828,13 +5826,23 @@ func (p *MCPProxyServer) handleReadCache(ctx context.Context, request mcp.CallTo // in that case there's nothing this layer can subdivide, so the oversize // text flows through unchanged. p.logger receives a zap.Warn if the cache // write fails so the resulting "cache key not found" is diagnosable. + // + // A re-cached page is stamped with the PARENT entry's snapshot, not the + // redeemer's (Spec 105 FR-001, monotone recursive provenance). Because + // the child's snapshot equals the parent's, the child's readers are + // exactly the parent's readers: a child never becomes redeemable by a + // caller the parent refused, nor unreadable to the producer whose payload + // it continues. (Under caller-kind-first ordering the redeemer is not + // necessarily broader in reach — a session-profiled administrator passes + // the gate — which is why the parent's snapshot, not "the narrower of the + // two", is the rule.) text, reTruncated := maybeTruncateAndCacheText( string(jsonResult), "read_cache", args, len(response.Records), p.currentTruncator(), - p.cacheStoreAs(reader), + p.cacheStoreAs(childPageProducer(response, reader)), p.logger, ) diff --git a/internal/server/mcp_call_tool_direct_test.go b/internal/server/mcp_call_tool_direct_test.go index e4482092c..3696e338d 100644 --- a/internal/server/mcp_call_tool_direct_test.go +++ b/internal/server/mcp_call_tool_direct_test.go @@ -8,6 +8,9 @@ import ( "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/cache" ) // Every truncation banner mcpproxy emits points the caller at read_cache. That @@ -35,9 +38,14 @@ func TestCallToolDirect_ReadCacheIsRoutable(t *testing.T) { func TestCallToolDirect_ReadCachePagesStoredRecords(t *testing.T) { proxy := createTestMCPProxyServer(t) + // Spec 105 FR-002 (task T031 inversion): an unstamped Store is legacy + // provenance and is refused for every caller, so the record is seeded + // stamped as the anonymous /mcp caller — the authorization the + // unauthenticated CallToolDirect below acts under. const key = "cache-key-direct" content := `{"tools":[{"name":"github:get_repo"},{"name":"github:list_issues"}]}` - require.NoError(t, proxy.cacheManager.Store(key, "retrieve_tools", map[string]interface{}{"query": "manage"}, content, "tools", 2)) + require.NoError(t, proxy.cacheManager.StoreAs(key, "retrieve_tools", map[string]interface{}{"query": "manage"}, content, "tools", 2, + cache.Authorization{CallerKind: cache.CallerKindAnonymous})) request := mcp.CallToolRequest{} request.Params.Name = "read_cache" @@ -61,3 +69,44 @@ func TestCallToolDirect_ReadCachePagesStoredRecords(t *testing.T) { require.Len(t, page.Records, 2) assert.Equal(t, "github:get_repo", page.Records[0]["name"]) } + +// Spec 105 FR-001/FR-010 (gap FR001-G5, task T025): the REST direct call path +// (`POST /api/v1/tools/call` → CallToolDirect) is a read_cache redemption +// door too, and an agent token's refusal there must be as non-disclosing as +// on the MCP surface: a key produced under a broader authorization answers +// with the SAME error text as a key that never existed. Status parity +// already held (both are isError → HTTP 500); the body did not. +func TestCallToolDirect_AgentReadCacheRefusalMatchesNotFound(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + + broad := agentCtx([]string{"github", "weather"}, []string{auth.PermRead, auth.PermWrite}, "") + narrow := agentCtx([]string{"weather"}, []string{auth.PermRead}, "") + liveKey, _ := produceTruncatedKey(t, proxy, broad) + + readVia := func(ctx context.Context, key string) error { + request := mcp.CallToolRequest{} + request.Params.Name = "read_cache" + request.Params.Arguments = map[string]interface{}{ + "key": key, "offset": float64(0), "limit": float64(50), + } + _, err := proxy.CallToolDirect(ctx, request) + return err + } + + // Controls: the producer reads its own key here; the administrator's + // nonexistent-key body is unchanged. + require.NoError(t, readVia(broad, liveKey), "the producing token reads its entry on the REST path") + adminAbsent := readVia(adminCtx(), "no-such-key") + require.Error(t, adminAbsent) + require.Contains(t, adminAbsent.Error(), "cache key not found") + + live := readVia(narrow, liveKey) + absent := readVia(narrow, "no-such-key") + require.Error(t, live, "a narrower token must be refused the broader entry") + require.Error(t, absent) + assert.NotContains(t, live.Error(), "github:", "the refusal must not leak the out-of-scope payload") + assert.Equal(t, absent.Error(), live.Error(), + "on the REST path a broader-scope key must be indistinguishable from a nonexistent key") + assert.Contains(t, live.Error(), "cache key not found") +} diff --git a/internal/server/mcp_read_cache_authz_test.go b/internal/server/mcp_read_cache_authz_test.go index 6faf3f38c..1b63b9d06 100644 --- a/internal/server/mcp_read_cache_authz_test.go +++ b/internal/server/mcp_read_cache_authz_test.go @@ -78,13 +78,22 @@ func TestReadCache_NarrowerTokenOnSameSessionCannotReadBroaderEntry(t *testing.T setTruncateLimit(proxy, 1_000_000) // Every page, not only the first: an attacker who is refused page 0 just - // asks for page 1. The refusal must be THE authorization refusal — a - // key-not-found or any other error would pass a vacuous "IsError" check. + // asks for page 1. Spec 105 FR-001 (task T031 inversion): for a scoped + // caller the refusal is NON-DISCLOSING — byte-identical to the body a key + // that never existed produces — so it cannot be asserted by its wording. + // It is kept non-vacuous two ways: the administrator control below proves + // the key is live (the refusal is the gate, not a miss), and the body is + // compared against the nonexistent-key body rather than merely IsError. + adminControl := readCacheAs(t, proxy, auth.WithAuthContext(session, auth.AdminContext()), key, 0) + require.False(t, adminControl.IsError, "control: the key is live — an administrator pages it") + absent := readCacheAs(t, proxy, narrow, "0000000000000000000000000000000000000000000000000000000000000000", 0) + require.True(t, absent.IsError) + require.Contains(t, resultText(t, absent), "cache key not found") for offset := range fullResp.Tools { result := readCacheAs(t, proxy, narrow, key, offset) assert.True(t, result.IsError, "offset %d: a narrower token must not read a broader token's cache entry", offset) - assert.Contains(t, resultText(t, result), "not readable with this credential", - "offset %d: refusal must be the authorization gate, not an unrelated failure", offset) + assert.Equal(t, resultText(t, absent), resultText(t, result), + "offset %d: the refusal must be the nonexistent-key body (non-disclosing)", offset) assert.NotContains(t, resultText(t, result), "github:", "offset %d: refused read must not leak the out-of-scope payload", offset) assert.NotContains(t, resultText(t, result), `"records"`, @@ -168,8 +177,16 @@ func TestReadCache_DeletedPinnedProfileRevokesCachedAccess(t *testing.T) { after := readCacheAs(t, proxy, pinned, match[1], 0) assert.True(t, after.IsError, "a deleted pinned profile must revoke cached access") - assert.Contains(t, resultText(t, after), "not readable with this credential") + // Spec 105 FR-001 (task T031 inversion): the revocation is non-disclosing + // — the stale pin sees the nonexistent-key body, not a wording that + // confirms the entry exists. The administrator control proves it does. + absent := readCacheAs(t, proxy, pinned, "0000000000000000000000000000000000000000000000000000000000000000", 0) + require.True(t, absent.IsError) + assert.Equal(t, resultText(t, absent), resultText(t, after), "a revoked read must be indistinguishable from a miss") + assert.Contains(t, resultText(t, after), "cache key not found") assert.NotContains(t, resultText(t, after), "github:") + control := readCacheAs(t, proxy, auth.WithAuthContext(context.Background(), auth.AdminContext()), match[1], 0) + require.False(t, control.IsError, "control: the entry is still live for an administrator") } // The reverse direction stays open: an admin (unrestricted) reader could have diff --git a/internal/server/mcp_read_cache_scope_test.go b/internal/server/mcp_read_cache_scope_test.go new file mode 100644 index 000000000..71ac92418 --- /dev/null +++ b/internal/server/mcp_read_cache_scope_test.go @@ -0,0 +1,487 @@ +package server + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.etcd.io/bbolt" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/cache" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/registries" +) + +// Spec 105 FR-001/FR-002 at the read_cache handler (tasks T021, T022, T024, +// T025; gaps FR001-G1, G2, G4, G5). The cache-package tests pin the gate; +// these pin what the MCP surface does with the gate's answer: which callers +// are refused, what a refusal looks like, and whose snapshot a recursive +// child page inherits. + +// allPerms is every permission tier — HasPermission is exact-match, so an +// "unrestricted" agent fixture must spell them all out. +var allPerms = []string{auth.PermRead, auth.PermWrite, auth.PermDestructive} + +// userCtx returns a context authenticated as a server-edition OAuth user — +// the `user` caller kind, bounded to its own identity (Spec 024) — in the +// shape auth.UserContext mints in production: no server grant, which every +// dispatch gate treats as deny-all. userCtxScoped is the allowlist-scoped +// shape the set_profile tests use. +func userCtx(id string) context.Context { + return auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeUser, UserID: id, Email: id + "@example.com", Role: "user", + }) +} + +// userCtxScoped is userCtx with a server allowlist and permission set — a +// user context is scoped by auth.CanAccessServer / HasPermission at every +// dispatch gate exactly like an agent token (profile_tool_test.go, "server +// edition user scoped like visibility"). +func userCtxScoped(id string, allowed []string, perms []string) context.Context { + return auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeUser, UserID: id, Email: id + "@example.com", Role: "user", + AllowedServers: allowed, Permissions: perms, + }) +} + +// readCachePage is readCacheAs with a caller-chosen page size; the shared +// helper pages one record at a time, which can never re-truncate. +func readCachePage(t *testing.T, proxy *MCPProxyServer, ctx context.Context, key string, offset, limit int) *mcp.CallToolResult { + t.Helper() + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "key": key, "offset": float64(offset), "limit": float64(limit), + } + result, err := proxy.handleReadCache(ctx, req) + require.NoError(t, err) + require.NotNil(t, result) + return result +} + +// produceTruncatedKey runs retrieve_tools as ctx under a limit that forces +// truncation and returns the minted read_cache key plus the untruncated +// response for comparison. The limit is restored afterwards. +func produceTruncatedKey(t *testing.T, proxy *MCPProxyServer, ctx context.Context) (key string, full retrieveToolsResponse) { + t.Helper() + args := map[string]interface{}{"query": "manage", "limit": float64(10)} + req := mcp.CallToolRequest{} + req.Params.Arguments = args + + setTruncateLimit(proxy, 1_000_000) + fullResult, err := proxy.handleRetrieveTools(ctx, req) + require.NoError(t, err) + require.False(t, fullResult.IsError, resultText(t, fullResult)) + require.NoError(t, json.Unmarshal([]byte(resultText(t, fullResult)), &full)) + require.GreaterOrEqual(t, len(full.Tools), 2, "fixture must yield at least two records") + + setTruncateLimit(proxy, len(resultText(t, fullResult))/2) + truncated, err := proxy.handleRetrieveTools(ctx, req) + require.NoError(t, err) + match := cacheKeyRE.FindStringSubmatch(resultText(t, truncated)) + require.Len(t, match, 2, "truncated response must carry a read_cache key") + setTruncateLimit(proxy, 1_000_000) + return match[1], full +} + +// expireCacheEntry rewrites the entry's expiry into the past in place, the +// way the cache package's own expiry test does. +func expireCacheEntry(t *testing.T, proxy *MCPProxyServer, key string) { + t.Helper() + require.NoError(t, proxy.storage.GetDB().Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(cache.CacheBucket)) + data := bucket.Get([]byte(key)) + require.NotNil(t, data, "premise: entry %q exists", key) + var rec cache.Record + require.NoError(t, rec.UnmarshalBinary(data)) + rec.ExpiresAt = time.Now().Add(-time.Hour) + out, err := rec.MarshalBinary() + require.NoError(t, err) + return bucket.Put([]byte(key), out) + })) +} + +// FR001-G1 at the handler: a legacy (nil-producer) entry is refused for the +// administrator and for the anonymous /mcp caller — the two kinds that could +// redeem it before — with no `records` in the answer. +func TestReadCache_LegacyEntryRefusedForAdminAndAnonymous(t *testing.T) { + proxy := createTestMCPProxyServer(t) + const key = "legacy-entry" + require.NoError(t, proxy.cacheManager.Store(key, "retrieve_tools", map[string]interface{}{"query": "manage"}, + `{"tools":[{"name":"github:SENTINEL_LEGACY"},{"name":"github:second"}]}`, "tools", 2)) + + for _, tc := range []struct { + name string + ctx context.Context + }{ + {"admin", adminCtx()}, + {"anonymous (no auth context)", context.Background()}, + {"anonymous (admin-shaped context)", auth.WithAuthContext(context.Background(), auth.AnonymousContext())}, + {"wildcard agent", agentCtx([]string{"*"}, allPerms, "")}, + {"user (server edition)", userCtx("01HUSER")}, + } { + t.Run(tc.name, func(t *testing.T) { + // Re-seed: the first refused redemption invalidates the entry. + require.NoError(t, proxy.cacheManager.Store(key, "retrieve_tools", map[string]interface{}{"query": "manage"}, + `{"tools":[{"name":"github:SENTINEL_LEGACY"},{"name":"github:second"}]}`, "tools", 2)) + result := readCachePage(t, proxy, tc.ctx, key, 0, 50) + assert.True(t, result.IsError, "%s must be refused a legacy entry: %s", tc.name, resultText(t, result)) + assert.NotContains(t, resultText(t, result), `"records"`, "a refused legacy read must not return a page") + assert.NotContains(t, resultText(t, result), "SENTINEL_LEGACY", "a refused legacy read must not leak content") + _, present := proxy.cacheManager.Peek(key) + assert.False(t, present, "%s: the legacy entry must be invalidated on first redemption", tc.name) + }) + } +} + +// FR001-G2 at the handler: the proxy's own registry and guesser entries are +// non-redeemable through read_cache for administrators, indistinguishable +// from a missing key for agents, and never evicted by the refusal (their keys +// are guessable; the registry and guesser readers depend on them). +func TestReadCache_InternalEntriesRefusedWithoutEviction(t *testing.T) { + registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "servers": []map[string]interface{}{{ + "server": map[string]interface{}{ + "name": "io.example/SENTINEL_REGISTRY_SERVER", + "description": "a registry listing", + "remotes": []interface{}{ + map[string]interface{}{"type": "streamable-http", "url": "https://example.com/mcp"}, + }, + }, + }}, + "metadata": map[string]interface{}{}, + }) + })) + defer registry.Close() + + // AllowPrivateRegistryFetch flips the process-wide SSRF allow-policy when + // the runtime loads this config (registries.SetRegistriesFromConfig); + // restore the default catalog and policy so an order-shuffled + // TestBuildRegistrySourceEntry_RejectsSSRFLiteralIP still rejects literal + // IPs (same fix as #1222). Registered before the write so LIFO cleanup + // runs it while the httptest registry is still serving. + t.Cleanup(func() { registries.SetRegistriesFromConfig(nil) }) + proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Registries = []config.RegistryEntry{{ + ID: "scope-reg", Name: "Scope Reg", URL: registry.URL, ServersURL: registry.URL, + Protocol: "modelcontextprotocol/registry", + }} + cfg.AllowPrivateRegistryFetch = true + }) + + // The registry entry is written by the real runtime writer. + servers, _, err := rt.SearchRegistryServers("scope-reg", "", "", 5) + require.NoError(t, err) + require.Len(t, servers, 1) + registryKey := fmt.Sprintf("registry-servers:%s:%s:%s:%d", "scope-reg", "", "", 5) + _, ok := proxy.cacheManager.Peek(registryKey) + require.True(t, ok, "premise: the registry writer persisted %q", registryKey) + + // The guesser entry carries the same internal stamp its writer applies + // (asserted in internal/experiments); it is seeded here with that stamp. + const npmKey = "npm:@acme/SENTINEL_NPM_PACKAGE" + require.NoError(t, proxy.cacheManager.StoreAs(npmKey, "repo_guess", + map[string]interface{}{"package_name": "@acme/SENTINEL_NPM_PACKAGE", "type": "npm"}, + `{"type":"npm","package_name":"@acme/SENTINEL_NPM_PACKAGE","exists":true}`, "", 1, + cache.Authorization{CallerKind: "internal"})) + + agent := agentCtx([]string{"*"}, allPerms, "") + for _, key := range []string{registryKey, npmKey} { + t.Run(key, func(t *testing.T) { + admin := readCachePage(t, proxy, adminCtx(), key, 0, 50) + assert.True(t, admin.IsError, "an administrator must not redeem an internal entry: %s", resultText(t, admin)) + assert.NotContains(t, resultText(t, admin), "SENTINEL_", "a refused internal read must not leak the payload") + assert.NotContains(t, resultText(t, admin), `"records"`) + + // Scoped callers (agent tokens, server-edition users) cannot tell a + // live internal key from an absent one. + for _, scoped := range []struct { + name string + ctx context.Context + }{{"agent", agent}, {"user", userCtx("01HUSER")}} { + live := readCachePage(t, proxy, scoped.ctx, key, 0, 50) + absent := readCachePage(t, proxy, scoped.ctx, key+"-does-not-exist", 0, 50) + require.True(t, live.IsError) + require.True(t, absent.IsError) + assert.Equal(t, resultText(t, absent), resultText(t, live), + "%s: a scoped caller must not be able to tell a live internal key from an absent one", scoped.name) + assert.Contains(t, resultText(t, live), "cache key not found") + } + + _, stillThere := proxy.cacheManager.Peek(key) + assert.True(t, stillThere, "a refused redemption must not evict the internal entry %q", key) + }) + } + + // The registry's own reader still serves the (unevicted) entry. + servers, info, err := rt.SearchRegistryServers("scope-reg", "", "", 5) + require.NoError(t, err) + require.Len(t, servers, 1) + require.NotNil(t, info) +} + +// FR001-G4: recursive provenance is monotone. A child page minted while an +// administrator pages an agent's entry carries the PARENT's snapshot (the +// agent's — the redeemer is broader, not narrower), so the producing agent +// can read the child it could have produced. Before this feature the child +// was stamped with the redeemer, and the producer was locked out of its own +// payload's continuation. +func TestReadCache_RecursiveChildInheritsParentProducer(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + + producer := agentCtx([]string{"*"}, allPerms, "") + k1, full := produceTruncatedKey(t, proxy, producer) + + // The administrator pages K1 whole under a limit that forces read_cache's + // own output to be truncated and re-cached as K2. + adminPage := readCachePage(t, proxy, adminCtx(), k1, 0, 50) + require.False(t, adminPage.IsError, "premise: an administrator reads an agent's entry: %s", resultText(t, adminPage)) + setTruncateLimit(proxy, len(resultText(t, adminPage))/2) + adminTrunc := readCachePage(t, proxy, adminCtx(), k1, 0, 50) + setTruncateLimit(proxy, 1_000_000) + require.False(t, adminTrunc.IsError) + match := cacheKeyRE.FindStringSubmatch(resultText(t, adminTrunc)) + require.Len(t, match, 2, "premise: the administrator's oversize page mints a child key") + k2 := match[1] + require.NotEqual(t, k1, k2) + + rec, ok := proxy.cacheManager.Peek(k2) + require.True(t, ok) + require.NotNil(t, rec.Producer, "the child page must carry a producer snapshot") + parentName := auth.AuthContextFromContext(producer).AgentName + assert.Equal(t, cache.CallerKindAgent, rec.Producer.CallerKind, "the child carries the PARENT's (agent) snapshot, not the administrator redeemer's") + assert.Equal(t, parentName, rec.Producer.Principal) + + // The producing agent reads the child it could have produced. + child := readCachePage(t, proxy, producer, k2, 0, 50) + require.False(t, child.IsError, "the parent's producer must read the recursive child: %s", resultText(t, child)) + var page struct { + Records []map[string]interface{} `json:"records"` + } + require.NoError(t, json.Unmarshal([]byte(resultText(t, child)), &page)) + require.Len(t, page.Records, len(full.Tools)) + assert.Equal(t, full.Tools[0]["name"], page.Records[0]["name"]) +} + +// FR001-G5: an agent's read_cache refusal is non-disclosing. A key that +// exists but was produced under a broader authorization, a key that never +// existed and a key whose entry expired all answer with the SAME text — the +// not-found body — so the refusal cannot be used as an existence oracle. +// Administrators are unaffected (they read the live key; nonexistent keeps +// its body). +func TestReadCache_AgentRefusalIsNonDisclosing(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + + broad := agentCtx([]string{"github", "weather"}, []string{auth.PermRead, auth.PermWrite}, "") + narrow := agentCtx([]string{"weather"}, []string{auth.PermRead}, "") + + liveKey, _ := produceTruncatedKey(t, proxy, broad) + expiredKey, _ := produceTruncatedKey(t, proxy, broad) + expireCacheEntry(t, proxy, expiredKey) + absentKey := "0000000000000000000000000000000000000000000000000000000000000000" + + adminLive := readCachePage(t, proxy, adminCtx(), liveKey, 0, 1) + require.False(t, adminLive.IsError, "control: the administrator reads the live key") + adminAbsent := readCachePage(t, proxy, adminCtx(), absentKey, 0, 1) + require.True(t, adminAbsent.IsError) + require.Contains(t, resultText(t, adminAbsent), "cache key not found") + + live := readCachePage(t, proxy, narrow, liveKey, 0, 1) + absent := readCachePage(t, proxy, narrow, absentKey, 0, 1) + expired := readCachePage(t, proxy, narrow, expiredKey, 0, 1) + require.True(t, live.IsError) + require.True(t, absent.IsError) + require.True(t, expired.IsError) + assert.NotContains(t, resultText(t, live), "github:") + + assert.Equal(t, resultText(t, absent), resultText(t, live), + "a broader-scope key must be indistinguishable from a nonexistent key for the narrow token") + assert.Equal(t, resultText(t, absent), resultText(t, expired), + "an expired key must be indistinguishable from a nonexistent key for the narrow token") + assert.Contains(t, resultText(t, live), "cache key not found", + "the unified body keeps the not-found substring agents already handle") + // Page 1 as well as page 0: the body must not vary with the offset either. + assert.Equal(t, resultText(t, absent), resultText(t, readCachePage(t, proxy, narrow, liveKey, 1, 1))) +} + +// Critique round 2, finding 2: the administrator-facing refusal bodies were +// asserted nowhere (a mutation hiding the reason from administrators too +// passed the whole suite), and the pre-feature parity body — the anonymous +// /mcp caller refused an authenticated administrator's entry — lost its only +// assertion when T031 inverted the agent tests. Administrators get the +// REASON: legacy provenance (invalidated), an internal entry, or, for the +// anonymous caller, an authenticated administrator's entry (SC-005 parity +// control). The same keys answer a scoped caller with the not-found body. +func TestReadCache_AdministratorRefusalBodiesNameTheReason(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + anonymous := auth.WithAuthContext(context.Background(), auth.AnonymousContext()) + agent := agentCtx([]string{"*"}, allPerms, "") + + // Legacy provenance: the administrator is told it predates stamping. + const legacyKey = "legacy-entry" + require.NoError(t, proxy.cacheManager.Store(legacyKey, "retrieve_tools", map[string]interface{}{"query": "manage"}, + `{"tools":[{"name":"github:SENTINEL_LEGACY"}]}`, "tools", 1)) + legacy := readCachePage(t, proxy, adminCtx(), legacyKey, 0, 50) + require.True(t, legacy.IsError) + assert.Contains(t, resultText(t, legacy), "predates provenance stamping", "the administrator gets the legacy reason") + assert.NotContains(t, resultText(t, legacy), "SENTINEL_LEGACY") + + // Internal entry: the administrator is told it is mcpproxy's own. + const internalKey = "registry-servers:official:::10" + require.NoError(t, proxy.cacheManager.StoreAs(internalKey, "registry-servers", nil, + `[{"id":"srv-1","name":"SENTINEL_INTERNAL"}]`, "", 1, cache.Authorization{CallerKind: cache.CallerKindInternal})) + internal := readCachePage(t, proxy, adminCtx(), internalKey, 0, 50) + require.True(t, internal.IsError) + assert.Contains(t, resultText(t, internal), "internal to mcpproxy", "the administrator gets the internal-entry reason") + assert.NotContains(t, resultText(t, internal), "SENTINEL_INTERNAL") + + // Anonymous below authenticated administrator: the pre-feature body. + adminKey, _ := produceTruncatedKey(t, proxy, adminCtx()) + control := readCachePage(t, proxy, adminCtx(), adminKey, 0, 1) + require.False(t, control.IsError, "control: the producing administrator reads its entry") + anon := readCachePage(t, proxy, anonymous, adminKey, 0, 1) + require.True(t, anon.IsError, "the anonymous caller ranks below an authenticated administrator") + assert.Contains(t, resultText(t, anon), "not readable with this credential", "SC-005 parity: the anonymous caller's body is unchanged") + assert.NotContains(t, resultText(t, anon), "github:") + + // The same three keys are plain misses for a scoped caller. + absent := readCachePage(t, proxy, agent, "0000000000000000000000000000000000000000000000000000000000000000", 0, 1) + require.True(t, absent.IsError) + for _, key := range []string{internalKey, adminKey} { + got := readCachePage(t, proxy, agent, key, 0, 1) + require.True(t, got.IsError) + assert.Equal(t, resultText(t, absent), resultText(t, got), "%s: a scoped caller gets the not-found body, never the reason", key) + } + require.NoError(t, proxy.cacheManager.Store(legacyKey, "retrieve_tools", map[string]interface{}{"query": "manage"}, + `{"tools":[{"name":"github:SENTINEL_LEGACY"}]}`, "tools", 1)) + got := readCachePage(t, proxy, agent, legacyKey, 0, 1) + require.True(t, got.IsError) + assert.Equal(t, resultText(t, absent), resultText(t, got), "a scoped caller gets the not-found body for a legacy entry too") +} + +// Codex round 6, server finding 1: a reader the header ADMITS (the producing +// agent, digest-equal) whose entry's body is undecodable pays the body decode +// — it is entitled to the entry, so that decode is not a refusal oracle — +// but must not then receive the refusal shape: the only responses sharing +// the not-found body are the ones decided on the header. It gets a distinct +// admitted-class body ("unreadable", invalidated) — the same body an +// administrator gets — and the entry is gone, so the next read is a plain +// miss. +func TestReadCache_AdmittedReaderOfUnreadableEntryGetsDistinctBody(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + agent := agentCtx([]string{"github", "weather"}, []string{auth.PermRead}, "") + absentKey := "0000000000000000000000000000000000000000000000000000000000000000" + + for _, tc := range []struct { + name string + reader context.Context + }{{"digest-equal agent", agent}, {"administrator", adminCtx()}} { + t.Run(tc.name, func(t *testing.T) { + key, _ := produceTruncatedKey(t, proxy, agent) + control := readCachePage(t, proxy, tc.reader, key, 0, 1) + require.False(t, control.IsError, "control: the reader is admitted to the intact entry") + corruptCacheEntryBody(t, proxy, key) + + got := readCachePage(t, proxy, tc.reader, key, 0, 1) + require.True(t, got.IsError) + absent := readCachePage(t, proxy, tc.reader, absentKey, 0, 1) + require.True(t, absent.IsError) + assert.NotEqual(t, resultText(t, absent), resultText(t, got), + "an admitted reader's unreadable entry must not wear the not-found shape") + assert.Contains(t, resultText(t, got), "unreadable") + assert.Contains(t, resultText(t, got), "invalidated") + assert.NotContains(t, resultText(t, got), "github:") + + again := readCachePage(t, proxy, tc.reader, key, 0, 1) + require.True(t, again.IsError) + assert.Equal(t, resultText(t, absent), resultText(t, again), "the entry was invalidated: a second read is a plain miss") + }) + } +} + +// corruptCacheEntryBody truncates the stored value's last byte in place, so +// the frame header (the prefix) stays intact and admits exactly whom it did, +// while the JSON body behind it no longer decodes. +func corruptCacheEntryBody(t *testing.T, proxy *MCPProxyServer, key string) { + t.Helper() + require.NoError(t, proxy.storage.GetDB().Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(cache.CacheBucket)) + data := bucket.Get([]byte(key)) + require.NotNil(t, data, "premise: entry %q exists", key) + return bucket.Put([]byte(key), append([]byte(nil), data[:len(data)-1]...)) + })) +} + +// Codex round 4, finding 1: the user-kind snapshot retained only the user id, +// and redemption compared only the id — so a user allowed {github} produced a +// github entry and, once narrowed to {weather} (or bound to a disjoint or +// deleted profile), still redeemed it because the id matched, although the +// dispatch gates refuse that user's own call to github. A user is scoped by +// AllowedServers/Permissions/profile like an agent (profile_tool_test.go), +// so the snapshot carries them and the read gate holds it to them; identity +// equality is necessary, not sufficient. Non-disclosing for the user like +// any scoped refusal; the administrator still reads. Research D16 (codex +// round 5): the door decides on the header digest alone, so the SAME user +// with a WIDER grant is refused too — the header carries no identity an +// unrestricted-user rule could check, and FR-001 does not oblige admitting +// a strictly wider bounded reader — fail-closed, and non-disclosing. +func TestReadCache_UserSnapshotIsContainedLikeAnAgent(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + const id = "01HUSER" + github := userCtxScoped(id, []string{"github"}, []string{auth.PermRead}) + weather := userCtxScoped(id, []string{"weather"}, []string{auth.PermRead}) + wider := userCtxScoped(id, []string{"github", "weather"}, []string{auth.PermRead, auth.PermWrite}) + otherUser := userCtxScoped("01HOTHER", []string{"github", "weather"}, allPerms) + // Premise: the narrowed user cannot dispatch to github. + require.False(t, auth.AuthContextFromContext(weather).CanAccessServer("github")) + + key, full := produceTruncatedKey(t, proxy, github) + stamp := proxy.cacheAuthorization(github) + require.Equal(t, cache.CallerKindUser, stamp.CallerKind) + require.Equal(t, []string{"github"}, stamp.AllowedServers, "the user snapshot must carry the server grant") + require.Equal(t, []string{auth.PermRead}, stamp.Permissions, "the user snapshot must carry the permission set") + + own := readCachePage(t, proxy, github, key, 0, 50) + require.False(t, own.IsError, "the producing user reads its own entry: %s", resultText(t, own)) + var page cache.ReadCacheResponse + require.NoError(t, json.Unmarshal([]byte(resultText(t, own)), &page)) + require.Len(t, page.Records, len(full.Tools)) + + absentKey := "0000000000000000000000000000000000000000000000000000000000000000" + for _, tc := range []struct { + name string + ctx context.Context + }{ + {"same user narrowed to weather", weather}, + {"same user, production no-grant context", userCtx(id)}, + {"other user with a wider grant", otherUser}, + {"same user with a wider grant is refused (D16: digest-equal only)", wider}, + } { + t.Run(tc.name, func(t *testing.T) { + live := readCachePage(t, proxy, tc.ctx, key, 0, 50) + absent := readCachePage(t, proxy, tc.ctx, absentKey, 0, 50) + require.True(t, live.IsError, "must be refused: %s", resultText(t, live)) + require.True(t, absent.IsError) + assert.NotContains(t, resultText(t, live), "github:") + assert.Equal(t, resultText(t, absent), resultText(t, live), "the refusal must not disclose the key's existence") + assert.Contains(t, resultText(t, live), "cache key not found") + }) + } + // Refusals do not evict: the producer and the administrator still read. + require.False(t, readCachePage(t, proxy, github, key, 0, 50).IsError, "the producing user still reads after the refusals") + require.False(t, readCachePage(t, proxy, adminCtx(), key, 0, 50).IsError, "the administrator reads any snapshot") +} diff --git a/internal/server/scope_cache_fixtures_test.go b/internal/server/scope_cache_fixtures_test.go new file mode 100644 index 000000000..647955eed --- /dev/null +++ b/internal/server/scope_cache_fixtures_test.go @@ -0,0 +1,551 @@ +package server + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "sync" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + mcpserver "github.com/mark3labs/mcp-go/server" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.etcd.io/bbolt" + "go.uber.org/zap" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/cache" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/index" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/secret" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/truncate" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/upstream" +) + +// Spec 105 FR-001 / FR-002 / FR-013 mandatory fixtures (task T027, gap +// FR001-G7). Each fixture is named in the spec text and was absent from the +// suite: +// +// (a) upgrade: a pre-feature record is refused, returns no content, and is +// absent after the store is restarted (SC-004); +// (b) fresh internal entry: refused for administrators, agents and the +// anonymous caller, never evicted (SC-005 named exception); +// (c) recursive child on the REST direct call path, plus the spec's named +// fixture — a session-profiled administrator redeems a broad entry and +// a pinned wildcard agent is refused the child on MCP and REST; +// (d) pinned token on REST: direct dispatch to the out-of-pin server is +// refused AND cache redemption of an entry containing it is refused +// with the nonexistent-key body (Scope Boundary exception); +// (e) held call: the snapshot is the one resolved at dispatch, so a session +// narrowed while the upstream call is in flight neither re-stamps the +// entry nor redeems it (passes on the merge base — regression pin); +// (f) empty server grant (codex round 1): an agent token with no allowed +// servers is deny-all on every dispatch gate and must be deny-all on +// redemption too — an identically-stamped record answers with the +// nonexistent-key body on MCP and REST. + +// newRestartableProxy builds the minimal proxy on a caller-owned directory +// and returns it with an explicit close, so a test can stop it and open a +// second proxy on the SAME data — the only way to prove an invalidation +// reached disk rather than an in-memory view. +func newRestartableProxy(t *testing.T, dir string) (*MCPProxyServer, func()) { + t.Helper() + logger := zap.NewNop() + + sm, err := storage.NewManager(dir, logger.Sugar()) + require.NoError(t, err) + idx, err := index.NewManager(dir, logger) + require.NoError(t, err) + + cfg := config.DefaultConfig() + cfg.DataDir = dir + cfg.ToolsLimit = 20 + um := upstream.NewManager(logger, cfg, nil, secret.NewResolver(), nil) + cm, err := cache.NewManager(sm.GetDB(), logger) + require.NoError(t, err) + tr := truncate.NewTruncator(0) + proxy := NewMCPProxyServer(sm, idx, um, cm, func() *truncate.Truncator { return tr }, logger, nil, false, cfg, nil) + + var once sync.Once + closeFn := func() { + once.Do(func() { + cm.Close() + _ = idx.Close() + _ = sm.Close() + }) + } + t.Cleanup(closeFn) + return proxy, closeFn +} + +// putPreFeatureRecord writes a cache record in the exact wire shape a +// pre-feature binary persisted: no producer, no version. +func putPreFeatureRecord(t *testing.T, db *bbolt.DB, key, fullContent, recordPath string, total int) { + t.Helper() + now := time.Now() + doc := map[string]interface{}{ + "key": key, "tool_name": "retrieve_tools", "args": map[string]interface{}{"query": "manage"}, + "timestamp": now, "full_content": fullContent, "record_path": recordPath, + "total_records": total, "total_size": len(fullContent), "expires_at": now.Add(time.Hour), + "access_count": 0, "last_accessed": now, "created_at": now, + } + data, err := json.Marshal(doc) + require.NoError(t, err) + require.NoError(t, db.Update(func(tx *bbolt.Tx) error { + return tx.Bucket([]byte(cache.CacheBucket)).Put([]byte(key), data) + })) +} + +// callToolDirectText runs one CallToolDirect and returns the text it yields, +// with the error (nil on success) — the REST layer's view of the outcome. +func callToolDirectText(t *testing.T, proxy *MCPProxyServer, ctx context.Context, tool string, args map[string]interface{}) (string, error) { + t.Helper() + request := mcp.CallToolRequest{} + request.Params.Name = tool + request.Params.Arguments = args + out, err := proxy.CallToolDirect(ctx, request) + if err != nil { + return "", err + } + blocks, ok := out.([]mcp.Content) + require.True(t, ok, "CallToolDirect returns raw content blocks, got %T", out) + require.NotEmpty(t, blocks) + text, ok := blocks[0].(mcp.TextContent) + require.True(t, ok) + return text.Text, nil +} + +func readCacheDirect(t *testing.T, proxy *MCPProxyServer, ctx context.Context, key string) (string, error) { + t.Helper() + return callToolDirectText(t, proxy, ctx, "read_cache", map[string]interface{}{ + "key": key, "offset": float64(0), "limit": float64(50), + }) +} + +// (a) Upgrade fixture — SC-004. +func TestScopeCacheFixture_UpgradeRecordRefusedAndAbsentAfterRestart(t *testing.T) { + dir := t.TempDir() + proxy, closeProxy := newRestartableProxy(t, dir) + + const key = "pre-feature-key" + putPreFeatureRecord(t, proxy.storage.GetDB(), key, + `{"tools":[{"name":"github:SENTINEL_UPGRADE"},{"name":"github:second"}]}`, "tools", 2) + _, present := proxy.cacheManager.Peek(key) + require.True(t, present, "premise: the pre-feature record is readable by the decoder") + + // Each leg re-seeds the record (critique round 2, finding 3): the first + // refused redemption invalidates it, so without re-seeding every later + // leg would pass vacuously as a plain miss of an absent key. + seed := func() { + t.Helper() + putPreFeatureRecord(t, proxy.storage.GetDB(), key, + `{"tools":[{"name":"github:SENTINEL_UPGRADE"},{"name":"github:second"}]}`, "tools", 2) + _, ok := proxy.cacheManager.Peek(key) + require.True(t, ok, "premise: the pre-feature record is live before the leg") + } + for _, tc := range []struct { + name string + ctx context.Context + }{ + {"administrator", adminCtx()}, + {"agent", agentCtx([]string{"*"}, allPerms, "")}, + } { + seed() + result := readCachePage(t, proxy, tc.ctx, key, 0, 50) + assert.True(t, result.IsError, "%s must be refused the pre-feature record: %s", tc.name, resultText(t, result)) + assert.NotContains(t, resultText(t, result), "SENTINEL_UPGRADE", "%s: no content may be returned", tc.name) + assert.NotContains(t, resultText(t, result), `"records"`, "%s: no page may be returned", tc.name) + _, present = proxy.cacheManager.Peek(key) + assert.False(t, present, "%s on MCP: the pre-feature record must be invalidated on first redemption", tc.name) + + // The REST door refuses too — against a LIVE record. + seed() + text, err := readCacheDirect(t, proxy, tc.ctx, key) + assert.Error(t, err, "%s on REST must be refused, got %q", tc.name, text) + if err != nil { + assert.NotContains(t, err.Error(), "SENTINEL_UPGRADE") + } + _, present = proxy.cacheManager.Peek(key) + assert.False(t, present, "%s on REST: the pre-feature record must be invalidated on first redemption", tc.name) + } + + // Restart on the same data directory. + closeProxy() + reopened, _ := newRestartableProxy(t, dir) + _, present = reopened.cacheManager.Peek(key) + assert.False(t, present, "the pre-feature record must be absent after restart") + after := readCachePage(t, reopened, adminCtx(), key, 0, 50) + assert.True(t, after.IsError) + assert.Contains(t, resultText(t, after), "cache key not found", "after restart the key is a plain miss") +} + +// (b) Fresh internal entry — SC-005 named exception. Registry and guesser +// entries are written after the upgrade with the internal stamp (their writer +// tests assert the stamp); every read_cache caller is refused, the entry is +// kept for its internal readers, and an agent cannot distinguish it from an +// absent key on MCP or REST. +func TestScopeCacheFixture_FreshInternalEntryRefusedForEveryCaller(t *testing.T) { + proxy := createTestMCPProxyServer(t) + const key = "registry-servers:official:::10" + require.NoError(t, proxy.cacheManager.StoreAs(key, "registry-servers", nil, + `[{"id":"srv-1","name":"SENTINEL_INTERNAL"}]`, "", 1, cache.Authorization{CallerKind: "internal"})) + + agent := agentCtx([]string{"*"}, allPerms, "") + for _, tc := range []struct { + name string + ctx context.Context + }{ + {"administrator", adminCtx()}, + {"anonymous", auth.WithAuthContext(context.Background(), auth.AnonymousContext())}, + {"agent", agent}, + } { + result := readCachePage(t, proxy, tc.ctx, key, 0, 50) + assert.True(t, result.IsError, "%s must be refused a fresh internal entry: %s", tc.name, resultText(t, result)) + assert.NotContains(t, resultText(t, result), "SENTINEL_INTERNAL") + _, err := readCacheDirect(t, proxy, tc.ctx, key) + assert.Error(t, err, "%s on REST must be refused", tc.name) + } + rec, present := proxy.cacheManager.Peek(key) + require.True(t, present, "an internal entry is refused WITHOUT eviction") + require.NotNil(t, rec.Producer) + assert.Equal(t, "internal", rec.Producer.CallerKind) + if got, err := proxy.cacheManager.Get(key); assert.NoError(t, err) { + assert.Contains(t, got.FullContent, "SENTINEL_INTERNAL", "the ungated internal reader still serves it") + } + + live := readCachePage(t, proxy, agent, key, 0, 50) + absent := readCachePage(t, proxy, agent, key+"-absent", 0, 50) + assert.Equal(t, resultText(t, absent), resultText(t, live), "MCP: internal key ≡ absent key for an agent") + _, liveErr := readCacheDirect(t, proxy, agent, key) + _, absentErr := readCacheDirect(t, proxy, agent, key+"-absent") + require.Error(t, liveErr) + require.Error(t, absentErr) + assert.Equal(t, absentErr.Error(), liveErr.Error(), "REST: internal key ≡ absent key for an agent") + assert.Contains(t, liveErr.Error(), "cache key not found", "REST: the shared body is the not-found one, not some earlier pre-check") +} + +// (c) Recursive child on the REST direct call path: the child page an +// administrator mints while paging an agent's entry through +// /api/v1/tools/call carries the agent's snapshot, so the agent reads it. +func TestScopeCacheFixture_RecursiveChildOnREST(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + + producer := agentCtx([]string{"*"}, allPerms, "") + k1, full := produceTruncatedKey(t, proxy, producer) + + adminPage, err := readCacheDirect(t, proxy, adminCtx(), k1) + require.NoError(t, err, "premise: the administrator pages the agent's entry on REST") + setTruncateLimit(proxy, len(adminPage)/2) + adminTrunc, err := readCacheDirect(t, proxy, adminCtx(), k1) + setTruncateLimit(proxy, 1_000_000) + require.NoError(t, err) + match := cacheKeyRE.FindStringSubmatch(adminTrunc) + require.Len(t, match, 2, "premise: the oversize REST page mints a child key") + k2 := match[1] + + rec, ok := proxy.cacheManager.Peek(k2) + require.True(t, ok) + require.NotNil(t, rec.Producer) + assert.Equal(t, cache.CallerKindAgent, rec.Producer.CallerKind, "the REST child carries the parent's (agent) snapshot") + + child, err := readCacheDirect(t, proxy, producer, k2) + require.NoError(t, err, "the parent's producer must read the recursive child on REST") + var page struct { + Records []map[string]interface{} `json:"records"` + } + require.NoError(t, json.Unmarshal([]byte(child), &page)) + require.Len(t, page.Records, len(full.Tools)) +} + +// (c') The spec's named fixture (FR-001): an administrator with SESSION +// profile {github} redeems an entry containing weather; the recursive child +// is then requested by a wildcard full-permission agent pinned to {github} — +// refused on MCP and on REST, with the nonexistent-key body. The child is an +// administrator snapshot (parent's, or the narrower session-profiled one) and +// an agent never qualifies for an administrator snapshot; the administrator's +// own redemption of the broad entry is the caller-kind-first rule (D5). +func TestScopeCacheFixture_ProfiledAdminChildNotRedeemableByPinnedAgent(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + proxy.config.Servers = []*config.ServerConfig{{Name: "github", Enabled: true}, {Name: "weather", Enabled: true}} + proxy.config.Profiles = []config.ProfileConfig{{Name: "research", Servers: []string{"github"}}} + + // Unscoped administrator produces the broad entry (it lists weather). + k1, full := produceTruncatedKey(t, proxy, adminCtx()) + require.True(t, func() bool { + for _, tool := range full.Tools { + if strings.HasPrefix(fmt.Sprint(tool["name"]), "weather:") { + return true + } + } + return false + }(), "premise: the broad entry contains a weather tool") + + // Administrator bound to session profile {github}. + helper := mcpserver.NewMCPServer("test", "1.0.0") + session := helper.WithContext(context.Background(), &fakeClientSession{id: "profiled-admin-session"}) + profiledAdmin := auth.WithAuthContext(session, auth.AdminContext()) + proxy.sessionStore.SetActiveProfile("profiled-admin-session", "research") + name, scope := proxy.resolveActiveProfile(profiledAdmin) + require.Equal(t, "research", name) + require.NotNil(t, scope) + require.False(t, scope.Allows("weather"), "premise: the session profile excludes weather") + + adminPage := readCachePage(t, proxy, profiledAdmin, k1, 0, 50) + require.False(t, adminPage.IsError, "caller-kind first: a profile-bound administrator redeems any snapshot (D5): %s", resultText(t, adminPage)) + setTruncateLimit(proxy, len(resultText(t, adminPage))/2) + adminTrunc := readCachePage(t, proxy, profiledAdmin, k1, 0, 50) + setTruncateLimit(proxy, 1_000_000) + require.False(t, adminTrunc.IsError) + match := cacheKeyRE.FindStringSubmatch(resultText(t, adminTrunc)) + require.Len(t, match, 2, "premise: the profiled administrator's oversize page mints a child key") + k2 := match[1] + rec, ok := proxy.cacheManager.Peek(k2) + require.True(t, ok) + require.NotNil(t, rec.Producer) + assert.Equal(t, cache.CallerKindAdmin, rec.Producer.CallerKind, "the child is an administrator snapshot") + // Under D5 the pinned agent below is refused by KIND whichever + // administrator snapshot the child carries, so the kind alone does not + // pin parent-stamping (critique round 2, finding 1). The parent was + // produced UNSCOPED; a child stamped with the redeemer would carry + // Profile "research", ProfileScoped true, ProfileServers {github}. + // The security assertion for monotone provenance across kinds is + // carried by TestReadCache_RecursiveChildInheritsParentProducer. + assert.False(t, rec.Producer.ProfileScoped, "the child carries the PARENT's unscoped snapshot, not the session-profiled redeemer's") + assert.Empty(t, rec.Producer.Profile) + assert.Empty(t, rec.Producer.ProfileServers) + + pinned := agentCtx([]string{"*"}, allPerms, "research") + absentKey := "0000000000000000000000000000000000000000000000000000000000000000" + child := readCachePage(t, proxy, pinned, k2, 0, 50) + absent := readCachePage(t, proxy, pinned, absentKey, 0, 50) + require.True(t, child.IsError, "a pinned wildcard agent must not redeem the administrator's child: %s", resultText(t, child)) + assert.NotContains(t, resultText(t, child), "weather:") + assert.Equal(t, resultText(t, absent), resultText(t, child), "MCP: the refusal is the nonexistent-key body") + + _, childErr := readCacheDirect(t, proxy, pinned, k2) + _, absentErr := readCacheDirect(t, proxy, pinned, absentKey) + require.Error(t, childErr, "REST: a pinned wildcard agent must not redeem the administrator's child") + require.Error(t, absentErr) + assert.Equal(t, absentErr.Error(), childErr.Error(), "REST: the refusal is the nonexistent-key body") + assert.Contains(t, childErr.Error(), "cache key not found", "REST: equality alone would also hold for a shared pre-check error") +} + +// (d) Pinned token on REST (Scope Boundary exception): a token allowing +// {github, weather} but pinned to research={github} is refused a direct call +// to weather through /api/v1/tools/call, and is refused — with the +// nonexistent-key body — the redemption of an unpinned entry containing +// weather through the same endpoint's read_cache branch. +func TestScopeCacheFixture_PinnedTokenRESTDispatchAndRedemptionParity(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + proxy.config.Servers = []*config.ServerConfig{{Name: "github", Enabled: true}, {Name: "weather", Enabled: true}} + proxy.config.Profiles = []config.ProfileConfig{{Name: "research", Servers: []string{"github"}}} + + unpinned := agentCtx([]string{"github", "weather"}, []string{auth.PermRead}, "") + pinned := agentCtx([]string{"github", "weather"}, []string{auth.PermRead}, "research") + + key, full := produceTruncatedKey(t, proxy, unpinned) + require.True(t, func() bool { + for _, tool := range full.Tools { + if strings.HasPrefix(fmt.Sprint(tool["name"]), "weather:") { + return true + } + } + return false + }(), "premise: the unpinned entry contains a weather tool") + + // Direct dispatch to the out-of-pin server is refused on the REST path. + _, dispatchErr := callToolDirectText(t, proxy, pinned, contracts.ToolVariantRead, + map[string]interface{}{"name": "weather:get_forecast", "args": map[string]interface{}{}}) + require.Error(t, dispatchErr, "the pin must refuse direct dispatch to weather") + assert.Contains(t, dispatchErr.Error(), "not in profile 'research'") + + // The producing (unpinned) token reads its entry on the same endpoint. + _, err := readCacheDirect(t, proxy, unpinned, key) + require.NoError(t, err, "control: the unpinned producer redeems its own entry on REST") + + // Cache redemption by the pinned token is refused with the + // nonexistent-key body. + _, redeemErr := readCacheDirect(t, proxy, pinned, key) + _, absentErr := readCacheDirect(t, proxy, pinned, "no-such-key") + require.Error(t, redeemErr, "the pinned token must not redeem an unpinned entry containing weather") + require.Error(t, absentErr) + assert.NotContains(t, redeemErr.Error(), "weather:") + assert.Equal(t, absentErr.Error(), redeemErr.Error(), "the redemption refusal must be the nonexistent-key body") + assert.Contains(t, redeemErr.Error(), "cache key not found") +} + +// (e) Held call — regression pin (passes on the merge base): the producer +// snapshot is captured when the call is authorized, not when the response +// comes back to be truncated. A session selection narrowed from {a,b} to {a} +// while the upstream call to b is in flight leaves the entry stamped {a,b}, +// and that entry is not redeemable under {a} on MCP or REST; the {a,b} +// session still reads it. +func TestScopeCacheFixture_HeldCallKeepsDispatchTimeSnapshot(t *testing.T) { + proxy, rt := createTestProxyWithRuntimeCfg(t, + []*config.ServerConfig{{Name: "a", Enabled: true}, {Name: "b", Enabled: true}}, + func(cfg *config.Config) { + cfg.Profiles = []config.ProfileConfig{ + {Name: "wide", Servers: []string{"a", "b"}}, + {Name: "narrow", Servers: []string{"a"}}, + } + }) + up := startCountingUpstream(t, proxy, rt, "b", readSpec("held")) + + // Replace the stub handler with one that blocks until released and then + // answers with a payload large enough to be truncated into a cache key. + records := make([]string, 0, 40) + for i := 0; i < 40; i++ { + records = append(records, fmt.Sprintf(`{"id":%d,"note":"SENTINEL_HELD padding padding padding padding padding"}`, i)) + } + payload := "[" + strings.Join(records, ",") + "]" + started := make(chan struct{}) + release := make(chan struct{}) + var startOnce sync.Once + up.mcpSrv.AddTool(mcp.Tool{Name: "held", Description: "Read held", InputSchema: mcp.ToolInputSchema{Type: "object"}}, + func(_ context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + up.record(request.Params.Name) + startOnce.Do(func() { close(started) }) + <-release + return mcp.NewToolResultText(payload), nil + }) + + const sid = "held-session" + helper := mcpserver.NewMCPServer("test", "1.0.0") + session := helper.WithContext(context.Background(), &fakeClientSession{id: sid}) + agent := auth.WithAuthContext(session, &auth.AuthContext{ + Type: auth.AuthTypeAgent, AgentName: "held-agent", TokenPrefix: "mcp_agt_held", + AllowedServers: []string{"a", "b"}, Permissions: []string{auth.PermRead}, + }) + proxy.sessionStore.SetActiveProfile(sid, "wide") + setTruncateLimit(proxy, len(payload)/4) + + req := mcp.CallToolRequest{} + req.Params.Name = contracts.ToolVariantRead + req.Params.Arguments = map[string]interface{}{"name": "b:held", "args": map[string]interface{}{}} + var ( + result *mcp.CallToolResult + err error + done = make(chan struct{}) + ) + go func() { + defer close(done) + result, err = proxy.handleCallToolVariant(agent, req, contracts.ToolVariantRead) + }() + select { + case <-started: + case <-time.After(10 * time.Second): + close(release) + t.Fatal("the upstream call never started") + } + // Narrow the session selection while the call is held, then let it finish. + proxy.sessionStore.SetActiveProfile(sid, "narrow") + close(release) + <-done + setTruncateLimit(proxy, 1_000_000) + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError, "the call authorized under {a,b} completes: %s", resultText(t, result)) + require.Equal(t, int64(1), up.count.Load()) + match := cacheKeyRE.FindStringSubmatch(resultText(t, result)) + require.Len(t, match, 2, "premise: the held call's oversize response mints a key") + key := match[1] + + rec, ok := proxy.cacheManager.Peek(key) + require.True(t, ok) + require.NotNil(t, rec.Producer) + assert.Equal(t, "wide", rec.Producer.Profile, "the entry is stamped with the profile in effect at dispatch") + assert.ElementsMatch(t, []string{"a", "b"}, rec.Producer.ProfileServers) + + // Under the narrowed session ({a}) the entry is not redeemable, on MCP or + // REST; the {a,b} session still reads it. + narrowed := readCachePage(t, proxy, agent, key, 0, 50) + assert.True(t, narrowed.IsError, "an entry stamped {a,b} must not be redeemable under {a}: %s", resultText(t, narrowed)) + assert.NotContains(t, resultText(t, narrowed), "SENTINEL_HELD") + _, restErr := readCacheDirect(t, proxy, agent, key) + assert.Error(t, restErr, "REST: an entry stamped {a,b} must not be redeemable under {a}") + + proxy.sessionStore.SetActiveProfile(sid, "wide") + wide := readCachePage(t, proxy, agent, key, 0, 50) + require.False(t, wide.IsError, "the {a,b} session reads the entry it produced: %s", resultText(t, wide)) + assert.Contains(t, resultText(t, wide), "SENTINEL_HELD") +} + +// (f) Empty server grant (codex round 1, MUST-FIX). Token creation +// normalises an empty allowed_servers to ["*"], but a server-edition +// rotation that narrows the grant persists nil, and auth.CanAccessServer / +// serverInScope treat that as deny-all: the token can call no upstream tool. +// Before the fix cache.CouldHaveProduced let coversServers([], []) succeed, +// so such a token could redeem an agent record whose snapshot also carried +// an empty grant — content the deny-all gates would never have let it +// produce. Now the gated predicate refuses an empty-grant agent reader +// outright, and the refusal is the nonexistent-key body on both doors. +func TestScopeCacheFixture_EmptyGrantAgentIsDenyAllOnRedemption(t *testing.T) { + proxy := createTestMCPProxyServer(t) + seedEntryBuilderFixture(t, proxy) + proxy.config.Servers = []*config.ServerConfig{{Name: "github", Enabled: true}, {Name: "weather", Enabled: true}} + + for _, grant := range []struct { + name string + allowed []string + }{ + {"nil grant", nil}, + {"empty list grant", []string{}}, + } { + t.Run(grant.name, func(t *testing.T) { + denyAll := agentCtx(grant.allowed, []string{auth.PermRead}, "") + require.False(t, auth.AuthContextFromContext(denyAll).CanAccessServer("github"), + "premise: an empty grant is deny-all on the dispatch gate") + + // Direct dispatch is refused on the REST path — the same door + // the redemption below goes through. + _, dispatchErr := callToolDirectText(t, proxy, denyAll, contracts.ToolVariantRead, + map[string]interface{}{"name": "github:list_repos", "args": map[string]interface{}{}}) + require.Error(t, dispatchErr, "premise: the empty grant refuses direct dispatch") + + // An identically-stamped deny-all record: the reader's own + // snapshot, byte for byte, as a producer. + stamp := proxy.cacheAuthorization(denyAll) + require.Equal(t, cache.CallerKindAgent, stamp.CallerKind) + require.Empty(t, stamp.AllowedServers) + key := strings.Repeat("e", 63) + map[bool]string{true: "0", false: "1"}[grant.allowed == nil] + require.NoError(t, proxy.cacheManager.StoreAs(key, "retrieve_tools", + map[string]interface{}{"query": "manage"}, `[{"name":"SENTINEL_EMPTY_GRANT"}]`, "", 1, stamp)) + absentKey := strings.Repeat("f", 64) + + // Control: an administrator redeems it (kind first), so the + // record is live and readable — the refusal below is the gate. + adminPage := readCachePage(t, proxy, adminCtx(), key, 0, 50) + require.False(t, adminPage.IsError, "control: administrator reads the empty-grant record: %s", resultText(t, adminPage)) + require.Contains(t, resultText(t, adminPage), "SENTINEL_EMPTY_GRANT") + + // MCP: live key ≡ absent key for the empty-grant reader. + live := readCachePage(t, proxy, denyAll, key, 0, 50) + absent := readCachePage(t, proxy, denyAll, absentKey, 0, 50) + require.True(t, live.IsError, "the empty-grant agent must not redeem an identically-stamped record: %s", resultText(t, live)) + require.True(t, absent.IsError) + assert.NotContains(t, resultText(t, live), "SENTINEL_EMPTY_GRANT") + assert.Equal(t, resultText(t, absent), resultText(t, live), "MCP: live key ≡ absent key for the empty-grant agent") + assert.Contains(t, resultText(t, live), "cache key not found") + + // REST (/api/v1/tools/call read_cache branch): same parity. + liveText, liveErr := readCacheDirect(t, proxy, denyAll, key) + _, absentErr := readCacheDirect(t, proxy, denyAll, absentKey) + require.Error(t, liveErr, "REST: the empty-grant agent must not redeem the record: %s", liveText) + require.Error(t, absentErr) + assert.Equal(t, absentErr.Error(), liveErr.Error(), "REST: live key ≡ absent key for the empty-grant agent") + assert.Contains(t, liveErr.Error(), "cache key not found") + + // The refusal does not evict: the administrator still reads it. + after := readCachePage(t, proxy, adminCtx(), key, 0, 50) + require.False(t, after.IsError, "a refused redemption of a stamped record must not evict it") + }) + } +} diff --git a/specs/105-agent-scope-hardening/contracts/refusals.md b/specs/105-agent-scope-hardening/contracts/refusals.md index fbdaaa66f..29fc86baa 100644 --- a/specs/105-agent-scope-hardening/contracts/refusals.md +++ b/specs/105-agent-scope-hardening/contracts/refusals.md @@ -14,6 +14,7 @@ Caller in every row: agent token `allowed={a}`, `perms={read}`, no pin, unless s | `call_tool_write a:write_tool` (authorized, over-tier) | `Permission denied … 'write'`, zero upstream | n/a | tier refusal is **not** non-disclosing by design | | `call_tool_read a:ghost` (unresolved identity on a known server) | insufficient-permission, zero upstream | same | every caller incl. admin (D4, SC-005 named exception); unknown-server branch unchanged | | `read_cache K` (K produced under broader scope) | `cache key not found` | `cache key not found` | ≡ on MCP and REST `/api/v1/tools/call`; internal/legacy keys same body | +| `read_cache K` (K admitted on the header, body undecodable/disagreeing) | `Cache entry is unreadable … invalidated` | `cache key not found` | **not** ≡ by design: the refusal shape is decided on the fixed record header only; a reader the header admits gets an admitted-class outcome (`ErrEntryUnreadable`, entry invalidated), the same body for every caller kind, so nothing sharing the not-found shape was decided on the body | | `code_execution script=missing` | error without `Available scripts` | same | ≡; admin enumerates | | `upstream_servers tail_log b` | `tailLogNotFound` (exists) | same | ≡ (regression) | | `set_profile ` | one format string (exists) | same | ≡ (regression) | diff --git a/specs/105-agent-scope-hardening/research.md b/specs/105-agent-scope-hardening/research.md index f9650d7c5..34a48af4b 100644 --- a/specs/105-agent-scope-hardening/research.md +++ b/specs/105-agent-scope-hardening/research.md @@ -88,3 +88,9 @@ Per [prior-diff-assessment.md](prior-diff-assessment.md): port hunks 1–6 and 1 ## D14 — Test harness sequencing (astra r1 finding 14) **Decision**: PRs A–G ship **standalone** tests (the per-gap tests in tasks.md) using the shared fixtures from Phase 1 (`scope_fixture_test.go`); H1 introduces `runScopeScenario` and re-registers those scenarios by US id. No PR depends on H1. Parallel PRs B/D/E/H0 share **no function**; B and E both edit `mcp.go` in disjoint regions (5613-5690 vs 5787-5803). Same-function hotspots that force serial order: A/G in `handleCallToolVariant`, C/G in `toolVisibleToSession`, F/G in `directEntryInScope`, direct catalog construction and direct describe resolution. + +## D16 — Header-only cache read gate (FR-001, SC-005; PR B codex round 5) + +**Decision**: the `read_cache` gate decides **in O(1) from the fixed frame header alone** and never loads the producer snapshot or decodes the value it refuses. It admits when (a) the reader is an administrator kind (FR-001 caller-kind-first, D5; the anonymous kind still never reads an authenticated administrator's entry), (b) the reader is an **unrestricted agent** — `AllowedServers` contains `"*"`, no pin, no effective profile — whose permission tier set covers the producer's (recorded as bits in the header; a superset of anything an agent could have produced), or (c) the reader's **effective-authorization digest equals the producer digest** stored in the header: SHA-256 of the canonical snapshot — caller kind, principal, sorted/deduplicated `AllowedServers`, sorted `Permissions`, `ProfilePin`, `ProfileScoped`, sorted `ProfileServers`; the profile *name* is excluded (the gate compares server sets, not names; a stale pin keeps its name while resolving to deny-all, which the header's deny-all bit refuses first). **Every other reader is refused** without loading anything: in particular a **strictly wider but bounded** reader — an `{a,b}` grant over an `{a}` entry, an unscoped session over its own profiled entry, the same user with a grown grant — is now refused. FR-001 obliges the door to refuse non-supersets; it does not oblige it to admit every superset, so refusing that rare shape is fail-closed and permitted; the predicate cells that pinned its admission are inverted with names stating the new contract. The snapshots bucket stays, keyed by the same digest, **for administrator diagnostics only** — no read path consults it. **Legacy (pre-frame / undecodable-frame) entries** are refused and deleted **without decoding**: the entry count is folded out exactly (no decode needed), the size is not (unknown without a payload-sized decode), and the existing cleanup sweep — which walks and decodes every record anyway — **recomputes `TotalEntries`/`TotalSizeBytes` exactly from the bucket**, so statistics are eventually consistent (over-count bounded by the legacy payload, for at most one `CleanupInterval`) while the refusal stays O(1). +**Rationale**: codex rounds 2–5 kept finding the same root: any refusal that must *load* the producer snapshot (round 4's content-addressed snapshot behind a 128-entry LRU was O(1) only when warm — a restart or 129 distinct snapshots made the first probe decode a fleet-sized snapshot) or *decode* the legacy value (round 4's one-shot `preFramePayloadSize`) does work proportional to hidden state, which the spec's non-disclosing refusal forbids in the timing class. Measured before the fix: a cold scope refusal against a 5,000-server producer allocated 1,657,864 B against a 58,104 B miss; a legacy 4 MB entry's refusal allocated 8,500,992 B. After: wide = small = miss = 25,112 B on every probe, cold included. Including the principal in the digest is what lets a user be gated on the header (identity is necessary for users) and makes "digest-equal" mean *the same credential* for agents; a same-scope sibling agent is therefore refused too (fail-closed, same rule). +**Alternatives**: a larger LRU or pre-warming at startup (still O(snapshot) somewhere, and a warm cache is itself hidden state); a bounded snapshot in the header (round 3, rejected in round 4: refused legitimate fleets); keeping the one-shot legacy decode (round 4, rejected in round 5: it is the oracle). Rejected. diff --git a/specs/105-agent-scope-hardening/tasks.md b/specs/105-agent-scope-hardening/tasks.md index cc9fd9387..1cba92b43 100644 --- a/specs/105-agent-scope-hardening/tasks.md +++ b/specs/105-agent-scope-hardening/tasks.md @@ -60,27 +60,27 @@ ### Failing tests -- [ ] T021 [US1] FR001-G1: store legacy (nil producer) record → every reader kind incl. admin/anonymous gets `ErrUnauthorizedRead`; `Peek` false; close+reopen bbolt → absent; server-level admin `handleReadCache` → IsError without `records` — `internal/cache/manager_legacy_test.go` (new) + `internal/server/mcp_read_cache_authz_test.go` -- [ ] T022 [P] [US1] FR001-G2: store `registry-servers:…` and `npm:…` keys via the runtime/guesser writers → admin `readCacheAs` IsError, no payload; agent live-vs-absent key identical text; entry still `Peek`-able (no eviction) — `internal/server/mcp_read_cache_authz_test.go` -- [ ] T023 [P] [US1] FR001-G3: store, rewrite `ExpiresAt` past, `Get` errs, `Peek` false, on-disk count == in-memory count — `internal/cache/manager_test.go` (`TestExpiredRecords` extended to assert absence) -- [ ] T024 [P] [US1] FR001-G4: agent W `["*"]` produces K1; admin pages K1 → K2; W reads K2 → success — `internal/server/mcp_read_cache_authz_test.go` -- [ ] T025 [P] [US1] FR001-G5: narrow ctx `[weather]`; live broad key vs nonexistent → identical text on `handleReadCache` and `CallToolDirect`; `cache key not found` substring kept — `internal/server/mcp_read_cache_authz_test.go` + `internal/server/mcp_call_tool_direct_test.go` -- [ ] T026 [P] [US1] FR001-G6: table — admin reader qualifies for ANY snapshot regardless of its profile (unscoped, narrower, wider, empty, deleted profile → true); agent reader vs admin-produced snapshot → false; pinned wildcard agent vs unpinned agent entry → false; empty-profile agent reader → false (deny-all guard for agents only) — `internal/cache/authorization_test.go` (D5 / FR-001 `spec.md:124`) -- [ ] T027 [P] [US1] FR001-G7: upgrade fixture (raw JSON record without `producer` and with `version:0`), fresh-internal-entry, recursive-child on MCP + REST, pinned-token REST (direct dispatch refused AND `read_cache` refused with nonexistent-key body), held-call narrowing (passes; regression) — `internal/server/scope_cache_fixtures_test.go` (new) +- [x] T021 [US1] FR001-G1: store legacy (nil producer) record → every reader kind incl. admin/anonymous gets `ErrUnauthorizedRead`; `Peek` false; close+reopen bbolt → absent; server-level admin `handleReadCache` → IsError without `records` — `internal/cache/manager_legacy_test.go` (new) + `internal/server/mcp_read_cache_authz_test.go` +- [x] T022 [P] [US1] FR001-G2: store `registry-servers:…` and `npm:…` keys via the runtime/guesser writers → admin `readCacheAs` IsError, no payload; agent live-vs-absent key identical text; entry still `Peek`-able (no eviction) — `internal/server/mcp_read_cache_authz_test.go` +- [x] T023 [P] [US1] FR001-G3: store, rewrite `ExpiresAt` past, `Get` errs, `Peek` false, on-disk count == in-memory count — `internal/cache/manager_test.go` (`TestExpiredRecords` extended to assert absence) +- [x] T024 [P] [US1] FR001-G4: agent W `["*"]` produces K1; admin pages K1 → K2; W reads K2 → success — `internal/server/mcp_read_cache_authz_test.go` +- [x] T025 [P] [US1] FR001-G5: narrow ctx `[weather]`; live broad key vs nonexistent → identical text on `handleReadCache` and `CallToolDirect`; `cache key not found` substring kept — `internal/server/mcp_read_cache_authz_test.go` + `internal/server/mcp_call_tool_direct_test.go` +- [x] T026 [P] [US1] FR001-G6: table — admin reader qualifies for ANY snapshot regardless of its profile (unscoped, narrower, wider, empty, deleted profile → true); agent reader vs admin-produced snapshot → false; pinned wildcard agent vs unpinned agent entry → false; empty-profile agent reader → false (deny-all guard for agents only) — `internal/cache/authorization_test.go` (D5 / FR-001 `spec.md:124`) +- [x] T027 [P] [US1] FR001-G7: upgrade fixture (raw JSON record without `producer` and with `version:0`), fresh-internal-entry, recursive-child on MCP + REST, pinned-token REST (direct dispatch refused AND `read_cache` refused with nonexistent-key body), held-call narrowing (passes; regression) — `internal/server/scope_cache_fixtures_test.go` (new) ### Implementation -- [ ] T028 [US1] `Record.Version` + `Authorization.Kind = internal`; `GetRecordsAs`: nil/unknown version → refuse + delete inside the committed `Update` (tx fn returns nil; refusal surfaced outside), stats mutate on commit only; internal → refuse without eviction; **caller kind first** — admin reader qualifies for any snapshot, agent never for an admin snapshot, deny-all guard applies to agent readers only; rename `Unrestricted()` → `IsAdministrator()` (D2, D5) — `internal/cache/models.go:26-43`, `internal/cache/manager.go:108-245`, `internal/cache/authorization.go:52-95` -- [ ] T029 [P] [US1] Stamp internal writers `CallerKindInternal` in `internal/runtime/runtime.go:2226` and `internal/experiments/guesser.go:349` -- [ ] T030 [US1] `ReadCacheResponse.Producer` (`json:"-"`) carries the parent's authorization to child page stores; `handleReadCache` collapses unauthorized/not-found/expired into the `cache key not found` body for agent callers (real BBolt errors distinct; activity log keeps the real reason) — `internal/server/mcp.go:5613-5690`, `internal/server/cache_authz.go:54-86`, `internal/server/content_forward.go:256` +- [x] T028 [US1] `Record.Version` + `Authorization.Kind = internal`; `GetRecordsAs`: nil/unknown version → refuse + delete inside the committed `Update` (tx fn returns nil; refusal surfaced outside), stats mutate on commit only; internal → refuse without eviction; **caller kind first** — admin reader qualifies for any snapshot, agent never for an admin snapshot, deny-all guard applies to agent readers only; rename `Unrestricted()` → `IsAdministrator()` (D2, D5) — `internal/cache/models.go:26-43`, `internal/cache/manager.go:108-245`, `internal/cache/authorization.go:52-95` +- [x] T029 [P] [US1] Stamp internal writers `CallerKindInternal` in `internal/runtime/runtime.go:2226` and `internal/experiments/guesser.go:349` +- [x] T030 [US1] `ReadCacheResponse.Producer` (`json:"-"`) carries the parent's authorization to child page stores; `handleReadCache` collapses unauthorized/not-found/expired into the `cache key not found` body for agent callers (real BBolt errors distinct; activity log keeps the real reason) — `internal/server/mcp.go:5613-5690`, `internal/server/cache_authz.go:54-86`, `internal/server/content_forward.go:256` ### Inverted pinned tests -- [ ] T031 [US1] Invert `internal/cache/authorization_test.go:65-72,93-121,123-154`, `internal/server/mcp_read_cache_authz_test.go:86,171`, `internal/server/mcp_call_tool_direct_test.go:35-48` (seed stamped records instead of unstamped) +- [x] T031 [US1] Invert `internal/cache/authorization_test.go:65-72,93-121,123-154`, `internal/server/mcp_read_cache_authz_test.go:86,171`, `internal/server/mcp_call_tool_direct_test.go:35-48` (seed stamped records instead of unstamped) ### Verification -- [ ] T032 [US1] Common verification + `go test -race ./internal/cache/... ./internal/runtime/... ./internal/experiments/...`; `-tags server` build to `/dev/null` +- [x] T032 [US1] Common verification + `go test -race ./internal/cache/... ./internal/runtime/... ./internal/experiments/...`; `-tags server` build to `/dev/null` - [~] T033 [US1] Live check: daemon with a pre-feature `config.db` copy; `read_cache` on an old key refused, key gone after restart - [~] T034 [US1] Astra rounds on FR-001/002 + FR001-G1…G7; quote final `VERDICT:`