Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ tree-sitter-go = "0.25"
# disagree, `expand_alias` says which case and how it is closed.
shell-words = "1"

[target.'cfg(unix)'.dependencies]
# Git discovers hooks with access(X_OK), which checks the caller's permission
# class and ACLs. Use the same OS check through a safe API.
rustix = { version = "1", features = ["fs"] }

# `strip` is deliberately absent: stripping is a packaging step, and a stripped
# binary with no build-id cannot be symbolicated from a core. `debug = 1` keeps
# the line tables so a crash names the line it happened on.
Expand Down
28 changes: 26 additions & 2 deletions docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2179,11 +2179,21 @@ be in scope, for a question that is none of the other three (the doubly nested
form is the enum's own spelling). `collect = "git-refs"` replaces the argv walk
for `git`, whose published text is positional.

For `gh`, an explicit repository flag takes precedence over `GH_REPO`, which
takes precedence over the current origin. An environment-selected destination
is checked for ownership and visibility as its own repository. Host-qualified
GitHub destinations bind visibility and ownership probes to that host;
unsupported enterprise hosts are refused. An explicit host takes precedence
over `GH_HOST`. Repository placeholders in API endpoints can still be expanded
by `gh` for visibility and text checks, but an ownership rule requires a
concrete destination and refuses unresolved placeholders.

**`public-target` asks this repository before it asks anybody else.** Where the
policy declares a top-level `visibility`, it settles the question offline:
`public` is in scope, `private` and `internal` are out of it, and nothing is
spawned. A declaration is about one repository, so it is read only where no
`target_flags` value is present and the destination came from `origin`. A named
`target_flags` value is present and the destination came from `origin`; for
`gh`, neither `GH_REPO` nor `GH_HOST` may override the local context. A named
destination always goes to the forge, including `gh -R this-owner/this-repo`,
because the name carries no host and `--hostname` or `GH_HOST` can make
`owner/repo` a repository on a different forge. A tree whose `origin` cannot be
Expand Down Expand Up @@ -2792,6 +2802,9 @@ characters, so it is compared as a string and matches no tool.
**Which calls arrive is the harness's decision, not this binary's.** Re-deciding
it here would be a second matcher free to disagree with the first, and an
operator would have to hold both. What arrives is checked.
Tool-name rules also run when arguments contain only numbers, booleans, nulls,
or empty containers. Such calls have no argument-text subject; an explicitly
supplied empty string still has one.

### What runs where there is no policy

Expand Down Expand Up @@ -3244,6 +3257,10 @@ A report also needs a `results` object and a `risks` array; missing or mistyped
fields are unread, never an empty risk list. npm tolerance requires the
underlying `results.metadata_mismatch` and the risk's `threat_description` to
agree exactly before their differences are accepted.
Each submitted registry dependency must appear in the report: npm aliases are
resolved to their package names and PyPI names are normalized. Missing names
are reported as unread even when other entries are clean. A reported risk
still refuses the run, with any missing coverage printed alongside it.
Its older admission, "Some rules failed to run while scanning \<package\>" at
exit 0 — the two email-domain rules time out routinely — is still read, and
still names the packages and how many rules. None of that is a finding: it is
Expand Down Expand Up @@ -3582,7 +3599,11 @@ refused, never repointed; and a `default_install_hook_types` naming a hook
type outside the four is refused, because that type would otherwise stop
firing while the install read as one that worked. lefthook is refused by name:
it installs and owns its own hooks, and a `core.hooksPath` written here would
displace them.
displace them. When no `core.hooksPath` is set, installation also checks the
default hooks directory Git resolves, including the shared directory of linked
worktrees. Existing executable hooks there are named and refused before any
files or configuration change; migrate their behavior before retrying.
Executable `.sample` files and non-executable files are inert.

**`--adopt`** is the one exception to "never replaced", and it replaces only
the comments. A file in the directory without this command's marker is
Expand All @@ -3607,6 +3628,9 @@ command and **not** matching (an older install, or edited since — rerunning
and different — with the diff wherever the lines differ — and whether
`core.hooksPath` actually points git at the directory. Exit `0` only when all
four match and git runs them; `1` otherwise.
On Unix, executable access is checked for the invoking user, including the
applicable permission class and ACLs. An execute bit for another class alone
does not establish that Git can run the hook.

## `uphold probe` — can each hook refuse?

Expand Down
13 changes: 12 additions & 1 deletion src/guard/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ pub(crate) fn target_refusal(
rule: &Rule,
target: Option<&str>,
forge: Option<crate::shim::Forge>,
hostname: Option<&str>,
asked: &mut BTreeMap<String, push::Owned>,
) -> Result<Option<Refusal>> {
let Some(builtin) = rule.builtin() else {
Expand All @@ -328,6 +329,16 @@ pub(crate) fn target_refusal(
// `git::owner_repo` is the one parser, so `--repo owner/name`, a full forge
// url and the remote all become the same pair. A value it cannot split is
// not a destination this guard can judge.
if let Some(named) = target
&& crate::shim::repository_placeholder("gh", named)?
{
return Err(Fatal::new(format!(
"rule {:?}: this API destination contains repository placeholders whose effective \
identity could not be established for ownership checks. Nothing was published; \
name the destination repository directly in the API endpoint",
rule.id
)));
}
let Some((owner, repo)) = target.and_then(crate::git::owner_repo) else {
return Err(Fatal::new(format!(
"rule {:?}: this invocation publishes to a repository and nothing here says \
Expand All @@ -344,7 +355,7 @@ pub(crate) fn target_refusal(
root,
policy,
rule,
(&owner, &repo),
(&owner, &repo, hostname),
forge,
asked,
push::Seam::Command,
Expand Down
47 changes: 28 additions & 19 deletions src/guard/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,23 +151,35 @@ pub(crate) enum Owned {
///
/// Spawned through [`crate::shim::inner_tool`] and never `Command::new`: the
/// `gh` on PATH is this binary, and an unmarked probe re-enters the shim it is
/// standing behind.
pub(crate) fn forge_owns(cache: &mut BTreeMap<String, Owned>, owner: &str, repo: &str) -> Owned {
/// standing behind. `hostname`, where known, binds both probes and the cache
/// entry to that server rather than gh's ambient default host.
pub(crate) fn forge_owns(
cache: &mut BTreeMap<String, Owned>,
owner: &str,
repo: &str,
hostname: Option<&str>,
) -> Owned {
let key = format!("{owner}/{repo}");
if let Some(known) = cache.get(&key) {
let cache_key = hostname.map_or_else(|| key.clone(), |host| format!("{host}/{key}"));
if let Some(known) = cache.get(&cache_key) {
return known.clone();
}
let answer = ask_forge(owner, &key);
cache.insert(key, answer.clone());
let answer = ask_forge(owner, &key, hostname);
cache.insert(cache_key, answer.clone());
answer
}

/// The two requests, without the cache in front of them.
fn ask_forge(owner: &str, key: &str) -> Owned {
let login = match crate::shim::inner_tool("gh")
.args(["api", "user", "--jq", ".login"])
.output()
{
fn ask_forge(owner: &str, key: &str, hostname: Option<&str>) -> Owned {
let probe = |endpoint: &str, field: &str| {
let mut command = crate::shim::inner_tool("gh");
command.args(["api", endpoint, "--jq", field]);
if let Some(host) = hostname {
command.args(["--hostname", host]);
}
command.output()
};
let login = match probe("user", ".login") {
Ok(output) if output.status.success() => {
String::from_utf8_lossy(&output.stdout).trim().to_owned()
}
Expand All @@ -185,10 +197,7 @@ fn ask_forge(owner: &str, key: &str) -> Owned {
if login.eq_ignore_ascii_case(owner) {
return Owned::Yes;
}
match crate::shim::inner_tool("gh")
.args(["api", &format!("repos/{key}"), "--jq", ".permissions.admin"])
.output()
{
match probe(&format!("repos/{key}"), ".permissions.admin") {
Ok(output) if output.status.success() => {
match String::from_utf8_lossy(&output.stdout).trim() {
"true" => Owned::Yes,
Expand Down Expand Up @@ -240,12 +249,12 @@ pub(crate) fn unowned(
root: &Path,
policy: &Policy,
rule: &Rule,
destination: (&str, &str),
destination: (&str, &str, Option<&str>),
forge: Option<Forge>,
asked: &mut BTreeMap<String, Owned>,
seam: Seam,
) -> Result<Option<Refusal>> {
let (owner, repo) = destination;
let (owner, repo, hostname) = destination;
let name = format!("{owner}/{repo}");

let (workspace, pinned) = workspace_owner(root, policy, rule)?;
Expand Down Expand Up @@ -352,7 +361,7 @@ pub(crate) fn unowned(
// host neither client answers for, gets exactly the answer it got before
// this branch existed rather than an invented one.
let verdict = match forge {
Some(Forge::GitHub) => Some(forge_owns(asked, owner, repo)),
Some(Forge::GitHub) => Some(forge_owns(asked, owner, repo, hostname)),
Some(Forge::GitLab) | None => None,
};
if verdict == Some(Owned::Yes) {
Expand Down Expand Up @@ -470,7 +479,7 @@ pub(crate) fn prevent_public_push(request: &Request<'_>) -> Result<Option<Refusa
request.root,
request.policy,
request.rule,
(&owner, &repo),
(&owner, &repo, None),
forge,
&mut asked,
Seam::Push,
Expand Down Expand Up @@ -528,7 +537,7 @@ mod tests {
&root,
&policy,
rule,
destination,
(destination.0, destination.1, None),
None,
&mut BTreeMap::new(),
seam,
Expand Down
25 changes: 13 additions & 12 deletions src/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,8 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result<E
))
})?;

// The label is what the report calls this call, and nothing else conditions
// on it. An event that does not carry one is checked exactly as thoroughly
// with the harness name in its place: refusing over a field that only
// decides a word in a message would be a gate firing on work it had no
// finding about, which is how a gate on a hot path gets deleted.
// The label names the call in reports and is its `tool` subject. A missing
// name retains the harness fallback; supplied argument text is still read.
let label = event
.pointer(shape.label)
.and_then(Value::as_str)
Expand All @@ -180,8 +177,8 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result<E
// event that is not JSON exits 2 rather than passing.
//
// What is NOT this case: a subject that is present and holds no strings. A
// call carrying only numbers and flags publishes no text there is a rule
// about, it was read, and it passed. Nor is a subject that is a bare string
// call carrying only numbers and flags still has a tool name to judge.
// Nor is a subject that is a bare string
// rather than an object -- that is text, and it is checked as text. The
// question here is whether the subject was found, never what shape the
// harness chose for it.
Expand Down Expand Up @@ -215,10 +212,7 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result<E

let mut collected = Walked::default();
walk_strings(subject, None, &mut collected);
// Read, and carrying no text this binary has a rule about.
if collected.prose.is_empty() && collected.headlines.is_empty() {
return Ok(Exit::Clean);
}
let has_text = !collected.prose.is_empty() || !collected.headlines.is_empty();
let text = collected.prose.join("\n");
let headlines: Vec<text::Headline> = collected
.headlines
Expand All @@ -231,7 +225,14 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result<E

let mut report = String::new();

for verdict in text::judged(text::Seam::Hook, &root, &policy, label, &text, &headlines)? {
for verdict in text::judged(
text::Seam::Hook,
&root,
&policy,
label,
has_text.then_some(text.as_str()),
&headlines,
)? {
match verdict {
// The message as well as the finding. At the shim and at `--text`
// the message is what tells the author what to do instead; here the
Expand Down
Loading
Loading