From cbb97b607f5eb2948c4b2a8b62036b40191eb50b Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Thu, 3 Sep 2026 06:16:54 -0400 Subject: [PATCH] test(policy-regex): pin the allowlist whole-value fit from outside lib.rs The tests covering the whole-value fit of `deny_path_value_unless` live in the same file as the code, so an edit that loses the fit can take them with it. This one names only the public surface. Change-Id: I9efa651180636f0169cc7d62ceb9806839bb908c --- .../tests/allowlist_is_whole_value.rs | 177 ++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 crates/didbot-policy-regex/tests/allowlist_is_whole_value.rs diff --git a/crates/didbot-policy-regex/tests/allowlist_is_whole_value.rs b/crates/didbot-policy-regex/tests/allowlist_is_whole_value.rs new file mode 100644 index 00000000..c60abcc0 --- /dev/null +++ b/crates/didbot-policy-regex/tests/allowlist_is_whole_value.rs @@ -0,0 +1,177 @@ +//! The whole-value fit of a `deny_path_value_unless` allowlist, pinned from +//! **outside** `src/lib.rs`. +//! +//! The in-crate tests for this fit live in the same file as the code they +//! cover, so an edit to `src/lib.rs` that loses the fit can take those tests +//! with it and still report a green suite. This file names only the public +//! surface — [`RegexEvaluator::deny_path_value_unless`] and [`ValuePattern`] +//! — so it holds across any rewrite of that file and fails when the fit is +//! lost. +//! +//! The property: an `allowed` entry is matched against the value in its +//! entirety, not searched within it. An entry naming `alice` admits `alice` +//! and refuses `alice-evil`. Widening it is the one direction a deny-only +//! system must never fail in, because it *permits* what the policy author +//! refused. + +use didbot_policy_regex::{ + AccountKind, Change, CompiledId, DeploymentAttributes, Diff, Evaluator, Outcome, + RegexEvaluator, Subject, SubjectAttributes, SubjectKind, Universal, ValuePattern, WriteAction, +}; +use serde_json::{json, Value}; + +const COLLECTION: &str = "app.bsky.actor.profile"; +const REASON: &str = "display name must be one of the allowed set"; + +fn universal() -> Universal<'static> { + Universal { + deployment: DeploymentAttributes { + pds_did: "did:web:pds.example", + pds_hostname: "pds.example", + }, + now: time::OffsetDateTime::UNIX_EPOCH, + } +} + +/// A write setting `displayName` to `value`, shaped exactly as the evaluator +/// expects so the policy actually reaches its value predicate. The control +/// assertions below prove this fixture does produce a matching change: the +/// same builder, with a value the allowlist names, yields `Outcome::Allow`. +fn set_display_name<'a>(changes: &'a [Change<'a>]) -> Subject<'a> { + Subject::Write { + account: "did:plc:agent", + attributes: SubjectAttributes { + kind: SubjectKind::Known(AccountKind::Agent), + handle: Some("agent.example.com"), + }, + universal: universal(), + client_id: "https://client.example/client-metadata.json", + collection: COLLECTION, + action: WriteAction::Create, + diff: Diff { changes }, + } +} + +fn evaluator(allowed: &[ValuePattern<'_>]) -> RegexEvaluator { + RegexEvaluator::deny_path_value_unless( + r"^app\.bsky\.actor\.profile$", + r"^displayName$", + allowed, + REASON, + ) + .expect("patterns compile") +} + +fn outcome(eval: &RegexEvaluator, value: &Value) -> Outcome { + let changes = [Change { + path: "displayName", + before: None, + after: Some(value), + }]; + eval.evaluate(&set_display_name(&changes), CompiledId(0)) + .expect("evaluation succeeds") +} + +fn rejected() -> Outcome { + Outcome::Reject { + reason: REASON.to_owned(), + } +} + +/// An allowlist entry naming `alice` admits `alice` and nothing built around +/// it. The `alice` case is the control: it shares this test's fixture and +/// this test's evaluator, so the refusals below cannot be passing because the +/// fixture failed to produce a change the policy sees. +#[test] +fn an_allowlist_entry_names_the_whole_value() { + let eval = evaluator(&[ValuePattern::Regex("alice")]); + + assert_eq!( + outcome(&eval, &json!("alice")), + Outcome::Allow, + "control: the exact value the allowlist entry names must be allowed, \ + or the refusals in this test prove nothing about the fit" + ); + + for value in ["alice-evil", "evil-alice", "evil-alice-evil", "alicex"] { + assert_eq!( + outcome(&eval, &json!(value)), + rejected(), + "an allowlist entry naming `alice` must not admit `{value}`" + ); + } +} + +/// The whole-value fit is applied around the author's pattern rather than +/// trusted to be in it, so the classic anchoring mistake — `^alice|bob$`, +/// which parses as `(^alice)|(bob$)` — cannot widen an allowlist. +#[test] +fn a_misplaced_anchor_cannot_widen_an_allowlist() { + let eval = evaluator(&[ValuePattern::Regex("^alice|bob$")]); + + for value in ["alice", "bob"] { + assert_eq!( + outcome(&eval, &json!(value)), + Outcome::Allow, + "control: `{value}` is named by the entry and must be allowed" + ); + } + + for value in ["alice-evil", "evil-bob"] { + assert_eq!( + outcome(&eval, &json!(value)), + rejected(), + "a misplaced anchor must not let `{value}` through" + ); + } +} + +/// A multi-entry allowlist — the "small allowed set" shape the arm exists +/// for — holds the fit on every entry, not just the first. +#[test] +fn every_entry_of_an_allowlist_names_the_whole_value() { + let eval = evaluator(&[ValuePattern::Regex("alice"), ValuePattern::Regex("bob")]); + + for value in ["alice", "bob"] { + assert_eq!( + outcome(&eval, &json!(value)), + Outcome::Allow, + "control: `{value}` is a named entry and must be allowed" + ); + } + + for value in ["alice-evil", "bob-evil", "not-bob"] { + assert_eq!( + outcome(&eval, &json!(value)), + rejected(), + "no entry of the allowlist may admit `{value}`" + ); + } +} + +/// `ValuePattern::Contains` is a substring test in both arms, as its name +/// says. The fit does not apply to it, and this guard must not be read as +/// claiming otherwise — an edit that "fixed" `Contains` to be +/// whole-value would be a different bug. +#[test] +fn contains_is_still_a_substring_test() { + let eval = evaluator(&[ValuePattern::Contains( + didbot_policy_regex::SubjectAttribute::Handle, + )]); + + assert_eq!( + outcome(&eval, &json!("agent.example.com")), + Outcome::Allow, + "control: the handle itself contains the handle" + ); + assert_eq!( + outcome(&eval, &json!("[agent] agent.example.com posting")), + Outcome::Allow, + "a `Contains` entry is satisfied by a substring match" + ); + assert_eq!( + outcome(&eval, &json!("someone else entirely")), + rejected(), + "a `Contains` entry still refuses a value that lacks the substring" + ); +} -- 2.51.2