Repository navigation
feat(accounts): per-organization envelope keys, and a verified organization on every door (Refs #1013) - #1038
Merged
Conversation
The read and write half of the per-organization key. The schema and the crypto-shredding guard landed with #1033; this is what actually seals under a customer's key. TenantCipher resolves per organization: a binding means that organization's key, no binding means the deployment's — and no binding stays the DEFAULT, because "the customer does not care" is the common case and a deployment that had to provision a key per organization before it could store anything would be worse for everyone. Four decisions worth the review's attention: - **The organization is passed EXPLICITLY, never read from the context.** An implicit read returning "" would seal a customer's credential under this deployment's key, silently, for exactly the customer who went to the trouble of bringing their own. An absent organization is refused; mutation-proven. - **A value is routed by what the ENVELOPE names, not by the binding.** The two legitimately disagree: an organization just given a key still holds credentials sealed under the deployment's, and those must keep opening or granting a customer a key would make their existing credentials unreadable. Tested as its own case. - **A revoked key refuses with ErrKeyRevoked and the recorded reason**, never business.ErrInvalidSecretEnvelope. That sentinel means "re-enter this credential", so reporting a destroyed key through it would tell a customer to reconnect the source whose data they instructed us to destroy. Mutation-proven. - **A binding that cannot be READ refuses** rather than falling through. "The lookup failed" and "there is no binding" are different answers, and conflating them seals a customer's data under our key. The keyed hash never moves: API-key lookup is BY the hash, and the lookup is what discovers which organization the key belongs to, so a hash under an organization's key could never be found. The Cloud KMS sealer bound to an organization's key refuses MAC outright rather than producing a value nothing will match. Both backends gained SealerFor. Vault needed more than a parameter: its ciphertext carries a key VERSION and not a key NAME, so a per-organization key could not be told from the deployment's on read. Its payload now carries an optional leading key segment — base64url contains no ":", so the pre-per-key shape is unambiguous and, because the deployment's own key still writes exactly that shape, nothing already stored moves. The MAC key is a separate field from the envelope key for the same reason: one may move, the other may never. Sealers are cached per key REFERENCE, not per organization: two organizations on one key share one, and building one is a round trip to the key service. Not wired into business yet, so nothing writes a per-org key through a request path; that is the next commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on's scope (Refs #1013) Reconsidering the previous commit's "passed explicitly, never read from the context", which was the right mechanism defended with the wrong reason — and which left a hole. The reason was wrong: I was objecting to LENIENCY, not to implicitness. An implicit read that refuses when there is no verified organization has none of the downside I was guarding against, and "the system already resolves this, so do not restate it" is the principle this repository applies everywhere else. The mechanism survives anyway, for a reason that is actually about the data: - the delivery paths have no request scope at all. The outbound sender opens a subscription's signing secret as app_webhook_worker, outside any tenant transaction, so there is no verified organization to read — the row is all there is. - a value's organization is a property of the ROW, not of whoever is asking. Sealing under whatever organization the current request happens to be scoped to would be a different question that agrees most of the time, which is the worst kind of coincidence. So it stays an argument, but it is now documented as the row's own organization (sub.OrgID, source.OrgID) rather than something threaded down, which is what it always was at every call site. **The hole that reasoning hid:** an argument independent of the transaction's scope can DISAGREE with it, and row-level security binds the scope, never the argument — so nothing in the stack caught a row sealed for one organization inside a transaction scoped to another. requireConsistentScope refuses that now, both directions, on seal and on open, while still admitting the no-scope delivery case. The scope reader is injected as a variable so this package does not import the request-identity plumbing and a test can drive both answers. Mutation-proven: accepting a disagreeing scope fails the new test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… transport (Refs #1013) wool's organization was stamped in one place — the gRPC interceptor's stampRequestIdentity — so the six other paths that install a verified identity carried none: a delegated record read, a module parent, a delegated read audience, readable sources, and the work-context authority twice. Anything downstream that asks whose request this is got nothing on those paths. Binding it inside WithVerifiedDatabaseIdentity covers all seven, and covers them at the one door that has already verified the pair. That matters more than the coverage: wool's identity keys are EXPORTED, so anything may set them, and the dimension is only worth reading if the writers are known. A test derives the writer set from the syntax tree and pins it at exactly two, with each one's reason, so a third — especially one fed by caller-controlled input — fails rather than quietly making the dimension forgeable. Two writers rather than one, because the two ids have different availability. The verified door binds nothing without BOTH uuids, by design: database scope needs both. A verified user who has not selected an organization yet is an ordinary request that binds no scope, and their logs and rate-limit key should still name them — so the user id stays stamped unconditionally in the interceptor. Deduping that was a regression, caught by the gateway-rotation tests in pkg/adapters, and it is why the split is now written down. This does NOT make wool an authority source, and nothing here reads identity from it to decide anything. pkg/auth states that transport headers and wool values are deliberately insufficient database or authorization-cache authority, and the verified identity's context key stays private for that reason. The direction is one-way: verified identity -> wool, for observability and for the organization-defaulted reads that already consult it. Upstream, codefly-dev/core#743 fixes why this has to use the exported keys directly: wool.Wool.WithOrgID cannot propagate at all — `with` assigns to the Wool's own ctx and Get returns a fresh Wool per call, so the derived context is discarded. Proven against the pinned core before writing it this way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
antoinetoussaint-byte
added a commit
that referenced
this pull request
Oct 8, 2026
…privileged mutation, complete revocation, usable keys only, and a correct migration choice R1019-N02. requireMFA was made fail-closed for a privileged actor, but the privilege-granting and platform-security mutations did not reach it: each ran its authorization guard and then its mutation. A helper nothing calls enforces nothing. The role, scope and principal grants and the platform-admin mutations (suspend, unsuspend, session revoke, entitlement override) now gate a factor, AFTER the authorization guard and BEFORE the mutation — a check the write has already passed is not a check, which is the shape the first attempt at this produced and the source test below now forbids. UpsertFeatureFlag is deliberately not gated: it mutates nothing, answering FailedPrecondition for a retired inventory. Ordinary tenant mutations — dashboards, teams, invitations, notifications, user settings — are deliberately not gated either; whether a second factor belongs there is a product decision about an operator's whole roster, not a property of this host. NOT done, and recorded in module/FEATURES.md rather than left implied: the method policy still declares MFA_REQUIREMENT_NONE for these methods, so the existing policy/handler lockstep gate is satisfied either way and a later edit could remove a gate without reddening anything. Aligning the declarations is the durable fix. It is a descriptor change — it moves the contract digests and the published client — and the proto generator needs a container runtime this environment could not start, so it wants its own verified regeneration. The stopgap is explicit: a test that reads the source and requires the gate, before the mutation, for every method on a named list. R1019-N04. Logout reported success when the family revocation succeeded and an access-token revocation did not, which tells a caller their session ended when an issued access token stays usable until it expires. Every revocation is now required; the family revocation runs last so a failed marker cannot discard the durable half, and each step is idempotent so a retry completes the remainder. An incomplete sign-out reaches the caller as a retryable failure with a fixed reason. R1019-N03. The datasource keys accepted the placeholder this module ships in its public local defaults and stayed usable. A deployed runtime now treats an unusable value — marker-carrying, or below the signing floor — as ABSENT, which leaves the purpose unavailable: the state minting and redemption already define, and the reason this does not refuse at boot the way a perimeter credential does. Refusing to start would turn an unprovisioned optional purpose into an outage. R1019-N08. The identity-provider migration treated the destination group's shipped placeholder as provisioned, so it overrode a real value still delivered under the old group — defeating the fallback precisely on the cells that had not migrated, which are the only ones it exists for. Selection now requires an operator-supplied value, excluding every shipped marker, and falls back to a provisioned legacy value with the migration warning. Local development still resolves to its shipped default. Every fix is mutation-checked, each with a positive control. The factor test counts how many calls actually reached the gate per platform grade, so a grade refused earlier by its role floor cannot pass vacuously. Rebase note (onto origin/main f5a104c). This commit also corrected a KEY_ROTATION.md paragraph to say that splitting the shared Vault transit key per purpose was this repository's work and not done. Main's #1033 and #1038 have since done it, behind a selected key service with per-organization envelope keys, so the paragraph now points at those sections instead of claiming an open item. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
antoinetoussaint-byte
added a commit
to crisfiorini-oss/module-saas-starter
that referenced
this pull request
Oct 12, 2026
…oses codefly-dev#1018) (codefly-dev#1019) * fix(accounts): an OAuth state is accepted once, and is refused when single use cannot be recorded (SA-F-STATE) The state signer promised single use and dropped the promise whenever its nonce store could not answer: a validly-signed, unexpired state was admitted again, any number of times, for as long as the outage lasted. The provider's own single-use authorization code is a second barrier, not a substitute — it is the provider's guarantee, not this host's, and this signer's contract is that a state is accepted once. Verify now returns the canonical invalid-state error when the consumer errors, logging the cause so an outage is still diagnosable while the caller learns only that the state is invalid. TestL1OAuthStateReplayRefusedWhenNonceStoreFails verifies twice, because the behaviour restored is not 'a replay is refused' but 'nothing is admitted while single use is unrecordable'. Its positive control asserts a working store still admits the first use, so the refusal is the store's absence and not a signer that refuses everything. Mutation-checked: restoring the admit makes it fail and leaves the control green. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): a refused refresh and a refused OAuth begin answer a stable public reason (SA-F-REFRESH500) The session store answers one indistinguishable rejection for every refresh it can refuse — unknown token, revoked family, expired or idled-out session, reuse — so that it is not an oracle. The adapter then returned the business layer's wrapped error unmapped, which undid that twice: the response named the internal call path, and a refusal was reported as a server error, so an expired refresh cookie, the most ordinary thing a browser can present, arrived as HTTP 500 and counted against availability. RefreshToken now maps the two refusal sentinels to unauthenticated with a fixed reason and logs the cause. Anything that is not a refusal still passes through as itself: a store that cannot be reached is a genuine server-side failure, and dressing it as a credential problem would send an operator to the wrong half. BeginOAuth is reached unauthenticated and echoed the same wrapped chain; it now answers invalid-argument for a refused provider or redirect and failed-precondition when the signer is unavailable, with the cause logged. Mutation-checked: restoring either echo fails the new tests, and the infrastructure-failure control stays green under the fix, so the mapping cannot be satisfied by collapsing every failure into a credential error. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): the reserved module audience is mintable only from a module identity (SA-F-WC-RESERVED) Every peer that verifies the `module-capabilities` audience reads a context carrying it as a composed module acting with no person present, and skips the questions it would ask of a delegated actor on the strength of that. The mints a signed-in person reaches placed no restriction on the audience, so a person could mint or exchange for the reserved spelling. Nothing downstream granted them anything — an agent id cannot equal a module principal id — but the premise peers rely on was enforced nowhere, and a property two services away is not a boundary. Every mint and exchange that takes its audience from a caller's message now refuses the reserved one, before the caller's authority is resolved: the task mint, the installation mint, a root and a child session, the audience exchange and a renewal. The refusal also sits inside the exchange helper, because the internal delegated-read exchange reaches it directly with an audience a composition declared, and a declaration is no more entitled to the reserved spelling than a request is. TestEveryRequestSuppliedAudienceRefusesTheReservedOne reads the source and requires the refusal in every method that reads an audience off a request, so a mint added later cannot skip it silently; an ordinary audience passes the guard, so the suite cannot be satisfied by a server that refuses everything. Mutation-checked: removing the guard fails all six mints and the coverage test. module/WORK_CONTEXTS.md gains the section stating the invariant. Rebase note (onto origin/main f5a104ce3). Main landed this invariant independently, as requireMintableAudience: the same refusal of the reserved audience on the same mints, with PermissionDenied rather than InvalidArgument, an empty-audience refusal beside it, and a closed-vocabulary check after it. Its call sites replace the ones this commit added, so what remains here is this finding's own tests — re-pointed at main's refusal — and the WORK_CONTEXTS.md section. The source-level test is the part worth keeping: it holds every caller-supplied mint to the gate and requires the gate's error to be RETURNED, so neither deleting a call site nor discarding its result passes. It covers the two entry points an unauthenticated caller cannot reach — the installation mint, which demands an internal service credential first, and a renewal, which gates the audience it RESOLVES after inheritance. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): a privileged actor's sensitive operation requires a second factor, enrolled or not (SA-F-MFA) requireMFA looked for an enrolled device, found none, and admitted. So the one principal whose compromise is the whole platform or the whole tenant was the one the gate could not touch: a platform administrator or an organization owner with nothing enrolled passed every sensitive operation it guards — role grants, GDPR delete, impersonation, entitlement overrides, waitlist review. For a privileged actor — any platform role, or ownership of the organization the verified identity names — the absence of recent AAL2 evidence is now a refusal. That is also what makes 'a privileged principal holds a second factor' enforceable at all: the alternative to enrolling is not reaching these operations. Organization ADMIN is deliberately outside the rule, and the reason is recorded beside the code: admin is a role an owner grants and withdraws, where owner carries the tenant's own lifecycle, and sweeping admin in would change the gate for a population no operator chose. For every other member the gate stays opt-in per user; making it unconditional there is an operator's policy decision about their roster, not a property of this host. The organization is read from the verified identity the auth interceptor installed rather than from a request field or a log value, so it is the same tenant the membership read is scoped to. TestRequireMFARefusesUnenrolledPrivileged covers all three platform grades and the org owner; its control asserts a plain member, an org admin and a membership-less caller still pass, so the refusal is the privilege rather than a gate that refuses everybody. Mutation-checked. The impersonation rig carried MFAVerifiedAt with no assurance level and no method, so it never had recent AAL2 and passed only on the old admit. It now stamps genuine AAL2 evidence, which is what a console must present. module/FEATURES.md: the two rows that described the old opt-in-only behaviour. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): the refresh credential is a cookie on every authentication-completing surface, and signing out ends the session (SA-F-LOGOUT, SA-F-REFRESH-BODY) Both findings are one reworking of the same thing — which surface hands the refresh credential to whom — so they land together rather than as two passes over the same file. SA-F-LOGOUT. The browser's logout posts an empty body and its HttpOnly refresh cookie. The middleware moved that cookie into the request body for refresh and not for logout, so field validation rejected the request before Logout ran. The session family stayed active, the access-token id was never revoked, and the device list still showed the session — while the browser's own cookie was cleared, so the sign-out looked complete and ended nothing. A refresh token copied earlier kept minting access tokens for the rest of the idle window. The cookie is now moved into the body for logout exactly as for refresh. An explicit body token still wins: the cookie is the browser's carrier, not an override. Logout additionally revokes the VERIFIED session's access half, from the credential the caller presented rather than from the message, so the access side of a sign-out does not depend on the body at all. Not taken here, and stated rather than left to be inferred: LogoutRequest's refresh_token keeps its min_len=1, so a logout carrying neither a body token nor the cookie is still refused by field validation. Making it optional is a contract change that moves the descriptor digests and the published client, which is not worth stapling to this PR; the product's own sign-out does not need it. SA-F-REFRESH-BODY. The cookie lift covered four REST paths named as literals. The WebAuthn completion was not one of them — it shares its response message with the TOTP completion, and only the TOTP path was listed — so it returned the credential as readable content. The Connect surface of the same methods returned it too, serving the same browser over the same origin. The REST set is now held against the service descriptor by TestEveryAuthCompletionCookiesTheRefreshToken, in both directions: every POST route whose response message carries refresh_token must be lifted, and nothing may be listed that no route returns one on. The Connect handlers route through unaryCookieingRefreshToken, which clears the field by reflection and sets the same cookie, and a source-level test requires it of every handler whose response carries the field. One exemption, declared by name with its reason rather than simply absent: the OAuth 2.1 token endpoint. Its caller is a registered client exchanging a code or a refresh token over a back-channel POST, RFC 6749 §5.1 defines that response as a body with refresh_token in it, and a Set-Cookie there reaches nothing that could use it. Both coverage tests require the exemption set to be exactly that one entry. While here, the refresh cookie's Secure flag became a property of the deployment instead of the request. It was derived from the Host header, which a caller supplies, so a request arriving at a deployed cell with Host: localhost was answered with a cookie the browser would then send over plaintext. The local-development exemption is now wired from codefly.IsLocal() beside SetExposeAuthErrorDetail, and its zero value keeps Secure. TestL1LogoutForwardsCookieRefreshToken is the register's logout reproducer, inverted. Mutation-checked: dropping logout from the cookie-consuming set, the WebAuthn path from the lifted set, and the lift from one Connect handler each fail their test, and the Secure-flag test is checked in both directions. module/FEATURES.md: the logout rows and the refresh-rotation row. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): an identity assertion in the request body is honoured only in header-jwt mode (SA-F-HEADERJWT) The login route's header-jwt branch read the same validator field the authorization-code exchange uses, and every provider mode wires that field. So a deployment configured for OIDC accepted a request body carrying `header_jwt` and handed the token to the OIDC validator — which checks a signature, an issuer, an audience and an expiry, and no nonce. The nonce is the code exchange's binding to a single authorize request and lives in the state that exchange holds, so the body path completed a sign-in with no state, no proof-of-possession verifier and no nonce: exactly the id_token injection and replay the nonce exists to stop. The precondition was holding any unexpired id_token for the configured client. The header-jwt validator is now its own field, set only by work.go's header-jwt branch. In every other mode it is nil and the credential is refused before any validator sees the token. TestL1VHeaderJWTBodyRefusedInOIDCMode is the verifier's reproducer, inverted, and asserts the code-exchange validator was never consulted rather than only that the call failed. Its counterpart wires the header-jwt validator and asserts the same credential IS validated there, so the refusal is the mode and not a path that refuses everything. Mutation-checked: pointing the branch back at the shared field fails both. identity.env records which credential each provider mode accepts, beside the key that decides it. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: a deployed service refuses a shipped placeholder perimeter credential, by name (SA-F-PLACEHOLDER) This module ships working local defaults for its secret groups, out of a public repository, and the configuration-defaults gate requires each of them to carry a placeholder marker so a real credential can never be committed as one. Nothing read that the other way round: a value carrying a marker is a value everybody knows, and no deployed runtime rejected one. An unprovisioned secret group was therefore not a denied group — it was a live credential with a published value, and the perimeter rested on mesh authorization alone. accounts and auth-gateway now refuse to start, outside local development, when a credential that decides perimeter membership carries a shipped marker or is shorter than 32 characters. The refusal names the group-qualified key an operator must provision and never prints a value. The length floor is not a second spelling of the marker check: it also refuses a short hand-typed value carrying no marker, which is the other way a published-strength credential arrives. Scope is deliberate. Only the cluster-internal token, its rotation predecessor and the gateway-provenance token are checked — the values a service trusts to decide that a caller is inside. An optional feature's unprovisioned credential is a different question with a different answer, the feature is off, and conflating them would wedge a cell that simply does not use one. A coverage test pins the checked set in each service, so a fourth trust-deciding key cannot be added unchecked while looking checked. The marker list and the floor are duplicated in two Go modules with no shared local package, so module/tools/shipped_placeholder_lockstep_test.go holds all three copies identical. A marker on only one side is invisible in the diff that causes it and fails in one of two ways: a published value a cell still accepts, or a shipped default that side can no longer boot on. Mutation-checked by removing one marker from accounts. module/AGENTS.md: the paragraph that said nothing rejects a shipped placeholder, replaced by what now does and what deliberately still does not. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth-gateway): rate limits key on the real client, and a deployed gateway refuses to start without the stores its controls need (SA-F-RATELIMIT, SA-F-REVOKE-NOOP) Three controls degraded into something that read as working, and two findings cover them. They land together because they are one rule: a control whose dependency is absent refuses rather than reporting success. SA-F-RATELIMIT, this repository's half. An empty TRUSTED_PROXY_CIDRS was accepted. With no trusted hop clientIP answers the peer, which behind the frontend is the frontend's own pod: every anonymous caller shares one budget and every authentication-factor attempt shares one, so roughly ten bogus MFA completions a minute deny the factor to the whole cell and a thousand anonymous requests a minute deny login to all of it. A denial of service delivered by the control meant to prevent one, and invisible: the limiter was enabled, the buckets were enforced, and nothing said they had collapsed onto one key. A deployed gateway now refuses an empty range, naming the key; local development, where the peer IS the client, still accepts it. The limiter also fell back to per-replica in-memory counters whenever its store was absent or unreachable. That is not the budget it reports: with N replicas the effective limit is N times the configured one, and which replica a caller lands on decides their share, which for an authentication-factor budget is the difference between a bounded number of attempts and a bounded number per replica. Deployed: refuse. The per-replica limiter is now a separate constructor rather than a flag on the main one, so a deployed wiring cannot reach it. Not done, and stated rather than left as a gap: the authentication-factor budget is not additionally keyed on the MFA transaction. SP-GW-09 asks for the originating client, which is what it now keys on, and accounts' own five-attempt transaction lock already bounds one transaction across every replica. Adding the dimension means buffering and parsing the request body at the edge, a new failure mode on the login path for a bound that already exists one hop in. The platform repository owns setting the range on every cell and forwarding the ingress hop. SA-F-REVOKE-NOOP. With no store URL the gateway installed a no-op revoker and a log line. That revoker reports every token as live, so a signed-out session, a killed device and an ended impersonation window all keep authenticating until their tokens expire naturally. Nothing on the request path can tell a no-op revoker from an empty revocation set, so the gateway reports healthy throughout. The fail-closed behaviour on a store ERROR was already right; this is the same rule for a store that was never there. Tests assert both directions in each case: the deployed refusal, and that local development still gets its fallback. The collapse itself is asserted as a property, two clients behind one frontend pod keying distinctly with a trusted range and identically without one, so the boot refusal's stated reason is checked rather than claimed. Mutation-checked: disabling each of the three refusals fails its test. Docs: a new section in module/services/auth-gateway/AGENTS.md on what a deployed gateway refuses to start without, and gateway.env now says what an empty range costs beside the key. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth-gateway): a cross-origin grant on a forwarded response is the gateway's own (SA-F-CORS) accounts' generated CORS handler hands an empty allowlist to a library that reads empty as allow-all, so it answers a wildcard origin with credentials true. The gateway forwarded those headers verbatim on every path where it had granted nothing, so the public surface granted a cross-origin read the gateway had granted nobody, and an empty allowlist, which should grant nothing, granted everything. Browsers refuse a credentialed read under a wildcard, which bounded the reach to anonymous responses, but the fail-closed-by-name property was gone. Every proxied response is now wrapped, granted or not: the wrapper strips access-control-* before stamping, and with no grant it strips and stamps nothing. It already stripped where a grant replaced the upstream's; the missing half was the much more common path where there was no grant to replace. Vary is still only added with a grant, so the public surface's cache entries are not fragmented by an origin the response does not depend on. What this does NOT fix, and who owns it: accounts' generated cors_gen.go and connect_gen.go, where the empty allowlist becomes allow-all and the value is read with a raw environment accessor. Both are generated by the go-grpc agent and are not this repository's to hand-edit; the defect is in that template. accounts' listener is reachable in-mesh, so the generated handler's answer still stands there — the gateway closes the public surface, which is what SP-GW-11's probe reads, and the template owns the rest. Tested through the gateway with an upstream that answers a wildcard with credentials: for an unregistered origin and for no origin at all, no access-control header leaves the gateway, and the request itself is unaffected. A registered origin's grant replaces the upstream's rather than joining it, and allow-credentials never appears whatever the upstream said. Mutation-checked. module/services/auth-gateway/AGENTS.md records the rule and names the template as the owner of the other half. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth-gateway): a module or solution upstream receives the person's identity, not their session (SA-F-BEARER) A host access token is the person's whole session: one host-wide audience, their full authority, valid at every other upstream and at the host's own API. The gateway forwarded it, and any cookie, unchanged to every composed module and solution. So a compromised module or solution pod, a single logged request header, or an upstream that redeems the token to mint contexts for further audiences each yielded replayable full-authority sessions for every viewer who had used that upstream. proxyTo now removes Authorization, Cookie and Proxy-Authorization for every runtime-registered route, module and solution alike, including the public solution asset surface. A catalog route is untouched: accounts is the host's own API, where the session is the credential, and a test asserts that so the strip cannot be satisfied by forwarding nothing to anybody. An upstream receives what it should have been receiving all along: the identity ext_authz stamps — subject, tenant, session, credential kind, scope ceiling — which names the person without carrying their authority. THIS BREAKS AN UPSTREAM THAT READS THE FORWARDED BEARER, by design. The register names two consumer repositories that do, and the audit filed their half separately: one reads the bearer in its chat handler and re-mints Work Contexts with it, and the solution runtime's authenticated fetch relies on it. Their replacement is a host-minted capability bound to the upstream's own audience — the Work Context surface — not a bearer borrowed from a person. Sequencing is an operator decision and is listed in the pull request rather than decided here. The companion half of the same invariant, a per-resource audience in place of the single host-wide one, is PR #1007's (issue #1003): it binds a solution tool endpoint's token to that endpoint's resource. This change is what stops the generic session reaching the upstream in the first place. TestModuleUpstreamReceivesNoSessionBearer and TestPublicSolutionPathForwardsNoCredential are the catalogue's two tests, and the second is the verifier's reproducer. Mutation-checked. The pre-existing assertion that a solution receives the bearer is inverted rather than deleted, so the change in contract is visible in the diff. Docs: module/services/auth-gateway/AGENTS.md gains the rule and names the consumer obligation; module/SOLUTION_REGISTRATION.md's upstream-constraints section no longer describes a forwarded user bearer. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: the verified public origin is operator configuration, never a request header (SA-F-ORIGIN) The frontend's own rendered endpoint is a loopback placeholder — which is what the render produces, so on every cell — and the frontend fell back to the caller's X-Forwarded-Host and stamped it, under the internal token, as the VERIFIED public origin. accounts then bound an OAuth redirect, the authenticator relying-party origin and (with no application base URL, as on a cell) emailed links to a host the caller had chosen. The solution proxy's same-origin check compared the browser's Origin against the same caller-supplied value, so a caller that supplied both sides matched itself. Three independent changes, because the origin is decided in three places and any one of them deciding it from a request is the whole defect. The frontend no longer takes a request origin at all: resolveCodeflyGatewayContext dropped the parameter. The order is the configured origin, then the SDK-discovered own endpoint when the render carries a real host, and on a deployed build nothing else — a loopback placeholder with nothing configured refuses, naming application/APP_BASE_URL, rather than asking the caller. Local development keeps its loopback endpoint, which is the host the browser reaches anyway; only the build tells the two apart, and nothing a caller sends does. The solution proxy compares the browser's Origin against that configured origin, and refuses when there is none: nothing to compare against is not a reason to admit. The origin is resolved separately from the trust context, so a deployment missing the internal token is still a different condition from one with no verified origin — conflating them would refuse a same-origin request over a credential nobody asked about. accounts records a forwarded origin as verified only when it equals the configured one. That is the barrier that does not depend on the frontend: the frontend holds the internal token legitimately, so a credential check alone could never tell a forwarded origin from a forwarded choice. A cell without APP_BASE_URL is a cell whose verified origin is a request parameter, so accounts refuses to start without it. publicRequestOrigin and requestPublicOrigin are gone, with the test that pinned the first. The solutions-surfaces route resolved the host origin a client fetches a remote's modules from the same way, and now reads the configured one. Tests: the configured origin wins over a render that carries a real host; a deployed build with a loopback placeholder refuses by name; local development keeps the loopback endpoint; near-misses of the configured origin (a subdomain, a suffixed host, another port, another scheme) are each refused in accounts; a forged cross-site request supplying its own forwarded host is refused by the proxy; a deployment with no configured origin refuses every proxied request. Each mutation-checked, and the positive controls — a same-origin POST behind a TLS-terminating ingress, an unpinned local runtime accepting a well-formed origin — assert the refusals are the pin rather than a blanket no. Owned elsewhere, and not worked around here: the CLI renders the frontend's own endpoint as a loopback placeholder (which is why the fallback existed), and the platform's ingress should overwrite X-Forwarded-Host for product hosts. Docs: application.env says what APP_BASE_URL binds and that a deployed runtime requires it. PR #1007 adds the same key to that file for the OAuth issuer; whichever lands second rebases. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(frontend, marketing): the product and marketing origins pin HTTPS (SA-F-HSTS) The edge redirects plaintext to TLS, which protects the redirect and nothing before it: a browser's FIRST request to a host it has never pinned goes out in the clear, carrying whatever the address bar or a link gave it, and an attacker on the path answers it. The origin has to tell the browser to remember, and neither origin did. Both now emit Strict-Transport-Security with a one-year max-age, in the frontend's baseline header set (so every route carries it, like the framing and nosniff headers beside it) and in the marketing site's own header block. Deliberately WITHOUT includeSubDomains or preload, with the reason recorded beside the value: both commit hosts this service does not own — a sibling subdomain served over plaintext breaks under the first, and the second is effectively irreversible — so they are the ingress operator's to add once they know the host tree. Their absence is asserted, so it reads as a decision rather than an omission. Browsers ignore the header on a plaintext origin, so local development is unaffected. The e2e security-headers spec asserts it on real responses alongside the framing and CSP checks it already made. The infrastructure repository owns emitting it at the ingress if an operator prefers it there; emitting it from both is harmless (the stricter policy wins) and this is the half that holds whatever fronts the service. Also records, in module/services/frontend/AGENTS.md, what SA-F-REMOTECODE asks for and why this pull request does not do it: nothing pins what a cross-origin manifest serves, and closing that means either dropping the cross-origin-remote shape (and the runtime origin derivation, its listing fetch, cache, outage reporting and policy-size guard, which exist only to carry foreign origins — 17 tests' worth of covered behaviour) or adding a publication-time digest the registration contract does not have. Both are decisions about the solution contract rather than defects to repair, and the register's own note is that the live policy lists no foreign origin and every deployed remote is same-origin today. Left for its own change with the owner's decision, rather than taken unilaterally inside a security pull request. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: the identity provider's own credentials live in a group only accounts declares (SA-F-IDPKEY, SA-F-GWTOKEN this repo's half) A declared configuration group is injected whether the service reads it or not, so a credential's blast radius is the set of services that DECLARE its group. Which group a credential lives in is therefore the security decision, and two findings are the same mistake about it. SA-F-IDPKEY. The frontend declares `identity` because it renders the login page from that group's public facts — the issuer, the client id, the display name, the scope, all of which the browser receives anyway. A deployment that puts the identity provider's own credentials in that same group hands them to the frontend too: a public-facing process holding the provider's administration credential and client secret, reading neither, which widens what a frontend compromise is worth for nothing. IDENTITY_CLIENT_SECRET and IDENTITY_MANAGEMENT_API_KEY now live in a new `identity-provider` secret group that only accounts declares. The read falls back to `identity` while a cell still delivers them there, and warns once per key naming the move: which group a value arrives under is the composition's to change, and a hard cutover would take sign-in down on every cell between this landing and that edit. The warning is how an operator learns a cell has not moved; the fallback goes when they have. SA-F-GWTOKEN, this repository's half. The finding is that the gateway-provenance credential — which makes accounts believe a forwarded identity, so a holder can assert any user — is bundled with the internal token and delivered to twelve workloads that never read it. The delivery is the platform repository's: the secret group, and the twelve external secrets rendered from it. What this repository owes is that its own composition cannot express the bundling, and that the trust site reads only the narrow group — which it already did, unheld by anything. module/tools/credential_group_scope_test.go now holds both: which services may declare each credential-bearing group (`gateway-trust` only the identity plane, `identity-provider`, `github-app` and `vault` only accounts), and that no group other than `gateway-trust` declares a GATEWAY_TOKEN key. The second is the other way a credential spreads — not a service declaring a group it does not read, but the key appearing in a second group nearly everything declares, under a name that makes it look like the same credential. Both gates are mutation-checked, and the first fails on exactly the live shape: adding `identity` to the credential list names the frontend. Each refuses an empty holder set too, so a group going dead cannot leave the gate proving nothing. module/AGENTS.md records the rule and why the identity group was split. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): the datasource message-authentication keys are delivered per purpose, not derived from the perimeter credential (SA-F-KEYREUSE this repo's half) The datasource content-ticket key and the account-link state key were both derived from CODEFLY_INTERNAL_TOKEN — the cell-wide perimeter credential every service and composed module holds. Two consequences, neither visible from either end: every holder of that credential could compute both keys and forge what they authenticate, and rotating the perimeter credential silently invalidated every outstanding content ticket and link state, a blast radius nothing recorded. Both now come from a new `datasource-keys` secret group that only accounts declares, one delivered key per purpose, so rotating one rotates one thing. An absent key leaves its purpose unavailable rather than falling back to a derivation. A key reachable from a credential thirteen workloads hold is not a key, and a fallback would make the fix conditional on nobody forgetting — so the operator step is real: a cell must provision both keys before its datasource paths work again, and until it does, minting a content ticket is refused rather than signed with something weaker. The link key stays domain-separated from whatever value it is given, so a content ticket and a link state can never pass for each other even where an operator provisions one value for both. Tests: an absent key in either direction leaves its purpose unavailable and does not borrow the other's, and one provisioned value for both still yields two distinct keys. The pre-existing unset-key test is kept and widened rather than replaced, so what it caught before is still caught. NOT this repository's, and named rather than left implied: one Vault transit key still serves every sealing purpose, which the key-service work owns, and the documents module derives its collection-cursor key from the same internal token in its own tree. module/KEY_ROTATION.md gains the section stating the rule, what it costs to rotate one of these, and what is still outstanding. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: the call sites a build tag and an untyped transform hid Two call sites my local verification could not see, both found by CI on the first push and both the same shape: a tool that skips files rather than checking them. `pkg/business/service_test.go` still called `Service.Logout` with the pre-change arity. It is a DB-gated file, so `-tags pure` — the way these suites run without Docker — does not SKIP it, it does not COMPILE it: `go build`, `go vet` and the whole pure suite were green while the call site was wrong. One missing argument reddened `Codefly quality` compile, lint and test together, and reported auth-gateway and frontend as skipped on a failed prerequisite. `src/test/pipeline-gateway.test.ts` built a `CodeflyRuntimeReader` literal without the two members the public origin now depends on. vitest transforms with esbuild and does not typecheck, so 3876 tests passed over it; `tsc --noEmit` is what sees it. The literal is replaced by a helper that carries the whole interface, so the next member added to it fails in one place rather than two. Both are verification failures on my side rather than design changes, so neither alters behaviour. `GOWORK=off go vet ./...` now passes untagged in all six Go modules — which compiles every test file without running any, needing no database — and `npm run typecheck` passes. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): the review's four accounts majors — a factor at every privileged mutation, complete revocation, usable keys only, and a correct migration choice R1019-N02. requireMFA was made fail-closed for a privileged actor, but the privilege-granting and platform-security mutations did not reach it: each ran its authorization guard and then its mutation. A helper nothing calls enforces nothing. The role, scope and principal grants and the platform-admin mutations (suspend, unsuspend, session revoke, entitlement override) now gate a factor, AFTER the authorization guard and BEFORE the mutation — a check the write has already passed is not a check, which is the shape the first attempt at this produced and the source test below now forbids. UpsertFeatureFlag is deliberately not gated: it mutates nothing, answering FailedPrecondition for a retired inventory. Ordinary tenant mutations — dashboards, teams, invitations, notifications, user settings — are deliberately not gated either; whether a second factor belongs there is a product decision about an operator's whole roster, not a property of this host. NOT done, and recorded in module/FEATURES.md rather than left implied: the method policy still declares MFA_REQUIREMENT_NONE for these methods, so the existing policy/handler lockstep gate is satisfied either way and a later edit could remove a gate without reddening anything. Aligning the declarations is the durable fix. It is a descriptor change — it moves the contract digests and the published client — and the proto generator needs a container runtime this environment could not start, so it wants its own verified regeneration. The stopgap is explicit: a test that reads the source and requires the gate, before the mutation, for every method on a named list. R1019-N04. Logout reported success when the family revocation succeeded and an access-token revocation did not, which tells a caller their session ended when an issued access token stays usable until it expires. Every revocation is now required; the family revocation runs last so a failed marker cannot discard the durable half, and each step is idempotent so a retry completes the remainder. An incomplete sign-out reaches the caller as a retryable failure with a fixed reason. R1019-N03. The datasource keys accepted the placeholder this module ships in its public local defaults and stayed usable. A deployed runtime now treats an unusable value — marker-carrying, or below the signing floor — as ABSENT, which leaves the purpose unavailable: the state minting and redemption already define, and the reason this does not refuse at boot the way a perimeter credential does. Refusing to start would turn an unprovisioned optional purpose into an outage. R1019-N08. The identity-provider migration treated the destination group's shipped placeholder as provisioned, so it overrode a real value still delivered under the old group — defeating the fallback precisely on the cells that had not migrated, which are the only ones it exists for. Selection now requires an operator-supplied value, excluding every shipped marker, and falls back to a provisioned legacy value with the migration warning. Local development still resolves to its shipped default. Every fix is mutation-checked, each with a positive control. The factor test counts how many calls actually reached the gate per platform grade, so a grade refused earlier by its role floor cannot pass vacuously. Rebase note (onto origin/main f5a104ce3). This commit also corrected a KEY_ROTATION.md paragraph to say that splitting the shared Vault transit key per purpose was this repository's work and not done. Main's #1033 and #1038 have since done it, behind a selected key service with per-organization envelope keys, so the paragraph now points at those sections instead of claiming an open item. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth-gateway): only a final response is sanitized (R1019-N09) R1019-N09. A 1xx is informational: net/http sends it and keeps the same header map open for the real response, which the reverse proxy then fills from the upstream. Stamping on it marked sanitization complete, so the final upstream headers went out untouched. An early hint is not an answer, and only a final response is stamped now. Tested through real local HTTP servers, because the behaviour is net/http's handling of an early hint and a recorder does not reproduce it. Mutation-checked in both directions. Rebase note (onto origin/main f5a104ce3). R1019-N05 is gone with its subject. It concerned a process-local module prefix-claim cache standing in for a durable store; #953 removed runtime module registration altogether, and a module route is now resolved per request from the delivered registry (gateway_module_routes.go, g.solutions.resolveModuleRouting), which is shared and durable by construction and answers 503 when no snapshot has loaded. There is no cache left to track an authority with. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): a usable Connect refresh carrier, a public reason for a failed refresh, and one origin spelling (R1019-N07, N10, N12) R1019-N07. Removing the credential from the Connect response body left a carrier that worked in one direction: a Connect client had nowhere to read it from, and the refresh and logout calls that need it failed field validation before reaching the verifier. The Connect refresh and logout now consume the credential from a Cookie header when the message does not carry one, before dispatch — after validation it would be too late — and an explicit body credential still wins, the same precedence the REST middleware uses. The cookie's path scope is a deliberate choice and is now written down beside it: `/v1/auth` is the REST auth surface and NOT the Connect procedure prefix, so a browser does not attach the cookie to a Connect call. Widening it to `/` would attach the refresh credential to every request this origin serves, including the ones the frontend proxies onward. The defined journey is therefore: authenticate over Connect, refresh and sign out over REST, where the cookie is in scope — with the Connect calls still available to a caller that supplies the credential itself. R1019-N10. Only the recognized refusals were sanitized, so a host-side failure returned its wrapped chain, naming the internal call path and the store's address. A failure that is not a refusal now answers a stable retryable reason with the cause logged. A deliberate status from further in keeps its own reason and is returned WITHOUT its wrapper — returning the error unchanged would carry the internal chain out alongside the good reason, which is the same defect one layer along. R1019-N11. Renewal's guard saw only the audience the REQUEST named, so an omitted one passed it and then inherited the parent's. The effective audience — the one that gets signed — is now checked after inheritance and before signing, and a source test holds that ordering. R1019-N12. An explicit default port is the same origin as none, and the two sides spelled it differently: a browser and the frontend drop it, a configured value may carry it. Both sides now go through one canonicalization, applied when the configured origin is installed as well as when a forwarded one arrives. A non-default port still distinguishes an origin, asserted in both directions so the rule does not collapse genuinely different origins. Each mutation-checked, each with a positive control. Rebase note (onto origin/main f5a104ce3). R1019-N11 — re-checking the audience a renewal EFFECTIVELY signs, rather than the one the request named — is closed by main. RenewWorkContext resolves the inherited audience into a local and gates that value with requireVocabularyAudience, once, after the inheritance and before signing. That is what this finding asked for, in one gate instead of two. The ordering is held from the source by TestR1019RenewInheritedAudience. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: the eight fix-removal mutations that survived the review (R1019-N15) The review ran 26 representative fix-removal mutations and found eight surviving the tests I had landed. A mutation that survives means the test suite does not hold the fix, whatever the suite's result says, so each survivor gets a test that fails when the fix is removed. - **Gateway boot wiring.** Every deployed refusal is only a refusal if main wires it, and removing a call site left every unit test green while a deployed gateway would start on a published credential, with no shared store for revocation, rate limits or prefix claims, and with no trusted-proxy range. All five wirings are now asserted from the source, each matched with codefly.IsLocal() specifically — wiring one to a constant is the same defect as not wiring it. - **Marketing HSTS.** The marketing service had no header test at all; it has one now, covering the transport pin, the absence of includeSubDomains and preload, and the framing and sniffing headers beside them, so removing any of the four is detected rather than only the newest. - **Datasource key independence.** The existing test asserted only that the two keys DIFFER, which a link key derived from the ticket input still satisfies — so the two purposes could share one secret again, which is the whole of the finding. The test now varies one input at a time and requires the other purpose not to move. - **Logout session revocation, Connect response lifting, identity-provider reader selection and datasource configuration** are covered by the tests landed with their own findings; each was re-checked here by removing the fix and confirming red. Also corrects a claim of mine the review found overstated: I wrote that a deployed wiring cannot reach the in-process rate limiter. The option exists and any caller could pass it. What holds is narrower and is what the new source assertion actually establishes — the only production call site passes codefly.IsLocal(), and production neither hard-enables the option nor uses the per-replica constructor. Refs #1018 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: a required perimeter credential must be present, and the marketing header test must compile (R1019-N16, N17) N16: tests/security-headers.test.ts imported next.config.mjs with no declaration, so the import was implicitly `any` and its callbacks were implicit-any too. The project compiles with allowJs disabled, so the configuration carries no types of its own; tests/next-config.d.ts declares the shape the tests read. Both `npm run typecheck` and the compile gate were failing on it. The test also accepted any value CONTAINING `max-age=`, which `not-max-age=` does. A header value is a list of directives, so the assertions now read the parsed list and a directive counts only as its own member. Both mutations — renaming the directive, and adding includeSubDomains/preload — now fail. N17: refusePerimeterCredential returned nil for an empty value in both services, on the reasoning that whether a credential is required belongs to each loader. No loader asked, so a deployed cell could start holding no perimeter credential at all. A perimeterCredential now carries whether it is the CURRENT value or a rotation's PREVIOUS half: an absent current credential is refused by its group-qualified name, and an absent rotation half still starts, because a cell that is not mid-rotation carries none and refusing that would make the ordinary state an outage. The accounts suite asserted the defect as correct — NoError with nothing provisioned at all — so that assertion is now the refusal, with the rotation halves still admitted beside it. The new coverage goes through requireStartupConfiguration, which is what main calls, and carries a positive control so an unconditionally refusing startup path cannot pass it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: one spelling of an origin on both sides, a refusal that cannot read as success, and the rest of the pre-fix prose (R1019-N10, N12, N01) N12: the frontend canonicalizes with the WHATWG URL parser and this service with net/url, which is laxer in four ways — host case, an IPv6 literal's form, a Unicode hostname, and a host-less authority. Each produced a browser-equivalent spelling that compared unequal to the configured origin, and the refusal read as a misconfiguration. All four are settled in canonicalPublicOriginHost, and the port in canonicalPublicOriginPort: a leading zero and an empty explicit port normalize, and a port outside 1-65535 is refused rather than ignored. Port 0 is the one deliberate divergence from the parser, which accepts and keeps it. Nothing can be served on port 0, and the frontend's canonicalOrigin now applies the same range check, so the two agree. The vectors are the agreement. Both sides assert behaviour on the same table — expected values taken from the parser rather than reasoned about — and module/tools/public_origin_vectors_lockstep_test.go holds the two copies identical, because a case fixed on one side and left failing on the other is two green suites and a deployed origin that refuses itself. N10: being a status does not make its message public-safe. Pass-through is now restricted to codes that answer the CALLER; a code that answers "this host failed" carries whatever the failing layer said, so it is logged and replaced. OK is outside the set deliberately: Status.Err() is nil for OK, so admitting one returned a nil error beside a nil response and a caller would have read a success that never happened. status.Error(codes.OK, "") is itself nil and cannot be injected, so the test writes the carrier out. N01: the review named two comments; the same pre-fix narrative was in seven more places in this diff, each describing the mechanism rather than the invariant. All rewritten. One note about startup ordering is kept — it describes what a late check cost a bring-up, not a bypass. Nothing is force-pushed; the Disclosure list in the PR body covers what is already published. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(accounts): a factor at principal creation and revocation, and a source gate that cannot be satisfied by a comment (R1019-N02, N15) Minting a principal creates a credential-bearing actor and revoking one is an irreversible security action, so both are privilege-granting mutations and both reached their write with no factor evidence — for a platform super-admin and for an organization owner alike. Both now gate, after the authorization guard and before the mutation. RevokePrincipal resolved the actor inside each branch of its authorization split, which is how a gate comes to be added to one branch and not the other; it is resolved once before the branch now. DisableAgentPrincipal and EnableAgentPrincipal are deliberately not gated: they admit on an internal service credential, where there is no human actor and so no factor to require. The source-level gate was satisfied by two mutations the review found (N15). It required requireMFA to APPEAR; discarding its error left the call in place and enforced nothing, and a comment carrying the expected text satisfied the substring search while the guard was gone. It now parses the file, strips every comment, and matches only the form that binds, tests and returns the same variable — compared by captured name, since RE2 has no backreferences. It also anchors the ordering to the MUTATING call rather than the first service call in the body, because RevokePrincipal reads the principal first to decide which authorization applies. The RPC-level tests record the writes instead of panicking, so "the mutation was reached" is an assertion rather than a crash — reaching the write without a factor is the finding, and a test has to be able to say it happened. FEATURES.md said ordinary tenant mutations are not gated and left it there. The grant paths admit an organization ADMIN, who is not privileged, so requireMFA applies its opt-in rule and an ENROLLED admin without recent evidence is now refused where nothing asked before. That is intended — granting authority inside a tenant is not an ordinary tenant mutation — but it was undocumented and untested. Both halves are now asserted: enrolled is refused, unenrolled is still admitted, so no roster is forced to enrol by this change. The containment actions' freshness requirement is recorded for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: the eight mutations that still survived, and the stripper that was silently a no-op (R1019-N15) Each surviving mutation named a check that could not tell enforcement from the appearance of it. The Connect response lift: every assertion either called cookieTheRefreshToken directly or read the source for the WRAPPER'S NAME, so removing the call from inside the wrapper left the helper working when a test called it and the handlers still naming a wrapper that no longer called it. There is now one test that drives the wrapper itself, and it is the only one that fails when that call goes away. The datasource keys: every assertion was about the DERIVATION — that two keys differ, that a placeholder is refused — and none about where the value comes from, so substituting another provisioned secret changed nothing any test looked at. The internal token passes every other check in usableDatasourceKey, so the group is the only thing that distinguishes it, and that is what is now asserted: blank in its own group with the perimeter credential provisioned, the purpose must be unavailable. The gateway's deployed placeholder validation is now reached by the N17 tests, which call requirePerimeterCredentials(false) directly rather than only through pure comparisons. The ignored-error mutations (GrantScope, the inherited audience, the gateway startup check) all left the call in place and discarded its result. The source checks now match only the form that binds, tests and RETURNS the same variable, compared by captured name since RE2 has no backreferences. The comment mutations are why source checks now read the file with comments stripped. That stripper was itself a no-op for its first revision: parser.ParseFile leaves File.Comments EMPTY without parser.ParseComments, so it stripped nothing while looking like it worked, and the mutation it was written for was caught only by the return-form rule. It now strips — by Position().Offset rather than offsets derived by hand — and a test proves it removes a comment and keeps the code, against a file written for the purpose: asserting against its own file would compare its own assertion literals, and a needle in both can never be shown absent. Marketing's HSTS assertion accepted any value CONTAINING max-age=, which not-max-age= does; it reads the parsed directive list now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: the precise blocker behind each finding this change leaves open (R1019-N06, SA-F-INSTALL) Both are blocked on a missing contract rather than a missing check, and saying so without saying which contract leaves the next person to rediscover it. For SP-GW-07's second half: ExchangeAudience re-binds an EXISTING Work Context, and the caller at the edge holds a browser session, which is not one — so what is missing is a mint from a verified session to a context bound to one upstream's audience. It is also per request on the proxy path, so its latency and failure mode become those of all federated traffic, and the authority to narrow to is the installation ceiling the host cannot currently look up. For the installation check: InstallationService can install, uninstall, transfer and GetInstallation BY INSTALLATION UUID, and nothing answers "does this organization have an installation of the solution with this id" — which is the question the proxy has to ask. SOLUTION_REGISTRATION.md §4a now also states what does bound that surface today, so the document cannot be read as claiming an installation check it does not make. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: one validator for the public origin, and a resource-bound tool token is the exception that proves SP-GW-07 (r2 f1, f2, f3, f4, f5, f7) f1 (blocker). APP_BASE_URL was validated at boot by url.Parse plus field rules and at request time by IDNA canonicalization. An origin the first accepted and the second could not canonicalize made a cell boot GREEN and then refuse every authentication — sign-in, the OAuth redirect, the relying-party origin, emailed links, the solution proxy's same-origin check — because SetConfiguredPublicOrigin had silently stored a value no canonicalized candidate could ever equal. The fix that matters is that boot now validates with the SAME function the request path uses, so a second validator cannot drift back in. Measured case: `https://my_app.example`. The raw spelling is still what boot RETURNS. It is also the token `iss` and the published authorization-server metadata, where a verifier compares it exactly, so canonicalizing it there would silently change `iss` for any deployment whose origin carries an explicit :443. Only the comparison canonicalizes. f1 also had a second cause: idna.Lookup enforces DNS hostname policy, which refuses an UNDERSCORE that browsers accept and internal hostnames routinely carry. The profile is now MapForLookup without StrictDomainName, so that spelling is accepted on both sides. idna.Punycode would have been the wrong relaxation: it does no case folding, so "BÜCHER.example" and "bücher.example" encode to DIFFERENT punycode and could never compare equal. A vector now pins that. f2. The setter returns an error and the call site refuses. Storing the raw value on a canonicalization failure looked like the conservative branch and was the opposite: it turned a diagnosable configuration error into an opaque per-request refusal for the life of the process. f3. The shared vectors omitted exactly the spellings where the two implementations disagreed, so all three tests passed while the sides answered differently — the lockstep gate proves the two TABLES match and can say nothing about a case absent from both. The underscore host is now accepted on both sides; a hyphen-edged label, which the parser accepts and IDNA refuses, is now refused on both, by an explicit label check on the frontend. Both files say the table is the full extent of the agreement. f4. The disposition claimed #1007 was unmerged. It merged as 056803fe2, and reading it showed the opposite of both my claims: it CLOSES the finding for `/solutions/<id>/mcp` — ext_authz answers 401 to a token that names only the host — and leaves `/solutions/<id>/mcp/` open, because the predicate matched the segment exactly. Routers, proxies and clients treat those as one endpoint, and MCP's own transport addresses sub-routes under it, so the whole `mcp` subtree is now the protected resource. Reconciling that with SA-F-BEARER is the substantive change here, and it was a live contradiction the rebase surfaced: my stripping broke the merged MCP contract, whose runtime mints the viewer's Work Context from the forwarded token. The two are not in conflict once the credential is named correctly. A HOST-WIDE session is still removed from every upstream. A RESOURCE-BOUND tool token is forwarded, because ext_authz refuses an unbound token on a tool path, so a tool request reaching the proxy has already been proved to carry an audience naming that solution and nothing else — which is the audience-bound credential SP-GW-07's positive half asks for. Read off the path, not re-derived from the token: admission already proved it, and a second JWT parse would be a second verifier to keep in agreement with the first. f5. The field comment that still advertised the derivation SA-F-KEYREUSE removed. Named in the previous review and carried; it now names the delivered key and its group. f7. executableSource moved to source_assertions_test.go (three test files use it, and it lived in the one named for a single finding); the four datasource-key tests moved to datasource_keys_test.go. Every fix carries the test that fails without it, and each was mutation-checked individually. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * credential issuance needs a factor, a sign-out ends the family, and an origin spelling both services keep The third review's three majors and one of its minors, each at the place that owns the behaviour. CREDENTIAL ISSUANCE REQUIRES A FACTOR (SA-F-MFA, the half that was open). CreateAPIKey ran the interactive-session, scope, mint-ceiling and organization-admin gates and then minted. An owner with a verified session and no enrolled factor could ask for `*:*` and get a credential that authenticates as them for as long as it lives, without the session that asked for it and across a password change — which is what a factor on a privileged mutation protects. Both halves, because they described different hosts: the handler now runs requireMFA after the authorization gates and before the mint, and the DECLARED policy says MFA_REQUIREMENT_IF_ENROLLED_RECENT_STEP_UP where it said MFA_REQUIREMENT_NONE. The declaration is what the authorization-coverage gate and the generated gateway catalog read. Regenerated with CI's PINNED CLI (0.1.179, checksum-verified against the release manifest) rather than the newer local one, after the no-change run AGENTS.md asks for came back clean; the diff is the two descriptor copies, the contract digests and the catalogs, with no facade-import or stamp churn. A SIGN-OUT ENDS THE WHOLE REFRESH FAMILY. Logout revoked the family durably and wrote its access-revocation marker under the CURRENT session id. Every refresh mints a new session id and keeps the family, so an access token minted before the last refresh carried a different `sid`, matched no marker, and stayed valid until its natural expiry: a token stolen before a refresh survived the sign-out that followed it. The family is the identifier that does not rotate, so the access token now carries it (`fid`) and the sign-out revokes it. Both sides check it — accounts' VerifyAccess and the gateway's ext_authz — and a new module/tools gate holds the claim names together, because the two are separate Go modules with no shared type and a rename on one side would silently stop the other finding it. An access-only impersonation window has no family and stamps nothing, so one marker cannot reach every such token at once. AN INCOMPLETE SIGN-OUT NO LONGER DESTROYS ITS OWN RETRY. The 503 that says "logout incomplete, retry" was answered while the middleware cleared the refresh cookie on every logout response — the one answer that asks for a retry was also the one that destroyed the credential a retry needs, and the frontend swallowed the failure and rendered a finished sign-out. The cookie is now cleared only on success, logout() reports whether it completed, and the sign-out page surfaces an incomplete one with a retry instead of redirecting to the login page. Local state is still cleared either way: the person asked to sign out, and leaving them signed in there would neither end the session nor tell them anything. ONE ORIGIN SPELLING BOTH SERVICES KEEP (SA-F-ORIGIN, R1019-N12's remainder). A numeric spelling of an IPv4 address is read by the WHATWG parser as an address and re-rendered — `2130706433`, `0x7f000001`, `127.1` and `0177.0.0.1` all come back as `127.0.0.1` — while Go's net/url preserves what was written. Configuring either side with one gave the two services different values from the same configuration, so every same-origin comparison failed and sign-in refused. Both sides refuse it now, by name on the accounts side, and the four spellings are in the shared vector table. The frontend rule is narrowed to exactly that rewrite — the parsed host is a dotted quad and not what was written. My first attempt refused any hostname the parser changed, which also refused the two rewrites the two sides AGREE on: IPv6 compression and IDNA encoding. Three vectors caught it. Also here: the registration document is taken from main, which drops the conflict markers this branch committed, the registration procedures #953 deleted, and a section claiming the proxy cannot require an installation that main's admission check contradicts. The SDK's contract digest moved with the policy value, so @codefly-dev/saas-sdk goes 0.3.17 -> 0.3.18 in the package, the dependent's pin and the lockfile. Its public TypeScript API is unchanged: the only difference in the generated bindings is one byte of the embedded descriptor. Mutation-checked, each individually: removing the factor call or discarding its error; reverting the declaration; not writing the family marker; ignoring the family at verification; stamping one shared family id for everyone; clearing the cookie on every logout again; and dropping the numeric-host rule on either side. Every one fails its test, and the controls assert the opposite direction — an unenrolled admin still proceeds, another family's session survives a sign-out, a completed sign-out still clears the cookie, and the agreed rewrites still canonicalize. Refs #1018 Co-Authored-By: Claude Opus 5 (1M …
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1013. Deliberately not
Closes— the business wiring is still owed,see "What is still owed".
Builds on #1033, which landed the key-service seam, both backends, the cutover,
the re-seal verb, and migration 30's
org_key_bindingstable with thecrypto-shredding guard. This is the half that actually seals under a customer's
key.
Per-organization keys
TenantCipherresolves per organization: a binding means that organization'skey, no binding means the deployment's — and no binding stays the default,
because "the customer does not care" is the common case and a deployment that
had to provision a key per organization before it could store anything would be
worse for everyone.
Four decisions worth review attention, all tested, three mutation-proven:
now. The two legitimately disagree: an organization just given a key still
holds credentials sealed under the deployment's, and those must keep opening —
otherwise granting a customer a key would make their existing credentials
unreadable.
ErrKeyRevokedand the recorded reason, neverbusiness.ErrInvalidSecretEnvelope. That sentinel means "re-enter thiscredential", so routing a destroyed key through it would tell a customer to
reconnect the source whose data they instructed us to destroy.
lookup failed" and "there is no binding" are different answers, and conflating
them seals a customer's data under our key.
security binds the scope, never the argument, so a row sealed for one
organization inside a transaction scoped to another would otherwise go through
silently. The no-scope case is still admitted, because the outbound webhook
sender opens a subscription's secret as
app_webhook_workeroutside any tenanttransaction, with only the row to go on.
The keyed hash never moves off the deployment's key: API-key lookup is by the
hash, and that lookup is what discovers the organization, so a hash under an
organization's key could never be found. A Cloud KMS sealer bound to an
organization's key refuses
MACoutright rather than producing a value nothingwill ever match.
Vault needed more than a parameter
Its ciphertext carries a key version, not a key name — so a
per-organization Vault key was indistinguishable from the deployment's on read,
and Vault answers a foreign key's ciphertext with the same failure it gives a
corrupt one. Its payload now carries an optional leading key segment; base64url
contains no
:, so the pre-per-key shape is unambiguous, and because thedeployment's own key still writes exactly that shape nothing already stored
moves. The MAC key became a separate field from the envelope key for the same
reason: one may move, the other may never.
Sealers are cached per key reference, not per organization — two
organizations on one key share one, and building one is a round trip to the key
service.
A verified organization on every door
wool's organization was stamped in one place, the gRPC interceptor, so the six
other paths that install a verified identity carried none (delegated record
access, module parent, delegated read audience, readable sources, work-context
authority ×2). Binding it inside
WithVerifiedDatabaseIdentitycovers all seven,at the one door that has already verified the pair.
A test derives the writer set from the syntax tree and pins it at exactly
two, with each one's reason. The keys are exported, so anything may set them,
and the dimension is only worth reading if the writers are known — a third,
especially one fed by caller-controlled input, fails rather than quietly making
it forgeable.
Two rather than one because the ids have different availability: the verified
door binds nothing without both uuids (database scope needs both), while a
verified user who has not selected an organization is an ordinary request that
binds no scope and should still be named in logs and the rate-limit key.
Deduping that was a regression, caught by the gateway-rotation tests.
This does not make wool an authority source, and nothing here reads identity
from it to decide anything.
pkg/authstates that transport headers and woolvalues are deliberately insufficient database or authorization-cache authority,
and the verified identity's context key stays private for that reason. The
direction is one-way.
Companion upstream PR
codefly-dev/core#743 —
wool.Wool.WithOrgIDcannot propagate at all:withassigns to the Wool's own ctx andGetreturns a fresh Wool per call, sothe derived context is discarded. Proven against the pinned core before writing
this against the exported keys instead. That PR adds the package-level form that
works; this one does not depend on it landing.
What is still owed
The business wiring:
EncryptTenantSecret/DecryptTenantSecretreachingthe six organization-scoped call sites, each handing over its row's organization,
plus the
org_key_bindingsread inpkg/infraandVerifiedScopewired to theverified identity. Nothing writes a per-organization key through a request path
yet, which is why this does not close #1013.
Also raised and not done here, because it is a DB-layer change with its own RLS
test surface:
WithOrgTxtakes an explicit organization at every call site, sothe caller-vs-scope mismatch is representable everywhere and only some paths
guard it. The better shape is a regular accessor that derives the organization
from the verified caller and a separate privileged one that takes it explicitly —
which would make this PR's scope check redundant by construction.
Test plan
TenantCiphercases: deployment default, the organization's own key,a value predating the binding, revocation, absent organization, the
per-key cache, an unreadable binding, the keyed hash
delivery case
through, and accepting a disagreeing scope each fail their test
nothing, and the two-writer gate
go vet ./...untagged,-tags puresuites,./pkg/keyservice, accountsroot,
./pkg/adapters, boundary and naming gateswedged at a migration from another session's agents, so
./pkg/infraand./pkg/businesshave not run on this branch. CI grades them on a cleandatabase.
🤖 Generated with Claude Code