From 1a488fda012b668377fb01c2c03882062ee4101c Mon Sep 17 00:00:00 2001 From: HackingGate Date: Fri, 16 Oct 2026 21:04:47 +0900 Subject: [PATCH] Normalize publication subjects and command destination context Give text adapters and command publication an explicit Invocation subject contract, preserving absent, empty and unread input and independent tool identity. Bind command scope, ownership and text destination checks to one lazy normalized destination with host and selection provenance. Move pattern_refusal, pattern_finding and consult into shim::checker. Move scope_holds, forge, visibility, gh_repository_target, declared_standing and effective_scope into shim::destination. Replace resolve_target with Destination::resolve, and ScopeMemo::holds and ScopeMemo::standing with the bound CommandContext methods. No rule bodies, checker exit contracts, editor behavior or process handoff are changed. Add an allowed/refused fixture matrix across scan, guard, hook, command and editor paths, unread-surface cases, and destination-observation tests. Document the shared boundary and the adapters' remaining limits. Validated 543 unit tests and 257 focused CLI/structural tests; clippy --all-targets passes. Implements #373 and the first cohesive module boundaries for #374. --- docs/DESIGN.md | 34 ++ docs/REFERENCE.md | 7 + src/hook.rs | 40 +- src/invocation.rs | 142 +++++++ src/main.rs | 31 +- src/shim.rs | 778 +++++-------------------------------- src/shim/checker.rs | 194 +++++++++ src/shim/destination.rs | 486 +++++++++++++++++++++++ src/text.rs | 68 ++-- tests/publication_paths.rs | 188 +++++++++ 10 files changed, 1223 insertions(+), 745 deletions(-) create mode 100644 src/invocation.rs create mode 100644 src/shim/checker.rs create mode 100644 src/shim/destination.rs create mode 100644 tests/publication_paths.rs diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 1e6aa7f..2c488e3 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -323,6 +323,40 @@ examples. forge), so `collect = "git-refs"` replaces the argv walk rather than forking the shim. +### Publication adapter boundaries + +`invocation::Invocation` is the shared adapter-to-policy subject contract. It +carries the seam, report identity, and either pathless prose with separately +identified headlines or command-collected subject kinds. `Observed` distinguishes +a missing subject from a present empty string and from an unread required +surface. Tool identity remains available when a harness call has no argument +strings. `text::judged` consumes the pathless form; the shim consumes named +subjects under the same authoritative `text::Judged` and `text::Seam` table. +Passing a command's subjects to the pathless evaluator is an error. + +Publication commands have additional facts that raw text cannot establish. +`shim::destination::CommandContext` binds the canonical arguments and collected +target to one invocation. It resolves the destination lazily and retains its +repository spelling, host, forge client and selection provenance. Scope, +ownership and destination-sensitive text guards consume that observation. +Local visibility is reused only for a destination selected from origin without +an ambient host override. A required destination that remains unresolved takes +the existing explicit unresolved-scope path; it cannot become a clean answer. + +The command grammar and collection remain in `shim`; they do not evaluate +policy. `shim::checker` evaluates one selected rule against one subject, with +pattern or subprocess checks and the existing checker exit contract. The shim +orchestrator selects rules, asks scope, judges destination then subjects, and +hands the original arguments and any replayed stdin to the process. Editor +re-entry uses the same command context and checker implementations. These are +internal modules of the same binary, with no separate installation lifecycle. + +Harness hooks collect strings and recognized headline fields. They do not +interpret arbitrary tool arguments as forge destinations; direct text checks +also have no destination supplied by their caller. The existing text guards' +repository context remains unchanged. Command normalization does not extend +that coverage to another transport or an unknown command grammar. + ### `before` scopes a checker Until v3 a checker was consulted by every declared shim: a check written for a diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 4acc372..5966055 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -2806,6 +2806,13 @@ Tool-name rules also run when arguments contain only numbers, booleans, nulls, or empty containers. Such calls have no argument-text subject; an explicitly supplied empty string still has one. +The hook does not infer a destination host or repository from arbitrary tool +argument names. It checks the strings and tool identity using the policy where +the call was made. Destination ownership and `public-target` checks remain +command-adapter checks; raw `scan --text` and `guard --text` input likewise +supplies no destination. A missing required argument surface is an unread +subject and exits `2`, rather than being treated as a call with no prose. + ### What runs where there is no policy Every other seam requires a policy and exits `2` without one, because a git hook diff --git a/src/hook.rs b/src/hook.rs index 636c3e7..51a12b8 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -51,6 +51,7 @@ use serde_json::Value; use crate::error::{Exit, Fatal, Result}; use crate::guard; +use crate::invocation::{Invocation, Observed}; use crate::shim::{Walked, walk_strings}; use crate::text::{self, Verdict}; @@ -182,15 +183,19 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result) -> Result = collected @@ -225,14 +232,17 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result { + /// The surface was read and contains no subject of this kind. + Absent, + /// A subject was read, including a deliberately empty string. + Present(T), + /// The required surface could not be read, with evidence for the caller. + Unread(String), +} + +/// The prose and separately identified headlines of a pathless publication. +pub(crate) struct TextInput<'a> { + /// Whole prose; absence leaves independently observed tool identity intact. + pub prose: Observed<&'a str>, + /// Headlines retain their report labels and separate guard treatment. + pub headlines: &'a [Headline], +} + +impl Observed { + /// Reading an unavailable surface is an infrastructure error, never clean. + pub(crate) fn read(&self) -> Result> { + match self { + Self::Absent => Ok(None), + Self::Present(value) => Ok(Some(value)), + Self::Unread(why) => Err(Fatal::new(why)), + } + } +} + +enum Input<'a> { + Text(TextInput<'a>), + Command(&'a [Subject]), +} + +/// Identity and observed subjects, before choosing applicable rule kinds. +/// Destination/scope observations belong to the command adapter's context; +/// raw text and harness events do not establish a forge destination. +pub(crate) struct Invocation<'a> { + /// Determines the authoritative set of rule kinds consulted. + pub seam: Seam, + /// Adapter identity in reports; a hook/command label is also its tool. + pub label: &'a str, + input: Input<'a>, +} + +impl<'a> Invocation<'a> { + /// Normalize pathless prose without inferring a destination or path. + pub(crate) const fn text( + seam: Seam, + label: &'a str, + prose: Observed<&'a str>, + headlines: &'a [Headline], + ) -> Self { + Self { + seam, + label, + input: Input::Text(TextInput { prose, headlines }), + } + } + + /// Preserve the command grammar's named subject kinds for policy dispatch. + pub(crate) const fn command(label: &'a str, subjects: &'a [Subject]) -> Self { + Self { + seam: Seam::Command, + label, + input: Input::Command(subjects), + } + } + + /// A tool name is independently observable, including when the tool's + /// argument object contains no strings. A source filename is not a tool. + pub(crate) fn tool(&self) -> Option<&str> { + matches!(self.seam, Seam::Hook | Seam::Command).then_some(self.label) + } + + /// A command cannot lose its scope by entering the pathless evaluator. + pub(crate) fn text_input(&self) -> Result<&TextInput<'a>> { + match &self.input { + Input::Text(text) => Ok(text), + Input::Command(_) => Err(Fatal::new( + "command subjects require command scope and judgment; they cannot be judged as pathless text", + )), + } + } + + /// Pathless text cannot acquire command subject kinds by guessing. + pub(crate) fn command_subjects(&self) -> Result<&[Subject]> { + match &self.input { + Input::Command(subjects) => Ok(subjects), + Input::Text(_) => Err(Fatal::new( + "pathless text has no command-collected subject kinds", + )), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn absent_prose_keeps_tool_identity_and_unread_prose_is_an_error() { + let invocation = Invocation::text(Seam::Hook, "publish_tool", Observed::Absent, &[]); + assert_eq!(invocation.tool(), Some("publish_tool")); + assert!( + invocation + .text_input() + .unwrap() + .prose + .read() + .unwrap() + .is_none() + ); + let unread = Invocation::text( + Seam::Hook, + "publish_tool", + Observed::Unread("arguments could not be read".to_owned()), + &[], + ); + assert!(unread.text_input().unwrap().prose.read().is_err()); + } + + #[test] + fn subject_shapes_cannot_silently_cross_evaluators() { + let command = Invocation::command("faux", &[]); + assert!(command.text_input().is_err()); + let text = Invocation::text(Seam::Scan, "body.md", Observed::Present(""), &[]); + assert!(text.command_subjects().is_err()); + assert_eq!(text.tool(), None); + assert_eq!(text.text_input().unwrap().prose.read().unwrap(), Some(&"")); + } +} diff --git a/src/main.rs b/src/main.rs index 68947a0..8697899 100644 --- a/src/main.rs +++ b/src/main.rs @@ -89,6 +89,7 @@ mod hook; mod hooks; mod init; mod install; +mod invocation; mod out; mod pins; mod policy; @@ -767,17 +768,25 @@ fn guard_command(arguments: &[OsString]) -> Result { // Both arms become a refusal, because that is the word this command // reports in: a finding reached through `--text` and one reached // through a guard are the same verdict on the same text. - let refusals: Vec = - text::judged(text::Seam::Guard, &root, &policy, &source, Some(&text), &[])? - .into_iter() - .map(|verdict| match verdict { - text::Verdict::Guard(refusal) => refusal, - text::Verdict::Rule(failure) => guard::Refusal { - report: failure.report(), - id: failure.label, - }, - }) - .collect(); + let refusals: Vec = text::judged( + &invocation::Invocation::text( + text::Seam::Guard, + &source, + invocation::Observed::Present(&text), + &[], + ), + &root, + &policy, + )? + .into_iter() + .map(|verdict| match verdict { + text::Verdict::Guard(refusal) => refusal, + text::Verdict::Rule(failure) => guard::Refusal { + report: failure.report(), + id: failure.label, + }, + }) + .collect(); for refusal in &refusals { eprintln!("guard refused: {}", guard::refused_by(&policy, refusal)); eprintln!("{}", refusal.report.trim_end()); diff --git a/src/shim.rs b/src/shim.rs index d4e8f27..d4f1eca 100644 --- a/src/shim.rs +++ b/src/shim.rs @@ -43,6 +43,14 @@ use serde::Deserialize; use crate::config::{Policy, Rule}; use crate::error::{Exit, Fatal, Result}; use crate::git; +use crate::invocation::Invocation; + +mod checker; +mod destination; + +pub(crate) use checker::pattern_finding; +use checker::{consult, pattern_refusal}; +use destination::{CommandContext, effective_scope}; /// Where an invocation is going, when that is a meaningful question at all. #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Default)] @@ -3216,269 +3224,6 @@ impl Shim { } Ok(collected) } - - /// Whether one scope predicate holds for this invocation. - /// - /// The predicate is a parameter rather than the table's own, because a rule - /// may carry its own `command.scope`: the table's answers for the command, - /// and a rule that applies on every egress -- host identity in a - /// pull-request body is worth refusing whatever the destination -- says so - /// beside its `command.before` rather than inheriting the table's idea of - /// the question. - /// - /// The policy is a parameter for the sake of one arm: `public-target` is a - /// question this repository may already have answered in writing, and a - /// predicate that asks a forge anyway is a network call -- and a token, and - /// a rate limit -- spent on a fact that was sitting in `principles.toml` - /// the whole time. See [`declared_standing`]. - pub(crate) fn scope_holds( - &self, - scope: &Scope, - root: &Path, - policy: &Policy, - collected: &Collected, - argv: &[String], - ) -> Result { - match scope { - Scope::Always => Ok(Standing::Holds), - Scope::PublicTarget => { - let Some(target) = self.resolve_target(root, collected)? else { - // No answer is not "public", and it is not "not public" - // either. `gh` unauthenticated, rate-limited, offline, or a - // repository with no `origin` all land here, and what the - // caller does about it is the caller's decision -- see - // [`Unresolved`]. - return Ok(Standing::CouldNotTell(String::from( - "no target could be resolved, so whether the `public-target` checks \ - apply here could not be established", - ))); - }; - // What this repository says about itself, before anybody is - // asked. Read here and not before the resolver, because a tree - // with no `origin` has no destination for a declaration to be - // about, and that is the resolver's answer above rather than - // this one's. - if let Some(standing) = declared_standing(root, policy, collected, self)? { - return Ok(standing); - } - match self.visibility(root, &target).as_deref() { - Ok("public") => Ok(Standing::Holds), - Ok(_) => Ok(Standing::DoesNotHold), - // The cause, in the words the forge used. A rate limit, an - // unauthenticated `gh` and a deleted repository are three - // different instructions to the reader, and a refusal - // naming none of them is one the reader can only bypass. - Err(silence) => Ok(Standing::CouldNotTell(format!( - "the forge did not say whether {target} is public, so whether the \ - `public-target` checks apply here could not be established. {}", - silence.sentence() - ))), - } - } - Scope::PublicRegistry => { - if argv.iter().any(|argument| argument == "--dry-run") { - return Ok(Standing::DoesNotHold); - } - // Two independent reasons this is nobody's business, and either - // one is enough: a package marked private cannot be published - // at all, and a registry that is not the public one is - // somebody's internal infrastructure. - if let Ok(text) = std::fs::read_to_string(root.join("package.json")) - && text.contains("\"private\"") - && json_bool_field(&text, "private") - { - return Ok(Standing::DoesNotHold); - } - let registry = collected - .target - .clone() - .unwrap_or_else(|| String::from("https://registry.npmjs.org")); - Ok(if registry.contains("registry.npmjs.org") { - Standing::Holds - } else { - Standing::DoesNotHold - }) - } - Scope::Command { command } => { - let status = Command::new("sh") - .arg("-c") - .arg(command) - .current_dir(root) - .status() - .map_err(|error| { - Fatal::new(format!("{}: scope command: {error}", self.command)) - })?; - Ok(if status.success() { - Standing::Holds - } else { - Standing::DoesNotHold - }) - } - } - } - - /// Which forge can answer for this repository, where that has an answer. - fn forge(&self, root: &Path) -> Option { - // The command is the strongest evidence there is: somebody running - // `glab` is publishing to GitLab whatever else is configured. - match self.command.as_str() { - "gh" => return Some(Forge::GitHub), - "glab" => return Some(Forge::GitLab), - _ => {} - } - // Otherwise the remote decides, which is what the `git` shim needs: - // one shim stands in front of a command that pushes to either. - Forge::of_url(&git::remote_url(root, "origin")?) - } - - /// What the forge says the target's visibility is, in the forge's own word. - /// - /// Asking `gh` for every target is why the shipped `glab` shim could never - /// resolve one: `gh api repos//` answers about GitHub and - /// about nothing else, so a GitLab remote fell through the `None` arm on - /// every invocation and a shim declared `public-target` was inert. - /// - /// Both vocabularies come back unchanged, and GitLab's `internal` is why: - /// it means public to everyone with an account on the instance, which is - /// neither public to the internet nor private. Only `public` is treated as - /// public by the caller, so a forge that grows a fourth word does not - /// quietly become one of the three. - fn visibility(&self, root: &Path, target: &str) -> std::result::Result { - let Some(forge) = self.forge(root) else { - return Err(Silence::Refused(String::from( - "no forge client is known for that host, so there was nobody to ask", - ))); - }; - match forge { - Forge::GitHub => { - let name = git::owner_repo(target).map_or_else( - || target.to_owned(), - |(owner, repo)| format!("{owner}/{repo}"), - ); - let endpoint = format!("repos/{name}"); - let mut args = vec!["api", endpoint.as_str(), "--jq", ".visibility"]; - let host = git::host(target); - if let Some(host) = host.as_deref() { - args.extend(["--hostname", host]); - } - forge_field("gh", &args, None) - } - // Deliberately not `--jq`: `glab api` is not `gh api`, and a shim - // that is inert the day one flag differs is the defect this arm - // exists to end. The project id is a path, so its separators are - // escaped -- `owner/name` and `group/sub/name` are each one id. - Forge::GitLab => forge_field( - "glab", - &["api", &format!("projects/{}", target.replace('/', "%2F"))], - Some("visibility"), - ), - } - } - - fn resolve_target(&self, root: &Path, collected: &Collected) -> Result> { - let explicit = collected - .target - .as_deref() - .filter(|target| !target.is_empty()); - // `gh`'s repo override is command line first, then GH_REPO. In - // particular an environment-selected repository is never `origin`, - // even when its owner/name happen to match the checkout. - let gh_repo = self.command == "gh" && self.target != Target::None; - let environment = gh_repo - .then(|| { - std::env::var("GH_REPO") - .ok() - .filter(|value| !value.is_empty()) - }) - .flatten(); - if let Some(named) = explicit.or(environment.as_deref()) { - if gh_repo && repository_placeholder("gh", named)? { - // API placeholders are expanded by gh's own remote/default - // resolution, which is not established by an origin guess. - // Visibility can ask gh to expand these same placeholders; - // the ownership guard separately refuses unresolved names. - return Ok(Some(named.to_owned())); - } - return if gh_repo { - gh_repository_target(named).map(Some) - } else { - Ok(Some(named.to_owned())) - }; - } - if gh_repo { - return git::remote_url(root, "origin") - .map(|url| gh_repository_target(&url)) - .transpose(); - } - Ok(match self.target { - Target::None => None, - Target::GitRemote => git::remote_url(root, "origin") - .and_then(|url| git::owner_repo(&url)) - .map(|(owner, repo)| format!("{owner}/{repo}")), - Target::ForgeRepo => { - // Both forge CLIs answer this, and every spec that wanted it - // wrote the same call. The remote is the same answer without a - // network round trip, so it is tried first. - git::remote_url(root, "origin") - .and_then(|url| git::owner_repo(&url)) - .map(|(owner, repo)| format!("{owner}/{repo}")) - } - }) - } -} - -/// Resolve the host-bearing spellings accepted by `gh` without treating an -/// enterprise repository as its same-named github.com counterpart. Enterprise -/// support is deliberately absent here, as it is in `Forge::of_url`. -fn gh_repository_target(named: &str) -> Result { - let parts: Vec<_> = named.split('/').collect(); - let host = if named.contains("://") || named.contains(':') { - git::host(named) - } else if parts.len() == 3 { - parts.first().map(|hostname| hostname.to_lowercase()) - } else { - None - }; - let environment_host = std::env::var("GH_HOST") - .ok() - .filter(|configured| !configured.is_empty()); - if let Some(hostname) = host.as_ref().or(environment_host.as_ref()) - && !matches!( - hostname.to_lowercase().as_str(), - "github.com" | "www.github.com" - ) - { - return Err(Fatal::new(format!( - "gh: destination {named:?} uses host {hostname:?}, whose repository authority \ - cannot be verified by this shim. Nothing was published; use a supported \ - GitHub destination with a matching GH_HOST" - ))); - } - let path = if named.contains("://") || named.contains(':') { - git::owner_repo(named).map(|(owner, repo)| format!("{owner}/{repo}")) - } else { - match parts.as_slice() { - [owner, repo] | [_, owner, repo] => Some(format!("{owner}/{repo}")), - _ => None, - } - }; - let Some(path) = path.filter(|path| { - path.split('/').all(|part| { - !part.is_empty() - && part - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || b"-_.".contains(&byte)) - }) - }) else { - return Err(Fatal::new(format!( - "gh: destination {named:?} could not be resolved as [HOST/]OWNER/REPO. \ - Nothing was published" - ))); - }; - Ok(match host { - Some(hostname) => format!("https://{hostname}/{path}"), - None => path, - }) } /// What one invocation is about to publish. @@ -3540,72 +3285,6 @@ impl Forge { } } -/// What the policy already says about this destination, where it speaks for it. -/// -/// `Some` is a declaration that settles the question offline, and it is the same -/// declaration `guard::names::target_is_public` reads: a repository whose -/// `principles.toml` says `visibility = "private"` has stated the fact, and a -/// shim that ran `gh api repos//` anyway needed an authenticated -/// `gh` on every `git push` to learn it a second time. When GitHub rate-limits -/// the account that lookup fails, the failure is a [`Standing::CouldNotTell`], -/// and `unresolved = "refuse"` -- the default -- stops every push in every tree -/// over a fact nobody had to ask for. -/// -/// `None` sends the caller to the forge, and the narrowing that makes this safe -/// is the destination it will read the declaration for. A declaration is about -/// ONE repository, so this reads it only where the destination is the one -/// [`Shim::resolve_target`] took from `origin` -- which is to say where nothing -/// on the command line or in `GH_REPO` named a destination, and `GH_HOST` -/// did not change the host context. A `target_flags` value is a -/// destination somebody typed, and a typed `owner/repo` carries no host: `-R -/// acme/widget` under a `--hostname` or a `GH_HOST` is a repository on another -/// forge that happens to share a name with this one, and there is nothing in -/// two matching path segments that can tell it apart from this tree. So every -/// named destination goes to the forge exactly as it did before, including the -/// ones that name this repository, and what is bought offline is the case the -/// cost was actually in: the push that names nothing and means `origin`. -fn declared_standing( - root: &Path, - policy: &Policy, - collected: &Collected, - shim: &Shim, -) -> Result> { - if shim.command == "gh" - && (std::env::var("GH_REPO").is_ok_and(|repo| !repo.is_empty()) - || std::env::var("GH_HOST").is_ok_and(|host| !host.is_empty())) - { - return Ok(None); - } - if collected - .target - .as_deref() - .is_some_and(|named| !named.is_empty()) - { - return Ok(None); - } - // Lazily, and only here. Reading it is allowed to run `visibility_from`, which - // is a process, and a predicate that spawned one for a target the declaration - // does not speak for would trade the lookup this exists to avoid for another - // one. - let Some(declared) = policy.declared_visibility(root)? else { - return Ok(None); - }; - // Held to the three spellings before it arrives -- a written one at load, a - // `visibility_from` answer as it is read -- so `None` here is unreachable - // rather than a fourth word being guessed at. It falls through to the forge - // regardless: a word this seam cannot read is not an answer this seam may - // invent. - Ok( - crate::config::visibility_is_public(&declared).map(|public| { - if public { - Standing::Holds - } else { - Standing::DoesNotHold - } - }), - ) -} - /// Why a forge CLI gave no answer, told apart by what it printed. /// /// One classifier rather than three. `guard::names::lookup`, `guard::push` and @@ -5368,190 +5047,6 @@ fn json_bool_field(text: &str, field: &str) -> bool { .unwrap_or(false) } -/// One pattern rule's verdict over one subject. -/// -/// The same engine `scan` searches files with, asked about a string that never -/// becomes one -- same regex grammar, same meaning. `require_regexp` refuses a -/// subject the pattern is absent from; `regexp` refuses one it is present in. -/// Both name the kind and the pattern, because the reader's next step is to -/// fix the text, and "refused" without which-test-failed is a round trip. -/// -/// `prose_regexp` is the third, and the subject reaches it as whole-text prose: -/// a pull-request body is a document, so a fenced example in one is skipped -/// here exactly as it is in a committed document, and a sentence wrapped by -/// whatever composed the body is unwrapped before the pattern sees it. A rule -/// that refused a shape in a document and allowed it in the body announcing -/// that document would be one rule with two answers. -fn pattern_refusal(rule: &Rule, subject: &Subject) -> Result> { - Ok(pattern_finding(rule, subject)?.map(|finding| format!("{finding}\n{}", rule.message()))) -} - -/// What a pattern rule found in one subject, as the line a report names it by, -/// without the rule's message. Split from [`pattern_refusal`] for the hook, -/// whose report carries the message as its own paragraph, the way `--text` -/// prints it. -pub(crate) fn pattern_finding(rule: &Rule, subject: &Subject) -> Result> { - let multiline = rule.files().multiline; - if let Some(pattern) = rule.prose_regexp() { - let matcher = crate::prose::compile(pattern, &rule.id)?; - let Some(span) = crate::prose::of_text(&subject.value) - .into_iter() - .find(|span| matcher.is_match(&span.text)) - else { - return Ok(None); - }; - return Ok(Some(format!( - "{}: the {} subject matches {pattern:?}: {}", - rule.id, subject.kind, span.text - ))); - } - if let Some(pattern) = rule.require_regexp() { - let hits = crate::engine::search_text( - &subject.value, - &crate::engine::Query::regex(pattern, multiline), - &rule.id, - )?; - if hits.is_empty() { - return Ok(Some(format!( - "{}: the {} subject does not satisfy {pattern:?}", - rule.id, subject.kind - ))); - } - return Ok(None); - } - let Some(pattern) = rule.regexp() else { - return Ok(None); - }; - let hits = crate::engine::search_text( - &subject.value, - &crate::engine::Query::regex(pattern, multiline), - &rule.id, - )?; - let Some(hit) = hits.first() else { - return Ok(None); - }; - Ok(Some(format!( - "{}: the {} subject matches {pattern:?}: {}", - rule.id, - subject.kind, - hit.text.trim_end() - ))) -} - -/// Run one checker over one subject. -/// -/// The contract cmd-shims documented, unchanged and now the only one: the -/// subject on stdin, its kind in the environment, 0 to pass, 1 to refuse, 2 to -/// say it could not look. A checker written in anything satisfies it. -/// -/// All three pipes are worked at once, and that is not tidiness. A pipe holds -/// about 64 KiB: writing a longer subject blocks until the checker reads it, -/// and a checker that writes more than a bufferful blocks until this process -/// reads THAT. Writing the whole subject first and reading afterwards means -/// each side is waiting for the other and neither ever moves -- on exactly the -/// long bodies a guard most needs to see, and with no output at all to say -/// what happened. -fn consult(root: &Path, rule: &Rule, subject: &Subject) -> Result> { - // `exec`, not `values_from`. They were one field called `run` in v2, and a - // checker whose command reads empty passes everything it is asked about -- - // which is also why an ABSENT command is an error here rather than a - // default: a rule mis-dispatched to this function once ran `sh -c ""`, - // approved everything, and the pass was indistinguishable from a checker - // that looked. - let Some(run) = rule.exec().filter(|command| !command.trim().is_empty()) else { - return Err(Fatal::new(format!( - "{}: consulted as an `exec` checker while declaring no `exec` command, so \ - nothing could have been asked -- a dispatch hole, not a pass", - rule.id - ))); - }; - let mut child = Command::new("sh") - .arg("-c") - .arg(run) - .current_dir(root) - .env("UPHOLD_KIND", subject.kind) - .stdin(Stdio::piped()) - .stderr(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn() - .map_err(|error| Fatal::new(format!("{}: {error}", rule.id)))?; - - let mut stdin = child.stdin.take(); - let mut stdout = child.stdout.take(); - let mut stderr = child.stderr.take(); - let body = subject.value.as_bytes(); - - let (written, report) = - std::thread::scope(|scope| -> Result<(std::io::Result<()>, Vec)> { - let writer = scope.spawn(move || -> std::io::Result<()> { - let Some(pipe) = stdin.as_mut() else { - return Ok(()); - }; - pipe.write_all(body)?; - pipe.flush() - // Dropped on the way out of this closure, which is the end of - // input the checker is waiting for. - }); - // Drained rather than read: the contract puts a checker's report on - // stderr, but a checker that writes to stdout still fills a pipe, - // and a full pipe nobody empties is the same deadlock from the - // other side. - let drain = scope.spawn(move || { - if let Some(pipe) = stdout.as_mut() { - drop(pipe.read_to_end(&mut Vec::new())); - } - }); - let mut report = Vec::new(); - if let Some(pipe) = stderr.as_mut() { - drop(pipe.read_to_end(&mut report)); - } - let written = writer.join().map_err(|_| { - Fatal::new(format!( - "{}: the thread feeding it the subject died", - rule.id - )) - })?; - drain.join().map_err(|_| { - Fatal::new(format!("{}: the thread draining its output died", rule.id)) - })?; - Ok((written, report)) - })?; - - let status = child - .wait() - .map_err(|error| Fatal::new(format!("{}: {error}", rule.id)))?; - let report = String::from_utf8_lossy(&report); - let report = report.trim(); - match (status.code(), written) { - // A refusal stands even where the write did not finish. A checker that - // stopped reading and then said no had already seen enough to say it, - // and turning that into an infrastructure error would teach people to - // re-run until the refusal went away. - (Some(1), _) => Ok(Some(format!( - "{} refused a {} subject: {report}", - rule.id, subject.kind - ))), - // The write result is an answer, not something to drop with `.ok()`: a - // subject that never arrived is the one case where a 0 means nothing at - // all, because the checker approved whatever part of it got through and - // that is not what this invocation is about to publish. - (_, Err(error)) => Err(Fatal::new(format!( - "{} did not take the whole {} subject ({error}), so its answer is not about what \ - would be published: {report}", - rule.id, subject.kind - ))), - (Some(0), Ok(())) => Ok(None), - // 2 is could-not-look, and it is not a pass. A checker that could not - // read what it was handed has established nothing. - (other, Ok(())) => Err(Fatal::new(format!( - "{} exited {} on a {} subject: {report}", - rule.id, - other.unwrap_or(-1), - subject.kind - ))), - } -} - /// What a file IS, not where it sits: device and inode. `canonicalize` sees /// through symlinks but a hard link canonicalizes to its own path, so a path /// comparison would call a hard-linked shim "the real command". Metadata @@ -6191,92 +5686,6 @@ fn install_editor( Ok(()) } -/// The scope one rule is judged under: its own where it wrote one, the -/// table's where it did not. -fn effective_scope<'a>(rule: &'a Rule, shim: &'a Shim) -> &'a Scope { - rule.command - .as_ref() - .and_then(|where_| where_.scope.as_ref()) - .unwrap_or(&shim.scope) -} - -/// One invocation's scope answers, each predicate evaluated at most once. -/// -/// A memo rather than a per-rule call, because `public-target` asks a forge -/// and three rules behind one table would ask it three times -- and because -/// the could-not-resolve arm says its piece on stderr, which said three times -/// reads as three failures. -#[derive(Default)] -struct ScopeMemo { - answers: BTreeMap, -} - -impl ScopeMemo { - /// Whether this scope holds, with a predicate that could not be asked - /// answered by the table's `unresolved`. - /// - /// The conversion lives here rather than at each call site because there - /// are six of them and one of them forgetting is the whole defect: a - /// could-not-tell read as "does not hold" skips the checkers, and the - /// destination guard behind them promises exit 2 on exactly that. - fn holds( - &mut self, - shim: &Shim, - scope: &Scope, - root: &Path, - policy: &Policy, - collected: &Collected, - argv: &[String], - ) -> Result { - match self.standing(shim, scope, root, policy, collected, argv)? { - Standing::Holds => Ok(true), - Standing::DoesNotHold => Ok(false), - Standing::CouldNotTell(why) => match shim.unresolved { - Unresolved::Refuse => Err(Fatal::new(format!( - "{}: {why}. A check that could not look is not a check that passed, so \ - nothing was published. Name the destination explicitly, run this where \ - the repository has a remote and the forge can be reached, or write \ - `unresolved = \"run\"` on this `[[shim]]` table to run the command \ - unchecked instead", - shim.command - ))), - Unresolved::Run => Ok(false), - }, - } - } - - /// The predicate's own answer, asked once per invocation. - /// - /// The `run` half of `unresolved` says its piece here rather than in - /// `holds`, so a table that stands three rules down over one unreachable - /// forge prints one line and not three. - fn standing( - &mut self, - shim: &Shim, - scope: &Scope, - root: &Path, - policy: &Policy, - collected: &Collected, - argv: &[String], - ) -> Result { - let key = match scope { - Scope::Always => return Ok(Standing::Holds), - Scope::PublicTarget => String::from("public-target"), - Scope::PublicRegistry => String::from("public-registry"), - Scope::Command { command } => format!("command:{command}"), - }; - if let Some(answer) = self.answers.get(&key) { - return Ok(answer.clone()); - } - let answer = shim.scope_holds(scope, root, policy, collected, argv)?; - if let (Standing::CouldNotTell(why), Unresolved::Run) = (&answer, shim.unresolved) { - eprintln!("uphold shim: {why}, and the command ran anyway. This is not a pass."); - } - self.answers.insert(key, answer.clone()); - Ok(answer) - } -} - /// Run the user's editor, then judge what it produced. /// /// Refusing here is what makes it a checkpoint rather than a report: `gh` and @@ -6369,22 +5778,21 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> // `text-guards` rule whose own scope held runs other rules, and each of // those is judged under its own scope off this same memo. let mut scoping = match policy.shims.iter().find(|shim| shim.command == name) { - Some(shim) => Some((shim, shim.collect(root, &opened_for)?, ScopeMemo::default())), + Some(shim) => { + let collected = shim.collect(root, &opened_for)?; + Some(( + shim, + CommandContext::new(shim, root, &collected, &opened_for), + )) + } None => None, }; let checkers: Vec<&Rule> = match &mut scoping { - Some((shim, collected, scopes)) => { + Some((shim, scopes)) => { let shim = *shim; let mut kept: Vec<&Rule> = Vec::new(); for rule in &named { - if scopes.holds( - shim, - effective_scope(rule, shim), - root, - policy, - collected, - &opened_for, - )? { + if scopes.holds(effective_scope(rule, shim), policy)? { kept.push(*rule); } } @@ -6403,9 +5811,10 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> // this command means no destination either, and the text guards judge // against `origin` as every seam without one does. let destination = match &scoping { - Some((shim, collected, _)) => shim.resolve_target(root, collected)?, + Some((_, context)) => context.destination()?.target.clone(), None => None, }; + let invocation = Invocation::command(name, std::slice::from_ref(&subject)); let mut refusals: Vec = Vec::new(); for rule in checkers { if crate::guard::bypassed(&rule.id) { @@ -6422,7 +5831,7 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> let Some(kind) = crate::text::Judged::of(rule) else { continue; }; - if !rule.judged_at(crate::text::Seam::Command) { + if !rule.judged_at(invocation.seam) { continue; } match kind { @@ -6439,14 +5848,7 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> // As in `run`: only a rule this command names carries a // scope written about it here. _ if !named.iter().any(|stands| stands.id == inner.id) => Ok(true), - Some((shim, collected, scopes)) => scopes.holds( - shim, - effective_scope(inner, shim), - root, - policy, - collected, - &opened_for, - ), + Some((shim, scopes)) => scopes.holds(effective_scope(inner, shim), policy), // No `[[shim]]` table names this command, so there is no // scope to judge under and nothing to stand down for. None => Ok(true), @@ -6663,7 +6065,6 @@ pub(crate) fn run( // Whether the message the forge composes was asked for and judged, which // makes the notice that it was not untrue. let mut composed_read = false; - let mut scopes = ScopeMemo::default(); let reading = shim.reading(&words); if let Reading::Unclear(flag) = &reading { // Said out loud, and for the same reason the unresolvable-target arm in @@ -6702,16 +6103,10 @@ pub(crate) fn run( ))); } collected = shim.collect(root, &words)?; - let in_scope = scopes.holds(shim, &shim.scope, root, policy, &collected, &words)?; + let mut scopes = CommandContext::new(shim, root, &collected, &words); + let in_scope = scopes.holds(&shim.scope, policy)?; for rule in &checkers { - if scopes.holds( - shim, - effective_scope(rule, shim), - root, - policy, - &collected, - &words, - )? { + if scopes.holds(effective_scope(rule, shim), policy)? { any_applies = true; } } @@ -6750,11 +6145,12 @@ pub(crate) fn run( // subject loop, and asked even where the invocation collected no // subject at all. // - // Resolved ONCE, before the loop, and only where some rule judges a - // destination. `resolve_target` reads `--repo`/`-R` off argv and + // Read from the invocation's one observation, before the loop, where + // some rule judges a + // destination. The context reads the collected `--repo`/`-R` target, // otherwise `GH_REPO` for gh, then `git remote get-url origin`; it - // asks no forge. An ordinary invocation still pays no round trip: `Scope::PublicTarget`'s - // `visibility()` is not called from here at all, and the forge question + // asks no forge. An ordinary invocation still pays no round trip: the + // visibility lookup is not called from here at all, and the forge question // inside `target_refusal` is reached only for a destination the // allow-list has ALREADY refused -- a call that was about to be stopped // either way. Nothing that would have run waits on somebody's service. @@ -6769,11 +6165,12 @@ pub(crate) fn run( rule.builtin() .is_some_and(|builtin| crate::guard::TARGET_GUARDS.contains(&builtin)) }) { - let target = shim.resolve_target(root, &collected)?; + let destination = scopes.destination()?; + let target = destination.target.clone(); // A qualified override or origin carries its own host, which // takes precedence over gh's ambient default for both probes. - let hostname = target.as_deref().and_then(git::host); - let forge = shim.forge(root); + let hostname = destination.hostname.clone(); + let forge = destination.forge; let mut asked = BTreeMap::new(); for rule in &checkers { if crate::guard::bypassed(&rule.id) { @@ -6782,14 +6179,7 @@ pub(crate) fn run( // The rule's own scope where it wrote one, the table's where it // did not -- the same reading the text path takes, answered // from the same memo. - if !scopes.holds( - shim, - effective_scope(rule, shim), - root, - policy, - &collected, - &words, - )? { + if !scopes.holds(effective_scope(rule, shim), policy)? { continue; } if let Some(refusal) = crate::guard::target_refusal( @@ -6822,8 +6212,9 @@ pub(crate) fn run( // path says rather than whatever checkout the command was typed // in. The resolver the `public-target` scope and the destination // guard read, so the three cannot disagree about one invocation. - let destination = shim.resolve_target(root, &collected)?; - for subject in &collected.subjects { + let destination = scopes.destination()?.target.clone(); + let invocation = Invocation::command(name, &collected.subjects); + for subject in invocation.command_subjects()? { // An empty subject is not a subject a checker can read: a // program handed "" on stdin, or a text guard asked whether "" // names a private repository, is being asked to judge text that @@ -6854,14 +6245,7 @@ pub(crate) fn run( // The rule's own scope where it wrote one, the table's // where it did not -- answered from the memo, so a // destination is looked up once however many rules ask. - if !scopes.holds( - shim, - effective_scope(rule, shim), - root, - policy, - &collected, - &words, - )? { + if !scopes.holds(effective_scope(rule, shim), policy)? { continue; } // `subjects` narrows every kind of checker the same way: @@ -6877,7 +6261,7 @@ pub(crate) fn run( let Some(kind) = crate::text::Judged::of(rule) else { continue; }; - if !rule.judged_at(crate::text::Seam::Command) { + if !rule.judged_at(invocation.seam) { continue; } match kind { @@ -6914,14 +6298,7 @@ pub(crate) fn run( if !checkers.iter().any(|named| named.id == inner.id) { return Ok(true); } - scopes.holds( - shim, - effective_scope(inner, shim), - root, - policy, - &collected, - &words, - ) + scopes.holds(effective_scope(inner, shim), policy) }; if let Some(refusal) = crate::guard::text_refusal( root, @@ -8064,6 +7441,49 @@ mod tests { ); } + #[test] + fn all_destination_consumers_share_the_first_observation() { + let root = crate::fixture::scratch("shim-destination-context"); + std::fs::create_dir_all(&root).unwrap(); + crate::fixture::git(&root, &["init", "-q"]); + crate::fixture::git( + &root, + &[ + "remote", + "add", + "origin", + "https://github.com/example-user/first", + ], + ); + let mut shim = gh(); + // This adapter does not interpret ambient gh environment variables. + shim.command = String::from("faux"); + let words = argv("pr create"); + let context = CommandContext::new(&shim, &root, &Collected::default(), &words); + assert_eq!( + context.destination().unwrap().target.as_deref(), + Some("example-user/first") + ); + crate::fixture::git( + &root, + &[ + "remote", + "set-url", + "origin", + "https://github.com/example-user/second", + ], + ); + assert_eq!( + context.destination().unwrap().target.as_deref(), + Some("example-user/first") + ); + let next = CommandContext::new(&shim, &root, &Collected::default(), &words); + assert_eq!( + next.destination().unwrap().target.as_deref(), + Some("example-user/second") + ); + } + #[test] fn one_scope_predicate_is_asked_once_however_many_rules_ask_it() { // `public-target` asks a forge, and three rules behind one table would @@ -8076,33 +7496,13 @@ mod tests { let scope = Scope::Command { command: format!("echo asked >> {}", asked.display()), }; - let mut memo = ScopeMemo::default(); + let shim = npm(); + let words = argv("publish"); + let mut memo = CommandContext::new(&shim, &dir, &Collected::default(), &words); for _ in 0..3 { - assert!( - memo.holds( - &npm(), - &scope, - &dir, - &undeclared(), - &Collected::default(), - &argv("publish") - ) - .unwrap() - ); - // A different predicate is a different key, and is not answered - // out of the first one's memo. - assert!( - !memo - .holds( - &npm(), - &Scope::PublicRegistry, - &dir, - &undeclared(), - &Collected::default(), - &argv("publish") - ) - .unwrap() - ); + assert!(memo.holds(&scope, &undeclared()).unwrap()); + // Different predicates retain separate answers in one context. + assert!(!memo.holds(&Scope::PublicRegistry, &undeclared()).unwrap()); } assert_eq!( std::fs::read_to_string(&asked).unwrap().lines().count(), diff --git a/src/shim/checker.rs b/src/shim/checker.rs new file mode 100644 index 0000000..adb6b08 --- /dev/null +++ b/src/shim/checker.rs @@ -0,0 +1,194 @@ +//! Evaluate one declared checker against one normalized subject. +//! Selection, scope and check order belong to the shim orchestrator. + +use std::io::{Read, Write}; +use std::path::Path; +use std::process::{Command, Stdio}; + +use super::Subject; +use crate::config::Rule; +use crate::error::{Fatal, Result}; + +/// One pattern rule's verdict over one subject. +/// +/// The same engine `scan` searches files with, asked about a string that never +/// becomes one -- same regex grammar, same meaning. `require_regexp` refuses a +/// subject the pattern is absent from; `regexp` refuses one it is present in. +/// Both name the kind and the pattern, because the reader's next step is to +/// fix the text, and "refused" without which-test-failed is a round trip. +/// +/// `prose_regexp` is the third, and the subject reaches it as whole-text prose: +/// a pull-request body is a document, so a fenced example in one is skipped +/// here exactly as it is in a committed document, and a sentence wrapped by +/// whatever composed the body is unwrapped before the pattern sees it. A rule +/// that refused a shape in a document and allowed it in the body announcing +/// that document would be one rule with two answers. +pub(super) fn pattern_refusal(rule: &Rule, subject: &Subject) -> Result> { + Ok(pattern_finding(rule, subject)?.map(|finding| format!("{finding}\n{}", rule.message()))) +} + +/// What a pattern rule found in one subject, as the line a report names it by, +/// without the rule's message. Split from [`pattern_refusal`] for the hook, +/// whose report carries the message as its own paragraph, the way `--text` +/// prints it. +pub(crate) fn pattern_finding(rule: &Rule, subject: &Subject) -> Result> { + let multiline = rule.files().multiline; + if let Some(pattern) = rule.prose_regexp() { + let matcher = crate::prose::compile(pattern, &rule.id)?; + let Some(span) = crate::prose::of_text(&subject.value) + .into_iter() + .find(|span| matcher.is_match(&span.text)) + else { + return Ok(None); + }; + return Ok(Some(format!( + "{}: the {} subject matches {pattern:?}: {}", + rule.id, subject.kind, span.text + ))); + } + if let Some(pattern) = rule.require_regexp() { + let hits = crate::engine::search_text( + &subject.value, + &crate::engine::Query::regex(pattern, multiline), + &rule.id, + )?; + if hits.is_empty() { + return Ok(Some(format!( + "{}: the {} subject does not satisfy {pattern:?}", + rule.id, subject.kind + ))); + } + return Ok(None); + } + let Some(pattern) = rule.regexp() else { + return Ok(None); + }; + let hits = crate::engine::search_text( + &subject.value, + &crate::engine::Query::regex(pattern, multiline), + &rule.id, + )?; + let Some(hit) = hits.first() else { + return Ok(None); + }; + Ok(Some(format!( + "{}: the {} subject matches {pattern:?}: {}", + rule.id, + subject.kind, + hit.text.trim_end() + ))) +} + +/// Run one checker over one subject. +/// +/// The contract cmd-shims documented, unchanged and now the only one: the +/// subject on stdin, its kind in the environment, 0 to pass, 1 to refuse, 2 to +/// say it could not look. A checker written in anything satisfies it. +/// +/// All three pipes are worked at once, and that is not tidiness. A pipe holds +/// about 64 KiB: writing a longer subject blocks until the checker reads it, +/// and a checker that writes more than a bufferful blocks until this process +/// reads THAT. Writing the whole subject first and reading afterwards means +/// each side is waiting for the other and neither ever moves -- on exactly the +/// long bodies a guard most needs to see, and with no output at all to say +/// what happened. +pub(super) fn consult(root: &Path, rule: &Rule, subject: &Subject) -> Result> { + // `exec`, not `values_from`. They were one field called `run` in v2, and a + // checker whose command reads empty passes everything it is asked about -- + // which is also why an ABSENT command is an error here rather than a + // default: a rule mis-dispatched to this function once ran `sh -c ""`, + // approved everything, and the pass was indistinguishable from a checker + // that looked. + let Some(run) = rule.exec().filter(|command| !command.trim().is_empty()) else { + return Err(Fatal::new(format!( + "{}: consulted as an `exec` checker while declaring no `exec` command, so \ + nothing could have been asked -- a dispatch hole, not a pass", + rule.id + ))); + }; + let mut child = Command::new("sh") + .arg("-c") + .arg(run) + .current_dir(root) + .env("UPHOLD_KIND", subject.kind) + .stdin(Stdio::piped()) + .stderr(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .map_err(|error| Fatal::new(format!("{}: {error}", rule.id)))?; + + let mut stdin = child.stdin.take(); + let mut stdout = child.stdout.take(); + let mut stderr = child.stderr.take(); + let body = subject.value.as_bytes(); + + let (written, report) = + std::thread::scope(|scope| -> Result<(std::io::Result<()>, Vec)> { + let writer = scope.spawn(move || -> std::io::Result<()> { + let Some(pipe) = stdin.as_mut() else { + return Ok(()); + }; + pipe.write_all(body)?; + pipe.flush() + // Dropped on the way out of this closure, which is the end of + // input the checker is waiting for. + }); + // Drained rather than read: the contract puts a checker's report on + // stderr, but a checker that writes to stdout still fills a pipe, + // and a full pipe nobody empties is the same deadlock from the + // other side. + let drain = scope.spawn(move || { + if let Some(pipe) = stdout.as_mut() { + drop(pipe.read_to_end(&mut Vec::new())); + } + }); + let mut report = Vec::new(); + if let Some(pipe) = stderr.as_mut() { + drop(pipe.read_to_end(&mut report)); + } + let written = writer.join().map_err(|_| { + Fatal::new(format!( + "{}: the thread feeding it the subject died", + rule.id + )) + })?; + drain.join().map_err(|_| { + Fatal::new(format!("{}: the thread draining its output died", rule.id)) + })?; + Ok((written, report)) + })?; + + let status = child + .wait() + .map_err(|error| Fatal::new(format!("{}: {error}", rule.id)))?; + let report = String::from_utf8_lossy(&report); + let report = report.trim(); + match (status.code(), written) { + // A refusal stands even where the write did not finish. A checker that + // stopped reading and then said no had already seen enough to say it, + // and turning that into an infrastructure error would teach people to + // re-run until the refusal went away. + (Some(1), _) => Ok(Some(format!( + "{} refused a {} subject: {report}", + rule.id, subject.kind + ))), + // The write result is an answer, not something to drop with `.ok()`: a + // subject that never arrived is the one case where a 0 means nothing at + // all, because the checker approved whatever part of it got through and + // that is not what this invocation is about to publish. + (_, Err(error)) => Err(Fatal::new(format!( + "{} did not take the whole {} subject ({error}), so its answer is not about what \ + would be published: {report}", + rule.id, subject.kind + ))), + (Some(0), Ok(())) => Ok(None), + // 2 is could-not-look, and it is not a pass. A checker that could not + // read what it was handed has established nothing. + (other, Ok(())) => Err(Fatal::new(format!( + "{} exited {} on a {} subject: {report}", + rule.id, + other.unwrap_or(-1), + subject.kind + ))), + } +} diff --git a/src/shim/destination.rs b/src/shim/destination.rs new file mode 100644 index 0000000..768f9a9 --- /dev/null +++ b/src/shim/destination.rs @@ -0,0 +1,486 @@ +//! Normalize one command destination and apply scope predicates to that same +//! observation. Grammar supplies collected targets; policy remains the caller +//! of the scope and guard decisions. No subjects or processes are handed off here. + +use std::collections::BTreeMap; +use std::path::Path; +use std::process::Command; + +use super::{ + Collected, Forge, Scope, Shim, Silence, Standing, Target, Unresolved, forge_field, + json_bool_field, repository_placeholder, +}; +use crate::config::{Policy, Rule}; +use crate::error::{Fatal, Result}; +use crate::git; + +/// How the adapter selected the destination. Only Origin can describe this +/// checkout's visibility; equal repository names do not establish that fact. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum Provenance { + /// This command has no destination semantics. + Absent, + /// A command option or API path named the destination. + Explicit, + /// The forge client's environment selected the destination. + Environment, + /// The checkout's origin selected the destination. + Origin, +} + +/// One destination observation shared by scopes, ownership and text guards. +#[derive(Debug)] +pub(super) struct Destination { + /// Canonical host-bearing repository spelling, or no resolved repository. + pub(super) target: Option, + /// Client that can answer for the supported destination. + pub(super) forge: Option, + /// Host override carried to both visibility and ownership queries. + pub(super) hostname: Option, + provenance: Provenance, + ambient_host: bool, +} + +impl Destination { + fn resolve(shim: &Shim, root: &Path, selected: Option<&str>) -> Result { + let explicit = selected.filter(|value| !value.is_empty()); + let gh_repo = shim.command == "gh" && shim.target != Target::None; + let environment = gh_repo + .then(|| { + std::env::var("GH_REPO") + .ok() + .filter(|value| !value.is_empty()) + }) + .flatten(); + let ambient_host = + shim.command == "gh" && std::env::var("GH_HOST").is_ok_and(|value| !value.is_empty()); + let (target, provenance) = if let Some(named) = explicit.or(environment.as_deref()) { + let target = if gh_repo && !repository_placeholder("gh", named)? { + gh_repository_target(named)? + } else { + named.to_owned() + }; + ( + Some(target), + if explicit.is_some() { + Provenance::Explicit + } else { + Provenance::Environment + }, + ) + } else if gh_repo { + ( + git::remote_url(root, "origin") + .map(|url| gh_repository_target(&url)) + .transpose()?, + Provenance::Origin, + ) + } else { + match shim.target { + Target::None => (None, Provenance::Absent), + Target::GitRemote | Target::ForgeRepo => ( + git::remote_url(root, "origin") + .and_then(|url| git::owner_repo(&url)) + .map(|(owner, repo)| format!("{owner}/{repo}")), + Provenance::Origin, + ), + } + }; + let hostname = target.as_deref().and_then(git::host); + Ok(Self { + target, + forge: shim.forge(root), + hostname, + provenance, + ambient_host, + }) + } +} + +impl Shim { + /// Whether one scope predicate holds for this invocation. + /// + /// The predicate is a parameter rather than the table's own, because a rule + /// may carry its own `command.scope`: the table's answers for the command, + /// and a rule that applies on every egress -- host identity in a + /// pull-request body is worth refusing whatever the destination -- says so + /// beside its `command.before` rather than inheriting the table's idea of + /// the question. + /// + /// The policy is a parameter for the sake of one arm: `public-target` is a + /// question this repository may already have answered in writing, and a + /// predicate that asks a forge anyway is a network call -- and a token, and + /// a rate limit -- spent on a fact that was sitting in `principles.toml` + /// the whole time. See [`declared_standing`]. + fn scope_answer( + &self, + scope: &Scope, + root: &Path, + policy: &Policy, + context: &CommandContext<'_>, + ) -> Result { + match scope { + Scope::Always => Ok(Standing::Holds), + Scope::PublicTarget => { + let destination = context.destination()?; + let Some(target) = destination.target.as_deref() else { + // No answer is not "public", and it is not "not public" + // either. `gh` unauthenticated, rate-limited, offline, or a + // repository with no `origin` all land here, and what the + // caller does about it is the caller's decision -- see + // [`Unresolved`]. + return Ok(Standing::CouldNotTell(String::from( + "no target could be resolved, so whether the `public-target` checks \ + apply here could not be established", + ))); + }; + // What this repository says about itself, before anybody is + // asked. Read here and not before the resolver, because a tree + // with no `origin` has no destination for a declaration to be + // about, and that is the resolver's answer above rather than + // this one's. + if let Some(standing) = declared_standing(root, policy, destination)? { + return Ok(standing); + } + match Self::visibility(destination).as_deref() { + Ok("public") => Ok(Standing::Holds), + Ok(_) => Ok(Standing::DoesNotHold), + // The cause, in the words the forge used. A rate limit, an + // unauthenticated `gh` and a deleted repository are three + // different instructions to the reader, and a refusal + // naming none of them is one the reader can only bypass. + Err(silence) => Ok(Standing::CouldNotTell(format!( + "the forge did not say whether {target} is public, so whether the \ + `public-target` checks apply here could not be established. {}", + silence.sentence() + ))), + } + } + Scope::PublicRegistry => { + if context.argv.iter().any(|argument| argument == "--dry-run") { + return Ok(Standing::DoesNotHold); + } + // Two independent reasons this is nobody's business, and either + // one is enough: a package marked private cannot be published + // at all, and a registry that is not the public one is + // somebody's internal infrastructure. + if let Ok(text) = std::fs::read_to_string(root.join("package.json")) + && text.contains("\"private\"") + && json_bool_field(&text, "private") + { + return Ok(Standing::DoesNotHold); + } + let registry = context + .target + .clone() + .unwrap_or_else(|| String::from("https://registry.npmjs.org")); + Ok(if registry.contains("registry.npmjs.org") { + Standing::Holds + } else { + Standing::DoesNotHold + }) + } + Scope::Command { command } => { + let status = Command::new("sh") + .arg("-c") + .arg(command) + .current_dir(root) + .status() + .map_err(|error| { + Fatal::new(format!("{}: scope command: {error}", self.command)) + })?; + Ok(if status.success() { + Standing::Holds + } else { + Standing::DoesNotHold + }) + } + } + } + + /// Which forge can answer for this repository, where that has an answer. + pub(super) fn forge(&self, root: &Path) -> Option { + // The command is the strongest evidence there is: somebody running + // `glab` is publishing to GitLab whatever else is configured. + match self.command.as_str() { + "gh" => return Some(Forge::GitHub), + "glab" => return Some(Forge::GitLab), + _ => {} + } + // Otherwise the remote decides, which is what the `git` shim needs: + // one shim stands in front of a command that pushes to either. + Forge::of_url(&git::remote_url(root, "origin")?) + } + + /// What the forge says the target's visibility is, in the forge's own word. + /// + /// Asking `gh` for every target is why the shipped `glab` shim could never + /// resolve one: `gh api repos//` answers about GitHub and + /// about nothing else, so a GitLab remote fell through the `None` arm on + /// every invocation and a shim declared `public-target` was inert. + /// + /// Both vocabularies come back unchanged, and GitLab's `internal` is why: + /// it means public to everyone with an account on the instance, which is + /// neither public to the internet nor private. Only `public` is treated as + /// public by the caller, so a forge that grows a fourth word does not + /// quietly become one of the three. + fn visibility(destination: &Destination) -> std::result::Result { + let Some(forge) = destination.forge else { + return Err(Silence::Refused(String::from( + "no forge client is known for that host, so there was nobody to ask", + ))); + }; + let target = destination.target.as_deref().unwrap_or_default(); + match forge { + Forge::GitHub => { + let name = git::owner_repo(target).map_or_else( + || target.to_owned(), + |(owner, repo)| format!("{owner}/{repo}"), + ); + let endpoint = format!("repos/{name}"); + let mut args = vec!["api", endpoint.as_str(), "--jq", ".visibility"]; + if let Some(host) = destination.hostname.as_deref() { + args.extend(["--hostname", host]); + } + forge_field("gh", &args, None) + } + // Deliberately not `--jq`: `glab api` is not `gh api`, and a shim + // that is inert the day one flag differs is the defect this arm + // exists to end. The project id is a path, so its separators are + // escaped -- `owner/name` and `group/sub/name` are each one id. + Forge::GitLab => forge_field( + "glab", + &["api", &format!("projects/{}", target.replace('/', "%2F"))], + Some("visibility"), + ), + } + } + + #[cfg(test)] + pub(crate) fn scope_holds( + &self, + scope: &Scope, + root: &Path, + policy: &Policy, + collected: &Collected, + argv: &[String], + ) -> Result { + self.scope_answer( + scope, + root, + policy, + &CommandContext::new(self, root, collected, argv), + ) + } +} + +/// Resolve the host-bearing spellings accepted by `gh` without treating an +/// enterprise repository as its same-named github.com counterpart. Enterprise +/// support is deliberately absent here, as it is in `Forge::of_url`. +fn gh_repository_target(named: &str) -> Result { + let parts: Vec<_> = named.split('/').collect(); + let host = if named.contains("://") || named.contains(':') { + git::host(named) + } else if parts.len() == 3 { + parts.first().map(|hostname| hostname.to_lowercase()) + } else { + None + }; + let environment_host = std::env::var("GH_HOST") + .ok() + .filter(|configured| !configured.is_empty()); + if let Some(hostname) = host.as_ref().or(environment_host.as_ref()) + && !matches!( + hostname.to_lowercase().as_str(), + "github.com" | "www.github.com" + ) + { + return Err(Fatal::new(format!( + "gh: destination {named:?} uses host {hostname:?}, whose repository authority \ + cannot be verified by this shim. Nothing was published; use a supported \ + GitHub destination with a matching GH_HOST" + ))); + } + let path = if named.contains("://") || named.contains(':') { + git::owner_repo(named).map(|(owner, repo)| format!("{owner}/{repo}")) + } else { + match parts.as_slice() { + [owner, repo] | [_, owner, repo] => Some(format!("{owner}/{repo}")), + _ => None, + } + }; + let Some(path) = path.filter(|path| { + path.split('/').all(|part| { + !part.is_empty() + && part + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || b"-_.".contains(&byte)) + }) + }) else { + return Err(Fatal::new(format!( + "gh: destination {named:?} could not be resolved as [HOST/]OWNER/REPO. \ + Nothing was published" + ))); + }; + Ok(match host { + Some(hostname) => format!("https://{hostname}/{path}"), + None => path, + }) +} + +/// What the policy already says about this destination, where it speaks for it. +/// +/// `Some` is a declaration that settles the question offline, and it is the same +/// declaration `guard::names::target_is_public` reads: a repository whose +/// `principles.toml` says `visibility = "private"` has stated the fact, and a +/// shim that ran `gh api repos//` anyway needed an authenticated +/// `gh` on every `git push` to learn it a second time. When GitHub rate-limits +/// the account that lookup fails, the failure is a [`Standing::CouldNotTell`], +/// and `unresolved = "refuse"` -- the default -- stops every push in every tree +/// over a fact nobody had to ask for. +/// +/// `None` sends the caller to the forge, and the narrowing that makes this safe +/// is the destination it will read the declaration for. A declaration is about +/// ONE repository, so this reads it only where the destination is the one +/// [`Destination::resolve`] took from `origin` -- which is to say where nothing +/// on the command line or in `GH_REPO` named a destination, and `GH_HOST` +/// did not change the host context. A `target_flags` value is a +/// destination somebody typed, and a typed `owner/repo` carries no host: `-R +/// acme/widget` under a `--hostname` or a `GH_HOST` is a repository on another +/// forge that happens to share a name with this one, and there is nothing in +/// two matching path segments that can tell it apart from this tree. So every +/// named destination goes to the forge exactly as it did before, including the +/// ones that name this repository, and what is bought offline is the case the +/// cost was actually in: the push that names nothing and means `origin`. +fn declared_standing( + root: &Path, + policy: &Policy, + destination: &Destination, +) -> Result> { + if destination.provenance != Provenance::Origin || destination.ambient_host { + return Ok(None); + } + // Lazily, and only here. Reading it is allowed to run `visibility_from`, which + // is a process, and a predicate that spawned one for a target the declaration + // does not speak for would trade the lookup this exists to avoid for another + // one. + let Some(declared) = policy.declared_visibility(root)? else { + return Ok(None); + }; + // Held to the three spellings before it arrives -- a written one at load, a + // `visibility_from` answer as it is read -- so `None` here is unreachable + // rather than a fourth word being guessed at. It falls through to the forge + // regardless: a word this seam cannot read is not an answer this seam may + // invent. + Ok( + crate::config::visibility_is_public(&declared).map(|public| { + if public { + Standing::Holds + } else { + Standing::DoesNotHold + } + }), + ) +} + +/// The scope one rule is judged under: its own where it wrote one, the +/// table's where it did not. +pub(super) fn effective_scope<'a>(rule: &'a Rule, shim: &'a Shim) -> &'a Scope { + rule.command + .as_ref() + .and_then(|where_| where_.scope.as_ref()) + .unwrap_or(&shim.scope) +} + +/// One invocation's scope answers, each predicate evaluated at most once. +/// +/// A memo rather than a per-rule call, because `public-target` asks a forge +/// and three rules behind one table would ask it three times -- and because +/// the could-not-resolve arm says its piece on stderr, which said three times +/// reads as three failures. +pub(super) struct CommandContext<'a> { + shim: &'a Shim, + root: &'a Path, + argv: &'a [String], + target: Option, + answers: BTreeMap, + destination: std::cell::OnceCell>, +} + +impl<'a> CommandContext<'a> { + /// Bind all decisions to one collected target and canonical argument list. + pub(super) fn new( + shim: &'a Shim, + root: &'a Path, + collected: &Collected, + argv: &'a [String], + ) -> Self { + Self { + shim, + root, + argv, + target: collected.target.clone(), + answers: BTreeMap::new(), + destination: std::cell::OnceCell::new(), + } + } + /// Resolve lazily, once: an out-of-scope command keeps its previous behavior, + /// while every consumer that needs a destination sees the same observation. + pub(super) fn destination(&self) -> Result<&Destination> { + self.destination + .get_or_init(|| { + Destination::resolve(self.shim, self.root, self.target.as_deref()) + .map_err(|error| error.to_string()) + }) + .as_ref() + .map_err(|why| Fatal::new(why.clone())) + } + + /// Whether this scope holds, with a predicate that could not be asked + /// answered by the table's `unresolved`. + /// + /// The conversion lives here rather than at each call site because there + /// are six of them and one of them forgetting is the whole defect: a + /// could-not-tell read as "does not hold" skips the checkers, and the + /// destination guard behind them promises exit 2 on exactly that. + pub(super) fn holds(&mut self, scope: &Scope, policy: &Policy) -> Result { + match self.standing(scope, policy)? { + Standing::Holds => Ok(true), + Standing::DoesNotHold => Ok(false), + Standing::CouldNotTell(why) => match self.shim.unresolved { + Unresolved::Refuse => Err(Fatal::new(format!( + "{}: {why}. A check that could not look is not a check that passed, so \ + nothing was published. Name the destination explicitly, run this where \ + the repository has a remote and the forge can be reached, or write \ + `unresolved = \"run\"` on this `[[shim]]` table to run the command \ + unchecked instead", + self.shim.command + ))), + Unresolved::Run => Ok(false), + }, + } + } + + /// The predicate's own answer, asked once per invocation. + /// + /// The `run` half of `unresolved` says its piece here rather than in + /// `holds`, so a table that stands three rules down over one unreachable + /// forge prints one line and not three. + fn standing(&mut self, scope: &Scope, policy: &Policy) -> Result { + let key = match scope { + Scope::Always => return Ok(Standing::Holds), + Scope::PublicTarget => String::from("public-target"), + Scope::PublicRegistry => String::from("public-registry"), + Scope::Command { command } => format!("command:{command}"), + }; + if let Some(answer) = self.answers.get(&key) { + return Ok(answer.clone()); + } + let answer = self.shim.scope_answer(scope, self.root, policy, self)?; + if let (Standing::CouldNotTell(why), Unresolved::Run) = (&answer, self.shim.unresolved) { + eprintln!("uphold shim: {why}, and the command ran anyway. This is not a pass."); + } + self.answers.insert(key, answer.clone()); + Ok(answer) + } +} diff --git a/src/text.rs b/src/text.rs index d570df0..83109e7 100644 --- a/src/text.rs +++ b/src/text.rs @@ -23,6 +23,7 @@ use std::path::PathBuf; use crate::config::{self, Check, CheckKind, Literals, Policy, Rule}; use crate::engine::{self, Query}; use crate::error::{Exit, Fatal, Result, verdict}; +use crate::invocation::{Invocation, Observed}; use crate::report::Failure; use crate::sources; @@ -299,24 +300,24 @@ pub(crate) struct Headline { /// Everything a piece of published text is judged by, at one seam. /// -/// The single assembly. `label` is what the subject is called in a report -- -/// the tool name at the hook, the source at `--text` -- and is also matched as -/// the hook's `tool` subject. `text` is the prose; `headlines` are the subjects -/// the seam could tell apart from it, which only the harness hook can, by a tool call's -/// argument names. Every checker but the guards reads the two joined, as it -/// always has; `None` means the call has no argument text, while its tool name -/// remains a subject. The guards are handed each headline as one, so the -/// lookalike pass a title gets at the shim is the pass it gets here. +/// The single assembly. The invocation carries report identity, observed prose +/// and separately identified headlines. Only the hook exposes that identity as +/// a `tool` subject; raw text filenames are labels. An absent prose subject +/// leaves the tool observable, while unread required prose stops judgment. +/// Checkers read prose and headlines joined; guards receive each headline +/// separately, preserving the same headline treatment as command collection. pub(crate) fn judged( - seam: Seam, + invocation: &Invocation<'_>, root: &std::path::Path, policy: &Policy, - label: &str, - text: Option<&str>, - headlines: &[Headline], ) -> Result> { - let has_text = text.is_some() || !headlines.is_empty(); - let text = text.unwrap_or_default(); + let input = invocation.text_input()?; + let prose = input.prose.read()?.copied(); + let headlines = input.headlines; + let seam = invocation.seam; + let label = invocation.label; + let has_text = prose.is_some() || !headlines.is_empty(); + let text = prose.unwrap_or_default(); let joined; let whole = if headlines.is_empty() { text @@ -389,12 +390,16 @@ pub(crate) fn judged( .collect(), // Only the hook consults these here, and only the rules whose // `seams` names it -- see `Seam::runs_by_default`. - Judged::Patterns => { - patterns_over(policy, seam, label, has_text.then_some(whole), headlines)? - .into_iter() - .map(Verdict::Rule) - .collect() - } + Judged::Patterns => patterns_over( + policy, + seam, + invocation.tool(), + has_text.then_some(whole), + headlines, + )? + .into_iter() + .map(Verdict::Rule) + .collect(), // The shim's, and the shim does not arrive here: it holds a subject // and a per-rule scope this function is not given, so it dispatches // rule by rule through `Judged::of` instead. @@ -474,7 +479,7 @@ fn merged_by_rule(refusals: Vec) -> Vec, text: Option<&str>, headlines: &[Headline], ) -> Result> { @@ -497,14 +502,16 @@ fn patterns_over( }, )); } - subjects.push(( - None, - crate::shim::Subject { - kind: "tool", - value: label.to_owned(), - added: None, - }, - )); + if let Some(tool) = tool { + subjects.push(( + None, + crate::shim::Subject { + kind: "tool", + value: tool.to_owned(), + added: None, + }, + )); + } subjects.extend(headlines.iter().map(|headline| { ( Some(headline.label.as_str()), @@ -554,7 +561,8 @@ fn patterns_over( pub(crate) fn check(found: Option<&(PathBuf, PathBuf)>, source: &str) -> Result { let text = read(source)?; let (root, policy) = load_for(found)?; - let verdicts = judged(Seam::Scan, &root, &policy, source, Some(&text), &[])?; + let invocation = Invocation::text(Seam::Scan, source, Observed::Present(&text), &[]); + let verdicts = judged(&invocation, &root, &policy)?; for verdict in &verdicts { match verdict { diff --git a/tests/publication_paths.rs b/tests/publication_paths.rs new file mode 100644 index 0000000..6006ebe --- /dev/null +++ b/tests/publication_paths.rs @@ -0,0 +1,188 @@ +//! The same applicable policy fixture through every supported publication path. +//! Process exits and the harness refusal document are different protocols; both +//! must distinguish refusal, permission and an unread required surface. + +#![expect( + clippy::let_underscore_must_use, + clippy::tests_outside_test_module, + clippy::unwrap_used, + reason = "The fixture drives processes; a harness failure is a test failure" +)] + +mod support; + +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output, Stdio}; + +const POLICY: &str = r#" +[rule.publication-word] +message = "Remove the forbidden publication word." +prose_regexp = 'publication-forbidden-word' +files.include = ["."] +command.before = ["faux"] + +[[shim]] +command = "faux" +match = ["pr:create"] +text_flags = ["-b", "--body"] +scope = "always" +"#; + +fn workspace() -> PathBuf { + let root = support::scratch("publication-paths"); + std::fs::create_dir_all(root.join("policy")).unwrap(); + std::fs::create_dir_all(root.join("bin")).unwrap(); + std::fs::write(root.join("policy/principles.toml"), POLICY).unwrap(); + let command = root.join("bin/faux"); + std::fs::write(&command, "#!/bin/sh\nprintf 'publication command ran\\n'\n").unwrap(); + let mut permissions = std::fs::metadata(&command).unwrap().permissions(); + std::os::unix::fs::PermissionsExt::set_mode(&mut permissions, 0o755); + std::fs::set_permissions(command, permissions).unwrap(); + root +} + +fn invoke(root: &Path, args: &[&str], input: &str, editor: bool) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_uphold")); + command + .args(args) + .current_dir(root) + .env( + "PATH", + format!( + "{}:{}", + root.join("bin").display(), + std::env::var("PATH").unwrap_or_default() + ), + ) + .env("HOME", "/srv/example") + .env_remove("UPHOLD_ALLOW") + .env_remove("UPHOLD_SHIM_INNER") + .env_remove("GH_REPO") + .env_remove("GH_HOST") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if editor { + std::fs::write(root.join("body.md"), input).unwrap(); + command + .env("UPHOLD_SHIM_EDITOR_REAL", "true") + .env("UPHOLD_SHIM_EDITOR_ARGV", "pr create"); + } + let mut child = command.spawn().unwrap(); + if let Err(error) = child.stdin.take().unwrap().write_all(input.as_bytes()) { + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe); + } + child.wait_with_output().unwrap() +} + +#[test] +fn one_publication_fixture_is_refused_and_allowed_at_every_applicable_entry_path() { + let root = workspace(); + for (text, refused) in [ + ("publication-forbidden-word", true), + ("ordinary publication", false), + ] { + // Effect-by-entry-path matrix: each path reads the same prose and rule. + // Hook permission is exit 0 plus an absent refusal document; process + // seams use exit 1 for refusal. Shim permission also reaches handoff. + for path in ["scan", "guard", "hook", "shim", "editor"] { + let event = + serde_json::json!({"tool_name":"create_publication", "tool_input":{"body":text}}) + .to_string(); + let output = match path { + "scan" => invoke(&root, &["scan", "--text", "-"], text, false), + "guard" => invoke(&root, &["guard", "--text", "-"], text, false), + "hook" => invoke(&root, &["hook", "codex"], &event, false), + "shim" => invoke( + &root, + &["shim", "faux", "pr", "create", "-b", text], + "", + false, + ), + "editor" => invoke( + &root, + &["shim", "--as-editor", "faux", "body.md"], + text, + true, + ), + _ => unreachable!(), + }; + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + assert_eq!( + output.status.code(), + Some(if path == "hook" { + 0 + } else { + i32::from(refused) + }), + "{path}: {stderr}" + ); + assert_eq!( + stderr.contains("publication-word"), + refused, + "{path}: {stderr}" + ); + if path == "hook" { + if refused { + let document: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!( + document + .pointer("/hookSpecificOutput/permissionDecision") + .and_then(serde_json::Value::as_str), + Some("deny") + ); + } else { + assert!(stdout.is_empty(), "{stdout}"); + } + } + if path == "shim" { + assert_eq!( + stdout.contains("publication command ran"), + !refused, + "{stdout}" + ); + } + } + } +} + +#[test] +fn unread_required_surfaces_never_report_permission() { + let root = workspace(); + for path in ["scan", "guard", "hook", "shim"] { + let output = match path { + "scan" => invoke(&root, &["scan", "--text", "missing.txt"], "", false), + "guard" => invoke(&root, &["guard", "--text", "missing.txt"], "", false), + "hook" => invoke( + &root, + &["hook", "codex"], + r#"{"tool_name":"create_publication"}"#, + false, + ), + "shim" => { + let policy = POLICY.replace("text_flags =", "file_flags = [\"-F\"]\ntext_flags ="); + std::fs::write(root.join("policy/principles.toml"), policy).unwrap(); + invoke( + &root, + &["shim", "faux", "pr", "create", "-F", "missing.txt"], + "", + false, + ) + } + _ => unreachable!(), + }; + assert_eq!( + output.status.code(), + Some(2), + "{path}: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + output.stdout.is_empty(), + "{path}: {}", + String::from_utf8_lossy(&output.stdout) + ); + } +}