Development record

guard's env wrapper table listed -S/--split-string as a value-owning flag for the SEPARATE-TOKEN spelling only ('env -S git push --force origin main'), but GNU env -S/--split-string also accepts the value glued to the flag ('env -Sgit push --force origin main', 'env --split-string=git push --force origin main'), which re-splits identically and executes identically; a fix attempted in bug-hunt loop round 9 (bugfix/env-split-string-guard-bypass) closed only the separate-token form and was BLOCKed at pre-PR review by both an independent correctness reviewer and an independent security reviewer, who each independently reproduced that the glued forms remain a complete, silent bypass of every guard blocker on the live PreToolUse path; suggested remediation (from both reviewers, converging independently): in commandOf/skipWrapperArgs, treat -S/--split-string as a value-owning env flag in all three spellings — the exact separate-token form (-S as its own token, value in the following token), the glued form (-S<value>, len>2), and the --split-string=<value> form (value non-empty) — and in every case read the first word of <value> as command position, rather than either treating the whole glued token as a consumed wrapper flag or skipping the following token outright in the separate-token case — no nested re-tokenizing required, contained to the same two functions

issueresolvedcriticaliss-200

Attempt 2 (2026-08-15) — BLOCKED by both independent reviewers; the splice-and-restart approach is rejected

bugfix/iss-200-env-split-string-guard-bypass-v2 (unpushed) implemented the converged remediation above — split the -S/--split-string value and read its first word as command position, contained to commandOf/skipWrapperArgs. The three plain spellings block, but an independent correctness review and an independent security review each BLOCKed the diff. The approach itself, not a detail of it, is the problem, so it is recorded here rather than patched again:

New quadratic cost regression (security, blocking). The splice-and- restart re-allocates and re-walks the whole token slice once per -S encountered, so commandOf becomes O(n²) where the pre-fix walk was linear. Measured end-to-end through the front door: a 384 KB crafted payload took ~35 s wall (the same input is ~4 ms on main), scaling cleanly 4× per doubling, so the 1 MiB stdin cap the code already enforces admits roughly five minutes. The PreToolUse hook that is supposed to gate the command is the thing stalled. This defect is introduced by the fix; main has no such behaviour.
The three in-scope spellings remain defeatable (correctness + security, blocking). strings.Fields is not env's splitter and the restart drops env's option parser, so all of these still read the wrong command position and return allow: a value with a quoted flag (env strips the quotes, the guard keeps them); a value using env's \_ argument separator (no whitespace, so Fields yields one word); and a value whose first word is an env option such as -i/-u (real env re-parses it as a flag, the walk reads it as the command). Verified against coreutils 9.10 and macOS BSD env.

The complete env -S spelling set the next attempt must cover (this is the acceptance corpus, consolidated here rather than split across new issues): separate-token, glued -S<value>, --split-string=<value>, GNU long-option abbreviation (--split=, --s=), a bundled short cluster carrying -S (-iS…, -iSgit…), a value with env quote-removal, a value using the \_ separator, and a value whose first word is itself an env option or --.

Design question for the maintainer — this needs a decision before a third attempt, per the pattern that shelved iss-189/190. Faithfully modelling env's parser (bounded, quote-aware, option-aware) is materially more than the "no nested re-tokenizing" the converged remediation assumed. The alternative is to treat env -S<anything> as an explicit, tested non-match the way tokenize.go documents its sh -c v1 gap — but that means allow, which for a denylist guard is a fail-open, the opposite of safe. A denylist guard cannot simply "refuse when unsure" without false-blocking legitimate env -S use. Resolving that tension (parse-faithfully-but-bounded vs. a narrowly-scoped refusal for the force-push class vs. accepting a documented residual) is the real work, and it is a design call, not a coding one.

Frontmatter

id	iss-200
type	issue
lifecycle	resolved
severity	critical

.abcd/work/issues/resolved/iss-200-guard-env-split-string-glued-form-bypass.md
open on GitHub ↗ · commit history ↗

mentions7

dcn-26080800000000022026-08-08 — bug-hunt loop round 9 (state issue #197): a multi-angle sweep (5
dcn-26081500000000012026-08-15 — three forward plans adopted at a maintainer grill, absorbing the existing queue rather than adding to it: `2026-08-15-plugin-user-safety.md` (execution priority 1; owns install-experience Cut B pick-up and the v0.5.0 security remainder; fix queue opens iss-200 then iss-195), `2026-08-15-predictable-development.md` (priority 2; itd-109 promotion is the headline; absorbs the v0.5.0 consistency remainder), `2026-08-15-facilitator-experience.md` (continuous, rides along; supersedes the explainability plan's pick-up role). The plan 1/3 boundary is touch-vs-machinery: what a facilitator directly invokes or reads versus automation that guarantees behaviour without human action. Each plan admits items by a stated admission test with the ledger remaining the backlog of record. Same grill: iss-189/190 retired by SUPERSESSION, not wontfix — the defects are real, the local-patch avenue is exhausted (three BLOCKed rounds), so both resolve into the new iss-229 (the recorded 2026-08-08 galloping-probe/trueMatchEnd structural design) with their repros as its acceptance corpus; and itd-111's one-tap micro-prompt open question graduated to its own capture iss-230, homed in the facilitator plan. Rejected: wontfixing 189/190 (asserts the defects don't matter), a full 104-issue triage into the plans (stale mirror of the ledger), and executing the plans in the listed 1-2-3 order (the safety floor and the compounding accelerators come first).
dcn-26081500000000042026-08-15 — iss-200 (env -S guard bypass) fix attempt 2 BLOCKED by two independent reviewers (correctness + security), round stopped per the plugin-user-safety plan's first-attempt-BLOCK STOP. The splice-and-restart approach (`commandOf` re-tokenizes the -S value and re-walks) is REJECTED, not to be re-patched: it introduced a quadratic cost regression (O(n^2), ~5 min at the 1 MiB stdin cap — a DoS on the very hook meant to gate the command) AND still let all three in-scope spellings through (env quote-removal, the `\_` separator, and a leading env option in the value all desync a strings.Fields split and a command-position restart). Branch bugfix/iss-200-env-split-string-guard-bypass-v2 left unpushed. iss-200 enriched with the complete env -S spelling set the next attempt must cover and the design question it now turns on: faithfully modelling env's parser (bounded/quote-aware/option-aware) is more than the "no nested re-tokenizing" the remediation assumed, while the tokenize.go "sh -c unparseable => documented non-match" precedent is a FAIL-OPEN for a denylist guard, so it does not transfer. This is the iss-189/190 pattern — repeated local-patch BLOCKs on one root cause mean the next move is an agreed design, not another patch. Rejected here: hot-patching the reviewers' suggested fixes and re-reviewing in-session (pushing through the STOP on a security-critical guard).
iss-189adjacency probe's trailing \b can be satisfied by the window edge instead of real content, causing a spurious over-redaction. scanAllPatterns (scanner.go) slices the line to a bounded window (line[end:min(end+512,len(line))]) before running each pattern's adjacencyProbe, to keep one probe attempt's cost constant regardless of what follows (iss-185's fix). Some probes retain an internal trailing \b of their own (e.g. net_lan_hostname's `\b` at the very end of its alternation) which normally requires a real non-word character (or true end of string) after the match. But when the probe is truncated to the window, "end of window" is treated by the regex engine exactly like "end of string" — so if the window happens to end immediately after what LOOKS like a complete hostname/token, the trailing \b is satisfied by the artificial window boundary rather than by real surrounding content, even though the FULL, untruncated line would not have matched there (the real next byte continues the token). Concrete repro: a line built as `"AKIA"+16*"Q"` (a confirmed match, establishing the probe's junction) + 506 filler bytes + ".local" + "zzz" — the window (512 bytes from the junction) ends exactly after "local", so net_lan_hostname's probe matches "b...local" (satisfied by end-of-window) even though the real line continues with "zzz" right after ".local", meaning the FULL line's own un-windowed net_lan_hostname pattern would NOT match there at all (no real word boundary follows in the actual text). Impact: a spurious net:lan_hostname finding (warn severity only, not hard_fail) causes Redact to needlessly rewrite up to 512 bytes of legitimate content as fingerprinted/starred output — not a security leak (over-redaction, not under-redaction) but a correctness/usability defect: legitimate file content gets mangled on a contrived but reachable input shape. Suggested fix (from the reviewer that found this): when a probe match's end lands exactly at the window's limit AND limit < len(line) (i.e. the window was actually truncated, not naturally at the line's end), re-run that one probe against the full, untruncated line[end:] before accepting the match — paying the larger cost only in this rare edge case rather than on every probe. Found during a merge-gate correctness review of iss-185's PR (bugfix/iss-185-scanner-adjacent-secret-boundary-bypass); non-blocking (contrived input, warn-severity-only impact), so not fixed inline — captured for its own verify-and-fix cycle.
iss-231guard warn tier is silent on the live PreToolUse hook: internal/surface/cli/guard.go maps VerdictWarn to Fprintln(stderr, message) + return nil = exit 0, but the failOpen path's own comment (same function) documents that a pre-tool-use hook exiting 0 has its stderr DISCARDED — which is exactly why failOpen uses exit 1 to be loud. So every warn-tier entry (e.g. git-reset-hard) writes a message nobody ever sees and the command runs as if allowed. The warn tier is effectively invisible. Fix direction: map VerdictWarn to exit 1 (the same non-blocking-but-loud status failOpen already relies on), so a warn both runs and surfaces its message. Load-bearing for iss-200's Option-D design, whose 'warn the middle with context' model is impossible while warn is silent; also a standalone correctness bug independent of iss-200.
iss-232itd-111 latent coupling: the ahoy install staleness refusal keys on the binary's vcs.revision (via vintage current-from-BuildInfo). Release binaries are VCS-stamped today (release.yml runs make build VERSION=<tag> from a git checkout, -buildvcs=auto default), so pinned users have a determinable vintage and proceed. But if a future release build ever sets -buildvcs=false or builds from a .git-less source archive, Known flips false for every pinned user and ahoy install begins REFUSING until they pass --allow-stale-binary. Add a guard/test on the release build config asserting the shipped binary carries vcs.revision, so this coupling cannot silently break. Non-blocking; surfaced by the independent ruthless review of feat/itd-111-staleness. NOTE: captured in the iss-200 design worktree to avoid an allocator collision (iss-231 taken there); it is an itd-111/release-build follow-up, not iss-200 work.
itd-2610161016401115Drains run unattended and visibly