Evaluate auth().isSystem() (cratestack 0.7.10) against the seven-times-repeated hasRole(system) gap and #155's golden test

[stephane-segning]

Summary

cratestack 0.7.10 (just bumped in #TBD) ships PR #500, which adds auth().isSystem() as a first-class policy-term builtin — a cratestack_core::SystemContext is now the only structurally-unforgeable way to mint a CoolContext for which is_system() is true, and auth().isSystem() can name that principal directly in a model's @@allow, for both read (list/detail) and write (create/update/delete) policies.

This is worth a deliberate look here, not a drive-by schema edit, because of how much history this exact gap has in vsms.

Why this matters here specifically

AGENTS.md's "Invariants that fail the build rather than production" section documents a single mistake shape found live seven separate times (App, AppClient, SenderIdRegistration, OperatorPrefixRule #94; Provider, Job #100; Message list/detail #96; DeliveryReceipt #121) and explicitly predicted an eighth: a model whose @@allow omits hasRole('system') doesn't error — CrateStack's list-route policy denial is row-level filtering to an empty array, so an internal system-context read just quietly returns nothing and the caller behaves as though the table were empty.

#155 closed the loop on detection with crates/sms-api/tests/system_context_golden_list_live_postgres.rs: a live-Postgres, seed-then-read golden test that (a) derives the full model list from schema/schema.cstack itself so it can't silently drift, (b) keeps a hand-maintained, justified classification (SYSTEM_READABLE_MODELS / NOT_REQUIRED_TO_BE_SYSTEM_READABLE) that fails loudly the moment a new model isn't classified either way, and (c) actually seeds a row and reads it back under a real system-role context against a real database for every model that's supposed to be system-readable. As of #155, all 13 models an internal system context reads today correctly admit hasRole('system'); the other 6 have no internal reader to break.

auth().isSystem() is a candidate to replace hasRole('system') as the mechanism itself, not just another way to spell the same string:

• hasRole('system') today works because sms-auth's AuthProvider mints a synthetic role: "system" claim for internal callers, and every @@allow clause has to remember to check for it — a convention, not something the type system or CrateStack enforces. auth().isSystem() instead asks whether the CoolContext itself was constructed via SystemContext — per PR #500, "the sole, structurally-unforgeable way to mint a system CoolContext," with an explicit test proving no HTTP request can produce one (http_request_cannot_produce_a_system_context).
• That closes a strictly worse failure mode than the one No invariant guards the hasRole(system) policy gap — found live seven times, an eighth is expected #155 guards: today, if role: "system" ever leaked into a real issued token (AGENTS.md's M1 section already flags this as the one state that must never happen — "system is synthetic and no issued token may carry it"), hasRole('system') policies would trust it. auth().isSystem() can't be spoofed by a claim at all, only by actually holding a SystemContext value, which only sms-api/sms-worker/sms-auth's own internal code can construct.

What this issue is NOT asking for

Not a schema edit. Not a decision to migrate today. This is a research/decision ticket:

1. Does adopting auth().isSystem() fully replace hasRole('system') in schema/schema.cstack, or do both need to coexist (e.g. is there a real distinction in this schema between "the request carries a system role claim" and "this is genuinely internal server code" that's worth keeping separate)?
2. If it's a full replacement: is it a mechanical, one-PR migration (swap every hasRole('system') clause for auth().isSystem(), verify byte-identical DDL per the standing regeneration discipline in AGENTS.md — a policy-only @@allow change never touches DDL), or does it interact with anything non-mechanical (e.g. sms_auth::system_context()'s own construction, Procedures::sys())?
3. Interaction with No invariant guards the hasRole(system) policy gap — found live seven times, an eighth is expected #155's golden test, specifically: system_context_golden_list_live_postgres.rs currently greps schema/schema.cstack's text for the literal string hasRole('system') in its per-model assertions (see e.g. lines asserting "hasRole('system') is absent from Provider's create policy"). If any model's policy is rewritten to auth().isSystem(), those specific string assertions would need updating to match — the live seed-then-read behavior they're proving stays valid (a SystemContext-backed read should still succeed), but the assertion text checking why it should succeed would go stale and either false-fail or (worse) silently stop asserting the right thing if left unchanged. Whoever picks this up should treat No invariant guards the hasRole(system) policy gap — found live seven times, an eighth is expected #155's test file as something this change touches, not something it can leave alone.
4. Whether auth().isSystem() is available for procedure-level policies at all — PR #500's own Scope section says explicitly it is not ("Procedure-level policies... #486's scope and the spike both concern model policies only"), so sms_api::require_permission's Layer 2 checks (#24, sms:send etc.) are unaffected either way.

References

• Upstream: cratestack/cratestack#486 (ADR 0038 blocker B1), cratestack/cratestack#500
• This repo: AGENTS.md's "Invariants that fail the build rather than production" section (the seven-instances history), crates/sms-api/tests/system_context_golden_list_live_postgres.rs (No invariant guards the hasRole(system) policy gap — found live seven times, an eighth is expected #155)

[stephane-segning]

Recommendation: don't adopt now — revisit when a claim-derived role/human-login flow lands

Checked auth().isSystem() against the actual 0.7.10 source (vendored at
~/.cargo/registry/src/index.crates.io-*/cratestack-core-0.7.10/src/context/system.rs
and cratestack-macros-0.7.10/src/policy/model/term.rs), against this repo's own
crates/sms-api/src/auth.rs / procedures.rs, and against #155's golden test. Two
independent things need to be true for adoption to be worth the churn: (1) it has to
actually close the gap it's pitched against, and (2) the cost has to be proportionate.
Neither holds today.

It doesn't touch the bug the issue opens with

The seven (soon eight-if-it-happened-again) repeats were all omission bugs — a
model's @@allow simply never mentioned the system predicate, so CrateStack's
row-filtering denial silently returned an empty array. auth().isSystem() is exactly
as omittable as hasRole('system'): nothing about the new predicate makes it less
likely to be left out of a new model's @@allow. crates/sms-api/tests/system_context_golden_list_live_postgres.rs
(#155) is what actually guards against that shape now, and it does so by seeding a row
and reading it back under a real DB — not by string-matching the schema — so it works
identically regardless of which predicate spelling a model uses. Swapping predicates
doesn't strengthen that guard at all.

One correction to the issue's own sub-question 3, checked directly: the per-model
"hasRole('system')" strings in that test file (e.g. system_context_golden_list_live_postgres.rs:364-365)
are not compared against schema.cstack — they're purely embedded in the assert!
panic message inside the assert_system_can_read_back! macro (lines 315-340). A model
migrated to auth().isSystem() would make that message describe the old clause, which
is misleading if the test ever fails, but it does not make the test silently stop
asserting the right thing — the live seed-then-read behavior it actually checks is
unaffected either way. Still worth fixing the strings for hygiene in any migration PR,
just not the load-bearing risk the issue worried about.

The security scenario it closes isn't live in this codebase

GatewayAuth::authenticate (crates/sms-api/src/auth.rs:257-269) hardcodes
role: "app".to_owned() as a Rust string literal — it is never derived from claims
in any way. AGENTS.md's own comment at that call site says as much: role is "Never
'system' — that role is constructed exactly once, inside Procedures::sys(), never
from a token." So the scenario the issue's own text raises ("an AuthProvider that
mints system for HTTP callers hands out the key that signs every token") describes a
bug this code doesn't have, not one it does.

It's also worth being precise about what auth().isSystem() actually forecloses,
verified against cratestack-core-0.7.10/src/context/system.rs's own tests
(forged_system_field_in_payload_is_ignored, system_flag_does_not_survive_serde_round_trip):
SystemContext closes the wire-forgery vector specifically — nothing in a
deserialized token/claims payload can ever set CoolContext.system = true, because that
field is #[serde(skip)] and private. It does not close the more general "a buggy
AuthProvider calls the wrong constructor" class: SystemContext::for_service(name) is
a public associated function, so a future authenticate() that (mistakenly) does
SystemContext::for_service(&claims.sub).into_context() would compile and mint a
forged system context exactly as easily as today's code minting role: claims.role
would satisfy hasRole('system'). Both mechanisms still require the AuthProvider
implementation to be written correctly; isSystem() narrows which mistake is
dangerous (a deliberate wrong function call vs. a claim-driven field), it doesn't
remove the need for review.

Net: real, but modest and currently inert, defense-in-depth. The moment this
deployment gains a human-login flow or a claim-derived role/permission vocabulary (the
same milestone #97/#98's own scope-cut notes already flag as when Layer 2 becomes
load-bearing) is the moment this protection actually starts mattering.

Migration cost is real, and larger than a schema regex-replace

grep -c "^\s*@@allow(.*hasRole('system')" schema/schema.cstack → 32 @@allow
clauses across 14 distinct models (App, AppClient, ClientAssertion,
DeliveryReceipt, Job, Message, MessagePart, OauthClient, OauthSigningKey,
OperatorPrefixRule, OptOut, Provider, SenderIdRegistration, WebhookAttempt).
No &&-combined clauses exist (checked: grep -n "hasRole('system').*&&\|&&.*hasRole('system')"
returns nothing), so every occurrence is OR'd with the human-role checks — a mechanical
swap on the schema side, and per cratestack-macros-0.7.10/src/policy/model/term.rs's
parse_policy_term, both hasRole('system') and auth().isSystem() compile to
row-filter predicates with no DDL footprint (policy clauses aren't reflected in SQL at
all), so this repo's own standing rule applies: a policy-only @@allow change is not
regenerated, and cratestack migrate diff would be a no-op to confirm, not a step to
skip.

The part the issue's own sub-question 2 undersells: auth().isSystem() only ever
returns true for a CoolContext built through SystemContext, and this repo's
current system-context construction (Principal { role: "system", kind: App, .. }.into_context(),
which goes through CoolContext::authenticated(...)) can never satisfy it —
confirmed by reading cratestack-core-0.7.10/src/context.rs:96-104: authenticated()
unconditionally sets system: false. So this is not "swap the schema and you're done."
Every construction site that mints a system-role context has to switch to
SystemContext::for_service(...) in the same PR, or the two disagree the moment the
schema change lands and every internal system read breaks:

• 6 production call sites: crates/sms-api/src/procedures.rs:183 (Procedures::sys()),
  crates/sms-api/src/auth.rs:161 (system_context()), crates/sms-worker/src/dispatch.rs:35,
  crates/sms-worker/src/scheduler.rs:88, crates/sms-worker/src/jobs.rs:129,
  app/sms-gateway/src/main.rs:366.
• ~16 test-file fn sys() helpers duplicating the same construction, across
  crates/sms-api/tests/*.rs, crates/sms-auth/tests/*.rs, crates/sms-worker/tests/*.rs,
  app/sms-gateway/tests/*.rs, app/sms-worker/tests/kill9_reclaim_live.rs.

None of the OR-combined clauses (e.g. auth().kind == "user" || appId == auth().appId || hasRole('system')
on AppClient's list/detail policy) are at risk from SystemContext's narrower claim
set (service/id only, no sub/kind/appId) — those fields simply resolve absent
for a system caller and the other OR branches are what real request-derived callers
still hit — but it's worth someone actually re-confirming that against a live read per
model before merging, the same standing discipline #155 itself used.

Per this repo's own "hard cutover, not gradual" convention, this has to land as one
coordinated PR touching all ~22 sites plus the schema, not an incremental migration —
that's a bigger, riskier PR than "regenerate the byte-identical migration and move on."

Open questions I could not settle from source alone

• Whether SystemContext::for_service's audit-actor shape (actor.id = "system:<service>",
  per context/system.rs:58-69) changes what sms_api's existing audit rows record for
  internal writes compared to today's Principal-derived sub (e.g. "sms-api:procedures")
  — cosmetic or a real audit-trail format change, a maintainer call either way.
• Whether it's worth doing the call-site cutover to SystemContext for its own sake
  (cleaner call sites, no duplicated fn sys() per file) independent of the schema
  predicate question — that's a real but separate refactor this issue didn't ask about.

What would change my mind: a human-login/richer-role-claim issuance path landing
(making role claim-derived somewhere in the token pipeline, which is exactly the
condition under which hasRole('system') becomes forgeable-by-construction-mistake),
or evidence the duplicated fn sys() helpers have themselves caused a bug — neither is
true today.