diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index d1e9ba2..f0eda94 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -465,6 +465,23 @@ checkers down exactly where the lookup failed. That is exit `2` before the command runs, and `unresolved = "run"` on the `[[shim]]` table is the opt-out — see [the shims](#uphold-shim--the-shims). +The rows above are what the forge said, and the shim reaches them only where it +had to ask at all. A repository that declares its own `visibility` in its policy +file has answered the question already, so `public-target` reads the declaration +and asks nobody — `public` puts the checks in scope, `private` and `internal` +stand them down, and no `gh` runs. That declaration speaks for this repository +and for no other, so it is read for one destination only: the one the shim took +from `origin` because nothing on the command line named another. A `gh -R +owner/repo` goes to the forge exactly as it always did, and it does so **even +where the name it carries is this repository's own** — a typed `owner/repo` +has no host in it, so under a `--hostname` or a `GH_HOST` the same two names +are a different forge's repository with its own visibility, and two matching +path segments cannot tell the two apart. So the `2` above is still the answer +for a named destination the forge could not be asked about, and for a repository +that declares nothing; what is bought offline is the case the cost was actually +in — the push that names nothing and means `origin`, which no longer needs an +authenticated `gh` to be told what the policy already said. + So **`gh` must be authenticated wherever these rules run**, CI included. In a GitHub Actions job that means `GH_TOKEN: ${{ github.token }}`; the job token reads this repository and public ones and answers `404` for everything else, @@ -1161,7 +1178,7 @@ stamped on it, the range about to be pushed. | `no-merge-commit` | a commit finishing a merge or a squash merge | | `no-stale-hook-pins` | a pin left behind its upstream, or naming no ref — in `.pre-commit-config.yaml` **and** lefthook `remotes:`, at any depth in the tree; a pin it **could not check** is exit `2` | | `no-hand-copied-base-rule` | a rule this policy writes out by hand under an id a bundled set already ships, from a set it does not inherit. Reads the **policy**, not the tree. At `pre-commit` only what the change adds; at `manual` the whole sweep | -| `no-stale-visibility` | a declared `private` the forge no longer serves. Reads the **declaration** and the forge, not the tree; a forge that did not answer is exit `2` and never "confirmed private" | +| `no-stale-visibility` | a declared `private` the forge no longer serves. Reads the **declaration** and the forge, not the tree; a forge that did not answer is exit `2` and never "confirmed private", and it says which silence it met — a 404 reads differently from a rate limit | Declared like any other rule, in the same file and the same id namespace. **`git.hooks` is the whole registration.** @@ -1504,8 +1521,27 @@ nested form is the enum's own spelling; the flatter one this page used to show never parsed.) `collect = "git-refs"` replaces the argv walk for `git`, whose published text is positional. -**A scope that could not be evaluated is not a scope that said no.** -`public-target` is the one predicate that asks somebody else, and `gh` +**`public-target` asks this repository before it asks anybody else.** The +policy's own top-level `visibility` is a statement of the fact the predicate +needs, so where there is one it settles the question offline: `public` is in +scope, `private` and `internal` are out of it, and nothing is spawned. The +narrowing that makes reading it safe is that a declaration is about ONE +repository, and the only destination this seam can be sure of is the one it +derived itself. So the declaration is read where a `target_flags` value is +absent — where the destination came from `origin` — and nowhere else. A named +destination goes to the forge whatever it names: `gh -R other/repo` obviously, +and `gh -R this-owner/this-repo` too, because the name carries no host and +`--hostname` and `GH_HOST` both make `owner/repo` a different repository on a +different forge. A tree whose `origin` cannot be read has no destination for a +declaration to be about, and a policy that declares nothing changes nothing. +The guard seam does the same thing one step less strictly (see +`guard::names::target_is_public`), and the difference is deliberate: a guard +reads text belonging to this repository, while a shim stands in front of a +command that may be publishing somewhere else entirely. + +**A scope that could not be evaluated is not a scope that said no.** Where the +question does reach a forge, `public-target` is the one predicate that asks +somebody else, and `gh` unauthenticated, a rate limit, no network, or a repository with no `origin` all answer nothing. That was read as "out of scope", which stood every checker behind the table down — including `prevent-unowned-target`, whose own contract @@ -1518,6 +1554,31 @@ reach `public-target` — neither the table's own `scope` nor the `command.scope of any rule naming the command — because a parameter nothing reads is configuration that looks like it works. +**A silence is told apart from the other silences.** "The forge did not say" was +one sentence for four different situations, and the one thing the reader needed +in order to act was the one thing it left out: a 404, an unauthenticated `gh`, a +rate limit and a `gh` that is not installed are four instructions, not one. So a +failed forge call is classified once — in `shim::Silence`, which the shim seam, +`no-stale-visibility` and `prevent-public-push` all read, because three +classifiers would be free to disagree about one exit code and only one of them +could be right — and the refusal carries the cause: + +```text +uphold shim: the forge did not say whether acme/widget is public, so whether the +`public-target` checks apply here could not be established. The forge is +rate-limiting this client; the budget resets in about 15 minutes. This is not an +answer about the repository -- it is the same silence for a public one and a +private one. +``` + +The reset time is the one question this tool asks in reply to a failure, and it +asks the one endpoint GitHub exempts from the limit it is reporting, so the ask +cannot deepen the hole it is describing. **None of this changes a verdict.** +Every one of these is still exit `2`, still not a pass, and still not a cached +answer — a wait somebody can sit out is worth naming precisely because the +alternative on offer is `UPHOLD_ALLOW`, and a reader who cannot tell a rate limit +from a deleted repository reaches for the bypass either way. + **An alias is expanded before the `match` list is consulted.** `match` names verbs literally, and every one of these commands lets a person rename one: `git -c alias.p=push p origin HEAD:refs/heads/x`, a persisted `[alias] p = @@ -1671,9 +1732,12 @@ than the seam. The fourth is uphold getting out of its own way, and it is documented here rather than left implicit because a reader who meets the line deserves to know -what set it. A `public-target` scope asks the forge whether the destination is -public, and it asks by running `gh api repos// --jq .visibility`; -a `git-remote` target asks by running `git remote get-url origin`. PATH answers +what set it. A `public-target` scope asks whether the destination is public, and +where this repository has not declared its own `visibility` — or where the +command named a destination, which a declaration does not speak for — it asks +by running `gh api repos// --jq .visibility`; a `git-remote` target +asks by running `git remote get-url origin`, which is how an unnamed destination +is resolved in the first place and so runs either way. PATH answers `gh` and `git` with the shim, so **every question this tool asks on the way to a verdict is a command it stands in front of**. On 2026-09-02 that closed into a loop against the released binary inside this repository's own checkout: the diff --git a/src/guard/names.rs b/src/guard/names.rs index 3b81765..3ce3f37 100644 --- a/src/guard/names.rs +++ b/src/guard/names.rs @@ -42,6 +42,7 @@ use super::{Refusal, Request, Stage}; use crate::config::{Policy, Rule}; use crate::error::{Fatal, Result}; use crate::git; +use crate::shim::Silence; /// A repository name, with what a sentence put on the end taken off. /// @@ -96,6 +97,15 @@ pub(crate) enum Visibility { pub(crate) struct Resolved { pub visibility: Visibility, pub canonical: Option, + /// Why there was no answer, where there was none. + /// + /// `Visibility` says only that the check did not happen; this says which of + /// the four ways it did not, in the words the forge used. A caller that + /// refuses over an unchecked claim owes the reader that much -- "the forge + /// did not say" is the same sentence for a deleted repository and for an + /// account that is over its budget for the next eleven minutes, and only one + /// of those is worth waiting out. + pub silence: Option, } /// Any `host.tld/owner/repo` or its scp-like `host.tld:owner/repo`, with the @@ -383,28 +393,34 @@ pub(crate) fn lookup(cache: &mut BTreeMap, owner: &str, repo: Resolved { visibility, canonical, + silence: None, } } - // The forge said no. WHICH no it said is the whole question, and it is - // in stderr: `gh` writes `gh: Not Found (HTTP 404)` for a name it will - // not show us, and `gh: Bad credentials (HTTP 401)` for a client it - // will not talk to. Only the first is a fact about the name. + // The forge said no. WHICH no it said is the whole question, and + // [`Silence`] is where that line is drawn -- once, for this seam and for + // the two others that ask a forge the same kind of question. Only a + // `NotFound` is a fact about the name. // // Anything else -- 401, 403 and a rate limit, 5xx, a status line that // is not there, or no `gh` at all -- is the check not happening, and // reporting that as an inconclusive finding is how an unauthenticated // run passed every name in the tree. - Ok(output) => Resolved { - visibility: if String::from_utf8_lossy(&output.stderr).contains("(HTTP 404)") { - Visibility::Unknown - } else { - Visibility::Unavailable - }, - canonical: None, - }, - Err(_) => Resolved { + Ok(output) => { + let silence = Silence::of("gh", &output); + Resolved { + visibility: if silence == Silence::NotFound { + Visibility::Unknown + } else { + Visibility::Unavailable + }, + canonical: None, + silence: Some(silence), + } + } + Err(error) => Resolved { visibility: Visibility::Unavailable, canonical: None, + silence: Some(Silence::unreachable("gh", &error)), }, }; cache.insert(key, resolved.clone()); @@ -626,6 +642,7 @@ fn judge( Resolved { visibility: Visibility::Private, canonical: None, + silence: None, } } else { lookup(&mut cache, &owner, &repo) @@ -1306,6 +1323,7 @@ mod tests { Resolved { visibility, canonical: canonical.map(str::to_owned), + silence: None, } } diff --git a/src/guard/push.rs b/src/guard/push.rs index 6698be1..d87280e 100644 --- a/src/guard/push.rs +++ b/src/guard/push.rs @@ -32,7 +32,7 @@ use super::{Refusal, Request}; use crate::config::{Policy, Rule}; use crate::error::Result; use crate::git; -use crate::shim::Forge; +use crate::shim::{Forge, Silence}; /// Which seam is asking, for the two phrases that differ between them. /// @@ -132,23 +132,13 @@ pub(crate) enum Owned { Yes, /// The forge answered, and neither is true. No, - /// No answer: no `gh` on PATH, no credentials, no network, or output that - /// is neither a yes nor a no. Carries the first line of what `gh` said, so - /// the reader is told which of those it was. + /// No answer: no `gh` on PATH, no credentials, a rate limit, no network, or + /// output that is neither a yes nor a no. Carries one whole sentence saying + /// which of those it was, because "the forge could not be asked" is a + /// different instruction to the reader in each case. CouldNotAsk(String), } -/// The first line of what a failed `gh` wrote, which is the part worth printing. -fn first_line(stderr: &[u8]) -> String { - let text = String::from_utf8_lossy(stderr); - let line = text.lines().next().unwrap_or_default().trim().to_owned(); - if line.is_empty() { - String::from("it said nothing") - } else { - line - } -} - /// Ask the forge whether the operator owns this destination, once per run. /// /// Only ever reached for a destination the allow-list has already refused, so @@ -181,11 +171,13 @@ fn ask_forge(owner: &str, key: &str) -> Owned { Ok(output) if output.status.success() => { String::from_utf8_lossy(&output.stdout).trim().to_owned() } - Ok(output) => return Owned::CouldNotAsk(first_line(&output.stderr)), - Err(error) => return Owned::CouldNotAsk(format!("gh could not be run ({error})")), + Ok(output) => return Owned::CouldNotAsk(Silence::of("gh", &output).sentence()), + Err(error) => { + return Owned::CouldNotAsk(Silence::unreachable("gh", &error).sentence()); + } }; if login.is_empty() { - return Owned::CouldNotAsk(String::from("`gh api user` printed no login")); + return Owned::CouldNotAsk(String::from("`gh api user` exited 0 and printed no login.")); } // GitHub logins are case-insensitive, and a pin written in the case the // profile page shows is the same account as one written in the case a url @@ -205,18 +197,22 @@ fn ask_forge(owner: &str, key: &str) -> Owned { // `permissions` block that is not there answers `null`. Neither // is a no about ownership. other => Owned::CouldNotAsk(format!( - "`gh api repos/{key}` answered {other:?}, which is neither true nor false" + "`gh api repos/{key}` answered {other:?}, which is neither true nor false." )), } } - // A 404 here is a definite no, and only because the request above - // succeeded: the client works and is authenticated, and a forge that - // will not show this repository to that identity is not a forge saying - // the identity administers it. Every other failure -- 401, 403, a rate - // limit, 5xx -- is the check not happening. - Ok(output) if String::from_utf8_lossy(&output.stderr).contains("(HTTP 404)") => Owned::No, - Ok(output) => Owned::CouldNotAsk(first_line(&output.stderr)), - Err(error) => Owned::CouldNotAsk(format!("gh could not be run ({error})")), + // Classified once, by the one classifier this binary has for the + // question -- see [`Silence`]. A 404 here is a definite no, and only + // because the request above succeeded: the client works and is + // authenticated, and a forge that will not show this repository to that + // identity is not a forge saying the identity administers it. Every + // other failure -- 401, 403, a rate limit, 5xx -- is the check not + // happening, and each of them says so in its own words. + Ok(output) => match Silence::of("gh", &output) { + Silence::NotFound => Owned::No, + silence => Owned::CouldNotAsk(silence.sentence()), + }, + Err(error) => Owned::CouldNotAsk(Silence::unreachable("gh", &error).sentence()), } } @@ -412,8 +408,8 @@ pub(crate) fn unowned( Some(Owned::CouldNotAsk(why)) => { return Err(crate::error::Fatal::new(format!( "rule {:?}: {report}\n\nThe forge could not be asked whether you own {name}, \ - so the allow-list is the only answer there is and it is not a whole one: \ - {why}. Could not look is not a pass. Authenticate `gh`, or name the \ + so the allow-list is the only answer there is and it is not a whole one. \ + {why} Could not look is not a pass. Authenticate `gh`, or name the \ destination on the rule, or bypass this run deliberately with \ UPHOLD_ALLOW={}.", rule.id, rule.id diff --git a/src/guard/visibility.rs b/src/guard/visibility.rs index fff7237..b860c39 100644 --- a/src/guard/visibility.rs +++ b/src/guard/visibility.rs @@ -32,6 +32,7 @@ use super::{Refusal, Request}; use crate::config::visibility_is_public; use crate::error::{Fatal, Result}; use crate::git; +use crate::shim::Silence; /// Refuse a declared privacy the forge has stopped serving. /// @@ -103,7 +104,8 @@ pub(crate) fn no_stale_visibility(request: &Request<'_>) -> Result Ok(Some(Refusal { id: id.clone(), report: format!( @@ -119,14 +121,17 @@ pub(crate) fn no_stale_visibility(request: &Request<'_>) -> Result Err(Fatal::new(format!( + // one that was deleted or renamed. + Visibility::Unknown => Err(Fatal::new(format!( "{id}: the forge did not say whether {owner}/{repo} is public, so the declared \ {declared:?} was not checked. A 404 is a private repository, a deleted one, a \ renamed one, and a request that carried no credentials -- this rule can \ @@ -134,5 +139,19 @@ pub(crate) fn no_stale_visibility(request: &Request<'_>) -> Result Err(Fatal::new(format!( + "{id}: the forge was not able to say whether {owner}/{repo} is public, so the \ + declared {declared:?} was not checked. {}\n\nThis rule can disprove a claim of \ + privacy and can never confirm one, so it does not read silence as agreement. \ + Could not look is not a pass -- bypass this run deliberately with \ + UPHOLD_ALLOW={id}.", + resolved.silence.as_ref().map_or_else( + || String::from("The forge gave no reason."), + Silence::sentence + ) + ))), } } diff --git a/src/shim.rs b/src/shim.rs index f064c3d..e98dbcc 100644 --- a/src/shim.rs +++ b/src/shim.rs @@ -1504,10 +1504,17 @@ impl Shim { /// 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 { @@ -1525,12 +1532,25 @@ impl Shim { 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)? { + return Ok(standing); + } match self.visibility(root, &target).as_deref() { - Some("public") => Ok(Standing::Holds), - Some(_) => Ok(Standing::DoesNotHold), - None => Ok(Standing::CouldNotTell(format!( + 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" + `public-target` checks apply here could not be established. {}", + silence.sentence() ))), } } @@ -1601,8 +1621,13 @@ impl Shim { /// 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) -> Option { - match self.forge(root)? { + 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 => forge_field( "gh", &["api", &format!("repos/{target}"), "--jq", ".visibility"], @@ -1686,25 +1711,234 @@ 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 named a destination at all. 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, +) -> Result> { + 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 +/// [`forge_field`] all ask a forge a question that can fail four ways, and all +/// three used to draw their own line -- or, at this seam, none at all: a +/// non-zero exit was one indistinguishable `None`, so a rate limit, an +/// unauthenticated `gh` and a deleted repository arrived at the user as the same +/// sentence about a 404. Three classifiers would be free to disagree about one +/// `Output` and only one of them could be right. +/// +/// The line is in stderr, because `gh api` writes the API's error BODY to +/// stdout: `gh: Not Found (HTTP 404)` for a name it will not show us, `gh: Bad +/// credentials (HTTP 401)` for a client it will not talk to, and an "API rate +/// limit exceeded" for a client it has stopped talking to for now. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum Silence { + /// The forge answered, and the answer was that it will show us no + /// repository by this name. A fact about the name -- though not ONE fact: + /// private, deleted, renamed, mistyped, and a request that carried no + /// credentials all land here. + NotFound, + /// The client is over its budget. Not a fact about the name at all, and the + /// one silence that ends by itself -- so it carries the wait where the forge + /// would say. + RateLimited { resets_in_minutes: Option }, + /// Anything else: no credentials, no network, no `gh`, a 5xx, a status line + /// that is not there. The first line of what the tool said, because a + /// message that names the cause is the difference between a check somebody + /// fixes and a check somebody bypasses. + Refused(String), +} + +impl Silence { + /// Classify a failed run of `program`. + /// + /// The rate-limit arm asks one further question, and it is the only question + /// in this binary asked in reply to a failure: `gh api rate_limit` is the + /// endpoint GitHub exempts from the limit it is reporting, so the ask cannot + /// deepen the hole it is describing. `None` for the wait where even that did + /// not answer -- the limit is still the fact, and the clock is the courtesy. + pub(crate) fn of(program: &str, output: &std::process::Output) -> Self { + let stderr = String::from_utf8_lossy(&output.stderr); + if stderr.contains("(HTTP 404)") { + return Self::NotFound; + } + if stderr.to_lowercase().contains("rate limit") { + return Self::RateLimited { + resets_in_minutes: rate_limit_minutes(program), + }; + } + Self::Refused(first_line(&stderr).unwrap_or_else(|| { + format!( + "`{program}` exited {}", + output + .status + .code() + .map_or_else(|| String::from("on a signal"), |code| code.to_string()) + ) + })) + } + + /// The forge could not be run at all -- no `gh` on PATH, no permission to + /// exec it. Distinct from a `gh` that ran and refused, and reported that way. + pub(crate) fn unreachable(program: &str, error: &std::io::Error) -> Self { + Self::Refused(format!("`{program}` could not be run ({error})")) + } + + /// The clause a caller puts after "the forge did not say ...". + /// + /// One sentence per cause, and the point of having them is that they are + /// not interchangeable: a rate limit is waited out, an unauthenticated + /// client is fixed, a 404 is five different repositories at once. A caller + /// with a better sentence for its own question is free to write one -- + /// `guard::visibility` does, because "no repository by that name" means + /// something specific when the name is your own `origin`. + pub(crate) fn sentence(&self) -> String { + match self { + Self::NotFound => String::from( + "The forge will show no repository by that name, which is a private one, a \ + deleted one, a renamed one, and a request that carried no credentials.", + ), + Self::RateLimited { resets_in_minutes } => resets_in_minutes.map_or_else( + || { + String::from( + "The forge is rate-limiting this client. This is not an answer about \ + the repository -- it is the same silence for a public one and a \ + private one.", + ) + }, + |minutes| { + format!( + "The forge is rate-limiting this client; the budget resets in about \ + {minutes} minute{}. This is not an answer about the repository -- it \ + is the same silence for a public one and a private one.", + if minutes == 1 { "" } else { "s" } + ) + }, + ), + Self::Refused(what) => format!("The forge was asked and said: {what}"), + } + } +} + +/// Minutes until the client's forge budget resets, where the forge will say. +fn rate_limit_minutes(program: &str) -> Option { + if program != "gh" { + return None; + } + let output = inner_tool(program) + .args(["api", "rate_limit", "--jq", ".rate.reset"]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let reset: u64 = String::from_utf8_lossy(&output.stdout) + .trim() + .parse() + .ok()?; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .ok()? + .as_secs(); + // Rounded UP, so a wait somebody still has to sit through is never reported + // as no wait at all. A reset already in the past is a clock that disagrees + // with the forge's, and a wait nobody should be told to take. + Some(reset.saturating_sub(now).div_ceil(60)).filter(|minutes| *minutes > 0) +} + +/// The first non-empty line of what a tool printed, trimmed. +fn first_line(text: &str) -> Option { + text.lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .map(str::to_owned) +} + /// Run a forge CLI and read one word out of what it printed. /// /// `field` names a JSON key to pull out where the CLI cannot be asked to do it; /// `None` means the whole of stdout is the answer. -fn forge_field(program: &str, args: &[&str], field: Option<&str>) -> Option { - let output = inner_tool(program).args(args).output().ok()?; +fn forge_field( + program: &str, + args: &[&str], + field: Option<&str>, +) -> std::result::Result { + let output = match inner_tool(program).args(args).output() { + Ok(output) => output, + Err(error) => return Err(Silence::unreachable(program, &error)), + }; if !output.status.success() { - return None; + return Err(Silence::of(program, &output)); } let text = String::from_utf8_lossy(&output.stdout); - let value = match field { - Some(field) => json_string_field(&text, field)?, - None => text.trim().to_owned(), - }; - let value = value.trim().to_lowercase(); + let value = field.map_or_else( + || Some(text.trim().to_owned()), + |field| json_string_field(&text, field), + ); + let value = value.unwrap_or_default().trim().to_lowercase(); // An empty answer is not an answer. `--jq` on a field that is not there // prints a blank line and exits 0, and treating that as a visibility would - // be a lookup that did not happen wearing the face of one that did. - (!value.is_empty() && value != "null").then_some(value) + // be a lookup that did not happen wearing the face of one that did. It is + // its own silence rather than a refusal, because a run that SUCCEEDED and + // said nothing is not a forge that would not talk to us. + if value.is_empty() || value == "null" { + return Err(Silence::Refused(format!( + "`{program}` exited 0 and printed no visibility" + ))); + } + Ok(value) } /// Parse the document and read a TOP-LEVEL key, rather than scan for a needle. @@ -2313,10 +2547,11 @@ impl ScopeMemo { shim: &Shim, scope: &Scope, root: &Path, + policy: &Policy, collected: &Collected, argv: &[String], ) -> Result { - match self.standing(shim, scope, root, collected, argv)? { + match self.standing(shim, scope, root, policy, collected, argv)? { Standing::Holds => Ok(true), Standing::DoesNotHold => Ok(false), Standing::CouldNotTell(why) => match shim.unresolved { @@ -2343,6 +2578,7 @@ impl ScopeMemo { shim: &Shim, scope: &Scope, root: &Path, + policy: &Policy, collected: &Collected, argv: &[String], ) -> Result { @@ -2355,7 +2591,7 @@ impl ScopeMemo { if let Some(answer) = self.answers.get(&key) { return Ok(answer.clone()); } - let answer = shim.scope_holds(scope, root, collected, argv)?; + 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."); } @@ -2468,6 +2704,7 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> shim, effective_scope(rule, shim), root, + policy, collected, &opened_for, )? { @@ -2522,6 +2759,7 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> shim, effective_scope(inner, shim), root, + policy, collected, &opened_for, ), @@ -2782,9 +3020,16 @@ pub(crate) fn run( ))); } collected = shim.collect(root, &words)?; - let in_scope = scopes.holds(shim, &shim.scope, root, &collected, &words)?; + let in_scope = scopes.holds(shim, &shim.scope, root, policy, &collected, &words)?; for rule in &checkers { - if scopes.holds(shim, effective_scope(rule, shim), root, &collected, &words)? { + if scopes.holds( + shim, + effective_scope(rule, shim), + root, + policy, + &collected, + &words, + )? { any_applies = true; } } @@ -2850,7 +3095,14 @@ 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, &collected, &words)? { + if !scopes.holds( + shim, + effective_scope(rule, shim), + root, + policy, + &collected, + &words, + )? { continue; } if let Some(refusal) = crate::guard::target_refusal( @@ -2895,7 +3147,14 @@ 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, &collected, &words)? { + if !scopes.holds( + shim, + effective_scope(rule, shim), + root, + policy, + &collected, + &words, + )? { continue; } // `subjects` narrows every kind of checker the same way: @@ -2952,6 +3211,7 @@ pub(crate) fn run( shim, effective_scope(inner, shim), root, + policy, &collected, &words, ) @@ -3082,6 +3342,22 @@ mod tests { line.split_whitespace().map(str::to_owned).collect() } + /// A policy that declares nothing, which is the state every case that is + /// not about a declaration is in: `public-target` then asks the forge + /// exactly as it always did. + fn undeclared() -> Policy { + Policy::default() + } + + /// A policy that says what this repository is, the way `principles.toml` + /// says it. + fn declaring(visibility: &str) -> Policy { + Policy { + visibility: Some(String::from(visibility)), + ..Policy::default() + } + } + fn named(shim: &Shim, line: &str) -> bool { matches!(shim.reading(&argv(line)), Reading::Named) } @@ -3280,6 +3556,7 @@ mod tests { npm.scope_holds( &Scope::PublicRegistry, &dir, + &undeclared(), &Collected::default(), &argv("publish") ) @@ -3298,6 +3575,7 @@ mod tests { npm.scope_holds( &Scope::PublicRegistry, Path::new("."), + &undeclared(), &Collected::default(), &argv("publish --dry-run") ) @@ -3595,6 +3873,7 @@ mod tests { gh().scope_holds( &Scope::Always, Path::new("."), + &undeclared(), &Collected::default(), &argv("pr create") ) @@ -3618,6 +3897,7 @@ mod tests { .scope_holds( &Scope::PublicTarget, Path::new("."), + &undeclared(), &Collected::default(), &argv("pr create") ) @@ -3645,7 +3925,297 @@ mod tests { assert_eq!(unknown_host.forge(&dir), None); assert!(matches!( unknown_host - .scope_holds(&Scope::PublicTarget, &dir, &collected, &argv("pr create")) + .scope_holds( + &Scope::PublicTarget, + &dir, + &undeclared(), + &collected, + &argv("pr create") + ) + .unwrap(), + Standing::CouldNotTell(why) if why.contains("did not say whether acme/widget is public") + )); + } + + /// A repository whose `origin` points at `url`, and whose forge no resolver + /// knows. + /// + /// The host is deliberately unrecognised, which is what makes "the forge + /// was not asked" an assertion rather than a hope: the only answer the + /// lookup can give here is [`Standing::CouldNotTell`], so a definite + /// standing coming back out is proof that the declaration answered and + /// nothing was spawned. The `gh` marker file the CLI tests use asserts the + /// same thing from outside; this asserts it where a stray `gh` on the + /// runner's PATH cannot reach. + fn repository_pointing_at(kind: &str, url: &str) -> PathBuf { + let dir = crate::fixture::scratch(kind); + std::fs::create_dir_all(&dir).unwrap(); + for args in [ + &["init", "-q", "-b", "main"][..], + &["remote", "add", "origin", url][..], + ] { + let status = Command::new("git") + .args(args) + .current_dir(&dir) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .unwrap(); + assert!(status.success(), "git {args:?} failed"); + } + dir + } + + /// A shim whose command names no forge, so nothing here can reach one. + fn no_forge_cli() -> Shim { + let mut shim = gh(); + shim.command = String::from("faux"); + shim + } + + #[test] + fn a_declared_private_visibility_answers_without_asking_the_forge() { + // The defect, as it was reported: a repository whose `principles.toml` + // says `visibility = "private"` made a network call on every `git push` + // to learn a fact the policy states offline, and when the account was + // rate-limited the lookup failed and the default `unresolved` refused + // every push in every tree. + let dir = repository_pointing_at( + "shim-declared-private", + "https://forge.example/acme/widget.git", + ); + assert_eq!( + no_forge_cli() + .scope_holds( + &Scope::PublicTarget, + &dir, + &declaring("private"), + &Collected::default(), + &argv("pr create") + ) + .unwrap(), + Standing::DoesNotHold + ); + // And the same fixture with nothing declared, which is where that + // answer would otherwise have had to come from. + assert!(matches!( + no_forge_cli() + .scope_holds( + &Scope::PublicTarget, + &dir, + &undeclared(), + &Collected::default(), + &argv("pr create") + ) + .unwrap(), + Standing::CouldNotTell(why) if why.contains("did not say whether acme/widget is public") + )); + } + + #[test] + fn a_declared_public_visibility_puts_the_checks_in_scope_offline() { + // The other half, and it is the half that must not be lost: standing + // the checks DOWN on a declaration nobody read would be the failure + // this change could cause, so the `public` word has to reach `Holds` + // without a forge exactly as `private` reaches `DoesNotHold`. + let dir = repository_pointing_at( + "shim-declared-public", + "https://forge.example/acme/widget.git", + ); + assert_eq!( + no_forge_cli() + .scope_holds( + &Scope::PublicTarget, + &dir, + &declaring("public"), + &Collected::default(), + &argv("pr create") + ) + .unwrap(), + Standing::Holds + ); + } + + #[test] + fn a_declaration_about_this_repository_says_nothing_about_another_one() { + // Where the shim is stricter than the guard, and it has to be: `-R + // other-owner/their-repo` is an explicit foreign destination, and this + // policy's `visibility` is a statement about this tree. Reading it at + // somebody else's repository would stand the checks down over a fact + // about a different place entirely. + let dir = repository_pointing_at( + "shim-declared-foreign", + "https://forge.example/acme/widget.git", + ); + let elsewhere = Collected { + target: Some(String::from("other-owner/their-repo")), + ..Collected::default() + }; + assert!(matches!( + no_forge_cli() + .scope_holds( + &Scope::PublicTarget, + &dir, + &declaring("private"), + &elsewhere, + &argv("pr create") + ) + .unwrap(), + Standing::CouldNotTell(why) + if why.contains("did not say whether other-owner/their-repo is public") + )); + } + + /// One failed run of `sh`, standing in for a forge CLI that said this. + /// + /// A real `Output` rather than a hand-built one, because the field this + /// classifier reads is stderr and the thing worth asserting is that a + /// process's stderr reaches it. + fn said(stderr: &str, code: i32) -> std::process::Output { + Command::new("sh") + .args(["-c", &format!("printf '%s' \"$1\" >&2; exit {code}")]) + .arg("sh") + .arg(stderr) + .output() + .expect("sh runs") + } + + #[test] + fn a_404_is_the_one_silence_that_is_about_the_name() { + assert_eq!( + Silence::of("glab", &said("glab: Not Found (HTTP 404)", 1)), + Silence::NotFound + ); + } + + #[test] + fn a_rate_limit_is_not_reported_as_a_404() { + // The report this came from: a rate-limited push was refused with a + // paragraph explaining what a 404 means, which is neither what happened + // nor anything the reader could act on. `glab` rather than `gh` so the + // reset is not looked up -- the classification is what is under test. + let silence = Silence::of( + "glab", + &said("glab: API rate limit exceeded for user ID 1. (HTTP 403)", 1), + ); + assert_eq!( + silence, + Silence::RateLimited { + resets_in_minutes: None + } + ); + assert!( + silence.sentence().contains("rate-limiting this client"), + "{}", + silence.sentence() + ); + assert!( + !silence.sentence().contains("404"), + "{}", + silence.sentence() + ); + } + + #[test] + fn an_unauthenticated_client_is_reported_in_the_words_it_used() { + let silence = Silence::of("glab", &said("glab: Bad credentials (HTTP 401)", 1)); + assert_eq!( + silence, + Silence::Refused(String::from("glab: Bad credentials (HTTP 401)")) + ); + assert!( + silence.sentence().contains("Bad credentials"), + "{}", + silence.sentence() + ); + } + + #[test] + fn a_forge_that_said_nothing_at_all_is_still_named_by_its_exit() { + // A tool that fails silently used to arrive as an empty reason inside a + // sentence that promised one. The exit code is the only fact there is, + // so it is the fact reported. + assert_eq!( + Silence::of("glab", &said("", 7)), + Silence::Refused(String::from("`glab` exited 7")) + ); + } + + #[test] + fn a_destination_somebody_typed_is_asked_about_even_where_it_names_this_repository() { + // The narrowing, asserted at the case that looks safest. `-R + // acme/widget` inside `acme/widget` names what appears to be this + // repository, and two matching path segments are still not this + // repository: a typed `owner/repo` carries no host, so under a + // `--hostname` or a `GH_HOST` the same two names are a different forge's + // repository with its own visibility. Nothing here can tell those apart, + // so nothing here answers for either -- the forge does, as it always + // did. + let dir = repository_pointing_at( + "shim-declared-named", + "https://forge.example/acme/widget.git", + ); + let named = Collected { + target: Some(String::from("acme/widget")), + ..Collected::default() + }; + assert!(matches!( + no_forge_cli() + .scope_holds( + &Scope::PublicTarget, + &dir, + &declaring("private"), + &named, + &argv("pr create") + ) + .unwrap(), + Standing::CouldNotTell(why) if why.contains("did not say whether acme/widget is public") + )); + } + + #[test] + fn an_internal_declaration_is_not_a_public_one() { + // `internal` is public to everyone with an account on the instance, + // which is neither public to the internet nor private -- and only + // `public` puts the `public-target` checks in scope here. + let dir = repository_pointing_at( + "shim-declared-internal", + "git@forge.example:acme/widget.git", + ); + assert_eq!( + no_forge_cli() + .scope_holds( + &Scope::PublicTarget, + &dir, + &declaring("internal"), + &Collected::default(), + &argv("pr create") + ) + .unwrap(), + Standing::DoesNotHold + ); + } + + #[test] + fn a_declaration_needs_an_origin_to_be_about_anything() { + // No `origin` is nothing to compare the target against, so there is no + // establishing that the declaration speaks for this destination and the + // forge is where the question goes -- unchanged from before. + let dir = crate::fixture::scratch("shim-declared-no-origin"); + std::fs::create_dir_all(&dir).unwrap(); + let elsewhere = Collected { + target: Some(String::from("acme/widget")), + ..Collected::default() + }; + assert!(matches!( + no_forge_cli() + .scope_holds( + &Scope::PublicTarget, + &dir, + &declaring("private"), + &elsewhere, + &argv("pr create") + ) .unwrap(), Standing::CouldNotTell(why) if why.contains("did not say whether acme/widget is public") )); @@ -3669,6 +4239,7 @@ mod tests { .scope_holds( &Scope::PublicRegistry, &dir, + &undeclared(), &Collected::default(), &argv("publish") ) @@ -3683,7 +4254,13 @@ mod tests { }; assert_eq!( npm() - .scope_holds(&Scope::PublicRegistry, &dir, &internal, &argv("publish")) + .scope_holds( + &Scope::PublicRegistry, + &dir, + &undeclared(), + &internal, + &argv("publish") + ) .unwrap(), Standing::DoesNotHold ); @@ -3708,6 +4285,7 @@ mod tests { &npm(), &scope, &dir, + &undeclared(), &Collected::default(), &argv("publish") ) @@ -3719,6 +4297,7 @@ mod tests { &npm(), &Scope::PublicRegistry, &dir, + &undeclared(), &Collected::default(), &argv("publish") ) diff --git a/tests/guard_cli.rs b/tests/guard_cli.rs index 6a5a852..1874ba9 100644 --- a/tests/guard_cli.rs +++ b/tests/guard_cli.rs @@ -1302,6 +1302,93 @@ fn an_origin_with_no_owner_and_repository_in_it_is_not_a_checked_claim() { assert!(text.contains("Could not look is not a pass"), "{text}"); } +// The two cases that DO reach a forge, through the fixture's own `gh`. Both are +// exit 2 and both leave the declaration unchecked -- the verdict was never the +// question. What is asserted here is that they do not say the same thing: for a +// while they did, and an account four minutes into a rate limit was handed a +// paragraph about what a 404 means. + +/// A `gh` that is over its budget, and a `rate_limit` endpoint that says when. +/// +/// The reset is built at call time because the message counts forward from now. +/// GitHub exempts `rate_limit` from the limit it reports, which is what makes +/// asking it in reply to a 403 something other than digging. +fn gh_is_rate_limited(root: &Path, resets_in_seconds: u64) { + let reset = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() + + resets_in_seconds; + gh_says( + root, + &format!( + "case \"$*\" in\n\ + 'api rate_limit --jq .rate.reset') echo {reset} ;;\n\ + *) echo 'gh: API rate limit exceeded for user ID 1. (HTTP 403)' >&2; exit 1 ;;\n\ + esac\n" + ), + ); +} + +#[test] +fn a_rate_limited_forge_is_not_reported_as_a_repository_that_is_not_there() { + let root = repository( + "visibility = \"private\"\n\n[rule.no-stale-visibility]\n\ + builtin = \"no-stale-visibility\"\n\n\ + [rule.no-stale-visibility.git]\nhooks = [\"pre-push\"]\n", + ); + git( + &root, + &[ + "remote", + "add", + "origin", + "https://github.com/acme/widget.git", + ], + ); + gh_is_rate_limited(&root, 15 * 60); + + let output = push_guard(&root, &[]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + let text = stderr(&output); + assert!(text.contains("rate-limiting this client"), "{text}"); + // Counted forward and rounded up, so the reader is told how long to wait + // rather than left to compare two epochs. + assert!(text.contains("resets in about 15 minutes"), "{text}"); + // The wording that used to be printed here, and the reason this test exists. + assert!(!text.contains("A 404 is a private repository"), "{text}"); + assert!(text.contains("UPHOLD_ALLOW=no-stale-visibility"), "{text}"); +} + +#[test] +fn a_forge_that_will_show_no_such_repository_still_gets_the_404_reading() { + // The other half: splitting the arms must not cost the case the original + // wording was right for. A 404 over a repository declared private is the + // ordinary answer to an unauthenticated request, and saying so is the whole + // value of the paragraph. + let root = repository( + "visibility = \"private\"\n\n[rule.no-stale-visibility]\n\ + builtin = \"no-stale-visibility\"\n\n\ + [rule.no-stale-visibility.git]\nhooks = [\"pre-push\"]\n", + ); + git( + &root, + &[ + "remote", + "add", + "origin", + "https://github.com/acme/widget.git", + ], + ); + gh_says(&root, "echo 'gh: Not Found (HTTP 404)' >&2\nexit 1\n"); + + let output = push_guard(&root, &[]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + let text = stderr(&output); + assert!(text.contains("A 404 is a private repository"), "{text}"); + assert!(!text.contains("rate-limiting"), "{text}"); +} + // ── where a push is allowed to go ──────────────────────────────────── /// A pre-push guard told nothing but its flags. diff --git a/tests/shim_cli.rs b/tests/shim_cli.rs index a19f8f0..b0785f5 100644 --- a/tests/shim_cli.rs +++ b/tests/shim_cli.rs @@ -2623,6 +2623,263 @@ fn a_gitlab_project_id_is_the_same_destination_with_its_separator_escaped() { ); } +// ── a visibility this repository already declared ──────────────────── + +/// A `public-target` shim over a repository that says what it is. +/// +/// The `visibility` line is the whole point of the fixture: it is the same +/// declaration `guard::names::target_is_public` has always read, written where +/// a policy writes it, and the question it answers is exactly the one the shim +/// was sending to a forge. +const DECLARED_POLICY: &str = r#" +visibility = "private" + +[rule.no-published-markers] +message = "remove the marker" +exec = "uphold guard --text -" + +[rule.no-published-markers.command] +before = ["faux"] + +[rule.prevent-ai-author] +builtin = "prevent-ai-author" + +[rule.prevent-ai-author.git] +hooks = ["commit-msg"] + +[[shim]] +command = "faux" +match = ["pr:create"] +text_flags = ["-t", "--title"] +target_flags = ["-R", "--repo"] +target = "git-remote" +scope = "public-target" +"#; + +/// The workspace above with an `origin`, and a `gh` that records being run. +/// +/// The stub fails rather than answering, and writes its marker before it does: +/// "asked, and could not be told" and "never asked" are then two different +/// states on disk rather than one exit code, which is what makes a forge call +/// that should not have happened visible at all. It is also what a rate-limited +/// account looks like, which is the failure this behaviour was reported from. +fn declared_workspace(policy: &str, origin: &str) -> PathBuf { + let root = workspace(policy); + stub( + &root, + "gh", + "#!/bin/sh\necho asked >> \"$(dirname \"$0\")/../gh.asked\"\nexit 1\n", + ); + Command::new(support::real_git()) + .args(["remote", "add", "origin", origin]) + .current_dir(&root) + .stdout(Stdio::null()) + .status() + .unwrap(); + root +} + +fn forge_was_asked(root: &Path) -> bool { + root.join("gh.asked").exists() +} + +#[test] +fn a_declared_private_visibility_answers_the_scope_with_no_forge_call() { + // The report: a repository whose policy says `private` still ran `gh api + // repos//` on every `git push`, and when the account was + // rate-limited the lookup failed, the standing was could-not-tell, and the + // default `unresolved` refused every push in every tree -- over a fact the + // policy had stated offline the whole time. + let root = declared_workspace(DECLARED_POLICY, "https://github.com/acme/widget.git"); + let output = shim( + &root, + &["faux", "pr", "create", "-t", "Generated with Claude Code"], + ); + // Out of scope, so the checks do not apply and the command runs -- which is + // the policy's own answer about its own destination, not a check skipped. + assert_eq!(code(&output), 0, "{}", stderr(&output)); + assert!(stdout(&output).contains("faux ran:"), "{}", stdout(&output)); + assert!( + !forge_was_asked(&root), + "the forge was asked about a repository whose policy had already answered" + ); +} + +#[test] +fn a_declared_public_visibility_puts_the_checks_in_scope_with_no_forge_call() { + // The half that must not be lost. Reading the declaration is only safe if + // `public` reaches the checkers exactly as a forge saying `public` did: + // standing checks down on a word nobody read would be the failure this + // change could cause. + let root = declared_workspace( + DECLARED_POLICY + .replace("visibility = \"private\"", "visibility = \"public\"") + .as_str(), + "https://github.com/acme/widget.git", + ); + let output = shim( + &root, + &["faux", "pr", "create", "-t", "Generated with Claude Code"], + ); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + assert!( + !stdout(&output).contains("faux ran:"), + "{}", + stdout(&output) + ); + assert!( + !forge_was_asked(&root), + "the forge was asked about a repository whose policy had already answered" + ); +} + +#[test] +fn a_declaration_about_this_repository_is_not_read_at_somebody_elses() { + // Where the shim is stricter than the guard, and it has to be: `-R + // other-owner/their-repo` is an explicit foreign destination, and what this + // policy says about this tree says nothing whatever about that one. So the + // forge is asked, it cannot answer, and a question that could not be asked + // has never been a pass here. + let root = declared_workspace(DECLARED_POLICY, "https://github.com/acme/widget.git"); + let output = shim( + &root, + &[ + "faux", + "pr", + "create", + "-R", + "other-owner/their-repo", + "-t", + "An ordinary title", + ], + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + !stdout(&output).contains("faux ran:"), + "{}", + stdout(&output) + ); + assert!( + stderr(&output).contains("other-owner/their-repo"), + "{}", + stderr(&output) + ); + assert!( + forge_was_asked(&root), + "a foreign destination was answered out of this repository's own declaration" + ); +} + +#[test] +fn a_repository_that_declares_nothing_still_asks_the_forge() { + // The unchanged path, asserted rather than assumed: a policy with no + // `visibility` line has said nothing, and silence is not an answer this + // seam may supply on the repository's behalf. + let root = declared_workspace( + DECLARED_POLICY + .replace("visibility = \"private\"\n", "") + .as_str(), + "https://github.com/acme/widget.git", + ); + let output = shim(&root, &["faux", "pr", "create", "-t", "An ordinary title"]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + !stdout(&output).contains("faux ran:"), + "{}", + stdout(&output) + ); + assert!( + forge_was_asked(&root), + "nothing was declared and nothing was asked either" + ); +} + +#[test] +fn a_rate_limited_forge_says_so_where_the_scope_could_not_be_established() { + // The shim seam drew no line at all: every non-zero exit from `gh` was one + // indistinguishable "the forge did not say", so the sentence that reached + // the user over a rate limit was the same sentence a deleted repository + // would have produced -- and neither named the thing to do about it. + let root = workspace( + DECLARED_POLICY + .replace("visibility = \"private\"\n", "") + .as_str(), + ); + let reset = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() + + 8 * 60; + stub( + &root, + "gh", + &format!( + "#!/bin/sh\ncase \"$*\" in\n\ + 'api rate_limit --jq .rate.reset') echo {reset} ;;\n\ + *) echo 'gh: API rate limit exceeded for user ID 1. (HTTP 403)' >&2; exit 1 ;;\n\ + esac\n" + ), + ); + Command::new(support::real_git()) + .args([ + "remote", + "add", + "origin", + "https://github.com/acme/widget.git", + ]) + .current_dir(&root) + .stdout(Stdio::null()) + .status() + .unwrap(); + + let output = shim(&root, &["faux", "pr", "create", "-t", "An ordinary title"]); + // Still exit 2. The verdict was never the question -- a scope that could not + // be evaluated is not a scope that said no, and that is unchanged. + assert_eq!(code(&output), 2, "{}", stderr(&output)); + let text = stderr(&output); + assert!( + text.contains("did not say whether acme/widget is public"), + "{text}" + ); + assert!(text.contains("rate-limiting this client"), "{text}"); + assert!(text.contains("resets in about 8 minutes"), "{text}"); +} + +#[test] +fn a_typed_destination_naming_this_repository_is_still_asked_about() { + // The narrowing at the case that looks safest, and the reason the shim is + // stricter than the guard even here. `-R acme/widget` inside `acme/widget` + // names what appears to be this repository -- but a typed `owner/repo` + // carries no host, so under a `--hostname` or a `GH_HOST` those same two + // names are a different forge's repository with its own visibility, and two + // matching path segments cannot tell the two apart. So the declaration + // answers only for the destination `origin` supplied, and everything + // somebody typed goes to the forge exactly as it did before. + let root = declared_workspace(DECLARED_POLICY, "https://github.com/acme/widget.git"); + let output = shim( + &root, + &[ + "faux", + "pr", + "create", + "-R", + "acme/widget", + "-t", + "Generated with Claude Code", + ], + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + !stdout(&output).contains("faux ran:"), + "{}", + stdout(&output) + ); + assert!( + forge_was_asked(&root), + "a typed destination was answered out of this repository's own declaration" + ); +} + // A shim that can be handed its own probe. // // On 2026-09-02 the released binary fork-bombed a workstation from inside this