Repository navigation
fix(agents): refuse git send-email command options and Unicode-folded or non-ASCII task programs (HACK-1438, HACK-1461) - #490
Merged
Conversation
…elain command runners (HACK-1438) The task-catalog inline-command denylist accepted `git send-email --to-cmd/--cc-cmd/--header-cmd/--sendmail-cmd` and `--smtp-server=/path`, each of which runs a command. send-email reads options with Perl's Getopt::Long defaults, so the one-dash and `+` prefixes, any letter case and unique prefixes are refused too, scoped to the send-email subcommand (dashed, absolute or mixed-case). Exact `--to`, `--cc` and `-h` still pass. A sweep of Git's porcelain adds the remaining command runners, each scoped to its subcommand: `grep -O`/`--open-files-in-pager`, `daemon --access-hook`, `instaweb -d`/`--httpd`, `svn --authors-prog`, plus the `remote-ext` helper, `ext::` URLs and `archimport` (which pastes its operands into a shell pipeline). Subcommand-scoped tables are now matched case-insensitively because a case-insensitive filesystem runs `git-send-email` for `git Send-Email`. Refs HACK-1438 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gs after the subcommand (HACK-1438) Merge origin/next (d09e1b7) into the HACK-1438 branch. #475 (HACK-1423) made subcommand-scoped Git flags count only after the operand that names the subcommand. Apply the same rule to the Getopt::Long send-email table and the HACK-1438 subcommandInline entries, keeping the lowercased subcommand lookup: one scopedEntries helper now serves both tables by operand index. `git log -o -- Grep` and `git rev-list --header send-email` now pass; `git log -- Grep -o` and every send-email spelling after the subcommand stay refused. Update the collateral test and docs to the one rule, and add tests that kill the `--`/`-` empty-name, `=ext::` and mid-argument `ext::` mutations. Refs HACK-1438 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-options Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs HACK-1438 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a denylisted program (HACK-1461) A case-insensitive filesystem (APFS, NTFS) looks names up with Unicode case folding, so `git ſend-email --to-cmd=cmd` and `git filter-branch` ran the denylisted Git programs while the catalog's `toLowerCase()` comparison accepted them. Program, subcommand and module names are now compared after NFKC, removal of default-ignorable code points and upper-then-lower case folding, and a listed program or a Git subcommand spelled with any non-ASCII character is refused whatever follows. Docs describe the folding and the `help.autocorrect` limit next to the alias limit; the HACK-1438 collateral test comment no longer claims a subcommand counts at any operand. Refs HACK-1461 Refs HACK-1438 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…task runs (HACK-1461) The G109 verifier showed U+1E9E (capital sharp s) still slipped through: APFS folds it to ss, so a program named ẞh runs ssh, but upper-then-lower case folding turned it into ß. Fold lower, upper, then lower so ẞ becomes ss, and refuse any program run as argv[0] whose file name has a non-ASCII character, listed or not, so a folding this check does not predict cannot reach a listed program. Later arguments keep the folded list check. Tests for ẞh (bare, absolute, as an operand) and unlisted non-ASCII programs; both changes proven by mutation. Refs HACK-1461 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
roodboi
marked this pull request as ready for review
October 24, 2026 23:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs HACK-1438
Root cause
The task-catalog denylist (
isInlineCommandArgvinsrc/lib/task-catalog-recipe.ts) did not model thegit send-emailoptions that run a command. Onnext(1281184), every one of these was accepted:--to-cmd,--cc-cmd,--header-cmdand--sendmail-cmd, in both the--opt cmdand--opt=cmdforms.--smtp-server=/abs/path. send-email runs an absolute path given here as a sendmail program.A second reason it was open:
git send-emailis a Perl script that reads its options with Getopt::Long's defaults (pass_throughonly). Getopt::Long's grammar is wider than Git's parse-options. It accepts--,-or+before a long name, ignores letter case, and accepts any unique prefix. So even a rule written for--to-cmdwould miss-to-cmd=cmd,+to-cmd=cmdand--CC-CMD=cmd. I confirmed each spelling against real Git 2.56 in a scratch repo. Each one ran a marker command:--to-cmd X,--to-cmd=X,--cc-cmd=X,--header-cmd=X,--sendmail-cmd=X,--smtp-server=/abs,--cc-cm=X,--hea=X,--sendm=X,--to-cm=X,--CC-CMD=X,-cc-cmd=X,-to-cmd=X,-header-cmd X,-sendmail-cmd=X,+to-cmd=X,-smtp-server=/abs, andgit Send-Email --cc-cmd=X. The last one works because APFS is case-insensitive, so Git finds thegit-send-emailfile forSend-Email.The
-c sendemail.ccCmd=…/toCmd/headerCmd/sendmailCmd/smtpServerconfig forms also ran, including identity-scopedsendemail.<id>.ccCmd. They were already refused, becausegit -cand--config-envare refused whatever the key. This PR adds tests for them.Fix (
src/lib/task-catalog-recipe.ts)subcommandGetoptLongtable on the evaluator, plus agetoptLongSelectsmatcher. The matcher strips a--,-or+prefix, takes the name up to=, and lowercases it. It refuses the argument when the name is a prefix of an inline option. The one exception is a name that exactly matches another option (to,cc,h), because Getopt::Long takes an exact name as that option.git send-emailgetsto-cmd,cc-cmd,header-cmd,sendmail-cmdandsmtp-server.send-emailoperand the same waysubcommandInlineis, so other commands are not refused (git rev-list --header,git status -s,git format-patch --to=…,git log --cc). The dashed form (git-send-email,/usr/lib/git-core/git-send-email) goes through the existing dashed-builtin mapping.grep -O/--open-files-in-pager(scoped togrep)daemon --access-hook(scoped todaemon)instaweb -d/--httpd(scoped toinstaweb)svn --authors-prog(scoped tosvn)remote-extwordext::URLs (newextUrlsflag)archimportwordTests (
tests/project-task-catalog.test.ts)HACK-1438: git send-email options that run a command(gap case).--git-dirand mixed-case forms. Also refused: the-cconfig forms in any key case, identity-scoped andsmtpServerkeys,--config-env, and-c sendemail.ccCmd=… -c alias.se=send-email se, which only the-crule catches.--to=… --cc …,--suppress-cc=all,--sender=…,--no-header-cmd,--smtp-server-port=587,--no-to,-h, and the same option names under other subcommands.HACK-1438: Git porcelain options that run a command(gap case). Covers the sweep additions, with scoped accepted controls:git grep -n,git grep --only-matching --or,git branch -d,git svn fetch --authors-file=…,git daemon --export-all,git instaweb --stop, anhttps://remote andgit add extension.txt.HACK-1438: documented send-email and grep fail-closed collateral stays refused. Coverssend-email -s,--smtp,--smtp-server=mail,git grep -o, and a path operandGrepscoping-O.Evidence
Before and after. I probed
isInlineCommandArgvdirectly with 24 argvs that should be refused. Onnext, 22 of them were accepted; only the 2-c/--config-envforms were refused. After the change, all 24 are refused, and the probed controls (--to=…,--cc=…,git log --cc,git format-patch --to=…) are still accepted.Mutation testing. I applied 25 mutations one at a time, ran
-t HACK-1438after each, and then restored the source. Every mutation fails at least one new test:--prefix+prefixgitargv)to-cmd,cc-cmd,header-cmd,sendmail-cmdsmtp-servergit -cgrepscope--open-files-in-pagerdaemoninstaweb -dinstaweb --httpdsvnremote-extwordarchimportwordext::URLsext::subcommandInlineFocused tests.
bun test tests/project-task-catalog.test.tsreported 40 pass, 0 fail. It ran with a private HOME, HACK_HOME and DOCKER_CONFIG andHACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1, and with--timeout 60000. The run with the default 5 s timeout hit 3 timeouts in slow existing domain tests (tracked pins, stale service definition, edit during read) while the host load average was about 106. None of the timeouts were in the changed tests.Gates. Root
bun run checkpassed, includingprivacy-check: ok, with only existing warnings and none in the changed files.bunx biome checkon the changed files was clean.bun run --cwd packages/cli typecheckreported 0 errors. gitleaks 8.30.1 overorigin/next..HEADfound no leaks.Not regenerated. The CLI surface and consumer agent guidance did not change, so the CLI reference and plugin bundles were not regenerated.
Full suite. The hosted CI
testjob at this head.Git porcelain sweep
Sources:
--git-completion-helper-allfor every command thatgit --list-cmds=builtins,main,others,nohelperslists on Git 2.56, plus the script sources ongit/gitmaster (git-send-email.perl,git-svn.perl,perl/Git/SVN.pm,git-instaweb.sh,git-p4.py,git-cvsimport.perl,git-cvsexportcommit.perl,git-archimport.perl,git-cvsserver.perl,git-web--browse.sh,git-mergetool.sh,git-difftool--helper.sh,git-submodule.sh,daemon.c,builtin/grep.c,imap-send.c). Version history (2.19 and later) comes from the existing table and #463/#475.send-email --to-cmd/--cc-cmd/--header-cmd/--sendmail-cmdsend-email --smtp-server=/abs-c/--config-envsendemail.*Cmd,sendemail.smtpServer,core.sshCommand,core.pager,core.fsmonitor, ...-c/--config-envrefused whatever the key)grep -O<pager>/--open-files-in-pager=<pager>use_shell)grepdaemon --access-hook=<cmd>use_shell)daemoninstaweb -d/--httpd=<cmd>instawebsvn --authors-prog=<cmd>Git::SVNsvn(git-svn is not installed here; read from source)git remote-ext <remote> <url>,ext::<cmd>URLsprotocol.ext.allow=neverblocks it, but config can allow it.EXT::is also blocked by Git ("cannot handle remote-EXT as a builtin").git archimport <archive/branch>tla abrowse … $limit |git archimport 'x/y;cmd #'rancmdeven without tla.--upload-pack,--receive-pack,--exec(clone,fetch,pull,ls-remote,push,archive,fetch-pack,send-pack),clone -u,rebase --exec/-x,difftool --extcmd/-x,submodule foreach,bisect run,filter-branch,rebase--helper --add-exec-commandsls-remotehas no-u(its--execis a hidden alias and is covered).rebase--interactive --cmd(2.20–2.33)nextyetcolumn --commandfor-each-ref --shell,cat-file --batch-commanddifftool/mergetool -t/--tool,web--browse -b,instaweb -b*.cmdgit hook run,clone/init --template(template hooks run during clone),--exec-path,merge/rebase/pull -s <name>(git-merge-<name>onPATH),transport::addressURLs (git-remote-<transport>)git config <key> <cmd>writes (core.pager,sendemail.ccCmd,include.path, ...)cvsimport -d :ext:host(runs$CVS_RSH host cvs server),p4,cvsserver,cvsexportcommit,imap-send(imap.tunnel)cvsexportcommitrev-parses its commit operands before it uses them in shell strings.Gaps
git se --cc-cmd cmdwithalias.se = send-email). This is the existing documented limit and is out of scope.format-patch(send-email -s,-t,--smtp).--smtp-serveris refused whatever its value.git grep -ois refused, because flags are case-folded; use--only-matching.Grepscopes-O. The same applies to the existingclone -uandrebase --add-exec-commands.subcommandInlineto count only after the subcommand's operand. When it is rebased onto this change (or this onto it), it should apply the same after-the-operand rule tosubcommandGetoptLong, and keep the lowercased lookup fromscopedEntries. The two PRs touch the same lines inevaluatesCodeand the docs paragraph, so expect a textual conflict.Round 2: merged with
next, one scoping rule (HACK-1423 + HACK-1438)The verifier found that this PR conflicted with
nextafter #475 (HACK-1423) and #480 landed. The conflict was not only textual. #475 made subcommand-scoped Git flags count only after the operand that names the subcommand. This PR counted them at any operand. Its collateral test asserted thatgit log -o -- Grepis refused, which contradicts #475.Rule chosen: #475's. A scoped flag counts only after the operand that names its subcommand, because Git reads a subcommand's options only after the subcommand's name. This PR's lowercased subcommand lookup is kept, so
git Send-Email --cc-cmd=cmdis still refused.What changed (merge commit 6cf5296, then a clean merge of 78487f7 in 0556dd9):
src/lib/task-catalog-recipe.ts.scopedFlags(from fix(agents): refuse Git rebase--interactive --cmd and scope subcommand flags after the subcommand #475) andscopedEntries(from this PR) are now one helper,scopedEntries({ table, positional, at }). It returns the table entries for operands before argumentat, and it looks up each operand lowercased. BothsubcommandInlineandsubcommandGetoptLonguse it, so the send-email Getopt::Long options follow the same after-the-subcommand rule asclone -u. fix(agents): refuse Git rebase--interactive --cmd and scope subcommand flags after the subcommand #475's--cmdinline flag and its comments are kept.git log -- Grep -ois refused andgit log -o -- Greppasses. New accepted controls where the flag comes before the subcommand word:git rev-list --header send-email,git log -s -- Send-Email,git diff -Oorderfile -- grep,git push -d origin instaweb. Every send-email spelling aftersend-emailis still refused. New cases kill the two mutations that survived round 1:git send-email --to=… -- pandgit send-email --to=… -pass (--and-name no option),git push --repo=ext::cmdis refused (ext::after=), andgit add docs/ext::notes.mdpasses (ext::only at the start of an argument or after=).docs/reference/ax-task-catalog.md). The contract table keeps both fix(agents): refuse Git rebase--interactive --cmd and scope subcommand flags after the subcommand #475's rows and this PR's rows. The scoping row now listsgit rev-list --header send-email,git push -d origin instawebandgit log -o -- Grepas accepted, andgit log -- Grep -oas refused. The limitations paragraph uses the single after-the-subcommand rule, says the lookup ignores letter case, and drops the round-1 text ("matched at any operand ... a path namedGrepscopes those flags to it").The "Interaction with #475" gap and the "any operand" collateral bullet above are now resolved by this round.
Evidence (round 2)
Focused tests.
bun test tests/project-task-catalog.test.ts: 42 pass, 0 fail. Run with a private HOME, HACK_HOME and DOCKER_CONFIG andHACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1.Mutation testing. I applied each mutation alone, ran the focused file, and restored the source:
atcheck)subcommandGetoptLongcounts at any operandsubcommandInlinecounts at any operand--,-)ext::matched only at the start of an argumentext::matched anywhere in an argumenthfrom the send-email exact listindex > atinstead ofindex >= atGates. Root
bun run checkexit 0, withprivacy-check: ok.bun run --cwd packages/cli typecheckexit 0, with 0 errors. gitleaks 8.30.1dirover agit log -p origin/next..HEADexport (next = 78487f7): no leaks. The CLI surface and consumer agent guidance did not change, so nothing was regenerated.Full suite. Hosted CI at 0556dd9, which contains
next78487f7.Round 3: Unicode case folding no longer bypasses the denylist (HACK-1461), merged with
nextRefs HACK-1461
The round-2 verifier found that APFS (and NTFS) look up names with Unicode case folding, but the denylist folded names with JavaScript's
toLowerCase(). Sogit ſend-email --to-cmd=cmd(U+017F long s),git-ſend-email,git inſtaweb --httpd=cmd,git ſvn clone --authors-prog=cmdandgit filter-branch --tree-filter cmd(U+FB01 fi ligature) were accepted. HACK-1461 records that two of them ran a marker command live on macOS.What changed (7483db3, on top of the merge of
nextcd35034 in 84d0379):src/lib/task-catalog-recipe.ts.foldNamereplacestoLowerCase()everywhere the denylist compares a name: program basenames (programName), Git helper porcelains (gitSubcommand), the subcommand-scoped tables (scopedEntries), thewordscheck, first-operandsubcommands, and Python REPL/trampoline modules (isListedModule).foldNameover-folds on purpose (fail-closed). It applies NFKC (fibecomesfi,ſbecomess, the Kelvin sign becomesK), removes default-ignorable code points such as a zero-width space, then upper-cases before lower-casing (soßbecomesssand dotlessıbecomesi).evaluatesCode, so the result does not depend on any one filesystem's folding table:ſh scripts/x.sh,git-ſtatus).git ſtatus,git --work-tree dir pack-refſ).git commit -m 'café',git log --author=Zoë) and unlisted programs (cat ſend-email) still pass.tests/project-task-catalog.test.ts). New testHACK-1461: names that a case-insensitive filesystem folds to a listed name are refused:Ksh -c,awK), dotless i (git ınstaweb,ıex), folding after the first operand (git log filter-branch,git log bıſect,git help ſend-email --to-cmd=…,git log filter-branch,python3 -m code) and the non-ASCII refusals.filter-branchandsend-email --to-cmdrefusals.docs/reference/ax-task-catalog.md).help.autocorrectnext to the repository-alias limit. With autocorrect set to run the correction,git send-emai --to-cmd=cmdrunssend-email, but the catalog sees an unknown subcommand and does not scope the flag to it.Evidence (round 3)
Focused tests.
bun test tests/project-task-catalog.test.ts: 43 pass, 0 fail, run withHACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1. The file's fixtures use their own private HOME, HACK_HOME and registry.Mutation testing. I applied each mutation alone, then ran the HACK-1461 test (it failed every time) and a pure
isInlineCommandArgvprobe over the same 30 cases, then restored the source.foldNameis plaintoLowerCase()(the pre-fix folding)ıex,git log filter-branch,git log bıſect,git help ſend-email --to-cmd=id,git log filter-branch,python3 -m code,ſh scripts/x.shſh scripts/x.sh,git-ſtatusgit ſtatus,git --work-tree dir pack-refſıex,git log bıſectpython3 -m codegit log filter-branchwordsandscopedEntriesback ontoLowerCase()The Kelvin cases do not flip under the
toLowerCase()mutation, because JavaScript already lowercases U+212A tok. They stay in the test as regression cases.Live macOS check (built CLI).
bun run build. In a scratch project with a private HOME, HACK_HOME and DOCKER_CONFIG,.hack/tasks.jsonheld the HACK-1461 argv, each with atouch <marker>command. I then ran./dist/hack project task list --json --path=<scratch>.inline_command, as is the ASCIIsend-email --to-cmdcontrol. The benignlintcontrol gets past the inline check and is refused later asinput_path_invalid(the scratch project is not a Git repository).task-catalog-recipe.ts(0556dd9) let the long-s, dashed, fi and dotless-i recipes through the inline check; they too stopped only atinput_path_invalid.Gates.
bun run check: exit 0, withprivacy-check: ok. Its 34 warnings were there before this change, and none is in the changed files.bun run --cwd packages/cli typecheck: 0 errors.dirover agit log -p origin/next..HEADexport: no leaks.Merge.
nextcd35034 is merged in 84d0379 with no conflicts, so the branch is no longer behind.Full suite. Hosted CI run 40134680598 (pull_request, headSha 7483db3) passed every check: the
testaggregate and test shards 1/4 to 4/4, docker-e2e shards 1/4 to 4/4, secret-scan, Candidate core on Ubuntu and macOS, the Config compiler jobs, Runtime state models, runtime-images and linux-process-lifetime. While CI ran,nextmoved to 191382d (fix(ci): fetch each commit's workflow once in the Bun crash census and skip comment lines in jobs #489, fix(native): pin storage binding call sites, tag carrier state mismatches and restore owned listeners on timeout #497). Neither touches this PR's files, andgit merge-treemerges it cleanly, so GitHub shows BEHIND, not a conflict.Gaps (round 3)
git ſend-email --to-cmdexecution in this round, because the agent harness blocks a private-HOMEgitrun outside its worktree. HACK-1461 already records that the bypass executes. This round shows that the catalog now refuses those argv.help.autocorrectand repository aliases are documented limits. They are not detected, because the check does not read Git config.Round 4 (312de30): capital sharp s and non-ASCII programs
The G109 verifier found a HIGH gap:
ẞh(U+1E9E, capital sharp s) resolves to/usr/bin/sshon the case-insensitive macOS filesystem, butfoldNamedid not fold it, because JavaScript lowercases U+1E9E toß, notss.foldNamenow runs NFKC, strips default-ignorables, then applies lower, upper, lower. The upper step mapsßtoSS, soẞhandßhfold tossh.ẞh,/usr/bin/ẞh,./scripts/ünïcode.sh,café). A case-insensitive filesystem may run such a name as an ASCII program thatfoldNamedoes not predict, so the check fails closed. Reviewed scripts must use ASCII file names. Later arguments are still checked throughfoldNameagainst the listed programs (cat ẞhis refused).tests/project-task-catalog.test.tsaddsẞh host id,/usr/bin/ẞh -o ProxyCommand=id x,cat ẞh,./scripts/ünïcode.shandcafé --versionas refused cases. The accepted controls did not change. 43 pass.cat ẞhcase. Removing the argv[0] rule fails the HACK-1461 test.docs/reference/ax-task-catalog.mddocuments theẞfold, the argv[0] refusal and the ASCII-name requirement.packages/clitypecheck has 0 errors,privacy:checkis ok, and gitleaks found no leaks in the commit patch. Hosted CI at 312de30 is the full-suite evidence.🤖 Generated with Claude Code