diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 5a5f0ff..b206415 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -1360,6 +1360,18 @@ every path in every document as a name would be one lookup per path: | `/repo` | refused | refused | | `otherowner/repo`, bare | refused | **not seen** | | an organisation named on its own | refused | **not seen** | +| `..v`, a schema id | **passed** | **not seen** | + +**A schema id is not a repository name**, and the last row is where that is +decided. An organisation that publishes document formats writes its own name +into every id it publishes, so the bare-owner search read each of them as the +organisation named on its own and no pull request could quote the id of the +schema it was changing. The id is told from the name by what follows the owner: +one or more dotted segments and then a version segment, `acme.widget_state.v1` +and the `.schema.json` file that carries it. Nothing else moves. A schema id has +no slash in it, so `/` under a declared owner is refused exactly as +before, and so is the organisation written on its own in a sentence -- there +being no id after the name is the whole difference. An unreadable source is exit `2`, because a rule with no owners refuses nothing and would report a clean tree over a list it could not read. diff --git a/policy/base/published-text.toml b/policy/base/published-text.toml index 4c6e2cc..6605153 100644 --- a/policy/base/published-text.toml +++ b/policy/base/published-text.toml @@ -99,6 +99,14 @@ command.scope = "always" # condition the family fires under always holds at this seam. A private # repository that inherits `private-names` keeps its hook-side guards standing # down; this one does not stand down, and that is the point. +# +# What it does NOT refuse, because this seam is where it was measured: a schema +# id of the shape `..v`. An organisation that publishes +# document formats carries its own name in every id it publishes, and reading +# those as the organisation named on its own left a pull-request body unable to +# say which schema it was adding -- with the ids being exactly what the change +# was about. `/` and the organisation alone are refused as before; +# see REFERENCE.md, "Why the owner list is worth declaring". [rule.no-published-private-repo-names] builtin = "no-private-repo-names" visibility = "public" diff --git a/src/guard/names.rs b/src/guard/names.rs index 3ce3f37..570f9a9 100644 --- a/src/guard/names.rs +++ b/src/guard/names.rs @@ -253,13 +253,52 @@ fn candidates(text: &str, owners: &OwnerMatchers) -> BTreeSet<(String, String)> } } for (owner, matcher) in &owners.bare { - if matcher.is_match(text) { + // Every occurrence, not `is_match`, because one of the shapes an owner + // name occurs in is not a mention of the organisation at all. A text + // whose only occurrences are schema ids has nothing in it to refuse. + if matcher + .find_iter(text) + .any(|hit| !opens_a_schema_id(text, hit.end())) + { found.insert((owner.clone(), String::new())); } } found } +/// The tail of a versioned schema id, read from where the owner name ends. +/// +/// `..v`: at least one dotted segment, then a version +/// segment, then a boundary. Anchored, because what is being asked is whether +/// the id CONTINUES from the name just matched -- a version number somewhere +/// later in the sentence says nothing about this occurrence. +static SCHEMA_ID_TAIL: OnceLock = OnceLock::new(); + +fn schema_id_tail() -> &'static Regex { + SCHEMA_ID_TAIL.get_or_init(|| { + crate::engine::literal_pattern(r"^(?:\.[A-Za-z0-9_]+)+\.v[0-9]+(?:[^A-Za-z0-9_]|$)") + }) +} + +/// Whether the owner name ending at `end` is the first segment of a schema id. +/// +/// An organisation that publishes document formats names itself in every one of +/// their ids -- `acme.widget_state.v1` is the id of a FORMAT, and the format is +/// usually what the pull request adding it is about. The bare-owner search read +/// that as the organisation named on its own, so a repository could not quote +/// the id of the schema it was changing: measured on one workspace where every +/// published contract carries the prefix, and the author elided the line. +/// +/// Only this arm, and deliberately. A schema id has no slash in it, so nothing +/// here reaches `/` in any of its forms -- a repository name under +/// a private owner is refused exactly as before, and so is the organisation +/// written on its own in a sentence. The id is told from the name by what +/// FOLLOWS it, which is the one place the two shapes differ. +fn opens_a_schema_id(text: &str, end: usize) -> bool { + text.get(end..) + .is_some_and(|tail| schema_id_tail().is_match(tail)) +} + /// The patterns that depend only on the OWNER, compiled once per judgement. /// /// Built here rather than inside `candidates` because `candidates` is asked @@ -1492,6 +1531,70 @@ mod tests { assert!(found.is_empty(), "{found:?}"); } + #[test] + fn a_declared_owner_written_on_its_own_is_a_finding() { + // The control the exemption below is measured against: nothing about a + // schema id makes the organisation in a sentence acceptable. + let declared = vec!["acme".to_owned()]; + let found = named("a change landed in acme this week", &declared, None); + assert!( + found.contains(&("acme".to_owned(), String::new())), + "{found:?}" + ); + } + + #[test] + fn a_schema_id_is_not_the_organisation_named_on_its_own() { + // `..v` is the id of a document FORMAT. It carries + // the organisation's name the way every id in a published set does, and + // reading it as the organisation left a pull request unable to name the + // schema it was adding. + let declared = vec!["acme".to_owned()]; + let found = named( + "This adds the contract whose id is acme.widget_state.v1.", + &declared, + None, + ); + assert!(found.is_empty(), "{found:?}"); + + // And the file that carries it, which is the same id with the + // convention's suffix on the end. + let with_suffix = named("see acme.widget_state.v1.schema.json", &declared, None); + assert!(with_suffix.is_empty(), "{with_suffix:?}"); + } + + #[test] + fn a_repository_name_under_a_declared_owner_is_still_a_finding() { + // The other half of the planted pair. A schema id has no slash in it, + // so the exemption cannot reach this shape -- `/` is what + // the rule exists for, and a text carrying both is refused over the + // second one. + let declared = vec!["acme".to_owned()]; + let found = named( + "acme.widget_state.v1 is defined in acme/widget-contracts", + &declared, + None, + ); + assert!( + found.contains(&("acme".to_owned(), "widget-contracts".to_owned())), + "{found:?}" + ); + } + + #[test] + fn a_version_that_is_not_a_version_segment_is_not_a_schema_id() { + // The exemption is the whole shape or nothing. A dotted name with no + // `v` on the end is not an id this convention publishes, and reading + // it as one would quiet any sentence that happened to put a dot after + // the organisation. + let declared = vec!["acme".to_owned()]; + let found = named("acme.internal runs the build", &declared, None); + assert!( + found.contains(&("acme".to_owned(), String::new())), + "{found:?}" + ); + } + #[test] fn a_declared_owner_with_a_dot_is_escaped_not_interpreted() { let declared = vec!["acme.corp".to_owned()]; diff --git a/tests/text_cli.rs b/tests/text_cli.rs index a424b43..b7b3782 100644 --- a/tests/text_cli.rs +++ b/tests/text_cli.rs @@ -345,3 +345,72 @@ fn a_prose_rule_with_no_command_is_left_out_of_the_text_seam() { let output = scan_text(&root, b"Arguably the count is right.\n", EXAMPLE_HOME); assert_eq!(code(&output), 0, "{}", stderr(&output)); } + +// ── a schema id is not a repository name ───────────────────────────── + +/// The private-name guard at the text seam, with the owner declared on the rule +/// so no forge is asked: a declared owner is the operator saying so, and a test +/// that needed `gh` would be a test of this machine's credentials. +/// +/// `visibility = "public"` because that is the condition the whole family fires +/// under, and the text seam is the one place it holds whatever the repository +/// itself is. +const PRIVATE_NAMES_POLICY: &str = r#" +visibility = "public" + +[rule.no-private-repo-names] +builtin = "no-private-repo-names" +private_owners = ["acme"] +command.before = ["gh"] + +[[shim]] +command = "gh" +match = ["pr:create"] +text_flags = ["-b", "--body"] +scope = "always" +"#; + +/// The planted pair, run through the binary, one verdict each. +/// +/// An organisation that publishes document formats writes its own name into +/// every id it publishes -- `acme.widget_state.v1` -- and the bare-owner search +/// read that as the organisation named on its own. The cost was measured: a +/// pull request could not say which schema it was adding, in the one repository +/// where that is the subject. +#[test] +fn a_schema_id_passes_the_text_seam_and_a_repository_name_does_not() { + let root = workspace(PRIVATE_NAMES_POLICY); + + let id = guard_text( + &root, + b"This adds the contract whose id is acme.widget_state.v1.\n", + EXAMPLE_HOME, + None, + ); + assert_eq!(code(&id), 0, "{}", stderr(&id)); + + let name = guard_text( + &root, + b"This adds the contract, defined in acme/widget-contracts.\n", + EXAMPLE_HOME, + None, + ); + assert_eq!(code(&name), 1, "{}", stderr(&name)); + let report = stderr(&name); + assert!(report.contains("acme/widget-contracts"), "{report}"); + + // And the organisation in a sentence, which is the shape the exemption must + // not reach: no id follows the name, so there is nothing to tell it from. + let alone = guard_text( + &root, + b"This was hit while working on something at acme.\n", + EXAMPLE_HOME, + None, + ); + assert_eq!(code(&alone), 1, "{}", stderr(&alone)); + assert!( + stderr(&alone).contains("named on its own"), + "{}", + stderr(&alone) + ); +}