From 3bb30f05ec1dfd861916ed6f48e83beede643eff Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Tue, 1 Sep 2026 15:14:24 -0400 Subject: [PATCH] test(oauth): assert the ceiling invariant over a corpus, not examples Each case names the production change that makes it fail; all were checked against a deliberately broken rule first. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: I14030d7404320503fc17d5d271347d34bb9893c6 --- crates/didbot-serve/src/oauth/scope.rs | 409 +++++++++++++++++++++++++ 1 file changed, 409 insertions(+) diff --git a/crates/didbot-serve/src/oauth/scope.rs b/crates/didbot-serve/src/oauth/scope.rs index 98cd8c92..83117f78 100644 --- a/crates/didbot-serve/src/oauth/scope.rs +++ b/crates/didbot-serve/src/oauth/scope.rs @@ -899,6 +899,415 @@ impl fmt::Display for ScopeSet { } } +/// The authorization boundary, tested as a property rather than as a table +/// of examples. +/// +/// `plan/adversarial.md`'s class: a ceiling check is not interesting where +/// the request is plainly inside or plainly outside, it is interesting where +/// the request is *adjacent* to what the ceiling holds — one label further +/// down an NSID, one action wider, one byte over a size cap, a different +/// `aud`. The corpus below is built out of those adjacencies, and the tests +/// assert invariants over every pairing of it, so a new atom kind added to +/// the grammar is covered by construction rather than by somebody +/// remembering to add rows. +/// +/// Every test here names the production change that makes it fail. The one +/// order-dependence bug this module has already had came from a ceiling +/// atom being visited in a particular order, which no single example can +/// see. +#[cfg(test)] +mod ceiling_boundary { + use super::*; + + /// Atoms chosen so that many pairs are adjacent rather than disjoint: + /// prefixes at each depth, a wildcard, enumerations that overlap + /// partially, sizes on either side of each other, and both `aud` + /// states. + const CORPUS: &[&str] = &[ + "atproto", + "transition:generic", + "transition:chat.bsky", + "transition:email", + "repo:*", + "repo:app.bsky.*", + "repo:app.bsky.feed.*", + "repo:app.bsky.feed.post", + "repo:app.bsky.feed.like", + "repo:app.bsky.*?action=create", + "repo:app.bsky.feed.post?action=create,update", + "repo:app.bsky.feed.post?action=delete", + "repo:chat.bsky.convo.message", + "rpc:*", + "rpc:com.atproto.repo.*", + "rpc:com.atproto.repo.createRecord", + "rpc:com.atproto.repo.createRecord?aud=did:web:one.example", + "rpc:com.atproto.repo.createRecord?aud=did:web:two.example", + // A *bounded* wildcard on each optional field. These are the atoms + // that make `Scope::intersect`'s own match arms reachable at all: + // when one side plainly contains the other the two `contains` + // short-circuits at the top of `intersect` answer first, and + // `intersect_aud`/`intersect_size`/`ActionSet::intersect` are never + // called. A corpus without them tests the short-circuits and + // nothing else. + "rpc:com.atproto.repo.*?aud=did:web:one.example", + "rpc:*?aud=did:web:two.example", + "repo:*?action=create", + "repo:app.bsky.*?action=create,delete", + "blob:*/*", + "blob:image/*?maxSize=1000", + "blob:*/*?maxSize=500", + "blob:image/*", + "blob:image/png", + "blob:image/png?maxSize=1000", + "blob:image/png?maxSize=999", + "identity:*", + "identity:*?action=manage", + "identity:handle", + "identity:handle?action=manage", + "account:email", + "account:email?action=manage", + "account:status", + ]; + + fn corpus() -> Vec { + CORPUS + .iter() + .map(|atom| Scope::parse(atom).expect("corpus atom parses")) + .collect() + } + + /// Every pairing of the corpus, as (request, ceiling) sets of one atom + /// each, plus a handful of multi-atom ceilings — the shape where a + /// ceiling atom that grants one action and a sibling that grants + /// another must both survive. + fn pairs() -> Vec<(ScopeSet, ScopeSet)> { + let atoms = corpus(); + let mut out = Vec::new(); + for request in &atoms { + for ceiling in &atoms { + out.push(( + ScopeSet::new(vec![request.clone()]), + ScopeSet::new(vec![ceiling.clone()]), + )); + } + } + for request in &atoms { + out.push(( + ScopeSet::new(vec![request.clone()]), + ScopeSet::new(atoms.clone()), + )); + out.push(( + ScopeSet::new(atoms.clone()), + ScopeSet::new(vec![request.clone()]), + )); + } + out + } + + /// **The invariant this module exists for.** Nothing a ceiling hands + /// back may admit a request the ceiling itself would not admit. Stated + /// over every atom of every granted set, against every atom of the + /// ceiling. + /// + /// Fails if: `Scope::intersect` ever returns a scope wider than the + /// ceiling side — `intersect_size` taking `max` instead of `min`, + /// `ActionSet::intersect` unioning instead of intersecting, + /// `intersect_aud` dropping a `Some` in favour of `None`, or an + /// `NsidPattern::intersect` that widened `Exact` to its `Prefix`. + #[test] + fn nothing_granted_is_outside_the_ceiling() { + for (request, ceiling) in pairs() { + let Ok(granted) = request.intersect(&ceiling) else { + continue; + }; + for atom in &granted.0 { + assert!( + ceiling.0.iter().any(|c| c.contains(atom)), + "granted `{atom}` is admitted by no atom of ceiling `{ceiling}` \ + (request `{request}`, whole grant `{granted}`)" + ); + } + } + } + + /// The other half: a grant may narrow a request but never widen it. An + /// app that asked for `repo:app.bsky.feed.post` must not walk away + /// holding `repo:app.bsky.*` because the ceiling happened to be wider. + /// + /// Fails if: `Scope::intersect`'s two `contains` short-circuits at the + /// top ever return the *ceiling* side rather than the narrower one — + /// which is a one-character edit and exactly the kind of thing that + /// reads as correct. + #[test] + fn nothing_granted_is_outside_the_request() { + for (request, ceiling) in pairs() { + let Ok(granted) = request.intersect(&ceiling) else { + continue; + }; + for atom in &granted.0 { + assert!( + request.0.iter().any(|r| r.contains(atom)), + "granted `{atom}` was not asked for in `{request}` \ + (ceiling `{ceiling}`, whole grant `{granted}`)" + ); + } + } + } + + /// Narrowing is a fixpoint: re-applying the same ceiling to what it + /// already granted changes nothing. A ceiling that kept narrowing on + /// each pass would mean the grant on a token response is not the grant + /// a later check would compute. + /// + /// Fails if: intersection stops being idempotent — most plausibly by + /// `ScopeSet::new`'s subsumption pass dropping an atom it should keep, + /// so the second pass sees a different set than the first produced. + #[test] + fn re_applying_the_ceiling_changes_nothing() { + for (request, ceiling) in pairs() { + let Ok(granted) = request.intersect(&ceiling) else { + continue; + }; + let again = granted + .intersect(&ceiling) + .expect("what a ceiling granted is inside that ceiling"); + assert_eq!( + again, granted, + "re-applying ceiling `{ceiling}` to `{granted}` changed it to `{again}` \ + (request `{request}`)" + ); + } + } + + /// A ceiling is a set of policy records with no inherent order, so + /// reversing it must not change what it grants. The module has already + /// had one order-dependence bug; this states the property over the + /// whole corpus rather than over the three atoms that bug happened to + /// involve. + /// + /// Fails if: `ScopeSet::intersect` goes back to taking the first + /// overlapping ceiling atom rather than the union over all of them, or + /// `ScopeSet::new`'s subsumption pass becomes order-sensitive. + #[test] + fn reversing_the_ceiling_grants_the_same_thing() { + let atoms = corpus(); + for request in &atoms { + let request = ScopeSet::new(vec![request.clone()]); + let forward = ScopeSet(atoms.clone()); + let mut backward_atoms = atoms.clone(); + backward_atoms.reverse(); + let backward = ScopeSet(backward_atoms); + + assert_eq!( + request.intersect(&forward).map(|g| g.to_string()), + request.intersect(&backward).map(|g| g.to_string()), + "ceiling order changed the grant for `{request}`" + ); + } + } + + /// The degenerate ceiling. An empty ceiling is what + /// `RefuseAllScopePolicy` returns and what a deployment with no policy + /// records has, and it must admit nothing at all — the scope analogue + /// of the empty operator secret, where "the operator configured + /// nothing" must never read as "the operator configured everything". + /// + /// Fails if: `ScopeSet::intersect` ever treats an empty ceiling as + /// unrestricted, or as a set that vacuously admits. + #[test] + fn an_empty_ceiling_admits_nothing() { + let empty = ScopeSet::default(); + for atom in corpus() { + let request = ScopeSet::new(vec![atom.clone()]); + assert!( + request.intersect(&empty).is_err(), + "the empty ceiling admitted `{atom}`" + ); + } + // And an empty *request* against an empty ceiling is a grant of + // nothing rather than a refusal: there is no atom to refuse. + assert_eq!(empty.intersect(&empty), Ok(ScopeSet::default())); + } + + /// Adjacency, where a wildcard's meaning is easiest to get wrong: a + /// ceiling naming one thing must not admit its neighbour, or a name + /// that merely shares a textual prefix with it. This is the same + /// label-boundary rule `plan/adversarial.md`'s zone containment section + /// is about, on the other grammar in this codebase that has one. + /// + /// Every pair here is genuinely *disjoint* — there is no request both + /// admit — so the ceiling must refuse outright rather than narrow. The + /// overlapping cases are the next test's, because "refuses" and + /// "narrows to exactly this" are different claims and running them + /// together is how a narrowing that is secretly a widening gets missed. + /// + /// Fails if: `under_prefix` drops its `.` check and becomes a bare + /// `starts_with`, or `resource_contains`/`aud_contains`/`MimePattern` + /// gain a prefix or substring test, or `Transition::covers` widens. + #[test] + fn a_ceiling_refuses_the_scope_next_door() { + for (ceiling, refused) in [ + // A textual prefix that crosses no label boundary. + ("repo:app.bsky.feed.*", "repo:app.bsky.feedgen.post"), + ("rpc:com.atproto.repo.*", "rpc:com.atproto.repository.list"), + // A sibling. + ("repo:app.bsky.feed.post", "repo:app.bsky.feed.like"), + // A different audience. + ( + "rpc:com.atproto.repo.createRecord?aud=did:web:one.example", + "rpc:com.atproto.repo.createRecord?aud=did:web:two.example", + ), + // Disjoint action sets. + ( + "repo:app.bsky.feed.post?action=create", + "repo:app.bsky.feed.post?action=delete", + ), + // MIME halves are independent, and neither is a prefix test. + ("blob:image/png", "blob:image/jpeg"), + ("blob:image/*", "blob:video/mp4"), + // A different named resource. `identity:`/`account:` resources + // are opaque strings in this grammar, so a resource whose name + // merely *starts with* a granted one is exactly the input a + // prefix comparison would wave through — and nothing in the + // grammar stops a client asking for one. + ("account:email", "account:status"), + ("identity:handle", "account:email"), + ("account:email", "account:emailConfirmed"), + ("identity:handle", "identity:handleHistory"), + // `transition:` covers what it covers and no more. + ("transition:chat.bsky", "repo:app.bsky.feed.post"), + ("transition:chat.bsky", "repo:chat.bskyfoo.convo"), + ("transition:email", "account:status"), + ("transition:generic", "identity:handle"), + ("transition:generic", "account:email"), + // The base scope grants no capability of its own. + ("atproto", "repo:app.bsky.feed.post"), + ("atproto", "rpc:com.atproto.repo.createRecord"), + ] { + let ceiling = ScopeSet::parse(ceiling).expect("parses"); + let request = ScopeSet::parse(refused).expect("parses"); + assert!( + request.intersect(&ceiling).is_err(), + "ceiling `{ceiling}` admitted the disjoint request `{request}`" + ); + } + } + + /// The overlapping half, pinned to the exact grant rather than to + /// "something was granted". A request wider than the ceiling is + /// narrowed to the ceiling, never met in full — and asserting the whole + /// resulting string is what makes "narrowed" a claim a widening bug can + /// fail rather than a claim any non-empty answer satisfies. + /// + /// Fails if: any of the narrowing rules inverts — `intersect_size` + /// taking the larger cap, `intersect_aud` dropping a `Some` for `None`, + /// `ActionSet::intersect` unioning, or `NsidPattern::intersect` + /// returning the wider pattern of the two. + #[test] + fn a_wider_request_is_narrowed_to_exactly_the_ceiling() { + for (ceiling, request, expected) in [ + // A parent request is met only at the child the ceiling names. + ( + "repo:app.bsky.feed.post", + "repo:app.bsky.feed.*", + "repo:app.bsky.feed.post", + ), + ( + "repo:app.bsky.feed.*", + "repo:app.bsky.*", + "repo:app.bsky.feed.*", + ), + ("repo:app.bsky.feed.*", "repo:*", "repo:app.bsky.feed.*"), + // Actions narrow to the overlap, not the union. + ( + "repo:app.bsky.feed.post?action=create", + "repo:app.bsky.feed.post?action=create,delete", + "repo:app.bsky.feed.post?action=create", + ), + // An unbounded ceiling still yields the enumerated request, and + // an enumerated ceiling still bounds an unbounded request. + ( + "repo:app.bsky.feed.post?action=create,update,delete", + "repo:app.bsky.feed.post", + "repo:app.bsky.feed.post?action=create,update,delete", + ), + // An audience-bounded ceiling bounds an unbounded request. + ( + "rpc:com.atproto.repo.createRecord?aud=did:web:one.example", + "rpc:com.atproto.repo.createRecord", + "rpc:com.atproto.repo.createRecord?aud=did:web:one.example", + ), + // Size caps take the smaller, from either side. + ( + "blob:image/png?maxSize=1000", + "blob:image/png?maxSize=1001", + "blob:image/png?maxSize=1000", + ), + ( + "blob:image/png?maxSize=1001", + "blob:image/png?maxSize=1000", + "blob:image/png?maxSize=1000", + ), + ( + "blob:image/png?maxSize=1000", + "blob:image/png", + "blob:image/png?maxSize=1000", + ), + // MIME narrows one half at a time. + ("blob:image/png", "blob:image/*", "blob:image/png"), + ("blob:image/*", "blob:*/*", "blob:image/*"), + // A named resource bounds a `*` request. + ("identity:handle", "identity:*", "identity:handle"), + // `transition:generic` is wide, but not wider than what was + // asked for. + ( + "transition:generic", + "repo:app.bsky.feed.post", + "repo:app.bsky.feed.post", + ), + ] { + let ceiling_set = ScopeSet::parse(ceiling).expect("parses"); + let request_set = ScopeSet::parse(request).expect("parses"); + let granted = request_set + .intersect(&ceiling_set) + .unwrap_or_else(|err| panic!("`{request}` under `{ceiling}` was refused: {err}")); + assert_eq!( + granted.to_string(), + expected, + "`{request}` under ceiling `{ceiling}`" + ); + } + } + + /// A grant is always reachable a second time: whatever a ceiling + /// granted, presenting exactly that grant as the request must yield it + /// back unchanged. A refresh that re-checked the ceiling and got less + /// than the token already carries would silently downgrade a live + /// session. + /// + /// Fails if: intersection stops being reflexive on its own output — + /// the same class as the idempotence test above, caught from the + /// request side rather than the ceiling side. + #[test] + fn a_grant_re_requested_is_granted_in_full() { + for (request, ceiling) in pairs() { + let Ok(granted) = request.intersect(&ceiling) else { + continue; + }; + if granted.0.is_empty() { + continue; + } + let regranted = granted + .intersect(&ceiling) + .expect("a grant is inside the ceiling that produced it"); + assert_eq!( + regranted, granted, + "re-requesting grant `{granted}` under ceiling `{ceiling}` yielded `{regranted}`" + ); + } + } +} + #[cfg(test)] mod tests { use super::*; -- 2.51.2