diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9f7a2c3..ec7f09f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -215,7 +215,7 @@ jobs: needs: dedupe if: needs.dedupe.outputs.passed != 'true' name: kani proofs - runs-on: ubuntu-slim + runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 with: diff --git a/Cargo.lock b/Cargo.lock index acb537b..dcaa73d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1344,6 +1344,7 @@ dependencies = [ "ignore", "jsonschema", "linkify", + "percent-encoding", "pulldown-cmark", "quick-junit", "regex", diff --git a/Cargo.toml b/Cargo.toml index f9e8fc9..ad1057b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,9 @@ ignore = "0.4" # The engine under `grep-regex`, used directly where a pattern is matched # against a string already in memory rather than searched through a file. regex = "1" +# Read a tool call's strings through their percent-encoding (RFC 3986 section +# 2.1) at `uphold hook`, so a URL carrying `%40` is judged as what it decodes to. +percent-encoding = "2" # Where a URL written out in prose ends and the sentence's punctuation begins, # for `citations-linked`. A URL pattern of our own gets that boundary wrong on # a trailing period, a closing parenthesis and a quote; this crate's tests are diff --git a/README.md b/README.md index 22a3af2..8e1f85c 100644 --- a/README.md +++ b/README.md @@ -338,6 +338,16 @@ second matcher in this binary. An unknown harness name is refused rather than guessed at, because where to find the text in its event cannot be derived from its name. +The Claude Code plugin also sends `Bash`, `WebFetch` and `WebSearch` through +the same hook. Those calls publish nothing to a forge, so a compiled-in +`[[hook]]` table judges them only by the rules about this machine and its user: +host identity, and the running user's own email address as git and the +environment configure it, read through URL encoding and withheld from the +refusal and the evidence. `git@github.com` and `react@18` are nobody's address +and pass. This checks the arguments the harness hands over, not the request a +client eventually sends. See +[the shell and web tools](docs/REFERENCE.md#shell-and-web-tools). + For Claude Code, enable the `uphold` plugin this repository ships, at the tag of the release you installed: diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 4f6d198..a772ae1 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -1152,7 +1152,9 @@ files.word = true The name says what fails: a literal describing **this machine** — username, home path, hostname and its identifying segments, default-route addresses — found in content. Sources: `running-os-identity`, `running-os-metadata` -(identity plus route), `running-default-route`. `forbidden_literals_from` +(identity plus route), `running-default-route`, and `running-user-email` (every +`user.email` git resolves, `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_EMAIL` and +`EMAIL`, whose matches are always reported as `[REDACTED_MATCH]`). `forbidden_literals_from` names any command producing one literal per line (`labelvalue`, or a bare line as its own label): @@ -3244,12 +3246,13 @@ the folder needs no install of their own: } ``` -The plugin's `hooks/hooks.json` holds three entries, each running +The plugin's `hooks/hooks.json` holds four entries, each running `uphold hook claude-code`: | event | matcher | options | |---|---|---| | `PreToolUse` | `mcp__github__.*` | `onFailure: "block"` | +| `PreToolUse` | `^(Bash\|WebFetch\|WebSearch)$` | `onFailure: "block"` | | `SessionStart` | | `async: true` | | `Stop` | | `timeout: 900`, `onFailure: "block"` | @@ -3262,6 +3265,74 @@ remove it. A cloud session does not add marketplaces from a repository's `extraKnownMarketplaces`, so the plugin does not reach one through project settings. +### Shell and web tools + +The `Bash`, `WebFetch` and `WebSearch` entry reaches the same judgment as every +other call, through a compiled-in `[[hook]]` table that says these calls +publish nothing to a forge: + +```toml +[[hook]] +tools = '^(Bash|WebFetch|WebSearch)$' +publishes = false +``` + +So they are judged by the rules that mean the same against any text: the +literal rules, and the pattern rules a policy writes for `seams = ["hook"]`. +The guards and the prose rules are about a publication and are recorded as out +of scope, so a private repository's name in a `cd` and an authorship marker in +a drafted commit message are not refused here. They are refused when the text +is published, at the shim or the forge's hook. + +Two literal rules run wherever the call is made, policy or none: +`no-running-os-identity-metadata`, and at this seam only `no-running-user-email`, +which reads `running-user-email`. The second refuses the running user's own +address. It does not refuse every email-shaped string: an SSH remote, a package +pin and an image digest look like addresses and identify nobody. A refusal and +its `--junit` record name the rule and where the address was configured, never +the address or the line it was on. Each string the call carries, except a +title or commit subject, is also read through up to four layers of +percent-encoding (RFC 3986 section 2.1), so `owner%40example.test` and +`owner%2540example.test` are found. + +A repository changes either end in policy. Declaring a rule that reads +`running-user-email` replaces the fallback, and like every `forbidden_literals` +rule it needs `files.*` and is then a content rule `uphold scan` runs over +those files too: scope it to where the address must never appear, since a +maintainer field or a `SECURITY.md` contact carries it on purpose. + +```toml +# The address in tool calls, in the docs tree, and nowhere else in the tree; +# a shared role address the user also sends as is exempt. +[rule.no-running-user-email] +message = "Leave the address out." +forbidden_literals = "running-user-email" +ignore_literals = ["security@example.test"] +files.include = ["docs"] + +# Every address, whoever's, in a tool call. +[rule.no-address-in-tool-calls] +message = "Leave addresses out." +regexp = '[A-Za-z0-9._%+-]+@[A-Za-z0-9-]+(\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}' +seams = ["hook"] +command.before = ["gh"] +command.scope = "always" +``` + +Both rules, and the user-email fallback, also judge the GitHub MCP server's +calls, which are the hook seam too. The shim and `--text` seams do not get the +fallback: a `Signed-off-by` trailer at `commit-msg` is the author's decision. A +repository that wants it there declares the rule. + +The check reads the arguments the harness supplies. It cannot see headers an +HTTP client adds after the hook, shell concatenation, the contents of a script +or an uploaded file, environment variables, or tools that never call a local +hook. A pass is not an attestation about the request that is eventually sent. +The Codex plugin's matcher is `mcp__.*`, so it does not send its shell tool +here. + +### Installing the Codex plugin + For [Codex](https://developers.openai.com/codex/), this repository publishes `.agents/plugins/marketplace.json`, pointing to `plugins/codex`. Add the marketplace at the same release tag as the installed binary: @@ -3362,6 +3433,8 @@ shim = "gh" # take scope and unresolved from this [[shim]] table # or, instead of shim: # scope = "public-target" # as on [[shim]]; "always" where neither is written # unresolved = "refuse" # as on [[shim]]; "run" is the opt-out +# or, for a call that publishes to no forge, with none of the above: +# publishes = false # judged by literal rules and seams = ["hook"] patterns only ``` That table is compiled in, from @@ -3424,6 +3497,9 @@ configured for. A table in an the scope is right at the shim, but that scope reads an npm command line, which no tool call has: at the hook it is unresolved, and the refusal names the rule. A pattern or prose rule takes `seams = ["shim"]` to stay off the hook. +`publishes = false` beside `skip`, `target`, `forge`, `owner`, `repo`, `shim`, +`scope` or `unresolved` is refused: those say where a publication goes, and +the table says there is none. See [Shell and web tools](#shell-and-web-tools). ### What runs where there is no policy diff --git a/docs/schema/principles.schema.json b/docs/schema/principles.schema.json index 97899b7..9800ee0 100644 --- a/docs/schema/principles.schema.json +++ b/docs/schema/principles.schema.json @@ -184,6 +184,11 @@ "description": "JSON pointer (RFC 6901) into the call's input, to the owner. Not the\nempty pointer, which names the whole input: an object is never an\nowner's name.", "type": "string" }, + "publishes": { + "default": true, + "description": "`false` where these calls publish nothing to a forge, but what they\ncarry still leaves the session: a shell command, a fetched URL, a\nsearch query. Only the rules that mean the same against any text judge\nthem -- the literal rules, which read the running host and user, and\nthe pattern rules whose `seams` names `hook`. The guards and the prose\nrules are about a forge publication and are recorded as out of scope:\na private repository's name in a `cd` is not that name published.\n`true` (the default) is every other table. Not written beside `skip`,\n`target`, `shim`, `scope` or `unresolved`, which say where a\npublication goes and which of its rules stand there.", + "type": "boolean" + }, "repo": { "description": "JSON pointer (RFC 6901) into the call's input, to the repository. Not\nthe empty pointer, as for `owner`.", "type": "string" diff --git a/plugins/claude-code/hooks/hooks.json b/plugins/claude-code/hooks/hooks.json index b47d91d..731aca6 100644 --- a/plugins/claude-code/hooks/hooks.json +++ b/plugins/claude-code/hooks/hooks.json @@ -1,6 +1,12 @@ { "hooks": { "PreToolUse": [ + { + "matcher": "^(Bash|WebFetch|WebSearch)$", + "hooks": [ + {"type": "command", "command": "uphold hook claude-code", "onFailure": "block"} + ] + }, { "matcher": "mcp__github__.*", "hooks": [ diff --git a/policy/hook.default.toml b/policy/hook.default.toml index 60a8fa5..50016cb 100644 --- a/policy/hook.default.toml +++ b/policy/hook.default.toml @@ -101,3 +101,15 @@ forge = "github" owner = "/owner" repo = "/repo" shim = "gh" + +# A harness's own shell and web tools: Claude Code's `Bash`, `WebFetch` and +# `WebSearch`. They publish nothing to a forge, and what they carry still +# leaves the session -- to a process, a fetched host, a search provider. So +# `publishes = false`: the literal rules judge them, which read this machine's +# identity and the running user's own addresses and mean the same against any +# text, as do the pattern rules a policy writes with `seams = ["hook"]`. The +# guards and prose rules are about a publication and are recorded as out of +# scope. A private repository's name in a `cd` is not that name published. +[[hook]] +tools = '^(Bash|WebFetch|WebSearch)$' +publishes = false diff --git a/src/config.rs b/src/config.rs index 630d924..347934c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2946,6 +2946,18 @@ fn hook_defaults() -> Result> { .map_err(|error| Fatal::new(format!("the compiled-in `[[hook]]` tables: {error}"))) } +/// The compiled-in `[[hook]]` tables alone, for a hook call made where no +/// policy is: which calls publish is a fact about the tools, and holds whether +/// or not anybody wrote a policy where the session started. +pub(crate) fn compiled_hooks() -> Result> { + load_hooks( + Path::new("policy/hook.default.toml"), + &[], + None, + hook_defaults()?, + ) +} + /// Each `[[hook]]` table parsed into the shape that carries its checks, and /// `unresolved` held to the reading [`validate_shim_unresolved`] gives it on /// `[[shim]]`: written only where some scope this table's calls reach can fail diff --git a/src/config/rule.rs b/src/config/rule.rs index b87542e..1962c1c 100644 --- a/src/config/rule.rs +++ b/src/config/rule.rs @@ -224,7 +224,8 @@ pub(crate) struct Written { #[serde(untagged)] pub(crate) enum Literals { /// A named built-in source of literals describing the running machine -- - /// `running-os-identity`, `running-os-metadata`, `running-default-route`. + /// `running-os-identity`, `running-os-metadata`, `running-default-route`, + /// `running-user-email`. /// The name says what fails: a literal describing THIS machine, found in /// content. Named { forbidden_literals: String }, diff --git a/src/guard/mod.rs b/src/guard/mod.rs index 4de91f2..e8cd9f5 100644 --- a/src/guard/mod.rs +++ b/src/guard/mod.rs @@ -665,7 +665,14 @@ pub(crate) fn text_refusal( // not a policy rule at all. Gating them on the table's scope would only // stand the identity check down for private destinations, which is the // arrangement `command.scope = "always"` on this rule exists to undo. - "text-literals" => crate::text::failures_in(root, policy, text, Some(kind), ledger), + "text-literals" => crate::text::failures_in( + crate::text::Seam::Guard, + root, + policy, + text, + Some(kind), + ledger, + ), _ => Ok(()), } } diff --git a/src/hook.rs b/src/hook.rs index 7923c24..ef1eebb 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -169,6 +169,41 @@ fn drop_unsent(collected: &mut Walked) { .retain(|(key, text)| !(key == "commit_title" && text.is_empty())); } +/// How many percent-decoding layers a string is read through. +/// +/// A URL carries an address as `person%40example.test`, and a query string +/// built from one can carry it as `%2540`. Each layer that changes the text is +/// judged beside the text as written, so a literal or a pattern finds the +/// value however many times it was encoded, up to this bound. Bounded because +/// each layer is another copy of the string: past four, an encoding is not a +/// URL any client built, and the text as written is still judged. +const DECODED_LAYERS: usize = 4; + +/// Add, after the strings a call carries, each percent-decoded layer of them +/// that differs from the one before (RFC 3986 section 2.1). +/// +/// Here and not in `walk_strings`, which also reads the document a shim's +/// `gh api --input` sends: that document is published as written, and what a +/// forge stores is the undecoded text. A tool call's URL or query is decoded +/// by whoever receives it, so what it discloses is what it decodes to. +fn decoded_layers(prose: &mut Vec) { + let mut layers = Vec::new(); + for text in prose.iter() { + let mut current = text.clone(); + for _ in 0..DECODED_LAYERS { + let next = percent_encoding::percent_decode_str(¤t) + .decode_utf8_lossy() + .into_owned(); + if next == current { + break; + } + layers.push(next.clone()); + current = next; + } + } + prose.extend(layers); +} + /// Judge one pending tool call, read from stdin in the harness's own shape. /// /// `found` is the policy where the call was made, and it is an `Option` here @@ -297,6 +332,10 @@ pub(crate) fn run( // down as set aside rather than asked. let named = policy.hooks.iter().find(|table| table.names(label)); let read = named.is_some_and(|table| table.skips(label)); + // A shell command, a fetched URL, a search query: text that leaves the + // session and reaches no forge. The rules about this machine and its user + // read it; the rules about a publication stand down, recorded as such. + let unpublished = named.is_some_and(|table| !table.publishes()); let mut scoping = table::Scoping::new(named, &policy.shims, label, subject, &root); let destination = if read { None @@ -309,6 +348,7 @@ pub(crate) fn run( walk_strings(subject, None, &mut collected); } drop_unsent(&mut collected); + decoded_layers(&mut collected.prose); let has_text = !collected.prose.is_empty() || !collected.headlines.is_empty(); let text = collected.prose.join("\n"); let headlines: Vec = collected @@ -350,9 +390,16 @@ pub(crate) fn run( // On a read, only a pattern rule about the tool name is in scope: the // text rules were set aside above, and a pattern rule about text has // none to read, so it is recorded as set aside rather than as a pass. + // + // On a call that publishes nothing, only the pattern rules a policy + // wrote for this seam are asked beside the literal rules, which are + // never asked for their scope: a guard or a prose rule is about a + // publication, and there is none. &mut |rule| { if read { Ok(rule.selects_subject("tool")) + } else if unpublished && text::Judged::of(rule) != Some(text::Judged::Patterns) { + Ok(false) } else { scoping.holds(rule) } @@ -388,8 +435,10 @@ pub(crate) fn run( // enclosing superproject's is not borrowed. Where there is no policy the // guards and the prose rules have nothing to run, and saying so on stderr // is the difference between a check that passed and a check that did not - // happen. - if found.is_none() { + // happen. Not for a call that publishes nothing, where the guards and the + // prose rules are out of scope with a policy or without one: saying so on + // every shell command would be the narration that gets a matcher deleted. + if found.is_none() && !unpublished { eprintln!( "uphold hook: no policy where this call was made, so the guards did not run \ and only the host-identity rules did. That is partial coverage, not a pass." diff --git a/src/hook/table.rs b/src/hook/table.rs index c34ff83..7f57a6b 100644 --- a/src/hook/table.rs +++ b/src/hook/table.rs @@ -75,6 +75,22 @@ pub(crate) struct Written { /// default) or `run`. As on `[[shim]]`. #[serde(default)] unresolved: Option, + /// `false` where these calls publish nothing to a forge, but what they + /// carry still leaves the session: a shell command, a fetched URL, a + /// search query. Only the rules that mean the same against any text judge + /// them -- the literal rules, which read the running host and user, and + /// the pattern rules whose `seams` names `hook`. The guards and the prose + /// rules are about a forge publication and are recorded as out of scope: + /// a private repository's name in a `cd` is not that name published. + /// `true` (the default) is every other table. Not written beside `skip`, + /// `target`, `shim`, `scope` or `unresolved`, which say where a + /// publication goes and which of its rules stand there. + #[serde(default = "publishes_by_default")] + publishes: bool, +} + +const fn publishes_by_default() -> bool { + true } /// Where a `[[hook]]` table's calls are going: the `[[shim]]` targets a tool @@ -132,6 +148,7 @@ pub(crate) struct Table { skip: std::collections::BTreeSet, destination: Option, pub(crate) answers: Answers, + publishes: bool, } impl Written { @@ -148,6 +165,23 @@ impl Written { }) }; let tools = compile(&self.tools)?; + if !self.publishes + && (!self.skip.is_empty() + || self.target != HookTarget::None + || self.forge.is_some() + || self.owner.is_some() + || self.repo.is_some() + || self.shim.is_some() + || self.scope.is_some() + || self.unresolved.is_some()) + { + return Err(format!( + "the `[[hook]]` for {tools_text:?} says `publishes = false` beside `skip`, \ + `target`, `forge`, `owner`, `repo`, `shim`, `scope` or `unresolved`. Those say \ + where a publication goes and which rules stand there, and a call that \ + publishes nothing has neither; take them off, or take `publishes` off" + )); + } if let Some(name) = self.skip.iter().find(|name| { name.contains("__") || name.is_empty() @@ -233,6 +267,7 @@ impl Written { skip, destination, answers, + publishes: self.publishes, }) } } @@ -261,6 +296,11 @@ impl Table { } } + /// Whether these calls publish to a forge. See [`Written::publishes`]. + pub(crate) const fn publishes(&self) -> bool { + self.publishes + } + /// Whether this table speaks for `tool`. pub(crate) fn names(&self, tool: &str) -> bool { self.tools.is_match(tool) @@ -463,6 +503,29 @@ mod tests { written().load().unwrap() } + /// A call that publishes nothing has no destination and no reads, so a + /// table saying both is one whose second half nothing reads. + #[test] + fn a_table_that_publishes_nothing_names_no_destination() { + let quiet: Written = + toml::from_str("tools = '^(Bash|WebFetch)$'\npublishes = false\n").unwrap(); + let table = quiet.load().unwrap(); + assert!(!table.publishes()); + assert!(github().publishes()); + for extra in [ + "skip = [\"read\"]", + "target = \"forge-repo\"\nforge = \"github\"\nowner = \"/o\"\nrepo = \"/r\"", + "shim = \"gh\"", + "scope = \"always\"", + "unresolved = \"run\"", + ] { + let written: Written = + toml::from_str(&format!("tools = '^Bash$'\npublishes = false\n{extra}\n")).unwrap(); + let error = written.load().unwrap_err(); + assert!(error.contains("publishes = false"), "{extra}: {error}"); + } + } + #[test] fn a_read_is_skipped_and_a_write_is_not() { let table = github(); diff --git a/src/sources.rs b/src/sources.rs index 6333d62..5a43069 100644 --- a/src/sources.rs +++ b/src/sources.rs @@ -18,6 +18,12 @@ pub(crate) struct Needle { pub label: String, pub value: String, pub word: bool, + /// The value is never reported, not even as the line it was found on. Set + /// by a source whose value is a person's contact address: a refusal and + /// its evidence are read, kept and published further than the text the + /// rule stopped, and a report quoting the address would be the disclosure. + /// The needle's label says which value matched; the value stays here. + pub withheld: bool, } /// Names that are never a personal identity leak. @@ -148,6 +154,7 @@ fn push(needles: &mut Vec, label: &str, value: Option, word: boo label: label.to_owned(), value, word, + withheld: false, }); } @@ -333,6 +340,88 @@ fn running_default_route() -> Vec { needles } +/// The addresses the running user sends mail and commits as. +/// +/// Read where the running user's tools read them: every `user.email` git +/// resolves at `root` (system, global, repository and included files, through +/// `git config --get-all`), and the variables git and mail clients take an +/// address from -- `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_EMAIL` and `EMAIL` +/// (git-commit-tree(1), "COMMIT INFORMATION"). The value lives only in this +/// process: no policy file names it, and every needle is [`Needle::withheld`], +/// so no report does either. +/// +/// Not every email-shaped string. An address is personal by whose it is, not +/// by its shape: `git@github.com`, `react@18` and `image@sha256:...` are +/// email-shaped and identify nobody, and a check that refused them on every +/// shell call would be switched off within the day. A repository that does +/// want any address refused writes a `regexp` rule with `seams = ["hook"]`. +/// +/// Searched as written and lowercased: a domain is case-insensitive (RFC 5321 +/// section 2.4), and the local part is in practice. No address configured is +/// no needles, which is an answer -- nothing here is known to be anyone's -- +/// and git failing to run at all is the could-not-look it is. +fn running_user_email(root: &Path) -> Answer { + // A machine with no git is an ordinary container, and a source that made + // every shell call there exit 2 would get the matcher deleted. The + // variables are still read, and stderr says what was not, as the + // default-route source does when `ip` cannot run. + let configured = match crate::git::try_run(root, &["config", "--get-all", "user.email"]) { + Ok(answer) => answer.unwrap_or_default(), + Err(error) => { + eprintln!( + "uphold: {error}, so the running-user-email source read only \ + GIT_AUTHOR_EMAIL, GIT_COMMITTER_EMAIL and EMAIL, and no address git is \ + configured with was searched for." + ); + String::new() + } + }; + let mut needles: Vec = Vec::new(); + let mut seen = BTreeSet::new(); + let found = configured + .lines() + .map(|line| ("git config user.email", line.to_owned())) + .chain( + ["GIT_AUTHOR_EMAIL", "GIT_COMMITTER_EMAIL", "EMAIL"] + .into_iter() + .filter_map(|name| env(name).map(|value| (name, value))), + ); + for (from, value) in found { + let value = value.trim(); + if !is_address(value) { + continue; + } + for spelling in [value.to_owned(), value.to_lowercase()] { + if seen.insert(spelling.clone()) { + needles.push(Needle { + label: from.to_owned(), + value: spelling, + word: false, + withheld: true, + }); + } + } + } + Ok(needles) +} + +/// Whether a configured value is an address worth searching for: one `@` +/// with a local part before it and a dotted domain after it (RFC 5321 section +/// 4.1.2's `Mailbox`, loosely), and no space. `EMAIL=@` or a bare name is a +/// misconfiguration, and as a needle it would refuse `react@18` under a +/// report that withholds what matched. +fn is_address(value: &str) -> bool { + let Some((local, domain)) = value.split_once('@') else { + return false; + }; + !local.is_empty() + && !domain.contains('@') + && !value.contains(char::is_whitespace) + && domain.split('.').filter(|label| !label.is_empty()).count() >= 2 + && !domain.starts_with('.') + && !domain.ends_with('.') +} + /// Run a command and read one needle per line of its stdout. /// /// This is what replaced `policy/sources.py`. A repository that needed a custom @@ -386,6 +475,7 @@ fn command_source(run: &str, root: &Path) -> Answer { enum Question { OsIdentity, DefaultRoute, + UserEmail, Command(String), } @@ -424,6 +514,7 @@ fn ask(question: &Question, root: &Path) -> Answer { .or_insert_with(|| match question { Question::OsIdentity => Ok(running_os_identity()), Question::DefaultRoute => Ok(running_default_route()), + Question::UserEmail => running_user_email(root), Question::Command(run) => command_source(run, root), }) .clone() @@ -447,6 +538,7 @@ pub(crate) fn resolve( let answered = match source { "running-os-identity" => ask(&Question::OsIdentity, root), "running-default-route" => ask(&Question::DefaultRoute, root), + "running-user-email" => ask(&Question::UserEmail, root), "running-os-metadata" => ask(&Question::OsIdentity, root).and_then(|mut all| { all.extend(ask(&Question::DefaultRoute, root)?); Ok(all) @@ -468,7 +560,8 @@ pub(crate) fn resolve( unknown => { return Err(Fatal::new(format!( "{label}: unknown literal source {unknown:?}; known sources are \ - running-os-identity, running-os-metadata, running-default-route, command" + running-os-identity, running-os-metadata, running-default-route, \ + running-user-email, command" ))); } }; @@ -489,6 +582,26 @@ pub(crate) fn resolve( mod tests { use super::*; + #[test] + fn only_a_value_shaped_like_an_address_is_searched_for() { + for value in ["owner@example.test", "o.w+n@sub.example.test"] { + assert!(is_address(value), "{value}"); + } + for value in [ + "@", + "owner", + "owner@", + "@example.test", + "react@18", + "a@b@c.d", + "o w@x.y", + "o@.x", + "o@x.", + ] { + assert!(!is_address(value), "{value}"); + } + } + #[test] fn an_address_says_which_machine_unless_it_is_loopback_link_local_or_multicast() { for address in ["192.168.1.20", "10.0.0.5", "8.8.8.8"] { diff --git a/src/text.rs b/src/text.rs index d810237..23307b0 100644 --- a/src/text.rs +++ b/src/text.rs @@ -38,6 +38,10 @@ use crate::sources; /// for whether anything already covers it have to mean the same string. const RUNNING_OS_IDENTITY: &str = "running-os-identity"; +/// The built-in literal source that reads the running user's own addresses. +/// Named once for the reason [`RUNNING_OS_IDENTITY`] is. +const RUNNING_USER_EMAIL: &str = "running-user-email"; + /// Used when the caller's repository declares no dynamic rules of its own, or /// has no policy file at all. /// @@ -66,6 +70,41 @@ fn fallback_rule() -> Rule { rule } +/// The hook's second fallback: the running user's own addresses, in a tool +/// call's arguments. +/// +/// At the hook only. A tool call is where an agent hands what it was told +/// about its user to a search engine, a fetched URL or a shell, none of which +/// the user chose to tell. A commit message is not: a `Signed-off-by` trailer +/// at `commit-msg` and a maintainer address a repository publishes on purpose +/// carry the address because the author put it there, and a fallback refusing +/// them would be refusing the author's own decision. A repository that wants +/// the address refused at its other text seams declares a rule reading +/// `running-user-email`, and one that exempts an address -- a published +/// security contact -- shadows this one with `ignore_literals`. +/// +/// Every needle this source yields is withheld, so neither the refusal nor +/// the evidence carries the address. +fn email_fallback_rule() -> Rule { + let mut rule = Rule::synthetic( + "no-running-user-email", + Check::ForbiddenLiterals { + literals: Literals::Named { + forbidden_literals: String::from(RUNNING_USER_EMAIL), + }, + ignore_literals: None, + }, + ); + rule.message = Some(String::from( + "Do not send the running user's email address in a tool call's arguments. The \ + policy checker reads the addresses git and the environment are configured with \ + and searches what the call was about to send; the matched address and the line \ + it was on are withheld here. Leave the address out, or use a placeholder such as \ + person@example.test.", + )); + rule +} + /// The text to judge, or a refusal. /// /// `from_utf8_lossy` stood here, and it is the quiet version of the failure @@ -499,7 +538,7 @@ pub(crate) fn judged( return Ok(()); } match kind { - Judged::Literals => failures_in(root, policy, whole, Some(&subject), ledger), + Judged::Literals => failures_in(seam, root, policy, whole, Some(&subject), ledger), // Each guard asked under the caller's scope: every one at `scan // --text` and `guard --text`, and at the hook the ones its table // stands down. The shim passes its own memo through `Judged::of`. @@ -809,7 +848,13 @@ pub(crate) fn check(found: Option<&(PathBuf, PathBuf)>, source: &str) -> Result< pub(crate) fn load_for(found: Option<&(PathBuf, PathBuf)>) -> Result<(PathBuf, Policy)> { match found { Some((root, policy_path)) => Ok((root.clone(), config::load(root, policy_path)?)), - None => Ok((std::env::current_dir()?, Policy::default())), + None => Ok(( + std::env::current_dir()?, + Policy { + hooks: config::compiled_hooks()?, + ..Policy::default() + }, + )), } } @@ -817,9 +862,10 @@ pub(crate) fn load_for(found: Option<&(PathBuf, PathBuf)>) -> Result<(PathBuf, P /// already-loaded policy, each answering into `ledger` once per needle found /// or once to pass. /// -/// The literal rules a text seam runs: the policy's, and the running-host -/// fallback where none of them reads the running host's identity. -fn literal_rules(policy: &Policy) -> Vec { +/// The literal rules a text seam runs: the policy's, the running-host +/// fallback where none of them reads the running host's identity, and at the +/// hook the running-user fallback where none of them reads the user's address. +fn literal_rules(policy: &Policy, seam: Seam) -> Vec { // The test is for the identity rule itself, not for the CHECK KIND it // happens to use. Asking whether any `forbidden_literals` rule existed made // an unrelated one -- a repository's own list of literals, a command @@ -839,6 +885,13 @@ fn literal_rules(policy: &Policy) -> Vec { { owned.push(fallback_rule()); } + if seam == Seam::Hook + && !owned + .iter() + .any(|rule| rule.forbidden_literals() == Some(RUNNING_USER_EMAIL)) + { + owned.push(email_fallback_rule()); + } owned } @@ -860,7 +913,7 @@ pub(crate) fn set_aside(policy: &Policy, recorded: &str, ledger: &mut Vec false, }); - for rule in literal_rules(policy).iter().chain(asked) { + for rule in literal_rules(policy, Seam::Hook).iter().chain(asked) { ledger.push(Judgment::of(rule, Some(recorded), Answer::OutOfScope)); } } @@ -869,13 +922,14 @@ pub(crate) fn set_aside(policy: &Policy, recorded: &str, ledger: &mut Vec, ledger: &mut Vec, ) -> Result<()> { - let owned = literal_rules(policy); + let owned = literal_rules(policy, seam); for rule in &owned { let needles = asked( @@ -912,7 +966,7 @@ pub(crate) fn failures_in( .iter() .map(|hit| { let line = hit.line.unwrap_or(0); - if policy.redact_matches { + if policy.redact_matches || needle.withheld { format!("line {line}: [REDACTED_MATCH]") } else { format!("line {line}: {}", hit.text) diff --git a/tests/claude_plugin.rs b/tests/claude_plugin.rs index b713ff7..08d670b 100644 --- a/tests/claude_plugin.rs +++ b/tests/claude_plugin.rs @@ -57,6 +57,26 @@ fn refusals(hooks: &Value) -> Vec { found } +/// The shell and web tools reach the hook, which the compiled-in `[[hook]]` +/// table for them judges by the rules about this machine and its user. +#[test] +fn the_shell_and_web_tools_reach_the_hook() { + let hooks = shipped(); + let groups = hooks["hooks"]["PreToolUse"] + .as_array() + .expect("pre-tool groups"); + for tool in ["Bash", "WebFetch", "WebSearch"] { + assert!( + groups.iter().any(|group| { + regex::Regex::new(group["matcher"].as_str().expect("a matcher")) + .expect("a valid matcher") + .is_match(tool) + }), + "{tool} does not reach the hook" + ); + } +} + fn shipped() -> Value { let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("plugins/claude-code/hooks/hooks.json"); serde_json::from_str(&std::fs::read_to_string(&path).expect("hooks.json is readable")) diff --git a/tests/hook_cli.rs b/tests/hook_cli.rs index 76e7bd5..3894709 100644 --- a/tests/hook_cli.rs +++ b/tests/hook_cli.rs @@ -77,18 +77,44 @@ fn workspace(name: &str, policy: Option<&str>) -> PathBuf { /// `HOME` is set on every run, never inherited, for the reason `text_cli` gives: /// the identity needles are read from the environment the check runs in, and a -/// test that let the real one through would assert on this machine. +/// test that let the real one through would assert on this machine. The +/// running user's addresses are read the same way, from git's configuration +/// and the mail variables, so git is pointed at no configuration file at all +/// and those variables are taken away. fn hook(root: &Path, harness: &str, stdin: &str) -> Output { - let mut child = Command::new(env!("CARGO_BIN_EXE_uphold")) + hook_as(root, harness, stdin, &[], &[]) +} + +/// [`hook`], with `arguments` after the harness's name and `environment` set +/// over the isolated one. +fn hook_as( + root: &Path, + harness: &str, + stdin: &str, + arguments: &[&str], + environment: &[(&str, &str)], +) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_uphold")); + command .args(["hook", harness]) + .args(arguments) .current_dir(root) .env_remove("UPHOLD_ALLOW") + .env_remove("EMAIL") + .env_remove("GIT_AUTHOR_EMAIL") + .env_remove("GIT_COMMITTER_EMAIL") .env("HOME", "/home/example-user") + .env("XDG_CONFIG_HOME", "/home/example-user/.config") + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + // And no repository above the fixture: a scratch directory under a + // checkout would hand its `user.email` to every call here. + .env("GIT_CEILING_DIRECTORIES", root.parent().unwrap()) + .envs(environment.iter().copied()) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .unwrap(); + .stderr(Stdio::piped()); + let mut child = command.spawn().unwrap(); if let Err(error) = child.stdin.as_mut().unwrap().write_all(stdin.as_bytes()) { // The binary may answer and exit before it reads its input; that is its // verdict, not a failure to deliver the event. @@ -97,6 +123,227 @@ fn hook(root: &Path, harness: &str, stdin: &str) -> Output { child.wait_with_output().unwrap() } +// -- the running user's address --------------------------------------------- + +/// The address every test below runs as. `example.test` is reserved (RFC 2606 +/// section 2), so no assertion here is about a real person. +const ADDRESS: &str = "owner@example.test"; + +fn denied(output: &Output) -> bool { + serde_json::from_str::(stdout(output).trim()) + .ok() + .and_then(|document| { + document + .pointer("/hookSpecificOutput/permissionDecision") + .and_then(Value::as_str) + .map(|decision| decision == "deny") + }) + .unwrap_or(false) +} + +/// The running user's address, wherever a shell or web call carries it, is +/// refused, and the refusal never repeats it -- not as the value, not as the +/// line it was found on. +/// +/// The address is read from git's configuration and from the variables git +/// takes one from, so each source is driven on its own. The policy is +/// deliberately absent: a session starts where nobody wrote one, and the +/// fallback is what stands there. +#[test] +fn the_running_users_address_is_refused_and_withheld() { + let root = workspace("hook-user-email", None); + let config = root.join("gitconfig"); + std::fs::write(&config, format!("[user]\n\temail = {ADDRESS}\n")).unwrap(); + let config = config.to_str().unwrap().to_owned(); + let sources: [&[(&str, &str)]; 3] = [ + &[("GIT_CONFIG_GLOBAL", config.as_str())], + &[("GIT_AUTHOR_EMAIL", ADDRESS)], + &[("EMAIL", "Owner@Example.Test")], + ]; + let calls = [ + ( + "Bash", + serde_json::json!({"command": format!("curl -A 'agent ({ADDRESS})' https://example.test")}), + ), + ( + "WebFetch", + serde_json::json!({"url": "https://example.test/?contact=owner%40example.test"}), + ), + ( + "WebFetch", + serde_json::json!({"url": "https://example.test/?contact=owner%2540example.test"}), + ), + ("WebSearch", serde_json::json!({"query": ADDRESS})), + ( + "WebFetch", + serde_json::json!({"headers": {"User-Agent": format!("agent ({ADDRESS})")}}), + ), + ( + "mcp__github__create_issue", + serde_json::json!({"owner": "example-org", "repo": "example", "body": ADDRESS}), + ), + ]; + for harness in ["claude-code", "codex"] { + for source in sources { + for (tool, input) in &calls { + let output = hook_as(&root, harness, &call(tool, input), &[], source); + assert_eq!(code(&output), 0, "{tool}: {}", stderr(&output)); + assert!(denied(&output), "{tool} {source:?}: {}", stdout(&output)); + for stream in [stdout(&output), stderr(&output)] { + assert!(stream.contains("no-running-user-email"), "{stream}"); + assert!(!stream.to_lowercase().contains("owner"), "{stream}"); + assert!(!stream.contains("curl"), "{stream}"); + } + } + } + } +} + +/// Email-shaped text that is nobody's address passes: an SSH remote, a +/// package pin, an image digest, and another person's address. A check that +/// refused these before every shell command would be switched off within the +/// day, and the address it exists for would go out with the rest. +/// +/// And a shell call is not a publication: a guard the policy runs at a forge +/// -- here an authorship marker -- is recorded as out of scope rather than +/// asked, so a commit message being drafted in a heredoc is not refused as if +/// it had been posted. +#[test] +fn email_shaped_text_and_publication_guards_do_not_stop_a_shell_call() { + let root = workspace("hook-user-email-clean", Some(POLICY)); + let directory = root.join("evidence"); + std::fs::create_dir(&directory).unwrap(); + for command in [ + "git clone git@github.com:example-org/example.git", + "npm install react@18 && npx tool@latest", + "docker pull image@sha256:0123abcd", + "git commit -m 'x' -m 'Co-Authored-By: Claude '", + "printf '%s' someone-else@example.test", + ] { + let output = hook_as( + &root, + "claude-code", + &call("Bash", &serde_json::json!({"command": command})), + &["--junit", directory.to_str().unwrap()], + &[("GIT_AUTHOR_EMAIL", ADDRESS)], + ); + assert_eq!(code(&output), 0, "{command}: {}", stderr(&output)); + assert_eq!(stdout(&output), "", "{command}"); + } + for entry in std::fs::read_dir(&directory).unwrap() { + let record = std::fs::read_to_string(entry.unwrap().path()).unwrap(); + assert!(!record.contains(" = std::fs::read_dir(&directory) + .unwrap() + .map(|entry| std::fs::read_to_string(entry.unwrap().path()).unwrap()) + .collect(); + assert_eq!(records.len(), 2); + assert_eq!( + records + .iter() + .filter(|record| record.contains(" String { String::from_utf8_lossy(&output.stdout).into_owned() } @@ -1021,7 +1268,7 @@ fn every_call_writes_one_document_naming_each_rule_and_never_the_body() { assert!(!text.contains("Generated"), "{text}"); let (second, _) = support::junit::suite(files.last().unwrap()); - assert_eq!((second.tests, second.failures), (3, 0), "{second:?}"); + assert_eq!((second.tests, second.failures), (4, 0), "{second:?}"); } /// The hook runs once per publishing call, so one file would keep only the @@ -1922,12 +2169,13 @@ fn a_skipped_read_writes_a_record_of_rules_set_aside() { "prevent-ai-author", "no-empty-hedge-here", "no-running-os-identity-metadata", + "no-running-user-email", ] { let case = support::junit::case(&suite, id); assert_eq!(support::junit::state(case).0, "skipped", "{id}"); assert_eq!(support::junit::properties(case, "subject"), [tool]); } - assert_eq!((suite.tests, suite.failures), (3, 0), "{suite:?}"); + assert_eq!((suite.tests, suite.failures), (4, 0), "{suite:?}"); } /// A tool call has no remote, so `git-remote` is not a `[[hook]]` target: the