diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dbf488b..71a1631 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,6 +44,15 @@ the guard working, not a misconfiguration. `visibility` decides whether the private-name guards fire here at all. A private fork sets `private` and they stand down; a public one leaves `public`. +A workspace holding many repositories writes both lines many times — measured +across one fleet, 78 `owner` lines for seven distinct values. `owner_from` and +`visibility_from` take a command whose stdout is the value instead, so the fact +lives once outside the tree. They move the declaration and never look it up: +every way the command can fail to answer is exit `2`, because what a missing +declaration falls back to is the owner read off `origin` and the forge's view of +a visibility that is about to change. See +[REFERENCE.md](docs/REFERENCE.md#reading-a-repository-fact-from-a-command). + A third line is about *your machine* rather than your fork. `private_owners_from` reads a file that is one operator's and will not exist in your clone, so the policy also sets `private_owners_optional = true`: the diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 75faea9..c64cbc0 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -120,7 +120,7 @@ files.glob = ["*.yml", "*.yaml"] `inherit.sets` names bundled sets to inherit; it does not add settings. There is no `true` shorthand — naming the sets is cheap, and what a repository -inherits should be written in the repository. Fifteen are compiled into the +inherits should be written in the repository. Sixteen are compiled into the binary and mirrored in [`policy/base/`](../policy/base), each **named by what it refuses** so the name predicts the rule list: @@ -141,8 +141,9 @@ it refuses** so the name predicts the rule list: | `stale-pins` | a hook pinned at a revision its upstream has left, or at none — **installs `pre-push` and `manual`**, and reaches the network | | `unowned-push` | a push to an owner this repository has not named — **installs `pre-push`**, and refuses to run until the repository says who it is | | `private-names` | a private organisation or repository named in a commit message, a staged diff, or the tracked tree of a **public** repository — **installs five stages**, and refuses to run until the repository says whether it is published | +| `stale-visibility` | a policy declaring `private` over a repository the forge serves as **public** — **installs `pre-push` and `manual`**, and reaches the network. Refuses that one direction only; it can never confirm privacy | -The last six install git hooks. Taking one is a decision about what will be +The last seven install git hooks. Taking one is a decision about what will be refused and when, so each is named and argued separately: `stale-pins` reaches the network and cannot answer on a train, `invisible-characters` reads the tree at four stages and is the slowest thing in a hook, `unreviewed-history` stands @@ -175,6 +176,36 @@ with no network, and answers about the visibility a repository has *today*, which is the thing that changes on the day it matters. `visibility = "private"` is a real answer, not an opt-out: it says the condition does not hold here. +**What then checks the declaration** is `stale-visibility`, and it is a separate +set because it reaches the network. Declaring the visibility makes the guards +offline and deterministic, and it also makes the file a **cache with no +reconcile**: flip a repository to public and the policy goes on saying `private` +forever, with the three private-name guards standing down over a tree everybody +can read. + +The rule refuses **one direction and only one**, because that is the only +direction a probe can establish. No probe can prove a repository is private — a +`404` is a private repository, a deleted one, a renamed one, and a request that +carried no credentials — while any probe can disprove it, and disproving it is +what catches the leak. + +| declared | what the forge says | outcome | +|---|---|---| +| `public` | not asked | passes, and says no request was made | +| `private` / `internal` | `public` | **refused**, naming the flip | +| `private` / `internal` | `private` / `internal` | passes | +| `private` / `internal` | nothing it could be asked | **exit `2`** — never "confirmed private" | +| nothing declared | not asked | **exit `2`** — no claim to check | + +The last two rows are the design. If a failed lookup could settle the answer, +an offline laptop would flip the guards to `private` and disarm a disclosure +check in silence, which is fail-open on the one family where fail-open is +unacceptable. The declaration stays the input; the probe only ever refuses. + +It installs at `pre-push` and `manual` and never at `pre-commit`, for the reason +`stale-pins` gives: a guard that adds a network round trip to every commit is +one somebody comments out. + **Two different "unknowns", and only one of them is `refuse_unknown`'s.** A forge that *answers* `404` has told you something about the name: no repository it will show you is called that. A forge that could not be asked — no `gh`, no @@ -600,6 +631,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" | Declared like any other rule, in the same file and the same id namespace. **`git.hooks` is the whole registration.** @@ -645,7 +677,7 @@ enforced and is not. | `owner` | `prevent-public-push` | the owner this workspace is pinned to | | `allowed_owners` | `prevent-public-push` | owners a push may go to; defaults to the pinned owner | | `allowed_repos` | `prevent-public-push` | single repositories allowed through, `"owner/repo"` | -| `visibility` | the `no-private-repo-names` family | this repository's visibility, declared instead of looked up | +| `visibility` | the `no-private-repo-names` family, `no-stale-visibility` | this repository's visibility, declared instead of looked up | | `visibility_required` | the `no-private-repo-names` family | exit `2` rather than fall back to the forge when nothing has declared a visibility | | `private_owners` | the `no-private-repo-names` family | owners whose repositories are private regardless of what a forge says | | `private_owners_from` | the `no-private-repo-names` family | a command whose stdout is one private owner per line | @@ -654,7 +686,9 @@ enforced and is not. | `allow` | `prevent-unusual-unicode-in-files` | codepoints admitted, optionally under one glob — `"U+00A0:docs/captured/**"` | The "family" is `no-private-repo-names`, `-staged` and `-in-files`. No other -built-in reads any parameter. +built-in reads any parameter. `no-stale-visibility` reads `visibility` and +nothing else — everything else in that row is about judging names in text, which +the falsifier never does. **Three of these are also top-level policy fields**, and that is not a convenience. A rule arriving from a bundled set cannot be handed a parameter — @@ -672,6 +706,64 @@ private_owners_optional = true # only where this policy is cloned; see below A rule's own field wins where both are written. +### Reading a repository fact from a command + +`owner` and `visibility` are each written once per repository, and across one +fleet that is 78 `owner` lines carrying seven distinct values — 41 copies of one +string inside a single organisation. `owner_from` and `visibility_from` are the +same move `private_owners_from` already makes: a command whose stdout is the +value, run in the repository root, so a workspace fact is written once outside +the tree instead of once per tree. + +```toml +owner_from = "cat ${XDG_CONFIG_HOME:-$HOME/.config}/uphold/owner" +visibility_from = "cat .workspace/visibility-cache" +``` + +**They move a declaration; they do not look one up.** Deriving the owner from +`origin` is the defect rather than the fix — repointing `origin` at somebody +else's remote is the exact accident `prevent-public-push` exists to catch, and a +derived allow-list is repointed by the same command. The same applies to +visibility: a command that asks a forge what a repository is *today* hands three +guards' one scope condition to a network call, which answers nothing on a train, +nothing in CI without a token, and answers about the visibility a repository is +in the middle of changing. What the command reads must be a value somebody +decided, not the state being guarded. + +So every way the command can fail to answer is exit `2`: + +| what the command does | what happens | +|---|---| +| prints one value | that is the declaration, cached for the rest of the process | +| exits non-zero | exit `2`, naming the fallback it refused to take | +| exits `0` printing nothing | exit `2` — the file reads as a declaration and there is none | +| prints more than one non-empty line | exit `2` — one repository, one fact, and no first-line guess. A trailing blank line is not a second value; `cat` gives one back for any file that ends with one | +| prints a word that is not a visibility | exit `2`, naming the command rather than the file | + +There is deliberately **no `..._optional`** beside these, and the asymmetry with +`private_owners_from` is the point. An unreadable owner list degrades to a +narrower check, which can be reported and lived with. An unreadable owner +degrades to the tautology above, and an unreadable visibility degrades to +standing a disclosure guard down — neither is a degradation anybody should be +able to opt into. + +Two further refusals, both at load: + +- **`owner` beside `owner_from`** (or `visibility` beside `visibility_from`) is + refused. They are two statements of one fact, free to disagree, with nothing + anywhere to notice when they do — which is the defect the field exists to + remove, arriving through the field. +- **Neither may arrive by inheritance.** A bundled set or an `inherit.paths` + file carrying one is refused, for the reason `private-names` gives for not + shipping `private_owners_from`: a command arriving that way runs in every + inheriting repository on the strength of a version bump, with nothing in any + of those trees to review. + +The command runs at most once per process, on the first ask — the private-name +family asks about visibility three times, once per variant — and the answer is +never cached to disk. A declaration exists to avoid a stale answer, and a cache +outliving the run is a stale answer with a longer life. + **Why the owner list is worth declaring**, measured rather than asserted. A forge lookup only *adjudicates* names something already extracted, and a bare `owner/repo` is extracted only for declared owners and for this repository's diff --git a/policy/base/sets.lock.json b/policy/base/sets.lock.json index 7afeac3..21a4853 100644 --- a/policy/base/sets.lock.json +++ b/policy/base/sets.lock.json @@ -591,6 +591,25 @@ "manual" ] }, + { + "rules": [ + { + "builtin": "no-stale-visibility", + "git": { + "hooks": [ + "pre-push", + "manual" + ] + }, + "id": "no-stale-visibility" + } + ], + "set": "stale-visibility", + "stages": [ + "pre-push", + "manual" + ] + }, { "rules": [ { diff --git a/policy/base/stale-visibility.toml b/policy/base/stale-visibility.toml new file mode 100644 index 0000000..0806725 --- /dev/null +++ b/policy/base/stale-visibility.toml @@ -0,0 +1,62 @@ +# Base rule set: stale-visibility (a policy still claiming a privacy the forge +# has stopped serving) +# +# The guard that asks the forge whether this repository is public, and refuses +# when the policy says it is not. Pull in with: +# +# [inherit] +# sets = ["private-names", "stale-visibility"] +# +# WHY IT EXISTS. `visibility` is declared in the policy file and read by the +# three `private-names` guards as the condition they fire under. The forge owns +# that fact; the file holds a copy; nothing reconciles the two. A repository +# flipped from private to public goes on being judged by a file that says +# `private`, and the guards that would have caught a leak stand down over a +# repository everybody can read. Measured across one fleet: 78 policies declare +# a visibility, and nothing anywhere would notice if one of them stopped being +# true. +# +# IT DISPROVES AND NEVER CONFIRMS, AND THAT IS THE DESIGN. No probe can +# establish that a repository is PRIVATE -- a 404 is a private repository, a +# deleted one, a renamed one, and a request that carried no credentials, and a +# public index by construction lists only public repositories. Every probe can +# disprove privacy, and that is the direction that leaks. So this rule refuses +# exactly one state, declared private against a forge serving public, and reads +# nothing at all back into the declaration. +# +# The declaration therefore stays the input. A rule that let a failed lookup +# decide would let an offline laptop flip the guards to `private` and disarm a +# disclosure check in silence, which is fail-open on the one family where +# fail-open is unacceptable. Could not look is exit 2, never "confirmed +# private". +# +# A POLICY DECLARING `public` IS CLEAN WITHOUT A REQUEST. There is no claim of +# privacy to disprove and the private-name family is already at its strictest, +# so the rule says so and makes no network call. That is most repositories, and +# it is why the set costs them nothing. +# +# SEPARATE FROM `private-names`, AND THE REASON IS THE NETWORK -- the same +# reason `stale-pins` is a name of its own. This is the only other bundled rule +# that talks to a forge, so its verdict depends on where the machine running it +# is standing: on a train, in a runner with no credentials, behind a proxy, the +# question cannot be answered at all, and it answers exit 2 there rather than 0. +# A repository that cannot live with a network-dependent gate should inherit +# `private-names` and not this. +# +# THE STAGES ARE `stale-pins`'s, for the reason written there. `pre-push` is the +# last local moment before work is shared, and `manual` is the sweep. It is off +# `pre-commit` on purpose: a guard that adds a network round trip to every +# commit is one somebody comments out, and a check that gets switched off +# protects nothing. +# +# WHAT IT CANNOT SEE. It asks about `origin`, so a repository pushed somewhere +# `origin` does not name is not the repository being checked -- which is the +# same limit `prevent-public-push` has and refuses to derive its way around. +# It asks through `gh`, so a forge `gh` does not speak to answers Unavailable +# and the run exits 2 rather than passing. +[set] +stages = ["pre-push", "manual"] + +[rule.no-stale-visibility] +builtin = "no-stale-visibility" +git.hooks = ["pre-push", "manual"] diff --git a/src/config.rs b/src/config.rs index ff01d99..bd86dc6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -15,6 +15,8 @@ use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::OnceLock; use serde::{Deserialize, Serialize}; @@ -97,6 +99,15 @@ pub(crate) const BUNDLED: &[(&str, &str)] = &[ include_str!("../policy/base/invisible-characters.toml"), ), ("stale-pins", include_str!("../policy/base/stale-pins.toml")), + // The second network set, and separate from `private-names` for the reason + // `stale-pins` is separate from everything: a verdict that depends on + // where the machine running it is standing is a decision a repository + // takes on its own, not one it acquires by inheriting the family whose + // scope condition this checks. + ( + "stale-visibility", + include_str!("../policy/base/stale-visibility.toml"), + ), ( "unowned-push", include_str!("../policy/base/unowned-push.toml"), @@ -801,7 +812,7 @@ impl Rule { /// The search scoping, or ripgrep's defaults where the table is absent. pub(crate) fn files(&self) -> &Files { - static DEFAULTS: std::sync::OnceLock = std::sync::OnceLock::new(); + static DEFAULTS: OnceLock = OnceLock::new(); self.files .as_ref() .map_or_else(|| DEFAULTS.get_or_init(Files::default), |files| files) @@ -1282,6 +1293,45 @@ pub(crate) struct PolicyFile { /// `guard::names::target_is_public`. #[serde(default)] pub visibility: Option, + /// A command whose stdout is this repository's owner. + /// + /// The duplication is measured rather than asserted: across one fleet, 78 + /// policy files declare an `owner` for seven distinct values -- 41 copies of + /// one string inside a single organisation. That is a workspace fact + /// transcribed once per repository, and it is the same shape + /// [`PolicyFile::private_owners_from`] already answered by reading from + /// outside the tree. + /// + /// It is NOT permission to derive the owner from `origin`. Deriving it is + /// the defect rather than the fix -- repointing `origin` at somebody else's + /// remote is the exact accident `prevent-public-push` exists to catch, and a + /// derived allow-list is repointed by the same command. This field moves a + /// DECLARATION out of the tree; it never reads one off the thing being + /// guarded. + /// + /// Refused beside a literal `owner`, and refused in a bundled set or an + /// inherited file. See [`refuse_two_statements_of_one_fact`] and + /// [`refuse_inherited_declaration`]. + #[serde(default)] + pub owner_from: Option, + /// A command whose stdout is this repository's visibility. + /// + /// Same mechanism as [`PolicyFile::owner_from`], for a host the built-in + /// lookup does not speak to or a workspace that would rather answer from a + /// cached organisation index than a request per repository. The word it + /// prints is held to `public`, `private` or `internal` exactly as a written + /// one is. + /// + /// What it must not become is a probe. This is a declaration read from + /// somewhere else, and a command that asks a forge what a repository is + /// TODAY hands the private-name family's one scope condition to a network + /// call -- which answers nothing on a train, nothing in CI without a token, + /// and answers about the visibility a repository is in the middle of + /// changing. So a command that cannot answer must fail, and failing is exit + /// 2: a quiet `private` would stand the whole family down, which is + /// fail-open on the one rule family where fail-open is unacceptable. + #[serde(default)] + pub visibility_from: Option, /// A command whose output lists the owners this workspace treats as /// private, declared once for the whole policy. /// @@ -1375,6 +1425,27 @@ pub(crate) struct Policy { /// What this repository's publications are visible to, from the policy /// file's own `visibility`. See [`PolicyFile::visibility`]. pub visibility: Option, + /// Where the owner is declared when it is not written down, from the policy + /// file's own `owner_from`. See [`PolicyFile::owner_from`]. + pub owner_from: Option, + /// Where the visibility is declared when it is not written down, from the + /// policy file's own `visibility_from`. See [`PolicyFile::visibility_from`]. + pub visibility_from: Option, + /// What `owner_from` answered, so it is asked at most once per process. + /// + /// Two slots and not one map keyed by field name, deliberately: these are + /// two facts behind two commands, and a single cache filled in one pass + /// would run the visibility command for a caller that only asked who this + /// repository belongs to. The resolution itself IS parameterised -- one + /// [`declared`] reads either -- so what is duplicated here is storage, not a + /// unit of behaviour. + /// + /// Per process rather than on disk. A declaration exists to avoid a stale + /// answer, and a cache that outlives the run is a stale answer with a + /// longer life. + pub resolved_owner: OnceLock, + /// What `visibility_from` answered. See [`Policy::resolved_owner`]. + pub resolved_visibility: OnceLock, /// Where the private-owner list comes from, from the policy file's own /// `private_owners_from`. See [`PolicyFile::private_owners_from`]. pub private_owners_from: Option, @@ -1395,6 +1466,55 @@ pub(crate) struct Policy { } impl Policy { + /// Who this repository belongs to, as this policy declares it. + /// + /// The written `owner` if there is one, else whatever `owner_from` answers, + /// else nothing -- and "nothing" is what makes a caller fall back to the + /// remote, which every caller of this is careful to say out loud. + /// + /// A rule's own `owner` is NOT consulted here. It is narrower than the + /// policy's on purpose, so the rule reads it first and reaches this only + /// when it has none. + pub(crate) fn declared_owner(&self, root: &Path) -> Result> { + declared( + root, + "owner", + self.owner.as_deref(), + self.owner_from.as_deref(), + &self.resolved_owner, + ) + } + + /// What this repository's publications are visible to, as this policy + /// declares it, held to the three spellings whichever way it arrived. + /// + /// A written `visibility` was already held to them at load, where a typo is + /// a diff somebody can fix. A command's answer cannot be checked then -- + /// there is no answer until it runs -- so it is checked here, and the + /// refusal names the command rather than the file, because the file is + /// right and the thing it points at is not. + pub(crate) fn declared_visibility(&self, root: &Path) -> Result> { + let value = declared( + root, + "visibility", + self.visibility.as_deref(), + self.visibility_from.as_deref(), + &self.resolved_visibility, + )?; + if let Some(word) = value.as_deref() { + if visibility_is_public(word).is_none() { + return Err(Fatal::new(format!( + "`visibility_from` answered {word:?}, which is not a visibility. The \ + command must print \"public\", \"private\" or \"internal\" -- the word \ + decides whether the guards that fire only on a published tree fire here \ + at all, so there is no reading of an unrecognised one that is safe to \ + guess at." + ))); + } + } + Ok(value) + } + /// The bundled set a rule id arrived from, for the one line a reader sees. /// /// By id, because a refusal carries the id and ids are one namespace -- @@ -1471,6 +1591,76 @@ fn parse(path: &Path, text: &str) -> Result { /// be a permission granted to the author by the author, which is not a /// permission. Refused rather than ignored: a field read by nothing is the /// shape this schema exists to make unrepresentable. +/// Refuse a file that states one repository fact twice. +/// +/// The whole argument for reading a declaration from outside the tree is that +/// the fact has ONE place it is written. A file carrying `owner` and +/// `owner_from` has kept the copy the field exists to remove, and nothing +/// anywhere reconciles the two -- so the day they disagree is a day one of them +/// is silently wrong and the guard reading it cannot tell. +/// +/// At load, like the visibility spelling above it, because it is a fact about +/// the file rather than about any run of any hook. +fn refuse_two_statements_of_one_fact(path: &Path, file: &PolicyFile) -> Result<()> { + for (field, literal, from) in [ + ("owner", file.owner.is_some(), file.owner_from.is_some()), + ( + "visibility", + file.visibility.is_some(), + file.visibility_from.is_some(), + ), + ] { + if literal && from { + return Err(Fatal::at( + path, + format!( + "`{field}` and `{field}_from` are both declared, and they are two \ + statements of one fact -- free to disagree, with nothing here to notice \ + when they do. Keep the one that is true of every checkout of this \ + repository: `{field}_from` where the value belongs to the workspace, \ + `{field}` where it belongs to the repository. Delete the other" + ), + )); + } + } + Ok(()) +} + +/// Refuse a command that speaks for a repository from a file that repository +/// did not write. +/// +/// `owner_from` and `visibility_from` run a shell command. A bundled set +/// carrying one would run it in every inheriting repository on the strength of a +/// version bump, with nothing in any of those trees to review -- which is the +/// reason `private-names` already gives for not shipping `private_owners_from`, +/// and it is not weakened by the command being one line shorter. +/// +/// Refused rather than dropped. `owner` and `visibility` in an inherited file +/// are read by nothing and say nothing about it, which is the shape this +/// repository refuses everywhere else; those two are load-bearing in trees that +/// already exist, and these two are new and can start correct. +fn refuse_inherited_declaration(path: &Path, file: &PolicyFile, kind: &str) -> Result<()> { + for field in ["owner_from", "visibility_from"] { + let declared = match field { + "owner_from" => file.owner_from.is_some(), + _ => file.visibility_from.is_some(), + }; + if declared { + return Err(Fatal::at( + path, + format!( + "`{field}` runs a shell command and this file is {kind}. A command \ + arriving that way runs in every repository that inherits it on the \ + strength of a version bump, with nothing in any of those trees to \ + review -- which is why a set does not ship `private_owners_from` \ + either. Write the line in the repository's own policy file" + ), + )); + } + } + Ok(()) +} + fn refuse_set_header(path: &Path, file: &PolicyFile) -> Result<()> { if file.set.is_some() { return Err(Fatal::at( @@ -1491,6 +1681,7 @@ fn refuse_set_header(path: &Path, file: &PolicyFile) -> Result<()> { fn parse_bundled(name: &str, text: &str) -> Result { let path = Path::new("").join(format!("{name}.toml")); let file = parse(&path, text)?; + refuse_inherited_declaration(&path, &file, "a bundled set")?; let allowed = file.set.clone().unwrap_or_default().stages; for rule in file.rules.values() { for hook in rule.hooks() { @@ -1518,6 +1709,90 @@ fn parse_bundled(name: &str, text: &str) -> Result { Ok(file) } +/// One repository-level fact, read from a command instead of written down. +/// +/// Shared by `owner_from` and `visibility_from` because those differ only in +/// which fact they carry. One function, so the trimming, the refusal of a second +/// line and the words a failure prints are stated once and cannot drift into two +/// slightly different contracts. +/// +/// There is deliberately no `..._optional` escape hatch here, and the asymmetry +/// with `private_owners_from` is the point. An unreadable private-owner list +/// degrades to a NARROWER check, which can be reported and lived with. An +/// unreadable owner degrades to the owner read off `origin` -- the tautology +/// `prevent-public-push` exists to refuse -- and an unreadable visibility +/// degrades to the forge's answer about a visibility that is about to change. +/// Neither of those is a degradation anybody should be able to opt into. +fn read_declaration(root: &Path, field: &str, command: &str) -> Result { + let output = Command::new("sh") + .arg("-c") + .arg(command) + .current_dir(root) + .output() + .map_err(|error| Fatal::new(format!("`{field}`: could not run {command:?}: {error}")))?; + if !output.status.success() { + return Err(Fatal::new(format!( + "`{field}` ran {command:?}, which exited {}: {}\n\nA source that failed declared \ + nothing, and what a missing declaration falls back to is the thing this field \ + exists to replace: the owner read off `origin`, or the forge's view of a \ + visibility that is about to change. Fix the source, or write the value down.", + output.status.code().unwrap_or(-1), + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + let text = String::from_utf8_lossy(&output.stdout); + // Blank lines are dropped before the count, and that is the contract rather + // than an oversight in it: the rule is one VALUE, not one line of output. A + // trailing blank line is what `cat` gives back for a file that ends with + // one, and counting it would make this field refuse the exact shape it + // exists for. Two non-empty lines are still two answers and still refused. + let mut values = text.lines().map(str::trim).filter(|line| !line.is_empty()); + let Some(value) = values.next() else { + return Err(Fatal::new(format!( + "`{field}` ran {command:?}, which exited 0 and printed nothing. Silence is not an \ + answer here: it would leave this repository having declared nothing while the \ + policy file reads as though it had declared something." + ))); + }; + if values.next().is_some() { + return Err(Fatal::new(format!( + "`{field}` ran {command:?}, which printed more than one value. This is one fact \ + about one repository, and taking the first would pin the repository to whatever \ + the command happened to print first. Narrow it to the single value." + ))); + } + Ok(value.to_owned()) +} + +/// A declared fact: the written value, else the command's answer, else nothing. +/// +/// The command runs on the first ask and never again, because the guards ask +/// more than once -- the private-name family asks about visibility three times, +/// once per variant -- and a workspace that answers with an organisation index +/// is paying for a forge round trip each time. +fn declared( + root: &Path, + field: &str, + literal: Option<&str>, + from: Option<&str>, + cache: &OnceLock, +) -> Result> { + if let Some(value) = literal { + return Ok(Some(value.to_owned())); + } + let Some(command) = from else { + return Ok(None); + }; + if let Some(cached) = cache.get() { + return Ok(Some(cached.clone())); + } + let value = read_declaration(root, &format!("{field}_from"), command)?; + // `set` can only lose to a caller that filled the slot first, and that + // caller ran the same command against the same tree, so the loser's answer + // is the winner's answer. + Ok(Some(cache.get_or_init(|| value).clone())) +} + /// Is a declared visibility one that means "everyone can read this"? /// /// `None` where the word is not a visibility at all. One function because the @@ -1537,6 +1812,7 @@ pub(crate) fn load(root: &Path, policy_path: &Path) -> Result { let text = read_to_string(policy_path)?; let file = parse(policy_path, &text)?; refuse_set_header(policy_path, &file)?; + refuse_two_statements_of_one_fact(policy_path, &file)?; // Checked here rather than where a guard reads it. A misspelt visibility is // a fact about the file, and hearing about it when a hook fires means // hearing about it from whichever seam happened to run first, months after @@ -1587,6 +1863,7 @@ pub(crate) fn load(root: &Path, policy_path: &Path) -> Result { let extended = read_to_string(&path)?; let parsed = parse(&path, &extended)?; refuse_set_header(&path, &parsed)?; + refuse_inherited_declaration(&path, &parsed, "an inherited file")?; // Refused rather than merged, and refused rather than ignored. Only // `.rules` is merged below, so an inherited `[[shim]]` used to vanish -- // and vanish in the worst possible way, because the `exec` rule that @@ -1688,6 +1965,10 @@ pub(crate) fn load(root: &Path, policy_path: &Path) -> Result { path: policy_path.to_path_buf(), owner: file.owner.clone(), visibility: file.visibility.clone(), + owner_from: file.owner_from.clone(), + visibility_from: file.visibility_from.clone(), + resolved_owner: OnceLock::new(), + resolved_visibility: OnceLock::new(), private_owners_from: file.private_owners_from.clone(), private_owners_optional: file.private_owners_optional, redact_matches: file.redact_matches, diff --git a/src/guard/mod.rs b/src/guard/mod.rs index bc8b558..79875c0 100644 --- a/src/guard/mod.rs +++ b/src/guard/mod.rs @@ -21,6 +21,7 @@ pub(crate) mod push; pub(crate) mod scope; pub(crate) mod sets; pub(crate) mod unicode; +pub(crate) mod visibility; use std::path::Path; @@ -50,6 +51,11 @@ pub(crate) const EVERY_BUILTIN: &[&str] = &[ "no-local-merge", "no-merge-commit", "no-stale-hook-pins", + // The only other built-in that reaches a network, and the only one whose + // subject is a claim the POLICY makes about the repository rather than + // about a file in it. It refuses one direction -- declared private, served + // public -- because that is the only direction a probe can establish. + "no-stale-visibility", // Reads the POLICY rather than the tree or what git is about to do: the // only check here whose subject is the repository's own declarations. See // `sets` for why that is a check at all. @@ -298,6 +304,7 @@ pub(crate) fn evaluate(request: &Request<'_>) -> Result> { "no-private-repo-names-in-files" => names::in_tracked(request), "prevent-public-push" => push::prevent_public_push(request), "no-stale-hook-pins" => crate::pins::stale(request), + "no-stale-visibility" => visibility::no_stale_visibility(request), "no-hand-copied-base-rule" => sets::no_hand_copied_base_rule(request), other => Err(Fatal::new(format!("no built-in called {other:?}"))), } @@ -323,6 +330,10 @@ pub(crate) fn parameters(builtin: &str) -> &'static [&'static str] { "public_repos", "refuse_unknown", ], + // `visibility` and nothing else: the rule reads the declaration and + // compares it to the forge, and every other field in the family is + // about judging names in text, which this one never does. + "no-stale-visibility" => &["visibility"], "prevent-unusual-unicode-in-files" => &["allow"], _ => &[], } @@ -426,7 +437,7 @@ mod tests { "{id} is in EVERY_BUILTIN with no dispatch arm" ); } - assert_eq!(EVERY_BUILTIN.len(), 14); + assert_eq!(EVERY_BUILTIN.len(), 15); } #[test] diff --git a/src/guard/names.rs b/src/guard/names.rs index 94d90e1..2a6a3b5 100644 --- a/src/guard/names.rs +++ b/src/guard/names.rs @@ -58,8 +58,13 @@ fn clean_repo(name: &str) -> String { .to_owned() } +/// Shared with `guard::visibility`, which asks the same forge the same question +/// about this repository's own name. One mechanism rather than two: two ways of +/// asking whether a repository is public are two answers free to disagree, and +/// the falsifier's exit-state ranking rests on the distinction drawn below +/// between an answer and no answer. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum Visibility { +pub(crate) enum Visibility { Public, Private, /// The forge ANSWERED, and no repository it will show us has this name. @@ -88,9 +93,9 @@ enum Visibility { /// canonical name is how that is told apart from a genuine sibling: same /// repository, not a leak. `None` when there was no answer to be canonical. #[derive(Debug, Clone, PartialEq, Eq)] -struct Resolved { - visibility: Visibility, - canonical: Option, +pub(crate) struct Resolved { + pub visibility: Visibility, + pub canonical: Option, } /// Any `host.tld/owner/repo` or its scp-like `host.tld:owner/repo`, with the @@ -281,7 +286,7 @@ impl OwnerMatchers { /// Both fields come back from one request. `full_name` is what the forge /// redirected to, which is the only way to notice that the name asked about and /// the repository doing the asking are the same repository under two names. -fn lookup(cache: &mut BTreeMap, owner: &str, repo: &str) -> Resolved { +pub(crate) fn lookup(cache: &mut BTreeMap, owner: &str, repo: &str) -> Resolved { let key = format!("{owner}/{repo}"); if let Some(known) = cache.get(&key) { return known.clone(); @@ -361,11 +366,12 @@ fn target_is_public(root: &Path, policy: &Policy, rule: &Rule) -> Result, sources: &[(String, String)]) -> Result, sources: &[(String, String)]) -> Result) -> (Option, bool) { +fn workspace_owner(request: &Request<'_>) -> Result<(Option, bool)> { if let Some(owner) = request.rule.owner.as_deref() { - return (Some(owner.to_owned()), true); + return Ok((Some(owner.to_owned()), true)); } // The policy's own `owner`, which is the pin a rule arriving from a bundled // set can still reach: a set cannot be handed a parameter without writing // the rule out again, and identity is a property of the repository rather // than of any one rule in it. - if let Some(owner) = request.policy.owner.as_deref() { - return (Some(owner.to_owned()), true); + if let Some(owner) = request.policy.declared_owner(request.root)? { + return Ok((Some(owner), true)); } let derived = git::remote_url(request.root, "origin") .and_then(|url| git::owner_repo(&url)) .map(|(owner, _)| owner); - (derived, false) + Ok((derived, false)) } /// Where this push is actually going. @@ -66,7 +71,7 @@ pub(crate) fn prevent_public_push(request: &Request<'_>) -> Result) -> Result> { + let id = &request.rule.id; + + // The declaration is the subject. A rule that checks a claim against the + // world needs a claim, and with none there is nothing to falsify -- so this + // says so rather than passing, which would be a check that looked like it + // ran and examined nothing. + let declared = match request.rule.visibility.as_deref() { + Some(word) => word.to_owned(), + None => match request.policy.declared_visibility(request.root)? { + Some(word) => word, + None => { + return Err(Fatal::new(format!( + "{id}: nothing here declares this repository's visibility, so there is no \ + claim for this rule to check. Declare it once, at the top of the policy \ + file:\n\n visibility = \"private\" # or \"public\", or \"internal\"\n\n\ + or point `visibility_from` at a command that prints one word." + ))) + } + }, + }; + let Some(declared_public) = visibility_is_public(&declared) else { + return Err(Fatal::new(format!( + "{id}: visibility {declared:?} is not a visibility" + ))); + }; + + // Declared public: there is nothing here to falsify. The direction that + // leaks is a declaration of `private` over a repository that is public, and + // a policy already saying `public` has the private-name family at its + // strictest. Said out loud rather than returned silently, because a rule + // that reports nothing is indistinguishable from one that did not run. + if declared_public { + println!( + "{id}: declared {declared:?}, so there is no claim of privacy to disprove and no \ + request was made." + ); + return Ok(None); + } + + let Some(url) = git::remote_url(request.root, "origin") else { + return Err(Fatal::new(format!( + "{id}: this repository has no `origin` remote, so there is no name to ask the \ + forge about and the declared {declared:?} was checked against nothing.\n\n\ + Could not look is not a pass. Bypass this run deliberately with \ + UPHOLD_ALLOW={id}." + ))); + }; + let Some((owner, repo)) = git::owner_repo(&url) else { + return Err(Fatal::new(format!( + "{id}: could not read an owner and a repository out of the `origin` URL {url:?}, \ + so the declared {declared:?} was checked against nothing.\n\nCould not look is \ + not a pass. Bypass this run deliberately with UPHOLD_ALLOW={id}." + ))); + }; + + // The same lookup the private-name family uses, and deliberately not a + // second one. Two mechanisms answering one question are two answers free to + // disagree, and this one depends on the distinction that mechanism already + // draws: `gh` separates "the forge will not show us this" from "the forge + // would not talk to us", which is exactly the line between a fact and an + // absent check here. + let mut cache = BTreeMap::new(); + match lookup(&mut cache, &owner, &repo).visibility { + Visibility::Public => Ok(Some(Refusal { + id: id.clone(), + report: format!( + "the policy declares visibility {declared:?}, and the forge serves \ + {owner}/{repo} as PUBLIC.\n\nThe declaration is what the private-name \ + guards read as the condition they fire under, so while it says \ + {declared:?} they stand down -- over a repository everybody can read. \ + Change the declared visibility to \"public\" and let them run, or change \ + the repository back." + ), + })), + Visibility::Private => { + println!("{id}: declared {declared:?}, and the forge agrees."); + Ok(None) + } + // Neither of these disproves the declaration and neither confirms it. + // `Unknown` is the forge answering that it will show us no repository by + // this name, which for a repository declared private is the ordinary + // answer to an unauthenticated request -- and it is also the answer for + // one that was deleted or renamed. `Unavailable` is no answer at all. + // Both are the check not happening, and a check that did not happen has + // never been a pass anywhere in this binary. + Visibility::Unknown | Visibility::Unavailable => 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 \ + disprove a claim of privacy and can never confirm one, so it does not read \ + silence as agreement.\n\nCould not look is not a pass. Authenticate `gh`, or \ + bypass this run deliberately with UPHOLD_ALLOW={id}." + ))), + } +} diff --git a/tests/base_sets_cli.rs b/tests/base_sets_cli.rs index 6a2dc34..2635e1b 100644 --- a/tests/base_sets_cli.rs +++ b/tests/base_sets_cli.rs @@ -68,14 +68,35 @@ fn code(output: &Output) -> i32 { /// each. A test that called the classifier directly would be asserting on the /// function rather than on the behaviour a repository gets. fn guard_with_gh(root: &Path, stderr_line: &str, args: &[&str]) -> Output { + guard_with_stub_gh(root, &format!("echo '{stderr_line}' >&2\nexit 1\n"), args) +} + +/// The same, for a stub that ANSWERS rather than fails. +/// +/// The falsifier's whole subject is the difference between a forge saying +/// `public`, a forge saying `private`, and a forge that said neither, so a +/// helper that can only produce the third would leave two of the three states +/// untested. `guard_with_gh` above is this one with its body written for it. +fn guard_with_stub_gh(root: &Path, body: &str, args: &[&str]) -> Output { let bin = root.join("stub-bin"); std::fs::create_dir_all(&bin).unwrap(); let gh = bin.join("gh"); - std::fs::write( - &gh, - format!("#!/bin/sh\necho '{stderr_line}' >&2\nexit 1\n"), - ) - .unwrap(); + std::fs::write(&gh, format!("#!/bin/sh\n{body}")).unwrap(); + // The real git, ahead of whatever the developer's PATH puts there. A `git` + // shim loads the policy of the tree it is invoked in and fails closed when + // that policy will not load -- so on a machine with one installed, the shim + // rather than the guard would decide what `git remote get-url` answers in + // this fixture, and a guard that reads `origin` would be testing the + // installed binary instead of this one. + // + // Guarded like the mode bits below it: this file writes `#!/bin/sh` stubs + // and is Unix-shaped throughout, and an unguarded `std::os::unix` call in + // the middle of it would be the one line that stops the suite COMPILING + // elsewhere rather than merely failing. + #[cfg(unix)] + { + let _ = std::os::unix::fs::symlink(support::real_git(), bin.join("git")); + } #[cfg(unix)] { use std::os::unix::fs::PermissionsExt as _; @@ -837,3 +858,432 @@ fn every_guard_set_declares_the_stages_its_rules_install() { ); } } + +// --------------------------------------------------------------------------- +// A repository fact declared somewhere other than the policy file. +// +// `owner` and `visibility` are declared once per repository, which across one +// fleet means 78 copies of an `owner` line for seven distinct values. Reading +// the value from a command is the shape `private_owners_from` already uses, and +// the cases below are about the thing that makes it safe: it is a DECLARATION +// moved out of the tree, never a lookup of the fact being guarded, so every way +// it can fail to answer is exit 2 rather than the fallback it replaced. +// --------------------------------------------------------------------------- + +#[test] +fn an_owner_read_from_a_command_pins_a_push_the_way_a_written_one_does() { + // The pin, and the half that proves it is a pin: `unowned-push` sets + // `owner_required`, so a repository the guard considers unpinned exits 2 + // with the line to write. Exit 0 here means the command answered and the + // answer was treated as the declaration, not as a guess off `origin`. + let root = + repository("owner_from = \"printf 'acme\\n'\"\n\n[inherit]\nsets = [\"unowned-push\"]\n"); + commit_one(&root); + + let allowed = guard( + &root, + &[ + "--stage", + "pre-push", + "--remote-url", + "https://github.com/acme/widget.git", + ], + ); + assert_eq!(code(&allowed), 0, "{}", stderr(&allowed)); + // And it is the pinned mode, not the derived one. The guard says so on the + // allow path precisely because a derived owner allows everything it should + // have refused, and never mentions it. + assert!( + !stderr(&allowed).contains("DERIVED FROM ORIGIN"), + "{}", + stderr(&allowed) + ); + + let refused = guard( + &root, + &[ + "--stage", + "pre-push", + "--remote-url", + "https://github.com/someone-else/widget.git", + ], + ); + assert_eq!(code(&refused), 1, "{}", stderr(&refused)); +} + +#[test] +fn an_owner_source_that_failed_is_exit_two_and_not_the_owner_read_off_origin() { + // The whole reason there is no `owner_optional` beside this. An unreadable + // private-owner list degrades to a NARROWER check, which can be reported and + // lived with; an unreadable owner degrades to the owner read off `origin`, + // which is the tautology `prevent-public-push` exists to refuse. So the + // failure has to be louder than the fallback, not quieter. + let root = repository( + "owner_from = \"cat no-such-file-anywhere\"\n\n[inherit]\nsets = [\"unowned-push\"]\n", + ); + commit_one(&root); + + let output = guard( + &root, + &[ + "--stage", + "pre-push", + "--remote-url", + "https://github.com/someone-else/widget.git", + ], + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + let text = stderr(&output); + assert!(text.contains("owner_from"), "{text}"); + // Naming what it would otherwise have fallen back to, because a reader who + // cannot see the difference between exit 1 and exit 2 here will assume the + // guard did its job. + assert!(text.contains("read off `origin`"), "{text}"); +} + +#[test] +fn a_source_that_answers_more_than_once_is_refused_rather_than_read_partly() { + // This is one fact about one repository. Taking the first line would pin the + // repository to whatever the command happened to print first -- which is a + // decision nobody made, arriving as a silent success. + let root = repository( + "owner_from = \"printf 'acme\\nother\\n'\"\n\n[inherit]\nsets = [\"unowned-push\"]\n", + ); + commit_one(&root); + + let output = guard( + &root, + &[ + "--stage", + "pre-push", + "--remote-url", + "https://github.com/acme/widget.git", + ], + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("more than one value"), + "{}", + stderr(&output) + ); +} + +/// A trailing blank line is not a second answer. +/// +/// Pinned as a test because it reads as an oversight and is the contract: the +/// rule is one VALUE, not one line of output. `cat` gives a trailing blank line +/// back for any file that ends with one, so counting it would make the field +/// refuse the exact shape it exists for -- and the two-answer case beside it is +/// what the count is actually for. +#[test] +fn a_blank_line_after_the_answer_is_not_a_second_answer() { + let root = repository( + "owner_from = \"printf 'acme\\n\\n'\"\n\n[inherit]\nsets = [\"unowned-push\"]\n", + ); + commit_one(&root); + + let output = guard( + &root, + &[ + "--stage", + "pre-push", + "--remote-url", + "https://github.com/acme/widget.git", + ], + ); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn a_source_that_answers_nothing_at_all_is_refused_rather_than_read_as_absent() { + // Exit 0 and no output is the most dangerous shape a source has: the policy + // file reads as though the repository declared something, and it declared + // nothing. + let root = repository("owner_from = \"true\"\n\n[inherit]\nsets = [\"unowned-push\"]\n"); + commit_one(&root); + + let output = guard( + &root, + &[ + "--stage", + "pre-push", + "--remote-url", + "https://github.com/acme/widget.git", + ], + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("printed nothing"), + "{}", + stderr(&output) + ); +} + +#[test] +fn a_visibility_read_from_a_command_decides_whether_the_family_runs() { + // The scope condition of three guards, arriving from outside the tree. Both + // directions are asserted in one test on purpose: a `private` that stood the + // family down for the wrong reason -- because the command failed, say -- + // would be indistinguishable from this one without the `public` half beside + // it, and standing a disclosure guard down quietly is the failure mode. + let rule = "[rule.no-private-repo-names]\nbuiltin = \"no-private-repo-names\"\n\ + private_owners = [\"secretcorp\"]\n\n\ + [rule.no-private-repo-names.git]\nhooks = [\"commit-msg\"]\n"; + let root = repository(&format!( + "visibility_from = \"printf 'private\\n'\"\n\n{rule}" + )); + commit_one(&root); + write(&root, "msg.txt", "see secretcorp/thing\n"); + + let stood_down = guard(&root, &["--stage", "commit-msg", "--message", "msg.txt"]); + assert_eq!(code(&stood_down), 0, "{}", stderr(&stood_down)); + + std::fs::write( + root.join("policy/principles.toml"), + format!("visibility_from = \"printf 'public\\n'\"\n\n{rule}"), + ) + .unwrap(); + + let fired = guard(&root, &["--stage", "commit-msg", "--message", "msg.txt"]); + assert_eq!(code(&fired), 1, "{}", stderr(&fired)); + assert!( + stderr(&fired).contains("secretcorp/thing"), + "{}", + stderr(&fired) + ); +} + +#[test] +fn a_visibility_source_answering_a_word_that_is_not_a_visibility_is_exit_two() { + // A written `visibility` is held to the three spellings at load. A command's + // answer cannot be -- there is no answer until it runs -- so it is held to + // them as it is read, and the refusal names the command rather than the + // file, because the file is right and the thing it points at is not. + let root = repository( + "visibility_from = \"printf 'pubic\\n'\"\n\n[inherit]\nsets = [\"private-names\"]\n", + ); + commit_one(&root); + write(&root, "msg.txt", "a message\n"); + + let output = guard(&root, &["--stage", "commit-msg", "--message", "msg.txt"]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + let text = stderr(&output); + assert!(text.contains("visibility_from"), "{text}"); + assert!(text.contains("not a visibility"), "{text}"); +} + +#[test] +fn one_fact_written_down_and_also_read_from_a_command_is_refused_at_load() { + // Two statements of one fact, free to disagree, with nothing here to notice + // when they do -- which is the defect the field exists to remove, arriving + // through the field. + for (written, source) in [ + ("owner = \"acme\"", "owner_from = \"printf 'acme\\n'\""), + ( + "visibility = \"public\"", + "visibility_from = \"printf 'public\\n'\"", + ), + ] { + let root = repository(&format!( + "{written}\n{source}\n\n[inherit]\nsets = [\"process-residue\"]\n" + )); + commit_one(&root); + + let output = guard(&root, &["--stage", "pre-commit"]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("two statements of one fact"), + "{}", + stderr(&output) + ); + } +} + +#[test] +fn a_file_this_repository_did_not_write_may_not_carry_a_command_that_speaks_for_it() { + // The reason `private-names` gives for not shipping `private_owners_from`, + // and it is not weakened by the command being one line shorter: a command + // arriving by inheritance runs in every repository that inherits it on the + // strength of a version bump, with nothing in any of those trees to review. + // + // Refused rather than dropped. An inherited `owner` is read by nothing and + // says nothing about it, which is the "looks declared, enforced by nobody" + // shape refused everywhere else here; these two are new and start correct. + let root = repository("[inherit]\npaths = [\"policy/shared.toml\"]\n"); + std::fs::write( + root.join("policy/shared.toml"), + "owner_from = \"printf 'acme\\n'\"\n\n\ + [rule.no-shouting]\nregexp = \"SHOUTING\"\nmessage = \"do not shout\"\n", + ) + .unwrap(); + commit_one(&root); + + let output = guard(&root, &["--stage", "pre-commit"]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("owner_from"), + "{}", + stderr(&output) + ); +} + +// --------------------------------------------------------------------------- +// The declared visibility, checked against the forge that owns the fact. +// +// `stale-visibility` refuses exactly one state -- a policy claiming privacy over +// a repository the forge serves as public -- because that is the only direction +// a probe can establish and it is the direction that leaks. Every other answer +// is either clean or "could not look", and the cases below are mostly about +// keeping those two apart: a rule that read silence as agreement would disarm +// three disclosure guards on any machine with no credentials. +// --------------------------------------------------------------------------- + +/// A repository with an `origin` for the falsifier to ask about. +fn repository_with_origin(policy: &str) -> PathBuf { + let root = repository(policy); + git( + &root, + &[ + "remote", + "add", + "origin", + "https://github.com/acme/widget.git", + ], + ); + root +} + +/// A stub `gh api` answering the way the real one does: the `--jq` template the +/// lookup passes asks for visibility and full name as one tab-separated line. +fn gh_answers(visibility: &str) -> String { + format!("printf '{visibility}\\tacme/widget\\n'\nexit 0\n") +} + +#[test] +fn a_policy_declaring_public_has_no_claim_of_privacy_and_makes_no_request() { + // The state most repositories are in, and it must cost them nothing. The + // stub `gh` fails if it is called at all, so exit 0 here is evidence that + // no request was made rather than evidence that one succeeded. + let root = repository_with_origin( + "visibility = \"public\"\n\n[inherit]\nsets = [\"stale-visibility\"]\n", + ); + commit_one(&root); + + let output = guard_with_gh( + &root, + "gh: Bad credentials (HTTP 401)", + &["--stage", "pre-push"], + ); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + assert!( + String::from_utf8_lossy(&output.stdout).contains("no request was made"), + "{}", + String::from_utf8_lossy(&output.stdout) + ); +} + +#[test] +fn a_declared_privacy_the_forge_serves_as_public_is_refused() { + // The one refusable state, and the whole reason the rule exists: while the + // policy says `private` the three private-name guards stand down, over a + // repository everybody can read. + let root = repository_with_origin( + "visibility = \"private\"\n\n[inherit]\nsets = [\"stale-visibility\"]\n", + ); + commit_one(&root); + + let output = guard_with_stub_gh(&root, &gh_answers("public"), &["--stage", "pre-push"]); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + let text = stderr(&output); + assert!(text.contains("acme/widget"), "{text}"); + assert!(text.contains("PUBLIC"), "{text}"); + // And it says where the rule came from, which is the only thing a reader + // grepping the tree for the id would otherwise not find. + assert!(text.contains("[set: stale-visibility]"), "{text}"); +} + +#[test] +fn a_forge_that_agrees_the_repository_is_private_is_clean() { + let root = repository_with_origin( + "visibility = \"private\"\n\n[inherit]\nsets = [\"stale-visibility\"]\n", + ); + commit_one(&root); + + let output = guard_with_stub_gh(&root, &gh_answers("private"), &["--stage", "pre-push"]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + assert!( + String::from_utf8_lossy(&output.stdout).contains("the forge agrees"), + "{}", + String::from_utf8_lossy(&output.stdout) + ); +} + +#[test] +fn a_name_the_forge_will_not_show_us_is_never_read_as_agreement() { + // The case the whole design turns on. A 404 is the ORDINARY answer for a + // genuinely private repository asked about without credentials -- and it is + // also the answer for one that was deleted, renamed, or never existed. A + // rule that read it as "confirmed private" would let an unauthenticated + // runner confirm any claim at all, which is fail-open on the one family + // where fail-open is unacceptable. + for stderr_line in ["gh: Not Found (HTTP 404)", "gh: Bad credentials (HTTP 401)"] { + let root = repository_with_origin( + "visibility = \"private\"\n\n[inherit]\nsets = [\"stale-visibility\"]\n", + ); + commit_one(&root); + + let output = guard_with_gh(&root, stderr_line, &["--stage", "pre-push"]); + assert_eq!(code(&output), 2, "{stderr_line}: {}", stderr(&output)); + let text = stderr(&output); + assert!(text.contains("Could not look is not a pass"), "{text}"); + // Named, because a guard that fails on a machine with no credentials + // and offers no way past it is a guard somebody deletes. + assert!(text.contains("UPHOLD_ALLOW=no-stale-visibility"), "{text}"); + } +} + +#[test] +fn a_repository_that_declares_no_visibility_has_no_claim_for_this_rule_to_check() { + // Not a pass. The subject of this rule is a declaration, and with none there + // is nothing to falsify -- which is a different answer from having checked + // and found nothing wrong. + let root = repository_with_origin("[inherit]\nsets = [\"stale-visibility\"]\n"); + commit_one(&root); + + let output = guard_with_stub_gh(&root, &gh_answers("public"), &["--stage", "pre-push"]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("no \nclaim") || stderr(&output).contains("no claim"), + "{}", + stderr(&output) + ); +} + +#[test] +fn the_visibility_read_from_a_command_is_the_claim_this_rule_checks() { + // The two halves of #54 meeting: the declaration comes from outside the + // tree, and it is still a declaration -- checked against the forge exactly + // as a written one is, and never replaced by what the forge said. + let root = repository_with_origin( + "visibility_from = \"printf 'private\\n'\"\n\n[inherit]\nsets = [\"stale-visibility\"]\n", + ); + commit_one(&root); + + let output = guard_with_stub_gh(&root, &gh_answers("public"), &["--stage", "pre-push"]); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + assert!(stderr(&output).contains("PUBLIC"), "{}", stderr(&output)); +} + +#[test] +fn the_falsifier_never_runs_at_a_commit() { + // The stage decision, asserted rather than trusted to the set file. A guard + // that adds a network round trip to every commit is one somebody comments + // out, which is the reason `stale-pins` is off `pre-commit` too. + let root = repository_with_origin( + "visibility = \"private\"\n\n[inherit]\nsets = [\"stale-visibility\"]\n", + ); + commit_one(&root); + + let output = guard_with_stub_gh(&root, &gh_answers("public"), &["--stage", "pre-commit"]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +}