diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5742096..a2d3686 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -75,22 +75,30 @@ the policy file, and the two forms the note names are checked as well. See ## Where a decision goes -A change that needs a decision is a GitHub issue in the -[Design record](.github/ISSUE_TEMPLATE/design.md) shape before a pull request -exists: the decision in the body, and the research in a comment that carries -the version, date and evidence of each claim. A Design record carries exactly +A proposal that benefits from design discussion can use a GitHub issue in the +[Design record](.github/ISSUE_TEMPLATE/design.md) shape: the proposed decision +in the body, and the research in a comment that carries the version, date and +evidence of each claim. A Design record carries exactly one ruling label. The template files it with `ruling:pending`, and the ruling is recorded by replacing that label with `ruling:accepted`, `ruling:rejected` or `ruling:deferred`. -The repository receives only what runs, plus -[REFERENCE.md](docs/REFERENCE.md), which stays the normative statement of what -uphold does now. Tracked files carry no issue numbers, dates or status lines; -those stay in the Design record that rules the change. - -Decisions live in Design record issues. [DESIGN.md](docs/DESIGN.md) explains -the current architecture; [REFERENCE.md](docs/REFERENCE.md) defines its -behavior. Retired decision files remain available in Git history. +The repository carries runnable content and current-state documentation, +including [DESIGN.md](docs/DESIGN.md) for the architecture and +[REFERENCE.md](docs/REFERENCE.md) for normative behavior. +Tracked files carry no issue numbers, dates or status lines; +those stay in the tracker and Git history. + +The versioned source tree is authoritative for the delivered system: its +implementation, tests, [DESIGN.md](docs/DESIGN.md) and +[REFERENCE.md](docs/REFERENCE.md) describe the accepted architecture and +behavior together. Design record issues hold proposals, discussion and +historical context. An accepted proposal changes the delivered system only +through a reviewed repository change; an issue's status or age does not change +what a revision implements or whether it passes its checks. Keep the accepted +contract and the rationale needed to maintain it in the repository so a reader +can understand the system without consulting the issue tracker. Retired +decision files remain available in Git history. ## Working on the engine diff --git a/Cargo.lock b/Cargo.lock index 4cb23c0..6c81f48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,6 +63,12 @@ dependencies = [ "serde_core", ] +[[package]] +name = "bytes" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" + [[package]] name = "cc" version = "1.4.4" @@ -146,7 +152,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -232,6 +238,22 @@ version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" +[[package]] +name = "http" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "918d3568bebf352712bc2ef3d46a8bcf1a75b373be6539de198e9105cbbf9ce0" +dependencies = [ + "bytes", + "itoa", +] + +[[package]] +name = "httparse" +version = "1.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" + [[package]] name = "ignore" version = "0.4.33" @@ -318,6 +340,18 @@ dependencies = [ "autocfg", ] +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + +[[package]] +name = "percent-encoding" +version = "2.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" + [[package]] name = "phf" version = "0.11.3" @@ -374,6 +408,15 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "pyreq" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c6eb3ecbf06d7c20d3e767683ce199979ed7bfdb5ae421f0b6dcb9f391d3462f" +dependencies = [ + "regex", +] + [[package]] name = "quote" version = "1.0.47" @@ -442,6 +485,20 @@ version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" +[[package]] +name = "ring" +version = "0.17.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" +dependencies = [ + "cc", + "cfg-if", + "getrandom", + "libc", + "untrusted", + "windows-sys 0.52.0", +] + [[package]] name = "rustix" version = "1.1.5" @@ -452,7 +509,42 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys", + "windows-sys 0.61.2", +] + +[[package]] +name = "rustls" +version = "0.23.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" +dependencies = [ + "log", + "once_cell", + "ring", + "rustls-pki-types", + "rustls-webpki", + "subtle", + "zeroize", +] + +[[package]] +name = "rustls-pki-types" +version = "1.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f4925028c7eb5d1fcdaf196971378ed9d2c1c4efc7dc5d011256f76c99c0a96" +dependencies = [ + "zeroize", +] + +[[package]] +name = "rustls-webpki" +version = "0.103.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3c3cf1d8b1e7d4927e2d154c3fcb02979afb9939629c62cd9048d4f07b60ac2" +dependencies = [ + "ring", + "rustls-pki-types", + "untrusted", ] [[package]] @@ -582,6 +674,12 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b2231b7c3057d5e4ad0156fb3dc807d900806020c5ffa3ee6ff2c8c76fb8520" +[[package]] +name = "subtle" +version = "2.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" + [[package]] name = "syn" version = "2.0.119" @@ -755,6 +853,12 @@ dependencies = [ "rand", ] +[[package]] +name = "untrusted" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" + [[package]] name = "uphold" version = "1.31.0" @@ -765,6 +869,7 @@ dependencies = [ "grep-regex", "grep-searcher", "ignore", + "pyreq", "regex", "rustix", "serde", @@ -779,8 +884,43 @@ dependencies = [ "unicode-script", "unicode-security", "unicode_names2", + "ureq", ] +[[package]] +name = "ureq" +version = "3.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a7ac20be9b7726e0bbdbf974c059676d9acb1cd414961f570a4e8231cacd7fc" +dependencies = [ + "base64", + "log", + "percent-encoding", + "rustls", + "rustls-pki-types", + "ureq-proto", + "utf8-zero", + "webpki-roots", +] + +[[package]] +name = "ureq-proto" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f86fd172ccca569e458f61b6bdd6220965a9ef36e672a6852953b51a0e1583be" +dependencies = [ + "base64", + "http", + "httparse", + "log", +] + +[[package]] +name = "utf8-zero" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8c0a043c9540bae7c578c88f91dda8bd82e59ae27c21baca69c8b191aaf5a6e" + [[package]] name = "walkdir" version = "2.5.0" @@ -797,13 +937,22 @@ version = "0.11.1+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" +[[package]] +name = "webpki-roots" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dcd9d09a39985f5344844e66b0c530a33843579125f23e21e9f0f220850f22a" +dependencies = [ + "rustls-pki-types", +] + [[package]] name = "winapi-util" version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -812,6 +961,15 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-sys" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" +dependencies = [ + "windows-targets", +] + [[package]] name = "windows-sys" version = "0.61.2" @@ -821,6 +979,70 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-targets" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" +dependencies = [ + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_gnullvm", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", +] + +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" + +[[package]] +name = "windows_aarch64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" + +[[package]] +name = "windows_i686_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" + +[[package]] +name = "windows_i686_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" + +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + [[package]] name = "winnow" version = "1.0.4" @@ -847,6 +1069,12 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "zeroize" +version = "1.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e13084392c5e4bc371903e2935a5eaeed24905a7511356b883835e18a78f6879" + [[package]] name = "zmij" version = "1.0.23" diff --git a/Cargo.toml b/Cargo.toml index 1a72ba6..115caf7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -112,6 +112,11 @@ tree-sitter-go = "0.25" # positional word after them. Where this reader and git's `split_cmdline` # disagree, `expand_alias` says which case and how it is closed. shell-words = "1" +# The exact-release PyPI metadata used to verify optional extras, with bounded +# HTTPS requests and no asynchronous runtime. +ureq = { version = "3.4.2", default-features = false, features = ["rustls"] } +# Parse PEP 508 requirements before accepting the closed optional-extra form. +pyreq = "0.2.0" [target.'cfg(unix)'.dependencies] # Git discovers hooks with access(X_OK), which checks the caller's permission diff --git a/README.md b/README.md index 8673212..6782052 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,11 @@ sets supply, and the `.pre-commit-config.yaml` below at this binary's version has a policy, and leaves an existing hook config as it is, printing the block to add. The owner and the visibility are stated, never read from `origin`. +Share organization rules without copying them: `[inherit] remote` accepts a +GitHub `owner/name`, a full commit ID in `rev`, and a policy-file `path`. +Remote rules support the same overrides as bundled rules and show their +provenance in `uphold rules --effective`. See [remote inheritance](docs/REFERENCE.md). + **pre-commit / prek** — one manifest serves both, and no Rust toolchain is needed (`language: rust` bootstraps it). diff --git a/deny.toml b/deny.toml index a49c4f2..135dddf 100644 --- a/deny.toml +++ b/deny.toml @@ -41,6 +41,9 @@ allow = [ "Apache-2.0 WITH LLVM-exception", "BSD-2-Clause", "BSD-3-Clause", + # TLS implementation and the Mozilla root certificate data carried by ureq. + "ISC", + "CDLA-Permissive-2.0", "MIT", "Unicode-3.0", "Unicode-DFS-2016", diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 124d8a3..16a05b0 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -839,6 +839,38 @@ and is a one-line diff that says so. `[set]` in a repository's own policy, or in an `inherit.paths` file, is refused — nothing there ships compiled in, so nothing there has the problem the ceiling exists for. +Organization rules may instead come from a GitHub repository at an immutable +commit. They are merged after `inherit.paths`, before the repository's own +rules, and use the same `[override.]`, shadowing and `disabled_rules` behavior: + +```toml +[inherit] +remote = [{ repo = "example/policy", rev = "0123456789abcdef0123456789abcdef01234567", path = "policy/org.toml" }] + +[override.org-rule.files] +exclude = ["fixtures/**"] +``` + +`repo` is `owner/name` on GitHub. `rev` must be a full 40-character commit ID; +a branch, abbreviated ID or tag is refused with exit 2 because it can move. +Resolve a release tag to its commit before recording the pin. Missing commits, +unreachable repositories, missing files, symlinks, directories, non-UTF-8 +content and invalid policy all fail with exit 2. Nothing falls back to the +repository's latest revision. Each load fetches into a fresh temporary bare +repository and removes it afterwards; network access and Git credentials for +private sources must therefore be available wherever the policy runs. Git's +normal credential helpers and URL rewrites apply. Terminal and askpass +credential prompts are disabled; HTTP transfers below one byte per second +for 30 seconds abort. No checkout, submodules or source repository hooks run. + +Only rules are adopted. Remote files may not declare their own `[inherit]`, +`[override]` or `[[shim]]`: sources and shims remain explicit decisions in the +consumer's own policy. This deliberately leaves org-wide shim inheritance +unsupported, separately from the bundled-set constraint. `rules --effective` +shows each remote rule's repository, commit and path; `--json` includes them in +its `remote` object, preserved when an override adjusts the rule. An own rule +that replaces the inherited id has its own provenance instead. + `inherit.disabled_rules` drops an inherited rule, and naming an id nothing inherited defines is an error. `inherit.paths` merges extra policy files, repository-relative, after the bundled sets. A path must stay inside the @@ -3253,7 +3285,7 @@ itself calls clean. It trades a false negative for a false positive. So guarddog is run with `--output-format json` and its own `risks` list is counted. This is the one scanner whose findings are read here. The count is reported, never recomputed, with one named exception: a `metadata_mismatch` -risk made up only of npm's publish-time normalizations is printed as +risk made up only of known metadata normalizations is printed as tolerated rather than counted ([below](#what-npm-writes-at-publish-is-tolerated-by-name)). @@ -3397,19 +3429,41 @@ these is true: `: Manifest("` or the `"), package.json("` marker appears more than once is not read. -Two cases are not on the list: - -- `"install": "node-gyp rebuild"`, which npm adds where `binding.gyp` exists. An - install script runs at install time, so it is not the same risk as `start`. -- PyPI optional extras, which guarddog 3.2.0 compares against the required - dependencies. Its report keeps the names and drops the `extra ==` markers, - so the report alone cannot tell an extra from an injected dependency. Those - findings still need a waiver. +`"install": "node-gyp rebuild"`, which npm adds where `binding.gyp` exists, +is not on the list. An install script runs at install time, so it is not the +same risk as `start`. guarddog's report does not list the tarball's files, so the `server.js` that npm's step requires is not confirmed. guarddog also prints an absent value as `None`, the same way it prints a literal `"None"`. +### PyPI optional extras require release evidence + +guarddog compares every `requires_dist` name with the required dependencies +in `pyproject.toml`, including names declared only under optional extras. Its +report drops the markers, so uphold fetches the exact scanned release from +[PyPI's release JSON endpoint](https://docs.pypi.org/api/json/) only when the +entire difference consists of names present in PyPI metadata and absent from +`pyproject.toml`. It verifies the response's package and version, then parses +all `requires_dist` entries with `pyreq`. + +A name is tolerated only when every occurrence has a simple positive +`extra == "name"` marker (including the reversed equality). An occurrence +with no marker, a platform marker, a conjunction or an alternative keeps the +finding. This closed form covers the ordinary extras declarations without +evaluating conditions against the host. All reported names must qualify. Both copies of guarddog's +description must agree; malformed reports, unknown differences, missing +metadata or unparseable requirements retain the finding and remain eligible +for an exact-version waiver. + +The lookup uses HTTPS at `pypi.org`, refuses redirects, and is bounded to +15 seconds and 2 MiB per response. A failed lookup prints why the finding was +retained. A successful normalization prints `tolerated:` with the verified +names and release. No package-wide exemption or cross-run metadata cache is +created. This verifies registry metadata, not the artifact contents; PyPI +metadata can differ from the uploaded artifact. This does not repair guarddog's order-dependent choice of a wheel +root, so an unmatched PyPI metadata waiver is still not called removable. + ### Waiving a confirmed guarddog false positive Because the findings are read here, a finding somebody has looked at and judged diff --git a/src/config.rs b/src/config.rs index 2b6b567..603dcf0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -370,6 +370,9 @@ pub(crate) struct Inherit { /// and before the repository's own rules, in the order written. #[serde(default)] pub paths: Vec, + /// Organization policy fetched at an immutable GitHub commit, after local paths. + #[serde(default)] + pub remote: Vec, /// Ids dropped from everything inherited. A repository's own rule of the /// same id overrides instead, which is the other half of the same choice. #[serde(default)] @@ -758,6 +761,8 @@ pub(crate) enum Origin { Set(String), /// Inherited from an `inherit.paths` entry, named by the path as written. Path(String), + /// A policy in another repository, with its immutable revision and path. + Remote(crate::remote_policy::Remote), } impl Origin { @@ -765,7 +770,7 @@ impl Origin { pub(crate) fn set(&self) -> Option<&str> { match self { Self::Set(name) => Some(name), - Self::Own | Self::Path(_) => None, + Self::Own | Self::Path(_) | Self::Remote(_) => None, } } } @@ -1787,6 +1792,67 @@ fn inherited_path(root: &Path, policy_path: &Path, relative: &str) -> Result Result> { + refuse_set_header(path, &parsed)?; + refuse_inherited_declaration(path, &parsed, "an inherited file")?; + refuse_inherited_override(path, &parsed, "an inherited file")?; + // Only `.rules` is merged from an inherited file, and a top-level line + // that would vanish is refused rather than dropped. A bundled set's + // default is adopted because the set is reviewed with the binary; an + // inherited file supplies rules, while the consumer owns its identity. + if parsed.private_owners_file.is_some() { + return Err(Fatal::at( + path, + format!( + "{} declares `private_owners_file` at its top level, and an inherited \ + file's top-level declarations are not adopted. Write the line at the \ + top of the consumer's policy file", + path.display() + ), + )); + } + // 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 + // came with it survived, and `validate_shims` then reported that rule's + // `command.before` as naming a shim nobody declared. The author would be + // told to declare the shim they had in fact declared. Merging is the + // other available answer and it is not obviously right: a shim is the + // thing that stands in front of a command, and inheriting one silently + // puts a program in front of `git` on the strength of a path in an + // `[inherit]` line. Until that is a decision somebody makes on purpose, + // say so here. + if !parsed.shims.is_empty() { + return Err(Fatal::at( + path, + format!( + "{} declares {} `[[shim]]` table(s), and an inherited file's shims are \ + not adopted. A shim stands in front of a real command, which is not \ + something to acquire by inheriting a path. Move the `[[shim]]` into \ + the consumer's policy file", + path.display(), + parsed.shims.len() + ), + )); + } + if matches!(origin, Origin::Remote(_)) && parsed.inherit.is_some() { + return Err(Fatal::at( + path, + "an inherited file may not declare `[inherit]`; list each source in the repository's own policy", + )); + } + Ok(parsed + .rules + .into_values() + .map(|mut rule| { + rule.origin = origin.clone(); + rule + }) + .collect()) +} + /// Load a policy file, resolving `[inherit]` and validating the result. pub(crate) fn load(root: &Path, policy_path: &Path) -> Result { let text = read_to_string(policy_path)?; @@ -1871,53 +1937,21 @@ pub(crate) fn load_text(root: &Path, policy_path: &Path, text: &str) -> Result

Result

= file.rules.keys().map(String::as_str).collect(); let disabled = &inherit.disabled_rules; + let inheritable: Vec = inherited.iter().map(|rule| rule.id.clone()).collect(); report_reshaped_shadows(&inherited, &file.rules); let mut rules: Vec = inherited @@ -2036,20 +2071,6 @@ pub(crate) fn load_text(root: &Path, policy_path: &Path, text: &str) -> Result

= { - let mut names = Vec::new(); - for name in &inherit.sets { - if let Some((_, bundled)) = BUNDLED.iter().find(|(bundled, _)| *bundled == name) { - names.extend(parse_bundled(name, bundled)?.rules.into_keys()); - } - } - for relative in &inherit.paths { - let path = inherited_path(root, policy_path, relative)?; - let extended = read_to_string(&path)?; - names.extend(parse(&path, &extended)?.rules.into_keys()); - } - names - }; for id in disabled { if !inheritable.contains(id) { return Err(Fatal::at( @@ -2133,6 +2154,7 @@ fn report_reshaped_shadows(inherited: &[Rule], own: &BTreeMap) { let from = match &rule.origin { Origin::Set(name) => format!("the bundled set {name:?}"), Origin::Path(path) => format!("the inherited file {path:?}"), + Origin::Remote(remote) => format!("the remote policy {}", remote.label()), Origin::Own => continue, }; eprintln!( diff --git a/src/main.rs b/src/main.rs index a2a2d21..dc31399 100644 --- a/src/main.rs +++ b/src/main.rs @@ -95,6 +95,7 @@ mod pins; mod policy; mod probe; mod prose; +mod remote_policy; mod report; mod runner; mod scan; @@ -979,6 +980,8 @@ fn pruned(value: serde_json::Value) -> serde_json::Value { #[derive(serde::Serialize)] struct EffectiveRule<'rule> { id: &'rule str, + #[serde(skip_serializing_if = "Option::is_none")] + remote: Option<&'rule remote_policy::Remote>, git_hooks: &'rule [String], /// `git_hooks` alone cannot answer where a hookless rule runs, and a caller /// that has to guess guesses the scan -- which is how a claim on a rule @@ -1047,6 +1050,9 @@ fn effective_rules_command(as_json: bool) -> Result { rule.overridden.join(", ") ); } + if let config::Origin::Remote(remote) = &rule.origin { + println!(" source: {}", remote.label()); + } } return Ok(Exit::Clean); } @@ -1056,6 +1062,10 @@ fn effective_rules_command(as_json: bool) -> Result { .iter() .map(|rule| EffectiveRule { id: &rule.id, + remote: match &rule.origin { + config::Origin::Remote(remote) => Some(remote), + _ => None, + }, git_hooks: rule.hooks(), seams: rule.seams(), reach: (rule.reach() == config::Reach::Pinned).then_some(config::Reach::Pinned), diff --git a/src/remote_policy.rs b/src/remote_policy.rs new file mode 100644 index 0000000..c17391a --- /dev/null +++ b/src/remote_policy.rs @@ -0,0 +1,207 @@ +//! Rules from another repository are read as blobs, never checked out or run. +//! The commit is the pin: a branch or tag cannot substitute new policy under +//! the same configuration. Fetch failure never falls back to another revision. + +use std::path::{Component, Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use serde::{Deserialize, Serialize}; + +use crate::error::{Fatal, Result}; + +/// A consumer-selected policy blob, identified by GitHub repository, immutable +/// commit and path. The loader also uses this exact tuple as rule provenance. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub(crate) struct Remote { + /// GitHub repository in `owner/name` form. + pub repo: String, + /// Full commit object ID, never a movable branch or tag. + pub rev: String, + /// Regular policy blob relative to that commit's tree. + pub path: String, +} + +impl Remote { + /// Repository, commit and path as one diagnostic source name. + pub(crate) fn label(&self) -> String { + format!("{}@{}:{}", self.repo, self.rev, self.path) + } + + fn validate(&self, policy: &Path) -> Result<()> { + let parts: Vec<_> = self.repo.split('/').collect(); + if parts.len() != 2 + || parts.iter().any(|part| { + part.is_empty() + || matches!(*part, "." | "..") + || !part + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || b"-_.".contains(&byte)) + }) + { + return Err(Fatal::at( + policy, + "`inherit.remote.repo` must name a GitHub repository as `owner/name`", + )); + } + if self.rev.len() != 40 || !self.rev.bytes().all(|byte| byte.is_ascii_hexdigit()) { + return Err(Fatal::at( + policy, + "`inherit.remote.rev` must be a full 40-character commit ID; branches and tags can move, so resolve the intended revision to its immutable commit first", + )); + } + if self.path.is_empty() + || self.path.split('/').any(|part| matches!(part, "." | "..")) + || self.path.chars().any(char::is_control) + || Path::new(&self.path) + .components() + .any(|part| !matches!(part, Component::Normal(_))) + { + return Err(Fatal::at( + policy, + "`inherit.remote.path` must be a nonempty repository-relative file path without `.` or `..` components", + )); + } + Ok(()) + } + + /// Fetch and read this exact blob, or refuse without a substitute policy. + pub(crate) fn read(&self, policy: &Path) -> Result { + self.validate(policy)?; + let directory = Repository::new()?; + let fail = || { + Fatal::at( + policy, + format!( + "could not read remote policy {} at its pinned commit; nothing was inherited", + self.label() + ), + ) + }; + crate::git::run_elsewhere(&directory.0, &["init", "--bare", "--quiet", "--template="])?; + let url = format!("https://github.com/{}.git", self.repo); + // No checkout, submodule recursion, hooks or repository-supplied config. + // Git's normal credentials and URL rewrites remain available, including + // credentials for an organization's private policy repository. + let mut fetch = crate::shim::inner_tool("git"); + fetch + .current_dir(&directory.0) + .args([ + "-c", + "fetch.recurseSubmodules=false", + "fetch", + "--quiet", + "--depth=1", + "--no-tags", + "--", + &url, + &self.rev, + ]) + // Hook and agent invocations cannot answer credential prompts. + .env("GIT_TERMINAL_PROMPT", "0") + .env("GIT_ASKPASS", "") + .env("GIT_HTTP_LOW_SPEED_LIMIT", "1") + .env("GIT_HTTP_LOW_SPEED_TIME", "30") + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()); + if !crate::git::elsewhere(&mut fetch) + .status() + .map_err(|_| fail())? + .success() + { + return Err(fail()); + } + let resolved = crate::git::try_run_elsewhere( + &directory.0, + &["rev-parse", "--verify", "FETCH_HEAD^{commit}"], + )? + .ok_or_else(fail)?; + if !resolved.trim().eq_ignore_ascii_case(&self.rev) { + return Err(fail()); + } + // The checked mode, exact literal path and content must all name the + // same tree entry. Pathspec magic must not check one file and then + // read another, and `-z` prevents filename quoting from changing it. + let entry = crate::git::try_run_elsewhere( + &directory.0, + &[ + "--literal-pathspecs", + "ls-tree", + "-z", + &self.rev, + "--", + &self.path, + ], + )? + .ok_or_else(fail)?; + let record = entry + .strip_suffix('\0') + .filter(|record| !record.contains('\0')) + .ok_or_else(fail)?; + let (metadata, path) = record.split_once('\t').ok_or_else(fail)?; + let mut fields = metadata.split(' '); + let mode = fields.next().ok_or_else(fail)?; + let kind = fields.next().ok_or_else(fail)?; + let object = fields.next().ok_or_else(fail)?; + if fields.next().is_some() + || !matches!(mode, "100644" | "100755") + || kind != "blob" + || path != self.path + || object.len() != 40 + || !object.bytes().all(|byte| byte.is_ascii_hexdigit()) + { + return Err(fail()); + } + let mut command = crate::shim::inner_tool("git"); + command + .current_dir(&directory.0) + .args(["cat-file", "blob", object]); + let output = crate::git::elsewhere(&mut command) + .output() + .map_err(|_| fail())?; + if !output.status.success() { + return Err(fail()); + } + String::from_utf8(output.stdout).map_err(|_| { + Fatal::at( + policy, + format!("remote policy {} is not UTF-8", self.label()), + ) + }) + } +} + +/// An exclusive scratch object store. It is never shared by repositories or +/// reused after a fetch failure, and is removed on both success and failure. +struct Repository(PathBuf); + +impl Repository { + fn new() -> Result { + static NEXT: AtomicUsize = AtomicUsize::new(0); + loop { + let path = std::env::temp_dir().join(format!( + "uphold-remote-{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + let mut builder = std::fs::DirBuilder::new(); + #[cfg(unix)] + { + use std::os::unix::fs::DirBuilderExt as _; + builder.mode(0o700); + } + match builder.create(&path) { + Ok(()) => return Ok(Self(path)), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(error) => return Err(Fatal::at(&path, error)), + } + } + } +} + +impl Drop for Repository { + fn drop(&mut self) { + std::fs::remove_dir_all(&self.0).ok(); + } +} diff --git a/src/supply.rs b/src/supply.rs index e74b83b..90f230b 100644 --- a/src/supply.rs +++ b/src/supply.rs @@ -44,7 +44,7 @@ //! run with `--output-format json` and its own `risks` list is counted. The //! count is reported, never recomputed; the choice is to parse it or to run it //! for nothing. One named exception: a `metadata_mismatch` risk whose every -//! difference is one npm writes at publish (`PUBLISH_NORMALIZATIONS`) is +//! difference is a known npm rewrite or a verified `PyPI` optional extra is //! printed as tolerated rather than counted. //! //! guarddog is also not handed what it cannot look up. A `uv export` spells a @@ -101,6 +101,7 @@ use std::process::Command; use crate::config::{Policy, Waiver}; use crate::error::{Exit, Fatal, Result, verdict}; +mod pypi; mod references; /// The zizmor policy run where the repository has none of its own. @@ -1248,8 +1249,8 @@ const GUARDDOG_RULES: [&str; 10] = [ "metadata_mismatch", ]; -/// A difference guarddog's npm `metadata_mismatch` reports that npm itself -/// writes at publish, tolerated rather than counted. +/// The closed list of metadata normalizations supported by the report reader. +/// `OptionalExtras` additionally requires release evidence from [`pypi`]. /// /// The rule compares the registry's manifest for a release with the /// `package.json` inside its tarball. `npm publish` normalizes the manifest it @@ -1275,20 +1276,21 @@ const GUARDDOG_RULES: [&str; 10] = [ /// and a waiver still applies to it. Not on the list: `"install": "node-gyp /// rebuild"`, which npm adds where `binding.gyp` exists; an install script runs /// at install time, which `start` does not, so it joins only with a case and a -/// test. Nor `PyPI`'s optional extras, which guarddog 3.2.0 compares with the -/// required dependencies: its report keeps the names and drops the `extra ==` -/// markers, so it cannot tell an extra from an injected dependency. +/// test. `PyPI` optional extras use separate release evidence in [`pypi`], +/// because the report itself drops the markers that identify an extra. #[derive(Clone, Copy)] enum Normalization { DefaultStart, BinPrefixStripped, + OptionalExtras, } /// Every normalization tolerated, in the order a difference is tried against /// them. -const PUBLISH_NORMALIZATIONS: [Normalization; 2] = [ +const PUBLISH_NORMALIZATIONS: [Normalization; 3] = [ Normalization::DefaultStart, Normalization::BinPrefixStripped, + Normalization::OptionalExtras, ]; impl Normalization { @@ -1297,6 +1299,7 @@ impl Normalization { match self { Self::DefaultStart => "npm default start script", Self::BinPrefixStripped => "npm strips a leading node_modules/.bin/", + Self::OptionalExtras => "PyPI optional extras verified against release requires_dist", } } @@ -1310,6 +1313,7 @@ impl Normalization { Self::BinPrefixStripped => ["./node_modules/.bin/", "node_modules/.bin/"] .iter() .any(|prefix| tarball.strip_prefix(prefix) == Some(registry)), + Self::OptionalExtras => false, } } } @@ -1353,6 +1357,36 @@ fn tolerated(description: &str) -> Option> { (!accepted.is_empty()).then_some(accepted) } +/// A second source is consulted only for the exact `PyPI` report shape it can +/// explain. Unavailable or mismatched evidence leaves the scanner's finding +/// intact, so optional normalization never makes the scan depend on a pass. +fn tolerated_metadata( + ecosystem: &str, + name: &str, + version: &str, + description: &str, + metadata: &mut impl FnMut(&str, &str) -> std::result::Result, +) -> Option> { + match ecosystem { + "npm" => tolerated(description), + "pypi" => { + let differences = pypi::differences(description)?; + match metadata(name, version) { + Ok(release) => pypi::confirms(&release, name, version, &differences) + .map(|accepted| vec![accepted]), + Err(reason) => { + println!( + " retained: metadata_mismatch on pypi:{name}@{version} -- \ + could not verify optional extras: {reason}" + ); + None + } + } + } + _ => None, + } +} + /// guarddog's floor: the "rules failed to run while scanning" text, the JSON /// report's `errors` / `results` / `risks` keys and exit 0 on a finding were /// all measured against guarddog 3.2.0. @@ -1502,7 +1536,7 @@ enum Read { /// it for nothing. `risks` is guarddog's own list of what it objected to and /// nothing is re-judged: the count is reported, not recomputed. /// -/// A risk that is only npm's own publish-time rewrites ([`tolerated`]) is +/// A risk covered by [`tolerated_metadata`] is /// printed as tolerated, naming each, and is not held against the run. /// /// A risk a waiver covers is printed as waived, with the waiver's reason, and @@ -1522,6 +1556,7 @@ fn guarddog_read( code: Option, stdout: &str, stderr: &str, + metadata: &mut impl FnMut(&str, &str) -> std::result::Result, ) -> Read { // Its own admission first, because it survives whatever the report is. if let Some(reason) = rules_that_did_not_run(stdout).or_else(|| rules_that_did_not_run(stderr)) @@ -1603,11 +1638,10 @@ fn guarddog_read( .and_then(serde_json::Value::as_str) .or_else(|| risk.get("name").and_then(serde_json::Value::as_str)) .unwrap_or(""); - // npm's own publish-time rewrites, named on the line that lets + // Known metadata normalizations, named on the line that lets // them through. The risk and the underlying result must describe // the same difference; anything else keeps the risk counted. - if ecosystem == "npm" - && rule == "metadata_mismatch" + if rule == "metadata_mismatch" && let Some(description) = risk .get("threat_description") .and_then(serde_json::Value::as_str) @@ -1615,7 +1649,8 @@ fn guarddog_read( .get("metadata_mismatch") .and_then(serde_json::Value::as_str) == Some(description) - && let Some(accepted) = tolerated(description) + && let Some(accepted) = + tolerated_metadata(ecosystem, name, version, description, metadata) { println!( " tolerated: {rule} on {ecosystem}:{name}@{version} at {at} -- {}", @@ -1817,6 +1852,7 @@ fn guarddog(root: &Path, scope: &Scope, policy: &Policy) -> Result

{ status.status.code(), &String::from_utf8_lossy(&status.stdout), &String::from_utf8_lossy(&status.stderr), + &mut pypi::fetch, ) { Read::Clean => { ledger.read.insert("pypi"); @@ -1895,6 +1931,7 @@ fn guarddog(root: &Path, scope: &Scope, policy: &Policy) -> Result
{ status.status.code(), &String::from_utf8_lossy(&status.stdout), &String::from_utf8_lossy(&status.stderr), + &mut pypi::fetch, ) { Read::Clean => { ledger.read.insert("npm"); @@ -2255,6 +2292,7 @@ mod tests { Some(0), report, "", + &mut |_, _| Err(String::from("no registry evidence in this fixture")), ); match read { Read::Risks(..) => Ok(true), @@ -2425,9 +2463,8 @@ mod tests { assert_eq!(objected("npm", &bare.to_string()), Ok(true)); } - /// The npm normalizations are npm's, and `PyPI`'s optional extras are not on - /// the list: guarddog's report drops the `extra ==` markers, so pandas is - /// still a finding, and so is an npm-shaped description in a `PyPI` report. + /// Without release evidence, the dropped `extra ==` markers cannot be + /// recovered from the report. An npm-shaped `PyPI` report also stays a finding. #[test] fn pypi_metadata_mismatch_is_not_tolerated() { assert_eq!(objected("pypi", PANDAS), Ok(true)); @@ -2435,6 +2472,86 @@ mod tests { assert_eq!(objected("pypi", &npm_shaped), Ok(true)); } + #[test] + fn pypi_extras_need_release_evidence_and_agreeing_reports() { + let mut report: serde_json::Value = serde_json::from_str(PANDAS).unwrap(); + let description = "Dependency mismatch between PyPI metadata and pyproject.toml:\n In PyPI metadata but NOT in pyproject.toml: pyarrow\n"; + report[0]["result"]["results"]["metadata_mismatch"] = description.into(); + report[0]["result"]["risks"][0]["threat_description"] = description.into(); + for (requirements, clean) in [ + (vec!["pyarrow>=13; extra == 'parquet'"], true), + (vec!["pyarrow>=13"], false), + (vec!["pyarrow; extra == 'parquet'", "pyarrow"], false), + ] { + let read = guarddog_read( + "project", + "pypi", + &BTreeSet::from([String::from("pandas")]), + &[], + &mut Ledger::default(), + Some(0), + &report.to_string(), + "", + &mut |name, version| { + assert_eq!((name, version), ("pandas", "3.0.3")); + Ok(serde_json::json!({"info": { + "name": name, "version": version, "requires_dist": requirements, + }})) + }, + ); + assert_eq!(matches!(read, Read::Clean), clean); + } + report[0]["result"]["results"]["metadata_mismatch"] = "different evidence".into(); + let mut fetched = false; + let read = guarddog_read( + "project", + "pypi", + &BTreeSet::new(), + &[], + &mut Ledger::default(), + Some(0), + &report.to_string(), + "", + &mut |_, _| { + fetched = true; + Err(String::from("unexpected lookup")) + }, + ); + assert!(matches!(read, Read::Risks(..))); + assert!(!fetched); + } + + #[test] + fn verified_pypi_extras_leave_unrelated_risks_counted() { + let mut report: serde_json::Value = serde_json::from_str(PANDAS).unwrap(); + for other_risk in [false, true] { + if other_risk { + report[0]["result"]["risks"] + .as_array_mut() + .unwrap() + .push(serde_json::json!({"threat_rule": "typosquatting"})); + } + let read = guarddog_read( + "project", + "pypi", + &BTreeSet::from([String::from("pandas")]), + &[], + &mut Ledger::default(), + Some(0), + &report.to_string(), + "", + &mut |_, _| { + Ok(serde_json::from_str(include_str!( + "../tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json" + )) + .unwrap()) + }, + ); + assert_eq!(matches!(read, Read::Risks(..)), other_risk); + assert_eq!(matches!(read, Read::Clean), !other_risk); + } + } + /// guarddog unpacks under the cache directory, whatever `TMPDIR` says: the /// order its `metadata_mismatch` rule lists an unpacked wheel in is that /// filesystem's, so `TMPDIR` would otherwise be the verdict's input. diff --git a/src/supply/pypi.rs b/src/supply/pypi.rs new file mode 100644 index 0000000..e76668b --- /dev/null +++ b/src/supply/pypi.rs @@ -0,0 +1,293 @@ +//! Evidence for guarddog's comparison of optional extras with required dependencies. +//! +//! The report drops markers. Only the exact release's `requires_dist` can +//! establish that every occurrence of a reported name requires an extra. + +use std::collections::{BTreeMap, BTreeSet}; +use std::time::Duration; + +use pyreq::Requirement; +use serde_json::Value; + +/// The release endpoint, rather than the project endpoint, binds the evidence +/// to what guarddog scanned. `PyPI` documents it at . +fn release_url(name: &str, version: &str) -> Option { + if name.is_empty() { + return None; + } + let name = package_name(name)?; + // Versions are a single URL segment, never a caller-selected destination. + if !version.bytes().any(|byte| byte.is_ascii_digit()) + || !version + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || b".!+_-".contains(&byte)) + { + return None; + } + Some(format!("https://pypi.org/pypi/{name}/{version}/json")) +} + +/// Fetch bounded, exact-release registry evidence. Failure leaves the scanner's +/// risk available to the ordinary finding and waiver handling. +pub(super) fn fetch(name: &str, version: &str) -> Result { + let url = release_url(name, version).ok_or("invalid release identity")?; + let agent: ureq::Agent = ureq::Agent::config_builder() + .https_only(true) + .max_redirects(0) + .timeout_global(Some(Duration::from_secs(15))) + .build() + .into(); + read_response(agent.get(&url).call().map_err(|error| error.to_string())?) +} + +/// Redirect bodies and partial responses cannot supply release evidence even +/// when their JSON has the expected fields. +fn read_response(mut response: ureq::http::Response) -> Result { + if response.status() != 200 { + return Err(format!("unexpected PyPI HTTP status {}", response.status())); + } + let body = response + .body_mut() + .with_config() + .limit(2 * 1024 * 1024) + .read_to_string() + .map_err(|error| error.to_string())?; + serde_json::from_str(&body).map_err(|error| error.to_string()) +} + +/// Only the one-sided, names-only report has a meaning this evidence proves. +/// Unknown blocks, malformed names and duplicate normalized names stay findings. +pub(super) fn differences(description: &str) -> Option> { + let mut lines = description.lines(); + if lines.next()? != "Dependency mismatch between PyPI metadata and pyproject.toml:" { + return None; + } + let names = lines + .next()? + .strip_prefix(" In PyPI metadata but NOT in pyproject.toml: ")?; + if lines.next().is_some() { + return None; + } + let mut parsed = BTreeSet::new(); + for name in names.split(", ") { + if name.is_empty() || !parsed.insert(package_name(name)?) { + return None; + } + } + (!parsed.is_empty()).then_some(parsed) +} + +/// Names are parsed by the requirement reader, then constrained to the bare +/// name form so URL segments and report entries cannot carry extra syntax. +fn package_name(name: &str) -> Option { + let requirement = Requirement::parse(name).ok()?; + (requirement.name == name).then(|| super::references::normalise(name)) +} + +/// Confirm every reported name occurs only under a recognized optional-extra +/// marker in the named release. Any other occurrence prevents tolerance. +pub(super) fn confirms( + metadata: &Value, + name: &str, + version: &str, + differences: &BTreeSet, +) -> Option { + let info = metadata.get("info")?; + if package_name(info.get("name")?.as_str()?)? != package_name(name)? + || info.get("version")?.as_str()? != version + { + return None; + } + // The closed normalization recognizes a simple positive equality. More + // expressive markers stay findings; no host environment is guessed. + let extra_marker = regex::Regex::new( + r#"^(?:extra == "[A-Za-z0-9][A-Za-z0-9._-]*"|"[A-Za-z0-9][A-Za-z0-9._-]*" == extra)$"#, + ) + .ok()?; + let mut optional = BTreeMap::new(); + for raw in info.get("requires_dist")?.as_array()? { + let requirement = Requirement::parse(raw.as_str()?).ok()?; + let extra_only = requirement + .marker + .as_ref() + .is_some_and(|marker| extra_marker.is_match(&marker.to_string())); + optional + .entry(package_name(&requirement.name)?) + .and_modify(|all| *all &= extra_only) + .or_insert(extra_only); + } + if !differences + .iter() + .all(|dependency| optional.get(dependency) == Some(&true)) + { + return None; + } + Some(format!( + "{} ({})", + super::Normalization::OptionalExtras.name(), + differences + .iter() + .map(ToString::to_string) + .collect::>() + .join(", ") + )) +} + +#[cfg(test)] +mod tests { + use super::{confirms, differences, read_response, release_url}; + use serde_json::json; + use std::io::Write as _; + + const DESCRIPTION: &str = "Dependency mismatch between PyPI metadata and pyproject.toml:\n In PyPI metadata but NOT in pyproject.toml: pyarrow\n"; + + /// Exercise the production response reader over a local HTTP connection; + /// production URL selection always uses HTTPS at the fixed registry host. + fn response(status: u16, body: &str) -> Result { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + let body = body.to_owned(); + let server = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + write!(stream, "HTTP/1.1 {status} Fixture\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()).ok(); + }); + let agent: ureq::Agent = ureq::Agent::config_builder() + .max_redirects(0) + .http_status_as_error(false) + .timeout_global(Some(std::time::Duration::from_secs(5))) + .build() + .into(); + let result = read_response(agent.get(format!("http://{address}/")).call().unwrap()); + server.join().unwrap(); + result + } + + #[test] + fn only_a_complete_bounded_json_response_can_be_evidence() { + assert_eq!(response(200, "{}"), Ok(json!({}))); + for status in [206, 301, 302, 307, 404, 500] { + assert!(response(status, "{}").is_err(), "{status}"); + } + assert!(response(200, "not json").is_err()); + assert!(response(200, &" ".repeat(2 * 1024 * 1024 + 1)).is_err()); + } + + #[test] + fn the_recorded_pandas_report_is_explained_by_its_release_metadata() { + let report: serde_json::Value = serde_json::from_str(include_str!( + "../../tests/fixtures/guarddog/pypi-pandas-3.0.3.json" + )) + .unwrap(); + let metadata: serde_json::Value = serde_json::from_str(include_str!( + "../../tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json" + )) + .unwrap(); + let names = differences( + report[0]["result"]["results"]["metadata_mismatch"] + .as_str() + .unwrap(), + ) + .unwrap(); + assert!(confirms(&metadata, "pandas", "3.0.3", &names).is_some()); + } + + #[test] + fn every_occurrence_must_require_a_positive_extra() { + let names = differences(DESCRIPTION).unwrap(); + for (requirements, accepted) in [ + (vec!["pyarrow>=13; extra == 'parquet'"], true), + ( + vec!["pyarrow; python_version >= '3.10' and extra == 'parquet'"], + false, + ), + (vec!["pyarrow; 'parquet' == extra"], true), + ( + vec!["pyarrow; extra == 'parquet'", "pyarrow; extra == 'test'"], + true, + ), + ( + vec![ + "pyarrow; extra == 'parquet'", + "pyarrow; sys_platform == 'win32'", + ], + false, + ), + ( + vec!["pyarrow; extra == 'parquet' or python_version < '3.10'"], + false, + ), + (vec!["pyarrow; extra != 'parquet'"], false), + (vec!["pyarrow"], false), + (vec!["another; extra == 'parquet'"], false), + (vec!["pyarrow; extra =="], false), + (vec![], false), + ] { + let metadata = json!({"info": {"name": "example", "version": "1.0", "requires_dist": requirements}}); + assert_eq!( + confirms(&metadata, "example", "1.0", &names).is_some(), + accepted, + "{metadata}" + ); + } + } + + #[test] + fn evidence_must_name_the_scanned_release_and_every_difference() { + let names = differences(DESCRIPTION).unwrap(); + for metadata in [ + json!({"info": {"name": "different", "version": "1.0", "requires_dist": ["pyarrow; extra == 'x'"]}}), + json!({"info": {"name": "example", "version": "2.0", "requires_dist": ["pyarrow; extra == 'x'"]}}), + json!({"info": {"name": "example", "version": "1.0", "requires_dist": null}}), + json!({"info": {"name": "example", "version": "1.0", "requires_dist": [42]}}), + json!({}), + ] { + assert!(confirms(&metadata, "example", "1.0", &names).is_none()); + } + let two = differences(&DESCRIPTION.replace("pyarrow\n", "pyarrow, injected\n")).unwrap(); + let metadata = json!({"info": {"name": "Example_Kit", "version": "1.0", "requires_dist": ["PyArrow; extra == 'x'"]}}); + assert!(confirms(&metadata, "example-kit", "1.0", &names).is_some()); + assert!(confirms(&metadata, "example-kit", "1.0", &two).is_none()); + } + + #[test] + fn unknown_or_ambiguous_report_shapes_are_not_candidates() { + for description in [ + DESCRIPTION.replace("pyarrow", ""), + DESCRIPTION.replace("pyarrow", "pyarrow, PyArrow"), + DESCRIPTION.replace("pyarrow", "pyarrow>=13"), + DESCRIPTION.replace( + "pyarrow", + "pyarrow, injected\n In pyproject.toml but NOT in PyPI metadata: removed", + ), + format!("{DESCRIPTION}unrecognized difference\n"), + format!("{DESCRIPTION}\n"), + ] { + assert!(differences(&description).is_none(), "{description}"); + } + } + + #[test] + fn release_identity_cannot_change_the_destination() { + assert_eq!( + release_url("Example_Kit", "1.0+local"), + Some(String::from( + "https://pypi.org/pypi/example-kit/1.0+local/json" + )) + ); + for name in ["../evil", "https://elsewhere", "x?y", "x#y", ""] { + assert!(release_url(name, "1.0").is_none()); + } + for version in [ + "../evil", + "1.0/anything", + "1.0?x", + "1.0#x", + "%2f", + ".", + "..", + "", + ] { + assert!(release_url("example", version).is_none()); + } + } +} diff --git a/tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json b/tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json new file mode 100644 index 0000000..5eb1a96 --- /dev/null +++ b/tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json @@ -0,0 +1,92 @@ +{ + "info": { + "name": "pandas", + "version": "3.0.3", + "requires_dist": [ + "numpy>=1.26.0; python_version < \"3.14\"", + "numpy>=2.3.3; python_version >= \"3.14\"", + "python-dateutil>=2.8.2", + "tzdata; sys_platform == \"win32\"", + "tzdata; sys_platform == \"emscripten\"", + "hypothesis>=6.116.0; extra == \"test\"", + "pytest>=8.3.4; extra == \"test\"", + "pytest-xdist>=3.6.1; extra == \"test\"", + "pyarrow>=13.0.0; extra == \"pyarrow\"", + "bottleneck>=1.4.2; extra == \"performance\"", + "numba>=0.60.0; extra == \"performance\"", + "numexpr>=2.10.2; extra == \"performance\"", + "scipy>=1.14.1; extra == \"computation\"", + "xarray>=2024.10.0; extra == \"computation\"", + "fsspec>=2024.10.0; extra == \"fss\"", + "s3fs>=2024.10.0; extra == \"aws\"", + "gcsfs>=2024.10.0; extra == \"gcp\"", + "odfpy>=1.4.1; extra == \"excel\"", + "openpyxl>=3.1.5; extra == \"excel\"", + "python-calamine>=0.3.0; extra == \"excel\"", + "pyxlsb>=1.0.10; extra == \"excel\"", + "xlrd>=2.0.1; extra == \"excel\"", + "xlsxwriter>=3.2.0; extra == \"excel\"", + "pyarrow>=13.0.0; extra == \"parquet\"", + "pyarrow>=13.0.0; extra == \"feather\"", + "pyiceberg>=0.8.1; extra == \"iceberg\"", + "tables>=3.10.1; extra == \"hdf5\"", + "pyreadstat>=1.2.8; extra == \"spss\"", + "SQLAlchemy>=2.0.36; extra == \"postgresql\"", + "psycopg2>=2.9.10; extra == \"postgresql\"", + "adbc-driver-postgresql>=1.2.0; extra == \"postgresql\"", + "SQLAlchemy>=2.0.36; extra == \"mysql\"", + "pymysql>=1.1.1; extra == \"mysql\"", + "SQLAlchemy>=2.0.36; extra == \"sql-other\"", + "adbc-driver-postgresql>=1.2.0; extra == \"sql-other\"", + "adbc-driver-sqlite>=1.2.0; extra == \"sql-other\"", + "beautifulsoup4>=4.12.3; extra == \"html\"", + "html5lib>=1.1; extra == \"html\"", + "lxml>=5.3.0; extra == \"html\"", + "lxml>=5.3.0; extra == \"xml\"", + "matplotlib>=3.9.3; extra == \"plot\"", + "jinja2>=3.1.5; extra == \"output-formatting\"", + "tabulate>=0.9.0; extra == \"output-formatting\"", + "PyQt5>=5.15.9; extra == \"clipboard\"", + "qtpy>=2.4.2; extra == \"clipboard\"", + "zstandard>=0.23.0; extra == \"compression\"", + "pytz>=2020.1; extra == \"timezone\"", + "adbc-driver-postgresql>=1.2.0; extra == \"all\"", + "adbc-driver-sqlite>=1.2.0; extra == \"all\"", + "beautifulsoup4>=4.12.3; extra == \"all\"", + "bottleneck>=1.4.2; extra == \"all\"", + "fastparquet>=2024.11.0; extra == \"all\"", + "fsspec>=2024.10.0; extra == \"all\"", + "gcsfs>=2024.10.0; extra == \"all\"", + "html5lib>=1.1; extra == \"all\"", + "hypothesis>=6.116.0; extra == \"all\"", + "jinja2>=3.1.5; extra == \"all\"", + "lxml>=5.3.0; extra == \"all\"", + "matplotlib>=3.9.3; extra == \"all\"", + "numba>=0.60.0; extra == \"all\"", + "numexpr>=2.10.2; extra == \"all\"", + "odfpy>=1.4.1; extra == \"all\"", + "openpyxl>=3.1.5; extra == \"all\"", + "psycopg2>=2.9.10; extra == \"all\"", + "pyarrow>=13.0.0; extra == \"all\"", + "pyiceberg>=0.8.1; extra == \"all\"", + "pymysql>=1.1.1; extra == \"all\"", + "PyQt5>=5.15.9; extra == \"all\"", + "pyreadstat>=1.2.8; extra == \"all\"", + "pytest>=8.3.4; extra == \"all\"", + "pytest-xdist>=3.6.1; extra == \"all\"", + "python-calamine>=0.3.0; extra == \"all\"", + "pytz>=2020.1; extra == \"all\"", + "pyxlsb>=1.0.10; extra == \"all\"", + "qtpy>=2.4.2; extra == \"all\"", + "scipy>=1.14.1; extra == \"all\"", + "s3fs>=2024.10.0; extra == \"all\"", + "SQLAlchemy>=2.0.36; extra == \"all\"", + "tables>=3.10.1; extra == \"all\"", + "tabulate>=0.9.0; extra == \"all\"", + "xarray>=2024.10.0; extra == \"all\"", + "xlrd>=2.0.1; extra == \"all\"", + "xlsxwriter>=3.2.0; extra == \"all\"", + "zstandard>=0.23.0; extra == \"all\"" + ] + } +} diff --git a/tests/remote_policy_cli.rs b/tests/remote_policy_cli.rs new file mode 100644 index 0000000..fb742de --- /dev/null +++ b/tests/remote_policy_cli.rs @@ -0,0 +1,398 @@ +//! Exercise the normal loader against real Git blobs, with network URLs +//! rewritten per child process to an isolated local policy repository. +#![expect( + clippy::let_underscore_must_use, + clippy::indexing_slicing, + clippy::tests_outside_test_module, + clippy::unwrap_used, + reason = "CLI fixtures report failures by assertions" +)] +mod support; + +use std::path::PathBuf; +use std::process::{Command, Output}; + +struct Fixture { + root: PathBuf, + source: PathBuf, + config: PathBuf, + rev: String, +} + +impl Fixture { + fn new(text: &str) -> Self { + let root = support::scratch("remote-policy"); + let source = root.join("source"); + std::fs::create_dir_all(source.join("policy")).unwrap(); + support::git(&source, &["init", "-q", "-b", "main"]); + support::git(&source, &["config", "user.name", "Test"]); + support::git(&source, &["config", "user.email", "test@example.test"]); + std::fs::write(source.join("policy/org.toml"), text).unwrap(); + support::git(&source, &["add", "."]); + support::git(&source, &["commit", "-qm", "policy"]); + let answer = support::git_command(&source) + .args(["rev-parse", "HEAD"]) + .output() + .unwrap(); + let rev = String::from_utf8(answer.stdout).unwrap().trim().to_owned(); + let config = root.join("gitconfig"); + std::fs::write( + &config, + format!( + "[url \"file://{}\"]\n insteadOf = https://github.com/example/policy.git\n", + source.display() + ), + ) + .unwrap(); + std::fs::create_dir_all(root.join("consumer/policy")).unwrap(); + support::git(&root.join("consumer"), &["init", "-q", "-b", "main"]); + let fixture = Self { + root, + source, + config, + rev, + }; + fixture.policy(""); + fixture + } + + fn repin(&mut self) { + support::git(&self.source, &["add", "."]); + support::git(&self.source, &["commit", "-qm", "update"]); + let output = support::git_command(&self.source) + .args(["rev-parse", "HEAD"]) + .output() + .unwrap(); + String::from_utf8(output.stdout) + .unwrap() + .trim() + .clone_into(&mut self.rev); + self.policy(""); + } + + fn policy(&self, suffix: &str) { + self.with_remote("example/policy", &self.rev, "policy/org.toml", suffix); + } + + fn with_remote(&self, repo: &str, rev: &str, path: &str, suffix: &str) { + std::fs::write(self.root.join("consumer/policy/principles.toml"), format!( + "[inherit]\nremote = [{{ repo = {repo:?}, rev = {rev:?}, path = {path:?} }}]\n{suffix}" + )).unwrap(); + } + + fn run(&self, args: &[&str]) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_uphold")); + command + .args(args) + .current_dir(self.root.join("consumer")) + .env("GIT_CONFIG_GLOBAL", &self.config) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_TERMINAL_PROMPT", "0"); + support::without_git_environment(&mut command); + // Every load also exercises the environment exported to a Git hook. + command.env("GIT_DIR", self.root.join("consumer/.git")); + command.output().unwrap() + } +} + +const RULE: &str = "[rule.org-rule]\nregexp = 'ORG_BANNED'\nmessage = 'organization rule'\n[rule.org-rule.files]\nexclude = ['policy/**']\n"; + +#[cfg(unix)] +#[test] +fn an_authentication_failure_never_launches_an_interactive_prompt() { + use std::io::{BufRead as _, Write as _}; + use std::os::unix::fs::PermissionsExt as _; + + let fixture = Fixture::new(RULE); + let askpass = fixture.root.join("askpass"); + std::fs::write( + &askpass, + "#!/bin/sh\nprintf asked > \"$0.called\"\nexit 1\n", + ) + .unwrap(); + std::fs::set_permissions(&askpass, std::fs::Permissions::from_mode(0o755)).unwrap(); + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + std::fs::write(&fixture.config, format!( + "[url \"http://{address}/policy.git\"]\n insteadOf = https://github.com/example/policy.git\n" + )).unwrap(); + support::git( + &fixture.source, + &[ + "config", + "--file", + fixture.config.to_str().unwrap(), + "core.askPass", + askpass.to_str().unwrap(), + ], + ); + let server = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + stream + .set_read_timeout(Some(std::time::Duration::from_secs(5))) + .unwrap(); + let mut reader = std::io::BufReader::new(&stream); + let mut line = String::new(); + while reader.read_line(&mut line).unwrap() != 0 { + if line == "\r\n" { + break; + } + line.clear(); + } + stream.write_all(b"HTTP/1.1 401 Unauthorized\r\nWWW-Authenticate: Basic realm=\"fixture\"\r\nContent-Length: 0\r\nConnection: close\r\n\r\n").unwrap(); + }); + let output = fixture.run(&["rules", "--effective"]); + server.join().unwrap(); + assert_eq!( + output.status.code(), + Some(2), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(!fixture.root.join("askpass.called").exists()); +} + +#[test] +fn remote_rules_run_keep_provenance_and_accept_overrides() { + let fixture = Fixture::new(RULE); + fixture.policy("[override.org-rule]\nmessage = 'local wording'\n[override.org-rule.files]\nexclude = ['policy/**', 'skip.txt']\n"); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!( + output.status.code(), + Some(0), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let rules: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(rules[0]["remote"]["rev"], fixture.rev); + assert_eq!(rules[0]["remote"]["repo"], "example/policy"); + assert_eq!(rules[0]["remote"]["path"], "policy/org.toml"); + assert!( + rules[0]["overridden"] + .as_array() + .unwrap() + .contains(&serde_json::json!("message")) + ); + let human = fixture.run(&["rules", "--effective"]); + assert_eq!(human.status.code(), Some(0)); + let listing = String::from_utf8(human.stdout).unwrap(); + let lines: Vec<_> = listing.lines().collect(); + let rule_line = lines + .iter() + .position(|line| line.starts_with(" org-rule (")) + .unwrap(); + assert_eq!( + lines.get(rule_line + 1).copied(), + Some(format!(" source: example/policy@{}:policy/org.toml", fixture.rev).as_str()) + ); + std::fs::write(fixture.root.join("consumer/skip.txt"), "ORG_BANNED").unwrap(); + support::git(&fixture.root.join("consumer"), &["add", "."]); + let clean = fixture.run(&["scan"]); + assert_eq!( + clean.status.code(), + Some(0), + "{}", + String::from_utf8_lossy(&clean.stderr) + ); + std::fs::write(fixture.root.join("consumer/hit.txt"), "ORG_BANNED").unwrap(); + support::git(&fixture.root.join("consumer"), &["add", "."]); + let refused = fixture.run(&["scan"]); + assert_eq!( + refused.status.code(), + Some(1), + "{}", + String::from_utf8_lossy(&refused.stderr) + ); + assert!( + format!( + "{}{}", + String::from_utf8_lossy(&refused.stdout), + String::from_utf8_lossy(&refused.stderr) + ) + .contains("local wording"), + "{refused:?}" + ); +} + +#[test] +fn disabled_and_own_rules_use_the_same_remote_snapshot() { + let fixture = Fixture::new(RULE); + fixture.policy("disabled_rules = ['org-rule']\n"); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!( + output.status.code(), + Some(0), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + serde_json::from_slice::(&output.stdout).unwrap(), + serde_json::json!([]) + ); + fixture.policy( + "[rule.org-rule]\nregexp = 'LOCAL_BANNED'\nmessage = 'own rule'\nfiles.include = ['.']\nfiles.exclude = ['policy/**']\n", + ); + let own_output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!( + own_output.status.code(), + Some(0), + "{}", + String::from_utf8_lossy(&own_output.stderr) + ); + let rules: serde_json::Value = serde_json::from_slice(&own_output.stdout).unwrap(); + assert!(rules[0].get("remote").is_none()); +} + +#[test] +fn a_changed_source_cannot_change_the_pinned_policy() { + let fixture = Fixture::new(RULE); + std::fs::write( + fixture.source.join("policy/org.toml"), + "[rule.replacement]\nregexp = 'NEW'\nmessage = 'new rule'\n", + ) + .unwrap(); + support::git(&fixture.source, &["add", "."]); + support::git(&fixture.source, &["commit", "-qm", "change"]); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!( + output.status.code(), + Some(0), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let rules: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(rules[0]["id"], "org-rule"); +} + +#[test] +fn missing_commits_files_and_unreachable_repositories_fail_closed() { + let fixture = Fixture::new(RULE); + for (rev, path) in [ + ( + "1111111111111111111111111111111111111111", + "policy/org.toml", + ), + (fixture.rev.as_str(), "policy/missing.toml"), + ] { + fixture.with_remote("example/policy", rev, path, ""); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!(output.status.code(), Some(2)); + assert_eq!(output.stdout, Vec::::new()); + assert!(String::from_utf8_lossy(&output.stderr).contains("pinned commit")); + } + fixture.policy(""); + std::fs::remove_dir_all(&fixture.source).unwrap(); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!(output.status.code(), Some(2)); + assert_eq!(output.stdout, Vec::::new()); +} + +#[test] +fn mutable_pins_and_unsafe_paths_are_refused_before_network_access() { + let fixture = Fixture::new(RULE); + for (repo, rev, path, field) in [ + ("example/policy", "main", "policy/org.toml", "rev"), + ("example/policy", "v1", "policy/org.toml", "rev"), + ( + "https://example.test/policy", + fixture.rev.as_str(), + "policy/org.toml", + "repo", + ), + ( + "example/policy", + fixture.rev.as_str(), + "../org.toml", + "path", + ), + ("example/policy", fixture.rev.as_str(), "/org.toml", "path"), + ] { + fixture.with_remote(repo, rev, path, ""); + let output = fixture.run(&["rules", "--effective"]); + assert_eq!(output.status.code(), Some(2)); + assert!( + String::from_utf8_lossy(&output.stderr).contains(&format!("inherit.remote.{field}")) + ); + } +} + +#[test] +fn inherited_shims_and_transitive_inheritance_are_explicit_errors() { + for (text, evidence) in [ + ( + "[[shim]]\ncommand = 'gh'\nmatch = ['api:*']\n", + "shims are not adopted", + ), + ( + "[inherit]\nsets = ['process-residue']\n", + "may not declare `[inherit]`", + ), + ] { + let fixture = Fixture::new(text); + let output = fixture.run(&["rules", "--effective"]); + assert_eq!(output.status.code(), Some(2)); + assert!( + String::from_utf8_lossy(&output.stderr).contains(evidence), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } +} + +#[test] +fn invalid_remote_content_is_never_a_partial_policy() { + for bytes in [b"[rule.broken".as_slice(), b"\xff".as_slice()] { + let mut fixture = Fixture::new(RULE); + std::fs::write(fixture.source.join("policy/org.toml"), bytes).unwrap(); + fixture.repin(); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!(output.status.code(), Some(2)); + assert_eq!(output.stdout, Vec::::new()); + } +} + +#[cfg(unix)] +#[test] +fn a_remote_symlink_is_not_a_policy_blob() { + let mut fixture = Fixture::new(RULE); + std::os::unix::fs::symlink("org.toml", fixture.source.join("policy/link.toml")).unwrap(); + fixture.repin(); + fixture.with_remote("example/policy", &fixture.rev, "policy/link.toml", ""); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!(output.status.code(), Some(2)); + assert_eq!(output.stdout, Vec::::new()); +} + +#[test] +fn a_magic_looking_remote_filename_is_read_literally() { + let mut fixture = Fixture::new(RULE); + std::fs::write(fixture.source.join("abc.toml"), RULE).unwrap(); + let literal = RULE.replace("org-rule", "literal-rule"); + std::fs::write(fixture.source.join(":(literal)abc.toml"), literal).unwrap(); + fixture.repin(); + fixture.with_remote("example/policy", &fixture.rev, ":(literal)abc.toml", ""); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!( + output.status.code(), + Some(0), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let rules: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(rules[0]["id"], "literal-rule"); +} + +#[cfg(unix)] +#[test] +fn pathspec_magic_cannot_use_a_regular_files_mode_to_read_a_symlink() { + let mut fixture = Fixture::new(RULE); + // Without literal pathspecs, ls-tree checks abc.toml's regular-file mode + // while rev:path reads this symlink's target bytes as valid TOML. + std::fs::write(fixture.source.join("abc.toml"), RULE).unwrap(); + std::os::unix::fs::symlink(RULE, fixture.source.join(":(literal)abc.toml")).unwrap(); + fixture.repin(); + fixture.with_remote("example/policy", &fixture.rev, ":(literal)abc.toml", ""); + let output = fixture.run(&["rules", "--effective", "--json"]); + assert_eq!(output.status.code(), Some(2)); + assert_eq!(output.stdout, Vec::::new()); +}