Repository navigation
fix(agents): refuse rebase--helper --add-exec-commands and cover detached task surfaces - #463
Merged
Merged
Conversation
…ched task surfaces Close the task catalog follow-ups from #430 and #409. HACK-1411: older Git's `rebase--helper --add-exec-commands cmd` is now refused through a `rebase`-scoped inline flag, so `git config --add` still passes. Pin the mapping of a dashed builtin's trailing arguments (`git-config alias.x bisect--helper` stays refused), pin the lowercased porcelain (`git Rebase--Helper --add-exec-commands cmd`), document and test the new fail-closed collateral (`git push -u origin clone--helper`, `git add bisect--helper`, `git config --add alias.r rebase`), and label the helper case HACK-1373. HACK-1383: decision 7 lists both remedies the detached_checkout refusal offers (`--branch`/MCP `branch`, or `worktree.auto_branch=false`), and a regression covers CLI `task inspect` and both MCP task tools in a detached linked worktree, each surface with its own assertion. Refs HACK-1411, HACK-1383 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Found 1 test failure on Blacksmith runners: Failure
|
This was referenced Oct 24, 2026
Merged
roodboi
marked this pull request as ready for review
October 24, 2026 05:48
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
…phase and route state (#470) Refs HACK-1419 ## Summary This PR does **not** fix a root cause. The hosted evidence cannot tell a readiness race from a real routing defect, so it is not safe to change product code or relax the assertion. What it does is make the next failure conclusive: the log will name the URL, the phase, and the route state. ## What failed On #463, run 40057262783, job 120762873007 (`docker-e2e shard 1/4`), `native-config-networks` failed at the mixed routed ingress check: ``` Network command diagnostic {"step":"hack:exec","role":"primary","sequence":925,"expected":0, "exit":27,"timedOut":false,"durationMs":12872,...} FAIL native-config-networks (1391.6s) — Fixture command must exit 0; captures retained ``` - **What exit 27 means.** It is not curl's code 27. It is the fixture's own `NATIVE_NETWORK_PROBE_EXIT.timeout`: the in-container peer probe (`bun -e`, run through `hack exec <service>`) hit its 4 s `AbortSignal.timeout` while fetching `http://<target>:3000` or `http://<target>-alias:3000/`. - **Where it failed.** The failure came 70 s after "checking mixed routed ingress". At that point the census shows all 4 primary services running, plus the proxy. So the failing exec is in `markers(primary)` right after the routed `hack up`. In that phase `web` is attached to the external `hack-dev` ingress as well as its authored `outbound` bridge. - **Not a service-readiness race.** `observe(primary)` runs just before `markers`, and it requires every container to be `healthy`. The healthcheck is an in-container fetch of `127.0.0.1:3000`, so every server was already listening. A server that was not listening yet would refuse the connection and exit 26 at once, not stall until timeout. - **What the log cannot show.** It does not say which of the four service pairs failed, which URL (origin or alias), or whether name resolution or the HTTP exchange stalled. Hosted captures stay on the runner, and the run uploaded no artifact. ## Frequency I checked the docker-e2e shard jobs of the last 60 `ci.yml` runs on `next` and PR branches: 228 jobs, 215 green, 10 in progress, 3 failed. The 3 failures: - **This one:** exit 27 at the routed ingress check. It is the only occurrence. - **Run 39916341281 (next), shard 1:** a different `native-config-networks` failure, sibling-b `hack exec` with `E_COMPOSE_FAILED` storage-helper-start/probe-timeout. That is the storage helper budget, not ingress. - **Run 40055366376 (next), shard 4:** `native-compose-adoption-job-worktrees`, which is unrelated. So the exit 27 has appeared once in about 51 shard-1 runs. ## Candidate causes (none confirmed) 1. **Name resolution stalled.** A lost UDP reply from Docker's embedded DNS (127.0.0.11 is DNAT'd in the container netns) means the resolver waits about 5 s before retrying. That is longer than the probe's 4 s budget. 2. **A misroute specific to multi-homed `web`.** Here the name answers an address that is not on a bridge the two containers share. 3. **A transient connect stall on a fresh attachment.** Each of these produces exit 27 today, and they cannot be told apart from the log. ## Change (test fixture only) - **`nativeNetworkFixturePeerProbe` resolves first, then fetches.** It resolves the name (`Bun.dns.lookup`, IPv4 preferred) and then fetches the resolved address. Resolution and the HTTP exchange share the **same 4 s per-URL budget** (`NATIVE_NETWORK_PEER_PROBE_BUDGET_MS`), so the assertion is not loosened. Resolution failures get their own exit codes, `dnsRejected` 30 and `dnsTimeout` 31. Codes 26 and 27 now mean the HTTP phase only. - **The probe writes one closed stderr line before a failing exit:** `probe-evidence <origin|alias> <dns|http> <dnsMs> <elapsedMs> <addressCount>`. It never includes an address or body. - **On a failed peer probe the scenario logs `Network probe diagnostic {...}`.** The line contains: - the role, the service and target (fixed fixture names), the exit code, outcome and parsed evidence; - the route state, reduced to counts and booleans: both containers' state and health, attachment counts, shared bridges, whether the source is on the ingress, and how many addresses `getent ahostsv4 <probed name>` returned inside the source container, plus whether every one is the target's address on a shared bridge; - the outcome of **one diagnostic-only rerun**, which shows whether the stall is transient or persistent. The step still fails either way. - **Failures in the diagnostic path cannot hide the original failure.** Any error there logs `Network probe diagnostic unavailable`. ### How to read the next occurrence - **`phase:"dns"` with a passing rerun:** a resolver stall. Consider a resolver-aware budget. - **`resolvesToSharedTarget:false`:** a real routing or DNS defect. Fix it in the product, with a unit test. - **`phase:"http"`, a shared target address, and a failing rerun:** a persistent connectivity defect. ## Validation - **Unit tests:** `bun test tests/native-networks-fixture.test.ts` gives 63 pass. There are 4 new tests: probe phase and URL attribution, the evidence parser, route-state reduction, and the closed probe diagnostic. The existing probe exit-code test still passes. - **Mutation proof:** each of these 8 mutations kills the tests: DNS phase mislabelled, wrong DNS rejection code, IPv4 preference dropped, budget not shared, route `every` changed to `some`, evidence regex unanchored, rerun dropped, ingress ignored. - **Gates:** `bun run check` (includes privacy:check) passes. `bun run --cwd packages/cli typecheck` and `bun run typecheck` report 0 errors. Biome is clean on both changed files. gitleaks 8.30.1 finds no leaks over `origin/next..HEAD`. - **Docker:** nothing ran against a local Docker socket. Hosted `docker-e2e` shard 1 runs `native-config-networks` and exercises the new probe in real containers. ## Gaps - **Root cause still unknown.** This PR closes only the diagnosis gap. HACK-1419's acceptance ("cause identified") needs the next occurrence, which will now be conclusive, or a deliberate reproduction on an isolated Docker host. - **The diagnostic path has not run against real Docker.** It runs only when a probe fails, and its hosted coverage is limited to the success path. Its pure helpers are unit-tested. ## Hosted CI at 3e88132 - **Docker e2e:** all four `docker-e2e` shards pass, as does the `docker-e2e` aggregate. In shard 1 (run 40060640525, job 120773164774), `PASS native-config-networks (2288.2s)`, including the mixed routed ingress phase, with the new probe running in real containers. - **Other checks:** `test`, `secret-scan`, `linux-process-lifetime`, `runtime-images`, Runtime state models, and Config compiler (ubuntu, arm, macos-latest) pass. Candidate core and Config compiler on macOS were still running when this was written. - **Runtime:** the scenario took 2288 s here, against about 1290–1330 s on recent green runs. That runner was slow before the first probe ever ran: the probe-free created-before-start phase took 44 s against 15 s, and `bun install` took 47 s against 36 s. I attribute the difference to the runner, not the probe, but one run cannot rule out a small per-probe cost. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
… or non-ASCII task programs (HACK-1438, HACK-1461) (#490) Refs HACK-1438 ## Root cause The task-catalog denylist (`isInlineCommandArgv` in `src/lib/task-catalog-recipe.ts`) did not model the `git send-email` options that run a command. On `next` (1281184), every one of these was accepted: - `--to-cmd`, `--cc-cmd`, `--header-cmd` and `--sendmail-cmd`, in both the `--opt cmd` and `--opt=cmd` forms. - `--smtp-server=/abs/path`. send-email runs an absolute path given here as a sendmail program. A second reason it was open: `git send-email` is a Perl script that reads its options with Getopt::Long's defaults (`pass_through` only). 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-cmd` would miss `-to-cmd=cmd`, `+to-cmd=cmd` and `--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`, and `git Send-Email --cc-cmd=X`. The last one works because APFS is case-insensitive, so Git finds the `git-send-email` file for `Send-Email`. The `-c sendemail.ccCmd=…` / `toCmd` / `headerCmd` / `sendmailCmd` / `smtpServer` config forms also ran, including identity-scoped `sendemail.<id>.ccCmd`. They were already refused, because `git -c` and `--config-env` are refused whatever the key. This PR adds tests for them. ## Fix (`src/lib/task-catalog-recipe.ts`) - **New `subcommandGetoptLong` table** on the evaluator, plus a `getoptLongSelects` matcher. 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-email` gets `to-cmd`, `cc-cmd`, `header-cmd`, `sendmail- cmd` and `smtp-server`. - **Subcommand scoping.** The table is scoped to the `send-email` operand the same way `subcommandInline` is, 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. - **Case-insensitive lookup.** Both subcommand-keyed tables now look up operands lowercased, because of the case-insensitive filesystem point above. - **Sweep additions**, each refused the same way and each confirmed against real Git where it is installed (see the sweep below): - `grep -O` / `--open-files-in-pager` (scoped to `grep`) - `daemon --access-hook` (scoped to `daemon`) - `instaweb -d` / `--httpd` (scoped to `instaweb`) - `svn --authors-prog` (scoped to `svn`) - the `remote-ext` word - `ext::` URLs (new `extUrls` flag) - the `archimport` word ## Tests (`tests/project-task-catalog.test.ts`) - **`HACK-1438: git send-email options that run a command`** (gap case). - Refused: every spelling listed above, plus the dashed, absolute-path, behind-`--git-dir` and mixed-case forms. Also refused: the `-c` config forms in any key case, identity-scoped and `smtpServer` keys, `--config-env`, and `-c sendemail.ccCmd=… -c alias.se=send-email se`, which only the `-c` rule catches. - Accepted: `--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`, an `https://` remote and `git add extension.txt`. - **`HACK-1438: documented send-email and grep fail-closed collateral stays refused`.** Covers `send-email -s`, `--smtp`, `--smtp-server=mail`, `git grep -o`, and a path operand `Grep` scoping `-O`. ## Evidence - **Before and after.** I probed `isInlineCommandArgv` directly with 24 argvs that should be refused. On `next`, 22 of them were accepted; only the 2 `-c`/`--config-env` forms 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-1438` after each, and then restored the source. Every mutation fails at least one new test: | Mutation | Failing tests | | --- | --- | | Drop the Getopt::Long check | 2 | | Accept only the `--` prefix | 2 | | Drop the `+` prefix | 1 | | No abbreviations | 2 | | Case- sensitive option name | 1 | | Drop the exact list | 1 | | Case-sensitive subcommand lookup | 2 | | Unscoped (every `git` argv) | 1 | | Drop each of `to-cmd`, `cc-cmd`, `header-cmd`, `sendmail-cmd` | 1 each | | Drop `smtp-server` | 2 | | Drop `git -c` | 1 | | Drop the `grep` scope | 2 | | Drop `--open-files-in-pager` | 1 | | Drop `daemon` | 1 | | Drop `instaweb -d` | 1 | | Drop `instaweb --httpd` | 1 | | Drop `svn` | 1 | | Drop the `remote- ext` word | 1 | | Drop the `archimport` word | 1 | | Drop `ext::` URLs | 1 | | Case- sensitive `ext::` | 1 | | Unscoped `subcommandInline` | 1 | - **Focused tests.** `bun test tests/project-task-catalog.test.ts` reported 40 pass, 0 fail. It ran with a private HOME, HACK_HOME and DOCKER_CONFIG and `HACK_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 check` passed, including `privacy-check: ok`, with only existing warnings and none in the changed files. `bunx biome check` on the changed files was clean. `bun run --cwd packages/cli typecheck` reported 0 errors. gitleaks 8.30.1 over `origin/next..HEAD` found 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 `test` job at this head. ## Git porcelain sweep Sources: `--git-completion-helper-all` for every command that `git --list- cmds=builtins,main,others,nohelpers` lists on Git 2.56, plus the script sources on `git/git` master (`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. | Option or form | Runs | Status | | --- | --- | --- | | `send-email --to-cmd/--cc-cmd/--header- cmd/--sendmail-cmd` | shell command | **added** (Getopt::Long spellings) | | `send-email --smtp- server=/abs` | sendmail program | **added** (any value, fail-closed) | | `-c`/`--config-env` `sendemail.*Cmd`, `sendemail.smtpServer`, `core.sshCommand`, `core.pager`, `core.fsmonitor`, ... | config | already refused (`-c`/`--config-env` refused whatever the key) | | `grep -O<pager>` / `--open-files-in-pager=<pager>` | shell (`use_shell`) | **added**, scoped to `grep` | | `daemon --access-hook=<cmd>` | shell (`use_shell`) | **added**, scoped to `daemon` | | `instaweb -d/--httpd=<cmd>` | the daemon command line | **added**, scoped to `instaweb` | | `svn --authors-prog=<cmd>` | backticks in `Git::SVN` | **added**, scoped to `svn` (git-svn is not installed here; read from source) | | `git remote-ext <remote> <url>`, `ext::<cmd>` URLs | the URL as a command | **added**. Git's default `protocol.ext.allow=never` blocks it, but config can allow it. `EXT::` is also blocked by Git ("cannot handle remote-EXT as a builtin"). | | `git archimport <archive/branch>` | operand pasted into `tla abrowse … $limit \|` | **added**. `git archimport 'x/y;cmd #'` ran `cmd` even 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-commands` | shell | already modeled. `ls-remote` has no `-u` (its `--exec` is a hidden alias and is covered). | | `rebase--interactive --cmd` (2.20–2.33) | shell | in #475 (HACK-1423), not on `next` yet | | `column --command` | config lookup only | not a command | | `for-each-ref --shell`, `cat-file --batch-command` | quoting / stdin protocol | not a command | | `difftool/mergetool -t/--tool`, `web--browse -b`, `instaweb -b` | name must be a known tool or a configured `*.cmd` | config only | | `git hook run`, `clone/init --template` (template hooks run during clone), `--exec-path`, `merge/rebase/pull -s <name>` (`git-merge-<name>` on `PATH`), `transport::address` URLs (`git-remote-<transport>`) | a program file, not a string | not modeled (documented gap) | | `git config <key> <cmd>` writes (`core.pager`, `sendemail.ccCmd`, `include.path`, ...) | config read later | not modeled (existing documented gap) | | `cvsimport -d :ext:host` (runs `$CVS_RSH host cvs server`), `p4`, `cvsserver`, `cvsexportcommit`, `imap-send` (`imap.tunnel`) | env or config | not modeled. `cvsexportcommit` rev-parses its commit operands before it uses them in shell strings. | ## Gaps - **Repository aliases.** A repository alias still hides a subcommand from its scoped flags (`git se --cc-cmd cmd` with `alias.se = send-email`). This is the existing documented limit and is out of scope. - **Fail-closed collateral, documented.** - Any prefix of a refused send-email option is refused, even one Getopt::Long finds ambiguous and passes on to `format-patch` (`send-email -s`, `-t`, `--smtp`). - `--smtp-server` is refused whatever its value. - `git grep -o` is refused, because flags are case-folded; use `--only-matching`. - Subcommand names now match in any letter case at any operand, so a path named `Grep` scopes `-O`. The same applies to the existing `clone -u` and `rebase --add-exec-commands`. - **Interaction with #475 (HACK-1423, still a draft).** #475 changes `subcommandInline` to 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 to `subcommandGetoptLong`, and keep the lowercased lookup from `scopedEntries`. The two PRs touch the same lines in `evaluatesCode` and the docs paragraph, so expect a textual conflict. - **Not modeled.** Config writes and program-file runners (table above) are not modeled. ## Round 2: merged with `next`, one scoping rule (HACK-1423 + HACK-1438) The verifier found that this PR conflicted with `next` after #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 that `git log -o -- Grep` is 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=cmd` is still refused. What changed (merge commit 6cf5296, then a clean merge of 78487f7 in 0556dd9): - **`src/lib/task-catalog-recipe.ts`.** `scopedFlags` (from #475) and `scopedEntries` (from this PR) are now one helper, `scopedEntries({ table, positional, at })`. It returns the table entries for operands **before** argument `at`, and it looks up each operand lowercased. Both `subcommandInline` and `subcommandGetoptLong` use it, so the send-email Getopt::Long options follow the same after-the-subcommand rule as `clone -u`. #475's `--cmd` inline flag and its comments are kept. - **Tests.** The collateral test now asserts `git log -- Grep -o` is refused and `git log -o -- Grep` passes. 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 after `send-email` is still refused. New cases kill the two mutations that survived round 1: `git send-email --to=… -- p` and `git send-email --to=… -` pass (`--` and `-` name no option), `git push --repo=ext::cmd` is refused (`ext::` after `=`), and `git add docs/ext::notes.md` passes (`ext::` only at the start of an argument or after `=`). - **Docs (`docs/reference/ax-task-catalog.md`).** The contract table keeps both #475's rows and this PR's rows. The scoping row now lists `git rev-list --header send-email`, `git push -d origin instaweb` and `git log -o -- Grep` as accepted, and `git log -- Grep -o` as 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 named `Grep` scopes 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 and `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`. - **Mutation testing.** I applied each mutation alone, ran the focused file, and restored the source: | Mutation | Failing tests | | --- | --- | | Scoped flags count at any operand (drop the `at` check) | 5 | | Case-sensitive subcommand lookup | 2 | | `subcommandGetoptLong` counts at any operand | 1 | | `subcommandInline` counts at any operand | 4 | | Allow an empty Getopt::Long name (`--`, `-`) | 1 (survived round 1) | | `ext::` matched only at the start of an argument | 1 (survived round 1) | | `ext::` matched anywhere in an argument | 1 | | Drop `h` from the send-email exact list | 1 | | Getopt::Long without the one-dash prefix | 2 | | `index > at` instead of `index >= at` | 0, an equivalent mutation: the subcommand word itself cannot match a flag | - **Gates.** Root `bun run check` exit 0, with `privacy-check: ok`. `bun run --cwd packages/cli typecheck` exit 0, with 0 errors. gitleaks 8.30.1 `dir` over a `git log -p origin/next..HEAD` export (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 `next` 78487f7. ## Round 3: Unicode case folding no longer bypasses the denylist (HACK-1461), merged with `next` Refs 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()`. So `git ſend-email --to-cmd=cmd` (U+017F long s), `git-ſend-email`, `git inſtaweb --httpd=cmd`, `git ſvn clone --authors- prog=cmd` and `git 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 `next` cd35034 in 84d0379): - **`src/lib/task-catalog-recipe.ts`.** - A new `foldName` replaces `toLowerCase()` everywhere the denylist compares a name: program basenames (`programName`), Git helper porcelains (`gitSubcommand`), the subcommand-scoped tables (`scopedEntries`), the `words` check, first-operand `subcommands`, and Python REPL/trampoline modules (`isListedModule`). - `foldName` over-folds on purpose (fail-closed). It applies NFKC (`fi` becomes `fi`, `ſ` becomes `s`, the Kelvin sign becomes `K`), removes default-ignorable code points such as a zero-width space, then upper-cases before lower-casing (so `ß` becomes `ss` and dotless `ı` becomes `i`). - Option names keep ASCII-only folding. Git's parse-options and Getopt::Long compare options bytewise apart from ASCII letter case, and option values are left unfolded. - Two outright refusals in `evaluatesCode`, so the result does not depend on any one filesystem's folding table: - A listed program whose basename contains any non-ASCII character is refused whatever follows (`ſh scripts/x.sh`, `git-ſtatus`). - A Git subcommand (the first operand) that contains any non-ASCII character is refused (`git ſtatus`, `git --work-tree dir pack-refſ`). - Non-ASCII operands and values (`git commit -m 'café'`, `git log --author=Zoë`) and unlisted programs (`cat ſend-email`) still pass. - **Tests (`tests/project-task-catalog.test.ts`).** New test `HACK-1461: names that a case- insensitive filesystem folds to a listed name are refused`: - 18 refused spellings covering the long s, the fi ligature, the Kelvin sign (`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. - 8 accepted controls, plus the ASCII `filter-branch` and `send-email --to-cmd` refusals. - The collateral comment no longer claims a subcommand counts "at any operand". It now says the name is case-folded and scopes its flags from any operand that names it. - **Docs (`docs/reference/ax-task-catalog.md`).** - The refusal list describes the Unicode folding and the non-ASCII rule, and the contract table has a new row for it. - The limitations paragraph documents `help.autocorrect` next to the repository-alias limit. With autocorrect set to run the correction, `git send-emai --to-cmd=cmd` runs `send-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 with `HACK_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 `isInlineCommandArgv` probe over the same 30 cases, then restored the source. | Mutation | Cases that flip to accepted | | --- | --- | | `foldName` is plain `toLowerCase()` (the pre-fix folding) | 7: `ı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` | | No non-ASCII listed-program refusal | 2: `ſh scripts/x.sh`, `git-ſtatus` | | No non-ASCII Git subcommand refusal | 2: `git ſtatus`, `git --work-tree dir pack-refſ` | | No upper-case step | 2: `ıex`, `git log bıſect` | | NFC instead of NFKC | 1: `python3 -m code` | | Default-ignorables kept | 1: `git log filter-branch` | | `words` and `scopedEntries` back on `toLowerCase()` | 4 | The Kelvin cases do not flip under the `toLowerCase()` mutation, because JavaScript already lowercases U+212A to `k`. They stay in the test as regression cases. - **Live macOS check (built CLI).** - Setup: I ran `bun run build`. In a scratch project with a private HOME, HACK_HOME and DOCKER_CONFIG, `.hack/tasks.json` held the HACK-1461 argv, each with a `touch <marker>` command. I then ran `./dist/hack project task list --json --path=<scratch>`. - At this head, every Unicode recipe (long s, dashed long s, fi ligature, dotless i, Kelvin) is refused as `inline_command`, as is the ASCII `send-email --to-cmd` control. The benign `lint` control gets past the inline check and is refused later as `input_path_invalid` (the scratch project is not a Git repository). - A CLI built the same way from the round-2 `task-catalog-recipe.ts` (0556dd9) let the long-s, dashed, fi and dotless-i recipes through the inline check; they too stopped only at `input_path_invalid`. - The marker file was never created. The catalog has no execution path. - **Gates.** - Root `bun run check`: exit 0, with `privacy-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. - gitleaks 8.30.1 `dir` over a `git log -p origin/next..HEAD` export: no leaks. - The CLI surface and the consumer agent guidance did not change, so nothing was regenerated. - **Merge.** `next` cd35034 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 `test` aggregate 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, `next` moved to 191382d (#489, #497). Neither touches this PR's files, and `git merge-tree` merges it cleanly, so GitHub shows BEHIND, not a conflict. ### Gaps (round 3) - I did not repeat the direct `git ſend-email --to-cmd` execution in this round, because the agent harness blocks a private-HOME `git` run outside its worktree. HACK-1461 already records that the bypass executes. This round shows that the catalog now refuses those argv. - `help.autocorrect` and 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/ssh` on the case-insensitive macOS filesystem, but `foldName` did not fold it, because JavaScript lowercases U+1E9E to `ß`, not `ss`. - **Folding.** `foldName` now runs NFKC, strips default-ignorables, then applies lower, upper, lower. The upper step maps `ß` to `SS`, so `ẞh` and `ßh` fold to `ssh`. - **argv[0] rule.** A recipe whose program (argv[0]) has any non-ASCII character in its file name is now refused, listed or not (`ẞh`, `/usr/bin/ẞh`, `./scripts/ünïcode.sh`, `café`). A case-insensitive filesystem may run such a name as an ASCII program that `foldName` does not predict, so the check fails closed. Reviewed scripts must use ASCII file names. Later arguments are still checked through `foldName` against the listed programs (`cat ẞh` is refused). - **Tests.** `tests/project-task-catalog.test.ts` adds `ẞh host id`, `/usr/bin/ẞh -o ProxyCommand=id x`, `cat ẞh`, `./scripts/ünïcode.sh` and `café --version` as refused cases. The accepted controls did not change. 43 pass. - **Mutations.** Restoring the old fold fails the `cat ẞh` case. Removing the argv[0] rule fails the HACK-1461 test. - **Docs.** `docs/reference/ax-task-catalog.md` documents the `ẞ` fold, the argv[0] refusal and the ASCII-name requirement. - **Gates.** Biome is clean on the changed files (complexity is back under the limit), `packages/cli` typecheck has 0 errors, `privacy:check` is ok, and gitleaks found no leaks in the commit patch. Hosted CI at 312de30 is the full-suite evidence. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Refs HACK-1411
Refs HACK-1383
Follow-ups from the #430 (HACK-1373) and #409 (HACK-1358) verifiers. Fail-closed hardening of the read-only task catalog, plus test and doc fixes. Nothing here executes a task.
Root cause
<sub>--helperas<sub>, but:...opts.rest.map(gitSubcommand)changed to...opts.rest) survived.gitSubcommandwas not covered either (M6 survived).rebase--helper --add-exec-commandswas documented as an open gap. Adding it to thegitinline list would also refusegit config --add, because Git abbreviates long options.--branchas a remedy, but thedetached_checkoutmessage also offersworktree.auto_branch=false. The detached-worktree regression did not cover CLItask inspector the MCP task tools.Fix
src/lib/task-catalog-recipe.ts:subcommandInlinegainsrebase: ["--add-exec-commands"]. The flag counts only whenrebase--helpermaps torebase, or when some other operand isrebase..toLowerCase()stays. It is now documented and covered by a test: it keeps a mixed-casegit Rebase--Helpermatched against the lowercase scoped keys (fail-closed).tests/project-task-catalog.test.ts:issuelabel.git-config alias.x bisect--helper,git rebase--helper --add-exec-commands id,git-rebase--helper --add-exec idandgit Rebase--Helper --add-exec-commands id.git config --add alias.st statusandgit rebase--helper --continue.HACK-1411: documented Git helper fail-closed collateral stays refused, coveringgit push -u origin clone--helper,git add bisect--helperandgit config --add alias.r rebase.HACK-1383: CLI task inspect and the MCP task tools refuse an implicit instance in a detached linked worktree. Each surface (CLI inspect, MCP list, MCP inspect) has its own labeled exact-envelope assertion, and each also passes when the instance is named. The spawned CLI and the MCP child useprivateCliEnv.docs/reference/ax-task-catalog.md:--branch/MCPbranch, orworktree.auto_branch=false) and the surfaces that refuse.rebase--helpergap is closed, and the new collateral is listed.Evidence
Bun 1.4.2. Tests ran with a private
HOME,HACK_HOMEandDOCKER_CONFIG, and withHACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1.bun test tests/project-task-catalog.test.ts: 37 pass. One later rerun on a host with load average ~180 timed out the untouchedpins must be Git-tracked filestest at its default 5 s timeout. It passes when run alone. See Gaps....opts.rest(nogitSubcommand)git-config alias.x bisect--helperflips to accepted.toLowerCase()git Rebase--Helper --add-exec-commands idflipsrebasescoped entry--add-exec-commandsinto the unscopedgitinline listgit config --add alias.st statusis refusedgitoperandhandleInspectdefaults a branchcli inspectassertionbranchbun run check(includesprivacy:check) passes;bun run --cwd packages/cli typecheckreports 0 errors.origin/next..HEAD(as agit log -pexport): no leaks.testjob at this head.Gaps
pins must be Git-tracked files; untracked content is never hashedruns under the default 5 s timeout instead ofCASE_TIMEOUT_MS, so it can time out on a heavily loaded host. It is not changed here and is reported separately.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.