From f1305bb1617bb742ff812396785ed7686e0e7cec Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 2 Sep 2026 21:26:09 -0400 Subject: [PATCH 01/18] docs(server): design gateway_credentials at-rest encryption + server_secrets store (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addendum to the frozen `compass-server-llm-gateway` record: encryption-at-rest for the `gateway_credentials` value store, master-key custody, and a physically separate `server_secrets` store for server-only secrets. Decisions (all Matt-ruled): - **D1** application-layer AES-256-GCM envelope encryption; ciphertext+nonce+ key_version columns, no plaintext value column ever; fresh 96-bit nonce per write (2^32-per-key bound quantified against the OAuth-refresh write rate). - **D2** 256-bit master key auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with an every-boot read-back verify; key never touches the DB. - **D3** managed-plane KMS is a provider-URI deployment config, not a code fork. - **D4** `key_version` column from day one (cheap now, expensive retrofit). - **D5** redaction beyond fmt verbs (unexported key bytes / slog.LogValuer + MarshalJSON), never the RPC serialization type. - **D6 (mechanism C1)** server-only secrets get a NEW physically separate `server_secrets` store — a names-only table + a SECOND SpecResolver instance over it (shared profile) + an admin-gated SetServerSecret/DeleteServerSecret RPC. The delivery boundary is the resolver's manifest, so a server secret is STRUCTURALLY undeliverable to agent containers (default-closed by construction). Removes the public proto change the earlier SERVER_ONLY-delivery-kind fold required; the container FetchSecrets path is byte-for-byte unchanged. The PEM/webhook/Linear server secrets — server_only by convention only today, riding the inject-all path into every agent container — move into `server_secrets` in this PR chain (a DB-row move, provider values untouched), closing that pre-existing exposure (OQ-4, Matt-ruled: same PR chain). Tasks: T0 server_secrets store (prerequisite) → T1 envelope pkg → T2 boot provision seam → T3 ciphertext columns → T4 seal/open wiring → T5 tripwire + redaction assertions. Open sub-fork flagged for review: the server-provisioned master key's `declared_by` FK is specced NULLABLE (NULL = server-provisioned; honest provenance, no boot-ordering coupling) rather than attributed to the bootstrap-admin account or an @compass sentinel — a one-line T0 change either way with no downstream effect. Ledger: DL-324 (Storage). Spec-impact: adds `server_secrets` table + SetServerSecret/DeleteServerSecret admin RPC + gateway_credentials ciphertext columns to the RIG-2863 build. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 1 + ...-gateway-credentials-at-rest-encryption.md | 644 ++++++++++++++++++ 2 files changed, 645 insertions(+) create mode 100644 docs/designs/server/compass-gateway-credentials-at-rest-encryption.md diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index a3c954fe..0cf3c9c5 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,6 +112,7 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | +| DL-324 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable) with a 256-bit master key auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify. The master key and the existing PEM/webhook/Linear server secrets live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, no delivery/kind columns, a SECOND `SpecResolver` instance over it on the shared profile, an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC carrying a reserved-name guard) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The PEM/webhook/Linear rows move into `server_secrets` in the same PR chain (DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable) | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext > resume = 12` envelope field is NOT silently reintroduced — the collapsed diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md new file mode 100644 index 00000000..a4a41b02 --- /dev/null +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -0,0 +1,644 @@ +# gateway_credentials at-rest encryption + master-key custody + +Status: Active + +Addendum to the frozen record +[`compass-server-llm-gateway`](./compass-server-llm-gateway/design.md) +(§Credential storage and rotation, L312–373). Scope: encryption-at-rest for the +`gateway_credentials` value store ONLY. This record does not re-open the +store's shape, the CAS `version` discipline, the scope column, the +RPC-vs-Postgres-direct wiring, or the stack-token channel — those stay as the +frozen record decided. Ships as its own PR; frozen on merge; the +credential-store build proceeds against it. +One deliberate scope expansion, Matt-ruled (D6/T0): server-only secrets get a +physically SEPARATE `server_secrets` store (mechanism C1) — a new table plus +a second resolver instance — so the master key and the existing +PEM/webhook/Linear server secrets are structurally undeliverable to agent +containers. C1 needs NO public proto change: it REMOVES the +`SECRET_DELIVERY_SERVER_ONLY` proto/store/secrets enum additions (and the +FetchSecrets filter) the earlier delivery-kind mechanism required. + +## Problem / Intent + +The gateway record adds `gateway_credentials`, the FIRST value-persisting store +in compass — deliberately breaking the names-only half of the secrets +invariant. The invariant it breaks is stated at +`go/internal/secrets/secrets.go:20-22`: + +> "Values live only in the provider and this process's memory during a +> resolve; they are never persisted by Compass and never logged." + +The frozen record specifies the new store's shape ("api_key and OAuth-shaped +payloads (access/refresh/expiry), a monotonic `version` per row supplying the +CAS substrate, and a scope column", design.md:324-326) but says nothing about +encryption at rest. A security red-team flagged this HIGH (CWE-311/312/522/532): +a plaintext value column makes every `pg_dump`, read replica, backup, and +operator `SELECT` a bulk all-tenant credential exfiltration path. The record +already concedes the blast radius is total at the process level +(design.md:333-337): + +> "Blast radius, stated: a compromised gateway (holding one stack token) can +> read every tenant's provider credentials — the isolation boundary is the +> per-tenant pool scoping enforced server-side on the credential-list surface +> (below), not the TS process, which by design holds them all." + +That concession covers a compromised *gateway process*; it does not license a +plaintext *database*. This addendum adds the missing at-rest-encryption and +master-key-custody decision before the store is built. + +## Approach + +Matt has ruled the core approach; this section records it as decisions. + +### D1 — Application-layer AES-256-GCM envelope encryption + +The credential value payload (the full api_key / OAuth +access+refresh+expiry JSON) is encrypted application-side before it reaches +Postgres: AES-256-GCM, a fresh random 96-bit nonce per encryption (per write, +never reused), authenticated. The `gateway_credentials` row stores ONLY +`ciphertext` + `nonce` (+ `key_version`, D4); no plaintext value column exists +in the schema. A DB dump, replica, or SELECT alone yields nothing usable. + +Why the fresh-nonce discipline is absolute, quantified: reusing a 96-bit GCM +nonce under one key forfeits both confidentiality AND authenticity — a +two-message catastrophe (keystream XOR + authentication-key recovery), not a +gradual weakening. Random 96-bit nonces are collision-safe only to ~2^32 +encryptions per key (NIST SP 800-38D). The write rate is not one-per-credential: +every hourly OAuth refresh write-back is a fresh encryption ("OAuth +access/refresh tokens are refreshed by the gateway on the hour", +design.md:322-323). At plausible scale (10^3–10^4 credentials, hourly) that is +~10^7–10^8 encryptions/year — comfortably under 2^32, so v1's single live key +has years of headroom; key rotation (OQ-1) is the nonce-budget release valve +when scale grows, which is itself part of why OQ-1's deferral is safe. + +This is the first at-rest crypto in the Go tree — verified by grep this +session: no `crypto/aes` / `cipher.NewGCM` usage exists anywhere under `go/` +(existing `crypto/rand` uses are IDs/tokens/certs only, e.g. +`go/internal/store/ids.go:4`). The envelope helper is therefore a new (small, +justified) package: no existing abstraction carries symmetric at-rest crypto, +and the seam must be reusable if a later store ever needs the same discipline. + +### D2 — Master key auto-provisioned into the existing SecretSpec provider + +The master key never touches the DB and never requires a human step +(rule://no-human-clicks). Every running Compass already has exactly one +configured SecretSpec provider — the seam described at +`go/internal/secrets/secrets.go:10-13`: + +> "this package reads that registry, generates the SecretSpec manifest the +> resolver resolves against, calls SecretSpec to resolve the actual values +> from the configured provider (keyring/1Password/Vault/…)" + +On boot, Compass resolves the declared master-key secret +(`GATEWAY_CREDENTIALS_MASTER_KEY`). If absent, it generates a fresh 256-bit +key from `crypto/rand` and — serialized against concurrent booters through a +Postgres advisory lock (T2) — provisions it: + +- writes the value into the provider via `secrets.Resolver.Set` + (`go/internal/secrets/resolver.go:219`, `func (r *SpecResolver) Set(ctx + context.Context, name, value string) error` — "Set writes value into the + provider for name via the pinned CLI, feeding the value on stdin (never + argv, so it is not visible in the host process list)", + resolver.go:206-207); +- registers the name in the SEPARATE `server_secrets` store (D6) via the + server-internal `DeclareServerSecret` (T0) — a mirror of + `store.DeclareSecret` (`go/internal/store/secrets.go:82`, `func (s *Store) + DeclareSecret(ctx context.Context, actor AccountID, name string, delivery + SecretDelivery, kind SecretKind, provider, host string) error` — "It + stores NO value — the value lives in the SecretSpec provider", + secrets.go:74-75) MINUS the delivery/kind routing parameters, which do not + exist for server secrets (they are never container-delivered and never + reach the T5 materializer) — never into the user `secrets` table, whose + every row rides the container-delivery manifest; +- re-resolves and byte-compares before the key is ever used to encrypt (the + read-back verify, T2). + +Thereafter the key is resolved at boot — through the SERVER-SECRET resolver +instance (D6), the second `SpecResolver` reading `server_secrets` — and held +in process memory only, following the existing declared-secret boot-resolve +pattern the Forge App PEM uses (`newDeclaredSecretResolver`, +`go/server/serve.go:1469`: "resolves the declared server_only secret NAME to +its raw value bytes on each call", serve.go:1463-1464 — a "server_only" that +is convention-only prose today; D6 makes it a physically separate store). +Unlike the webhook secret's per-request resolve, the master key is resolved +once at startup and cached for the process lifetime +(a decrypt happens on every credential read; a provider round-trip per decrypt +would be the same amplification `cachedWebhookSecret` exists to prevent, +serve.go:1484-1491). + +### D3 — Managed plane: KMS as just another provider URI, zero core change + +The managed plane points the ONE declared master-key secret at a KMS-backed +SecretSpec provider URI. The core stays cloud-agnostic; there is no KMS SDK, +no cloud-conditional code path, and no self-host/managed fork in the crypto +code. KMS-grade custody is a deployment configuration, not a code change. + +### D4 — Key versioning column from day one + +Each row carries a `key_version` (smallint, starts at 1) identifying which +master-key generation encrypted it. Whether v1 ships active rotation is an +open question (OQ-1), but the column is load-bearing NOW: adding it later +means a schema migration plus a backfill under ambiguity about which key +encrypted which row. Cheap at creation, expensive retrofit. + +### D5 — Redaction discipline: Stringer alone is not enough + +Breaking the never-persisted half of the invariant does not touch the +never-logged half — but the established Stringer pattern +(`ResolvedSecret.String()`/`GoString()`, `go/internal/secrets/secrets.go:150-156`: + +> `func (s ResolvedSecret) String() string { return fmt.Sprintf("ResolvedSecret{name: %q, kind: %d, delivery: %d, value: }", …) }` +> `func (s ResolvedSecret) GoString() string { return s.String() }` + +) covers only fmt-verb formatting — and slog under a TextHandler, which +formats via the fmt path. It does NOT cover a slog `JSONHandler` (reflection +over exported fields, ignores `String()`), `json.Marshal`, or direct field +access — and T4's decrypted-payload type is exactly the OAuth JSON shape that +gets marshaled toward the gateway RPC, so it will have serializable fields. +A one-line handler swap must not silently un-redact the fleet. + +Therefore the value-bearing types this record adds MUST either: + +- keep secret fields UNEXPORTED with accessor methods (the `envelope.Key` + posture: unexported `[32]byte`, reflection-proof), or — where the payload + shape needs exported fields — implement `slog.LogValuer` AND a redacting + `MarshalJSON` alongside the `String()`/`GoString()` pair; and +- never serve as the RPC serialization type: the gateway-bound response is + built by an explicit proto/DTO conversion, never `json.Marshal` of the + payload type itself. + +The store never logs a row's value fields, and error paths wrap without +embedding plaintext or key material. + +### D6 — A physically separate `server_secrets` store (mechanism C1, Matt-ruled) + +The threat D2 must defeat: the declared-secrets registry is inject-all by +design — "NO per-agent grant column (the MVP injects the whole store into +every agent; per-agent scoping is a named FUTURE seam)" +(`go/internal/store/migrations/0001_init.sql:379-381`). FetchSecrets resolves +the WHOLE registry with no filter — `resolved, err := h.resolver.Resolve(ctx, +"runner fetch")` then `for _, s := range resolved { out = append(out, +resolvedSecretToProto(s)) }` (`go/internal/runnerhub/handler.go:303-311`) — +and the Runner materializes every resolved value into the container pre-exec +(`resolved, err := h.link.FetchSecretsByContainer(ctx, name)` → +`h.materializer.Install(ctx, handle.ID(), handle.HomeDir(), +handle.WorkspaceUID(), resolved)`, `go/internal/runner/host.go:378-383`). +Declared into the `secrets` table at all, the master key — which decrypts +every tenant credential — would land on disk inside every untrusted agent +sandbox, defeating the encryption entirely. + +**Matt ruled: a separate store, not a delivery flag.** Server-only secrets +get their OWN physically separate table, `server_secrets`, so they can NEVER +get mixed up with user secrets that are delivered to agent containers. +Container delivery becomes default-CLOSED by construction, not +default-open-minus-a-filter. + +Why the table boundary IS the delivery boundary: the delivery surface is the +resolver's MANIFEST. `SpecResolver` reads its declared set through the +`declarations` interface — `DeclaredSecrets(ctx context.Context) +([]store.SecretDeclaration, error)` (`go/internal/secrets/resolver.go:29-31`; +the `store declarations` struct field, resolver.go:57) — and `buildManifest` +"renders the SecretSpec manifest TOML for a declared set: one `[project]` +block and one `[profiles.]` block with every declared name as a +required key" (resolver.go:99-101; the function, resolver.go:105-128); +`Resolve` can only return names present in that manifest. Today ONE resolver +instance (`resolver := secrets.NewSpecResolver(st, secretsStateDir(cfg))`, +`go/server/serve.go:528`) serves BOTH the container path (FetchSecrets → +`Resolve`, handler.go:303-311) and the boot consumers (PEM/webhook via +`newDeclaredSecretResolver` → `Resolve`, serve.go:1469-1481). C1 splits the +READ: a store view over `server_secrets` (`ServerDeclaredSecrets`, T0) feeds +a SECOND `SpecResolver` instance. The container resolver keeps reading +`secrets`; its manifest never contains a server-secret name, so a server +secret is STRUCTURALLY undeliverable to containers — there is nothing to +filter. + +C1 keeps the SAME SecretSpec profile for both instances — the shared project +is `manifestProject = "compass"` (resolver.go:19) and the profile +`defaultProfile = "default"` (resolver.go:23; `WithProfile` exists, +resolver.go:78, but C1 does not use it) — so the provider keyspace is shared +and moving an existing secret between the two tables is a DB-row move with +the provider value untouched (load-bearing for OQ-4). + +**C1 REMOVES the public proto change** the previous delivery-kind mechanism +required: no `SECRET_DELIVERY_SERVER_ONLY` proto enum value, no +`SecretDeliveryServerOnly` store enum, no secrets-package +`DeliveryServerOnly`, no widened migration CHECK, and no FetchSecrets +delivery filter. The container FetchSecrets path is byte-for-byte UNCHANGED. +The delivery enum stays two-valued in all four representations: +`delivery SMALLINT NOT NULL CHECK (delivery IN (0, 1))` (0001_init.sql:388); +`SecretDeliveryFile SecretDelivery = 0` / `SecretDeliveryEnv SecretDelivery += 1` (`go/internal/store/secrets.go:18-25`); `DeliveryFile` / `DeliveryEnv` +(`go/internal/secrets/secrets.go:39-46`); `SECRET_DELIVERY_UNSPECIFIED = 0; +SECRET_DELIVERY_FILE = 1; SECRET_DELIVERY_ENV = 2;` +(`proto/compass/v1/compass.proto:160-164`). + +Writes into `server_secrets` go through a NEW admin-gated +`SetServerSecret`/`DeleteServerSecret` RPC (T0) — mirroring the user +`SetSecret` declare-then-Set flow (`go/server/secrets_service.go:92`) minus +delivery/kind, classified `adminOnly` in `classifyProcedure` +(`go/internal/auth/admin_gate.go:47`, :27; "An unrecognized path (ok=false) +is treated as adminOnly — fail closed, never admit an unknown method as +open", admin_gate.go:44-46). Today's user-facing `SetSecret` could not carry +a server secret anyway: `secretRoutingFromProto` accepts only File/Env +delivery (secrets_service.go:275-284) — C1 routes server secrets to the new +RPC instead. + +This retroactively fixes a pre-existing exposure: the App PEM, webhook +signing, and Linear OAuth secrets are server_only by convention only +("Declared server_only secret NAME holding the PRIMARY App PEM private key", +`go/cmd/compass-server/main.go:414-417`; webhook :418-421; Linear :435-450; +boot-resolved by name via `newDeclaredSecretResolver`, serve.go:1469-1481) +and today ride the same inject-all path into every agent container. Their +declared rows MOVE from `secrets` into `server_secrets` in this PR chain +(OQ-4, RESOLVED) — a DB-row move only, since the profile is shared. + +## Alternatives considered + +The core choice (envelope encryption + auto-provisioned key in the existing +provider) is Matt-ruled and not re-opened; the custody fork and the +server-secret containment-mechanism fork are recorded here because the +rejected branches are the ones a future reader will reach for first. + +- **Mechanism A: a real SERVER_ONLY delivery kind + FetchSecrets filter — + REJECTED (superseded by Matt's C1 ruling).** The previously folded design: + mint `SERVER_ONLY` across the four delivery representations (migration + CHECK widen, store enum, secrets package, a PUBLIC proto enum value) plus + a delivery filter in the FetchSecrets handler skipping SERVER_ONLY rows + before the append (handler.go:307-310). Rejected because it is + default-OPEN minus a subtractive filter: server and user secrets share one + table and one resolver manifest, and one edit — a dropped filter clause, a + mis-mapped enum arm — re-exposes the master key to every container. It + also costs a public proto change C1 does not need. +- **Mechanism C2: separate table + a separate SecretSpec PROFILE — + considered, DEFERRED.** The same `server_secrets` table, but the server + resolver pinned to its own profile (`WithProfile`, resolver.go:78) so even + the provider keyspace is isolated. Fullest isolation, but migrating the + existing PEM/webhook/Linear secrets would require RE-PROVISIONING their + provider values under the new profile — a provider-write migration over + live deployments. C1's manifest separation already delivers the + container-undeliverable property with a DB-row-only move; the profile + split remains available later if provider-keyspace isolation is ever + needed. +- **pgcrypto / key-in-DB — REJECTED.** `pgp_sym_encrypt` or a key stored in a + Postgres table/GUC puts the key inside the same blast radius as the + ciphertext: one `pg_dump` carries both, reducing the encryption to obfuscation. + The red-team requirement is specifically that the key lives OUTSIDE the DB. +- **Require an external cloud KMS — REJECTED for the OSS core.** A generic + self-hoster has no AWS/GCP KMS and must not need one; they have already + configured exactly one SecretSpec provider (keyring / 1Password / Vault / + env) to run Compass at all. Mandating KMS either forks the code + (self-host vs managed paths) or gates self-hosting on a cloud account. + The chosen approach *subsumes* this alternative: the managed plane gets + KMS-backed custody by pointing the declared secret at a KMS provider URI + (D3) — KMS becomes deployment config, not a code requirement. +- **Plaintext now + filed follow-up — REJECTED.** Ships a known HIGH hole and + creates a migration burden (encrypt-in-place over live rows) that the + build-it-encrypted-first ordering avoids entirely. Matt ruled the encryption + record lands BEFORE the store is built. + +## Plan + +### Global Constraints + +- Go server under `go/` (gateway itself is TS/Bun, but this record's code is + entirely Go server + store side); existing lint/test discipline applies. +- AES-256-GCM only; nonces from `crypto/rand`, 96-bit, fresh per encryption, + never counter-derived; key is 256-bit from `crypto/rand`. +- The master key NEVER appears in the DB, in logs, in argv (Set feeds stdin, + resolver.go:206-207), or in error strings. +- Auto-provisioning is zero-human-step (rule://no-human-clicks): first boot + generates, stores, and declares the key with no operator action. +- The names-only invariant (`secrets.go:20-22`) is preserved for EVERYTHING + except `gateway_credentials` values; both declared-name registries + (`secrets` AND the new `server_secrets`) stay names-only — the master + key's declaration row is names-only like any other. +- Every value-bearing type redacts under `%s`/`%v`/`%#v` (D5). +- The master key lives in the separate `server_secrets` store (D6): its name + never appears in the container resolver's manifest, so it is NEVER + materialized into an agent container — a structural property of the split + store, not a filter to maintain. +- Every Seal binds AAD = the row's stable identity (T4); ciphertexts are not + portable between rows. + +### T0 — The `server_secrets` store: table, resolver split, admin RPC (prerequisite) + +Implements D6 (mechanism C1). PREREQUISITE of T2 — the key cannot be +declared into a store that does not exist. + +- `Interfaces:` + - Migration (a NEW migration file, never an edit to 0001): `CREATE TABLE + server_secrets` — names-only, mirroring `secrets` + (0001_init.sql:382-413) MINUS the container-delivery routing: `name TEXT + PRIMARY KEY` (same env-var-name grammar, validated at the store door); + `declared_by TEXT REFERENCES accounts (id) ON DELETE RESTRICT`, + NULLABLE — `NULL` = server-provisioned (the master key; contrast + `secrets.declared_by`, which is `NOT NULL REFERENCES accounts (id) ON + DELETE RESTRICT`, 0001_init.sql:397-399 — justification below); + `created_at`/`updated_at`. NO `delivery` column (server secrets are + never container-delivered — that IS the point) and NO `kind` column + (they never reach the T5 materializer). + - declared_by nullability, justified: the boot provisioner declares the + master key with no human actor. `NULL` = server-provisioned is honest + provenance; attributing the row to the bootstrap-admin account + (available at that point in boot, serve.go:358-362) would falsify the + audit trail and couple key provisioning to account-bootstrap ordering. + Operator-declared rows (the new RPC) always carry the caller's account + id; `ON DELETE RESTRICT` still protects them. + - Store: `DeclareServerSecret(ctx, actor, name)` / + `DeleteServerSecretDeclaration(ctx, actor, name)` mirroring + `DeclareSecret` / `DeleteSecretDeclaration` (store/secrets.go:82, :160) + minus delivery/kind/provider/host (actor nullable-empty for the + server-provisioned path), plus the read `ServerDeclaredSecrets(ctx)` — + a thin store view whose `DeclaredSecrets(ctx context.Context) + ([]store.SecretDeclaration, error)` method (the `declarations` + interface shape, resolver.go:29-31) reads `server_secrets`, mapping + rows to `store.SecretDeclaration` with generic kind / zero delivery + (the resolver uses only the name to build its manifest), so + `NewSpecResolver` is reused UNCHANGED. + - Resolver split: a SECOND `SpecResolver` instance constructed over that + view — SAME profile (`defaultProfile = "default"`, resolver.go:23) and + project (`manifestProject = "compass"`, resolver.go:19), its own + manifest state dir. The container resolver (serve.go:528) is UNCHANGED + — no filter, nothing to filter. + - RPC: `SetServerSecret`/`DeleteServerSecret` on the secrets service — + mirrors the `SetSecret` declare-then-Set flow and its rollback + discipline (secrets_service.go:92-145) minus delivery/kind, targeting + `server_secrets`; admin-gated (`adminOnly` in `classifyProcedure`, + admin_gate.go:47). Carries the F3 reserved-name guard: rejects + `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix) with an + actionable error — rotation is OQ-1 machinery, never a raw overwrite. + - Row migration: MOVE the existing declared App-PEM / webhook / Linear + rows (the main.go:414-421, :435-450 conventions) from `secrets` into + `server_secrets` in the same migration — a DB-row move only; provider + values stay put under the shared profile (no re-provision). Closes the + pre-existing inject-all exposure (D6). Same PR chain: OQ-4, RESOLVED. +- Consumes: nothing from T1-T5 (pure prerequisite). +- Tests: a secret declared in `server_secrets` is ABSENT from a + FetchSecrets response (because it lives in the other table — no filter + involved) while `secrets` rows in the same boot ARE present; the server + resolver instance RESOLVES it; the container resolver's generated + manifest never contains its name; the new RPC rejects a non-admin + caller; the reserved master-key name is rejected on + SetServerSecret/DeleteServerSecret; the migrated PEM/webhook/Linear + names resolve through the server resolver and are gone from + FetchSecrets. + +### T1 — Envelope-crypto helper package + +New package `go/internal/secrets/envelope` (child of the secrets seam it +serves; no import cycle — it depends on nothing in `secrets`). + +- `Interfaces:` + - `type Key struct{ /* unexported [32]byte */ }` — redacts under + `%s`/`%v`/`%#v` (`String()`/`GoString()` → `"envelope.Key{}"`). + - `func NewKey() (Key, error)` — 32 bytes from `crypto/rand`. + - `func KeyFromBytes(b []byte) (Key, error)` — errors unless len == 32. + - `func (k Key) Seal(plaintext, aad []byte) (ciphertext, nonce []byte, err error)` + — AES-256-GCM, fresh 96-bit random nonce per call; `aad` is + authenticated, not encrypted (binds the ciphertext to its row identity — + T4). + - `func (k Key) Open(ciphertext, nonce, aad []byte) ([]byte, error)` — GCM + auth failure (tamper OR aad mismatch) returns an error naming no + plaintext/key material. + - Key encoding for provider storage: base64(std) of the 32 raw bytes + (SecretSpec values are strings; `Set` rejects empty, resolver.go:225-227). +- Consumes: `crypto/aes`, `crypto/cipher`, `crypto/rand` only. +- Tests: round-trip; tamper (flip a ciphertext/nonce byte → error); `Open` + under a different `aad` → error; nonce uniqueness across calls; redaction + of `Key` under all three verbs; `KeyFromBytes` length validation. + +### T2 — Master-key boot-provision seam + +Boot-time resolve-or-provision, in the server wiring next to the existing +declared-secret consumers (`go/server/serve.go`). DEPENDS ON T0 (the +`server_secrets` store and its resolver instance must exist before the key +is declared into it) and T1. + +- `Interfaces:` + - `func provisionGatewayMasterKey(ctx context.Context, resolver secrets.Resolver, st *store.Store) (envelope.Key, error)` + — `resolver` is the SERVER-SECRET resolver instance (T0). Resolve + `GATEWAY_CREDENTIALS_MASTER_KEY` through it; on absence: + `envelope.NewKey()` → `resolver.Set(ctx, name, encodedKey)` + (resolver.go:219; the value rides stdin, never argv, + resolver.go:206-207) → `st.DeclareServerSecret(ctx, "", name)` with + `declared_by = NULL` (server-provisioned; T0's nullable FK). No + delivery, no kind — those columns do not exist on `server_secrets`. + - **Concurrency — advisory-lock serialized (mandatory):** the whole + resolve→generate→Set→Declare sequence runs under a Postgres advisory + lock (`pg_advisory_xact_lock` on a constant key) — Postgres is the one + store all instances share. This replaces the draft's Set-then-Declare + with tolerated ErrConflict, which was a check-then-set race: two + concurrently booting instances both resolve-absent and both Set (last + writer wins in the provider); the loser's Declare hits ErrConflict, is + tolerated, and that instance proceeds to encrypt with a key the + provider no longer holds — silently undecryptable rows, discovered at + read time. The write path this builds on explicitly disclaims + concurrent safety: "The declare/set/rollback trio is not atomic and + assumes no concurrent same-name writer (the single-Runner MVP: + SetSecret is user-driven CLI)" (go/server/secrets_service.go:88-91). + Ordering inside the lock stays Set-before-Declare: the inverse leaves a + crash-window orphan declaration, and "an orphaned declaration is + required=true in the resolve manifest and would poison EVERY live + session's FetchSecrets" (secrets_service.go:86-88) — under C1 the + blast radius shifts but stays severe: an orphaned `server_secrets` + declaration is required=true in the SERVER resolver's manifest and + would fail every server-side resolve (master key, PEM, webhook, + Linear). A crash between Set + and Declare converges on the next boot (the undeclared name does not + resolve, so the provisioner re-generates, re-Sets, and Declares — no + row was ever encrypted under the orphaned value). + - **Read-back verify, every boot:** after provisioning AND on every + subsequent boot, re-resolve the name and byte-compare against the key + the process is about to encrypt with; on mismatch, refuse to serve + gateway-credential writes (fail closed). This holds regardless of the + lock: the provider is still a shared mutable surface (an operator can + write the name out-of-band through the secretspec CLI), even though the + user-driven SetSecret RPC can no longer race this name — it targets + `secrets` and cannot reach `server_secrets`, and the admin RPC's + reserved-name guard rejects it (F3). + - **Reserved name (F3, now partly structural):** the user-secret + `SetSecret`/`DeleteSecret` path targets `secrets` and physically CANNOT + reach `server_secrets` — it cannot touch the master key at all. (The + exposure this closes structurally: user-path deletes are name-keyed and + global — "a row is keyed by name alone, not (actor, name): any declared + name is a legal delete target regardless of who declared it", + go/internal/store/secrets.go:150-153 — and the SecretsService rides the + ambient-identity 0600 socket, serve.go:711-715.) The guard that remains: + the NEW `SetServerSecret`/`DeleteServerSecret` RPC (T0) — the one + surface that CAN reach `server_secrets` — rejects + `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix) with an + actionable error. Rotation is OQ-1's machinery, never a raw overwrite + through this surface. + - **Nil-resolver deployment:** a server built with no secrets surface is + legitimate today ("resolver may be nil on a server built with no + secrets surface (FetchSecrets then fails CodeFailedPrecondition rather + than panicking)", go/internal/runnerhub/handler.go:52-54; agents still + start, go/internal/runner/host.go:367-370). On such a server the + gateway credential store is NOT constructed; enabling the gateway is a + configuration error naming the missing secrets surface; a no-gateway + boot proceeds unchanged. + - Resolve path mirrors `newDeclaredSecretResolver` (serve.go:1469) but is + pointed at the SERVER-SECRET resolver instance and invoked once at + boot; the decoded `envelope.Key` is held in memory for the process + lifetime. + - Boot fails closed: a resolve/provision fault is a startup error, never a + fall-back-to-plaintext. +- Consumes: T0's `server_secrets` store + server resolver instance, T1 + `envelope`, `secrets.Resolver`, `store.DeclareServerSecret`. +- Produces: the process-lifetime `envelope.Key` handed to T4. +- Tests: fresh-boot provisions (Set + Declare called, key usable); + second-boot resolves without Set; two-writer interleaving (concurrent + provisioners converge on ONE key both read back identically); + Set-succeeded/Declare-crashed reboot converges; read-back mismatch → + fail closed; reserved-name SetServerSecret/DeleteServerSecret → + actionable reject; + nil-resolver + gateway enabled → configuration error naming the missing + surface; nil-resolver without gateway → boot proceeds unchanged; + provider fault → boot error. + +### T3 — Schema: ciphertext columns on gateway_credentials + +The `gateway_credentials` migration (owned by the store build the frozen +record plans) carries, for the value payload: + +- `Interfaces:` columns `value_ciphertext BYTEA NOT NULL`, + `value_nonce BYTEA NOT NULL`, `key_version SMALLINT NOT NULL DEFAULT 1`. + NO plaintext value column exists in any migration version — the store is + born encrypted (per the rejected plaintext-first alternative). +- The row's non-secret metadata (provider, scope, owner_user_id, `version` + CAS counter, expiry timestamp if queried-on) stays plaintext-queryable; + ONLY the credential value payload is inside the envelope. Expiry inside + vs. beside the ciphertext is settled by the store build; default: beside + (the gateway lists refreshable rows by expiry without decrypting). +- **Dependency edge, explicit:** this migration task DEPENDS ON T1+T2 + merged (same PR chain or a stated blocking dependency in the store + build's tracker) — the encrypt-before-any-row ordering must be + structural, not a prose sentence. `gateway_credentials` exists nowhere + under `go/` today (grep this session, zero matches), so the ordering is + currently satisfiable with no existing-rows hazard. T5's + ciphertext-at-rest assertion is the named CI tripwire that reds if a + plaintext value column ever appears. +- Tests: migration applies; NOT NULL enforced. + +### T4 — Wire crypto into the CredentialStore read/write path + +Every write path (initial credential save, gateway OAuth-refresh write-back +per design.md:367-371) seals before INSERT/UPDATE; every read path opens +after SELECT. This sits server-side under the RPC surface the frozen record +recommends (design.md:348-358), so the TS gateway never sees the key or the +crypto — it receives plaintext credentials over the stack-token-authenticated +RPC exactly as the frozen record already specifies. + +- `Interfaces:` the store's credential accessors take/return the decrypted + payload type (per D5: secret fields unexported with accessors, or + `slog.LogValuer` + redacting `MarshalJSON` alongside + `String`/`GoString`); the gateway-bound RPC response is built by an + explicit proto/DTO conversion, never by marshaling the payload type. The + `envelope.Key` is a construction-time dependency of the store/service + wrapper, not a per-call parameter. **AAD binds row identity:** every + Seal/Open passes `aad = row primary key + key_version` — the STABLE row + identity, never the CAS `version` counter (which increments per write). + Without AAD an attacker with DB write access can swap two rows' + ciphertext+nonce pairs and both still authenticate — a cross-tenant + credential substitution crossing exactly the boundary the frozen record + names ("the isolation boundary is the per-tenant pool scoping enforced + server-side", design.md:333-337). Writes stamp the current `key_version`; + reads select the key by the row's `key_version` (v1: single live key — a + mismatched version is a diagnosable error naming the expected/found + version numbers, never key material; see OQ-1). +- Consumes: T1, T2's key, T3's columns. +- Tests: see T5. + +### T5 — Tests: ciphertext-at-rest + redaction assertions + +- `Interfaces:` (test-only) + - **Ciphertext-at-rest assertion (the named CI tripwire, T3):** write a + credential through the store, then read the raw row via SQL — assert + the known plaintext substring (e.g. the api key literal) appears + NOWHERE in any column; assert `value_ciphertext != plaintext` and + decrypt-with-key round-trips. This is the guard that reds if a + plaintext value column ever appears. + - **Row-swap assertion (AAD):** swap two rows' ciphertext+nonce pairs at + the SQL level — `Open` MUST fail for both rows (the AAD binds row + identity). + - **Redaction assertion (beyond fmt verbs):** format the + decrypted-payload type and `envelope.Key` under `%s`, `%v`, `%+v`, + `%#v`; `json.Marshal` the payload type; log it through a + `slog.JSONHandler` capture — assert no secret bytes appear in ANY of + these. The fmt verbs alone mirror the existing pattern's intent + (secrets.go:155-156) but do not cover the reflection paths (D5). + - Refresh write-back path: refreshed OAuth tokens land re-sealed with a + fresh nonce (nonce differs from the previous row state). + - Wrong-key / tampered-row read → error, no partial plaintext; a + key_version-mismatch error is distinguishable from a GCM auth failure + (names the versions, never key material). + +## Tasks + +- [ ] T0 — `server_secrets` store (mechanism C1): new migration (table + + PEM/webhook/Linear row move), `ServerDeclaredSecrets` store view + + second SpecResolver instance (same profile), admin-gated + SetServerSecret/DeleteServerSecret RPC with the reserved-name guard, + absent-from-FetchSecrets + server-resolver-resolves red-green tests. + PREREQUISITE of T2. NO proto change. +- [ ] T1 — `go/internal/secrets/envelope`: Key/NewKey/KeyFromBytes/Seal/Open + (AAD-carrying) + redaction + unit tests +- [ ] T2 — boot resolve-or-provision seam via the server-secret resolver + (advisory-lock serialized, read-back verify, nil-resolver gating, + fail-closed boot; the reserved-name guard lives on T0's RPC). + DEPENDS ON T0 + T1. +- [ ] T3 — `gateway_credentials` migration columns: `value_ciphertext`, + `value_nonce`, `key_version`; no plaintext column ever. DEPENDS ON + T1+T2 merged (the store build's migration blocks on them). +- [ ] T4 — seal/open wiring in the CredentialStore read/write + refresh + write-back paths; AAD = row PK + key_version; key as + construction-time dependency +- [ ] T5 — ciphertext-at-rest assertion (CI tripwire), row-swap AAD test, + redaction incl. json.Marshal + slog JSONHandler, tamper/wrong-key + tests + +## Open Questions + +- **OQ-1 (deferrable, recommendation: defer): master-key rotation + + row re-encryption.** How does the master key rotate and how are existing + rows re-encrypted? Recommendation: the `key_version` column lands at v1 + (D4 — load-bearing now because retrofitting it is a migration under + ambiguity), but active rotation machinery — lazy re-encrypt on next write + plus a one-shot re-encrypt sweep — is DEFERRABLE to a follow-up record; v1 + runs a single live key version and treats a version mismatch as an error. + Why deferral is SAFE, not merely convenient: (1) provider key loss makes + every row unrecoverable, but the blast radius is BOUNDED — gateway + credentials are re-obtainable from their owners (re-enter the API key, + re-run the OAuth grant; the store holds api_key and OAuth-shaped payloads + a user configures, parent design.md:324-326), so the worst case is a + fleet re-authentication event, never permanent data loss; (2) rotation is + also the nonce-budget release valve, and D1's write-rate arithmetic shows + years of headroom under the 2^32 bound. Carried into T4/T5 regardless of + deferral: a wrong-key / key_version-mismatch decrypt error MUST be + operator-distinguishable from row corruption — the error names the + expected/found key_version numbers, never key material. +- **OQ-2 — RESOLVED (Matt; mechanism updated by the C1 ruling): master-key + containment.** The master key is declared into the physically separate + `server_secrets` store that D6/T0 build, boot-resolved through the + server-secret resolver instance (mirroring the Forge App PEM's + serve.go:1469 pattern), and never materialized into an agent container — + its name is not in the container resolver's manifest at all. An earlier + fold resolved this with a minted SERVER_ONLY delivery kind + FetchSecrets + filter; Matt's C1 ruling replaced that with the separate store (see + Alternatives, mechanism A) and REMOVED the public proto change it + required. +- **OQ-3 (load-bearing, recommendation: yes): encrypt api_key rows + identically to OAuth rows?** Recommendation: **yes** — one seal/open code + path for both payload shapes; both are secrets of the same sensitivity, and + a split path doubles the audit surface for zero benefit. Load-bearing: T3's + schema and T4's wiring assume one path. +- **OQ-4 — RESOLVED (Matt: same PR chain, mechanism C1): the + PEM/webhook/Linear server-secret rows move to `server_secrets` in THIS + record's PR chain**, closing the live pre-existing exposure (they ride + the inject-all path into every agent container today, D6). NOTE: C1 + makes the move cheap — because both resolver instances share one + SecretSpec profile (`defaultProfile`, resolver.go:23), this is a DB-row + move only: provider values stay put and nothing is re-provisioned. New + operator declarations go through the admin-gated `SetServerSecret` RPC + (T0); the row move for the ALREADY-declared names ships in T0's + migration. From 2d3cbd94560817f0b8e6aaaca34ba99aa231b141 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 13:27:59 -0400 Subject: [PATCH 02/18] docs(server): address review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 review of #859 found 2 high + 4 medium, all folded (no Matt ruling relitigated): - **F1 (high)** — corrected the false "user SetSecret path physically CANNOT reach the master key" claim: C1 splits the declaration registries, NOT the shared SecretSpec provider keyspace, so the reserved-name guard is now mandatory on BOTH the admin RPC and the authenticatedOpen user SetSecret/DeleteSecret path. Added the user-path red-green test and a boot key-fingerprint tripwire (T5) so a swapped provider value is detected, not silently adopted. - **F2 (high)** — replaced T0's non-executable in-migration DML row-move with a boot-time config-driven reconcile (server-secret names are per-deployment flag/env config the embedded migration cannot know), and enumerated all six names (primary + reviewer App PEM, webhook, three Linear secrets). - **F3 (medium)** — added the mandatory GRANT to compass_app/compass_system (0001's ON ALL TABLES snapshot does not cover later migrations) and a stated bucket-A RLS decision (Matt-ruled: server_secrets is deployment-global infra, no tenant_id, RLS off, added to the rls_pgtest bucketA allow-list). - **F4 (medium)** — OQ-3 resolved to D7 (Matt: api_key rows encrypted identically to OAuth rows, one seal/open path). - **F5 (medium)** — re-anchored four drifted 0001_init.sql citations (post-RIG-3106 line shift). - **F6 (medium)** — D1 column names aligned to T3/ledger (value_ciphertext/value_nonce). - **F7/F8 (low)** — added the RIG-2863 tracker ref; named the C2 residual risk concretely. Follow-up RIG-3237 filed for per-tenant at-rest isolation + gateway-topology exposure. Spec-impact: none (design record). Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 2 +- ...-gateway-credentials-at-rest-encryption.md | 165 +++++++++++++----- 2 files changed, 124 insertions(+), 43 deletions(-) diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index 0cf3c9c5..d0cb049c 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,7 +112,7 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | -| DL-324 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable) with a 256-bit master key auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify. The master key and the existing PEM/webhook/Linear server secrets live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, no delivery/kind columns, a SECOND `SpecResolver` instance over it on the shared profile, an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC carrying a reserved-name guard) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The PEM/webhook/Linear rows move into `server_secrets` in the same PR chain (DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable) | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | +| DL-324 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so a reserved-name guard rejecting the master-key name is MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext > resume = 12` envelope field is NOT silently reintroduced — the collapsed diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index a4a41b02..f975686d 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -2,6 +2,8 @@ Status: Active +Tracking: RIG-2863 (parent RIG-1715) + Addendum to the frozen record [`compass-server-llm-gateway`](./compass-server-llm-gateway/design.md) (§Credential storage and rotation, L312–373). Scope: encryption-at-rest for the @@ -56,8 +58,10 @@ The credential value payload (the full api_key / OAuth access+refresh+expiry JSON) is encrypted application-side before it reaches Postgres: AES-256-GCM, a fresh random 96-bit nonce per encryption (per write, never reused), authenticated. The `gateway_credentials` row stores ONLY -`ciphertext` + `nonce` (+ `key_version`, D4); no plaintext value column exists -in the schema. A DB dump, replica, or SELECT alone yields nothing usable. +`value_ciphertext` + `value_nonce` +(+ `key_version`, D4); no plaintext value column exists in any migration +version — the store is born encrypted. A DB dump, replica, or SELECT alone +yields nothing usable. Why the fresh-nonce discipline is absolute, quantified: reusing a 96-bit GCM nonce under one key forfeits both confidentiality AND authenticity — a @@ -175,7 +179,7 @@ embedding plaintext or key material. The threat D2 must defeat: the declared-secrets registry is inject-all by design — "NO per-agent grant column (the MVP injects the whole store into every agent; per-agent scoping is a named FUTURE seam)" -(`go/internal/store/migrations/0001_init.sql:379-381`). FetchSecrets resolves +(`go/internal/store/migrations/0001_init.sql:391-393`). FetchSecrets resolves the WHOLE registry with no filter — `resolved, err := h.resolver.Resolve(ctx, "runner fetch")` then `for _, s := range resolved { out = append(out, resolvedSecretToProto(s)) }` (`go/internal/runnerhub/handler.go:303-311`) — @@ -225,7 +229,7 @@ required: no `SECRET_DELIVERY_SERVER_ONLY` proto enum value, no `DeliveryServerOnly`, no widened migration CHECK, and no FetchSecrets delivery filter. The container FetchSecrets path is byte-for-byte UNCHANGED. The delivery enum stays two-valued in all four representations: -`delivery SMALLINT NOT NULL CHECK (delivery IN (0, 1))` (0001_init.sql:388); +`delivery SMALLINT NOT NULL CHECK (delivery IN (0, 1))` (0001_init.sql:399); `SecretDeliveryFile SecretDelivery = 0` / `SecretDeliveryEnv SecretDelivery = 1` (`go/internal/store/secrets.go:18-25`); `DeliveryFile` / `DeliveryEnv` (`go/internal/secrets/secrets.go:39-46`); `SECRET_DELIVERY_UNSPECIFIED = 0; @@ -278,7 +282,13 @@ rejected branches are the ones a future reader will reach for first. live deployments. C1's manifest separation already delivers the container-undeliverable property with a DB-row-only move; the profile split remains available later if provider-keyspace isolation is ever - needed. + needed. Named residual under C1: the shared profile leaves the provider + keyspace a shared mutable surface reachable by the user `SetSecret` path, + so the master key's provider value is guarded by the reserved-name check + (T0, both paths) plus the boot read-back verify RATHER than structurally; + C2 would make that isolation structural. Acceptable with the guard in + place; the follow-up (per-tenant credential at-rest isolation + + gateway-topology exposure) is where C2 is reconsidered (RIG-3237). - **pgcrypto / key-in-DB — REJECTED.** `pgp_sym_encrypt` or a key stored in a Postgres table/GUC puts the key inside the same blast radius as the ciphertext: one `pg_dump` carries both, reducing the encryption to obfuscation. @@ -328,15 +338,36 @@ declared into a store that does not exist. - `Interfaces:` - Migration (a NEW migration file, never an edit to 0001): `CREATE TABLE server_secrets` — names-only, mirroring `secrets` - (0001_init.sql:382-413) MINUS the container-delivery routing: `name TEXT + (0001_init.sql:393-425) MINUS the container-delivery routing: `name TEXT PRIMARY KEY` (same env-var-name grammar, validated at the store door); `declared_by TEXT REFERENCES accounts (id) ON DELETE RESTRICT`, NULLABLE — `NULL` = server-provisioned (the master key; contrast `secrets.declared_by`, which is `NOT NULL REFERENCES accounts (id) ON - DELETE RESTRICT`, 0001_init.sql:397-399 — justification below); + DELETE RESTRICT`, 0001_init.sql:410 — justification below); `created_at`/`updated_at`. NO `delivery` column (server secrets are never container-delivered — that IS the point) and NO `kind` column (they never reach the T5 materializer). + - Grants + RLS (mandatory — the new-table integration the shipped + multi-tenancy regime, RIG-3106, requires): the same new migration + issues `GRANT SELECT, INSERT, UPDATE, DELETE ON server_secrets TO + compass_app, compass_system`. This is NOT inherited: 0001's grant is a + one-time `ON ALL TABLES IN SCHEMA` snapshot (0001_init.sql:896) and there + is no `ALTER DEFAULT PRIVILEGES` anywhere in the migrations, so a table + created by a later migration receives no grant and every server-side + resolve would fail `permission denied for table server_secrets` (fails + closed, but breaks the T2 boot path). RLS posture: `server_secrets` is + declared **bucket-A infrastructure** (Matt-ruled) — NO `tenant_id` + column, RLS NOT enabled: the master key and the PEM/webhook/Linear + secrets are deployment-global (the master key decrypts EVERY tenant's + credentials), so there is no tenant to scope by. T0 adds `server_secrets` + to the `bucketA` allow-list in go/internal/store/rls_pgtest_test.go:544 + so the self-auditing RLS guard test stays a CONSCIOUS gate (that test + keys off `tenant_id`, so a bucket-A table is otherwise invisible to it — + the allow-list edit is what records the deliberate exemption). Live + cross-tenant EXPOSURE (one shared gateway process holding all tenants' + creds) is a gateway-topology property of the parent record + (design.md:333-337), out of scope here and tracked as a follow-up + (RIG-3237). - declared_by nullability, justified: the boot provisioner declares the master key with no human actor. `NULL` = server-provisioned is honest provenance; attributing the row to the bootstrap-admin account @@ -364,14 +395,33 @@ declared into a store that does not exist. mirrors the `SetSecret` declare-then-Set flow and its rollback discipline (secrets_service.go:92-145) minus delivery/kind, targeting `server_secrets`; admin-gated (`adminOnly` in `classifyProcedure`, - admin_gate.go:47). Carries the F3 reserved-name guard: rejects + admin_gate.go:47). Carries the reserved-name guard: rejects `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix) with an actionable error — rotation is OQ-1 machinery, never a raw overwrite. - - Row migration: MOVE the existing declared App-PEM / webhook / Linear - rows (the main.go:414-421, :435-450 conventions) from `secrets` into - `server_secrets` in the same migration — a DB-row move only; provider - values stay put under the shared profile (no re-provision). Closes the - pre-existing inject-all exposure (D6). Same PR chain: OQ-4, RESOLVED. + - User-path reserved-name guard (F1 — mandatory, NOT admin-RPC-only): + because C1 shares the provider keyspace (§D2 read-back verify), the + `authenticatedOpen` user `secretsService.SetSecret`/`DeleteSecret` path + can overwrite the master key's provider value. So `SetSecret`/`DeleteSecret` + MUST reject the reserved name + prefix BEFORE calling `resolver.Set`/ + `Delete` (secrets_service.go:124/214) — a T0 deliverable on the existing + user path, not only the new admin RPC. + - Row migration — a BOOT-TIME config-driven reconcile, NOT an in-migration + DML step (F2): the server-only secret NAMES are per-deployment config, + resolved from flags/env in `cmd/compass-server` (e.g. `appKeySecret = + firstNonEmpty(*f.appKeySecret, os.Getenv("COMPASS_FORGE_APP_KEY_SECRET"))`, + main.go:414-417/462) — a value the embedded SQL migration + (`//go:embed migrations/*.sql`, store.go:24-25) cannot know. So the move + runs at boot in the server wiring (where the config IS available): for + each configured server-secret NAME, if a row exists in `secrets` and not + in `server_secrets`, move it — idempotent, serialized under the T2 + advisory lock, no provider write (the shared profile makes it a pure row + move). The complete set is SIX names, not three: the PRIMARY App PEM + (`appKeySecret`, main.go:414-417), the webhook secret (`appWebhook`, + main.go:418-421), the REVIEWER App PEM (`reviewerAppKeySecret`, + main.go:430; consumed serve.go:1644), and the THREE Linear secrets + (client id, client secret, webhook — main.go:435-450). Closes the + pre-existing inject-all exposure (D6) for every configured name the + reconcile actually runs for. Same PR chain: OQ-4, RESOLVED. - Consumes: nothing from T1-T5 (pure prerequisite). - Tests: a secret declared in `server_secrets` is ABSENT from a FetchSecrets response (because it lives in the other table — no filter @@ -379,9 +429,13 @@ declared into a store that does not exist. resolver instance RESOLVES it; the container resolver's generated manifest never contains its name; the new RPC rejects a non-admin caller; the reserved master-key name is rejected on - SetServerSecret/DeleteServerSecret; the migrated PEM/webhook/Linear - names resolve through the server resolver and are gone from - FetchSecrets. + SetServerSecret/DeleteServerSecret AND on the user-path + SetSecret/DeleteSecret (a non-admin authenticated user calling SetSecret + with the reserved name is rejected and the provider value is unchanged — + F1); the server resolver can READ `server_secrets` through the normal + compass_app store path (the GRANT is present — F3); all six migrated + names (primary + reviewer App PEM, webhook, and the three Linear secrets) + resolve through the server resolver and are gone from FetchSecrets. ### T1 — Envelope-crypto helper package @@ -450,25 +504,43 @@ is declared into it) and T1. - **Read-back verify, every boot:** after provisioning AND on every subsequent boot, re-resolve the name and byte-compare against the key the process is about to encrypt with; on mismatch, refuse to serve - gateway-credential writes (fail closed). This holds regardless of the - lock: the provider is still a shared mutable surface (an operator can - write the name out-of-band through the secretspec CLI), even though the - user-driven SetSecret RPC can no longer race this name — it targets - `secrets` and cannot reach `server_secrets`, and the admin RPC's - reserved-name guard rejects it (F3). - - **Reserved name (F3, now partly structural):** the user-secret - `SetSecret`/`DeleteSecret` path targets `secrets` and physically CANNOT - reach `server_secrets` — it cannot touch the master key at all. (The - exposure this closes structurally: user-path deletes are name-keyed and - global — "a row is keyed by name alone, not (actor, name): any declared - name is a legal delete target regardless of who declared it", - go/internal/store/secrets.go:150-153 — and the SecretsService rides the - ambient-identity 0600 socket, serve.go:711-715.) The guard that remains: - the NEW `SetServerSecret`/`DeleteServerSecret` RPC (T0) — the one - surface that CAN reach `server_secrets` — rejects - `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix) with an - actionable error. Rotation is OQ-1's machinery, never a raw overwrite - through this surface. + gateway-credential writes (fail closed). This is necessary because the + provider keyspace is a shared mutable surface: C1 pins BOTH resolver + instances to the same SecretSpec project + profile (`manifestProject = + "compass"`, `defaultProfile = "default"`, resolver.go:19/23), so the + master key's provider VALUE is reachable by any writer of that + keyspace — an operator's out-of-band `secretspec set`, AND (absent the + guard below) the user-driven `SetSecret` RPC. The verify is a tripwire, + not a boundary: on a plain boot the resolved value IS "the key the + process is about to encrypt with", so a byte-compare cannot by itself + distinguish "my key" from "a swapped key". T5 strengthens it by binding + a non-secret key fingerprint (a salted digest, stored beside + `key_version`) so a swapped provider value is DETECTED at boot rather + than silently adopted. + - **Reserved-name guard (F1 — on EVERY provider-writing path):** C1 splits + the DECLARATION registries (two tables) but NOT the provider keyspace + (one shared profile, above). So the isolation C1 buys is that server + secrets are structurally undeliverable to CONTAINERS (the manifest + separation, §Mechanism C1) — it does NOT make the master key's provider + value unreachable from the user path. The user `SetSecret`/`DeleteSecret` + RPC (`authenticatedOpen`, any authenticated account — admin_gate.go:119) + declares into `secrets` but then calls `resolver.Set`, which shells + `secretspec set --profile default` (resolver.go:258-267) against + the SHARED keyspace — so a user calling `SetSecret` with name + `GATEWAY_CREDENTIALS_MASTER_KEY` would OVERWRITE the master key's + provider value (the running process keeps its cached key, but the next + boot adopts the attacker-chosen key → every existing row fails GCM auth, + every new row is sealed under a known key: the exact bulk-disclosure D1 + prevents). Therefore the reserved-name guard is MANDATORY on the user + `secretsService.SetSecret`/`DeleteSecret` path (reject the reserved name + and prefix BEFORE `resolver.Set`/`Delete`), a T0/T2 deliverable, AND on + the new `SetServerSecret`/`DeleteServerSecret` RPC. Both are tested + red-green (a non-admin user calling `SetSecret` with the reserved name is + rejected and the provider value is unchanged). Rotation is OQ-1's + machinery, never a raw overwrite through either surface. The name-keyed + global user delete ("a row is keyed by name alone, not (actor, name)", + go/internal/store/secrets.go:150-153) is why the guard covers the delete + path too, once a real provider hard-delete lands. - **Nil-resolver deployment:** a server built with no secrets surface is legitimate today ("resolver may be nil on a server built with no secrets surface (FetchSecrets then fails CodeFailedPrecondition rather @@ -567,6 +639,12 @@ RPC exactly as the frozen record already specifies. `slog.JSONHandler` capture — assert no secret bytes appear in ANY of these. The fmt verbs alone mirror the existing pattern's intent (secrets.go:155-156) but do not cover the reflection paths (D5). + - **Key-swap tripwire (F1):** persist a non-secret key fingerprint (a + salted digest of the master key, stored beside `key_version` in + `server_secrets` or a sibling row) at provision time; the boot read-back + verify recomputes it and refuses to serve on mismatch. Test: swap the + provider value out-of-band, reboot → boot fails closed (the fingerprint + distinguishes "a key" from "my key", which a bare byte-compare cannot). - Refresh write-back path: refreshed OAuth tokens land re-sealed with a fresh nonce (nonce differs from the previous row state). - Wrong-key / tampered-row read → error, no partial plaintext; a @@ -627,11 +705,13 @@ RPC exactly as the frozen record already specifies. filter; Matt's C1 ruling replaced that with the separate store (see Alternatives, mechanism A) and REMOVED the public proto change it required. -- **OQ-3 (load-bearing, recommendation: yes): encrypt api_key rows - identically to OAuth rows?** Recommendation: **yes** — one seal/open code - path for both payload shapes; both are secrets of the same sensitivity, and - a split path doubles the audit surface for zero benefit. Load-bearing: T3's - schema and T4's wiring assume one path. +- **OQ-3 — RESOLVED (Matt: yes, D7): api_key rows are encrypted identically + to OAuth rows.** One seal/open code path for both payload shapes; both are + secrets of the same sensitivity (a stored api_key is as long-lived and as + disclosure-critical as an OAuth refresh token), and a split path would + leave the most static secret class in plaintext for zero benefit. T3's + single-column schema and T4's single wiring path encode this — there is no + plaintext branch. - **OQ-4 — RESOLVED (Matt: same PR chain, mechanism C1): the PEM/webhook/Linear server-secret rows move to `server_secrets` in THIS record's PR chain**, closing the live pre-existing exposure (they ride @@ -640,5 +720,6 @@ RPC exactly as the frozen record already specifies. SecretSpec profile (`defaultProfile`, resolver.go:23), this is a DB-row move only: provider values stay put and nothing is re-provisioned. New operator declarations go through the admin-gated `SetServerSecret` RPC - (T0); the row move for the ALREADY-declared names ships in T0's - migration. + (T0); the move for the ALREADY-declared names is T0's boot-time + config-driven reconcile (per configured NAME, not an in-migration DML — + the names are per-deployment config the embedded migration cannot know). From cd7a7770ce7a7f70cba0be59d72d236009d4f0a9 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 14:01:38 -0400 Subject: [PATCH 03/18] docs(server): fix round-2 review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review of #859 returned 0 high / 3 medium / 3 low. The round-1 body fixes all held; the blockers were that the fix commit refreshed the record body but not the terse `## Tasks` checklist, plus one new defect the F2 fix introduced. - R2-1/R2-2 (medium): rewrite the T0 + T2 `## Tasks` checklist entries to match the fixed body — T2 no longer says the reserved-name guard is admin-RPC-only, and T0 now names the boot-time config-driven reconcile (not in-migration DML), all six server-secret names, the GRANT, and the bucketA allow-list edit. - R2-3 (medium, new defect): the F2 fix relocated the row move from a pre-policy migration to a post-policy boot reconcile, where the source `secrets` table is FORCE RLS and the ordinary tenant-scoped store path sees only the bootstrap tenant. Specify the reconcile runs under `store.WithSystemRole` (BYPASSRLS) — a fifth WithSystemRole entrypoint that extends DL-315's "only at the four named entrypoints" clause — and add the two-tenant reconcile red-green test. - R2-4 (low): add the key-swap fingerprint tripwire to the T5 checklist entry. - R2-5/R2-6 (low): correct two citation anchors added by the round-1 fixes (`rls_pgtest_test.go:544`->`:543`; `secrets_service.go:214`->`:208`). Additive review-fix commit atop the round-1 fix; no ledger delta (DL-324 is unchanged — the reconcile execution context is a T0 mechanism detail, ratified at the PR gate). Ledger-impact: none Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...-gateway-credentials-at-rest-encryption.md | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index f975686d..0d484013 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -360,7 +360,7 @@ declared into a store that does not exist. column, RLS NOT enabled: the master key and the PEM/webhook/Linear secrets are deployment-global (the master key decrypts EVERY tenant's credentials), so there is no tenant to scope by. T0 adds `server_secrets` - to the `bucketA` allow-list in go/internal/store/rls_pgtest_test.go:544 + to the `bucketA` allow-list in go/internal/store/rls_pgtest_test.go:543 so the self-auditing RLS guard test stays a CONSCIOUS gate (that test keys off `tenant_id`, so a bucket-A table is otherwise invisible to it — the allow-list edit is what records the deliberate exemption). Live @@ -403,7 +403,7 @@ declared into a store that does not exist. `authenticatedOpen` user `secretsService.SetSecret`/`DeleteSecret` path can overwrite the master key's provider value. So `SetSecret`/`DeleteSecret` MUST reject the reserved name + prefix BEFORE calling `resolver.Set`/ - `Delete` (secrets_service.go:124/214) — a T0 deliverable on the existing + `Delete` (secrets_service.go:124/208) — a T0 deliverable on the existing user path, not only the new admin RPC. - Row migration — a BOOT-TIME config-driven reconcile, NOT an in-migration DML step (F2): the server-only secret NAMES are per-deployment config, @@ -422,6 +422,29 @@ declared into a store that does not exist. (client id, client secret, webhook — main.go:435-450). Closes the pre-existing inject-all exposure (D6) for every configured name the reconcile actually runs for. Same PR chain: OQ-4, RESOLVED. + - Reconcile execution context (F2 follow-on — the relocation's RLS + consequence): the SOURCE table `secrets` is `FORCE ROW LEVEL SECURITY` + (it is in the `tenant_tables` array, 0001_init.sql:912; the DO loop at + :922-923 issues `ENABLE`/`FORCE ROW LEVEL SECURITY`). The old + in-migration approach ran on the raw owner pool BEFORE any policy existed + (store.go:89-90, "migrate() … runs on the raw pool as the owner, before + any policy exists to fight"); the boot-time reconcile runs AFTER migration, + so FORCE RLS binds even the owner, and the ordinary tenant-scoped store + path (`SET LOCAL ROLE compass_app` + `set_config('compass.tenant_id', …)`, + tenant_tx.go:140-141) sees only the bootstrap tenant's `secrets` rows — + every other tenant's server-secret row would be silently left on the + inject-all path. So the reconcile MUST run under `store.WithSystemRole(ctx)` + (arms `SET LOCAL ROLE compass_system`, the BYPASSRLS role, no tenant GUC — + tenant_tx.go:41-50), seeing every tenant's rows in one pass. This is a + FIFTH `WithSystemRole` entrypoint beyond the four DL-315 names (the + delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync, + consumer.go:316 + hub.go:753/814); DL-315's "ONLY at those named + entrypoints" clause is extended by this record to admit the boot + server-secret reconcile — a cross-tenant, request-path-free boot step of + the same shape. (The reviewer's raw-owner-pool alternative does NOT work: + boot is post-policy, so FORCE RLS binds the owner too.) Test: server-secret + rows declared under TWO different tenants are BOTH moved by one reconcile + pass and BOTH absent from FetchSecrets afterward. - Consumes: nothing from T1-T5 (pure prerequisite). - Tests: a secret declared in `server_secrets` is ABSENT from a FetchSecrets response (because it lives in the other table — no filter @@ -654,16 +677,25 @@ RPC exactly as the frozen record already specifies. ## Tasks - [ ] T0 — `server_secrets` store (mechanism C1): new migration (table + - PEM/webhook/Linear row move), `ServerDeclaredSecrets` store view + - second SpecResolver instance (same profile), admin-gated - SetServerSecret/DeleteServerSecret RPC with the reserved-name guard, - absent-from-FetchSecrets + server-resolver-resolves red-green tests. - PREREQUISITE of T2. NO proto change. + GRANT SELECT/INSERT/UPDATE/DELETE to compass_app/compass_system + + bucketA allow-list edit in rls_pgtest_test.go), `ServerDeclaredSecrets` + store view + second SpecResolver instance (same profile), admin-gated + SetServerSecret/DeleteServerSecret RPC AND the user-path reserved-name + guard on `secretsService.SetSecret`/`DeleteSecret` (reject before + `resolver.Set`/`Delete`), and a BOOT-TIME config-driven reconcile (NOT + in-migration DML) moving all SIX configured server-secret names (primary + + reviewer App PEM, webhook, three Linear) from `secrets` to + `server_secrets`, run under `store.WithSystemRole` (BYPASSRLS — the + source `secrets` table is FORCE RLS at boot); absent-from-FetchSecrets + + server-resolver-resolves + user-path-guard + two-tenant-reconcile + red-green tests. PREREQUISITE of T2. NO proto change. - [ ] T1 — `go/internal/secrets/envelope`: Key/NewKey/KeyFromBytes/Seal/Open (AAD-carrying) + redaction + unit tests - [ ] T2 — boot resolve-or-provision seam via the server-secret resolver - (advisory-lock serialized, read-back verify, nil-resolver gating, - fail-closed boot; the reserved-name guard lives on T0's RPC). + (advisory-lock serialized, read-back verify + key-fingerprint tripwire, + nil-resolver gating, fail-closed boot; the reserved-name guard is T0's, + on BOTH the admin SetServerSecret/DeleteServerSecret RPC AND the + authenticatedOpen user SetSecret/DeleteSecret path). DEPENDS ON T0 + T1. - [ ] T3 — `gateway_credentials` migration columns: `value_ciphertext`, `value_nonce`, `key_version`; no plaintext column ever. DEPENDS ON @@ -673,7 +705,8 @@ RPC exactly as the frozen record already specifies. construction-time dependency - [ ] T5 — ciphertext-at-rest assertion (CI tripwire), row-swap AAD test, redaction incl. json.Marshal + slog JSONHandler, tamper/wrong-key - tests + tests, and the key-swap tripwire (persist a salted key fingerprint at + provision; boot recompute + fail-closed on mismatch). ## Open Questions From 026897520743db6927df3a900d128194940f3bee Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 14:23:59 -0400 Subject: [PATCH 04/18] docs(server): fix round-3 review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 review returned 0 high / 4 medium / 3 low. All six round-2 fixes held; the blockers were three new grounded defects in round-2's reconcile-execution- context paragraph (the WithSystemRole ruling itself is correct and stands) plus one ledger-recording judgment call Matt ruled on. - M1 (medium): the paragraph justified WithSystemRole by claiming FORCE RLS binds even the owner at boot. False — the owner connection is a superuser and a superuser bypasses even FORCE (0001_init.sql:848-851; rls_pgtest_test.go: 282-284). Restated on the true, superuser-independent reason: the reconcile takes the compass_app path, a non-owner non-BYPASSRLS role confined to the bootstrap tenant, so a row under another tenant is silently skipped without WithSystemRole. - M2 (medium): the prescribed two-tenant same-name test is unconstructible — `secrets.name` is a global PRIMARY KEY (0001_init.sql:396), so at most one row per configured name exists deployment-wide. Reframed the motivation and the test to the real case: a row declared under a NON-bootstrap tenant. - M3 (medium): corrected the "fifth entrypoint" miscount — the shipped tree arms WithSystemRole at THREE call sites (consumer.go:316, hub.go:753/814), and hub.go:814 (forge-notification-ack) is already a BYPASSRLS site DL-315's four names omit. Enumeration restated against the shipped set. - L1/L2/L3 (low): cite the arming code (armQueue/beginTenantTx) not just the WithSystemRole entrypoint; quote DL-315 verbatim; move the reconcile test into T0's Tests enumeration. - M4 -> OQ-5 (Matt-ruled: a new ledger row): the DL-315 BYPASSRLS allow-list widening lands as DL-326 (Refines DL-315, which stays Active), restating the true shipped entrypoint set incl. the previously-unledgered forge-notification- ack arm. Discoverability at the surface where the BYPASSRLS-audit question gets asked, over prose buried in an at-rest-encryption record. Ledger-collision renumber: main's #870 (RIG-3096) merged its own DL-324 (forge live-test app-actor) after this stack forked. Renumbered this addendum's ledger rows to the next free sequential ids — the `gateway_credentials` encryption decision is DL-325 (was DL-324), the BYPASSRLS allow-list widening stays DL-326, and the sibling SubjectService PR (#868) takes DL-327 — leaving main's DL-324 untouched. Additive review-fix commit atop the round-2 fix. Ledger-impact: DL-325 (encryption, renumbered from DL-324), DL-326 (new, Refines DL-315) Spec-impact: none Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 3 +- ...-gateway-credentials-at-rest-encryption.md | 77 ++++++++++++++----- 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index d0cb049c..51ee0f93 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,7 +112,8 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | -| DL-324 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so a reserved-name guard rejecting the master-key name is MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | +| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so a reserved-name guard rejecting the master-key name is MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | +| DL-326 | The DL-315 `BYPASSRLS` system-role allow-list is EXTENDED to admit the boot server-secret reconcile — a cross-tenant, request-path-free boot step that moves the configured server-secret rows from `secrets` (FORCE RLS, tenant-scoped) into `server_secrets`, run under `store.WithSystemRole`; the entrypoint enumeration is restated against the SHIPPED set, which already includes the forge-notification-ack arm (`hub.go:814`) that DL-315's four names omit. DL-315's load-bearing NEVER-on-the-request-path clause is unchanged — every request-path query stays fail-closed under RLS. Refines DL-315 (which stays Active) | Active (Matt, 2026-09-04) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext > resume = 12` envelope field is NOT silently reintroduced — the collapsed diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index 0d484013..836fe307 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -429,22 +429,38 @@ declared into a store that does not exist. in-migration approach ran on the raw owner pool BEFORE any policy existed (store.go:89-90, "migrate() … runs on the raw pool as the owner, before any policy exists to fight"); the boot-time reconcile runs AFTER migration, - so FORCE RLS binds even the owner, and the ordinary tenant-scoped store - path (`SET LOCAL ROLE compass_app` + `set_config('compass.tenant_id', …)`, - tenant_tx.go:140-141) sees only the bootstrap tenant's `secrets` rows — - every other tenant's server-secret row would be silently left on the - inject-all path. So the reconcile MUST run under `store.WithSystemRole(ctx)` - (arms `SET LOCAL ROLE compass_system`, the BYPASSRLS role, no tenant GUC — - tenant_tx.go:41-50), seeing every tenant's rows in one pass. This is a - FIFTH `WithSystemRole` entrypoint beyond the four DL-315 names (the - delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync, - consumer.go:316 + hub.go:753/814); DL-315's "ONLY at those named - entrypoints" clause is extended by this record to admit the boot - server-secret reconcile — a cross-tenant, request-path-free boot step of - the same shape. (The reviewer's raw-owner-pool alternative does NOT work: - boot is post-policy, so FORCE RLS binds the owner too.) Test: server-secret - rows declared under TWO different tenants are BOTH moved by one reconcile - pass and BOTH absent from FetchSecrets afterward. + and if it took the ordinary tenant-scoped store path (`SET LOCAL ROLE + compass_app` + `set_config('compass.tenant_id', …)`, tenant_tx.go:140-141) + — a non-owner, non-BYPASSRLS role — it would be confined to the bootstrap + tenant's `secrets` rows. `secrets.name` is a GLOBAL primary key + (0001_init.sql:396, `name TEXT PRIMARY KEY`; `tenant_id` is a plain + non-key column at :413), so there is at most ONE row per configured + server-secret name in the whole deployment — but that row may have been + declared under a NON-bootstrap tenant, in which case a compass_app-scoped + reconcile cannot see it and silently skips it, leaving it on the inject-all + path forever. So the reconcile MUST run under `store.WithSystemRole(ctx)` + (tenant_tx.go:41-50), which arms `SET LOCAL ROLE compass_system` with no + tenant GUC (armQueue, tenant_tx.go:136-138; beginTenantTx, :202-204; + `compass_system` is `NOLOGIN BYPASSRLS`, 0001_init.sql:882-887), seeing + every tenant's rows in one pass. This adds a cross-tenant, request-path-free + boot step to the BYPASSRLS surface DL-315 governs. DL-315 names four + cross-tenant loops (delivery-cursor sweep, deliver-ack advance, reattach + recovery, lag-resync); the shipped tree arms the system role at THREE call + sites — consumer.go:316 (the delivery consumer Run loop, covering the + sweep/resync/recovery arms since the loop threads one ctx and never + re-roots it, consumer.go:315), hub.go:753 (deliver-ack advance), and + hub.go:814 (the FORGE-notification ack, an arm DL-315's four names do NOT + list — a pre-existing, unledgered extension of its allow-list). DL-315's + load-bearing "granted ONLY to those named background workers and NEVER on + the request path" clause (DECISIONS.md:113) is UNCHANGED — this is a boot + step, never the request path — but its named entrypoint set is widened, + recorded (OQ-5, Matt-ruled) as its own ledger row DL-326 that Refines + DL-315. (Note: a raw-owner-pool reconcile would move every + tenant's rows today only by accident — the owner connection is a superuser, + and a superuser bypasses even FORCE, 0001_init.sql:848-851 / + rls_pgtest_test.go:282-284. The tree deliberately refuses to rely on that + accident, which is why FORCE is set at all; the reconcile takes the explicit + BYPASSRLS path rather than inheriting an owner-privilege escape.) - Consumes: nothing from T1-T5 (pure prerequisite). - Tests: a secret declared in `server_secrets` is ABSENT from a FetchSecrets response (because it lives in the other table — no filter @@ -458,7 +474,11 @@ declared into a store that does not exist. F1); the server resolver can READ `server_secrets` through the normal compass_app store path (the GRANT is present — F3); all six migrated names (primary + reviewer App PEM, webhook, and the three Linear secrets) - resolve through the server resolver and are gone from FetchSecrets. + resolve through the server resolver and are gone from FetchSecrets; and a + configured server-secret row declared under a NON-bootstrap tenant is found + and moved by one reconcile pass and is absent from FetchSecrets afterward + (red without `WithSystemRole`: the compass_app-scoped reconcile sees zero + rows and skips it silently). ### T1 — Envelope-crypto helper package @@ -686,9 +706,11 @@ RPC exactly as the frozen record already specifies. in-migration DML) moving all SIX configured server-secret names (primary + reviewer App PEM, webhook, three Linear) from `secrets` to `server_secrets`, run under `store.WithSystemRole` (BYPASSRLS — the - source `secrets` table is FORCE RLS at boot); absent-from-FetchSecrets + - server-resolver-resolves + user-path-guard + two-tenant-reconcile - red-green tests. PREREQUISITE of T2. NO proto change. + compass_app-scoped path is confined to the bootstrap tenant, so a row + declared under another tenant is otherwise silently skipped); + absent-from-FetchSecrets + server-resolver-resolves + user-path-guard + + non-bootstrap-tenant-reconcile red-green tests. PREREQUISITE of T2. NO + proto change. - [ ] T1 — `go/internal/secrets/envelope`: Key/NewKey/KeyFromBytes/Seal/Open (AAD-carrying) + redaction + unit tests - [ ] T2 — boot resolve-or-provision seam via the server-secret resolver @@ -756,3 +778,18 @@ RPC exactly as the frozen record already specifies. (T0); the move for the ALREADY-declared names is T0's boot-time config-driven reconcile (per configured NAME, not an in-migration DML — the names are per-deployment config the embedded migration cannot know). +- **OQ-5 — RESOLVED (Matt: a new ledger row): how to RECORD the DL-315 + BYPASSRLS-allow-list widening.** T0's boot server-secret reconcile adds a + cross-tenant, request-path-free BYPASSRLS entrypoint (`store.WithSystemRole`) + beyond the set DL-315 governs — and the review found DL-315's own + enumeration is already stale (the shipped `hub.go:814` forge-notification-ack + arm is a BYPASSRLS site DL-315's four names omit). DL-315's load-bearing + NEVER-on-the-request-path clause is untouched; only its named-entrypoint set + widens. **Matt ruled: its OWN ledger row that Refines DL-315** (matching the + ledger's dense precedent for scope-amendment rows — DL-316/DL-317/DL-293/ + DL-072/DL-212 — and fixing the discoverability gap: an engineer auditing + "which sites may arm BYPASSRLS?" reads DL-315, not this at-rest-encryption + record). Landed as **DL-326** in this PR (this PR's encryption row is + DL-325; the sibling SubjectService PR takes DL-327), restating the true shipped + entrypoint set (three call sites incl. the forge-notification-ack arm) + + the reconcile; DL-315 stays Active. From 5ca1385caf4b0156efdc16f56cea4f18fd269caf Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 15:59:52 -0400 Subject: [PATCH 05/18] docs(server): fix round-4 review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 review (0 focus-area regressions; DL-326 entrypoints, renumber integrity, and R3 M1/M2/M3 all verified holding) surfaced one high + two mediums + two lows, all against the T0 prerequisite that OQ-5's ledgering made load-bearing this round. All findings verified against the tree at main `8fef2846` before folding; none is a design fork. - **H1 (T0 consumer-repoint):** the reconcile moves the six server-secret NAMES out of `secrets`, but the record pinned the container resolver (serve.go:528) as UNCHANGED and never repointed the boot consumers that resolve those names through it. As written that hard-fails boot for every GitHub-App deployment (`validateForgeSecret` → `forge secret %q not declared`) and silently unmounts the Linear webhook + write/notify lanes (`forgeSecretDeclared`/`buildLinearTokenSource` treat an absent name as a legitimate off-state). Added an explicit consumer-repoint deliverable (thread the SERVER resolver into buildForgeReadWiring/buildLinearWebhookWiring/ buildLinearTokenSource/wireForgeWriteCaller/buildForgeWriteService + the newDeclaredSecretResolver/newCachedWebhookSecret closures), pinned the reconcile-before-forge-wiring ordering, corrected the resolver-UNCHANGED sentence, and added positive forge-lane-wiring assertions to T0's test list (the silent modes need a positive assert, since absent is indistinguishable from a legitimate off-state). - **M1 (F1 guard scope):** widened the user-path guard from a reserved-name/ prefix match to a `server_secrets` MEMBERSHIP test on SetSecret/DeleteSecret (and DeclareSecret at the store door) — the five non-master names are arbitrary per-deployment config with no reserved prefix, so a reserved-name- only guard leaves the `authenticatedOpen` SetSecret path able to mint a shadow `secrets` row under a server-secret name and get it container- delivered. Corrected the C1 "structurally undeliverable" claim to state the guard, not the table split, is what makes undeliverability total; updated the C2 named residual to cover all six names. - **M2 (T2 bounded critical section):** the advisory-lock critical section had no time bound on the provider round-trips, so a hung provider holds a shared-constant lock indefinitely and wedges every booting instance. Added a bounded-ctx + try-lock/lock_timeout requirement mirroring the 30s serve.go:1731 precedent, plus the corresponding fleet-wedge test. - **L1:** package-qualified + symbol-anchored the BYPASSRLS entrypoint citations in the record body and the DL-326 ledger row (`go/internal/ runnerhub/hub.go` `forgeNotificationAck` etc.), since a bare `hub.go:814` drifts and defeats the discoverability the OQ-5 row exists for. - **L2:** fixed two citation slips in the reconcile-execution-context paragraph (`compass_system` CREATE ROLE is 0001_init.sql:879 not :882-887; the arming code is in `armTx` :199-215, shared by `beginTenantTx` :185-195, not "beginTenantTx :202-204"). Ledger-impact: DL-325, DL-326 (DL-326 citation symbol-anchored; both rows' record links now carry a resolving #anchor, required since the fold grew the record past the 50 KB design-ledger-gate threshold) Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 4 +- ...-gateway-credentials-at-rest-encryption.md | 128 ++++++++++++++---- 2 files changed, 104 insertions(+), 28 deletions(-) diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index 51ee0f93..afee61b4 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,8 +112,8 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | -| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so a reserved-name guard rejecting the master-key name is MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | -| DL-326 | The DL-315 `BYPASSRLS` system-role allow-list is EXTENDED to admit the boot server-secret reconcile — a cross-tenant, request-path-free boot step that moves the configured server-secret rows from `secrets` (FORCE RLS, tenant-scoped) into `server_secrets`, run under `store.WithSystemRole`; the entrypoint enumeration is restated against the SHIPPED set, which already includes the forge-notification-ack arm (`hub.go:814`) that DL-315's four names omit. DL-315's load-bearing NEVER-on-the-request-path clause is unchanged — every request-path query stays fail-closed under RLS. Refines DL-315 (which stays Active) | Active (Matt, 2026-09-04) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md) | +| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so a reserved-name guard rejecting the master-key name is MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | +| DL-326 | The DL-315 `BYPASSRLS` system-role allow-list is EXTENDED to admit the boot server-secret reconcile — a cross-tenant, request-path-free boot step that moves the configured server-secret rows from `secrets` (FORCE RLS, tenant-scoped) into `server_secrets`, run under `store.WithSystemRole`; the entrypoint enumeration is restated against the SHIPPED set, which already includes the forge-notification-ack arm (`go/internal/runnerhub/hub.go`, `forgeNotificationAck`, currently :814) that DL-315's four names omit. DL-315's load-bearing NEVER-on-the-request-path clause is unchanged — every request-path query stays fail-closed under RLS. Refines DL-315 (which stays Active) | Active (Matt, 2026-09-04) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#t0--the-server_secrets-store-table-resolver-split-admin-rpc-prerequisite) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext > resume = 12` envelope field is NOT silently reintroduced — the collapsed diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index 836fe307..ba5907fb 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -213,8 +213,15 @@ instance (`resolver := secrets.NewSpecResolver(st, secretsStateDir(cfg))`, READ: a store view over `server_secrets` (`ServerDeclaredSecrets`, T0) feeds a SECOND `SpecResolver` instance. The container resolver keeps reading `secrets`; its manifest never contains a server-secret name, so a server -secret is STRUCTURALLY undeliverable to containers — there is nothing to -filter. +secret is undeliverable to containers via the container manifest — there is +nothing to filter. That structural property holds only while no name in +`server_secrets` is ALSO re-declared into `secrets`: because the two tables +share a keyspace and `secrets.name` is a global PK with no cross-table +exclusion, an `authenticatedOpen` `SetSecret` could otherwise mint a shadow +`secrets` row under a server-secret name and get it container-delivered. The +F1 membership guard (T0 — reject any `server_secrets` name on the user +declare/set path) is what closes that gap and makes the undeliverability +total; the table split alone is necessary but not sufficient. C1 keeps the SAME SecretSpec profile for both instances — the shared project is `manifestProject = "compass"` (resolver.go:19) and the profile @@ -284,11 +291,14 @@ rejected branches are the ones a future reader will reach for first. split remains available later if provider-keyspace isolation is ever needed. Named residual under C1: the shared profile leaves the provider keyspace a shared mutable surface reachable by the user `SetSecret` path, - so the master key's provider value is guarded by the reserved-name check - (T0, both paths) plus the boot read-back verify RATHER than structurally; - C2 would make that isolation structural. Acceptable with the guard in - place; the follow-up (per-tenant credential at-rest isolation + - gateway-topology exposure) is where C2 is reconsidered (RIG-3237). + so all SIX server-secret names' provider values (master key + primary and + reviewer App PEM + webhook + two Linear client secrets) are guarded by the + F1 membership check (T0, both paths — reject any `server_secrets` name) + RATHER than structurally; the boot read-back verify additionally covers the + master key. C2 would make that isolation structural for all of them. + Acceptable with the guard in place; the follow-up (per-tenant credential + at-rest isolation + gateway-topology exposure) is where C2 is reconsidered + (RIG-3237). - **pgcrypto / key-in-DB — REJECTED.** `pgp_sym_encrypt` or a key stored in a Postgres table/GUC puts the key inside the same blast radius as the ciphertext: one `pg_dump` carries both, reducing the encryption to obfuscation. @@ -386,11 +396,34 @@ declared into a store that does not exist. rows to `store.SecretDeclaration` with generic kind / zero delivery (the resolver uses only the name to build its manifest), so `NewSpecResolver` is reused UNCHANGED. - - Resolver split: a SECOND `SpecResolver` instance constructed over that - view — SAME profile (`defaultProfile = "default"`, resolver.go:23) and - project (`manifestProject = "compass"`, resolver.go:19), its own - manifest state dir. The container resolver (serve.go:528) is UNCHANGED - — no filter, nothing to filter. + - Resolver split: a SECOND `SpecResolver` instance (the SERVER resolver) + constructed over that view — SAME profile (`defaultProfile = "default"`, + resolver.go:23) and project (`manifestProject = "compass"`, + resolver.go:19), its own manifest state dir. The container resolver's + CONSTRUCTION at serve.go:528 is unchanged — no filter, nothing to filter + — but its boot CONSUMERS must be re-pointed: once T0's reconcile moves + the six server-secret names out of `secrets`, every boot consumer that + still resolves those names through the serve.go:528 instance breaks, + because that instance's manifest no longer declares them. Two failure + modes verified in the tree: (a) HARD BOOT FAILURE — `validateForgeSecret` + (serve.go:1539-1549) returns `forge secret %q not declared` for an absent + name and is called on the primary App PEM (:1013), the App webhook secret + (:1016) and the reviewer App PEM (:1638), propagated to `failStartup` via + `buildForgeReadWiring`, so every GitHub-App deployment fails its first + post-upgrade boot; (b) SILENT SECURITY DEGRADATION — `forgeSecretDeclared` + (serve.go:1082-1083) treats an absent name as a legitimate off-state + (`nil, nil`), so `buildLinearWebhookWiring` silently unmounts the Linear + webhook ingress and `buildLinearTokenSource` (serve.go:1699-1717) silently + drops the Linear write + notify lanes. So T0 has an explicit + CONSUMER-REPOINT deliverable: thread the SERVER resolver instance (not the + serve.go:528 container instance) into `buildForgeReadWiring` (serve.go:567), + `buildLinearWebhookWiring`, `buildLinearTokenSource`, `wireForgeWriteCaller` + (:587) and `buildForgeWriteService`, and into the + `newDeclaredSecretResolver` / `newCachedWebhookSecret` closures + (serve.go:1025/1039/1085/1644) that mint the App token + verify webhook + HMAC. Ordering: the reconcile MUST run BEFORE `buildForgeReadWiring` + (serve.go:567) so the names are in `server_secrets` before any forge + consumer resolves them. - RPC: `SetServerSecret`/`DeleteServerSecret` on the secrets service — mirrors the `SetSecret` declare-then-Set flow and its rollback discipline (secrets_service.go:92-145) minus delivery/kind, targeting @@ -398,13 +431,23 @@ declared into a store that does not exist. admin_gate.go:47). Carries the reserved-name guard: rejects `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix) with an actionable error — rotation is OQ-1 machinery, never a raw overwrite. - - User-path reserved-name guard (F1 — mandatory, NOT admin-RPC-only): - because C1 shares the provider keyspace (§D2 read-back verify), the + - User-path server-secret guard (F1 — mandatory, NOT admin-RPC-only): C1 + shares the provider keyspace (§D2 read-back verify), so the `authenticatedOpen` user `secretsService.SetSecret`/`DeleteSecret` path - can overwrite the master key's provider value. So `SetSecret`/`DeleteSecret` - MUST reject the reserved name + prefix BEFORE calling `resolver.Set`/ - `Delete` (secrets_service.go:124/208) — a T0 deliverable on the existing - user path, not only the new admin RPC. + (any authenticated account, admin_gate.go:122-125) can overwrite a + server secret's provider value AND — because after OQ-4 the six + PEM/webhook/Linear names live in `server_secrets`, not `secrets` — + create a FRESH shadow row in `secrets` under that name (no primary-key + conflict), which the inject-all path then delivers into every container. + The reserved-name check alone is INSUFFICIENT: those six names are + arbitrary per-deployment config (main.go:414-450) and carry no reserved + prefix. So the guard is a MEMBERSHIP test, not a name/prefix match: + `SetSecret`/`DeleteSecret` (and `DeclareSecret` at the store door, so the + shadow row can never be created at all) MUST reject any name present in + `server_secrets` — checked BEFORE `resolver.Set`/`Delete` + (secrets_service.go:124/208) — in addition to the reserved master-key + name/prefix. A T0 deliverable on the existing user path, not only the new + admin RPC. - Row migration — a BOOT-TIME config-driven reconcile, NOT an in-migration DML step (F2): the server-only secret NAMES are per-deployment config, resolved from flags/env in `cmd/compass-server` (e.g. `appKeySecret = @@ -440,17 +483,20 @@ declared into a store that does not exist. reconcile cannot see it and silently skips it, leaving it on the inject-all path forever. So the reconcile MUST run under `store.WithSystemRole(ctx)` (tenant_tx.go:41-50), which arms `SET LOCAL ROLE compass_system` with no - tenant GUC (armQueue, tenant_tx.go:136-138; beginTenantTx, :202-204; - `compass_system` is `NOLOGIN BYPASSRLS`, 0001_init.sql:882-887), seeing + tenant GUC (armQueue, tenant_tx.go:136-138; armTx, :199-215, shared by + beginTenantTx, :185-195; `compass_system` is `NOLOGIN BYPASSRLS`, + 0001_init.sql:879, idempotently re-asserted :883), seeing every tenant's rows in one pass. This adds a cross-tenant, request-path-free boot step to the BYPASSRLS surface DL-315 governs. DL-315 names four cross-tenant loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync); the shipped tree arms the system role at THREE call - sites — consumer.go:316 (the delivery consumer Run loop, covering the - sweep/resync/recovery arms since the loop threads one ctx and never - re-roots it, consumer.go:315), hub.go:753 (deliver-ack advance), and - hub.go:814 (the FORGE-notification ack, an arm DL-315's four names do NOT - list — a pre-existing, unledgered extension of its allow-list). DL-315's + sites — `go/internal/delivery/consumer.go` (`Consumer.Run`, :316; covering + the sweep/resync/recovery arms since the loop threads one ctx and never + re-roots it, :315), `go/internal/runnerhub/hub.go` (deliver-ack advance, + :753), and `go/internal/runnerhub/hub.go` (`forgeNotificationAck`, :814 — + an arm DL-315's four names do NOT list, a pre-existing, unledgered + extension of its allow-list). Symbol names anchor these; the line numbers + are a convenience that drifts. DL-315's load-bearing "granted ONLY to those named background workers and NEVER on the request path" clause (DECISIONS.md:113) is UNCHANGED — this is a boot step, never the request path — but its named entrypoint set is widened, @@ -478,7 +524,17 @@ declared into a store that does not exist. configured server-secret row declared under a NON-bootstrap tenant is found and moved by one reconcile pass and is absent from FetchSecrets afterward (red without `WithSystemRole`: the compass_app-scoped reconcile sees zero - rows and skips it silently). + rows and skips it silently). CONSUMER-REPOINT positive assertions (the + (a)/(b) silent modes need them, since an absent name is indistinguishable + from a legitimate off-state): boot a server with all six names configured + and, after one reconcile pass, assert the GitHub App token source mints, + the GitHub and Linear webhook handlers are MOUNTED (non-nil), and the + Linear token source is non-nil — i.e. the forge lanes still wire off the + server resolver, not just that the names left `secrets`. MEMBERSHIP-GUARD + assertion (F1): a non-admin authenticated caller invoking `SetSecret` with + a configured server-secret NAME (not only the reserved master-key name) is + rejected, no `secrets` shadow row is created, and the provider value is + unchanged. ### T1 — Envelope-crypto helper package @@ -544,6 +600,26 @@ is declared into it) and T1. and Declare converges on the next boot (the undeclared name does not resolve, so the provisioner re-generates, re-Sets, and Declares — no row was ever encrypted under the orphaned value). + - **Bounded critical section (mandatory — a stuck provider must not wedge + the fleet):** the provider round-trips inside the lock (`resolver.Set` / + `Resolve`) inherit only the caller's ctx, which at boot is long-lived, and + `SpecResolver.Set` shells out via `exec.CommandContext(ctx, r.cli, …)` + (resolver.go:233) with no timeout of its own. A hung provider (1Password + awaiting biometric approval, an unreachable Vault, a half-open TCP) would + otherwise hold the transaction-scoped lock indefinitely, and because the + key is a shared constant EVERY other booting instance blocks on it — one + stuck provider becomes a fleet-wide boot wedge. So the provisioner derives + a ctx with an explicit timeout (mirror the 30s + `&http.Client{Timeout: 30 * time.Second}` precedent that already bounds + the Linear boot mint at serve.go:1731, under the comment "an unbounded + doer would let a half-open TCP … wedge Serve's whole boot") around the + resolve/Set calls, and acquires the lock with `pg_try_advisory_xact_lock` + in a bounded retry loop (or sets a session `lock_timeout`) so a booter that + cannot get the lock fails closed with a diagnosable startup error naming + the contended provisioning lock rather than parking forever. Test (T2): a + provider that never returns yields a bounded, diagnosable boot failure, and + a second instance blocked on the lock also fails bounded rather than + hanging. - **Read-back verify, every boot:** after provisioning AND on every subsequent boot, re-resolve the name and byte-compare against the key the process is about to encrypt with; on mismatch, refuse to serve From 096a8d154d83eb90552fdee898ed1d13de26d190 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 17:03:24 -0400 Subject: [PATCH 06/18] docs(server): fix round-5 review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 adversarial review of PR #859 returned 2H/3M/2L; all seven verified against the tree at main `8fef2846` before folding. None is a design fork — each corrects a plan-executability or internal-consistency defect that honors an invariant the record already asserts. Additive commit atop the bookmark. - **H1 (silent forge-write capability loss):** the T0 failure-mode taxonomy was two modes ((a) hard boot fail, (b) silent Linear degradation) but a THIRD exists that the reviewer-PEM `validateForgeSecret` (serve.go:1638) hides behind. `wireForgeWriteCaller` (:587) gates on `forgeWritesEnabled` (:1583); with both App PEM names moved out of the container resolver the predicate reads both-absent, `warnPartialForgeWriteSecrets` returns silently (:1826-1827), the write caller is never mounted, and :1638 is unreachable — agent forge WRITES fail-closed fleet-wide with no boot error. Split the taxonomy into three modes, moved :1638 into mode (c), and added the missing T0 positive assertion (forge WRITE caller MOUNTED after one reconcile pass). - **H2 (uncancellable Resolve wedge):** the T2 bounded-critical-section fix prescribed a ctx timeout, but `SpecResolver.Resolve` threads ctx only into `DeclaredSecrets` (resolver.go:136); the provider round-trip is `b.Load()` (resolver.go:165), whose SDK signature carries no ctx (secretspec-go v0.15.0 secretspec.go:245) and blocks in an uncancellable FFI call. State the Set(bounded)/Resolve(unbounded) asymmetry and prescribe the mechanism that works: run the boot resolve on its own goroutine + select on the derived ctx, so the provisioner returns a bounded error and releases the xact lock while the orphaned FFI goroutine is knowingly leaked. Restated the T2 test. - **M1 (reconcile skips default-named Linear pair):** pinned the reconcile name source to `cfg.Forge.resolved()` (serve.go:232-246), not the raw flag/env layer — two Linear names carry code defaults (serve.go:202-203) applied after that layer, so a reconcile over `main.go` flags sees `""` and silently skips them, leaving them on the inject-all path. Added a T0 test. - **M2 (buildDoors resolver-fork hazard):** named the `buildDoors` fork explicitly — its single `resolver` param (serve.go:677) feeds both `buildLinearWebhookWiring` (:788, MUST repoint) and `buildNetworkServer` (:803) -> `runnerhub.NewMountedHandler` (network_door.go:313, the container FetchSecrets path that MUST NOT). So `buildDoors` takes BOTH resolvers; swapping its one argument would invert D6. - **M3 (dangling D7 reference):** DL-325 and OQ-3 cited a decision "D7" the record never defined. Promoted the api_key/OAuth uniform-treatment ruling into a real `### D7` heading in the Approach section; OQ-3 and DL-325 now resolve. - **L1:** defined the reserved prefix `GATEWAY_CREDENTIALS_` inline at its three uses (was undefined). - **L2:** caveated the header "structurally undeliverable" claim with the F1 guard dependency the D6 body already states (the split alone is necessary but not sufficient). markdownlint clean; design-ledger-gate OK (293 rows, 123 headers); no dup DL ids. Record grew to 59547 B; DL-325/DL-326 anchors already present. Ledger-impact: DL-325, DL-326 (text unchanged; D7 referent now exists) Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...-gateway-credentials-at-rest-encryption.md | 182 ++++++++++++------ 1 file changed, 120 insertions(+), 62 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index ba5907fb..959feeb3 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -16,7 +16,9 @@ One deliberate scope expansion, Matt-ruled (D6/T0): server-only secrets get a physically SEPARATE `server_secrets` store (mechanism C1) — a new table plus a second resolver instance — so the master key and the existing PEM/webhook/Linear server secrets are structurally undeliverable to agent -containers. C1 needs NO public proto change: it REMOVES the +containers (the table split PLUS the F1 membership guard on the user +declare/set path — the split alone is necessary but not sufficient; see D6). +C1 needs NO public proto change: it REMOVES the `SECRET_DELIVERY_SERVER_ONLY` proto/store/secrets enum additions (and the FetchSecrets filter) the earlier delivery-kind mechanism required. @@ -263,6 +265,16 @@ and today ride the same inject-all path into every agent container. Their declared rows MOVE from `secrets` into `server_secrets` in this PR chain (OQ-4, RESOLVED) — a DB-row move only, since the profile is shared. +### D7 — One seal/open path for `api_key` and OAuth payloads + +Both stored payload shapes — an OAuth token bundle and a bare `api_key` — are +sealed and opened through the SAME envelope path (D1); there is no plaintext +branch and no per-shape code fork. Both are secrets of the same sensitivity: a +stored `api_key` is as long-lived and as disclosure-critical as an OAuth +refresh token, and a split path would leave the most static secret class in +plaintext for zero benefit. T3's single encrypted column and T4's single +wiring path encode this. (Matt-ruled; resolves OQ-3.) + ## Alternatives considered The core choice (envelope encryption + auto-provisioned key in the existing @@ -404,33 +416,52 @@ declared into a store that does not exist. — but its boot CONSUMERS must be re-pointed: once T0's reconcile moves the six server-secret names out of `secrets`, every boot consumer that still resolves those names through the serve.go:528 instance breaks, - because that instance's manifest no longer declares them. Two failure + because that instance's manifest no longer declares them. THREE failure modes verified in the tree: (a) HARD BOOT FAILURE — `validateForgeSecret` (serve.go:1539-1549) returns `forge secret %q not declared` for an absent - name and is called on the primary App PEM (:1013), the App webhook secret - (:1016) and the reviewer App PEM (:1638), propagated to `failStartup` via - `buildForgeReadWiring`, so every GitHub-App deployment fails its first - post-upgrade boot; (b) SILENT SECURITY DEGRADATION — `forgeSecretDeclared` - (serve.go:1082-1083) treats an absent name as a legitimate off-state - (`nil, nil`), so `buildLinearWebhookWiring` silently unmounts the Linear - webhook ingress and `buildLinearTokenSource` (serve.go:1699-1717) silently - drops the Linear write + notify lanes. So T0 has an explicit - CONSUMER-REPOINT deliverable: thread the SERVER resolver instance (not the - serve.go:528 container instance) into `buildForgeReadWiring` (serve.go:567), - `buildLinearWebhookWiring`, `buildLinearTokenSource`, `wireForgeWriteCaller` - (:587) and `buildForgeWriteService`, and into the - `newDeclaredSecretResolver` / `newCachedWebhookSecret` closures - (serve.go:1025/1039/1085/1644) that mint the App token + verify webhook - HMAC. Ordering: the reconcile MUST run BEFORE `buildForgeReadWiring` - (serve.go:567) so the names are in `server_secrets` before any forge - consumer resolves them. + name and is called on the primary App PEM (:1013) and the App webhook + secret (:1016) — both inside `buildBoardWebhookWiring` under + `buildForgeReadWiring`, propagated to `failStartup`, so every GitHub-App + deployment fails its first post-upgrade boot; (b) SILENT SECURITY + DEGRADATION — `forgeSecretDeclared` (serve.go:1082-1083) treats an absent + name as a legitimate off-state (`nil, nil`), so `buildLinearWebhookWiring` + silently unmounts the Linear webhook ingress and `buildLinearTokenSource` + (serve.go:1699-1717) silently drops the Linear write + notify lanes; + (c) SILENT CAPABILITY LOSS — `wireForgeWriteCaller` (:587) resolves the + forge-write set and gates on `forgeWritesEnabled` (:1583, over + `forgeWriteAppsConfigured` :270-275): with BOTH PEM names absent from the + container resolver's set the predicate reads both-absent, + `warnPartialForgeWriteSecrets` returns SILENTLY (:1826-1827, `havePrimary + == haveReviewer`), `wireForgeWriteCaller` returns nil (:1585), + `hub.SetForgeCaller` is NEVER called, and the reviewer-PEM + `validateForgeSecret` at :1638 is UNREACHABLE — so agent forge WRITES + fail-closed to `CodeUnavailable` fleet-wide with no error at boot. So T0 + has an explicit CONSUMER-REPOINT deliverable: thread the SERVER resolver + instance (not the serve.go:528 container instance) into + `buildForgeReadWiring` (serve.go:567), `buildLinearWebhookWiring`, + `buildLinearTokenSource`, `wireForgeWriteCaller` (:587) and + `buildForgeWriteService`, and into the `newDeclaredSecretResolver` / + `newCachedWebhookSecret` closures (serve.go:1025/1039/1085/1644) that mint + the App token + verify webhook HMAC. `buildLinearWebhookWiring` is called + from INSIDE `buildDoors` (serve.go:788), whose single `resolver` parameter + (:677) ALSO feeds `buildNetworkServer` (:803) → + `runnerhub.NewMountedHandler` (network_door.go:313) — the container + FetchSecrets path that MUST keep reading `secrets`; so `buildDoors` takes + BOTH resolver instances (or receives a pre-built `linearWebhookHandler` + from Serve), because swapping its one `resolver` argument would repoint the + container manifest at `server_secrets` and deliver every server secret into + every agent container, inverting D6. Ordering: the reconcile MUST run + BEFORE `buildForgeReadWiring` (serve.go:567) so the names are in + `server_secrets` before any forge consumer resolves them. - RPC: `SetServerSecret`/`DeleteServerSecret` on the secrets service — mirrors the `SetSecret` declare-then-Set flow and its rollback discipline (secrets_service.go:92-145) minus delivery/kind, targeting `server_secrets`; admin-gated (`adminOnly` in `classifyProcedure`, admin_gate.go:47). Carries the reserved-name guard: rejects - `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix) with an - actionable error — rotation is OQ-1 machinery, never a raw overwrite. + `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix + `GATEWAY_CREDENTIALS_`, so future master-key-family names are reserved + before they are declared) with an actionable error — rotation is OQ-1 + machinery, never a raw overwrite. - User-path server-secret guard (F1 — mandatory, NOT admin-RPC-only): C1 shares the provider keyspace (§D2 read-back verify), so the `authenticatedOpen` user `secretsService.SetSecret`/`DeleteSecret` path @@ -445,24 +476,33 @@ declared into a store that does not exist. `SetSecret`/`DeleteSecret` (and `DeclareSecret` at the store door, so the shadow row can never be created at all) MUST reject any name present in `server_secrets` — checked BEFORE `resolver.Set`/`Delete` - (secrets_service.go:124/208) — in addition to the reserved master-key - name/prefix. A T0 deliverable on the existing user path, not only the new + (secrets_service.go:124/208) — in addition to the reserved master-key name + `GATEWAY_CREDENTIALS_MASTER_KEY` and prefix `GATEWAY_CREDENTIALS_`. A T0 + deliverable on the existing user path, not only the new admin RPC. - Row migration — a BOOT-TIME config-driven reconcile, NOT an in-migration - DML step (F2): the server-only secret NAMES are per-deployment config, - resolved from flags/env in `cmd/compass-server` (e.g. `appKeySecret = - firstNonEmpty(*f.appKeySecret, os.Getenv("COMPASS_FORGE_APP_KEY_SECRET"))`, - main.go:414-417/462) — a value the embedded SQL migration - (`//go:embed migrations/*.sql`, store.go:24-25) cannot know. So the move - runs at boot in the server wiring (where the config IS available): for - each configured server-secret NAME, if a row exists in `secrets` and not - in `server_secrets`, move it — idempotent, serialized under the T2 - advisory lock, no provider write (the shared profile makes it a pure row - move). The complete set is SIX names, not three: the PRIMARY App PEM - (`appKeySecret`, main.go:414-417), the webhook secret (`appWebhook`, - main.go:418-421), the REVIEWER App PEM (`reviewerAppKeySecret`, - main.go:430; consumed serve.go:1644), and the THREE Linear secrets - (client id, client secret, webhook — main.go:435-450). Closes the + DML step (F2): the server-only secret NAMES are per-deployment config the + embedded SQL migration (`//go:embed migrations/*.sql`, store.go:24-25) + cannot know. So the move runs at boot in the server wiring (where the + config IS available): for each configured server-secret NAME, if a row + exists in `secrets` and not in `server_secrets`, move it — idempotent, + serialized under the T2 advisory lock, no provider write (the shared + profile makes it a pure row move). The name source is the RESOLVED config + `cfg.Forge.resolved()` (serve.go:232-246) — the SAME accessor every live + consumer uses (`buildLinearTokenSource` :1700, `buildBoardWebhookWiring` + :1002, `buildForgeWriteService` :1624), NOT the raw flag/env layer: two of + the names carry CODE DEFAULTS applied after that layer + (`defaultForgeLinearClientIDSecretName = "LINEAR_FORGE_CLIENT_ID"` / + `defaultForgeLinearClientSecretName = "LINEAR_FORGE_CLIENT_SECRET"`, + serve.go:202-203), so a reconcile reading the raw `main.go` flag/env layer + would see `""` for a default-named Linear pair and SILENTLY SKIP those two + rows, leaving them on the inject-all path forever — the exact exposure the + reconcile exists to close. The complete set is SIX names, not three: the + PRIMARY App PEM (`appKeySecret`, main.go:414-417), the webhook secret + (`appWebhook`, main.go:418-421), the REVIEWER App PEM + (`reviewerAppKeySecret`, main.go:430; consumed serve.go:1644), and the + THREE Linear secrets (client id, client secret, webhook — resolved names + per `cfg.Forge.resolved()`, provenance main.go:435-450). Closes the pre-existing inject-all exposure (D6) for every configured name the reconcile actually runs for. Same PR chain: OQ-4, RESOLVED. - Reconcile execution context (F2 follow-on — the relocation's RLS @@ -523,13 +563,18 @@ declared into a store that does not exist. resolve through the server resolver and are gone from FetchSecrets; and a configured server-secret row declared under a NON-bootstrap tenant is found and moved by one reconcile pass and is absent from FetchSecrets afterward - (red without `WithSystemRole`: the compass_app-scoped reconcile sees zero - rows and skips it silently). CONSUMER-REPOINT positive assertions (the - (a)/(b) silent modes need them, since an absent name is indistinguishable - from a legitimate off-state): boot a server with all six names configured - and, after one reconcile pass, assert the GitHub App token source mints, - the GitHub and Linear webhook handlers are MOUNTED (non-nil), and the - Linear token source is non-nil — i.e. the forge lanes still wire off the + rows and skips it silently). A deployment with the Linear pair declared under + the DEFAULT names (`LINEAR_FORGE_CLIENT_ID` / `LINEAR_FORGE_CLIENT_SECRET`) + and NO flag/env set is likewise found and moved by one reconcile pass and is + absent from FetchSecrets afterward (red if the reconcile reads the raw + flag/env layer instead of `resolved()`). CONSUMER-REPOINT positive + assertions (the (b)/(c) silent modes need them, since an absent name is + indistinguishable from a legitimate off-state): boot a server with all six + names configured and, after one reconcile pass, assert the GitHub App token + source mints, the GitHub and Linear webhook handlers are MOUNTED (non-nil), + the Linear token source is non-nil, AND the forge WRITE caller is MOUNTED + (`hub.SetForgeCaller` called / `RelayForgeCall` does not fail-close to + `CodeUnavailable`) — i.e. the forge read AND write lanes still wire off the server resolver, not just that the names left `secrets`. MEMBERSHIP-GUARD assertion (F1): a non-admin authenticated caller invoking `SetSecret` with a configured server-secret NAME (not only the reserved master-key name) is @@ -601,25 +646,37 @@ is declared into it) and T1. resolve, so the provisioner re-generates, re-Sets, and Declares — no row was ever encrypted under the orphaned value). - **Bounded critical section (mandatory — a stuck provider must not wedge - the fleet):** the provider round-trips inside the lock (`resolver.Set` / - `Resolve`) inherit only the caller's ctx, which at boot is long-lived, and - `SpecResolver.Set` shells out via `exec.CommandContext(ctx, r.cli, …)` - (resolver.go:233) with no timeout of its own. A hung provider (1Password - awaiting biometric approval, an unreachable Vault, a half-open TCP) would - otherwise hold the transaction-scoped lock indefinitely, and because the - key is a shared constant EVERY other booting instance blocks on it — one - stuck provider becomes a fleet-wide boot wedge. So the provisioner derives - a ctx with an explicit timeout (mirror the 30s - `&http.Client{Timeout: 30 * time.Second}` precedent that already bounds - the Linear boot mint at serve.go:1731, under the comment "an unbounded - doer would let a half-open TCP … wedge Serve's whole boot") around the - resolve/Set calls, and acquires the lock with `pg_try_advisory_xact_lock` - in a bounded retry loop (or sets a session `lock_timeout`) so a booter that + the fleet):** the provider round-trips inside the lock inherit only the + caller's ctx, which at boot is long-lived — but the two halves bound + DIFFERENTLY. `SpecResolver.Set` IS ctx-bounded: it shells out via + `exec.CommandContext(ctx, r.cli, …)` (resolver.go:233), so a ctx deadline + genuinely kills it. `SpecResolver.Resolve` is NOT: it threads ctx only into + `DeclaredSecrets` (resolver.go:136); the actual provider round-trip is + `b.Load()` (resolver.go:165), whose SDK signature carries NO ctx + (`func (b *Builder) Load() (*Resolved, error)`, secretspec-go v0.15.0 + secretspec.go:245) and which blocks in an uncancellable FFI call + (`nativeResolve` → `C.secretspec_resolve`, binding_cgo.go:26 / + binding_purego.go:118). A hung provider (1Password awaiting biometric + approval, an unreachable Vault, a half-open TCP) would otherwise hold the + transaction-scoped lock indefinitely, and because the key is a shared + constant EVERY other booting instance blocks on it — one stuck provider + becomes a fleet-wide boot wedge. So the provisioner (1) derives a ctx with + an explicit timeout (mirror the 30s `&http.Client{Timeout: 30 * + time.Second}` precedent that already bounds the Linear boot mint at + serve.go:1731) and, because the Resolve-side call cannot be cancelled, RUNS + THE BOOT RESOLVE/SET ON ITS OWN GOROUTINE AND SELECTS ON THAT CTX — so the + provisioner returns a diagnosable bounded startup error (naming the hung + provider) and its transaction is rolled back, RELEASING the xact-scoped + advisory lock, while the orphaned FFI goroutine is knowingly leaked for the + process's remaining boot-failing lifetime (acceptable: the boot is aborting + anyway); and (2) acquires the lock with `pg_try_advisory_xact_lock` in a + bounded retry loop (or sets a session `lock_timeout`) so a booter that cannot get the lock fails closed with a diagnosable startup error naming the contended provisioning lock rather than parking forever. Test (T2): a - provider that never returns yields a bounded, diagnosable boot failure, and - a second instance blocked on the lock also fails bounded rather than - hanging. + provider that never returns yields a bounded, diagnosable boot failure + whose transaction (and advisory lock) is released even though the provider + call itself cannot be cancelled, and a second instance blocked on the lock + also fails bounded rather than hanging. - **Read-back verify, every boot:** after provisioning AND on every subsequent boot, re-resolve the name and byte-compare against the key the process is about to encrypt with; on mismatch, refuse to serve @@ -652,7 +709,8 @@ is declared into it) and T1. every new row is sealed under a known key: the exact bulk-disclosure D1 prevents). Therefore the reserved-name guard is MANDATORY on the user `secretsService.SetSecret`/`DeleteSecret` path (reject the reserved name - and prefix BEFORE `resolver.Set`/`Delete`), a T0/T2 deliverable, AND on + `GATEWAY_CREDENTIALS_MASTER_KEY` and prefix `GATEWAY_CREDENTIALS_` BEFORE + `resolver.Set`/`Delete`), a T0/T2 deliverable, AND on the new `SetServerSecret`/`DeleteServerSecret` RPC. Both are tested red-green (a non-admin user calling `SetSecret` with the reserved name is rejected and the provider value is unchanged). Rotation is OQ-1's From 30b5d94f13ae636ba3c4325029fea4fbf99c9c9a Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 17:56:31 -0400 Subject: [PATCH 07/18] docs(server): fix round-6 review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-6 adversarial review returned 0H/2M/3L on the gateway-credentials at-rest encryption addendum. Both round-5 HIGH fixes verified correct and tree-accurate; the two mediums are defects the round-5 fold itself introduced or left, plus three low citation/mechanic nits. All folded as a new commit atop the bookmark (additive review-fix convention). - M1 (text-corruption regression): the round-5 edit deleted the opening half of the T0 non-bootstrap-tenant test's red-condition parenthetical, leaving a dangling `rows and skips it silently).` fragment. Restored the full clause (`red without WithSystemRole: the compass_app-scoped reconcile sees zero rows...`) — the single sentence stating why the test must fail, and the entire justification for DL-326's BYPASSRLS widening. T0 Tests-block paren balance re-verified 33/33. - M2 (self-contradictory enumeration): the C2 named-residual bullet counted "all SIX ... master key + ... two Linear client secrets", contradicting the record's canonical set (six MIGRATED names = primary+reviewer App PEM + webhook + three Linear secrets, master key a seventh provisioned directly). Restated as all SEVEN server-secret names with the master-key-vs-migrated split explicit, so the F1 membership guard's coverage set names the Linear webhook secret. - L1: qualified the T2 fingerprint placement to mirror T5 (persisted server-side alongside the master-key declaration; the server_secrets DDL has no column for it yet). - L2: named the H2 goroutine mechanic — the parent goroutine owns the pgx.Tx (not concurrency-safe) and does the rollback; only the ctx-less provider call is offloaded, reporting through a buffered (cap-1) channel so the leaked FFI goroutine can send and exit rather than block forever. - L3: softened the superuser-owner citation to what the cited lines establish (0001_init.sql:848-851 states the property; rls_pgtest_test.go asserts it of the harness, not production). markdownlint clean; design-ledger-gate OK (293 rows, 123 headers valid; DL-325/DL-326 anchors still resolve, record 60403 B). Ledger-impact: none (DL-325/DL-326 unchanged). Spec-impact: none (design record only). Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...-gateway-credentials-at-rest-encryption.md | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index 959feeb3..a863ac56 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -303,8 +303,10 @@ rejected branches are the ones a future reader will reach for first. split remains available later if provider-keyspace isolation is ever needed. Named residual under C1: the shared profile leaves the provider keyspace a shared mutable surface reachable by the user `SetSecret` path, - so all SIX server-secret names' provider values (master key + primary and - reviewer App PEM + webhook + two Linear client secrets) are guarded by the + so all SEVEN server-secret names' provider values (the master key plus the + six migrated names: primary and reviewer App PEM, the GitHub webhook secret, + and the three Linear secrets — client id, client secret, and webhook) are + guarded by the F1 membership check (T0, both paths — reject any `server_secrets` name) RATHER than structurally; the boot read-back verify additionally covers the master key. C2 would make that isolation structural for all of them. @@ -541,12 +543,13 @@ declared into a store that does not exist. the request path" clause (DECISIONS.md:113) is UNCHANGED — this is a boot step, never the request path — but its named entrypoint set is widened, recorded (OQ-5, Matt-ruled) as its own ledger row DL-326 that Refines - DL-315. (Note: a raw-owner-pool reconcile would move every - tenant's rows today only by accident — the owner connection is a superuser, - and a superuser bypasses even FORCE, 0001_init.sql:848-851 / - rls_pgtest_test.go:282-284. The tree deliberately refuses to rely on that - accident, which is why FORCE is set at all; the reconcile takes the explicit - BYPASSRLS path rather than inheriting an owner-privilege escape.) + DL-315. (Note: a raw-owner-pool reconcile would move every tenant's rows + only if the owner connection is a superuser — which the tree ASSUMES but + does not enforce: 0001_init.sql:848-851 states the property that a superuser + owner bypasses even FORCE, and rls_pgtest_test.go:282-284 asserts it of the + pgtest harness, not of production. The tree deliberately refuses to rely on + that assumption, which is why FORCE is set at all; the reconcile takes the + explicit BYPASSRLS path rather than inheriting an owner-privilege escape.) - Consumes: nothing from T1-T5 (pure prerequisite). - Tests: a secret declared in `server_secrets` is ABSENT from a FetchSecrets response (because it lives in the other table — no filter @@ -563,6 +566,7 @@ declared into a store that does not exist. resolve through the server resolver and are gone from FetchSecrets; and a configured server-secret row declared under a NON-bootstrap tenant is found and moved by one reconcile pass and is absent from FetchSecrets afterward + (red without `WithSystemRole`: the compass_app-scoped reconcile sees zero rows and skips it silently). A deployment with the Linear pair declared under the DEFAULT names (`LINEAR_FORGE_CLIENT_ID` / `LINEAR_FORGE_CLIENT_SECRET`) and NO flag/env set is likewise found and moved by one reconcile pass and is @@ -669,7 +673,13 @@ is declared into it) and T1. provider) and its transaction is rolled back, RELEASING the xact-scoped advisory lock, while the orphaned FFI goroutine is knowingly leaked for the process's remaining boot-failing lifetime (acceptable: the boot is aborting - anyway); and (2) acquires the lock with `pg_try_advisory_xact_lock` in a + anyway). The PARENT goroutine owns the transaction (`pgx.Tx` is not + concurrency-safe): it takes the advisory lock, offloads ONLY the ctx-less + provider call, and on the timeout branch performs the `Rollback` itself; the + offloaded call reports through a BUFFERED (cap-1) channel so the orphaned + FFI goroutine can complete its send and exit rather than blocking forever on + an abandoned receiver (bounding the leak on a crash-looping boot). And (2) + acquires the lock with `pg_try_advisory_xact_lock` in a bounded retry loop (or sets a session `lock_timeout`) so a booter that cannot get the lock fails closed with a diagnosable startup error naming the contended provisioning lock rather than parking forever. Test (T2): a @@ -690,9 +700,10 @@ is declared into it) and T1. not a boundary: on a plain boot the resolved value IS "the key the process is about to encrypt with", so a byte-compare cannot by itself distinguish "my key" from "a swapped key". T5 strengthens it by binding - a non-secret key fingerprint (a salted digest, stored beside - `key_version`) so a swapped provider value is DETECTED at boot rather - than silently adopted. + a non-secret key fingerprint (a salted digest, persisted server-side + alongside the master-key declaration — see T5 for placement, which the + `server_secrets` DDL has no column for yet) so a swapped provider value is + DETECTED at boot rather than silently adopted. - **Reserved-name guard (F1 — on EVERY provider-writing path):** C1 splits the DECLARATION registries (two tables) but NOT the provider keyspace (one shared profile, above). So the isolation C1 buys is that server From 882f6d79b99a9b697cca27f1d288d0b82f826a57 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 19:22:25 -0400 Subject: [PATCH 08/18] docs(server): fix round-7 review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-7 review returned 0 high / 4 medium / 3 low on PR #859; all seven grounded against the tree at `eb5ef7a1` and folded additively. ### Mediums - **M1 — key-swap tripwire had no schema home.** T5 placed the non-secret key fingerprint "beside `key_version` in `server_secrets`", but `key_version` is a T3 column on `gateway_credentials` and `server_secrets` is names-only — a circular, unimplementable reference for the T0-then-T2 executor. Gave the tripwire a real home: a single-row `server_key_state` table (`key_version`/`key_fingerprint`/`fingerprint_salt`) in T0's migration, bucket-A infrastructure joining the RLS allow-list; a salted digest is not a VALUE, so the names-only invariant is untouched. Rewrote the T2 pointer and T5 placement to name it; added it to the T0 Tasks line. - **M2 — superuser citation inverted.** The record claimed `rls_pgtest_test.go:282-284` asserts the superuser-owner property "not of production", but the cited comment reads "the pgtest harness (like production) connects as a SUPERUSER owner" — the opposite — and it is a prose comment, not an assertion. Rewrote the parenthetical to match the cited text (assumption stated in two comments, not an enforced invariant). - **M3 — bounded-critical-section self-contradiction.** One sentence said "runs the boot RESOLVE/SET on its own goroutine" while the same paragraph establishes `Set` is ctx-bounded (`exec.CommandContext`, resolver.go:233) and "offloads ONLY the ctx-less provider call". Under the RESOLVE/SET reading a leaked goroutine could `Set` after the parent's Rollback, outside the released advisory lock, overwriting a key another booter already sealed rows under — the exact race the lock exists to close. Made it unambiguous: only the ctx-less `Resolve` is offloaded; the parent runs `Set` in-lock on the success branch; the offloaded goroutine performs no provider write. - **M4 — F1 membership guard one-directional.** The user path rejected a `server_secrets` name, but `SetServerSecret` carried no mirror guard against a name already live in `secrets`, and the reconcile skipped a both-tables name — the same D6-inverting end state, reachable in the opposite order and unhealed. Restated the invariant order-free ("no name ever in both tables"), made F1 symmetric (mirror guard on the admin RPC), made the reconcile self-healing (deletes the `secrets` shadow row on collision), and added the two red-green tests. ### Lows - **L1 (DECISIONS.md DL-325)** — named the symmetric F1 membership guard alongside the reserved-name guard (the reserved-name check covers only 1 of 7 names). - **L2** — added an ordering note that failure mode (c) is masked by (a) on a full no-repoint and is the partial-repoint mode. - **L3** — split the mode-(b) citation to its two real sites (`forgeSecretDeclared` at serve.go:1746-1757; its caller's off-state at :1082-1083). Design record only; no code or proto change. Both prior-round HIGHs (forge taxonomy, Resolve cancellation asymmetry) re-verified intact. markdownlint 0, design-ledger-gate OK (293 rows, anchors resolve at 64364 B). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 2 +- ...-gateway-credentials-at-rest-encryption.md | 113 +++++++++++++----- 2 files changed, 84 insertions(+), 31 deletions(-) diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index afee61b4..8ebc56ba 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,7 +112,7 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | -| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so a reserved-name guard rejecting the master-key name is MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | +| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so TWO guards are MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path: the reserved-name guard on `GATEWAY_CREDENTIALS_*`, and — because the other six names are arbitrary per-deployment config with no reserved prefix — a SYMMETRIC F1 membership guard rejecting any name present in the other table (in both declaration orders). The table split alone is necessary but not sufficient for the structural claim. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | | DL-326 | The DL-315 `BYPASSRLS` system-role allow-list is EXTENDED to admit the boot server-secret reconcile — a cross-tenant, request-path-free boot step that moves the configured server-secret rows from `secrets` (FORCE RLS, tenant-scoped) into `server_secrets`, run under `store.WithSystemRole`; the entrypoint enumeration is restated against the SHIPPED set, which already includes the forge-notification-ack arm (`go/internal/runnerhub/hub.go`, `forgeNotificationAck`, currently :814) that DL-315's four names omit. DL-315's load-bearing NEVER-on-the-request-path clause is unchanged — every request-path query stays fail-closed under RLS. Refines DL-315 (which stays Active) | Active (Matt, 2026-09-04) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#t0--the-server_secrets-store-table-resolver-split-admin-rpc-prerequisite) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index a863ac56..baa1225b 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -216,14 +216,18 @@ READ: a store view over `server_secrets` (`ServerDeclaredSecrets`, T0) feeds a SECOND `SpecResolver` instance. The container resolver keeps reading `secrets`; its manifest never contains a server-secret name, so a server secret is undeliverable to containers via the container manifest — there is -nothing to filter. That structural property holds only while no name in -`server_secrets` is ALSO re-declared into `secrets`: because the two tables -share a keyspace and `secrets.name` is a global PK with no cross-table -exclusion, an `authenticatedOpen` `SetSecret` could otherwise mint a shadow -`secrets` row under a server-secret name and get it container-delivered. The -F1 membership guard (T0 — reject any `server_secrets` name on the user -declare/set path) is what closes that gap and makes the undeliverability -total; the table split alone is necessary but not sufficient. +nothing to filter. That structural property holds only while no name is +ever present in BOTH tables: because the two tables share a keyspace and each +`name` is a per-table PK with no cross-table exclusion, the same both-tables +end state is reachable in EITHER declaration order — an `authenticatedOpen` +`SetSecret` minting a shadow `secrets` row under a server-secret name, OR an +admin `SetServerSecret` declaring a server secret under a name already live in +`secrets` — and either gets that name container-delivered. The F1 membership +guard is therefore SYMMETRIC (T0 — enforced on every path that can create a +row in either table: reject a `server_secrets` name on the user declare/set +path, and reject a `secrets` name on the admin server-secret path) and the +boot reconcile is self-healing (below); together they make the +undeliverability total. The table split alone is necessary but not sufficient. C1 keeps the SAME SecretSpec profile for both instances — the shared project is `manifestProject = "compass"` (resolver.go:19) and the profile @@ -371,6 +375,19 @@ declared into a store that does not exist. `created_at`/`updated_at`. NO `delivery` column (server secrets are never container-delivered — that IS the point) and NO `kind` column (they never reach the T5 materializer). + - Key-state table (same migration): a single-row `server_key_state` table + (`id SMALLINT PRIMARY KEY DEFAULT 1 CHECK (id = 1)`, `key_version SMALLINT + NOT NULL DEFAULT 1`, `key_fingerprint BYTEA NOT NULL`, `fingerprint_salt + BYTEA NOT NULL`, `updated_at`) holds the T2/T5 key-swap tripwire's + non-secret salted digest. It is NOT a declared-name registry, so the + names-only invariant (§Global Constraints) is untouched — a salted digest + of a key is not that key's VALUE. Same-migration `GRANT SELECT, INSERT, + UPDATE ON server_key_state TO compass_app, compass_system` (the grant is + not inherited — see the `server_secrets` grant rationale above). It is + likewise **bucket-A infrastructure** (deployment-global, no `tenant_id`, + RLS NOT enabled) and joins `server_secrets` in the `bucketA` allow-list + (rls_pgtest_test.go:543) so the self-auditing RLS guard test stays a + conscious gate. - Grants + RLS (mandatory — the new-table integration the shipped multi-tenancy regime, RIG-3106, requires): the same new migration issues `GRANT SELECT, INSERT, UPDATE, DELETE ON server_secrets TO @@ -425,10 +442,13 @@ declared into a store that does not exist. secret (:1016) — both inside `buildBoardWebhookWiring` under `buildForgeReadWiring`, propagated to `failStartup`, so every GitHub-App deployment fails its first post-upgrade boot; (b) SILENT SECURITY - DEGRADATION — `forgeSecretDeclared` (serve.go:1082-1083) treats an absent - name as a legitimate off-state (`nil, nil`), so `buildLinearWebhookWiring` - silently unmounts the Linear webhook ingress and `buildLinearTokenSource` - (serve.go:1699-1717) silently drops the Linear write + notify lanes; + DEGRADATION — `forgeSecretDeclared` (serve.go:1746-1757) returns + `(false, nil)` for an absent name rather than an error, and its caller + `buildLinearWebhookWiring` maps that to a legitimate off-state — `if + !declared { return nil, nil }` (serve.go:1082-1083) — so the Linear webhook + ingress silently unmounts, and `buildLinearTokenSource` (serve.go:1699-1717) + drops the Linear write + notify lanes the same way (`return nil, nil` at + :1716); (c) SILENT CAPABILITY LOSS — `wireForgeWriteCaller` (:587) resolves the forge-write set and gates on `forgeWritesEnabled` (:1583, over `forgeWriteAppsConfigured` :270-275): with BOTH PEM names absent from the @@ -437,7 +457,16 @@ declared into a store that does not exist. == haveReviewer`), `wireForgeWriteCaller` returns nil (:1585), `hub.SetForgeCaller` is NEVER called, and the reviewer-PEM `validateForgeSecret` at :1638 is UNREACHABLE — so agent forge WRITES - fail-closed to `CodeUnavailable` fleet-wide with no error at boot. So T0 + fail-closed to `CodeUnavailable` fleet-wide with no error at boot. + (Ordering note: (c) is MASKED by (a) on a FULL no-repoint T0 — + `forgeWriteAppsConfigured` gates `havePrimary` on `App.AppID != 0` + (serve.go:272), which is exactly `boardIngestionEnabled()` (serve.go:224-226), + so any writes-enabled deployment hard-fails at the primary-App-PEM + `validateForgeSecret` (serve.go:1013) before reaching `wireForgeWriteCaller` + (:587). (c) is the PARTIAL-repoint mode: it becomes live the moment the read + path is repointed and the write path is not — which is precisely why the + write lane needs its own positive assertion rather than inheriting the read + lane's.) So T0 has an explicit CONSUMER-REPOINT deliverable: thread the SERVER resolver instance (not the serve.go:528 container instance) into `buildForgeReadWiring` (serve.go:567), `buildLinearWebhookWiring`, @@ -463,7 +492,13 @@ declared into a store that does not exist. `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix `GATEWAY_CREDENTIALS_`, so future master-key-family names are reserved before they are declared) with an actionable error — rotation is OQ-1 - machinery, never a raw overwrite. + machinery, never a raw overwrite. It ALSO carries the MIRROR membership + guard: it rejects any name already present in `secrets` (checked before the + `server_secrets` insert and before `resolver.Set`) with an actionable error + naming the conflicting user declaration — the F1 invariant is order-free + (no name is ever live in both tables) and must hold in BOTH declaration + orders, so the `secrets`→`server_secrets` direction is guarded here just as + the `server_secrets`→`secrets` direction is guarded on the user path below. - User-path server-secret guard (F1 — mandatory, NOT admin-RPC-only): C1 shares the provider keyspace (§D2 read-back verify), so the `authenticatedOpen` user `secretsService.SetSecret`/`DeleteSecret` path @@ -487,7 +522,11 @@ declared into a store that does not exist. embedded SQL migration (`//go:embed migrations/*.sql`, store.go:24-25) cannot know. So the move runs at boot in the server wiring (where the config IS available): for each configured server-secret NAME, if a row - exists in `secrets` and not in `server_secrets`, move it — idempotent, + exists in `secrets`, move it (deleting the `secrets` row) — INCLUDING when a + `server_secrets` row for that name already exists, in which case the pass + deletes the `secrets` shadow row and logs the reconciled collision. Leaving + a both-tables name in place would strand it on the inject-all path forever, + so the reconcile heals the collision rather than skipping it. Idempotent, serialized under the T2 advisory lock, no provider write (the shared profile makes it a pure row move). The name source is the RESOLVED config `cfg.Forge.resolved()` (serve.go:232-246) — the SAME accessor every live @@ -546,9 +585,10 @@ declared into a store that does not exist. DL-315. (Note: a raw-owner-pool reconcile would move every tenant's rows only if the owner connection is a superuser — which the tree ASSUMES but does not enforce: 0001_init.sql:848-851 states the property that a superuser - owner bypasses even FORCE, and rls_pgtest_test.go:282-284 asserts it of the - pgtest harness, not of production. The tree deliberately refuses to rely on - that assumption, which is why FORCE is set at all; the reconcile takes the + owner bypasses even FORCE, and rls_pgtest_test.go:281-284 records the same + assumption in a comment ("the pgtest harness (like production) connects as a + SUPERUSER owner"), which is why that test deliberately does not try to prove + FORCE. An assumption stated in two comments is not an enforced invariant; the reconcile takes the explicit BYPASSRLS path rather than inheriting an owner-privilege escape.) - Consumes: nothing from T1-T5 (pure prerequisite). - Tests: a secret declared in `server_secrets` is ABSENT from a @@ -668,17 +708,25 @@ is declared into it) and T1. an explicit timeout (mirror the 30s `&http.Client{Timeout: 30 * time.Second}` precedent that already bounds the Linear boot mint at serve.go:1731) and, because the Resolve-side call cannot be cancelled, RUNS - THE BOOT RESOLVE/SET ON ITS OWN GOROUTINE AND SELECTS ON THAT CTX — so the + THE ctx-LESS `Resolve` ON ITS OWN GOROUTINE AND SELECTS ON THAT CTX (`Set` is + ctx-bounded and stays on the parent, inside the lock) — so the provisioner returns a diagnosable bounded startup error (naming the hung provider) and its transaction is rolled back, RELEASING the xact-scoped advisory lock, while the orphaned FFI goroutine is knowingly leaked for the process's remaining boot-failing lifetime (acceptable: the boot is aborting anyway). The PARENT goroutine owns the transaction (`pgx.Tx` is not concurrency-safe): it takes the advisory lock, offloads ONLY the ctx-less - provider call, and on the timeout branch performs the `Rollback` itself; the - offloaded call reports through a BUFFERED (cap-1) channel so the orphaned - FFI goroutine can complete its send and exit rather than blocking forever on - an abandoned receiver (bounding the leak on a crash-looping boot). And (2) + provider READ (`Resolve`) and NEVER a provider write, and on the timeout + branch performs the `Rollback` itself and discards the buffered result + without acting on it. A `Set` reached after the parent's Rollback would land + OUTSIDE the released advisory lock and could overwrite a key another booter + has already provisioned and begun sealing rows under — reintroducing the + silently-undecryptable-rows failure the lock exists to prevent — so the + offloaded goroutine performs no `Set`; the parent runs the (ctx-bounded) + `Set` itself, in-lock, only on the success branch. The offloaded call + reports through a BUFFERED (cap-1) channel so the orphaned FFI goroutine can + complete its send and exit rather than blocking forever on an abandoned + receiver (bounding the leak on a crash-looping boot). And (2) acquires the lock with `pg_try_advisory_xact_lock` in a bounded retry loop (or sets a session `lock_timeout`) so a booter that cannot get the lock fails closed with a diagnosable startup error naming @@ -700,10 +748,10 @@ is declared into it) and T1. not a boundary: on a plain boot the resolved value IS "the key the process is about to encrypt with", so a byte-compare cannot by itself distinguish "my key" from "a swapped key". T5 strengthens it by binding - a non-secret key fingerprint (a salted digest, persisted server-side - alongside the master-key declaration — see T5 for placement, which the - `server_secrets` DDL has no column for yet) so a swapped provider value is - DETECTED at boot rather than silently adopted. + a non-secret key fingerprint (a salted digest, persisted in the T0 + `server_key_state` row — `key_fingerprint`/`fingerprint_salt`, not a + declared-name registry so the names-only invariant is untouched) so a + swapped provider value is DETECTED at boot rather than silently adopted. - **Reserved-name guard (F1 — on EVERY provider-writing path):** C1 splits the DECLARATION registries (two tables) but NOT the provider keyspace (one shared profile, above). So the isolation C1 buys is that server @@ -828,9 +876,10 @@ RPC exactly as the frozen record already specifies. these. The fmt verbs alone mirror the existing pattern's intent (secrets.go:155-156) but do not cover the reflection paths (D5). - **Key-swap tripwire (F1):** persist a non-secret key fingerprint (a - salted digest of the master key, stored beside `key_version` in - `server_secrets` or a sibling row) at provision time; the boot read-back - verify recomputes it and refuses to serve on mismatch. Test: swap the + salted digest of the master key, written to the T0 `server_key_state` row — + `key_fingerprint` + `fingerprint_salt`, alongside `key_version`) at provision + time; the boot read-back verify recomputes it and refuses to serve on + mismatch. Test: swap the provider value out-of-band, reboot → boot fails closed (the fingerprint distinguishes "a key" from "my key", which a bare byte-compare cannot). - Refresh write-back path: refreshed OAuth tokens land re-sealed with a @@ -842,6 +891,7 @@ RPC exactly as the frozen record already specifies. ## Tasks - [ ] T0 — `server_secrets` store (mechanism C1): new migration (table + + single-row `server_key_state` table for the key-swap tripwire digest + GRANT SELECT/INSERT/UPDATE/DELETE to compass_app/compass_system + bucketA allow-list edit in rls_pgtest_test.go), `ServerDeclaredSecrets` store view + second SpecResolver instance (same profile), admin-gated @@ -854,6 +904,9 @@ RPC exactly as the frozen record already specifies. compass_app-scoped path is confined to the bootstrap tenant, so a row declared under another tenant is otherwise silently skipped); absent-from-FetchSecrets + server-resolver-resolves + user-path-guard + + admin-path-mirror-guard (a `SetServerSecret` on a name already in + `secrets` is rejected) + reconcile-heals-both-tables-collision (one pass + deletes the `secrets` shadow row and the name leaves FetchSecrets) + non-bootstrap-tenant-reconcile red-green tests. PREREQUISITE of T2. NO proto change. - [ ] T1 — `go/internal/secrets/envelope`: Key/NewKey/KeyFromBytes/Seal/Open From 543c18d2eb8ef8c44f2893eede6ee21dded185d7 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 20:55:22 -0400 Subject: [PATCH 09/18] docs(server): fix round-8 review findings on the encryption addendum (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-8 review returned 0H/3M/6L; all seven round-7 folds HELD and both prior HIGHs (3-mode forge taxonomy, Resolve cancellation asymmetry) remain intact. The three mediums are consistency gaps the large round-7 M1/M4 folds left behind; all nine findings folded, each grounded against the tree at `eb5ef7a1`. - M (finding 1): the two M4 red-green tests landed only in the Tasks checklist, never in T0's own `- Tests:` contract bullet (the per-task test contract an executor implements against). Mirrored the admin-path MIRROR-GUARD and the RECONCILE-HEAL assertions into the `- Tests:` bullet. - M (finding 2): a D6 sentence claimed the user `SetSecret` path "could not carry a server secret anyway", contradicting the F1 user-path guard rationale 250 lines later. The cite (secrets_service.go:275-284) is byte-exact but the inference was false — rejecting an unspecified delivery does not stop a caller minting a shadow `secrets` row under a server-secret NAME. Narrowed the sentence to the true claim and made it point AT the guard as mandatory, not away from it. - M (finding 3): "together they make the undeliverability total" overstated against the record's own cited non-atomicity disclaimer (secrets_service.go:88-91: "not atomic and assumes no concurrent same-name writer"). Both guards are non-atomic check-then-act; a concurrent same-name writer is a residual both-tables window. Replaced "total" with the honest bound (the guards close every SEQUENTIAL order; the concurrent residual is the same single-writer MVP assumption the existing path already carries, atomic closure deferred to the multi-writer work the comment already flags). Also widened the boot reconcile from the six configured names to a full `server_secrets` scan so an operator-declared server secret raced into both tables self-heals too, regardless of provenance. - L (findings 4-9): pointer "above" -> "below" for the server_key_state grant rationale; named `server_key_state` in the names-only Global Constraint; added the bounded-offload clause to the every-boot read-back verify (same uncancellable Load on a steady-state boot); `admin_gate.go:119` -> `:122-125` (the wrapped-comment line -> the actual authenticatedOpen case block, matching the correct cite already at :504); re-wrapped the M2-fold's 101-col line to ~78; disambiguated the Tasks GRANT summary so server_key_state reads SELECT/INSERT/UPDATE (no DELETE) vs server_secrets's full grant. Record 64364 -> 67086 B (>50KB; both DECISIONS.md links still resolve — D1/T0 headings untouched). markdownlint 0, design-ledger-gate OK (293 rows, 123 headers). Interdiff +51/-15, record only. Refs RIG-2863 Spec-impact: design record only; no code or ledger-row change. The concurrent-writer atomic closure is deferred, not added — this record does not widen the single-writer contract. Co-authored-by: Matt Wilkinson --- ...-gateway-credentials-at-rest-encryption.md | 66 ++++++++++++++----- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index baa1225b..a5756125 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -226,8 +226,18 @@ admin `SetServerSecret` declaring a server secret under a name already live in guard is therefore SYMMETRIC (T0 — enforced on every path that can create a row in either table: reject a `server_secrets` name on the user declare/set path, and reject a `secrets` name on the admin server-secret path) and the -boot reconcile is self-healing (below); together they make the -undeliverability total. The table split alone is necessary but not sufficient. +boot reconcile is self-healing (below). The table split alone is necessary but +not sufficient. Together the symmetric guard and the reconcile close every +SEQUENTIAL declaration order. Each guard is a membership SELECT against the +other table followed by an INSERT into this one, and the trio "is not atomic and +assumes no concurrent same-name writer" (secrets_service.go:88-91) — the same +single-writer MVP assumption the existing `SetSecret` path already carries — +so a concurrent same-name `SetSecret`/`SetServerSecret` pair remains the one +residual both-tables window; the boot reconcile heals it on the next boot (see +the full-scan note below). Closing that window atomically (a shared per-name +advisory lock across both write paths) is deferred to the multi-writer work the +cited comment already flags, tracked with the per-tenant defer (RIG-3237); this +record does not widen the single-writer contract. C1 keeps the SAME SecretSpec profile for both instances — the shared project is `manifestProject = "compass"` (resolver.go:19) and the profile @@ -255,10 +265,13 @@ Writes into `server_secrets` go through a NEW admin-gated delivery/kind, classified `adminOnly` in `classifyProcedure` (`go/internal/auth/admin_gate.go:47`, :27; "An unrecognized path (ok=false) is treated as adminOnly — fail closed, never admit an unknown method as -open", admin_gate.go:44-46). Today's user-facing `SetSecret` could not carry -a server secret anyway: `secretRoutingFromProto` accepts only File/Env -delivery (secrets_service.go:275-284) — C1 routes server secrets to the new -RPC instead. +open", admin_gate.go:44-46). Today's user-facing `SetSecret` cannot declare a +row *as* a server secret — `secretRoutingFromProto` admits only File/Env +delivery (secrets_service.go:275-284), with no server-only delivery value — so +operators declare server secrets through the new RPC. It CAN, however, mint an +ordinary `secrets` row under a server-secret NAME (delivery=File/Env, an +arbitrary name), which is exactly why the F1 membership guard is mandatory on +this user path and not merely on the admin RPC. This retroactively fixes a pre-existing exposure: the App PEM, webhook signing, and Linear OAuth secrets are server_only by convention only @@ -349,7 +362,10 @@ rejected branches are the ones a future reader will reach for first. - The names-only invariant (`secrets.go:20-22`) is preserved for EVERYTHING except `gateway_credentials` values; both declared-name registries (`secrets` AND the new `server_secrets`) stay names-only — the master - key's declaration row is names-only like any other. + key's declaration row is names-only like any other. `server_key_state` is + not a declared-name registry and holds no value: a salted digest of the + master key plus its salt, which discloses nothing about a 256-bit random + key — the names-only invariant is untouched by it. - Every value-bearing type redacts under `%s`/`%v`/`%#v` (D5). - The master key lives in the separate `server_secrets` store (D6): its name never appears in the container resolver's manifest, so it is NEVER @@ -383,7 +399,7 @@ declared into a store that does not exist. names-only invariant (§Global Constraints) is untouched — a salted digest of a key is not that key's VALUE. Same-migration `GRANT SELECT, INSERT, UPDATE ON server_key_state TO compass_app, compass_system` (the grant is - not inherited — see the `server_secrets` grant rationale above). It is + not inherited — see the `server_secrets` grant rationale below). It is likewise **bucket-A infrastructure** (deployment-global, no `tenant_id`, RLS NOT enabled) and joins `server_secrets` in the `bucketA` allow-list (rls_pgtest_test.go:543) so the self-auditing RLS guard test stays a @@ -526,7 +542,13 @@ declared into a store that does not exist. `server_secrets` row for that name already exists, in which case the pass deletes the `secrets` shadow row and logs the reconciled collision. Leaving a both-tables name in place would strand it on the inject-all path forever, - so the reconcile heals the collision rather than skipping it. Idempotent, + so the reconcile heals the collision rather than skipping it. The heal is + NOT limited to the configured names: after moving the configured set, the + pass also scans `server_secrets` for any name that STILL has a live + `secrets` row (an operator-declared server secret raced into both tables, + which carries no reserved prefix and is not in `cfg.Forge.resolved()`) and + deletes that shadow row too, so every both-tables state self-heals + regardless of provenance. Idempotent, serialized under the T2 advisory lock, no provider write (the shared profile makes it a pure row move). The name source is the RESOLVED config `cfg.Forge.resolved()` (serve.go:232-246) — the SAME accessor every live @@ -588,8 +610,9 @@ declared into a store that does not exist. owner bypasses even FORCE, and rls_pgtest_test.go:281-284 records the same assumption in a comment ("the pgtest harness (like production) connects as a SUPERUSER owner"), which is why that test deliberately does not try to prove - FORCE. An assumption stated in two comments is not an enforced invariant; the reconcile takes the - explicit BYPASSRLS path rather than inheriting an owner-privilege escape.) + FORCE. An assumption stated in two comments is not an enforced invariant; + the reconcile takes the explicit BYPASSRLS path rather than inheriting an + owner-privilege escape.) - Consumes: nothing from T1-T5 (pure prerequisite). - Tests: a secret declared in `server_secrets` is ABSENT from a FetchSecrets response (because it lives in the other table — no filter @@ -623,7 +646,14 @@ declared into a store that does not exist. assertion (F1): a non-admin authenticated caller invoking `SetSecret` with a configured server-secret NAME (not only the reserved master-key name) is rejected, no `secrets` shadow row is created, and the provider value is - unchanged. + unchanged. MIRROR-GUARD assertion (F1, admin path): an admin calling + `SetServerSecret` with a name already present in `secrets` is rejected + before the `server_secrets` insert and before `resolver.Set`, no + `server_secrets` row is created, and the provider value is unchanged. + RECONCILE-HEAL assertion: a name live in BOTH tables at boot is healed by + one reconcile pass — the `secrets` shadow row is deleted, the collision is + logged, and the name is absent from FetchSecrets afterward (red if the + reconcile skips a name that already has a `server_secrets` row). ### T1 — Envelope-crypto helper package @@ -738,7 +768,12 @@ is declared into it) and T1. - **Read-back verify, every boot:** after provisioning AND on every subsequent boot, re-resolve the name and byte-compare against the key the process is about to encrypt with; on mismatch, refuse to serve - gateway-credential writes (fail closed). This is necessary because the + gateway-credential writes (fail closed). This re-resolve is the SAME + uncancellable `Load` (resolver.go:165) and uses the SAME bounded-offload + path as the provisioning resolve (timeout ctx + own goroutine + buffered + cap-1 channel), so on a steady-state boot — key already provisioned, + nothing to serialize — a hung provider still yields a bounded, diagnosable + startup error rather than a parked process. This is necessary because the provider keyspace is a shared mutable surface: C1 pins BOTH resolver instances to the same SecretSpec project + profile (`manifestProject = "compass"`, `defaultProfile = "default"`, resolver.go:19/23), so the @@ -758,7 +793,7 @@ is declared into it) and T1. secrets are structurally undeliverable to CONTAINERS (the manifest separation, §Mechanism C1) — it does NOT make the master key's provider value unreachable from the user path. The user `SetSecret`/`DeleteSecret` - RPC (`authenticatedOpen`, any authenticated account — admin_gate.go:119) + RPC (`authenticatedOpen`, any authenticated account — admin_gate.go:122-125) declares into `secrets` but then calls `resolver.Set`, which shells `secretspec set --profile default` (resolver.go:258-267) against the SHARED keyspace — so a user calling `SetSecret` with name @@ -892,7 +927,8 @@ RPC exactly as the frozen record already specifies. - [ ] T0 — `server_secrets` store (mechanism C1): new migration (table + single-row `server_key_state` table for the key-swap tripwire digest + - GRANT SELECT/INSERT/UPDATE/DELETE to compass_app/compass_system + + GRANT SELECT/INSERT/UPDATE/DELETE on `server_secrets` and + SELECT/INSERT/UPDATE on `server_key_state` to compass_app/compass_system + bucketA allow-list edit in rls_pgtest_test.go), `ServerDeclaredSecrets` store view + second SpecResolver instance (same profile), admin-gated SetServerSecret/DeleteServerSecret RPC AND the user-path reserved-name From 248c7d4dd481629da97e482b61bc044ed4736497 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 22:20:52 -0400 Subject: [PATCH 10/18] =?UTF-8?q?docs(design):=20fold=20#859=20R9=20review?= =?UTF-8?q?=20=E2=80=94=20proto-delta=20honesty=20+=20F1=20Tasks=20members?= =?UTF-8?q?hip=20guard=20(RIG-2863)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-9 review of the gateway_credentials at-rest encryption addendum returned 0H/2M/4L — the first fold in nine rounds with no new clipped-clause defect. All seven R7 folds and both R8 mediums held; both prior HIGHs (3-mode forge taxonomy, Resolve cancellation asymmetry) intact. The two mediums are genuine frozen-contract defects the R8 admin-RPC text surfaced, both driver-foldable off the frozen C1 ruling (false-premise correction / body-vs-Tasks consistency), no Matt fork. - **M1 — "NO public proto change" is false as stated.** The record asserts it four times as a load-bearing C1 property, but the `SetServerSecret`/`DeleteServerSecret` admin RPC C1 mandates (T0) IS an additive public-proto change: `SecretsService` lives in `proto/compass/v1/compass.proto` (the public `buf.gen.yaml` codegen lane, generating the checked-in `packages/compass-client/src/gen` + `go/gen` trees), and adding the RPCs is CI-enforced — `proto/moon.yml`'s `drift` re-gens and diffs the public trees, and `go/internal/auth/classify_exhaustive_test.go` reds CI on any generated procedure not classified. Narrowed the claim to its honest bound at all five sites (:21, :249/:254, :311/:316, :947→Tasks-T0, :995): C1 needs no public proto ENUM change and leaves FetchSecrets byte-for-byte unchanged, but adds two additive `SecretsService` methods. Added the CI consequence to Tasks-T0 (`moon run compass-proto:gen` + classify both procedures or `classify_exhaustive_test` reds). Mirrored into the DL-325 ledger row (edit-before-freeze). The C1 RULING is unchanged — it genuinely removes the SERVER_ONLY delivery-kind ENUM addition, the real and correct claim. - **M2 — Tasks under-specified the user-path guard as reserved-name-only.** Tasks T0 and T2 named only the reserved-name half, contradicting the body's central F1 finding (:526-531) that a reserved-name check is INSUFFICIENT because the six configured server-secret names are arbitrary operator config (`go/cmd/compass-server/main.go:414-450`; the Linear defaults `LINEAR_FORGE_CLIENT_ID`/ `LINEAR_FORGE_CLIENT_SECRET`, serve.go:202-203) carrying no `GATEWAY_CREDENTIALS_` prefix — so the guard must be a MEMBERSHIP test. A reserved-name-only executor would ship the exact `authenticatedOpen` shadow-row hole D6 exists to close. Made both Tasks lines name both halves (membership + reserved), matching the body and the Tests bullet (already correct). - **L1** — named the dangling "the trio" antecedent inline at :232 (the declare/set/rollback trio, introduced 475 lines later). - **L2** — stated the reconcile's lock discipline explicitly (:552): same `pg_advisory_xact_lock` constant key as T2, its OWN transaction taken/released before the T2 provisioning transaction, ordering constraint only that it precede `buildForgeReadWiring`. - **L3** — re-flowed the :545-556 reconcile paragraph to the surrounding width (removed the ragged mid-clause line the R8 full-scan fold left). - **L4** — noted that `WithSystemRole`'s own doc comment (tenant_tx.go:41-47) already names the forge-notification-ack arm, so the DL-315 staleness is in the ledger prose not the code's documentation; mirrored into the DL-326 row. Both mediums grounded against tree eb5ef7a1 (proto public-lane + classify_exhaustive_test + six-name provenance verified). markdownlint 0, design-ledger-gate OK (293 rows, 123 headers); both DL-325/DL-326 record anchors resolve (D1/T0 headings untouched). Ledger-impact: DL-325 proto clause narrowed to ENUM-only + admin-RPC method delta; DL-326 forge-arm clause notes the doc-comment already names the arm (both edit-before-freeze). Spec-impact: none (design record only). Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 4 +- ...-gateway-credentials-at-rest-encryption.md | 64 +++++++++++++------ 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index 8ebc56ba..9c46305c 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,8 +112,8 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | -| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so TWO guards are MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path: the reserved-name guard on `GATEWAY_CREDENTIALS_*`, and — because the other six names are arbitrary per-deployment config with no reserved prefix — a SYMMETRIC F1 membership guard rejecting any name present in the other table (in both declaration orders). The table split alone is necessary but not sufficient for the structural claim. No proto change, no FetchSecrets filter (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | -| DL-326 | The DL-315 `BYPASSRLS` system-role allow-list is EXTENDED to admit the boot server-secret reconcile — a cross-tenant, request-path-free boot step that moves the configured server-secret rows from `secrets` (FORCE RLS, tenant-scoped) into `server_secrets`, run under `store.WithSystemRole`; the entrypoint enumeration is restated against the SHIPPED set, which already includes the forge-notification-ack arm (`go/internal/runnerhub/hub.go`, `forgeNotificationAck`, currently :814) that DL-315's four names omit. DL-315's load-bearing NEVER-on-the-request-path clause is unchanged — every request-path query stays fail-closed under RLS. Refines DL-315 (which stays Active) | Active (Matt, 2026-09-04) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#t0--the-server_secrets-store-table-resolver-split-admin-rpc-prerequisite) | +| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so TWO guards are MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path: the reserved-name guard on `GATEWAY_CREDENTIALS_*`, and — because the other six names are arbitrary per-deployment config with no reserved prefix — a SYMMETRIC F1 membership guard rejecting any name present in the other table (in both declaration orders). The table split alone is necessary but not sufficient for the structural claim. No proto ENUM change and no FetchSecrets filter — the admin RPC adds two additive SecretsService methods (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | +| DL-326 | The DL-315 `BYPASSRLS` system-role allow-list is EXTENDED to admit the boot server-secret reconcile — a cross-tenant, request-path-free boot step that moves the configured server-secret rows from `secrets` (FORCE RLS, tenant-scoped) into `server_secrets`, run under `store.WithSystemRole`; the entrypoint enumeration is restated against the SHIPPED set, which already includes the forge-notification-ack arm (`go/internal/runnerhub/hub.go`, `forgeNotificationAck`, currently :814) that DL-315's four names omit — though `WithSystemRole`'s own doc comment already names it (tenant_tx.go:41-47), so the staleness is in DL-315's ledger prose, not the code's documentation. DL-315's load-bearing NEVER-on-the-request-path clause is unchanged — every request-path query stays fail-closed under RLS. Refines DL-315 (which stays Active) | Active (Matt, 2026-09-04) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#t0--the-server_secrets-store-table-resolver-split-admin-rpc-prerequisite) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext > resume = 12` envelope field is NOT silently reintroduced — the collapsed diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index a5756125..996f66d2 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -18,9 +18,13 @@ a second resolver instance — so the master key and the existing PEM/webhook/Linear server secrets are structurally undeliverable to agent containers (the table split PLUS the F1 membership guard on the user declare/set path — the split alone is necessary but not sufficient; see D6). -C1 needs NO public proto change: it REMOVES the +C1 needs no public proto ENUM change: it REMOVES the `SECRET_DELIVERY_SERVER_ONLY` proto/store/secrets enum additions (and the -FetchSecrets filter) the earlier delivery-kind mechanism required. +FetchSecrets filter) the earlier delivery-kind mechanism required, and leaves +the container FetchSecrets path byte-for-byte unchanged. It DOES add two +admin-gated methods to `SecretsService` (`proto/compass/v1/compass.proto`, +the public codegen lane) — an additive, non-breaking service-method addition, +not a change to an existing wire enum. ## Problem / Intent @@ -229,8 +233,9 @@ path, and reject a `secrets` name on the admin server-secret path) and the boot reconcile is self-healing (below). The table split alone is necessary but not sufficient. Together the symmetric guard and the reconcile close every SEQUENTIAL declaration order. Each guard is a membership SELECT against the -other table followed by an INSERT into this one, and the trio "is not atomic and -assumes no concurrent same-name writer" (secrets_service.go:88-91) — the same +other table followed by an INSERT into this one, and the underlying +declare/set/rollback trio it rides on "is not atomic and assumes no concurrent +same-name writer" (secrets_service.go:88-91) — the same single-writer MVP assumption the existing `SetSecret` path already carries — so a concurrent same-name `SetSecret`/`SetServerSecret` pair remains the one residual both-tables window; the boot reconcile heals it on the next boot (see @@ -246,7 +251,7 @@ resolver.go:78, but C1 does not use it) — so the provider keyspace is shared and moving an existing secret between the two tables is a DB-row move with the provider value untouched (load-bearing for OQ-4). -**C1 REMOVES the public proto change** the previous delivery-kind mechanism +**C1 REMOVES the public proto ENUM change** the previous delivery-kind mechanism required: no `SECRET_DELIVERY_SERVER_ONLY` proto enum value, no `SecretDeliveryServerOnly` store enum, no secrets-package `DeliveryServerOnly`, no widened migration CHECK, and no FetchSecrets @@ -308,7 +313,8 @@ rejected branches are the ones a future reader will reach for first. default-OPEN minus a subtractive filter: server and user secrets share one table and one resolver manifest, and one edit — a dropped filter clause, a mis-mapped enum arm — re-exposes the master key to every container. It - also costs a public proto change C1 does not need. + also costs a public proto ENUM widening — a change to an existing wire + contract — where C1's proto delta is two additive `SecretsService` methods. - **Mechanism C2: separate table + a separate SecretSpec PROFILE — considered, DEFERRED.** The same `server_secrets` table, but the server resolver pinned to its own profile (`WithProfile`, resolver.go:78) so even @@ -548,8 +554,11 @@ declared into a store that does not exist. `secrets` row (an operator-declared server secret raced into both tables, which carries no reserved prefix and is not in `cfg.Forge.resolved()`) and deletes that shadow row too, so every both-tables state self-heals - regardless of provenance. Idempotent, - serialized under the T2 advisory lock, no provider write (the shared + regardless of provenance. Idempotent, serialized under the SAME + `pg_advisory_xact_lock` constant key T2 uses — in its OWN transaction, + taken and released before the T2 provisioning transaction, so the two never + overlap; the reconcile's only ordering constraint is that it precede + `buildForgeReadWiring` (serve.go:567). No provider write (the shared profile makes it a pure row move). The name source is the RESOLVED config `cfg.Forge.resolved()` (serve.go:232-246) — the SAME accessor every live consumer uses (`buildLinearTokenSource` :1700, `buildBoardWebhookWiring` @@ -597,9 +606,10 @@ declared into a store that does not exist. the sweep/resync/recovery arms since the loop threads one ctx and never re-roots it, :315), `go/internal/runnerhub/hub.go` (deliver-ack advance, :753), and `go/internal/runnerhub/hub.go` (`forgeNotificationAck`, :814 — - an arm DL-315's four names do NOT list, a pre-existing, unledgered - extension of its allow-list). Symbol names anchor these; the line numbers - are a convenience that drifts. DL-315's + an arm DL-315's four names do NOT list — though `WithSystemRole`'s own doc + comment already names it (tenant_tx.go:41-47), so the staleness is in + DL-315's ledger prose, not in the code's documentation). Symbol names + anchor these; the line numbers are a convenience that drifts. DL-315's load-bearing "granted ONLY to those named background workers and NEVER on the request path" clause (DECISIONS.md:113) is UNCHANGED — this is a boot step, never the request path — but its named entrypoint set is widened, @@ -931,9 +941,14 @@ RPC exactly as the frozen record already specifies. SELECT/INSERT/UPDATE on `server_key_state` to compass_app/compass_system + bucketA allow-list edit in rls_pgtest_test.go), `ServerDeclaredSecrets` store view + second SpecResolver instance (same profile), admin-gated - SetServerSecret/DeleteServerSecret RPC AND the user-path reserved-name - guard on `secretsService.SetSecret`/`DeleteSecret` (reject before - `resolver.Set`/`Delete`), and a BOOT-TIME config-driven reconcile (NOT + SetServerSecret/DeleteServerSecret RPC AND the SYMMETRIC user-path F1 + guard on `secretsService.SetSecret`/`DeleteSecret` and + `store.DeclareSecret` — a MEMBERSHIP test rejecting any name present in + `server_secrets` (the six configured names carry no reserved prefix, so + a name/prefix match is insufficient) PLUS the reserved + `GATEWAY_CREDENTIALS_MASTER_KEY` name and `GATEWAY_CREDENTIALS_` prefix, + both checked before `resolver.Set`/`Delete`, and a BOOT-TIME + config-driven reconcile (NOT in-migration DML) moving all SIX configured server-secret names (primary + reviewer App PEM, webhook, three Linear) from `secrets` to `server_secrets`, run under `store.WithSystemRole` (BYPASSRLS — the @@ -943,15 +958,21 @@ RPC exactly as the frozen record already specifies. admin-path-mirror-guard (a `SetServerSecret` on a name already in `secrets` is rejected) + reconcile-heals-both-tables-collision (one pass deletes the `secrets` shadow row and the name leaves FetchSecrets) + - non-bootstrap-tenant-reconcile red-green tests. PREREQUISITE of T2. NO - proto change. + non-bootstrap-tenant-reconcile red-green tests. PREREQUISITE of T2. + Proto delta: two additive `SecretsService` methods, no enum change — + the checked-in public gen trees are drift-gated, so this needs the + `compass.proto` edit + `moon run compass-proto:gen`, and both new + procedure paths MUST be added to `classifyProcedure` as `adminOnly` or + `classify_exhaustive_test` reds CI + (`go/internal/auth/classify_exhaustive_test.go`). - [ ] T1 — `go/internal/secrets/envelope`: Key/NewKey/KeyFromBytes/Seal/Open (AAD-carrying) + redaction + unit tests - [ ] T2 — boot resolve-or-provision seam via the server-secret resolver (advisory-lock serialized, read-back verify + key-fingerprint tripwire, - nil-resolver gating, fail-closed boot; the reserved-name guard is T0's, - on BOTH the admin SetServerSecret/DeleteServerSecret RPC AND the - authenticatedOpen user SetSecret/DeleteSecret path). + nil-resolver gating, fail-closed boot; the F1 guards (membership + + reserved-name) are T0's, on BOTH the admin + SetServerSecret/DeleteServerSecret RPC AND the authenticatedOpen user + SetSecret/DeleteSecret path). DEPENDS ON T0 + T1. - [ ] T3 — `gateway_credentials` migration columns: `value_ciphertext`, `value_nonce`, `key_version`; no plaintext column ever. DEPENDS ON @@ -992,8 +1013,9 @@ RPC exactly as the frozen record already specifies. its name is not in the container resolver's manifest at all. An earlier fold resolved this with a minted SERVER_ONLY delivery kind + FetchSecrets filter; Matt's C1 ruling replaced that with the separate store (see - Alternatives, mechanism A) and REMOVED the public proto change it - required. + Alternatives, mechanism A) and REMOVED the public proto ENUM change it + required (the C1 admin RPC is an additive service-method delta, not a wire + enum change). - **OQ-3 — RESOLVED (Matt: yes, D7): api_key rows are encrypted identically to OAuth rows.** One seal/open code path for both payload shapes; both are secrets of the same sensitivity (a stored api_key is as long-lived and as From 345818bcbd306a2fb5c2947d8030dc749d36fe43 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 4 Sep 2026 23:02:09 -0400 Subject: [PATCH 11/18] =?UTF-8?q?docs(design):=20fold=20#859=20R10=20revie?= =?UTF-8?q?w=20=E2=80=94=20schedule=20T0's=20own=20doc-comment=20repairs?= =?UTF-8?q?=20+=20F1=20symmetry=20(RIG-2863)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 review of the `gateway_credentials` at-rest encryption addendum returned 0 high / 1 medium / 4 low; all five are driver-foldable off the frozen C1 mechanism (no design fork). The single medium is a scheduling gap this record's own R9 fold created but did not schedule. ### M1 — T0 must schedule the doc-comment repairs its own changes force (medium) T0 adds a fifth `store.WithSystemRole` site (the boot reconcile) and two `adminOnly` `SecretsService` methods. Both falsify shipped code comments the moment T0 lands: `tenant_tx.go`'s "applied ONLY at the four named background-loop entrypoints" ONLY-claim (:36-38, :43-45), and five "three/3 procedures authenticatedOpen" SecretsService comments (`admin_gate.go:116-121`, `network_door.go:288-290`, `secrets_service.go:6`, `serve.go:555`, `serve.go:749-750` — all verified byte-exact at `eb5ef7a1`). This is exactly the failure DL-326 was minted to prevent, relocated from the ledger into the code. Added an explicit doc-repair deliverable to Tasks T0 and softened the body L4 clause (:620-625) to note T0 itself widens the code doc's ONLY-claim, so both surfaces are corrected in this PR chain. ### F1 symmetry + store-door parity (lows) - **Tasks T0** re-attaches SYMMETRIC to the guard *pair* (not the user-path half alone, which could license skipping the admin half) and names the admin MIRROR membership guard as an interface deliverable, at both the RPC and the `store.DeclareServerSecret` store door. - **Store bullet** (:443-452): `DeclareServerSecret` now carries the mirror of the `store.DeclareSecret` membership guard, so a `server_secrets` shadow row can never be created regardless of which writer reaches it — F1 held at BOTH doors, not only at the RPC. - **Reconcile lock discipline** (:564-569): the boot reconcile now inherits T2's bounded acquisition (`pg_try_advisory_xact_lock` in a bounded retry loop, or a session `lock_timeout`) so a contended boot fails closed with a diagnosable error rather than parking — it is the first toucher of that shared key at boot. - **Delivery enum** (:259-261): narrowed the literally-false "two-valued in all four representations" (the proto rep quotes three enum values) to "gains no new value in any of the four representations — two live delivery kinds plus the proto3 `UNSPECIFIED` zero sentinel". Both prior HIGHs (3-mode forge taxonomy silent-degradation; Resolve cancellation asymmetry) and all R7/R8/R9 folds re-confirmed intact; exhaustive citation audit found zero drifted cites. Record 68683 → 70607 B (>50KB; both DL-325/DL-326 anchors resolve). markdownlint 0, design-ledger-gate OK (293 rows, 123 headers). Ledger-impact: none (DL-325/DL-326 rows unchanged this round). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...-gateway-credentials-at-rest-encryption.md | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index 996f66d2..04c7c688 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -256,7 +256,9 @@ required: no `SECRET_DELIVERY_SERVER_ONLY` proto enum value, no `SecretDeliveryServerOnly` store enum, no secrets-package `DeliveryServerOnly`, no widened migration CHECK, and no FetchSecrets delivery filter. The container FetchSecrets path is byte-for-byte UNCHANGED. -The delivery enum stays two-valued in all four representations: +The delivery enum gains no new value in any of the four representations — +two live delivery kinds throughout (plus the proto3 `UNSPECIFIED` zero +sentinel, which is not a delivery kind): `delivery SMALLINT NOT NULL CHECK (delivery IN (0, 1))` (0001_init.sql:399); `SecretDeliveryFile SecretDelivery = 0` / `SecretDeliveryEnv SecretDelivery = 1` (`go/internal/store/secrets.go:18-25`); `DeliveryFile` / `DeliveryEnv` @@ -442,7 +444,12 @@ declared into a store that does not exist. `DeleteServerSecretDeclaration(ctx, actor, name)` mirroring `DeclareSecret` / `DeleteSecretDeclaration` (store/secrets.go:82, :160) minus delivery/kind/provider/host (actor nullable-empty for the - server-provisioned path), plus the read `ServerDeclaredSecrets(ctx)` — + server-provisioned path). `DeclareServerSecret` carries the MIRROR of the + `store.DeclareSecret` membership guard — it rejects any name present in + `secrets` at the store door, so a `server_secrets` shadow row can never be + created regardless of which writer (the admin RPC, or any future second + writer that bypasses it) reaches it, keeping F1 held at BOTH doors, not + only at the RPC. It ALSO adds the read `ServerDeclaredSecrets(ctx)` — a thin store view whose `DeclaredSecrets(ctx context.Context) ([]store.SecretDeclaration, error)` method (the `declarations` interface shape, resolver.go:29-31) reads `server_secrets`, mapping @@ -555,7 +562,11 @@ declared into a store that does not exist. which carries no reserved prefix and is not in `cfg.Forge.resolved()`) and deletes that shadow row too, so every both-tables state self-heals regardless of provenance. Idempotent, serialized under the SAME - `pg_advisory_xact_lock` constant key T2 uses — in its OWN transaction, + `pg_advisory_xact_lock` constant key T2 uses, acquired with the SAME + bounded discipline (`pg_try_advisory_xact_lock` in a bounded retry loop, + or a session `lock_timeout`) so a contended boot fails closed with a + diagnosable error naming the contended lock rather than parking — in its + OWN transaction, taken and released before the T2 provisioning transaction, so the two never overlap; the reconcile's only ordering constraint is that it precede `buildForgeReadWiring` (serve.go:567). No provider write (the shared @@ -607,8 +618,11 @@ declared into a store that does not exist. re-roots it, :315), `go/internal/runnerhub/hub.go` (deliver-ack advance, :753), and `go/internal/runnerhub/hub.go` (`forgeNotificationAck`, :814 — an arm DL-315's four names do NOT list — though `WithSystemRole`'s own doc - comment already names it (tenant_tx.go:41-47), so the staleness is in - DL-315's ledger prose, not in the code's documentation). Symbol names + comment already names it (tenant_tx.go:41-47), so THIS staleness is in + DL-315's ledger prose, not in the code's documentation — but note T0 itself + then widens that same code doc's ONLY-claim (tenant_tx.go:36-38, :43-45) + to admit the boot reconcile as a fifth site, so both surfaces are corrected + in this PR chain, scheduled as a T0 deliverable above). Symbol names anchor these; the line numbers are a convenience that drifts. DL-315's load-bearing "granted ONLY to those named background workers and NEVER on the request path" clause (DECISIONS.md:113) is UNCHANGED — this is a boot @@ -941,19 +955,31 @@ RPC exactly as the frozen record already specifies. SELECT/INSERT/UPDATE on `server_key_state` to compass_app/compass_system + bucketA allow-list edit in rls_pgtest_test.go), `ServerDeclaredSecrets` store view + second SpecResolver instance (same profile), admin-gated - SetServerSecret/DeleteServerSecret RPC AND the SYMMETRIC user-path F1 - guard on `secretsService.SetSecret`/`DeleteSecret` and - `store.DeclareSecret` — a MEMBERSHIP test rejecting any name present in - `server_secrets` (the six configured names carry no reserved prefix, so - a name/prefix match is insufficient) PLUS the reserved - `GATEWAY_CREDENTIALS_MASTER_KEY` name and `GATEWAY_CREDENTIALS_` prefix, - both checked before `resolver.Set`/`Delete`, and a BOOT-TIME + SetServerSecret/DeleteServerSecret RPC carrying the MIRROR membership + guard (reject any name already present in `secrets`, checked before the + `server_secrets` insert and before `resolver.Set`, AND at the + `store.DeclareServerSecret` store door so a `server_secrets` shadow row + can never be created regardless of which writer reaches it) AND the + matching user-path half on `secretsService.SetSecret`/`DeleteSecret` and + `store.DeclareSecret` — together the SYMMETRIC F1 MEMBERSHIP test, + each direction rejecting any name present in the OTHER table (the six + configured names carry no reserved prefix, so a name/prefix match is + insufficient) PLUS the reserved `GATEWAY_CREDENTIALS_MASTER_KEY` name and + `GATEWAY_CREDENTIALS_` prefix, both checked before `resolver.Set`/`Delete`, and a BOOT-TIME config-driven reconcile (NOT in-migration DML) moving all SIX configured server-secret names (primary + reviewer App PEM, webhook, three Linear) from `secrets` to `server_secrets`, run under `store.WithSystemRole` (BYPASSRLS — the compass_app-scoped path is confined to the bootstrap tenant, so a row - declared under another tenant is otherwise silently skipped); + declared under another tenant is otherwise silently skipped) — PLUS the + doc-comment repairs T0's own changes require: `store.WithSystemRole`'s + ONLY-claim (tenant_tx.go:36-38, :43-45) widens from the four + background-loop entrypoints to ALSO cover this boot server-secret + reconcile (a FIFTH, non-loop site; per DL-326), and the five + "three/3 procedures authenticatedOpen" SecretsService comments + (admin_gate.go:116-121, network_door.go:288-290, secrets_service.go:6, + serve.go:555, serve.go:749-750) are restated as three authenticatedOpen + + two adminOnly once the two admin methods land; absent-from-FetchSecrets + server-resolver-resolves + user-path-guard + admin-path-mirror-guard (a `SetServerSecret` on a name already in `secrets` is rejected) + reconcile-heals-both-tables-collision (one pass From 0cd2dd1984ae868ebb3b3fc7ac56a6daf13fa691 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 00:57:09 -0400 Subject: [PATCH 12/18] =?UTF-8?q?docs(design):=20fold=20#859=20R11=20revie?= =?UTF-8?q?w=20=E2=80=94=20fix=20reconcile/store-door=20deadlock=20+=20com?= =?UTF-8?q?plete=20T0=20doc-repairs=20(RIG-2863)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R11 review returned 0H/1M/4L. The high is a self-inflicted regression the R10 F4 fold introduced; all five findings are driver-foldable off the frozen C1 mechanism (no Matt fork — pure consistency repair of an already-frozen mechanism, completing a deliverable, and reflow). ### H1 — reconcile/store-door deadlock (R10 F4 regression) The R10 fold made the `store.DeclareServerSecret` mirror guard **unconditional** ("shadow row can never be created regardless of which writer reaches it"), which collided head-on with T0's own boot reconcile — a writer whose defining precondition (a name still live in `secrets`) is exactly what the guard rejects. As frozen, T0 could not execute its own OQ-4 deliverable: the reconcile would fail closed on all six configured names on every existing deployment, leaving the inject-all exposure D6 exists to close open. Fix (driver-foldable ordering repair, not the guard-free-reconcile fork): pin the reconcile's intra-transaction order to **DELETE the `secrets` row FIRST, then INSERT into `server_secrets`** within its single `WithSystemRole` transaction, so the store-door membership SELECT sees the delete already applied. Narrowed the absolute "regardless of which writer" claim at both the body store-door bullet and the Tasks T0 restatement to "no writer can create a shadow row while the name is live in `secrets`", explicitly noting the reconcile satisfies the guard by deleting first. Added a `reconcile-orders-delete-before-insert` red-green test to the T0 list (an insert-before-delete reconcile reds against the guard). ### M1 — T0 doc-repair deliverable was under-enumerated The R10 fold scheduled five Go comment repairs but omitted two more doc comments T0's own two `adminOnly` `SecretsService` methods falsify: - `proto/compass/v1/compass.proto:175-179` (the `SecretsService` service comment) — the worst of the set: it is the public wire contract and propagates verbatim into the drift-gated `packages/compass-client/src/gen` + `packages/compass-agent/src/gen` trees, and `moon run compass-proto:gen` regenerates the same stale text, so the drift gate does NOT catch it. - `go/internal/auth/admin_gate.go:22-26` (the `adminOnly` type doc) — T0 adds the first non-CompassService members of this class. Both added to the "doc-comment repairs T0's own changes require" clause, plus an opportunistic note on the pre-existing (not T0-caused) staleness at `admin_gate.go:40-41` / `classify_exhaustive_test.go:45-46`. ### Lows - L1/L2 — reflowed two unwrapped-splice artifacts (the Tasks guard splice, the F2 lock-discipline rejoin) to the record's ~80-col convention. - L3 — folded into M1's opportunistic note. Record-only interdiff (+42/-15). No DECISIONS.md change (DL-325/DL-326 rows unchanged from R9). markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers); record 72488 B (>50KB — both DL-325/DL-326 anchors resolve, D1/T0 headings untouched). Ledger-impact: none (no DECISIONS.md edit this round). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...-gateway-credentials-at-rest-encryption.md | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index 04c7c688..c6268db1 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -445,11 +445,14 @@ declared into a store that does not exist. `DeclareSecret` / `DeleteSecretDeclaration` (store/secrets.go:82, :160) minus delivery/kind/provider/host (actor nullable-empty for the server-provisioned path). `DeclareServerSecret` carries the MIRROR of the - `store.DeclareSecret` membership guard — it rejects any name present in - `secrets` at the store door, so a `server_secrets` shadow row can never be - created regardless of which writer (the admin RPC, or any future second - writer that bypasses it) reaches it, keeping F1 held at BOTH doors, not - only at the RPC. It ALSO adds the read `ServerDeclaredSecrets(ctx)` — + `store.DeclareSecret` membership guard — it rejects any name STILL present + in `secrets` at the store door, so no writer (the admin RPC, or any future + second writer that bypasses it) can create a `server_secrets` shadow row + while the name is live in `secrets`; the boot reconcile satisfies the guard + by deleting the source `secrets` row FIRST, inside the same `WithSystemRole` + transaction, before its `DeclareServerSecret` insert — so F1 stays held at + BOTH doors, not only at the RPC. It ALSO adds the read + `ServerDeclaredSecrets(ctx)` — a thin store view whose `DeclaredSecrets(ctx context.Context) ([]store.SecretDeclaration, error)` method (the `declarations` interface shape, resolver.go:29-31) reads `server_secrets`, mapping @@ -551,7 +554,11 @@ declared into a store that does not exist. embedded SQL migration (`//go:embed migrations/*.sql`, store.go:24-25) cannot know. So the move runs at boot in the server wiring (where the config IS available): for each configured server-secret NAME, if a row - exists in `secrets`, move it (deleting the `secrets` row) — INCLUDING when a + exists in `secrets`, move it — within the reconcile's single + `WithSystemRole` transaction, DELETE the `secrets` row FIRST, then INSERT + the `server_secrets` row via `DeclareServerSecret`, so the store-door + membership guard sees the delete already applied and admits the insert + (atomic: no crash window strands the declaration) — INCLUDING when a `server_secrets` row for that name already exists, in which case the pass deletes the `secrets` shadow row and logs the reconciled collision. Leaving a both-tables name in place would strand it on the inject-all path forever, @@ -566,8 +573,8 @@ declared into a store that does not exist. bounded discipline (`pg_try_advisory_xact_lock` in a bounded retry loop, or a session `lock_timeout`) so a contended boot fails closed with a diagnosable error naming the contended lock rather than parking — in its - OWN transaction, - taken and released before the T2 provisioning transaction, so the two never + OWN transaction, taken and released before the T2 provisioning transaction, + so the two never overlap; the reconcile's only ordering constraint is that it precede `buildForgeReadWiring` (serve.go:567). No provider write (the shared profile makes it a pure row move). The name source is the RESOLVED config @@ -958,15 +965,17 @@ RPC exactly as the frozen record already specifies. SetServerSecret/DeleteServerSecret RPC carrying the MIRROR membership guard (reject any name already present in `secrets`, checked before the `server_secrets` insert and before `resolver.Set`, AND at the - `store.DeclareServerSecret` store door so a `server_secrets` shadow row - can never be created regardless of which writer reaches it) AND the + `store.DeclareServerSecret` store door so no writer can create a + `server_secrets` shadow row while the name is live in `secrets` — the boot + reconcile satisfies it by deleting the source `secrets` row first inside + its `WithSystemRole` transaction) AND the matching user-path half on `secretsService.SetSecret`/`DeleteSecret` and `store.DeclareSecret` — together the SYMMETRIC F1 MEMBERSHIP test, each direction rejecting any name present in the OTHER table (the six configured names carry no reserved prefix, so a name/prefix match is insufficient) PLUS the reserved `GATEWAY_CREDENTIALS_MASTER_KEY` name and - `GATEWAY_CREDENTIALS_` prefix, both checked before `resolver.Set`/`Delete`, and a BOOT-TIME - config-driven reconcile (NOT + `GATEWAY_CREDENTIALS_` prefix, both checked before `resolver.Set`/`Delete`, + and a BOOT-TIME config-driven reconcile (NOT in-migration DML) moving all SIX configured server-secret names (primary + reviewer App PEM, webhook, three Linear) from `secrets` to `server_secrets`, run under `store.WithSystemRole` (BYPASSRLS — the @@ -975,16 +984,34 @@ RPC exactly as the frozen record already specifies. doc-comment repairs T0's own changes require: `store.WithSystemRole`'s ONLY-claim (tenant_tx.go:36-38, :43-45) widens from the four background-loop entrypoints to ALSO cover this boot server-secret - reconcile (a FIFTH, non-loop site; per DL-326), and the five + reconcile (a FIFTH, non-loop site; per DL-326); the five "three/3 procedures authenticatedOpen" SecretsService comments (admin_gate.go:116-121, network_door.go:288-290, secrets_service.go:6, serve.go:555, serve.go:749-750) are restated as three authenticatedOpen - + two adminOnly once the two admin methods land; + + two adminOnly once the two admin methods land; the `adminOnly` type + doc (admin_gate.go:22-26) widens its "privileged CompassService + agent-session RPCs (and token issuance)" enumeration to admit the two + `SecretsService` server-secret writes; and the `SecretsService` service + comment (proto/compass/v1/compass.proto:175-179) is restated to cover the + two admin-gated server-secret methods and to note their authz is + door-side (`adminOnly` in `classifyProcedure`), not handler-side — this + one propagates through `moon run compass-proto:gen` into the checked-in + public gen trees (packages/compass-client/src/gen, + packages/compass-agent/src/gen), which the drift gate does NOT catch as + staleness on its own. + (Opportunistic in the same edit, flagged pre-existing not T0-caused: + admin_gate.go:40-41's "every generated CompassService and CommsService + procedure" phrasing is already stale — the switch covers SecretsService + too — as is the identical phrasing at classify_exhaustive_test.go:45-46; + correct both in the same pass, but neither gates T0.) Red-green tests: absent-from-FetchSecrets + server-resolver-resolves + user-path-guard + admin-path-mirror-guard (a `SetServerSecret` on a name already in `secrets` is rejected) + reconcile-heals-both-tables-collision (one pass deletes the `secrets` shadow row and the name leaves FetchSecrets) + - non-bootstrap-tenant-reconcile red-green tests. PREREQUISITE of T2. + reconcile-orders-delete-before-insert (a reconcile that INSERTs the + `server_secrets` row before deleting the `secrets` row reds against the + store-door membership guard) + non-bootstrap-tenant-reconcile. + PREREQUISITE of T2. Proto delta: two additive `SecretsService` methods, no enum change — the checked-in public gen trees are drift-gated, so this needs the `compass.proto` edit + `moon run compass-proto:gen`, and both new From 5a682c630e2bedeaa9d76c94e5ebbfb460396ce5 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 01:56:08 -0400 Subject: [PATCH 13/18] =?UTF-8?q?docs(design):=20fold=20#859=20R12=20revie?= =?UTF-8?q?w=20=E2=80=94=20name=20the=20WithTx=20seam=20the=20atomic=20rec?= =?UTF-8?q?oncile=20requires=20+=20correct=20gen-lane=20label=20(RIG-2863)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 12 returned 0H/2M/3L. First regression-free round in twelve: the reviewer confirmed the R11 deadlock fix HOLDS at all three sites and found no new defect in the added text. Both mediums are pre-existing gaps R11's own text made newly load-bearing; folded both plus one low. L2/L3 (cosmetic reflow, no width gate) left as-is. ### M1 — the atomic single-transaction claim was not implementable through the store API the record specifies (driver-foldable false-premise correction) R11's reconcile fix claimed "within the reconcile's single `WithSystemRole` transaction, DELETE the `secrets` row FIRST, then INSERT ... (atomic: no crash window)". But the record named the two operations as plain-signature store methods, and every such method executes through `scopedDBTX` as its OWN implicit transaction — one `pool.SendBatch` per statement (`go/internal/store/tenant_tx.go:80-92`; :64-70 states multi-statement-tx methods use `beginTenantTx` instead). So two plain calls are two transactions, the "atomic" claim is false as written, and a crash between them strands the name in NEITHER table — strictly worse than pre-fold (the heal scans only names live in `secrets`). The store carries exactly one multi-statement-tx seam: `(*Store).WithTx(ctx, fn)` (`coordination.go:368`), which runs `fn` in one `beginTenantTx` and honors the system role via `armTx` (`tenant_tx.go:199-215`), plus the shipped `…Tx(ctx, tx, …)` method convention (`coordination.go:86/168`, `dm.go:49/97`). Naming the seam completes the already-frozen atomicity requirement — not a fork. Reconcile bullet now specifies `store.WithTx(store.WithSystemRole(ctx), …)` with tx-bound `DeleteSecretDeclarationTx`/`DeclareServerSecretTx` variants and states why the plain methods would break atomicity. ### M2 — gen-lane label corrected (the reviewer's go/gen claim did not hold against the tree) The reviewer flagged the `SecretsService` service-comment repair as also propagating into `go/gen` (the connect-go tree). Verified against tree `eb5ef7a1`: the SERVICE comment ("User-facing secret registry writes…") has ZERO matches in either `go/gen` file — `compass.connect.go:803` is generator boilerplate and :805-812 are per-method comments T0 does not touch. So no go/gen target was missed. The real defect the finding also raised IS correct: the record mislabeled `packages/compass-agent/src/gen` as a "public" tree. Per `proto/moon.yml:35-40` it is an internal-only lane (`buf.gen.agent-ts.yaml`); only `packages/compass-client/src/gen` is public (`buf.gen.yaml`). Corrected the label; did not add the unsupported go/gen target. ### L1 — ORDERING assertion added to the T0 Plan Tests bullet The `reconcile-orders-delete-before-insert` assertion lived only in the Tasks test list; added a matching ORDERING assertion beside RECONCILE-HEAL in the Plan Tests bullet (the prescriptive list), including the both-commit-or-both-roll-back property M1 now pins. Record-only this round (DECISIONS.md DL-325/DL-326 unchanged). Record 72488 → 73667 B (>50KB; both anchors resolve). markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers). Interdiff +24/-10, record only. Ledger-impact: none. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...-gateway-credentials-at-rest-encryption.md | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index c6268db1..d7136368 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -554,11 +554,18 @@ declared into a store that does not exist. embedded SQL migration (`//go:embed migrations/*.sql`, store.go:24-25) cannot know. So the move runs at boot in the server wiring (where the config IS available): for each configured server-secret NAME, if a row - exists in `secrets`, move it — within the reconcile's single - `WithSystemRole` transaction, DELETE the `secrets` row FIRST, then INSERT - the `server_secrets` row via `DeclareServerSecret`, so the store-door - membership guard sees the delete already applied and admits the insert - (atomic: no crash window strands the declaration) — INCLUDING when a + exists in `secrets`, move it — inside ONE `store.WithTx(store.WithSystemRole(ctx), + …)` (coordination.go:368; `armTx` arms the BYPASSRLS system role for the whole + tx, tenant_tx.go:199-215), DELETE the `secrets` row FIRST, then INSERT + the `server_secrets` row, so the store-door membership guard sees the delete + already applied and admits the insert. Both statements run through tx-bound + `DeleteSecretDeclarationTx`/`DeclareServerSecretTx(ctx, tx, …)` variants + (the shipped `…Tx` convention, coordination.go:86/168, dm.go:49/97) — NOT the + plain-signature methods, each of which executes through `scopedDBTX` as its + OWN implicit transaction (one `pool.SendBatch` per statement, tenant_tx.go:64-70, + :80-92), which would make the two operations two transactions and break the + atomicity below (atomic: one transaction commits or rolls back both, so no + crash window strands the declaration in neither table) — INCLUDING when a `server_secrets` row for that name already exists, in which case the pass deletes the `secrets` shadow row and logs the reconciled collision. Leaving a both-tables name in place would strand it on the inject-all path forever, @@ -684,7 +691,12 @@ declared into a store that does not exist. RECONCILE-HEAL assertion: a name live in BOTH tables at boot is healed by one reconcile pass — the `secrets` shadow row is deleted, the collision is logged, and the name is absent from FetchSecrets afterward (red if the - reconcile skips a name that already has a `server_secrets` row). + reconcile skips a name that already has a `server_secrets` row). ORDERING + assertion: a reconcile that INSERTs the `server_secrets` row before deleting + the `secrets` row reds against the store-door membership guard, and both + statements commit or roll back together — a crash between them strands the + name in neither table (red if the DELETE and INSERT run as two transactions + rather than one `store.WithTx`). ### T1 — Envelope-crypto helper package @@ -995,10 +1007,12 @@ RPC exactly as the frozen record already specifies. comment (proto/compass/v1/compass.proto:175-179) is restated to cover the two admin-gated server-secret methods and to note their authz is door-side (`adminOnly` in `classifyProcedure`), not handler-side — this - one propagates through `moon run compass-proto:gen` into the checked-in - public gen trees (packages/compass-client/src/gen, - packages/compass-agent/src/gen), which the drift gate does NOT catch as - staleness on its own. + one propagates through `moon run compass-proto:gen` into two checked-in + TS gen trees — `packages/compass-client/src/gen` (the PUBLIC lane, + buf.gen.yaml) and `packages/compass-agent/src/gen` (the internal-only agent + lane, buf.gen.agent-ts.yaml, proto/moon.yml:35-40) — which the drift gate + does NOT catch as staleness on its own (it regenerates and diffs, so stale + comment text regenerates identically and passes). (Opportunistic in the same edit, flagged pre-existing not T0-caused: admin_gate.go:40-41's "every generated CompassService and CommsService procedure" phrasing is already stale — the switch covers SecretsService From 889644e4b0dd22bba6616ddfa16fe6046310d85c Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 02:47:49 -0400 Subject: [PATCH 14/18] =?UTF-8?q?docs(design):=20fold=20#859=20R13=20revie?= =?UTF-8?q?w=20=E2=80=94=20name=20the=20WithTx=20seam=20at=20the=20two=20m?= =?UTF-8?q?irror=20sites=20+=20fix=20ORDERING-assertion=20polarity=20(RIG-?= =?UTF-8?q?2863)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 13 adversarial review of the credentials-at-rest encryption record returned 0 High / 2 Medium / 2 Low — the first regression-free round in which every finding is a driver-foldable consistency repair off the frozen mechanism, no design fork. Both mediums grounded byte-exact against the tree at `eb5ef7a1`; the two lows are one cheap citation-precision fix and one confirmed-cosmetic reflow left as-is. Record-only (DECISIONS.md DL-325/DL-326 unchanged from R9). ### M1 — the two reconcile mirror sites named a construct the R12 fold removed R12 rewrote the row-migration bullet to run the reconcile's DELETE+INSERT inside one `store.WithTx(store.WithSystemRole(ctx), …)` transaction, but left the two mirror sites (the `DeclareServerSecret` store-door bullet and the T0 Tasks restatement) saying the delete happens "inside the `WithSystemRole` transaction". After the fold there is no such construct: `store.WithSystemRole(ctx)` is a context marker (`context.WithValue`, tenant_tx.go:47-49), not a transaction, and passing it to a plain store method is exactly the two-implicit-transactions path the fold forbids. An executor implementing the store door or working the Tasks checklist from either mirror alone would write the two-transaction crash window the record now also requires a test to red on. Both mirror phrases now name the actual single-transaction construct `store.WithTx(store.WithSystemRole(ctx), …)` and the tx-bound `DeclareServerSecretTx` insert. ### M2 — the R13-added ORDERING assertion inverted the atomicity polarity The ORDERING test assertion I added in R12 stated the failure mode as the guarantee: "a crash between them strands the name in neither table". Under one transaction a crash rolls both statements back, so the name stays in `secrets` — it is stranded in neither table only under the two-transaction implementation the record forbids. The record's own corrected reconcile bullet states the correct polarity at the D6 mechanism. Fixed the assertion to "leaves the name in `secrets`, never stranded in neither table", matching the reference; the trailing "red if the DELETE and INSERT run as two transactions rather than one `store.WithTx`" parenthetical now agrees with the clause it qualifies. ### L — off-by-two citation The gen-lane classification cite `proto/moon.yml:35-40` clipped the closing of the agent-ts internal-lane clause; corrected to `:35-41`. The substance (compass-agent/src/gen internal-only via buf.gen.agent-ts.yaml; compass-client/src/gen + go/gen public via buf.gen.yaml) was already accurate. The two reflow orphans (L2/L3) are confirmed grammatically whole with no width gate; left as-is. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...ss-gateway-credentials-at-rest-encryption.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index d7136368..8403b083 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -449,9 +449,10 @@ declared into a store that does not exist. in `secrets` at the store door, so no writer (the admin RPC, or any future second writer that bypasses it) can create a `server_secrets` shadow row while the name is live in `secrets`; the boot reconcile satisfies the guard - by deleting the source `secrets` row FIRST, inside the same `WithSystemRole` - transaction, before its `DeclareServerSecret` insert — so F1 stays held at - BOTH doors, not only at the RPC. It ALSO adds the read + by deleting the source `secrets` row FIRST, inside the ONE + `store.WithTx(store.WithSystemRole(ctx), …)` transaction the Row-migration + bullet specifies, before its `DeclareServerSecretTx` insert — so F1 stays + held at BOTH doors, not only at the RPC. It ALSO adds the read `ServerDeclaredSecrets(ctx)` — a thin store view whose `DeclaredSecrets(ctx context.Context) ([]store.SecretDeclaration, error)` method (the `declarations` @@ -694,9 +695,9 @@ declared into a store that does not exist. reconcile skips a name that already has a `server_secrets` row). ORDERING assertion: a reconcile that INSERTs the `server_secrets` row before deleting the `secrets` row reds against the store-door membership guard, and both - statements commit or roll back together — a crash between them strands the - name in neither table (red if the DELETE and INSERT run as two transactions - rather than one `store.WithTx`). + statements commit or roll back together — a crash between them leaves the name + in `secrets`, never stranded in neither table (red if the DELETE and INSERT run + as two transactions rather than one `store.WithTx`). ### T1 — Envelope-crypto helper package @@ -980,7 +981,7 @@ RPC exactly as the frozen record already specifies. `store.DeclareServerSecret` store door so no writer can create a `server_secrets` shadow row while the name is live in `secrets` — the boot reconcile satisfies it by deleting the source `secrets` row first inside - its `WithSystemRole` transaction) AND the + its single `store.WithTx(store.WithSystemRole(ctx), …)` transaction) AND the matching user-path half on `secretsService.SetSecret`/`DeleteSecret` and `store.DeclareSecret` — together the SYMMETRIC F1 MEMBERSHIP test, each direction rejecting any name present in the OTHER table (the six @@ -1010,7 +1011,7 @@ RPC exactly as the frozen record already specifies. one propagates through `moon run compass-proto:gen` into two checked-in TS gen trees — `packages/compass-client/src/gen` (the PUBLIC lane, buf.gen.yaml) and `packages/compass-agent/src/gen` (the internal-only agent - lane, buf.gen.agent-ts.yaml, proto/moon.yml:35-40) — which the drift gate + lane, buf.gen.agent-ts.yaml, proto/moon.yml:35-41) — which the drift gate does NOT catch as staleness on its own (it regenerates and diffs, so stale comment text regenerates identically and passes). (Opportunistic in the same edit, flagged pre-existing not T0-caused: From bc8bd02f79a46adcb0e53b124bf514d8e34c4d88 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 03:30:07 -0400 Subject: [PATCH 15/18] =?UTF-8?q?docs(design):=20fold=20#859=20R14=20revie?= =?UTF-8?q?w=20=E2=80=94=20extend=20the=20F1=20membership=20guard=20to=20t?= =?UTF-8?q?he=20DeclareServerSecretTx=20variant=20the=20reconcile=20uses?= =?UTF-8?q?=20(RIG-2863)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R14 review returned 0H/1M/2L, all driver-foldable off the frozen record. The medium is a pre-existing gap the R13 M1 fold newly exposed (not a regression): R13 correctly routed the boot reconcile through the tx-bound `DeclareServerSecretTx` variant, but the store-door membership guard was still specified only on the plain `DeclareServerSecret`. Read literally, an executor could build an unguarded `DeclareServerSecretTx` twin — bypassing F1's store door entirely and silently deleting the `reconcile-orders-delete-before-insert` tripwire (that test asserts the guard reds on the wrong-order reconcile; with no guard on the Tx path there is nothing to red). Folds (record-only; completes the already-frozen "F1 holds at BOTH doors" mechanism under the name the record already mandates — no fork, decides nothing): - **M (guard on the Tx variant)** — store-door bullet now states `DeclareServerSecret` AND its tx-bound `DeclareServerSecretTx` variant BOTH carry the mirror guard via one shared unexported check (so they can never drift); the Row-migration bullet adds that each `…Tx` variant carries the same store-door guard as its plain counterpart, so the DELETE-FIRST ordering is what admits the insert; the Tasks T0 deliverable names both variants at the store door. - **L (citation)** — `binding_cgo.go:26` → `:30` (the `C.secretspec_resolve(req)` call site; :26 is the doc-comment start). - **L (reflow orphans)** — confirmed cosmetic (no width gate, MD013 off), left as-is. Mechanical pass clean: guarded single-occurrence replaces; in-memory diff record-only +11/-6; every edited region re-read whole; backticks even, paren net 0, D1/T0 anchor headings byte-identical. markdownlint 0 errors; design-ledger-gate OK (293 rows, 123 headers). Record 73310 → 73666 B (>50KB; both DL-325/DL-326 anchors resolve). No DECISIONS.md change (DL-325/326 unchanged from R9). Ledger-impact: none (record-only; DL-325/DL-326 unchanged). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...ss-gateway-credentials-at-rest-encryption.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index 8403b083..d37d9d47 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -444,9 +444,11 @@ declared into a store that does not exist. `DeleteServerSecretDeclaration(ctx, actor, name)` mirroring `DeclareSecret` / `DeleteSecretDeclaration` (store/secrets.go:82, :160) minus delivery/kind/provider/host (actor nullable-empty for the - server-provisioned path). `DeclareServerSecret` carries the MIRROR of the - `store.DeclareSecret` membership guard — it rejects any name STILL present - in `secrets` at the store door, so no writer (the admin RPC, or any future + server-provisioned path). `DeclareServerSecret` AND its tx-bound + `DeclareServerSecretTx` variant BOTH carry the MIRROR of the + `store.DeclareSecret` membership guard (one shared unexported check both call, + so the two can never drift) — it rejects any name STILL present in `secrets` + at the store door, so no writer (the admin RPC, or any future second writer that bypasses it) can create a `server_secrets` shadow row while the name is live in `secrets`; the boot reconcile satisfies the guard by deleting the source `secrets` row FIRST, inside the ONE @@ -561,7 +563,9 @@ declared into a store that does not exist. the `server_secrets` row, so the store-door membership guard sees the delete already applied and admits the insert. Both statements run through tx-bound `DeleteSecretDeclarationTx`/`DeclareServerSecretTx(ctx, tx, …)` variants - (the shipped `…Tx` convention, coordination.go:86/168, dm.go:49/97) — NOT the + (the shipped `…Tx` convention, coordination.go:86/168, dm.go:49/97), each + carrying the SAME store-door membership guard as its plain counterpart via the + shared check, so the DELETE-FIRST ordering is what admits the insert — NOT the plain-signature methods, each of which executes through `scopedDBTX` as its OWN implicit transaction (one `pool.SendBatch` per statement, tenant_tx.go:64-70, :80-92), which would make the two operations two transactions and break the @@ -773,7 +777,7 @@ is declared into it) and T1. `b.Load()` (resolver.go:165), whose SDK signature carries NO ctx (`func (b *Builder) Load() (*Resolved, error)`, secretspec-go v0.15.0 secretspec.go:245) and which blocks in an uncancellable FFI call - (`nativeResolve` → `C.secretspec_resolve`, binding_cgo.go:26 / + (`nativeResolve` → `C.secretspec_resolve`, binding_cgo.go:30 / binding_purego.go:118). A hung provider (1Password awaiting biometric approval, an unreachable Vault, a half-open TCP) would otherwise hold the transaction-scoped lock indefinitely, and because the key is a shared @@ -978,7 +982,8 @@ RPC exactly as the frozen record already specifies. SetServerSecret/DeleteServerSecret RPC carrying the MIRROR membership guard (reject any name already present in `secrets`, checked before the `server_secrets` insert and before `resolver.Set`, AND at the - `store.DeclareServerSecret` store door so no writer can create a + `store.DeclareServerSecret` / `DeclareServerSecretTx` store door (both + variants, one shared check) so no writer can create a `server_secrets` shadow row while the name is live in `secrets` — the boot reconcile satisfies it by deleting the source `secrets` row first inside its single `store.WithTx(store.WithSystemRole(ctx), …)` transaction) AND the From fd35852008c5f63463cf2597aa94d00740d86d8d Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 04:21:44 -0400 Subject: [PATCH 16/18] =?UTF-8?q?docs(design):=20fold=20#859=20R15=20revie?= =?UTF-8?q?w=20=E2=80=94=20narrow=20the=20reconcile=20guard=20clause=20to?= =?UTF-8?q?=20the=20insert=20side=20+=20name=20DeleteSecretDeclarationTx?= =?UTF-8?q?=20in=20Tasks=20T0=20(RIG-2863)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #859 R15 adversarial review returned `0H/2M/2L`. This commit folds the driver-foldable medium (a self-inflicted regression from the R14 fold) and one low; the second medium is a cross-tenant security fork routed to Matt at the merge gate (see below), not folded. ### M1 (medium, FOLDED) — R14 fold widened the guard onto the DELETE side, breaking the reconcile heal arm The R14 fold wrote the Row-migration guard clause distributively over a two-item list (`DeleteSecretDeclarationTx`/`DeclareServerSecretTx`), so "each carrying the SAME store-door membership guard" extended the guard to the delete variant too. That is wrong two ways: (1) `DeleteSecretDeclaration` carries no store-door membership guard anywhere in the record, so the delete-side claim references a guard that does not exist; (2) if an executor builds one, it rejects exactly the deletes the reconcile heal arm requires (delete a `secrets` row for a name simultaneously live in `server_secrets`, :573-583), making the `reconcile-heals-both-tables-collision` acceptance criterion unsatisfiable against the same paragraph's mandated implementation. Narrowed the clause to the insert side only — `DeclareServerSecretTx` carries the guard mirroring its plain `DeclareServerSecret` counterpart — with an explicit parenthetical that the delete side carries no membership guard (the heal arm depends on being able to delete a shadow row whose name is live in `server_secrets`), so a later fold cannot re-widen it. Driver-foldable: narrows an over-broad clause back to the R14 contract and decides nothing. ### L (low, FOLDED) — DeleteSecretDeclarationTx mandatory in Plan, absent from Tasks checklist The tx-bound `DeleteSecretDeclarationTx` is load-bearing in the Row-migration bullet but never named in the Tasks T0 deliverable list, so an executor working the checklist would not build it (no `…Tx` sibling of `DeleteSecretDeclaration` exists today, store/secrets.go:160). Added it to the T0 reconcile deliverable as a new (both-NEW) tx-bound pair. ### M2 (medium, ROUTED TO MATT — not folded) The shared store-door membership check runs a SELECT against `secrets`, which is FORCE ROW LEVEL SECURITY. The reconcile caller arms BYPASSRLS (`store.WithSystemRole`); the admin-RPC caller (`SetServerSecret`) is tenant-scoped (`compass_app`, no BYPASSRLS). An admin `SetServerSecret` for a name whose `secrets` row was declared under a different tenant runs a membership SELECT that returns zero rows, admits the insert, and creates the both-tables state F1 forbids. The fix is to arm the guard's `secrets` membership SELECT under BYPASSRLS on the admin path too — a cross-tenant BYPASSRLS widening, which this record already declares a Matt-ruled security event (:620-626 documents the identical blind spot for the reconcile). Surfaced to Matt at the merge gate with the reviewer's recommended fix; not auto-folded. ### L (low, left as-is) — reflow orphans Cosmetic, no width gate (`.markdownlint.json` is `MD013: false`). Re-confirmed grammatically whole for a fourth round. Record-only (no DECISIONS.md change; DL-325/326 unchanged). markdownlint 0 issues; design-ledger-gate OK (293 rows, 123 headers). Record 73666 → 74082 B. Ledger-impact: none. Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- ...mpass-gateway-credentials-at-rest-encryption.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index d37d9d47..cfefbcfb 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -563,9 +563,12 @@ declared into a store that does not exist. the `server_secrets` row, so the store-door membership guard sees the delete already applied and admits the insert. Both statements run through tx-bound `DeleteSecretDeclarationTx`/`DeclareServerSecretTx(ctx, tx, …)` variants - (the shipped `…Tx` convention, coordination.go:86/168, dm.go:49/97), each - carrying the SAME store-door membership guard as its plain counterpart via the - shared check, so the DELETE-FIRST ordering is what admits the insert — NOT the + (the shipped `…Tx` convention, coordination.go:86/168, dm.go:49/97), with + `DeclareServerSecretTx` carrying the SAME store-door membership guard as its + plain `DeclareServerSecret` counterpart via the shared check (the delete side + carries no membership guard — the reconcile heal arm below depends on being + able to delete a `secrets` row whose name is already live in `server_secrets`), + so the DELETE-FIRST ordering is what admits the insert — NOT the plain-signature methods, each of which executes through `scopedDBTX` as its OWN implicit transaction (one `pool.SendBatch` per statement, tenant_tx.go:64-70, :80-92), which would make the two operations two transactions and break the @@ -996,7 +999,10 @@ RPC exactly as the frozen record already specifies. and a BOOT-TIME config-driven reconcile (NOT in-migration DML) moving all SIX configured server-secret names (primary + reviewer App PEM, webhook, three Linear) from `secrets` to - `server_secrets`, run under `store.WithSystemRole` (BYPASSRLS — the + `server_secrets` via the tx-bound + `DeleteSecretDeclarationTx`/`DeclareServerSecretTx` pair (both NEW — no + `…Tx` sibling of `DeleteSecretDeclaration` exists today, store/secrets.go:160), + run under `store.WithSystemRole` (BYPASSRLS — the compass_app-scoped path is confined to the bootstrap tenant, so a row declared under another tenant is otherwise silently skipped) — PLUS the doc-comment repairs T0's own changes require: `store.WithSystemRole`'s From 9fda61a318b6f3ce84472572a7dec0ea39b111c8 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 11:20:43 -0400 Subject: [PATCH 17/18] docs(design): F1 as a structural reserved-prefix partition, not a runtime membership guard (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matt-ruled design change to the RIG-2863 at-rest-encryption record, arising from the #859 R15 review's cross-tenant medium (M2): the F1 store-door membership guard ran a SELECT against `secrets` (FORCE RLS) on both the reconcile path (BYPASSRLS) and the admin RPC path (tenant-scoped `compass_app`), so an admin declaring a server secret whose name already had a `secrets` row under a different tenant would see zero rows, admit the insert, and create the both-tables state F1 forbids. Rather than patch the guard with request-path BYPASSRLS (which would breach DL-315's "never on the request path"), Matt ruled F1 into a structural partition. ### The change - **F1 is now a reserved-prefix name partition, enforced by a pure string check on both store doors.** Server-secret names carry a reserved prefix (`SERVER_` for the six forge secrets, `GATEWAY_CREDENTIALS_` for the master-key family). The admin `SetServerSecret`/`DeclareServerSecret` door REQUIRES one; the user `SetSecret`/`DeclareSecret` door REJECTS one. The two keyspaces are therefore disjoint by name — a both-tables state is UNREACHABLE in either declaration order, not healed after the fact. No cross-table membership SELECT, so no cross-tenant RLS-visibility question. **M2 is dissolved, not patched.** - **The boot reconcile is deleted.** With the partition structural, there is no both-tables state to repair: the six configured names are boot-declared under their `SERVER_`-prefixed names via an idempotent config-driven declare on the ordinary compass_app path (`server_secrets` is bucket-A, no RLS, so no BYPASSRLS, no cross-tenant read, no delete-before-insert ordering, no self-heal scan). - **DL-326 is removed from DECISIONS.md** — it existed only to record the reconcile's BYPASSRLS-allow-list widening, which no longer occurs. This record adds no cross-tenant BYPASSRLS site. DL-325's F1 description is updated from "membership guard" to the prefix partition, and its migration clause from "DB-row move" to "wiped + re-declared" (pre-production, per Matt). - **The six existing server secrets are wiped and re-declared** under their prefixed names through the new admin RPC — no in-place move, no provider-value migration (Matt: "just wipe and re-add, we don't need migration atp"). ### Sections rewritten Summary header, D6 (core F1 definition + profile-share note + retroactive-fix note), C2 alternative + named residual, T0 store-door bullet / resolver-split reconcile refs / admin-RPC guard / user-path guard / boot-declare bullet (replacing the reconcile apparatus) / Tests block, D2 master-key overwrite guard, Tasks T0 + T2, OQ-4 + OQ-5 (WITHDRAWN). A pre-existing DL-315 ledger-prose staleness (the shipped `hub.go:814` forge-notification-ack BYPASSRLS arm is absent from DL-315's four-name enumeration) surfaced during the earlier draft and is no longer this record's to fix (it no longer touches the BYPASSRLS surface); left as a separate follow-up. markdownlint 0 issues; design-ledger-gate OK (292 rows, 123 headers). Record 74082 → 68444 B. Record-only + one DECISIONS.md row edit + one row removal. Ledger-impact: DL-325 F1/migration clauses updated; DL-326 removed (its reconcile no longer exists). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 3 +- ...-gateway-credentials-at-rest-encryption.md | 521 ++++++++---------- 2 files changed, 223 insertions(+), 301 deletions(-) diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index 9c46305c..a2a2bf35 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,8 +112,7 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | -| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so TWO guards are MANDATORY on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path: the reserved-name guard on `GATEWAY_CREDENTIALS_*`, and — because the other six names are arbitrary per-deployment config with no reserved prefix — a SYMMETRIC F1 membership guard rejecting any name present in the other table (in both declaration orders). The table split alone is necessary but not sufficient for the structural claim. No proto ENUM change and no FetchSecrets filter — the admin RPC adds two additive SecretsService methods (supersedes the earlier SERVER_ONLY-delivery-kind fold). The server-secret rows move into `server_secrets` via a boot-time config-driven reconcile (the names are per-deployment config, not migration constants; DB-row move, no provider re-provision), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | -| DL-326 | The DL-315 `BYPASSRLS` system-role allow-list is EXTENDED to admit the boot server-secret reconcile — a cross-tenant, request-path-free boot step that moves the configured server-secret rows from `secrets` (FORCE RLS, tenant-scoped) into `server_secrets`, run under `store.WithSystemRole`; the entrypoint enumeration is restated against the SHIPPED set, which already includes the forge-notification-ack arm (`go/internal/runnerhub/hub.go`, `forgeNotificationAck`, currently :814) that DL-315's four names omit — though `WithSystemRole`'s own doc comment already names it (tenant_tx.go:41-47), so the staleness is in DL-315's ledger prose, not the code's documentation. DL-315's load-bearing NEVER-on-the-request-path clause is unchanged — every request-path query stays fail-closed under RLS. Refines DL-315 (which stays Active) | Active (Matt, 2026-09-04) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#t0--the-server_secrets-store-table-resolver-split-admin-rpc-prerequisite) | +| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so F1 is a STRUCTURAL reserved-prefix partition enforced on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path: every server-secret name carries a reserved prefix (`SERVER_` for the six forge secrets, `GATEWAY_CREDENTIALS_` for the master-key family), the admin door REQUIRES one and the user door REJECTS one, so the two keyspaces are disjoint by name and no name is ever live in both tables in either declaration order — a pure string check, no cross-table membership SELECT and so no cross-tenant RLS-visibility question. The table split alone is necessary but not sufficient for the structural claim; the prefix partition is what makes it sufficient. No proto ENUM change and no FetchSecrets filter — the admin RPC adds two additive SecretsService methods (supersedes the earlier SERVER_ONLY-delivery-kind fold). The existing server-secret rows are re-declared into `server_secrets` under their `SERVER_`-prefixed names by a boot-time config-driven declare (idempotent, on the ordinary compass_app path — `server_secrets` is bucket-A, no RLS, so no reconcile and no BYPASSRLS; the pre-production deployment is wiped and re-declared, not migrated in place), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext > resume = 12` envelope field is NOT silently reintroduced — the collapsed diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index cfefbcfb..e599af12 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -16,8 +16,10 @@ One deliberate scope expansion, Matt-ruled (D6/T0): server-only secrets get a physically SEPARATE `server_secrets` store (mechanism C1) — a new table plus a second resolver instance — so the master key and the existing PEM/webhook/Linear server secrets are structurally undeliverable to agent -containers (the table split PLUS the F1 membership guard on the user -declare/set path — the split alone is necessary but not sufficient; see D6). +containers (the table split PLUS the F1 reserved-prefix partition — server-secret +names carry a reserved prefix the user declare/set path rejects, making the two +keyspaces disjoint by construction; the split alone is necessary but not +sufficient; see D6). C1 needs no public proto ENUM change: it REMOVES the `SECRET_DELIVERY_SERVER_ONLY` proto/store/secrets enum additions (and the FetchSecrets filter) the earlier delivery-kind mechanism required, and leaves @@ -221,35 +223,36 @@ a SECOND `SpecResolver` instance. The container resolver keeps reading `secrets`; its manifest never contains a server-secret name, so a server secret is undeliverable to containers via the container manifest — there is nothing to filter. That structural property holds only while no name is -ever present in BOTH tables: because the two tables share a keyspace and each -`name` is a per-table PK with no cross-table exclusion, the same both-tables -end state is reachable in EITHER declaration order — an `authenticatedOpen` -`SetSecret` minting a shadow `secrets` row under a server-secret name, OR an -admin `SetServerSecret` declaring a server secret under a name already live in -`secrets` — and either gets that name container-delivered. The F1 membership -guard is therefore SYMMETRIC (T0 — enforced on every path that can create a -row in either table: reject a `server_secrets` name on the user declare/set -path, and reject a `secrets` name on the admin server-secret path) and the -boot reconcile is self-healing (below). The table split alone is necessary but -not sufficient. Together the symmetric guard and the reconcile close every -SEQUENTIAL declaration order. Each guard is a membership SELECT against the -other table followed by an INSERT into this one, and the underlying -declare/set/rollback trio it rides on "is not atomic and assumes no concurrent -same-name writer" (secrets_service.go:88-91) — the same -single-writer MVP assumption the existing `SetSecret` path already carries — -so a concurrent same-name `SetSecret`/`SetServerSecret` pair remains the one -residual both-tables window; the boot reconcile heals it on the next boot (see -the full-scan note below). Closing that window atomically (a shared per-name -advisory lock across both write paths) is deferred to the multi-writer work the -cited comment already flags, tracked with the per-tenant defer (RIG-3237); this -record does not widen the single-writer contract. +ever present in BOTH tables. F1 makes that structural, by NAME construction: +every server-secret name carries a RESERVED PREFIX (the master-key family keeps +`GATEWAY_CREDENTIALS_`; the six configured forge secrets carry `SERVER_`), and +the two store doors enforce the partition with a pure string check — the admin +`SetServerSecret`/`DeclareServerSecret` path REQUIRES a reserved server-secret +prefix, and the user `SetSecret`/`DeclareSecret` path REJECTS any name carrying +one. The two keyspaces are therefore disjoint by name construction: no name can +be declared into both tables in EITHER order, so the both-tables end state — a +user `SetSecret` minting a shadow `secrets` row under a server-secret name, or +an admin `SetServerSecret` declaring under a name already live in `secrets` — +is UNREACHABLE, not merely healed after the fact. F1 needs no cross-table +membership SELECT (so no cross-tenant RLS-visibility question) and no boot +reconcile: the partition is enforced at declaration time on both doors. The +table split alone is necessary but not sufficient; the reserved-prefix partition +is what makes it sufficient. The prefix check is a name test only — it does not +widen the underlying declare/set/rollback trio's single-writer contract ("is +not atomic and assumes no concurrent same-name writer", secrets_service.go:88-91), +which the existing `SetSecret` path already carries; per-name write atomicity +across paths is deferred to the multi-writer work that comment flags, tracked +with the per-tenant defer (RIG-3237). C1 keeps the SAME SecretSpec profile for both instances — the shared project is `manifestProject = "compass"` (resolver.go:19) and the profile `defaultProfile = "default"` (resolver.go:23; `WithProfile` exists, -resolver.go:78, but C1 does not use it) — so the provider keyspace is shared -and moving an existing secret between the two tables is a DB-row move with -the provider value untouched (load-bearing for OQ-4). +resolver.go:78, but C1 does not use it) — so the provider keyspace is shared. +Because the reserved-prefix partition renames the six forge secrets under +`SERVER_`, and the pre-production deployment carries no live data worth +preserving, the existing six declarations are simply WIPED and re-declared under +their prefixed names through the new admin RPC (OQ-4) — no in-place move, no +provider-value migration. **C1 REMOVES the public proto ENUM change** the previous delivery-kind mechanism required: no `SECRET_DELIVERY_SERVER_ONLY` proto enum value, no @@ -276,18 +279,20 @@ open", admin_gate.go:44-46). Today's user-facing `SetSecret` cannot declare a row *as* a server secret — `secretRoutingFromProto` admits only File/Env delivery (secrets_service.go:275-284), with no server-only delivery value — so operators declare server secrets through the new RPC. It CAN, however, mint an -ordinary `secrets` row under a server-secret NAME (delivery=File/Env, an -arbitrary name), which is exactly why the F1 membership guard is mandatory on -this user path and not merely on the admin RPC. +ordinary `secrets` row under a NAME that collides with a server secret +(delivery=File/Env), which is exactly why the F1 prefix guard is mandatory on +this user path and not merely on the admin RPC — the user path rejects any +reserved-prefixed name, so the collision cannot be declared. This retroactively fixes a pre-existing exposure: the App PEM, webhook signing, and Linear OAuth secrets are server_only by convention only ("Declared server_only secret NAME holding the PRIMARY App PEM private key", `go/cmd/compass-server/main.go:414-417`; webhook :418-421; Linear :435-450; boot-resolved by name via `newDeclaredSecretResolver`, serve.go:1469-1481) -and today ride the same inject-all path into every agent container. Their -declared rows MOVE from `secrets` into `server_secrets` in this PR chain -(OQ-4, RESOLVED) — a DB-row move only, since the profile is shared. +and today ride the same inject-all path into every agent container. They are +re-declared under `SERVER_`-prefixed names in the separate `server_secrets` +store in this PR chain (OQ-4, RESOLVED) — the pre-production deployment is wiped +and re-declared through the new admin RPC, not migrated in place. ### D7 — One seal/open path for `api_key` and OAuth payloads @@ -327,14 +332,19 @@ rejected branches are the ones a future reader will reach for first. container-undeliverable property with a DB-row-only move; the profile split remains available later if provider-keyspace isolation is ever needed. Named residual under C1: the shared profile leaves the provider - keyspace a shared mutable surface reachable by the user `SetSecret` path, - so all SEVEN server-secret names' provider values (the master key plus the - six migrated names: primary and reviewer App PEM, the GitHub webhook secret, - and the three Linear secrets — client id, client secret, and webhook) are + keyspace a shared mutable surface — no longer reachable by the user + `SetSecret` path (the F1 prefix guard rejects reserved-prefixed names there), + but still reachable by an operator's out-of-band `secretspec set` against the + shared provider — so all SEVEN server-secret names' provider values (the + master key plus the six re-declared names: primary and reviewer App PEM, the + GitHub webhook secret, and the three Linear secrets — client id, client + secret, and webhook) are guarded by the - F1 membership check (T0, both paths — reject any `server_secrets` name) - RATHER than structurally; the boot read-back verify additionally covers the - master key. C2 would make that isolation structural for all of them. + F1 reserved-prefix partition (T0, both paths — the user path rejects + reserved-prefixed names, the admin path requires them) at the DECLARATION + layer rather than at the provider-keyspace layer; the boot read-back verify + additionally covers the master key. C2 would make the provider-keyspace + isolation structural for all of them. Acceptable with the guard in place; the follow-up (per-tenant credential at-rest isolation + gateway-topology exposure) is where C2 is reconsidered (RIG-3237). @@ -444,17 +454,15 @@ declared into a store that does not exist. `DeleteServerSecretDeclaration(ctx, actor, name)` mirroring `DeclareSecret` / `DeleteSecretDeclaration` (store/secrets.go:82, :160) minus delivery/kind/provider/host (actor nullable-empty for the - server-provisioned path). `DeclareServerSecret` AND its tx-bound - `DeclareServerSecretTx` variant BOTH carry the MIRROR of the - `store.DeclareSecret` membership guard (one shared unexported check both call, - so the two can never drift) — it rejects any name STILL present in `secrets` - at the store door, so no writer (the admin RPC, or any future - second writer that bypasses it) can create a `server_secrets` shadow row - while the name is live in `secrets`; the boot reconcile satisfies the guard - by deleting the source `secrets` row FIRST, inside the ONE - `store.WithTx(store.WithSystemRole(ctx), …)` transaction the Row-migration - bullet specifies, before its `DeclareServerSecretTx` insert — so F1 stays - held at BOTH doors, not only at the RPC. It ALSO adds the read + server-provisioned path). `DeclareServerSecret` carries the F1 PREFIX guard + at the store door — it REQUIRES the name to carry a reserved server-secret + prefix (`SERVER_` or `GATEWAY_CREDENTIALS_`), rejecting any unprefixed name — + so no writer (the admin RPC, or any future second writer) can create a + `server_secrets` row under a name the user keyspace owns. Combined with the + user path's rejection of those same prefixes (below), the two doors partition + the keyspace by name, so F1 holds at BOTH doors, not only at the RPC — a pure + string check, no cross-table read, no transaction spanning both tables. It + ALSO adds the read `ServerDeclaredSecrets(ctx)` — a thin store view whose `DeclaredSecrets(ctx context.Context) ([]store.SecretDeclaration, error)` method (the `declarations` @@ -467,8 +475,9 @@ declared into a store that does not exist. resolver.go:23) and project (`manifestProject = "compass"`, resolver.go:19), its own manifest state dir. The container resolver's CONSTRUCTION at serve.go:528 is unchanged — no filter, nothing to filter - — but its boot CONSUMERS must be re-pointed: once T0's reconcile moves - the six server-secret names out of `secrets`, every boot consumer that + — but its boot CONSUMERS must be re-pointed: because the six server-secret + names are re-declared under `SERVER_`-prefixed names in `server_secrets` + (not `secrets`), every boot consumer that still resolves those names through the serve.go:528 instance breaks, because that instance's manifest no longer declares them. THREE failure modes verified in the tree: (a) HARD BOOT FAILURE — `validateForgeSecret` @@ -516,149 +525,76 @@ declared into a store that does not exist. BOTH resolver instances (or receives a pre-built `linearWebhookHandler` from Serve), because swapping its one `resolver` argument would repoint the container manifest at `server_secrets` and deliver every server secret into - every agent container, inverting D6. Ordering: the reconcile MUST run + every agent container, inverting D6. Ordering: the boot declare MUST run BEFORE `buildForgeReadWiring` (serve.go:567) so the names are in - `server_secrets` before any forge consumer resolves them. + `server_secrets` before any forge consumer resolves them (the boot-declare + bullet below restates this ordering). - RPC: `SetServerSecret`/`DeleteServerSecret` on the secrets service — mirrors the `SetSecret` declare-then-Set flow and its rollback discipline (secrets_service.go:92-145) minus delivery/kind, targeting `server_secrets`; admin-gated (`adminOnly` in `classifyProcedure`, - admin_gate.go:47). Carries the reserved-name guard: rejects - `GATEWAY_CREDENTIALS_MASTER_KEY` (or the reserved prefix - `GATEWAY_CREDENTIALS_`, so future master-key-family names are reserved - before they are declared) with an actionable error — rotation is OQ-1 - machinery, never a raw overwrite. It ALSO carries the MIRROR membership - guard: it rejects any name already present in `secrets` (checked before the - `server_secrets` insert and before `resolver.Set`) with an actionable error - naming the conflicting user declaration — the F1 invariant is order-free - (no name is ever live in both tables) and must hold in BOTH declaration - orders, so the `secrets`→`server_secrets` direction is guarded here just as - the `server_secrets`→`secrets` direction is guarded on the user path below. - - User-path server-secret guard (F1 — mandatory, NOT admin-RPC-only): C1 + admin_gate.go:47). Carries the F1 PREFIX guard: it REQUIRES the declared + name to carry a reserved server-secret prefix (`SERVER_` for the six forge + secrets, `GATEWAY_CREDENTIALS_` for the master-key family), rejecting any + unprefixed name with an actionable error. This is the admin half of the + structural partition (D6): because the user path (below) rejects those same + prefixes, a reserved-prefix name can only ever live in `server_secrets` and + an unprefixed name can only ever live in `secrets` — the two keyspaces are + disjoint by name, so no name is ever live in both tables in EITHER + declaration order, with no cross-table read. The reserved master-key name + `GATEWAY_CREDENTIALS_MASTER_KEY` is additionally rejected on + `SetServerSecret`/`DeleteServerSecret` (rotation is OQ-1 machinery, never a + raw overwrite), so an admin cannot clobber the auto-provisioned key. + - User-path prefix guard (F1 — mandatory, NOT admin-RPC-only): C1 shares the provider keyspace (§D2 read-back verify), so the `authenticatedOpen` user `secretsService.SetSecret`/`DeleteSecret` path (any authenticated account, admin_gate.go:122-125) can overwrite a - server secret's provider value AND — because after OQ-4 the six - PEM/webhook/Linear names live in `server_secrets`, not `secrets` — - create a FRESH shadow row in `secrets` under that name (no primary-key - conflict), which the inject-all path then delivers into every container. - The reserved-name check alone is INSUFFICIENT: those six names are - arbitrary per-deployment config (main.go:414-450) and carry no reserved - prefix. So the guard is a MEMBERSHIP test, not a name/prefix match: - `SetSecret`/`DeleteSecret` (and `DeclareSecret` at the store door, so the - shadow row can never be created at all) MUST reject any name present in - `server_secrets` — checked BEFORE `resolver.Set`/`Delete` - (secrets_service.go:124/208) — in addition to the reserved master-key name - `GATEWAY_CREDENTIALS_MASTER_KEY` and prefix `GATEWAY_CREDENTIALS_`. A T0 + server secret's provider value AND — absent the guard — mint a shadow + `secrets` row under a server-secret name, which the inject-all path then + delivers into every container. So the user path is the other half of the + partition: `SetSecret`/`DeleteSecret` (and `DeclareSecret` at the store + door, so the shadow row can never be created at all) MUST REJECT any name + carrying a reserved server-secret prefix (`SERVER_` or + `GATEWAY_CREDENTIALS_`) — checked BEFORE `resolver.Set`/`Delete` + (secrets_service.go:124/208). A pure string check, not a membership SELECT: + it needs no read of `server_secrets` and no cross-tenant visibility. A T0 deliverable on the existing user path, not only the new admin RPC. - - Row migration — a BOOT-TIME config-driven reconcile, NOT an in-migration + - Boot declaration — a BOOT-TIME config-driven declare, NOT an in-migration DML step (F2): the server-only secret NAMES are per-deployment config the embedded SQL migration (`//go:embed migrations/*.sql`, store.go:24-25) - cannot know. So the move runs at boot in the server wiring (where the - config IS available): for each configured server-secret NAME, if a row - exists in `secrets`, move it — inside ONE `store.WithTx(store.WithSystemRole(ctx), - …)` (coordination.go:368; `armTx` arms the BYPASSRLS system role for the whole - tx, tenant_tx.go:199-215), DELETE the `secrets` row FIRST, then INSERT - the `server_secrets` row, so the store-door membership guard sees the delete - already applied and admits the insert. Both statements run through tx-bound - `DeleteSecretDeclarationTx`/`DeclareServerSecretTx(ctx, tx, …)` variants - (the shipped `…Tx` convention, coordination.go:86/168, dm.go:49/97), with - `DeclareServerSecretTx` carrying the SAME store-door membership guard as its - plain `DeclareServerSecret` counterpart via the shared check (the delete side - carries no membership guard — the reconcile heal arm below depends on being - able to delete a `secrets` row whose name is already live in `server_secrets`), - so the DELETE-FIRST ordering is what admits the insert — NOT the - plain-signature methods, each of which executes through `scopedDBTX` as its - OWN implicit transaction (one `pool.SendBatch` per statement, tenant_tx.go:64-70, - :80-92), which would make the two operations two transactions and break the - atomicity below (atomic: one transaction commits or rolls back both, so no - crash window strands the declaration in neither table) — INCLUDING when a - `server_secrets` row for that name already exists, in which case the pass - deletes the `secrets` shadow row and logs the reconciled collision. Leaving - a both-tables name in place would strand it on the inject-all path forever, - so the reconcile heals the collision rather than skipping it. The heal is - NOT limited to the configured names: after moving the configured set, the - pass also scans `server_secrets` for any name that STILL has a live - `secrets` row (an operator-declared server secret raced into both tables, - which carries no reserved prefix and is not in `cfg.Forge.resolved()`) and - deletes that shadow row too, so every both-tables state self-heals - regardless of provenance. Idempotent, serialized under the SAME - `pg_advisory_xact_lock` constant key T2 uses, acquired with the SAME - bounded discipline (`pg_try_advisory_xact_lock` in a bounded retry loop, - or a session `lock_timeout`) so a contended boot fails closed with a - diagnosable error naming the contended lock rather than parking — in its - OWN transaction, taken and released before the T2 provisioning transaction, - so the two never - overlap; the reconcile's only ordering constraint is that it precede - `buildForgeReadWiring` (serve.go:567). No provider write (the shared - profile makes it a pure row move). The name source is the RESOLVED config - `cfg.Forge.resolved()` (serve.go:232-246) — the SAME accessor every live - consumer uses (`buildLinearTokenSource` :1700, `buildBoardWebhookWiring` - :1002, `buildForgeWriteService` :1624), NOT the raw flag/env layer: two of - the names carry CODE DEFAULTS applied after that layer - (`defaultForgeLinearClientIDSecretName = "LINEAR_FORGE_CLIENT_ID"` / + cannot know. So the declaration runs at boot in the server wiring (where the + config IS available): for each configured server-secret name, the + server-secret layer applies the reserved `SERVER_` prefix and ensures a + `server_secrets` row exists under the PREFIXED name via the admin store door + `DeclareServerSecret` — idempotent, a no-op when the row is already present. + The repointed forge consumers (the CONSUMER-REPOINT deliverable above) + resolve the SAME prefixed names, so the declared name and the resolved name + agree on both sides of the split. No cross-table read and no BYPASSRLS: `server_secrets` is bucket-A + infrastructure (no `tenant_id`, RLS NOT enabled — §Mechanism C1), so the + boot declare runs on the ordinary compass_app store path with no tenant + scoping to escape. Because F1 is a structural name partition (the user path + rejects any `SERVER_`/`GATEWAY_CREDENTIALS_`-prefixed name), no reconcile, + no self-heal scan, and no delete-before-insert ordering are needed: a + both-tables state is unreachable, not repaired after the fact. Ordering: the + boot declare MUST run BEFORE `buildForgeReadWiring` (serve.go:567) so the + names are in `server_secrets` before any forge consumer resolves them. + Serialized under the SAME `pg_advisory_xact_lock` constant key and bounded + discipline T2 uses (`pg_try_advisory_xact_lock` in a bounded retry loop, or + a session `lock_timeout`), so a contended boot fails closed with a + diagnosable error naming the contended lock rather than parking. The name + source is the RESOLVED config `cfg.Forge.resolved()` (serve.go:232-246) — + the SAME accessor every live forge consumer uses, so a default-named Linear + pair (`defaultForgeLinearClientIDName = "LINEAR_FORGE_CLIENT_ID"` / `defaultForgeLinearClientSecretName = "LINEAR_FORGE_CLIENT_SECRET"`, - serve.go:202-203), so a reconcile reading the raw `main.go` flag/env layer - would see `""` for a default-named Linear pair and SILENTLY SKIP those two - rows, leaving them on the inject-all path forever — the exact exposure the - reconcile exists to close. The complete set is SIX names, not three: the + serve.go:202-203) is declared under the same names the consumers resolve, + not the raw `main.go` flag/env layer. The complete set is SIX names: the PRIMARY App PEM (`appKeySecret`, main.go:414-417), the webhook secret - (`appWebhook`, main.go:418-421), the REVIEWER App PEM - (`reviewerAppKeySecret`, main.go:430; consumed serve.go:1644), and the - THREE Linear secrets (client id, client secret, webhook — resolved names - per `cfg.Forge.resolved()`, provenance main.go:435-450). Closes the - pre-existing inject-all exposure (D6) for every configured name the - reconcile actually runs for. Same PR chain: OQ-4, RESOLVED. - - Reconcile execution context (F2 follow-on — the relocation's RLS - consequence): the SOURCE table `secrets` is `FORCE ROW LEVEL SECURITY` - (it is in the `tenant_tables` array, 0001_init.sql:912; the DO loop at - :922-923 issues `ENABLE`/`FORCE ROW LEVEL SECURITY`). The old - in-migration approach ran on the raw owner pool BEFORE any policy existed - (store.go:89-90, "migrate() … runs on the raw pool as the owner, before - any policy exists to fight"); the boot-time reconcile runs AFTER migration, - and if it took the ordinary tenant-scoped store path (`SET LOCAL ROLE - compass_app` + `set_config('compass.tenant_id', …)`, tenant_tx.go:140-141) - — a non-owner, non-BYPASSRLS role — it would be confined to the bootstrap - tenant's `secrets` rows. `secrets.name` is a GLOBAL primary key - (0001_init.sql:396, `name TEXT PRIMARY KEY`; `tenant_id` is a plain - non-key column at :413), so there is at most ONE row per configured - server-secret name in the whole deployment — but that row may have been - declared under a NON-bootstrap tenant, in which case a compass_app-scoped - reconcile cannot see it and silently skips it, leaving it on the inject-all - path forever. So the reconcile MUST run under `store.WithSystemRole(ctx)` - (tenant_tx.go:41-50), which arms `SET LOCAL ROLE compass_system` with no - tenant GUC (armQueue, tenant_tx.go:136-138; armTx, :199-215, shared by - beginTenantTx, :185-195; `compass_system` is `NOLOGIN BYPASSRLS`, - 0001_init.sql:879, idempotently re-asserted :883), seeing - every tenant's rows in one pass. This adds a cross-tenant, request-path-free - boot step to the BYPASSRLS surface DL-315 governs. DL-315 names four - cross-tenant loops (delivery-cursor sweep, deliver-ack advance, reattach - recovery, lag-resync); the shipped tree arms the system role at THREE call - sites — `go/internal/delivery/consumer.go` (`Consumer.Run`, :316; covering - the sweep/resync/recovery arms since the loop threads one ctx and never - re-roots it, :315), `go/internal/runnerhub/hub.go` (deliver-ack advance, - :753), and `go/internal/runnerhub/hub.go` (`forgeNotificationAck`, :814 — - an arm DL-315's four names do NOT list — though `WithSystemRole`'s own doc - comment already names it (tenant_tx.go:41-47), so THIS staleness is in - DL-315's ledger prose, not in the code's documentation — but note T0 itself - then widens that same code doc's ONLY-claim (tenant_tx.go:36-38, :43-45) - to admit the boot reconcile as a fifth site, so both surfaces are corrected - in this PR chain, scheduled as a T0 deliverable above). Symbol names - anchor these; the line numbers are a convenience that drifts. DL-315's - load-bearing "granted ONLY to those named background workers and NEVER on - the request path" clause (DECISIONS.md:113) is UNCHANGED — this is a boot - step, never the request path — but its named entrypoint set is widened, - recorded (OQ-5, Matt-ruled) as its own ledger row DL-326 that Refines - DL-315. (Note: a raw-owner-pool reconcile would move every tenant's rows - only if the owner connection is a superuser — which the tree ASSUMES but - does not enforce: 0001_init.sql:848-851 states the property that a superuser - owner bypasses even FORCE, and rls_pgtest_test.go:281-284 records the same - assumption in a comment ("the pgtest harness (like production) connects as a - SUPERUSER owner"), which is why that test deliberately does not try to prove - FORCE. An assumption stated in two comments is not an enforced invariant; - the reconcile takes the explicit BYPASSRLS path rather than inheriting an - owner-privilege escape.) + (`appWebhook`, main.go:418-421), the REVIEWER App PEM (`reviewerAppKeySecret`, + main.go:430; consumed serve.go:1644), and the three Linear secrets (client + id, client secret, webhook — per `cfg.Forge.resolved()`, provenance + main.go:435-450). Closes the pre-existing inject-all exposure (D6) for every + configured name. Same PR chain: OQ-4, RESOLVED. - Consumes: nothing from T1-T5 (pure prerequisite). - Tests: a secret declared in `server_secrets` is ABSENT from a FetchSecrets response (because it lives in the other table — no filter @@ -668,43 +604,35 @@ declared into a store that does not exist. caller; the reserved master-key name is rejected on SetServerSecret/DeleteServerSecret AND on the user-path SetSecret/DeleteSecret (a non-admin authenticated user calling SetSecret - with the reserved name is rejected and the provider value is unchanged — - F1); the server resolver can READ `server_secrets` through the normal - compass_app store path (the GRANT is present — F3); all six migrated + with a `GATEWAY_CREDENTIALS_`-prefixed name is rejected and the provider value + is unchanged — F1); the server resolver can READ `server_secrets` through the + normal compass_app store path (the GRANT is present — F3); all six configured names (primary + reviewer App PEM, webhook, and the three Linear secrets) - resolve through the server resolver and are gone from FetchSecrets; and a - configured server-secret row declared under a NON-bootstrap tenant is found - and moved by one reconcile pass and is absent from FetchSecrets afterward - (red without `WithSystemRole`: the compass_app-scoped reconcile sees zero - rows and skips it silently). A deployment with the Linear pair declared under - the DEFAULT names (`LINEAR_FORGE_CLIENT_ID` / `LINEAR_FORGE_CLIENT_SECRET`) - and NO flag/env set is likewise found and moved by one reconcile pass and is - absent from FetchSecrets afterward (red if the reconcile reads the raw - flag/env layer instead of `resolved()`). CONSUMER-REPOINT positive + are boot-declared under their `SERVER_`-prefixed names, resolve through the + server resolver, and are gone from FetchSecrets; and the boot declare is + IDEMPOTENT — a second boot re-runs it as a no-op (no duplicate-row error, no + provider write). A deployment with the Linear pair configured under the + DEFAULT names (`LINEAR_FORGE_CLIENT_ID` / `LINEAR_FORGE_CLIENT_SECRET`) and + NO flag/env set is likewise boot-declared under their `SERVER_`-prefixed names + and absent from FetchSecrets (red if the boot declare reads the raw flag/env + layer instead of `resolved()`). CONSUMER-REPOINT positive assertions (the (b)/(c) silent modes need them, since an absent name is indistinguishable from a legitimate off-state): boot a server with all six - names configured and, after one reconcile pass, assert the GitHub App token + names configured and, after the boot declare, assert the GitHub App token source mints, the GitHub and Linear webhook handlers are MOUNTED (non-nil), the Linear token source is non-nil, AND the forge WRITE caller is MOUNTED (`hub.SetForgeCaller` called / `RelayForgeCall` does not fail-close to `CodeUnavailable`) — i.e. the forge read AND write lanes still wire off the - server resolver, not just that the names left `secrets`. MEMBERSHIP-GUARD - assertion (F1): a non-admin authenticated caller invoking `SetSecret` with - a configured server-secret NAME (not only the reserved master-key name) is - rejected, no `secrets` shadow row is created, and the provider value is - unchanged. MIRROR-GUARD assertion (F1, admin path): an admin calling - `SetServerSecret` with a name already present in `secrets` is rejected - before the `server_secrets` insert and before `resolver.Set`, no - `server_secrets` row is created, and the provider value is unchanged. - RECONCILE-HEAL assertion: a name live in BOTH tables at boot is healed by - one reconcile pass — the `secrets` shadow row is deleted, the collision is - logged, and the name is absent from FetchSecrets afterward (red if the - reconcile skips a name that already has a `server_secrets` row). ORDERING - assertion: a reconcile that INSERTs the `server_secrets` row before deleting - the `secrets` row reds against the store-door membership guard, and both - statements commit or roll back together — a crash between them leaves the name - in `secrets`, never stranded in neither table (red if the DELETE and INSERT run - as two transactions rather than one `store.WithTx`). + server resolver, not just that the names are in `server_secrets`. F1 PREFIX + PARTITION assertions: (user path) a non-admin authenticated caller invoking + `SetSecret`/`DeclareSecret` with a `SERVER_`- or `GATEWAY_CREDENTIALS_`-prefixed + name is REJECTED before `resolver.Set`, no `secrets` row is created, and the + provider value is unchanged; (admin path) an admin invoking `SetServerSecret` + with an UNPREFIXED name is REJECTED before the `server_secrets` insert and + before `resolver.Set`, no `server_secrets` row is created. Together these + assert the two keyspaces are disjoint by name, so no name can ever be live in + both tables — the structural F1 property, checked with pure string tests, no + cross-table or cross-tenant read. ### T1 — Envelope-crypto helper package @@ -838,31 +766,31 @@ is declared into it) and T1. `server_key_state` row — `key_fingerprint`/`fingerprint_salt`, not a declared-name registry so the names-only invariant is untouched) so a swapped provider value is DETECTED at boot rather than silently adopted. - - **Reserved-name guard (F1 — on EVERY provider-writing path):** C1 splits - the DECLARATION registries (two tables) but NOT the provider keyspace - (one shared profile, above). So the isolation C1 buys is that server - secrets are structurally undeliverable to CONTAINERS (the manifest - separation, §Mechanism C1) — it does NOT make the master key's provider - value unreachable from the user path. The user `SetSecret`/`DeleteSecret` - RPC (`authenticatedOpen`, any authenticated account — admin_gate.go:122-125) - declares into `secrets` but then calls `resolver.Set`, which shells - `secretspec set --profile default` (resolver.go:258-267) against - the SHARED keyspace — so a user calling `SetSecret` with name - `GATEWAY_CREDENTIALS_MASTER_KEY` would OVERWRITE the master key's - provider value (the running process keeps its cached key, but the next - boot adopts the attacker-chosen key → every existing row fails GCM auth, - every new row is sealed under a known key: the exact bulk-disclosure D1 - prevents). Therefore the reserved-name guard is MANDATORY on the user - `secretsService.SetSecret`/`DeleteSecret` path (reject the reserved name - `GATEWAY_CREDENTIALS_MASTER_KEY` and prefix `GATEWAY_CREDENTIALS_` BEFORE - `resolver.Set`/`Delete`), a T0/T2 deliverable, AND on - the new `SetServerSecret`/`DeleteServerSecret` RPC. Both are tested - red-green (a non-admin user calling `SetSecret` with the reserved name is - rejected and the provider value is unchanged). Rotation is OQ-1's - machinery, never a raw overwrite through either surface. The name-keyed - global user delete ("a row is keyed by name alone, not (actor, name)", - go/internal/store/secrets.go:150-153) is why the guard covers the delete - path too, once a real provider hard-delete lands. + - **Master-key overwrite guard (F1 prefix partition — on EVERY + provider-writing path):** C1 splits the DECLARATION registries (two tables) + but NOT the provider keyspace (one shared profile, above). So the isolation + C1 buys is that server secrets are structurally undeliverable to CONTAINERS + (the manifest separation, §Mechanism C1) — it does NOT by itself make the + master key's provider value unreachable from the user path. The user + `SetSecret`/`DeleteSecret` RPC (`authenticatedOpen`, any authenticated + account — admin_gate.go:122-125) declares into `secrets` but then calls + `resolver.Set`, which shells `secretspec set --profile default` + (resolver.go:258-267) against the SHARED keyspace — so absent a guard a user + calling `SetSecret` with name `GATEWAY_CREDENTIALS_MASTER_KEY` would + OVERWRITE the master key's provider value (the running process keeps its + cached key, but the next boot adopts the attacker-chosen key → every existing + row fails GCM auth, every new row is sealed under a known key: the exact + bulk-disclosure D1 prevents). The F1 prefix guard (D6) closes this: the + master-key name carries the reserved `GATEWAY_CREDENTIALS_` prefix, which the + user `secretsService.SetSecret`/`DeleteSecret` path REJECTS BEFORE + `resolver.Set`/`Delete` — a T0/T2 deliverable, the SAME string check that + enforces the keyspace partition, so no separate membership read is needed. + Tested red-green (a non-admin user calling `SetSecret` with a + `GATEWAY_CREDENTIALS_`-prefixed name is rejected and the provider value is + unchanged). Rotation is OQ-1's machinery, never a raw overwrite through + either surface. The name-keyed global user delete ("a row is keyed by name + alone, not (actor, name)", go/internal/store/secrets.go:150-153) is why the + guard covers the delete path too, once a real provider hard-delete lands. - **Nil-resolver deployment:** a server built with no secrets surface is legitimate today ("resolver may be nil on a server built with no secrets surface (FetchSecrets then fails CodeFailedPrecondition rather @@ -982,33 +910,26 @@ RPC exactly as the frozen record already specifies. SELECT/INSERT/UPDATE on `server_key_state` to compass_app/compass_system + bucketA allow-list edit in rls_pgtest_test.go), `ServerDeclaredSecrets` store view + second SpecResolver instance (same profile), admin-gated - SetServerSecret/DeleteServerSecret RPC carrying the MIRROR membership - guard (reject any name already present in `secrets`, checked before the - `server_secrets` insert and before `resolver.Set`, AND at the - `store.DeclareServerSecret` / `DeclareServerSecretTx` store door (both - variants, one shared check) so no writer can create a - `server_secrets` shadow row while the name is live in `secrets` — the boot - reconcile satisfies it by deleting the source `secrets` row first inside - its single `store.WithTx(store.WithSystemRole(ctx), …)` transaction) AND the - matching user-path half on `secretsService.SetSecret`/`DeleteSecret` and - `store.DeclareSecret` — together the SYMMETRIC F1 MEMBERSHIP test, - each direction rejecting any name present in the OTHER table (the six - configured names carry no reserved prefix, so a name/prefix match is - insufficient) PLUS the reserved `GATEWAY_CREDENTIALS_MASTER_KEY` name and - `GATEWAY_CREDENTIALS_` prefix, both checked before `resolver.Set`/`Delete`, - and a BOOT-TIME config-driven reconcile (NOT - in-migration DML) moving all SIX configured server-secret names (primary - + reviewer App PEM, webhook, three Linear) from `secrets` to - `server_secrets` via the tx-bound - `DeleteSecretDeclarationTx`/`DeclareServerSecretTx` pair (both NEW — no - `…Tx` sibling of `DeleteSecretDeclaration` exists today, store/secrets.go:160), - run under `store.WithSystemRole` (BYPASSRLS — the - compass_app-scoped path is confined to the bootstrap tenant, so a row - declared under another tenant is otherwise silently skipped) — PLUS the - doc-comment repairs T0's own changes require: `store.WithSystemRole`'s - ONLY-claim (tenant_tx.go:36-38, :43-45) widens from the four - background-loop entrypoints to ALSO cover this boot server-secret - reconcile (a FIFTH, non-loop site; per DL-326); the five + SetServerSecret/DeleteServerSecret RPC carrying the F1 PREFIX guard + (REQUIRE a reserved server-secret prefix — `SERVER_` or + `GATEWAY_CREDENTIALS_` — checked before the `server_secrets` insert and + before `resolver.Set`, AND at the `store.DeclareServerSecret` store door, + so no writer can create a `server_secrets` row under an unprefixed name) + AND the matching user-path half on `secretsService.SetSecret`/`DeleteSecret` + and `store.DeclareSecret` (REJECT any reserved-prefixed name before + `resolver.Set`/`Delete`) — together the STRUCTURAL F1 PREFIX PARTITION, + the two doors making the keyspaces disjoint by name so no name is ever live + in both tables in either order, a pure string check with no cross-table + read; PLUS the reserved `GATEWAY_CREDENTIALS_MASTER_KEY` name rejected on + the admin path too (rotation is OQ-1 machinery, not a raw overwrite), + and a BOOT-TIME config-driven declare (NOT + in-migration DML) declaring all SIX configured server-secret names (primary + + reviewer App PEM, webhook, three Linear) into `server_secrets` under + their `SERVER_`-prefixed names via `DeclareServerSecret` — idempotent, a + no-op when the row already exists, run on the ordinary compass_app store + path (`server_secrets` is bucket-A, RLS NOT enabled, so NO BYPASSRLS and NO + cross-tenant read is needed) — PLUS the + doc-comment repairs T0's own changes require: the five "three/3 procedures authenticatedOpen" SecretsService comments (admin_gate.go:116-121, network_door.go:288-290, secrets_service.go:6, serve.go:555, serve.go:749-750) are restated as three authenticatedOpen @@ -1030,14 +951,12 @@ RPC exactly as the frozen record already specifies. procedure" phrasing is already stale — the switch covers SecretsService too — as is the identical phrasing at classify_exhaustive_test.go:45-46; correct both in the same pass, but neither gates T0.) Red-green tests: - absent-from-FetchSecrets + server-resolver-resolves + user-path-guard + - admin-path-mirror-guard (a `SetServerSecret` on a name already in - `secrets` is rejected) + reconcile-heals-both-tables-collision (one pass - deletes the `secrets` shadow row and the name leaves FetchSecrets) + - reconcile-orders-delete-before-insert (a reconcile that INSERTs the - `server_secrets` row before deleting the `secrets` row reds against the - store-door membership guard) + non-bootstrap-tenant-reconcile. - PREREQUISITE of T2. + absent-from-FetchSecrets + server-resolver-resolves + + user-path-prefix-guard (a `SetSecret` on a reserved-prefixed name is + rejected, no `secrets` row created) + + admin-path-prefix-guard (a `SetServerSecret` on an UNPREFIXED name is + rejected, no `server_secrets` row created) + boot-declare-idempotent (a + second boot re-declares the six as a no-op). PREREQUISITE of T2. Proto delta: two additive `SecretsService` methods, no enum change — the checked-in public gen trees are drift-gated, so this needs the `compass.proto` edit + `moon run compass-proto:gen`, and both new @@ -1048,10 +967,9 @@ RPC exactly as the frozen record already specifies. (AAD-carrying) + redaction + unit tests - [ ] T2 — boot resolve-or-provision seam via the server-secret resolver (advisory-lock serialized, read-back verify + key-fingerprint tripwire, - nil-resolver gating, fail-closed boot; the F1 guards (membership + - reserved-name) are T0's, on BOTH the admin - SetServerSecret/DeleteServerSecret RPC AND the authenticatedOpen user - SetSecret/DeleteSecret path). + nil-resolver gating, fail-closed boot; the F1 prefix guards are T0's, on BOTH + the admin SetServerSecret/DeleteServerSecret RPC AND the authenticatedOpen + user SetSecret/DeleteSecret path). DEPENDS ON T0 + T1. - [ ] T3 — `gateway_credentials` migration columns: `value_ciphertext`, `value_nonce`, `key_version`; no plaintext column ever. DEPENDS ON @@ -1105,26 +1023,31 @@ RPC exactly as the frozen record already specifies. - **OQ-4 — RESOLVED (Matt: same PR chain, mechanism C1): the PEM/webhook/Linear server-secret rows move to `server_secrets` in THIS record's PR chain**, closing the live pre-existing exposure (they ride - the inject-all path into every agent container today, D6). NOTE: C1 - makes the move cheap — because both resolver instances share one - SecretSpec profile (`defaultProfile`, resolver.go:23), this is a DB-row - move only: provider values stay put and nothing is re-provisioned. New - operator declarations go through the admin-gated `SetServerSecret` RPC - (T0); the move for the ALREADY-declared names is T0's boot-time - config-driven reconcile (per configured NAME, not an in-migration DML — - the names are per-deployment config the embedded migration cannot know). -- **OQ-5 — RESOLVED (Matt: a new ledger row): how to RECORD the DL-315 - BYPASSRLS-allow-list widening.** T0's boot server-secret reconcile adds a - cross-tenant, request-path-free BYPASSRLS entrypoint (`store.WithSystemRole`) - beyond the set DL-315 governs — and the review found DL-315's own - enumeration is already stale (the shipped `hub.go:814` forge-notification-ack - arm is a BYPASSRLS site DL-315's four names omit). DL-315's load-bearing - NEVER-on-the-request-path clause is untouched; only its named-entrypoint set - widens. **Matt ruled: its OWN ledger row that Refines DL-315** (matching the - ledger's dense precedent for scope-amendment rows — DL-316/DL-317/DL-293/ - DL-072/DL-212 — and fixing the discoverability gap: an engineer auditing - "which sites may arm BYPASSRLS?" reads DL-315, not this at-rest-encryption - record). Landed as **DL-326** in this PR (this PR's encryption row is - DL-325; the sibling SubjectService PR takes DL-327), restating the true shipped - entrypoint set (three call sites incl. the forge-notification-ack arm) + - the reconcile; DL-315 stays Active. + the inject-all path into every agent container today, D6). Because the + reserved-prefix partition (D6) renames the six under `SERVER_`, and the + deployment is pre-production, the ALREADY-declared rows are WIPED and + re-declared under their prefixed names rather than migrated in place — no + provider-value migration (the shared profile means the provider values are + re-Set under the new names). New operator declarations go through the + admin-gated `SetServerSecret` RPC (T0); the six configured names are + boot-declared under their `SERVER_`-prefixed names by T0's idempotent + boot-time config-driven declare (per configured NAME, not an in-migration + DML — the names are per-deployment config the embedded migration cannot + know). +- **OQ-5 — WITHDRAWN (Matt: prefix partition, no reconcile): the DL-315 + BYPASSRLS-allow-list widening is no longer needed.** An earlier draft of this + record moved the six server-secret rows with a boot reconcile that had to run + under `store.WithSystemRole` (BYPASSRLS) to see cross-tenant `secrets` rows, + which would have added a BYPASSRLS entrypoint beyond the set DL-315 governs. + Matt's reserved-prefix partition (D6) removed the reconcile entirely — server + secrets are re-declared under `SERVER_`-prefixed names on the ordinary + compass_app store path (`server_secrets` is bucket-A, no RLS), so this record + adds NO cross-tenant BYPASSRLS site and needs no DL-315 amendment. There is no + DL-326 in this PR (the encryption row is DL-325; the sibling SubjectService PR + takes DL-327). The separate, PRE-EXISTING observation that DL-315's own + entrypoint enumeration is stale (the shipped `hub.go:814` forge-notification-ack + arm is a BYPASSRLS site DL-315's four names omit — `WithSystemRole`'s own doc + comment already lists it, tenant_tx.go:41-47) is not this record's to fix, since + this record no longer touches the BYPASSRLS surface; it is left as a pre-existing + DL-315 ledger-prose observation for a separate follow-up, not tracked by this + record. From 88bea704dbcea99682a68d37ca7ab243e17cefc4 Mon Sep 17 00:00:00 2001 From: mintaka Date: Sat, 5 Sep 2026 11:27:20 -0400 Subject: [PATCH 18/18] docs(design): renumber the encryption ledger row to DL-326 after a concurrent DL-325 collision (RIG-2863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rule://sync-before-submit` rebase onto current `main@origin` (now `523965b9`, was `eb5ef7a1`) surfaced a DL-number collision: RIG-3070's runner-trust-model split (PR #804) merged its ledger row as **DL-325** — the same number this record's encryption row claimed, allocated concurrently. Renumbered this record's encryption row `DL-325 → DL-326` (now free — the earlier reconcile-BYPASSRLS DL-326 was already removed in the prefix-partition rewrite). Updates: - `DECISIONS.md`: the encryption row is now DL-326; RIG-3070's runner split keeps DL-325 (line 149). - The record's OQ-5 note flips from "the encryption row is DL-325; there is no DL-326" to "the encryption row landed as DL-326 (a concurrent RIG-3070 merge took DL-325); the sibling SubjectService PR takes DL-327". The DL-326 → record link keeps its `#d1--application-layer-aes-256-gcm-envelope-encryption` anchor (record >50KB, anchor rule satisfied; the D1 heading is unchanged). No collision remains: DL-325 (runner), DL-326 (encryption, this PR), DL-327 (sibling SubjectService PR). markdownlint 0 issues; design-ledger-gate OK (293 rows, 124 headers). Ledger-impact: encryption row renumbered DL-325 → DL-326 (concurrent-merge collision). Spec-impact: none. Refs RIG-2863 Co-authored-by: Matt Wilkinson --- docs/designs/DECISIONS.md | 2 +- .../compass-gateway-credentials-at-rest-encryption.md | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/designs/DECISIONS.md b/docs/designs/DECISIONS.md index a2a2bf35..07c003d7 100644 --- a/docs/designs/DECISIONS.md +++ b/docs/designs/DECISIONS.md @@ -112,7 +112,7 @@ check enforces the mechanical half. Full rationale: | DL-314 | Postgres remains the store of record and the sole durability source of truth for committed comms/routing/session-binding state (restating DL-019's surviving "Postgres is the store of record" clause verbatim): JetStream is an at-least-once delivery TRANSPORT whose consumer state is disposable, never a second truth store; every fabric recovery path terminates in a Postgres cursor or row. Supersedes DL-019's "JetStream is comms-only" clause only — its store-of-record and transcript-blob-seam clauses (DL-093) survive (RIG-2861 OQ-1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#q3--the-eventing-substrate-one-nats-eventfabric-a-standalone-stack-service-jetstream-as-the-delivery-transport) | | DL-315 | The cross-tenant background/system loops (delivery-cursor sweep, deliver-ack advance, reattach recovery, lag-resync) run under a narrowly-scoped `BYPASSRLS` system role granted ONLY to those named background workers and NEVER on the request path; every request-path query stays fail-closed under RLS (RIG-2861 OQ-4 = option 1) | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | | DL-317 | Under multi-tenancy, user/system handle uniqueness is PER-ORGANIZATION not global (RIG-2921 = option A): `account_handles` gains a `tenant_id` and its partial-unique indexes become org-scoped, and the handle resolvers gain a tenant filter — two organizations may each hold `@matt`. The sole globally-unique identifier is the organization NAME, CASE-FOLDED for that uniqueness check (normalized index form, display casing preserved) — `Acme` and `acme` are the same organization (GitHub model). Refines DL-271's "user/system handles globally unique" clause to per-organization under multi-tenancy (single-tenant OSS stays degenerate: one org, so per-org == global); implemented on the RIG-2880 `account_handles` storage contract | Active (Matt, 2026-08-31) | [multi-tenancy & NATS substrate](infra/runtime/compass-managed-multitenancy/design.md#resolved-decisions-freeze--matt-2026-08-31) | -| DL-325 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so F1 is a STRUCTURAL reserved-prefix partition enforced on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path: every server-secret name carries a reserved prefix (`SERVER_` for the six forge secrets, `GATEWAY_CREDENTIALS_` for the master-key family), the admin door REQUIRES one and the user door REJECTS one, so the two keyspaces are disjoint by name and no name is ever live in both tables in either declaration order — a pure string check, no cross-table membership SELECT and so no cross-tenant RLS-visibility question. The table split alone is necessary but not sufficient for the structural claim; the prefix partition is what makes it sufficient. No proto ENUM change and no FetchSecrets filter — the admin RPC adds two additive SecretsService methods (supersedes the earlier SERVER_ONLY-delivery-kind fold). The existing server-secret rows are re-declared into `server_secrets` under their `SERVER_`-prefixed names by a boot-time config-driven declare (idempotent, on the ordinary compass_app path — `server_secrets` is bucket-A, no RLS, so no reconcile and no BYPASSRLS; the pre-production deployment is wiped and re-declared, not migrated in place), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | +| DL-326 | The `gateway_credentials` value payload is encrypted at rest application-side (AES-256-GCM envelope; `value_ciphertext`+`value_nonce`+`key_version` columns, no plaintext column ever; AAD binds the stable row identity so ciphertexts are not row-portable), covering api_key and OAuth rows on ONE seal/open path (D7). A 256-bit master key is auto-provisioned zero-human-step into the ONE configured SecretSpec provider under a Postgres advisory lock with every-boot read-back verify (+ a key-fingerprint tripwire). The master key and the existing server secrets (primary + reviewer App PEM, webhook, and the three Linear secrets) live in a NEW physically separate `server_secrets` store (mechanism C1: names-only table, bucket-A infrastructure — no `tenant_id`, RLS off, its own GRANT to `compass_app`/`compass_system`; no delivery/kind columns; a SECOND `SpecResolver` instance over it on the shared profile; an admin-gated `SetServerSecret`/`DeleteServerSecret` RPC) so server secrets are STRUCTURALLY undeliverable to agent containers — the container resolver's manifest never contains their names. C1 splits the DECLARATION registries, not the shared provider keyspace, so F1 is a STRUCTURAL reserved-prefix partition enforced on BOTH the admin RPC AND the `authenticatedOpen` user `SetSecret`/`DeleteSecret` path: every server-secret name carries a reserved prefix (`SERVER_` for the six forge secrets, `GATEWAY_CREDENTIALS_` for the master-key family), the admin door REQUIRES one and the user door REJECTS one, so the two keyspaces are disjoint by name and no name is ever live in both tables in either declaration order — a pure string check, no cross-table membership SELECT and so no cross-tenant RLS-visibility question. The table split alone is necessary but not sufficient for the structural claim; the prefix partition is what makes it sufficient. No proto ENUM change and no FetchSecrets filter — the admin RPC adds two additive SecretsService methods (supersedes the earlier SERVER_ONLY-delivery-kind fold). The existing server-secret rows are re-declared into `server_secrets` under their `SERVER_`-prefixed names by a boot-time config-driven declare (idempotent, on the ordinary compass_app path — `server_secrets` is bucket-A, no RLS, so no reconcile and no BYPASSRLS; the pre-production deployment is wiped and re-declared, not migrated in place), closing the pre-existing inject-all exposure. Managed-plane KMS is a provider-URI deployment config, not a code fork; rotation is deferred (single live key, `key_version` landed at v1, bounded key-loss blast radius — credentials are re-obtainable); per-tenant at-rest isolation + gateway-topology exposure is a tracked follow-up | Active (Matt, 2026-09-02) | [gateway-credentials at-rest encryption](server/compass-gateway-credentials-at-rest-encryption.md#d1--application-layer-aes-256-gcm-envelope-encryption) | > Note (2026-07-31, RIG-1570 R5): DL-065's retired internal `ResumeContext > resume = 12` envelope field is NOT silently reintroduced — the collapsed diff --git a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md index e599af12..22a44a3b 100644 --- a/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md +++ b/docs/designs/server/compass-gateway-credentials-at-rest-encryption.md @@ -1042,9 +1042,10 @@ RPC exactly as the frozen record already specifies. Matt's reserved-prefix partition (D6) removed the reconcile entirely — server secrets are re-declared under `SERVER_`-prefixed names on the ordinary compass_app store path (`server_secrets` is bucket-A, no RLS), so this record - adds NO cross-tenant BYPASSRLS site and needs no DL-315 amendment. There is no - DL-326 in this PR (the encryption row is DL-325; the sibling SubjectService PR - takes DL-327). The separate, PRE-EXISTING observation that DL-315's own + adds NO cross-tenant BYPASSRLS site and needs no DL-315 amendment. The encryption row landed as + DL-326 (a concurrent merge of RIG-3070's runner-trust-model split took DL-325; + the sibling SubjectService PR takes DL-327). This record adds no BYPASSRLS site, + so there is no separate DL-315-amendment row. The separate, PRE-EXISTING observation that DL-315's own entrypoint enumeration is stale (the shipped `hub.go:814` forge-notification-ack arm is a BYPASSRLS site DL-315's four names omit — `WithSystemRole`'s own doc comment already lists it, tenant_tx.go:41-47) is not this record's to fix, since