From e9df9a30eea2fa9c5a3c05d27ab70c3e012f7b47 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:09:00 +0100 Subject: [PATCH 01/14] fix(security): scope the scrub hook by the live shell directory, not the session root The Bash tool event that Claude Code hands a mod carries no working directory, so register() fell back to $.session.root() and flagged the scope as a fallback. The root is only where the session started; with the fallback flag set, the tracker exemption is skipped, so in a session started in the tracker checkout a plain `git commit -F msg.txt` for the tracker's own config was blocked whenever the message used embargo terms. git has no repository flag, so the `--repo` hint in the block message offered no way through. The mod API also exposes $.session.cwd(): the session's current directory, which Claude Code moves when a Bash call changes directory (and which is the calling agent's own directory for a subagent). register() now reads it before falling back to the root. The hook then scopes a bare command by the directory the shell is really in: a commit in the tracker checkout takes the tracker exemption, and one made after an earlier call moved the shell into an upstream worktree is scanned. A cd on the same line is still followed as before. An explicit event cwd still wins. The session-root fallback and its rule are unchanged for harnesses where the current directory is unavailable. Its block message no longer suggests `--repo` for a git command, which has no such flag. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 68 +++++++++++++++++++ .../hooks/public-surface-scrub.ts | 24 ++++++- 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index e05dc7fa7..3a6aa3fa8 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2210,6 +2210,74 @@ upstream_repo: apache/kafka }); assert(res?.result && res.result.stdout === 'executed', `Expected tool call to execute: ${JSON.stringify(res)}`); }); + + test('a bare tracker git commit is blocked without --repo advice when only the root is known', async ($: any, on: any) => { + setupTrackerEnv(on); + const res = await $.tool.call({ tool: 'Bash', command: 'git commit -m "Allocated CVE-2026-1234"' }); + assert(typeof res?.deny === 'string', `Expected deny: ${JSON.stringify(res)}`); + assert(!res.deny.includes('--repo'), `git block must not suggest --repo: ${res.deny}`); + }); + }); + + describe('register() reads the current directory from $.session.cwd()', () => { + const trackerProject = '\n| Key | Value |\n| tracker_repo | airflow-s/airflow-s |\n| upstream_repo | apache/airflow |\n'; + const trackerGitConfig = '\n[remote "origin"]\n url = https://github.com/airflow-s/airflow-s.git\n'; + const upstreamGitConfig = '\n[remote "origin"]\n url = https://github.com/apache/airflow.git\n'; + const msgFile = '/home/dev/msg.txt'; + + // Session root is always the tracker checkout; $.session.cwd() says where the shell is now. + function setupEnv(on: any, currentDir: string): void { + on('session.root', () => ({ value: '/home/dev/airflow-s' })); + on('session.cwd', () => ({ value: currentDir })); + on('fs.read', (_: any, e: any) => { + const p = (e.path || '').replace(/\\/g, '/'); + if (p.endsWith('/.apache-magpie-overrides/project.md')) return { value: trackerProject }; + if (p === '/home/dev/airflow-s/.git/config') return { value: trackerGitConfig }; + if (p === '/home/dev/scratch/airflow-wt/.git/config') return { value: upstreamGitConfig }; + if (p === msgFile) return { value: 'Note the CVE-2026-1234 advisory in the tracker config' }; + return { value: null }; + }); + on('env.get', () => ({ value: undefined })); + on('tool.call', () => ({ result: { stdout: 'executed' } })); + } + + test('a bare git commit -F passes when the current directory is the tracker checkout', async ($: any, on: any) => { + setupEnv(on, '/home/dev/airflow-s'); + const res = await $.tool.call({ tool: 'Bash', command: `git commit -F ${msgFile}` }); + assert(res?.result && res.result.stdout === 'executed', `Expected tool call to execute: ${JSON.stringify(res)}`); + }); + + test('a bare git commit is blocked after an earlier call moved the shell into an upstream worktree', async ($: any, on: any) => { + setupEnv(on, '/home/dev/scratch/airflow-wt'); + const res = await $.tool.call({ tool: 'Bash', command: `git commit -F ${msgFile}` }); + assert(typeof res?.deny === 'string', `Expected deny in upstream worktree: ${JSON.stringify(res)}`); + }); + + test('a cd on the same line still moves the scope away from the tracker', async ($: any, on: any) => { + setupEnv(on, '/home/dev/airflow-s'); + const res = await $.tool.call({ + tool: 'Bash', + command: 'cd /home/dev/scratch/airflow-wt && git commit -m "Fix CVE-2026-1234"', + }); + assert(typeof res?.deny === 'string', `Expected deny after cd: ${JSON.stringify(res)}`); + }); + + test('an event cwd takes precedence over $.session.cwd()', async ($: any, on: any) => { + setupEnv(on, '/home/dev/airflow-s'); + const res = await $.tool.call({ + tool: 'Bash', + cwd: '/home/dev/scratch/airflow-wt', + command: `git commit -F ${msgFile}`, + }); + assert(typeof res?.deny === 'string', `Expected deny on event cwd: ${JSON.stringify(res)}`); + }); + + test('gh pr create in the tracker directory is exempt like any verified tracker cwd', async ($: any, on: any) => { + // Not fallback any more, so the tracker cwd exemption applies like any verified cwd + setupEnv(on, '/home/dev/airflow-s'); + const res = await $.tool.call({ tool: 'Bash', command: `gh pr create -t "Config" --body-file ${msgFile}` }); + assert(res?.result && res.result.stdout === 'executed', `Expected verified tracker cwd to exempt gh: ${JSON.stringify(res)}`); + }); }); }); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 6bddadc22..4f404ee73 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2349,7 +2349,15 @@ export async function evaluatePublicSurfaceCommand( if (trackerFallbackHint) { // The Bash tool call carries no cwd, so a command run in the tracker checkout cannot be // told apart from one run after an earlier `cd ../`; an explicit target can. - reason += `\nIf this command targets the private tracker, name it explicitly: \`--repo ${trackerFallbackHint}\`.`; + if (parsed.commandType.startsWith('git ')) { + // git commit/tag/merge has no --repo flag; only the directory it runs in identifies the repository. + reason += + '\nThe current directory of this command could not be determined, so the private-tracker exemption ' + + 'was not applied. A git command has no repository flag; the exemption needs a known working directory ' + + 'inside the tracker checkout.'; + } else { + reason += `\nIf this command targets the private tracker, name it explicitly: \`--repo ${trackerFallbackHint}\`.`; + } } return { blocked: true, @@ -2376,6 +2384,20 @@ export function register(on: any): void { if (typeof command === 'string') { let initialCwd: string | undefined = e?.cwd || e?.directory; let isSessionRootFallback = false; + if (!initialCwd) { + // The Bash tool event carries no directory. $.session.cwd() is the session's current + // directory: the shell's directory as left by earlier Bash calls (a `cd` that persists + // across calls moves it), or the calling agent's own directory. $.session.root() is only + // where the session started, so it is used just when the current directory is unknown. + try { + const current = await $.session.cwd(); + if (typeof current === 'string' && current) { + initialCwd = current; + } + } catch { + // $.session.cwd may be absent in mock harnesses or older engines + } + } if (!initialCwd) { try { initialCwd = await $.session.root(); From fc4b74d5012ef054d1a6d9e5c91128a45b7f5594 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:11:24 +0100 Subject: [PATCH 02/14] fix(security): treat an earlier GIT_* assignment on the line as a git redirect The scrub hook already treats GIT_DIR / GIT_WORK_TREE leading the git command itself (`GIT_DIR=x git commit`) as a repository redirect that the cwd-based tracker exemption cannot cover. The same variables set by an earlier segment of the line slipped past it: with a verified tracker cwd, `export GIT_DIR=../upstream/.git && git commit -m ...` took the tracker exemption although the commit lands in the other repository. A git public-surface subcommand now loses the cwd-based exemptions (tracker and known non-upstream remote) and falls through to the term scan when an earlier segment on the same line sets or exports a GIT_* variable through export, declare, typeset, readonly or local (including `export GIT_DIR` of an earlier assignment), or through a bare `GIT_*=...` segment with no command after it. Any GIT_* name counts, not only the repository-selection ones: the list grows with git releases and a tracker commit has no reason to set any of them earlier on the same line. An assignment scoped to another command (`GIT_DIR=x ls`) does not persist and is not flagged. `set -a` style auto-export is out of scope. Claude Code does not carry variables across Bash calls, so the same line is the only place this can happen. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 43 +++++++++++++++++++ .../hooks/public-surface-scrub.ts | 41 +++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 3a6aa3fa8..177aa13a6 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2280,4 +2280,47 @@ upstream_repo: apache/kafka }); }); + describe('an earlier GIT_* assignment on the line redirects a bare git command', () => { + const config = { trackerRepo: 'airflow-s/airflow-s', upstreamRepo: 'apache/airflow' }; + const trackerGitConfig = '\n[remote "origin"]\n url = https://github.com/airflow-s/airflow-s.git\n'; + const readFile = async (p: string) => (p.endsWith('.git/config') ? trackerGitConfig : null); + + // Verified tracker cwd (not the session-root fallback), so only the GIT_* rule can block. + function evaluate(cmd: string) { + return evaluatePublicSurfaceCommand(cmd, { cwd: '/home/dev/airflow-s', projectConfig: config, readFile }); + } + + test('export, declare, typeset, readonly, local and bare assignments of GIT_* block', async () => { + for (const cmd of [ + 'export GIT_DIR=../u/.git && git commit -m "Fix CVE-2026-1234"', + 'GIT_WORK_TREE=x; git commit -m "Fix CVE-2026-1234"', + 'declare -x GIT_DIR=../u/.git && git commit -m "Fix CVE-2026-1234"', + 'typeset -x GIT_COMMON_DIR=../u/.git; git commit -m "Fix CVE-2026-1234"', + 'readonly GIT_INDEX_FILE=../u/.git/index && git commit -m "Fix CVE-2026-1234"', + 'local GIT_OBJECT_DIRECTORY=../u/.git/objects; git tag -a v1 -m "Fix CVE-2026-1234"', + 'GIT_DIR=../u/.git; export GIT_DIR; git commit -m "Fix CVE-2026-1234"', + 'export FOO=1 GIT_NAMESPACE=x && git merge topic -m "Fix CVE-2026-1234"', + ]) { + const res = await evaluate(cmd); + assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); + } + }); + + test('unrelated variables and a GIT_* assignment scoped to another command keep the exemption', async () => { + for (const cmd of [ + 'export FOO=1 && git commit -m "Fix CVE-2026-1234"', + 'GIT_DIR=../u/.git ls && git commit -m "Fix CVE-2026-1234"', + 'git commit -m "Fix CVE-2026-1234"', + ]) { + const res = await evaluate(cmd); + assertStrictEqual(res.blocked, false, `Expected tracker exemption for: ${cmd}: ${JSON.stringify(res)}`); + } + }); + + test('a neutral message still passes after a GIT_* assignment', async () => { + const res = await evaluate('export GIT_DIR=../u/.git && git commit -m "Refactor config loading"'); + assertStrictEqual(res.blocked, false, `Expected pass on neutral text: ${JSON.stringify(res)}`); + }); + }); + }); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 4f404ee73..c94a2debe 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2073,6 +2073,37 @@ const SAFE_READONLY_COMMANDS = new Set([ 'readonly', ]); +/** Shell builtins that assign or export variables for the rest of the command line. */ +const VARIABLE_DECLARATION_BUILTINS = new Set(['export', 'declare', 'typeset', 'readonly', 'local']); + +/** + * True when a subcommand sets or exports a `GIT_*` variable for the subcommands after it: + * `export GIT_DIR=...`, `declare -x GIT_WORK_TREE=...`, `export GIT_DIR` (exporting an earlier + * assignment), or a bare `GIT_DIR=...` segment with no command after it. Any `GIT_*` name counts, + * not only the repository-selection ones (GIT_DIR, GIT_WORK_TREE, GIT_COMMON_DIR, GIT_INDEX_FILE, + * GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_CEILING_DIRECTORIES, GIT_NAMESPACE, ...): + * the list grows with git releases, and a legitimate tracker commit has no reason to set any of them + * earlier on the same line. `GIT_DIR=... git commit` (an assignment leading the git command itself) + * is detected by parseSubcommand; `GIT_DIR=... ls` scopes the variable to `ls` and is not flagged. + */ +function setsGitVariableForLaterCommands(subcommand: string): boolean { + const words = tokenizeShellWords(subcommand.trim()); + let idx = 0; + let hasGitAssignment = false; + while (idx < words.length && /^[a-zA-Z_][a-zA-Z0-9_]*=/.test(words[idx])) { + if (/^GIT_[A-Za-z0-9_]*=/.test(words[idx])) hasGitAssignment = true; + idx++; + } + if (idx === words.length) { + return hasGitAssignment; // bare assignment segment: the variables persist for later subcommands + } + const binary = words[idx].split(/[/\\]/).pop()?.toLowerCase() ?? ''; + if (!VARIABLE_DECLARATION_BUILTINS.has(binary)) { + return false; + } + return words.slice(idx + 1).some((w) => /^GIT_[A-Za-z0-9_]*(=|$)/.test(w)); +} + /** * Evaluates whether a public-surface command violates embargo terms. * Evaluates per subcommand, respects project.md config for tracker/upstream resolution, @@ -2099,6 +2130,9 @@ export async function evaluatePublicSurfaceCommand( const subshellCwdStack: (string | undefined)[] = []; const seenRedirectedFiles = new Set(); let hasPotentiallyMutatingPriorCommand = false; + // Set once an earlier subcommand on the line sets or exports a GIT_* variable, which can point a + // later bare git command at another repository without changing directory. + let hasPriorGitVariable = false; for (const item of subcommands) { if (item.subshellDepthDeltaBefore) { @@ -2126,6 +2160,10 @@ export async function evaluatePublicSurfaceCommand( const rawSub = item.raw; const parsed = parseSubcommand(rawSub); + const isRedirectedByPriorGitVariable = hasPriorGitVariable && parsed.commandType.startsWith('git '); + if (setsGitVariableForLaterCommands(rawSub)) { + hasPriorGitVariable = true; + } // Track cd transitions: update virtualCwd across sequential operators (&&, ;, \n, ||) // Only exclude pipeline (|) and background (&) which run in subshells @@ -2256,7 +2294,8 @@ export async function evaluatePublicSurfaceCommand( parsed.commandType.includes('git-dir') || parsed.commandType.includes('work-tree') || parsed.commandType.includes('GIT_DIR') || - parsed.commandType.includes('wrapped') + parsed.commandType.includes('wrapped') || + isRedirectedByPriorGitVariable ) { // Repository context is redirected or indeterminate — cwd remoteInfo cannot exempt it // Proceed directly to inspection and dynamic-input fail-closed check From 33fe733a242e19ec19b6985f904185a28fe173fa Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:15:03 +0100 Subject: [PATCH 03/14] fix(security): fail closed on any earlier environment change before git or gh The GIT_* rule only recognised plain declaration builtins and git commands, so `builtin export GIT_DIR=...`, `GIT_DIR+=...`, an `eval` or `source`, or `export GH_REPO=...` before a bare `gh` command still let the later command take the directory-based exemption while acting on another repository. Fail closed instead: once an earlier segment on the line mentions any GIT_* or GH_* variable, runs an environment-opaque builtin (eval, source, ., set, read, mapfile, printf -v) after any builtin / command / control-word prefix, or declares variables with expansions, later git and gh commands lose the directory-based exemption and go through the term scan. A tracker commit has no reason to do any of this on the same line. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 26 ++++++- .../hooks/public-surface-scrub.ts | 68 ++++++++++++------- 2 files changed, 66 insertions(+), 28 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 177aa13a6..f49988999 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2280,7 +2280,7 @@ upstream_repo: apache/kafka }); }); - describe('an earlier GIT_* assignment on the line redirects a bare git command', () => { + describe('an earlier environment change on the line redirects a bare git or gh command', () => { const config = { trackerRepo: 'airflow-s/airflow-s', upstreamRepo: 'apache/airflow' }; const trackerGitConfig = '\n[remote "origin"]\n url = https://github.com/airflow-s/airflow-s.git\n'; const readFile = async (p: string) => (p.endsWith('.git/config') ? trackerGitConfig : null); @@ -2300,16 +2300,36 @@ upstream_repo: apache/kafka 'local GIT_OBJECT_DIRECTORY=../u/.git/objects; git tag -a v1 -m "Fix CVE-2026-1234"', 'GIT_DIR=../u/.git; export GIT_DIR; git commit -m "Fix CVE-2026-1234"', 'export FOO=1 GIT_NAMESPACE=x && git merge topic -m "Fix CVE-2026-1234"', + 'builtin export GIT_DIR=../u/.git; git tag -a v1 -m "Fix CVE-2026-1234"', + 'command export GIT_DIR=../u/.git; git commit -m "Fix CVE-2026-1234"', + 'GIT_DIR+=x; git commit -m "Fix CVE-2026-1234"', + 'GIT_DIR=../u/.git ls && git commit -m "Fix CVE-2026-1234"', + 'eval "$SETUP"; git commit -m "Fix CVE-2026-1234"', + 'source ./env.sh && git commit -m "Fix CVE-2026-1234"', + '. ./env.sh && git commit -m "Fix CVE-2026-1234"', + 'set -a; git commit -m "Fix CVE-2026-1234"', + 'printf -v X %s y; git commit -m "Fix CVE-2026-1234"', + 'export "$V"; git commit -m "Fix CVE-2026-1234"', ]) { const res = await evaluate(cmd); assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); } }); - test('unrelated variables and a GIT_* assignment scoped to another command keep the exemption', async () => { + test('an earlier GH_REPO or GH_HOST blocks a later bare gh command', async () => { + for (const cmd of [ + 'export GH_REPO=apache/airflow; gh issue create -t "CVE-2026-1234" -b "details"', + 'GH_HOST=example.com; gh pr create -t "Fix CVE-2026-1234" -b "x"', + ]) { + const res = await evaluate(cmd); + assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); + } + }); + + test('unrelated earlier commands keep the exemption', async () => { for (const cmd of [ 'export FOO=1 && git commit -m "Fix CVE-2026-1234"', - 'GIT_DIR=../u/.git ls && git commit -m "Fix CVE-2026-1234"', + 'echo ready && git commit -m "Fix CVE-2026-1234"', 'git commit -m "Fix CVE-2026-1234"', ]) { const res = await evaluate(cmd); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index c94a2debe..315ae3030 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2076,32 +2076,48 @@ const SAFE_READONLY_COMMANDS = new Set([ /** Shell builtins that assign or export variables for the rest of the command line. */ const VARIABLE_DECLARATION_BUILTINS = new Set(['export', 'declare', 'typeset', 'readonly', 'local']); +/** Shell builtins that can change the environment in ways the hook cannot see. */ +const ENVIRONMENT_OPAQUE_BUILTINS = new Set(['eval', 'source', '.', 'set', 'read', 'mapfile', 'readarray']); + +/** Prefixes that run the next word as the command (`builtin export ...`, `command -p eval ...`). */ +const COMMAND_RUNNING_PREFIXES = new Set(['builtin', 'command', 'then', 'do', 'else', '!']); + /** - * True when a subcommand sets or exports a `GIT_*` variable for the subcommands after it: - * `export GIT_DIR=...`, `declare -x GIT_WORK_TREE=...`, `export GIT_DIR` (exporting an earlier - * assignment), or a bare `GIT_DIR=...` segment with no command after it. Any `GIT_*` name counts, - * not only the repository-selection ones (GIT_DIR, GIT_WORK_TREE, GIT_COMMON_DIR, GIT_INDEX_FILE, - * GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_CEILING_DIRECTORIES, GIT_NAMESPACE, ...): - * the list grows with git releases, and a legitimate tracker commit has no reason to set any of them - * earlier on the same line. `GIT_DIR=... git commit` (an assignment leading the git command itself) - * is detected by parseSubcommand; `GIT_DIR=... ls` scopes the variable to `ls` and is not flagged. + * True when a subcommand may change which repository a LATER git or gh subcommand on the line acts + * on, without changing directory. Fails closed: the hook cannot model the shell's environment, so + * any of these counts — + * - any mention of a `GIT_*` or `GH_*` variable (`export GIT_DIR=...`, `GIT_WORK_TREE=x;`, + * `builtin export GIT_DIR=...`, `export GH_REPO=...`, `GIT_DIR+=...`, `unset GIT_DIR`, even + * `GIT_DIR=x ls`): git selects the repository from `GIT_DIR`, `GIT_WORK_TREE`, `GIT_COMMON_DIR` and + * others (the list grows with git releases), and gh from `GH_REPO` / `GH_HOST`; + * - a builtin whose effect on the environment is opaque (`eval`, `source`, `.`, `set`, `read`, + * `mapfile`, `printf -v`), after any `builtin` / `command` / control-word prefix; + * - a declaration builtin whose arguments expand (`export "$V"`, `declare -x $(...)`). + * A legitimate tracker commit has no reason to do any of this earlier on the same line. + * An assignment leading the git or gh command itself is detected by parseSubcommand. */ -function setsGitVariableForLaterCommands(subcommand: string): boolean { - const words = tokenizeShellWords(subcommand.trim()); +function mayRedirectLaterCommands(subcommand: string): boolean { + const text = subcommand.trim(); + if (/\b(?:GIT|GH)_[A-Za-z0-9_]*/.test(text)) { + return true; + } + const words = tokenizeShellWords(text); let idx = 0; - let hasGitAssignment = false; - while (idx < words.length && /^[a-zA-Z_][a-zA-Z0-9_]*=/.test(words[idx])) { - if (/^GIT_[A-Za-z0-9_]*=/.test(words[idx])) hasGitAssignment = true; + while (idx < words.length && /^[a-zA-Z_][a-zA-Z0-9_]*\+?=/.test(words[idx])) idx++; + while (idx < words.length && (COMMAND_RUNNING_PREFIXES.has(words[idx]) || /^-[a-zA-Z]+$/.test(words[idx]))) { idx++; } - if (idx === words.length) { - return hasGitAssignment; // bare assignment segment: the variables persist for later subcommands + if (idx >= words.length) { + return false; } const binary = words[idx].split(/[/\\]/).pop()?.toLowerCase() ?? ''; - if (!VARIABLE_DECLARATION_BUILTINS.has(binary)) { - return false; + if (ENVIRONMENT_OPAQUE_BUILTINS.has(binary)) { + return true; + } + if (binary === 'printf' && words.slice(idx + 1).includes('-v')) { + return true; } - return words.slice(idx + 1).some((w) => /^GIT_[A-Za-z0-9_]*(=|$)/.test(w)); + return VARIABLE_DECLARATION_BUILTINS.has(binary) && /[$`]/.test(text); } /** @@ -2130,9 +2146,10 @@ export async function evaluatePublicSurfaceCommand( const subshellCwdStack: (string | undefined)[] = []; const seenRedirectedFiles = new Set(); let hasPotentiallyMutatingPriorCommand = false; - // Set once an earlier subcommand on the line sets or exports a GIT_* variable, which can point a - // later bare git command at another repository without changing directory. - let hasPriorGitVariable = false; + // Set once an earlier subcommand on the line may have changed the environment (a GIT_* / GH_* + // variable, or an opaque builtin), which can point a later bare git or gh command at another + // repository without changing directory. + let hasPriorRedirect = false; for (const item of subcommands) { if (item.subshellDepthDeltaBefore) { @@ -2160,9 +2177,10 @@ export async function evaluatePublicSurfaceCommand( const rawSub = item.raw; const parsed = parseSubcommand(rawSub); - const isRedirectedByPriorGitVariable = hasPriorGitVariable && parsed.commandType.startsWith('git '); - if (setsGitVariableForLaterCommands(rawSub)) { - hasPriorGitVariable = true; + const isRedirectedByPriorCommand = + hasPriorRedirect && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); + if (mayRedirectLaterCommands(rawSub)) { + hasPriorRedirect = true; } // Track cd transitions: update virtualCwd across sequential operators (&&, ;, \n, ||) @@ -2295,7 +2313,7 @@ export async function evaluatePublicSurfaceCommand( parsed.commandType.includes('work-tree') || parsed.commandType.includes('GIT_DIR') || parsed.commandType.includes('wrapped') || - isRedirectedByPriorGitVariable + isRedirectedByPriorCommand ) { // Repository context is redirected or indeterminate — cwd remoteInfo cannot exempt it // Proceed directly to inspection and dynamic-input fail-closed check From df6755fd6db949972da40d5e09fe801ea47ffd7d Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:19:34 +0100 Subject: [PATCH 04/14] fix(security): allow-list what may precede a cwd-exempt git or gh command Each pass over the deny-list found another way to change the environment or the repository before a later git or gh command: control words (`if source ...; then`), `GIT_COMMON_DIR=...` leading the git command itself, `git remote set-url` / `git config`, `gh repo set-default`, untracked `pushd` / `popd`, function and alias definitions, `PATH=...`. Replace it with an allow-list. An earlier segment keeps a later git or gh command's directory-based exemption only when its command is `git` with add / status / diff / log / show, `gh ` with view / list / status / diff, `cd` (tracked by the loop), or one of echo, printf (without -v), test, [, true, false, :, pwd, ls, cat, with no leading variable assignment; brace-group braces are ignored. Any leading assignment on the git or gh command itself also removes its exemption. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 14 ++- .../hooks/public-surface-scrub.ts | 89 +++++++++++-------- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index f49988999..57d148473 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2310,6 +2310,14 @@ upstream_repo: apache/kafka 'set -a; git commit -m "Fix CVE-2026-1234"', 'printf -v X %s y; git commit -m "Fix CVE-2026-1234"', 'export "$V"; git commit -m "Fix CVE-2026-1234"', + 'export FOO=1 && git commit -m "Fix CVE-2026-1234"', + 'if source ./env.sh; then git commit -m "Fix CVE-2026-1234"; fi', + 'GIT_COMMON_DIR=/home/dev/upstream/.git git tag -a v1 -m "Fix CVE-2026-1234"', + 'git remote set-url origin https://github.com/apache/airflow.git && git commit -m "Fix CVE-2026-1234"', + 'git config core.worktree ../u && git commit -m "Fix CVE-2026-1234"', + 'pushd /home/dev/upstream && pushd /home/dev/airflow-s && popd && git commit -m "Fix CVE-2026-1234"', + 'f() { export GIT_DIR=../u/.git; }; f; git commit -m "Fix CVE-2026-1234"', + 'PATH=/tmp/x:$PATH; git commit -m "Fix CVE-2026-1234"', ]) { const res = await evaluate(cmd); assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); @@ -2320,6 +2328,9 @@ upstream_repo: apache/kafka for (const cmd of [ 'export GH_REPO=apache/airflow; gh issue create -t "CVE-2026-1234" -b "details"', 'GH_HOST=example.com; gh pr create -t "Fix CVE-2026-1234" -b "x"', + 'git remote set-url origin https://github.com/apache/airflow.git && gh issue create -t "CVE-2026-1234" -b "d"', + 'GH_REPO=apache/airflow gh issue create -t "CVE-2026-1234" -b "d"', + 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', ]) { const res = await evaluate(cmd); assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); @@ -2328,8 +2339,9 @@ upstream_repo: apache/kafka test('unrelated earlier commands keep the exemption', async () => { for (const cmd of [ - 'export FOO=1 && git commit -m "Fix CVE-2026-1234"', 'echo ready && git commit -m "Fix CVE-2026-1234"', + 'git add file.md && git commit -m "Fix CVE-2026-1234"', + 'git status && git commit -m "Fix CVE-2026-1234"', 'git commit -m "Fix CVE-2026-1234"', ]) { const res = await evaluate(cmd); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 315ae3030..3130f7bda 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2073,51 +2073,69 @@ const SAFE_READONLY_COMMANDS = new Set([ 'readonly', ]); -/** Shell builtins that assign or export variables for the rest of the command line. */ -const VARIABLE_DECLARATION_BUILTINS = new Set(['export', 'declare', 'typeset', 'readonly', 'local']); +/** + * Commands that cannot change the shell's environment or the repository's configuration for the + * commands after them on the line. `cd` changes only the directory, which the evaluation loop + * tracks itself (`pushd` / `popd` are not tracked, so they are not here). `git` is allowed only + * with a read-or-stage subcommand ({@link REPOSITORY_PRESERVING_GIT_SUBCOMMANDS}): others, such as + * `git remote set-url` or `git config`, can repoint a later command at another repository. + */ +const ENVIRONMENT_PRESERVING_COMMANDS = new Set([ + 'git', + 'echo', + 'printf', + 'test', + '[', + 'true', + 'false', + ':', + 'pwd', + 'ls', + 'cat', + 'cd', +]); -/** Shell builtins that can change the environment in ways the hook cannot see. */ -const ENVIRONMENT_OPAQUE_BUILTINS = new Set(['eval', 'source', '.', 'set', 'read', 'mapfile', 'readarray']); +const REPOSITORY_PRESERVING_GIT_SUBCOMMANDS = new Set(['add', 'status', 'diff', 'log', 'show']); -/** Prefixes that run the next word as the command (`builtin export ...`, `command -p eval ...`). */ -const COMMAND_RUNNING_PREFIXES = new Set(['builtin', 'command', 'then', 'do', 'else', '!']); +/** `gh ` verbs that only read (`gh issue view`); others, such as `gh repo set-default`, can repoint gh. */ +const READ_ONLY_GH_VERBS = new Set(['view', 'list', 'status', 'diff']); /** * True when a subcommand may change which repository a LATER git or gh subcommand on the line acts - * on, without changing directory. Fails closed: the hook cannot model the shell's environment, so - * any of these counts — - * - any mention of a `GIT_*` or `GH_*` variable (`export GIT_DIR=...`, `GIT_WORK_TREE=x;`, - * `builtin export GIT_DIR=...`, `export GH_REPO=...`, `GIT_DIR+=...`, `unset GIT_DIR`, even - * `GIT_DIR=x ls`): git selects the repository from `GIT_DIR`, `GIT_WORK_TREE`, `GIT_COMMON_DIR` and - * others (the list grows with git releases), and gh from `GH_REPO` / `GH_HOST`; - * - a builtin whose effect on the environment is opaque (`eval`, `source`, `.`, `set`, `read`, - * `mapfile`, `printf -v`), after any `builtin` / `command` / control-word prefix; - * - a declaration builtin whose arguments expand (`export "$V"`, `declare -x $(...)`). - * A legitimate tracker commit has no reason to do any of this earlier on the same line. - * An assignment leading the git or gh command itself is detected by parseSubcommand. + * on, without changing directory. An allow-list, failing closed: the shell has too many ways to + * change the environment (`export GIT_DIR=...` and its `builtin` / `command` / `declare` / `+=` + * forms, `export GH_REPO=...`, `eval`, `source`, function and alias definitions, `PATH=...`, ...) + * for a deny-list to stay complete. So only a segment whose command is on + * {@link ENVIRONMENT_PRESERVING_COMMANDS}, with no leading variable assignment (and no `printf -v`), + * keeps a later git or gh command's directory-based exemption. A legitimate tracker commit is run + * on its own or after one of those. */ function mayRedirectLaterCommands(subcommand: string): boolean { - const text = subcommand.trim(); - if (/\b(?:GIT|GH)_[A-Za-z0-9_]*/.test(text)) { - return true; - } - const words = tokenizeShellWords(text); - let idx = 0; - while (idx < words.length && /^[a-zA-Z_][a-zA-Z0-9_]*\+?=/.test(words[idx])) idx++; - while (idx < words.length && (COMMAND_RUNNING_PREFIXES.has(words[idx]) || /^-[a-zA-Z]+$/.test(words[idx]))) { - idx++; - } - if (idx >= words.length) { + // A brace group runs in the current shell; its braces change nothing themselves. + const words = tokenizeShellWords(subcommand.trim()).filter((w) => w !== '{' && w !== '}'); + if (words.length === 0) { return false; } - const binary = words[idx].split(/[/\\]/).pop()?.toLowerCase() ?? ''; - if (ENVIRONMENT_OPAQUE_BUILTINS.has(binary)) { - return true; + if (words[0] === 'gh') { + return !READ_ONLY_GH_VERBS.has(words[2] ?? ''); } - if (binary === 'printf' && words.slice(idx + 1).includes('-v')) { - return true; + if (!ENVIRONMENT_PRESERVING_COMMANDS.has(words[0])) { + return true; // includes a leading `VAR=value`, `export`, `builtin`, `eval`, `if`, `f() {`, ... } - return VARIABLE_DECLARATION_BUILTINS.has(binary) && /[$`]/.test(text); + if (words[0] === 'git') { + return !REPOSITORY_PRESERVING_GIT_SUBCOMMANDS.has(words[1] ?? ''); + } + return words[0] === 'printf' && words.slice(1).includes('-v'); +} + +/** + * True when the subcommand starts with a variable assignment (`GIT_COMMON_DIR=... git tag ...`): + * parseSubcommand recognises only `GIT_DIR` / `GIT_WORK_TREE` there, and git reads many more, so + * any leading assignment on a git or gh command removes its own directory-based exemption. + */ +function leadsWithAssignment(subcommand: string): boolean { + const first = tokenizeShellWords(subcommand.trim())[0] ?? ''; + return /^[A-Za-z_][A-Za-z0-9_]*\+?=/.test(first); } /** @@ -2178,7 +2196,8 @@ export async function evaluatePublicSurfaceCommand( const rawSub = item.raw; const parsed = parseSubcommand(rawSub); const isRedirectedByPriorCommand = - hasPriorRedirect && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); + (hasPriorRedirect || leadsWithAssignment(rawSub)) && + (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); if (mayRedirectLaterCommands(rawSub)) { hasPriorRedirect = true; } From 815b37762527cf9c3845aa65fca2059b0d35766b Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:23:21 +0100 Subject: [PATCH 05/14] fix(security): fail closed on side effects inside allow-listed commands An allow-listed command could still change repository state before a later git or gh command ran, after remote inspection had read the old state: through an output redirection (`printf ... > .git/config`), a command or process substitution (`echo $(git remote set-url ...)`), or `git diff --output=.git/config`. And an own-environment git command (`GIT_COMMON_DIR=... git tag`, `env ... git tag`, also after a prefix such as `then`) passed the first-word assignment check. An earlier segment with `>`, a backtick, `$(` or `<(` now counts as a redirect for later git and gh commands, as does an allow-listed git command with an `--output` / `-o` option. parseSubcommand records whether it unwrapped a leading assignment or an `env` wrapper, after any prefix, and such a git or gh command gets no directory-based exemption. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 9 ++++++ .../hooks/public-surface-scrub.ts | 29 ++++++++++++------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 57d148473..b16abdcbb 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2318,6 +2318,14 @@ upstream_repo: apache/kafka 'pushd /home/dev/upstream && pushd /home/dev/airflow-s && popd && git commit -m "Fix CVE-2026-1234"', 'f() { export GIT_DIR=../u/.git; }; f; git commit -m "Fix CVE-2026-1234"', 'PATH=/tmp/x:$PATH; git commit -m "Fix CVE-2026-1234"', + 'env GIT_COMMON_DIR=/home/dev/upstream/.git git tag -a v1 -m "Fix CVE-2026-1234"', + 'echo "[core] worktree = ../u" >> .git/config && git commit -m "Fix CVE-2026-1234"', + 'echo $(git remote set-url origin https://github.com/apache/airflow.git) && git commit -m "Fix CVE-2026-1234"', + 'echo `git config core.worktree ../u` && git commit -m "Fix CVE-2026-1234"', + 'cat <(git config core.worktree ../u) && git commit -m "Fix CVE-2026-1234"', + 'git diff --output=.git/config HEAD~1 && git commit -m "Fix CVE-2026-1234"', + 'if true; then GIT_COMMON_DIR=/home/dev/upstream/.git git tag -a v1 -m "Fix CVE-2026-1234"; fi', + 'env -i GIT_COMMON_DIR=/home/dev/upstream/.git git tag -a v1 -m "Fix CVE-2026-1234"', ]) { const res = await evaluate(cmd); assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); @@ -2331,6 +2339,7 @@ upstream_repo: apache/kafka 'git remote set-url origin https://github.com/apache/airflow.git && gh issue create -t "CVE-2026-1234" -b "d"', 'GH_REPO=apache/airflow gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', + "printf '[remote \"origin\"]\\nurl = https://github.com/apache/airflow.git\\n' > .git/config && gh issue create -t 'CVE-2026-1234' -b d", ]) { const res = await evaluate(cmd); assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 3130f7bda..b02710a57 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -673,6 +673,8 @@ export interface ParsedSubcommand { isGraphqlMutation?: boolean; isLiteralUnknownEndpoint?: boolean; hasGitDirOrWorkTree?: boolean; + /** The command runs with environment set for it: a leading `NAME=value` or an `env` wrapper, after any prefix. */ + setsOwnEnvironment?: boolean; } /** @@ -1131,6 +1133,7 @@ export function parseSubcommand(subcommand: string): ParsedSubcommand { cmdIdx++; } } else if (t === 'env') { + result.setsOwnEnvironment = true; cmdIdx++; while (cmdIdx < tokens.length && tokens[cmdIdx].startsWith('-')) { const envTok = tokens[cmdIdx]; @@ -1154,6 +1157,7 @@ export function parseSubcommand(subcommand: string): ParsedSubcommand { } else if (/^[a-zA-Z_][a-zA-Z0-9_]*=/.test(t)) { const eq = t.indexOf('='); leadingEnv[t.slice(0, eq)] = t.slice(eq + 1); + result.setsOwnEnvironment = true; cmdIdx++; } else { break; @@ -2111,6 +2115,12 @@ const READ_ONLY_GH_VERBS = new Set(['view', 'list', 'status', 'diff']); * on its own or after one of those. */ function mayRedirectLaterCommands(subcommand: string): boolean { + // Any output redirection, command substitution or process substitution can run or write + // something (`printf ... > .git/config`, `echo $(git remote set-url ...)`) before the later + // command runs, while remote inspection already read the old state: fail closed on all of them. + if (/[>`]|\$\(|<\(/.test(subcommand)) { + return true; + } // A brace group runs in the current shell; its braces change nothing themselves. const words = tokenizeShellWords(subcommand.trim()).filter((w) => w !== '{' && w !== '}'); if (words.length === 0) { @@ -2123,20 +2133,15 @@ function mayRedirectLaterCommands(subcommand: string): boolean { return true; // includes a leading `VAR=value`, `export`, `builtin`, `eval`, `if`, `f() {`, ... } if (words[0] === 'git') { - return !REPOSITORY_PRESERVING_GIT_SUBCOMMANDS.has(words[1] ?? ''); + // `git diff --output=.git/config` writes a file; any `--output` / `-o` option disqualifies. + return ( + !REPOSITORY_PRESERVING_GIT_SUBCOMMANDS.has(words[1] ?? '') || + words.slice(2).some((w) => w.startsWith('--output') || w === '-o') + ); } return words[0] === 'printf' && words.slice(1).includes('-v'); } -/** - * True when the subcommand starts with a variable assignment (`GIT_COMMON_DIR=... git tag ...`): - * parseSubcommand recognises only `GIT_DIR` / `GIT_WORK_TREE` there, and git reads many more, so - * any leading assignment on a git or gh command removes its own directory-based exemption. - */ -function leadsWithAssignment(subcommand: string): boolean { - const first = tokenizeShellWords(subcommand.trim())[0] ?? ''; - return /^[A-Za-z_][A-Za-z0-9_]*\+?=/.test(first); -} /** * Evaluates whether a public-surface command violates embargo terms. @@ -2196,7 +2201,9 @@ export async function evaluatePublicSurfaceCommand( const rawSub = item.raw; const parsed = parseSubcommand(rawSub); const isRedirectedByPriorCommand = - (hasPriorRedirect || leadsWithAssignment(rawSub)) && + // An own-environment git or gh command (`GIT_COMMON_DIR=... git tag`, `env ... git tag`, after + // any prefix): parseSubcommand acts only on GIT_DIR / GIT_WORK_TREE, and git reads many more. + (hasPriorRedirect || parsed.setsOwnEnvironment === true) && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); if (mayRedirectLaterCommands(rawSub)) { hasPriorRedirect = true; From 1155e1de86566be0c62153ba836e6813e436dca8 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:30:10 +0100 Subject: [PATCH 06/14] fix(security): honour inherited repository variables; drop printf from the allow-list A session started with GH_REPO, GIT_DIR or another repository-selecting variable in its environment repoints every bare git or gh command, but the directory-based exemption only looked at the directory. register() now reads those variables (literal $.env.get names, as the mod API requires) and any of them removes the exemption for the whole line. `printf -vNAME` assigns a variable (bash accepts the option glued to the name), so printf can change the environment and leaves the allow-list; echo covers the harmless case. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 15 +++++ .../hooks/public-surface-scrub.ts | 58 +++++++++++++++++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index b16abdcbb..124c8ab98 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2338,6 +2338,7 @@ upstream_repo: apache/kafka 'GH_HOST=example.com; gh pr create -t "Fix CVE-2026-1234" -b "x"', 'git remote set-url origin https://github.com/apache/airflow.git && gh issue create -t "CVE-2026-1234" -b "d"', 'GH_REPO=apache/airflow gh issue create -t "CVE-2026-1234" -b "d"', + 'printf -vGH_REPO %s apache/airflow; gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', "printf '[remote \"origin\"]\\nurl = https://github.com/apache/airflow.git\\n' > .git/config && gh issue create -t 'CVE-2026-1234' -b d", ]) { @@ -2346,6 +2347,20 @@ upstream_repo: apache/kafka } }); + test('a repository-selecting variable in the session environment removes the exemption', async () => { + for (const env of [{ GH_REPO: 'apache/airflow' }, { GIT_DIR: '/home/dev/upstream/.git' }]) { + for (const cmd of ['gh issue create -t "CVE-2026-1234" -b "d"', 'git commit -m "Fix CVE-2026-1234"']) { + const res = await evaluatePublicSurfaceCommand(cmd, { + cwd: '/home/dev/airflow-s', + projectConfig: config, + readFile, + env, + }); + assertStrictEqual(res.blocked, true, `Expected block for ${cmd} with ${JSON.stringify(env)}`); + } + } + }); + test('unrelated earlier commands keep the exemption', async () => { for (const cmd of [ 'echo ready && git commit -m "Fix CVE-2026-1234"', diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index b02710a57..0c3848ae2 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2087,7 +2087,6 @@ const SAFE_READONLY_COMMANDS = new Set([ const ENVIRONMENT_PRESERVING_COMMANDS = new Set([ 'git', 'echo', - 'printf', 'test', '[', 'true', @@ -2101,6 +2100,26 @@ const ENVIRONMENT_PRESERVING_COMMANDS = new Set([ const REPOSITORY_PRESERVING_GIT_SUBCOMMANDS = new Set(['add', 'status', 'diff', 'log', 'show']); +/** + * Environment variables that choose which repository git or gh acts on. Set in the session's own + * environment, any of them removes the directory-based exemption for every bare git / gh command. + */ +const REPOSITORY_SELECTING_ENV_VARS = [ + 'GH_REPO', + 'GH_HOST', + 'GIT_DIR', + 'GIT_WORK_TREE', + 'GIT_COMMON_DIR', + 'GIT_INDEX_FILE', + 'GIT_OBJECT_DIRECTORY', + 'GIT_ALTERNATE_OBJECT_DIRECTORIES', + 'GIT_NAMESPACE', + 'GIT_CEILING_DIRECTORIES', + 'GIT_CONFIG_GLOBAL', + 'GIT_CONFIG_SYSTEM', + 'GIT_CONFIG_COUNT', +]; + /** `gh ` verbs that only read (`gh issue view`); others, such as `gh repo set-default`, can repoint gh. */ const READ_ONLY_GH_VERBS = new Set(['view', 'list', 'status', 'diff']); @@ -2110,7 +2129,7 @@ const READ_ONLY_GH_VERBS = new Set(['view', 'list', 'status', 'diff']); * change the environment (`export GIT_DIR=...` and its `builtin` / `command` / `declare` / `+=` * forms, `export GH_REPO=...`, `eval`, `source`, function and alias definitions, `PATH=...`, ...) * for a deny-list to stay complete. So only a segment whose command is on - * {@link ENVIRONMENT_PRESERVING_COMMANDS}, with no leading variable assignment (and no `printf -v`), + * {@link ENVIRONMENT_PRESERVING_COMMANDS}, with no leading variable assignment, * keeps a later git or gh command's directory-based exemption. A legitimate tracker commit is run * on its own or after one of those. */ @@ -2139,7 +2158,7 @@ function mayRedirectLaterCommands(subcommand: string): boolean { words.slice(2).some((w) => w.startsWith('--output') || w === '-o') ); } - return words[0] === 'printf' && words.slice(1).includes('-v'); + return false; } @@ -2172,7 +2191,7 @@ export async function evaluatePublicSurfaceCommand( // Set once an earlier subcommand on the line may have changed the environment (a GIT_* / GH_* // variable, or an opaque builtin), which can point a later bare git or gh command at another // repository without changing directory. - let hasPriorRedirect = false; + let hasPriorRedirect = REPOSITORY_SELECTING_ENV_VARS.some((name) => Boolean(options?.env?.[name])); for (const item of subcommands) { if (item.subshellDepthDeltaBefore) { @@ -2495,7 +2514,36 @@ export function register(on: any): void { } catch { // $.env may be absent in mock harnesses } - const hookEnv = allowBypass ? { MAGPIE_ALLOW_EMBARGO_TERMS: allowBypass } : {}; + const hookEnv: { [key: string]: string | undefined } = allowBypass + ? { MAGPIE_ALLOW_EMBARGO_TERMS: allowBypass } + : {}; + // A repository-selecting variable inherited by the session (Claude launched with GH_REPO or + // GIT_DIR set) repoints every bare git / gh command, whatever the directory says. + // The mod API needs a literal name at every $.env.get call, so each variable is spelled out; + // keep this list in step with REPOSITORY_SELECTING_ENV_VARS. + const inherited: [string, () => Promise][] = [ + ['GH_REPO', () => $.env.get('GH_REPO')], + ['GH_HOST', () => $.env.get('GH_HOST')], + ['GIT_DIR', () => $.env.get('GIT_DIR')], + ['GIT_WORK_TREE', () => $.env.get('GIT_WORK_TREE')], + ['GIT_COMMON_DIR', () => $.env.get('GIT_COMMON_DIR')], + ['GIT_INDEX_FILE', () => $.env.get('GIT_INDEX_FILE')], + ['GIT_OBJECT_DIRECTORY', () => $.env.get('GIT_OBJECT_DIRECTORY')], + ['GIT_ALTERNATE_OBJECT_DIRECTORIES', () => $.env.get('GIT_ALTERNATE_OBJECT_DIRECTORIES')], + ['GIT_NAMESPACE', () => $.env.get('GIT_NAMESPACE')], + ['GIT_CEILING_DIRECTORIES', () => $.env.get('GIT_CEILING_DIRECTORIES')], + ['GIT_CONFIG_GLOBAL', () => $.env.get('GIT_CONFIG_GLOBAL')], + ['GIT_CONFIG_SYSTEM', () => $.env.get('GIT_CONFIG_SYSTEM')], + ['GIT_CONFIG_COUNT', () => $.env.get('GIT_CONFIG_COUNT')], + ]; + for (const [name, read] of inherited) { + try { + const value = await read(); + if (value) hookEnv[name] = value; + } catch { + // $.env may be absent in mock harnesses + } + } const evaluation = await evaluatePublicSurfaceCommand(command, { cwd: initialCwd, isSessionRootFallback, From c7ba61b1f69154cbc2fde368fc3ae5cc574d4919 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:34:34 +0100 Subject: [PATCH 07/14] fix(security): treat `||` and any expansion before git / gh as a redirect The directory tracking follows every `cd`, but `||` runs what follows only on failure, so `cd /upstream || cd /tracker; gh ...` left the tracked directory at the tracker while the shell stayed upstream. And an allow-listed echo could assign through `${GH_REPO:=...}`. A segment followed by `||`, or containing any `$` expansion, now counts as a redirect for later git and gh commands. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 2 ++ .../magpie-security/hooks/public-surface-scrub.ts | 13 ++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 124c8ab98..a0c248697 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2339,6 +2339,8 @@ upstream_repo: apache/kafka 'git remote set-url origin https://github.com/apache/airflow.git && gh issue create -t "CVE-2026-1234" -b "d"', 'GH_REPO=apache/airflow gh issue create -t "CVE-2026-1234" -b "d"', 'printf -vGH_REPO %s apache/airflow; gh issue create -t "CVE-2026-1234" -b "d"', + 'cd /home/dev/upstream || cd /home/dev/airflow-s; gh issue create -t "CVE-2026-1234" -b "d"', + 'echo "${GH_REPO:=apache/airflow}"; gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', "printf '[remote \"origin\"]\\nurl = https://github.com/apache/airflow.git\\n' > .git/config && gh issue create -t 'CVE-2026-1234' -b d", ]) { diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 0c3848ae2..1ab327367 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2134,10 +2134,11 @@ const READ_ONLY_GH_VERBS = new Set(['view', 'list', 'status', 'diff']); * on its own or after one of those. */ function mayRedirectLaterCommands(subcommand: string): boolean { - // Any output redirection, command substitution or process substitution can run or write - // something (`printf ... > .git/config`, `echo $(git remote set-url ...)`) before the later - // command runs, while remote inspection already read the old state: fail closed on all of them. - if (/[>`]|\$\(|<\(/.test(subcommand)) { + // Any output redirection, expansion, command substitution or process substitution can run, write + // or assign something (`printf ... > .git/config`, `echo $(git remote set-url ...)`, + // `echo ${GH_REPO:=...}`) before the later command runs, while remote inspection already read + // the old state: fail closed on all of them. + if (/[>`$]|<\(/.test(subcommand)) { return true; } // A brace group runs in the current shell; its braces change nothing themselves. @@ -2224,7 +2225,9 @@ export async function evaluatePublicSurfaceCommand( // any prefix): parseSubcommand acts only on GIT_DIR / GIT_WORK_TREE, and git reads many more. (hasPriorRedirect || parsed.setsOwnEnvironment === true) && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); - if (mayRedirectLaterCommands(rawSub)) { + // `||` runs what follows only on failure, so the directory tracking (which follows every `cd`) + // can disagree with the shell (`cd /upstream || cd /tracker`): treat it as a redirect. + if (mayRedirectLaterCommands(rawSub) || item.separatorAfter === '||') { hasPriorRedirect = true; } From fc539d63ec2aa1c9009ae3e659a24359b9537f80 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:36:45 +0100 Subject: [PATCH 08/14] fix(security): treat a conditional cd before git / gh as a redirect A `cd` after `&&` or `||` runs only conditionally (`false && cd /tracker; gh ...`), but the directory tracking followed it anyway, so a later command could take the tracker exemption from a directory the shell never entered. Such a `cd` now counts as a redirect for later git and gh commands; a leading or `;`-separated `cd` is still tracked. Generated-by: Claude Opus 5 --- .../magpie-security/hooks/public-surface-scrub.test.ts | 2 ++ plugins/magpie-security/hooks/public-surface-scrub.ts | 10 +++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index a0c248697..8dbf5d776 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2340,6 +2340,8 @@ upstream_repo: apache/kafka 'GH_REPO=apache/airflow gh issue create -t "CVE-2026-1234" -b "d"', 'printf -vGH_REPO %s apache/airflow; gh issue create -t "CVE-2026-1234" -b "d"', 'cd /home/dev/upstream || cd /home/dev/airflow-s; gh issue create -t "CVE-2026-1234" -b "d"', + 'false && cd /home/dev/airflow-s; gh issue create -t "CVE-2026-1234" -b "d"', + 'test -d /nope && cd /home/dev/airflow-s; git commit -m "Fix CVE-2026-1234"', 'echo "${GH_REPO:=apache/airflow}"; gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', "printf '[remote \"origin\"]\\nurl = https://github.com/apache/airflow.git\\n' > .git/config && gh issue create -t 'CVE-2026-1234' -b d", diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 1ab327367..1ef8b7e8d 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2194,6 +2194,7 @@ export async function evaluatePublicSurfaceCommand( // repository without changing directory. let hasPriorRedirect = REPOSITORY_SELECTING_ENV_VARS.some((name) => Boolean(options?.env?.[name])); + let prevSeparator: string | undefined; for (const item of subcommands) { if (item.subshellDepthDeltaBefore) { for (let d = 0; d < item.subshellDepthDeltaBefore; d++) { @@ -2225,11 +2226,14 @@ export async function evaluatePublicSurfaceCommand( // any prefix): parseSubcommand acts only on GIT_DIR / GIT_WORK_TREE, and git reads many more. (hasPriorRedirect || parsed.setsOwnEnvironment === true) && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); - // `||` runs what follows only on failure, so the directory tracking (which follows every `cd`) - // can disagree with the shell (`cd /upstream || cd /tracker`): treat it as a redirect. - if (mayRedirectLaterCommands(rawSub) || item.separatorAfter === '||') { + // The directory tracking follows every `cd`, but a `cd` after `&&` or `||` runs only + // conditionally (`false && cd /tracker`, `cd /upstream || cd /tracker`), so the tracked directory + // can disagree with the shell: a conditional `cd`, or anything followed by `||`, is a redirect. + const isConditionalCd = Boolean(parsed.cdDir) && (prevSeparator === '&&' || prevSeparator === '||'); + if (mayRedirectLaterCommands(rawSub) || item.separatorAfter === '||' || isConditionalCd) { hasPriorRedirect = true; } + prevSeparator = item.separatorAfter; // Track cd transitions: update virtualCwd across sequential operators (&&, ;, \n, ||) // Only exclude pipeline (|) and background (&) which run in subshells From 3176aa57fc80c62d885d83b6f188f1e6a38c64dd Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:39:26 +0100 Subject: [PATCH 09/14] fix(security): treat any cd after an earlier && or || as a redirect Checking only the separator right before a `cd` missed a conditional group: in `false && { true; cd /tracker; }`, the `cd` follows `;` inside the braces but still runs only if `false` succeeds. Once an `&&` or `||` has appeared on the line, every later `cd` now counts as a redirect for later git and gh commands. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 1 + .../magpie-security/hooks/public-surface-scrub.ts | 15 +++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 8dbf5d776..e85a351ce 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2341,6 +2341,7 @@ upstream_repo: apache/kafka 'printf -vGH_REPO %s apache/airflow; gh issue create -t "CVE-2026-1234" -b "d"', 'cd /home/dev/upstream || cd /home/dev/airflow-s; gh issue create -t "CVE-2026-1234" -b "d"', 'false && cd /home/dev/airflow-s; gh issue create -t "CVE-2026-1234" -b "d"', + 'false && { true; cd /home/dev/airflow-s; }; gh issue create -t "CVE-2026-1234" -b "d"', 'test -d /nope && cd /home/dev/airflow-s; git commit -m "Fix CVE-2026-1234"', 'echo "${GH_REPO:=apache/airflow}"; gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 1ef8b7e8d..c6f56accf 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2194,7 +2194,7 @@ export async function evaluatePublicSurfaceCommand( // repository without changing directory. let hasPriorRedirect = REPOSITORY_SELECTING_ENV_VARS.some((name) => Boolean(options?.env?.[name])); - let prevSeparator: string | undefined; + let hasSeenConditional = false; for (const item of subcommands) { if (item.subshellDepthDeltaBefore) { for (let d = 0; d < item.subshellDepthDeltaBefore; d++) { @@ -2226,14 +2226,17 @@ export async function evaluatePublicSurfaceCommand( // any prefix): parseSubcommand acts only on GIT_DIR / GIT_WORK_TREE, and git reads many more. (hasPriorRedirect || parsed.setsOwnEnvironment === true) && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); - // The directory tracking follows every `cd`, but a `cd` after `&&` or `||` runs only - // conditionally (`false && cd /tracker`, `cd /upstream || cd /tracker`), so the tracked directory - // can disagree with the shell: a conditional `cd`, or anything followed by `||`, is a redirect. - const isConditionalCd = Boolean(parsed.cdDir) && (prevSeparator === '&&' || prevSeparator === '||'); + // The directory tracking follows every `cd`, but once an `&&` or `||` has appeared on the line a + // later `cd` may run only conditionally (`false && cd /tracker`, `cd /upstream || cd /tracker`, + // `false && { true; cd /tracker; }`), so the tracked directory can disagree with the shell: + // any such `cd`, or anything followed by `||`, is a redirect. + const isConditionalCd = Boolean(parsed.cdDir) && hasSeenConditional; if (mayRedirectLaterCommands(rawSub) || item.separatorAfter === '||' || isConditionalCd) { hasPriorRedirect = true; } - prevSeparator = item.separatorAfter; + if (item.separatorAfter === '&&' || item.separatorAfter === '||') { + hasSeenConditional = true; + } // Track cd transitions: update virtualCwd across sequential operators (&&, ;, \n, ||) // Only exclude pipeline (|) and background (&) which run in subshells From 12e032c7689361651a979e744a34c002246b47e5 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:43:08 +0100 Subject: [PATCH 10/14] test(security): cover cd inside if / while / for / case before git or gh A `cd` inside a compound command runs only conditionally. The allow-list already treats a segment starting with `if`, `while`, `for`, `case` or another compound keyword as a redirect for every later command, so these lines go through the term scan; pin that with tests. Generated-by: Claude Opus 5 --- plugins/magpie-security/hooks/public-surface-scrub.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index e85a351ce..ef933bc4b 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2342,6 +2342,11 @@ upstream_repo: apache/kafka 'cd /home/dev/upstream || cd /home/dev/airflow-s; gh issue create -t "CVE-2026-1234" -b "d"', 'false && cd /home/dev/airflow-s; gh issue create -t "CVE-2026-1234" -b "d"', 'false && { true; cd /home/dev/airflow-s; }; gh issue create -t "CVE-2026-1234" -b "d"', + 'if false; then cd /home/dev/airflow-s; fi; gh issue create -t "CVE-2026-1234" -b "d"', + 'if true; then :; else cd /home/dev/airflow-s; fi; git commit -m "Fix CVE-2026-1234"', + 'while false; do cd /home/dev/airflow-s; done; gh issue create -t "CVE-2026-1234" -b "d"', + 'for d in; do cd /home/dev/airflow-s; done; git commit -m "Fix CVE-2026-1234"', + 'case x in y) cd /home/dev/airflow-s;; esac; gh issue create -t "CVE-2026-1234" -b "d"', 'test -d /nope && cd /home/dev/airflow-s; git commit -m "Fix CVE-2026-1234"', 'echo "${GH_REPO:=apache/airflow}"; gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', From 56e36478b6642c8c6d521681e845f1828929f24a Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:45:00 +0100 Subject: [PATCH 11/14] fix(security): no exemption for in-command substitution or a malformed cd A command substitution inside the git or gh command itself (`gh issue create -t "$(git remote set-url ...)"`) runs before the command resolves its repository, but only earlier segments were checked. Such a command now gets no directory-based exemption and goes through the dynamic-input check. Only a plain `cd ` is tracked: `cd a b` fails and leaves the shell where it was, and options or `cd -` move it somewhere the tracking does not model, so any other cd form counts as a redirect. Generated-by: Claude Opus 5 --- .../magpie-security/hooks/public-surface-scrub.test.ts | 5 +++++ plugins/magpie-security/hooks/public-surface-scrub.ts | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index ef933bc4b..337c167d4 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2347,6 +2347,11 @@ upstream_repo: apache/kafka 'while false; do cd /home/dev/airflow-s; done; gh issue create -t "CVE-2026-1234" -b "d"', 'for d in; do cd /home/dev/airflow-s; done; git commit -m "Fix CVE-2026-1234"', 'case x in y) cd /home/dev/airflow-s;; esac; gh issue create -t "CVE-2026-1234" -b "d"', + 'gh issue create -t "$(git remote set-url origin https://github.com/apache/airflow.git; echo CVE-2026-1234)" -b d', + 'git commit -m "`git config core.worktree ../u` Fix CVE-2026-1234"', + 'cd /home/dev/airflow-s unused extra; gh issue create -t "CVE-2026-1234" -b "d"', + 'cd -; git commit -m "Fix CVE-2026-1234"', + 'cd -P /home/dev/airflow-s; git commit -m "Fix CVE-2026-1234"', 'test -d /nope && cd /home/dev/airflow-s; git commit -m "Fix CVE-2026-1234"', 'echo "${GH_REPO:=apache/airflow}"; gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index c6f56accf..c58fc9f12 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2152,6 +2152,12 @@ function mayRedirectLaterCommands(subcommand: string): boolean { if (!ENVIRONMENT_PRESERVING_COMMANDS.has(words[0])) { return true; // includes a leading `VAR=value`, `export`, `builtin`, `eval`, `if`, `f() {`, ... } + if (words[0] === 'cd') { + // Only a plain `cd ` is tracked as a directory change; `cd a b` fails (too many operands) + // and leaves the shell where it was, and options or `cd -` move it somewhere the tracking + // does not model. + return words.length !== 2 || words[1].startsWith('-'); + } if (words[0] === 'git') { // `git diff --output=.git/config` writes a file; any `--output` / `-o` option disqualifies. return ( @@ -2224,7 +2230,9 @@ export async function evaluatePublicSurfaceCommand( const isRedirectedByPriorCommand = // An own-environment git or gh command (`GIT_COMMON_DIR=... git tag`, `env ... git tag`, after // any prefix): parseSubcommand acts only on GIT_DIR / GIT_WORK_TREE, and git reads many more. - (hasPriorRedirect || parsed.setsOwnEnvironment === true) && + // A substitution inside the git or gh command itself (`gh issue create -t "$(git remote + // set-url ...)"`) runs before the command resolves its repository. + (hasPriorRedirect || parsed.setsOwnEnvironment === true || /`|\$\(|<\(/.test(rawSub)) && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); // The directory tracking follows every `cd`, but once an `&&` or `||` has appeared on the line a // later `cd` may run only conditionally (`false && cd /tracker`, `cd /upstream || cd /tracker`, From 4ba2254535b66cc997613e67f5c7da56f3fe4ae0 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 01:47:07 +0100 Subject: [PATCH 12/14] fix(security): same fail-closed expansion check inside the git or gh command The in-command check caught `$(`, backticks and `<(` but not a plain `$` expansion (`"${GH_REPO:=...}"` assigns before gh resolves its repository) or `>(`. Use the same set as the earlier-segment check: any `$`, backtick, `<(` or `>(` removes the directory-based exemption. Generated-by: Claude Opus 5 --- plugins/magpie-security/hooks/public-surface-scrub.test.ts | 2 ++ plugins/magpie-security/hooks/public-surface-scrub.ts | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 337c167d4..ea4aa78aa 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2349,6 +2349,8 @@ upstream_repo: apache/kafka 'case x in y) cd /home/dev/airflow-s;; esac; gh issue create -t "CVE-2026-1234" -b "d"', 'gh issue create -t "$(git remote set-url origin https://github.com/apache/airflow.git; echo CVE-2026-1234)" -b d', 'git commit -m "`git config core.worktree ../u` Fix CVE-2026-1234"', + 'gh issue create -t "${GH_REPO:=apache/airflow} CVE-2026-1234" -b d', + 'gh issue create -t "CVE-2026-1234" -b d --body-file >(git remote set-url origin https://github.com/apache/airflow.git)', 'cd /home/dev/airflow-s unused extra; gh issue create -t "CVE-2026-1234" -b "d"', 'cd -; git commit -m "Fix CVE-2026-1234"', 'cd -P /home/dev/airflow-s; git commit -m "Fix CVE-2026-1234"', diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index c58fc9f12..f9f7567bb 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2230,9 +2230,10 @@ export async function evaluatePublicSurfaceCommand( const isRedirectedByPriorCommand = // An own-environment git or gh command (`GIT_COMMON_DIR=... git tag`, `env ... git tag`, after // any prefix): parseSubcommand acts only on GIT_DIR / GIT_WORK_TREE, and git reads many more. - // A substitution inside the git or gh command itself (`gh issue create -t "$(git remote - // set-url ...)"`) runs before the command resolves its repository. - (hasPriorRedirect || parsed.setsOwnEnvironment === true || /`|\$\(|<\(/.test(rawSub)) && + // An expansion or substitution inside the git or gh command itself (`gh issue create -t + // "$(git remote set-url ...)"`, `"${GH_REPO:=...}"`, `>(...)`) runs before the command + // resolves its repository; same fail-closed set as mayRedirectLaterCommands. + (hasPriorRedirect || parsed.setsOwnEnvironment === true || /[`$]|[<>]\(/.test(rawSub)) && (parsed.commandType.startsWith('git ') || parsed.commandType.startsWith('gh ')); // The directory tracking follows every `cd`, but once an `&&` or `||` has appeared on the line a // later `cd` may run only conditionally (`false && cd /tracker`, `cd /upstream || cd /tracker`, From bdba949bc8aa025b84185c8c94abe9c205644eba Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 14:30:45 +0100 Subject: [PATCH 13/14] fix(security): no cwd exemption under CDPATH or for bash -c / eval With CDPATH inherited, `cd name` searches the CDPATH entries before the current directory, so the tracked directory can name a tracker while the shell lands in an upstream checkout. A relative `cd` that CDPATH can redirect (not `/...` or `./...`) now counts as a redirect, and the hook reads CDPATH from the session environment. `bash -c '...'` and `eval '...'` invoking git or gh can cd or set GIT_DIR inside the string, but their command type did not match the step-5 redirect check, so the tracker exemption skipped the dynamic-input block. They now get no directory-based exemption. Generated-by: Claude Opus 5 --- .../hooks/public-surface-scrub.test.ts | 23 +++++++++++++++++++ .../hooks/public-surface-scrub.ts | 9 +++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index ea4aa78aa..8a5543394 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2378,6 +2378,29 @@ upstream_repo: apache/kafka } }); + test('an inherited CDPATH makes a relative cd a redirect', async () => { + const env = { CDPATH: '/home/dev/public' }; + for (const [cmd, blocked] of [ + ['cd child && gh issue create -t "CVE-2026-1234" -b "d"', true], + ['cd ./child && gh issue create -t "CVE-2026-1234" -b "d"', false], + ['cd /home/dev/airflow-s && git commit -m "Fix CVE-2026-1234"', false], + ] as const) { + const res = await evaluatePublicSurfaceCommand(cmd, { cwd: '/home/dev/airflow-s', projectConfig: config, readFile, env }); + assertStrictEqual(res.blocked, blocked, `Expected blocked=${blocked} for ${cmd}: ${JSON.stringify(res)}`); + } + }); + + test('bash -c and eval invoking git or gh get no cwd exemption', async () => { + for (const cmd of [ + `bash -c 'cd /home/dev/upstream; git commit -m "CVE-2026-1234"'`, + `sh -c 'gh issue create -t "CVE-2026-1234" -b d'`, + `eval 'cd /home/dev/upstream; git commit -m "CVE-2026-1234"'`, + ]) { + const res = await evaluate(cmd); + assertStrictEqual(res.blocked, true, `Expected block for: ${cmd}`); + } + }); + test('unrelated earlier commands keep the exemption', async () => { for (const cmd of [ 'echo ready && git commit -m "Fix CVE-2026-1234"', diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index f9f7567bb..7eb8a7462 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2240,7 +2240,10 @@ export async function evaluatePublicSurfaceCommand( // `false && { true; cd /tracker; }`), so the tracked directory can disagree with the shell: // any such `cd`, or anything followed by `||`, is a redirect. const isConditionalCd = Boolean(parsed.cdDir) && hasSeenConditional; - if (mayRedirectLaterCommands(rawSub) || item.separatorAfter === '||' || isConditionalCd) { + // With CDPATH set, `cd name` searches the CDPATH entries before the current directory, so the + // tracked directory can name a different checkout than the one the shell lands in. + const isCdPathCd = Boolean(parsed.cdDir) && Boolean(options?.env?.CDPATH) && !/^[/.]/.test(parsed.cdDir ?? ''); + if (mayRedirectLaterCommands(rawSub) || item.separatorAfter === '||' || isConditionalCd || isCdPathCd) { hasPriorRedirect = true; } if (item.separatorAfter === '&&' || item.separatorAfter === '||') { @@ -2377,6 +2380,8 @@ export async function evaluatePublicSurfaceCommand( parsed.commandType.includes('work-tree') || parsed.commandType.includes('GIT_DIR') || parsed.commandType.includes('wrapped') || + // `bash -c '...'` and `eval '...'` can cd or set GIT_DIR inside the string before git / gh runs + parsed.commandType.includes('invoking git/gh') || isRedirectedByPriorCommand ) { // Repository context is redirected or indeterminate — cwd remoteInfo cannot exempt it @@ -2554,6 +2559,8 @@ export function register(on: any): void { ['GIT_CONFIG_GLOBAL', () => $.env.get('GIT_CONFIG_GLOBAL')], ['GIT_CONFIG_SYSTEM', () => $.env.get('GIT_CONFIG_SYSTEM')], ['GIT_CONFIG_COUNT', () => $.env.get('GIT_CONFIG_COUNT')], + // Not repository-selecting by itself, but it moves a relative `cd` (see isCdPathCd). + ['CDPATH', () => $.env.get('CDPATH')], ]; for (const [name, read] of inherited) { try { From 91bbe4eae1447fe6538203682d490879d19a3521 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 14:32:10 +0100 Subject: [PATCH 14/14] fix(security): match bash's rule for when cd skips CDPATH Bash skips the CDPATH search only for `/...`, `.`, `..`, `./...` and `../...`; a name such as `.hidden` is still searched, so it counts as a CDPATH-redirectable cd too. Tests also pin that an earlier `CDPATH=...` or `export CDPATH=...` on the line already removes the exemption through the allow-list. Generated-by: Claude Opus 5 --- plugins/magpie-security/hooks/public-surface-scrub.test.ts | 3 +++ plugins/magpie-security/hooks/public-surface-scrub.ts | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 8a5543394..c244749d9 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -2357,6 +2357,8 @@ upstream_repo: apache/kafka 'test -d /nope && cd /home/dev/airflow-s; git commit -m "Fix CVE-2026-1234"', 'echo "${GH_REPO:=apache/airflow}"; gh issue create -t "CVE-2026-1234" -b "d"', 'gh repo set-default apache/airflow && gh issue create -t "CVE-2026-1234" -b "d"', + 'CDPATH=/home/dev/public; cd child; gh issue create -t "CVE-2026-1234" -b "d"', + 'export CDPATH=/home/dev/public; cd child; git commit -m "Fix CVE-2026-1234"', "printf '[remote \"origin\"]\\nurl = https://github.com/apache/airflow.git\\n' > .git/config && gh issue create -t 'CVE-2026-1234' -b d", ]) { const res = await evaluate(cmd); @@ -2382,6 +2384,7 @@ upstream_repo: apache/kafka const env = { CDPATH: '/home/dev/public' }; for (const [cmd, blocked] of [ ['cd child && gh issue create -t "CVE-2026-1234" -b "d"', true], + ['cd .hidden && gh issue create -t "CVE-2026-1234" -b "d"', true], ['cd ./child && gh issue create -t "CVE-2026-1234" -b "d"', false], ['cd /home/dev/airflow-s && git commit -m "Fix CVE-2026-1234"', false], ] as const) { diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 7eb8a7462..7bceacfc0 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -2241,8 +2241,10 @@ export async function evaluatePublicSurfaceCommand( // any such `cd`, or anything followed by `||`, is a redirect. const isConditionalCd = Boolean(parsed.cdDir) && hasSeenConditional; // With CDPATH set, `cd name` searches the CDPATH entries before the current directory, so the - // tracked directory can name a different checkout than the one the shell lands in. - const isCdPathCd = Boolean(parsed.cdDir) && Boolean(options?.env?.CDPATH) && !/^[/.]/.test(parsed.cdDir ?? ''); + // tracked directory can name a different checkout than the one the shell lands in. Bash skips + // the search only for `/...`, `.`, `..`, `./...` and `../...` (`.hidden` is still searched). + const isCdPathCd = + Boolean(parsed.cdDir) && Boolean(options?.env?.CDPATH) && !/^(\/|\.\.?(\/|$))/.test(parsed.cdDir ?? ''); if (mayRedirectLaterCommands(rawSub) || item.separatorAfter === '||' || isConditionalCd || isCdPathCd) { hasPriorRedirect = true; }