mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-13 23:52:40 +00:00
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:
parent
9493369ed2
commit
b89b65533f
2 changed files with 49 additions and 4 deletions
|
|
@ -1765,6 +1765,16 @@ describe('ShellTool', () => {
|
|||
// contract as `cd /elsewhere && git commit`.
|
||||
['env -C', 'env -C /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',
|
||||
async (_label, command) => {
|
||||
|
|
|
|||
|
|
@ -230,10 +230,21 @@ function tokeniseSegment(segment: string): string[] | null {
|
|||
// segment as repo-shifting (same contract as a leading
|
||||
// `GIT_DIR=...` assignment) so we don't stamp our trailer onto
|
||||
// a commit that landed in a different repository.
|
||||
if (
|
||||
(wrapper === 'env' && ENV_FLAGS_SHIFT_CWD.has(flag)) ||
|
||||
(wrapper === 'sudo' && SUDO_FLAGS_SHIFT_CWD.has(flag))
|
||||
) {
|
||||
//
|
||||
// Also catch the attached-value forms `--chdir=DIR` and the
|
||||
// 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;
|
||||
}
|
||||
// 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.
|
||||
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
|
||||
* leading `GIT_DIR=...`, `GIT_WORK_TREE=...`, etc. on a command makes
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue