From b89b65533f005a1266ccb9f1acf7b33a41c4f73b Mon Sep 17 00:00:00 2001
From: wenshao <shaojin.wensj@alibaba-inc.com>
Date: Thu, 7 May 2026 13:55:17 +0800
Subject: [PATCH] fix(attribution): catch attached-value forms of env/sudo
 cwd-shift flags
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 packages/core/src/tools/shell.test.ts | 10 +++++++
 packages/core/src/tools/shell.ts      | 43 ++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts
index 773eb344d..5e97236a1 100644
--- a/packages/core/src/tools/shell.test.ts
+++ b/packages/core/src/tools/shell.test.ts
@@ -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) => {
diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts
index b2e887be4..933de97ca 100644
--- a/packages/core/src/tools/shell.ts
+++ b/packages/core/src/tools/shell.ts
@@ -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