feat(#575): durable audience-floor grants + an operator surface for them - #737
Merged
Conversation
The floor, the three guards and the provider are all merged, but the only GrantStore was InMemoryGrantStore. That did not make the feature "not production-ready yet" — it made it actively dangerous, because the floor fails closed. An empty grant table is not "no policy configured", it is "nobody may do anything", so a restart did not degrade the feature: it shut every room. - migration 0035: audience_direct_grants + audience_role_grants - PostgresGrantStore, plus the admin router at /api/v1/admin/audience-grants - AUDIENCE_FLOOR_ENABLED (default off) is the switch Three decisions worth stating, because each fails in a way that looks fine: 1. The store MUST be allowed to throw. GrantStore's contract says a store that cannot answer must throw, because resolveCapabilities converts the throw into an `unresolved` member and closes the floor WITH A REASON. Swallowing a database error and returning [] would hand the floor a well-formed smaller capability set instead — indistinguishable from deliberate policy. So there is no try/catch on the read path, and a test pins that. 2. The service is published as a LATE-BOUND wrapper. The orchestrator plugin reads its services at activation, but graphPool is published BY a plugin during that same pass — so the pool does not exist when the store must be published. Same forward-reference shape the conductor's template registrar uses. An unhydrated store throws for the reason in (1); returning [] would silently narrow every room during boot. 3. Enabling the floor without Postgres refuses to boot rather than running unenforced. A boot refusal names the cause once; the alternative is a deployment that looks configured and behaves as though someone had forbidden everything. No foreign key from audience_role_grants.role_key to conductor_roles(key): #333 phase 2 lets role membership come from an external directory this deployment has no local row for, and a FK would make granting to such a role unrepresentable. Verification: middleware suite 6678 tests, 0 fail (12 pg suites skip without a database); the new pg suite 7/7 against a real Postgres in a throwaway container. typecheck, lint, #470 ratchet (3294) and #573 ratchet (406, unchanged) all green. Mutation-checked, all three died: - directGrants swallowing DB errors -> kills 2, including the end-to-end "a failing store closes the floor instead of shrinking it" - an unhydrated late-bound store returning [] -> kills 2 - lower-casing role keys on read -> kills 1 unit + 1 pg test
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.
The floor, all three guards and the provider are merged. The only
GrantStorewasInMemoryGrantStore.That did not make #575 "not production-ready yet" — it made it actively dangerous, and the reason is the one property this whole cluster is built on: the floor fails closed. An empty grant table is not "no policy configured", it is nobody may do anything. So a restart did not degrade the feature. It shut every room, and kept them shut until somebody re-seeded grants by hand.
What lands
middleware/migrations/0035_audience_grants.sqlaudience_direct_grants+audience_role_grantssrc/audience/postgresGrantStore.tsGrantStore, plus the admin mutationssrc/audience/lateBoundGrantStore.tssrc/audience/routes.ts/api/v1/admin/audience-grants, cookie authAUDIENCE_FLOOR_ENABLEDThree decisions, each of which fails in a way that looks fine
1. The store must be allowed to throw
GrantStore's contract says a store that cannot answer must throw, becauseresolveCapabilitiesconverts that throw into anunresolvedaudience member — which closes the floor with a reason an operator can act on.A store that caught a database error and returned
[]would instead hand the floor a perfectly well-formed smaller capability set. The intersection narrows, the room quietly refuses things, and the operator reads "the floor forbids it" while the actual cause — Postgres was unreachable — leaves no trace anywhere.So there is no try/catch on the read path, that is deliberate, and it is pinned by a test — including end-to-end through
resolveCapabilities, so the property cannot be broken by a well-meaning "let's make this robust" edit two layers away.2. The service is published late-bound
The orchestrator plugin reads the services it consumes at activation, which is why
index.tspublishes them beforeactivateAllInstalled(). ButgraphPoolis published by the knowledge-graph plugin during that same pass. At the moment the grant store must be published, the pool it needs does not exist.Same forward-reference shape the conductor's template registrar already uses: publish a wrapper now, point it at a holder, fill the holder in when
graphPoolresolves. An unhydrated store throws, for exactly the reason in (1) — returning[]there would silently narrow every room during boot, which is the worst possible moment for an invisible failure.The wrapper re-resolves on every call rather than latching the first answer; a test covers that too, since capturing once would pin the floor to what existed at publish time, which is nothing.
3. Enabling the floor without Postgres refuses to boot
Not a crash out of pedantry — the kinder failure. Unenforced would be one thing, but the floor fails closed, so an enabled floor with no durable store means every lookup throws, every room refuses every tool, recalls nothing and reads no attachment, while the deployment looks configured. A boot refusal names the cause once, at the only moment it is still cheap to fix.
Why the admin surface is not behind the same flag
It mounts whenever Postgres is present, independently of enforcement. Grants have to be seedable and reviewable before the floor starts enforcing — otherwise the only way to populate the table is to switch the floor on against an empty one, which is precisely the outage this PR exists to prevent.
Revoking something that was not granted returns 404, not a cheerful 200: the usual cause is a mis-spelled id, and the room's behaviour will not change.
No foreign key on
role_keyaudience_role_grants.role_keydeliberately does not referenceconductor_roles(key).#333 phase 2 made role membership answerable by a registry of sources, and a source may be an external directory this deployment holds no local row for. A foreign key would make "grant a capability to the Entra group everyone in support belongs to" unrepresentable — the exact case the role-source registry exists to serve. The cost is that a typo'd role key is accepted and grants nothing; that fails safe, and the admin list makes it visible.
Scope, stated honestly
Role grants additionally need a role source registered (#333 phase 2). Direct grants work on their own — an empty role registry is a complete answer, not a partial one, so it resolves rather than closing. There is no UI page; this is the API surface the gate needed.
Verification
npm run typecheck✅ ·npm run lint✅ · Epic: Integrated dev platform — isolated per-job runners that clone, develop, test, and ship PRs #470 ratchet 3294 ✅ ·npm run typechecknever typechecksmiddleware/test/#573 ratchet 406, unchanged ✅Mutation-checked — all three died:
directGrantsswallows DB errors, returns[][]Refs #575
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.