fix(attribution): catch attached-value forms of env/sudo cwd-shift flags

Round 13 audit found a real bug: `sudo --chdir=/tmp git commit`,
`env -C/tmp git commit`, `env --chdir=/tmp git commit`, and
`sudo -D/tmp git commit` were all silently slipping through the
cwd-shift detector and getting our `Co-authored-by` trailer stamped
onto commits that landed in a different repo.

Root cause: `shell-quote` tokenises both the long attached form
(`--chdir=/tmp`) and the short attached form (`-C/tmp`) as a single
argv entry. The previous SHIFT_CWD detector did set-membership only
against the bare flag (`{'-C', '--chdir'}` for env;
`{'-D', '--chdir'}` for sudo), so the attached-form tokens never
matched and `tokeniseSegment` returned a normally-attributable
`['git', 'commit', ...]` segment.

Fix: introduce `isShiftCwdFlag(flag, set)` that catches:
  - bare set-membership (existing behavior),
  - long attached: `--name=...` when `--name` is in the set,
  - short attached: `-Xanything` when `-X` is in the set and the
    token is longer than the flag itself.

The flag does NOT need to consume an extra value token in the
attached-form case (the value is already embedded), so the existing
TAKES_VALUE bookkeeping is unaffected — we just bail with `null`
from `tokeniseSegment` before reaching the value-skip step.

Tests added: `env --chdir=`, `env -C/...` (attached), `sudo --chdir=`,
`sudo -D/...` (attached) — each is asserted NOT to add a co-author
trailer. 154 shell tests pass; type-check + lint clean.
This commit is contained in:
wenshao 2026-05-07 13:55:17 +08:00
parent 9493369ed2
commit b89b65533f
2 changed files with 49 additions and 4 deletions

View file

@ -1765,6 +1765,16 @@ describe('ShellTool', () => {
// contract as `cd /elsewhere && git commit`. // contract as `cd /elsewhere && git commit`.
['env -C', 'env -C /tmp/other git commit -m "msg"'], ['env -C', 'env -C /tmp/other git commit -m "msg"'],
['env --chdir', 'env --chdir /tmp/other git commit -m "msg"'], ['env --chdir', 'env --chdir /tmp/other git commit -m "msg"'],
// Attached-value forms: `shell-quote` tokenises `--chdir=/tmp`
// and `-C/tmp` as single argv entries, so the bare-flag set
// membership check would miss them. Without explicit
// attached-form handling, `sudo --chdir=/tmp git commit` and
// `env -C/tmp git commit` would silently land our trailer on
// a commit in the wrong repo.
['env --chdir=', 'env --chdir=/tmp/other git commit -m "msg"'],
['env -C attached', 'env -C/tmp/other git commit -m "msg"'],
['sudo --chdir=', 'sudo --chdir=/tmp/other git commit -m "msg"'],
['sudo -D attached', 'sudo -D/tmp/other git commit -m "msg"'],
])( ])(
'should NOT add co-author for repo-redirecting %s assignment', 'should NOT add co-author for repo-redirecting %s assignment',
async (_label, command) => { async (_label, command) => {

View file

@ -230,10 +230,21 @@ function tokeniseSegment(segment: string): string[] | null {
// segment as repo-shifting (same contract as a leading // segment as repo-shifting (same contract as a leading
// `GIT_DIR=...` assignment) so we don't stamp our trailer onto // `GIT_DIR=...` assignment) so we don't stamp our trailer onto
// a commit that landed in a different repository. // a commit that landed in a different repository.
if ( //
(wrapper === 'env' && ENV_FLAGS_SHIFT_CWD.has(flag)) || // Also catch the attached-value forms `--chdir=DIR` and the
(wrapper === 'sudo' && SUDO_FLAGS_SHIFT_CWD.has(flag)) // short-form `-CDIR` / `-DDIR` that shell-quote tokenises as a
) { // single argv entry. Without this, `sudo --chdir=/tmp git
// commit` and `env -C/tmp git commit` would both pass through
// the bare-flag check (which is set-membership, not prefix-
// match) and silently land our trailer on a commit in the
// wrong repo.
const shiftSet =
wrapper === 'env'
? ENV_FLAGS_SHIFT_CWD
: wrapper === 'sudo'
? SUDO_FLAGS_SHIFT_CWD
: null;
if (shiftSet && isShiftCwdFlag(flag, shiftSet)) {
return null; return null;
} }
// Value-taking flag tables, per wrapper: `sudo -u user`, // Value-taking flag tables, per wrapper: `sudo -u user`,
@ -303,6 +314,30 @@ const ENV_FLAGS_SHIFT_CWD = new Set(['-C', '--chdir']);
// targets DIR's repo, not ours. Refuse the segment. // targets DIR's repo, not ours. Refuse the segment.
const SUDO_FLAGS_SHIFT_CWD = new Set(['-D', '--chdir']); const SUDO_FLAGS_SHIFT_CWD = new Set(['-D', '--chdir']);
/**
* Match a flag token against a SHIFT_CWD set, including attached-value
* forms. Bare `--chdir`/`-D`/`-C` are caught by direct set membership;
* the long attached form `--name=value` matches when `--name` is in the
* set, and the short attached form `-Xvalue` matches when `-X` is in
* the set AND the token is longer than the flag (so `-D` alone doesn't
* spuriously match `-D` against itself twice).
*/
function isShiftCwdFlag(flag: string, set: ReadonlySet<string>): boolean {
if (set.has(flag)) return true;
for (const f of set) {
if (f.startsWith('--') && flag.startsWith(f + '=')) return true;
if (
f.length === 2 &&
f.startsWith('-') &&
flag.startsWith(f) &&
flag.length > 2
) {
return true;
}
}
return false;
}
/** /**
* Environment variables that redirect git's repository selection. A * Environment variables that redirect git's repository selection. A
* leading `GIT_DIR=...`, `GIT_WORK_TREE=...`, etc. on a command makes * leading `GIT_DIR=...`, `GIT_WORK_TREE=...`, etc. on a command makes