Skip to content
Merged
37 changes: 37 additions & 0 deletions .github/scripts/resanitize-git-config.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/usr/bin/env bash
set -uo pipefail

# Re-sanitizes the git config surfaces a PAT-bearing git step is about to
# read, AFTER branch/agent code has run on the host. The inlined job-start
# sanitize steps are pre-checkout hygiene; between them and the push, the
# verification gates run branch test code on the host and the sandboxed
# agent has the workspace mounted — either can plant exec keys in the
# repo's LOCAL .git/config (the highest-precedence file, which the push
# reads) or rewrite the runner user's REAL global config: the gates' env
# redirect is inherited-env enforcement, not a filesystem boundary — a
# direct file write, `env -u GIT_CONFIG_GLOBAL git config --global`, or
# `git config --file "$HOME/.gitconfig"` all bypass it (probe-verified in
# the #8961 review).
#
# Invoked as `bash "${RUNNER_TEMP}/resanitize-git-config.sh"` from the
# copy the staging step took off the TRUSTED base checkout — never from
# the working tree, which holds the branch under test at call time.
#
# The allowlist and denylist are copies of the inlined pre-checkout
# sanitize steps in qwen-autofix.yml (which cannot call this script: it
# does not exist on disk before their checkout). The workflow contract
# tests pin every copy byte-identical — edit them together.

if [ -e .git ]; then
# Worktree-scoped config first, then the local allowlist sweep — same
# ordering rationale as the inlined step (config.worktree can carry
# core.hooksPath and is invisible to `git config --local`).
rm -f "$(git rev-parse --git-path config.worktree 2>/dev/null || echo /nonexistent)" 2>/dev/null || true

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1cfa5e0 — resanitize now removes .git/shallow alongside .git/commondir (both file twins of the env channels, both defused).

git config --local --unset-all extensions.worktreeConfig 2>/dev/null || true
Comment on lines +37 to +38

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1cfa5e0 — resanitize now rm -fs .git/commondir and .git/shallow (the file twins of GIT_COMMON_DIR/GIT_SHALLOW_FILE; a normal actions/checkout is not a linked worktree so neither legitimately exists), and both env-strip lists add GIT_COMMON_DIR + object-dir twins. Defense in depth: Push-and-report now refuses to push when git rev-parse HEAD != the gate's recorded verified_head, so a residual repo redirect can't deliver attacker content. Contract test pins the commondir/shallow removal and the HEAD guard.

git config --local --name-only --list 2>/dev/null \
| { grep -ivE '^(core\.(repositoryformatversion|bare|filemode|symlinks|ignorecase|precomposeunicode|logallrefupdates|worktree|hidedotfiles|protecthfs|protectntfs)|remote\.[^.]+\.(url|fetch|pushurl)|branch\.|extensions\.|gc\.|pack\.|fetch\.|index\.|safe\.|submodule\.[^.]+\.(url|active|branch))' || true; } \

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 1cfa5e0 — the fork and salvage fetches carry -c fetch.recurseSubmodules=false -c protocol.ext.allow=never, so a kept fetch.recurseSubmodules + a rewritten .git/modules ext:: URL can't recurse-and-execute with the PAT. Sweeping .git/modules wholesale would need per-submodule handling; neutralizing recursion at the fetch is the tighter, testable fix.

| while IFS= read -r key; do git config --local --unset-all "$key" 2>/dev/null || true; done
fi
{ git config --global --name-only --list 2>/dev/null || true; } \
| { grep -iE '^(core\.(hookspath|fsmonitor|pager|editor|sshcommand|askpass|alternaterefscommand|gitproxy)$|diff\.external$|diff\..+\.(command|textconv)$|merge\..+\.driver$|filter\.|alias\.|pager\.|difftool\.|mergetool\.|interactive\.difffilter$|sequence\.editor$|gpg\.(.+\.)?program$|init\.templatedir$|remote\..+\.(uploadpack|receivepack)$|submodule\..+\.update$|include\.|includeif\.|protocol\.ext\.allow$)' || true; } \
| while IFS= read -r key; do git config --global --unset-all "$key" 2>/dev/null || true; done

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8 — reproduced your probe independently on git 2.55 (listing omits XDG keys, --unset-all exits 5 with them live). The scrub in all four copies is now a loop over both files of the global scope, pointing GIT_CONFIG_GLOBAL at each in turn (the env var replaces the whole scope with exactly that file for reads and writes). The functional contract test plants an XDG fixture (diff.external=xdg-evil + a benign key) and asserts the exec key is removed while the benign key survives.

23 changes: 23 additions & 0 deletions .github/scripts/run-autofix-review-verification.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,29 @@ set -eo pipefail
# environment from the caller. WORKDIR and BRANCH are job-level env;
# GITHUB_OUTPUT and RUNNER_TEMP are runner-provided. None is defined here.

# Deterministic verification must not read the RUNNER's git config: the
# persistent pool accumulates state, and a leaked global exec knob fails
# branch tests the branch never caused. Measured counterexample, run
# 31516789251: a stray `diff.external=global-driver` in the runner user's
# ~/.gitconfig killed four per-hunk probe tests in packages/cli on #8613 —
# charged to the round (package tests are A/B-exempt), which burned the
# 18-minute repair on a failure no repair can reach and ended the round as
# a timeout. Every git this script or its checks spawn (vitest fixture
# repos included) reads a per-run throwaway global config instead — seeded
# with the workspace safe.directory actions/checkout put in the real one —
# and no system config — any system-level git setting the checks ever
# come to depend on (a CA bundle, a proxy) must be replicated via per-job
# env, not /etc/gitconfig, because the redirect silently drops it. The
# redirect also keeps a branch-authored `git config --global` from writing
# durable state onto the host: it lands in the throwaway file and dies
# with the run. Enforcement is inherited-env only — branch code writing
# the real file directly bypasses it, which is why the PAT-bearing steps
# re-run resanitize-git-config.sh afterwards.
export GIT_CONFIG_SYSTEM=/dev/null

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] GIT_CONFIG_SYSTEM=/dev/null redirect silently drops system-level git config. If a runner has a corporate proxy configured in /etc/gitconfig (http.proxy, http.sslCAInfo), git operations fail with opaque transport errors. The PR's own comment documents this trade-off. Consider adding a guarded ::warning:: diagnostic when system config exists.

Failure scenario: a self-hosted runner with corporate proxy in /etc/gitconfig — the gate silently drops the proxy config, all git commands fail with SSL errors, and the oncall engineer searches for network issues rather than the one-line export.

中文说明

[Suggestion] GIT_CONFIG_SYSTEM=/dev/null 重定向静默丢弃系统级 git 配置。如果 runner 有在 /etc/gitconfig 中配置的企业代理(http.proxy、http.sslCAInfo),git 操作会因模糊的传输错误失败。PR 自己的注释记录了此权衡。建议在系统配置存在时添加受保护的 ::warning:: 诊断。

失败场景:带企业代理的自托管 runner 在 /etc/gitconfig 中配置了代理——门静默丢弃代理配置,所有 git 命令因 SSL 错误失败,oncall 工程师搜索网络问题而非检查这一行 export。

— deepseek-v4-flash via Qwen Code /review (v0.21.10)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8 — both gates emit a guarded ::notice when /etc/gitconfig exists, naming the redirect and pointing at per-job env as the replication path, so an oncall sees the bypass in the step log instead of chasing transport errors.

export GIT_CONFIG_GLOBAL="${RUNNER_TEMP}/autofix-gate-gitconfig"
Comment on lines +41 to +42

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] GIT_CONFIG_SYSTEM=/dev/null redirect undocumented at the system level

The throwaway redirect sets GIT_CONFIG_SYSTEM=/dev/null, which silently drops all system-level git config. A future maintainer adds a system-level git config entry that the gates depend on (e.g., a CA bundle for registry access, a proxy, or a credential helper). The redirect silently bypasses it, and the gate fails with an opaque error (TLS error, auth failure). The comment block documents the global config rationale (pool runner pollution) but the system config bypass is not separately explained.

Suggested change
export GIT_CONFIG_SYSTEM=/dev/null
export GIT_CONFIG_GLOBAL="${RUNNER_TEMP}/autofix-gate-gitconfig"
Add a one-line note in the comment block: 'System config is also bypassed — any system-level git config the gates depend on must be replicated via the per-job environment.'

— deepseek-v4-flash via Qwen Code /review (v0.21.8)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1213ee8 — both gates' comment blocks now state that system config is bypassed too and that any system-level git setting the checks come to depend on (CA bundle, proxy) must be replicated via per-job env, not /etc/gitconfig.

Comment on lines +41 to +42

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8 — export GIT_CONFIG_COUNT=0 is now in both gate redirect blocks and both PAT-bearing steps, before their first git. Pinned in the contract tests, including functionally: the extracted redirect block is executed with an env-planted GIT_CONFIG_COUNT=1/GIT_CONFIG_KEY_0=core.fsmonitor and a hostile HOME, and a child git must resolve neither key afterwards. The PATH/GIT_EXEC_PATH leg of the GITHUB_ENV channel is noted but out of scope here.

: > "${GIT_CONFIG_GLOBAL}"
git config --file "${GIT_CONFIG_GLOBAL}" safe.directory "$(pwd)"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Test efficacy: hunk-survived — the GIT_CONFIG_SYSTEM=/dev/null and GIT_CONFIG_GLOBAL throwaway-file redirect can be reverted without any test failing. The existing 'throwaway global git config' test checks ordering (redirect appears before npm run build) but not functional effect. Adding a functional test (e.g., writing a hostile global config key before the gate and confirming the gate's git commands are unaffected) would strengthen the coverage.

Failure scenario: the redirect is silently removed or broken; the ordering test still passes (it checks string presence, not behavior), and the gate runs exposed to the host's global config.

中文说明

[Suggestion] 测试有效性:hunk 存活——GIT_CONFIG_SYSTEM=/dev/null 和 GIT_CONFIG_GLOBAL 一次性文件重定向可以在没有测试失败的情况下被还原。现有的 'throwaway global git config' 测试检查顺序(重定向出现在 npm run build 之前)但不检查功能效果。添加功能测试(例如,在门前写入敌对全局配置键并确认门的 git 命令不受影响)将加强覆盖。

失败场景:重定向被静默移除或破坏;顺序测试仍然通过(它检查字符串存在而非行为),门暴露在宿主机的全局配置下运行。

— deepseek-v4-flash via Qwen Code /review (v0.21.10)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8 — the contract test now EXECUTES the extracted redirect block: hostile HOME (diff.external=global-driver in ~/.gitconfig) plus an env-planted GIT_CONFIG_* key, asserting a child git resolves neither afterwards, that a git config --global write lands in the throwaway file (the hostile HOME file stays byte-identical), and the block's exit is clean. Reverting or hollowing out the redirect now fails this test on any host.


# Record whether the agent left a commit FIRST — this is a ref-only
# diff, so it runs before the failure.md early-exits and covers an
# agent that commits and then aborts. The failure handoff keys its
Expand Down
135 changes: 127 additions & 8 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -761,10 +761,39 @@ jobs:
- name: 'Sanitize workspace git config'
run: |-
set -uo pipefail
# The runner USER's global config is the same exec surface as the
# workspace config below: pool jobs run human-authored code (branch
# tests) as this user, and a stray `git config --global` outlives
# the job on the persistent pool. Measured: run 31516789251 found
# diff.external=global-driver in ~/.gitconfig, failing per-hunk
# probe tests in every later verification gate on this host. The
# gates read a throwaway global config now, so this scrub is host
# hygiene plus protection for THIS job's PAT-bearing git steps,
# which do read the real file. It runs BEFORE the .git early-exit:
# host hygiene owes nothing to the workspace existing. Denylist
# here, not the local allowlist below: the file belongs to the
# pool image, so routing/credential keys (http.*, url.*,
# credential.*) may be deliberate infra and are left alone — only
# the command-execution families go, plus include/includeIf (which
# can pull any of them back in) and protocol.ext.allow (which arms
# the command-executing ext:: transport a kept url.insteadOf could
# redirect to). Subsection slots are `.+`, never `[^.]+`: git
# subsection names may contain dots (`[diff "a.b"] command`
# flattens to diff.a.b.command and would slip past `[^.]+`);
# overmatching is harmless in a denylist. Guarded `|| true` twice:
# no global file and no match are both normal, and either would
# kill the step under the default `bash -e` + pipefail otherwise.
# The same denylist lives in resanitize-git-config.sh, which the
# PAT-bearing steps re-run AFTER branch code executed on the host;
# the workflow contract tests pin every copy byte-identical — edit
# them together.
{ git config --global --name-only --list 2>/dev/null || true; } \
| { grep -iE '^(core\.(hookspath|fsmonitor|pager|editor|sshcommand|askpass|alternaterefscommand|gitproxy)$|diff\.external$|diff\..+\.(command|textconv)$|merge\..+\.driver$|filter\.|alias\.|pager\.|difftool\.|mergetool\.|interactive\.difffilter$|sequence\.editor$|gpg\.(.+\.)?program$|init\.templatedir$|remote\..+\.(uploadpack|receivepack)$|submodule\..+\.update$|include\.|includeif\.|protocol\.ext\.allow$)' || true; } \
| while IFS= read -r key; do git config --global --unset-all "$key" 2>/dev/null || true; done

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8 — url\..+\.(insteadof|pushinsteadof)$ added to the denylist in all four copies (the rest of url.* stays for pool infra), and the fixture plants [url "https://mirror.example/"] insteadOf/pushInsteadOf asserting removal.

# `.git` is a directory in a normal checkout but a gitlink file in
# a worktree; -e covers both, and a missing .git (first run) too.
if [ ! -e .git ]; then
echo "no prior workspace; nothing to sanitize"
echo "no prior workspace; nothing local to sanitize"
exit 0
fi
# Worktree-scoped config FIRST: `extensions.worktreeConfig=true` is
Expand Down Expand Up @@ -894,6 +923,7 @@ jobs:
cp .github/scripts/check-settings-schema.sh "${RUNNER_TEMP}/check-settings-schema.sh"
cp .github/scripts/check-autofix-contracts.sh "${RUNNER_TEMP}/check-autofix-contracts.sh"
cp .github/scripts/resolve-owning-packages.sh "${RUNNER_TEMP}/resolve-owning-packages.sh"
cp .github/scripts/resanitize-git-config.sh "${RUNNER_TEMP}/resanitize-git-config.sh"

- name: 'Check bot credentials'
env:
Expand Down Expand Up @@ -1402,6 +1432,16 @@ jobs:
run: |-
BRANCH="autofix/issue-${ISSUE}"

# Hermetic git config for the gate and every check it spawns, same
# rationale and shape as run-autofix-review-verification.sh (a
# leaked global exec knob on the persistent pool must not fail
# branch tests, and a branch-authored `git config --global` must
# not outlive the run) — edit the two copies together.
export GIT_CONFIG_SYSTEM=/dev/null
export GIT_CONFIG_GLOBAL="${RUNNER_TEMP}/autofix-gate-gitconfig"
Comment on lines +1504 to +1505

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1213ee8. New .github/scripts/resanitize-git-config.sh re-runs the local allowlist sweep (worktree-config removal included) plus the global denylist scrub; it is staged into RUNNER_TEMP from the trusted base by both staging steps and invoked at the top of 'Publish PR' and 'Push and report', before any credential use — after the gates ran branch code, never from the working tree. Contract tests pin the staging (2 cp sites), the call-before-credential ordering in both steps, pin the script's allowlist/denylist byte-equal to the inlined step's, and functionally run the script against a fixture repo with planted local credential.helper/core.fsmonitor + polluted global — planted keys go, remote.origin.url stays.

: > "${GIT_CONFIG_GLOBAL}"
git config --file "${GIT_CONFIG_GLOBAL}" safe.directory "$(pwd)"

if [[ -f "${WORKDIR}/failure.md" && -n "$(git status --porcelain)" ]]; then
echo "❌ Agent wrote failure.md after leaving a dirty workspace:"
git status --short
Expand Down Expand Up @@ -1544,14 +1584,25 @@ jobs:
exit 1
fi
BRANCH="autofix/issue-${ISSUE}"
# The agent and the verification gate ran branch code on the host
# since the job-start sanitize; re-scrub the local and global git
# config surfaces this PAT-bearing step is about to read. Invoked
# from the trusted-base staged copy — the working tree holds the
# agent-authored branch at this point.
bash "${RUNNER_TEMP}/resanitize-git-config.sh"
Comment on lines +1713 to +1714

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1cfa5e0 — the stage step records the trusted PATH into GITHUB_OUTPUT before any branch code runs, and every PAT step + both gate steps now export PATH="${TRUSTED_PATH}" and unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH as their first lines, before the digest check — so a $GITHUB_ENV-planted PATH/preload can't swap the git/sha256sum/bash the gate runs on. Pinned in the contract test (PATH line + LD unsets in every PAT block, trusted_path recorded twice).

git config --local --unset-all http.https://github.com/.extraheader || true
git config core.hooksPath /dev/null
# Authenticate the push with a one-shot, host-scoped credential
# helper via `git -c`: nothing is written to the reused
# workspace's .git/config (no error path can strand it there),
# argv holds only the literal ${GITHUB_TOKEN} reference, and the
# host scope means it cannot answer a non-GitHub URL.
git -c credential."https://github.com".helper='!f(){ echo username=x-access-token; echo "password=${GITHUB_TOKEN}"; };f' \
# host scope means it cannot answer a non-GitHub URL. The leading
# empty credential.helper RESETS the inherited helper list first:
# helpers run in config order and the first to answer wins, so a
# helper planted at any earlier scope would otherwise see the
# request (and the env) before ours answers — probe-verified in
# the #8961 review.
git -c credential.helper= -c credential."https://github.com".helper='!f(){ echo username=x-access-token; echo "password=${GITHUB_TOKEN}"; };f' \
push --no-verify "https://github.com/${REPO}.git" "${BRANCH}"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8, both legs: the denylist now scrubs http.(.+.)?sslVerify/sslCAInfo (bare + URL-scoped; http.proxy stays kept — with verification forced on and the default CA, a planted proxy can only fail loudly, not read the credential), and all three one-shot helper chains lead with -c http.sslVerify=true. Count-pinned in the contract test (transport pin + helper reset + host-scoped helper = 3 sites) and the fixture plants [http "https://github.com"] sslVerify=false / sslCAInfo asserting removal.


PR_URL="$(gh pr create --repo "${REPO}" \
Expand Down Expand Up @@ -3171,10 +3222,39 @@ jobs:
- name: 'Sanitize workspace git config'
run: |-
set -uo pipefail
# The runner USER's global config is the same exec surface as the
# workspace config below: pool jobs run human-authored code (branch
# tests) as this user, and a stray `git config --global` outlives
# the job on the persistent pool. Measured: run 31516789251 found
# diff.external=global-driver in ~/.gitconfig, failing per-hunk
# probe tests in every later verification gate on this host. The
# gates read a throwaway global config now, so this scrub is host
# hygiene plus protection for THIS job's PAT-bearing git steps,
# which do read the real file. It runs BEFORE the .git early-exit:
# host hygiene owes nothing to the workspace existing. Denylist
# here, not the local allowlist below: the file belongs to the
# pool image, so routing/credential keys (http.*, url.*,
# credential.*) may be deliberate infra and are left alone — only
# the command-execution families go, plus include/includeIf (which
# can pull any of them back in) and protocol.ext.allow (which arms
# the command-executing ext:: transport a kept url.insteadOf could
# redirect to). Subsection slots are `.+`, never `[^.]+`: git
# subsection names may contain dots (`[diff "a.b"] command`
# flattens to diff.a.b.command and would slip past `[^.]+`);
# overmatching is harmless in a denylist. Guarded `|| true` twice:
# no global file and no match are both normal, and either would
# kill the step under the default `bash -e` + pipefail otherwise.
# The same denylist lives in resanitize-git-config.sh, which the
# PAT-bearing steps re-run AFTER branch code executed on the host;
# the workflow contract tests pin every copy byte-identical — edit
# them together.
{ git config --global --name-only --list 2>/dev/null || true; } \
| { grep -iE '^(core\.(hookspath|fsmonitor|pager|editor|sshcommand|askpass|alternaterefscommand|gitproxy)$|diff\.external$|diff\..+\.(command|textconv)$|merge\..+\.driver$|filter\.|alias\.|pager\.|difftool\.|mergetool\.|interactive\.difffilter$|sequence\.editor$|gpg\.(.+\.)?program$|init\.templatedir$|remote\..+\.(uploadpack|receivepack)$|submodule\..+\.update$|include\.|includeif\.|protocol\.ext\.allow$)' || true; } \
| while IFS= read -r key; do git config --global --unset-all "$key" 2>/dev/null || true; done
# `.git` is a directory in a normal checkout but a gitlink file in
# a worktree; -e covers both, and a missing .git (first run) too.
if [ ! -e .git ]; then
echo "no prior workspace; nothing to sanitize"
echo "no prior workspace; nothing local to sanitize"
exit 0
fi
# Worktree-scoped config FIRST: `extensions.worktreeConfig=true` is
Expand Down Expand Up @@ -3462,10 +3542,39 @@ jobs:
- name: 'Sanitize workspace git config'
run: |-
set -uo pipefail
# The runner USER's global config is the same exec surface as the
# workspace config below: pool jobs run human-authored code (branch
# tests) as this user, and a stray `git config --global` outlives
# the job on the persistent pool. Measured: run 31516789251 found
# diff.external=global-driver in ~/.gitconfig, failing per-hunk
# probe tests in every later verification gate on this host. The
# gates read a throwaway global config now, so this scrub is host
# hygiene plus protection for THIS job's PAT-bearing git steps,
# which do read the real file. It runs BEFORE the .git early-exit:
# host hygiene owes nothing to the workspace existing. Denylist
# here, not the local allowlist below: the file belongs to the
# pool image, so routing/credential keys (http.*, url.*,
# credential.*) may be deliberate infra and are left alone — only
# the command-execution families go, plus include/includeIf (which
# can pull any of them back in) and protocol.ext.allow (which arms
# the command-executing ext:: transport a kept url.insteadOf could
# redirect to). Subsection slots are `.+`, never `[^.]+`: git
# subsection names may contain dots (`[diff "a.b"] command`
# flattens to diff.a.b.command and would slip past `[^.]+`);
# overmatching is harmless in a denylist. Guarded `|| true` twice:
# no global file and no match are both normal, and either would
# kill the step under the default `bash -e` + pipefail otherwise.
# The same denylist lives in resanitize-git-config.sh, which the
# PAT-bearing steps re-run AFTER branch code executed on the host;
# the workflow contract tests pin every copy byte-identical — edit
# them together.
{ git config --global --name-only --list 2>/dev/null || true; } \
| { grep -iE '^(core\.(hookspath|fsmonitor|pager|editor|sshcommand|askpass|alternaterefscommand|gitproxy)$|diff\.external$|diff\..+\.(command|textconv)$|merge\..+\.driver$|filter\.|alias\.|pager\.|difftool\.|mergetool\.|interactive\.difffilter$|sequence\.editor$|gpg\.(.+\.)?program$|init\.templatedir$|remote\..+\.(uploadpack|receivepack)$|submodule\..+\.update$|include\.|includeif\.|protocol\.ext\.allow$)' || true; } \
| while IFS= read -r key; do git config --global --unset-all "$key" 2>/dev/null || true; done
# `.git` is a directory in a normal checkout but a gitlink file in
# a worktree; -e covers both, and a missing .git (first run) too.
if [ ! -e .git ]; then
echo "no prior workspace; nothing to sanitize"
echo "no prior workspace; nothing local to sanitize"
exit 0
fi
# Worktree-scoped config FIRST: `extensions.worktreeConfig=true` is
Expand Down Expand Up @@ -3585,6 +3694,7 @@ jobs:
cp .github/scripts/check-autofix-contracts.sh "${RUNNER_TEMP}/check-autofix-contracts.sh"
cp .github/scripts/resolve-owning-packages.sh "${RUNNER_TEMP}/resolve-owning-packages.sh"
cp .github/scripts/run-autofix-review-verification.sh "${RUNNER_TEMP}/run-autofix-review-verification.sh"
cp .github/scripts/resanitize-git-config.sh "${RUNNER_TEMP}/resanitize-git-config.sh"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1cfa5e0 — the stage step now also records verify_runner_sha256, and both gate steps ('Verification gate', 'Repair verification gate') run sha256sum -c against it (PATH pinned first) before invoking run-autofix-review-verification.sh. A mid-run overwrite between the two passes now flips the gate to FAILED instead of letting the branch define its own verdict. Contract test pins both digest checks + the single recording.

# The agent step runs AFTER prepare checks out the PR branch, so
# invoking the runner from the working tree would execute
# branch-controlled code on the host with the model key in env
Expand Down Expand Up @@ -3829,7 +3939,7 @@ jobs:
# and fine-grained PATs are documented as NOT receiving it.
# Prove push access NOW, before an agent round is spent, instead
# of 403ing at the report step after the work is done.
if ! git -c credential."https://github.com".helper='!f(){ echo username=x-access-token; echo "password=${GITHUB_TOKEN}"; };f' \
if ! git -c credential.helper= -c credential."https://github.com".helper='!f(){ echo username=x-access-token; echo "password=${GITHUB_TOKEN}"; };f' \
push --no-verify --dry-run "https://github.com/${HEAD_REPO}.git" HEAD:"${BRANCH}" > /dev/null 2>&1; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Fork push preflight step has the credential.helper= reset but no explanatory comment — the sibling Publish PR step has a detailed 8-line comment block explaining the rationale for the reset. Adding a brief comment would help future maintainers understand why the reset is load-bearing.

Failure scenario: a future maintainer does not understand the purpose of the empty credential.helper= and removes it, re-introducing the credential hijack vector.

中文说明

[Suggestion] Fork push 预检步骤有 credential.helper= 重置但缺少注释说明——兄弟 Publish PR 步骤有详细的 8 行注释块解释重置的理由。添加简短注释有助于未来维护者理解重置为何是关键的。

失败场景:未来维护者不理解空 credential.helper= 的用途并将其移除,重新引入凭据劫持向量。

— deepseek-v4-flash via Qwen Code /review (v0.21.10)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8 — the fork-preflight site now carries a brief comment naming both the helper-list reset and the transport pin, pointing at 'Publish PR' for the full rationale.

Comment on lines +4157 to 4158

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e7cb3b4 — the fork git fetch now leads with -c http.sslVerify=true -c credential.helper= (anonymous; a public repo's fork heads are always public, so it fails closed on a 401 instead of letting a planted global extraheader 401 into a planted helper that hands over the PAT). Contract test asserts the hardened fetch form and asserts the bare git fetch "https://github.com/${HEAD_REPO}…" is gone.

echo "🫥 fork push preflight failed for ${HEAD_REPO} (allow-edits grant or PAT type) — discarding without action or marker"
{
Expand Down Expand Up @@ -4748,6 +4858,13 @@ jobs:
exit 1
fi

# The gates ran branch test code on the host (and the sandboxed
# agent had the workspace mounted) since the job-start sanitize;
# re-scrub the local and global git config surfaces this

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e7cb3b4 — two ways: the salvage merge now runs git -c commit.gpgsign=false … merge --no-edit FETCH_HEAD, and the throwaway global redirect above means the merge no longer reads the host ~/.gitconfig/XDG commit.gpgsign in the first place. Contract test pins the -c commit.gpgsign=false on the merge invocation. Sorry the round-2 pass missed replying to R2-10 directly.

# PAT-bearing step is about to read. Invoked from the trusted-base
# staged copy — the working tree holds the PR branch at this point.
bash "${RUNNER_TEMP}/resanitize-git-config.sh"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e0031b8 — the staging steps record resanitize_sha256 into GITHUB_OUTPUT right after the cp, and both PAT-bearing steps run sha256sum -c against that expression-context value before executing the staged copy; a mismatch fails the step loudly. Contract test pins the digest recording (2 sites) and the verify-before-invoke ordering in both steps. You're right that the same class applies to the pre-existing staged scripts (run-autofix-review-verification.sh, run-agent.mjs + SKILL.md) — that predates this PR and touches the gate/agent plumbing, so I'll take it as a follow-up PR rather than widening this one further.

Comment on lines +5339 to +5340

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e7cb3b4 — the concurrent-job race is closed at the root: the PAT step now reads a per-run throwaway GIT_CONFIG_GLOBAL (+ GIT_CONFIG_SYSTEM=/dev/null) instead of the shared ~/.gitconfig, so a sibling job rewriting the real file during the sweep→push window (or the salvage fetch/merge windows) can't reach this push at all — no file-vs--c precedence contest remains. Matches your suggested shape (throwaway file + safe.directory seed, keeping resanitize/COUNT=0).

Comment on lines +5339 to +5340

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — verify-then-execute is inherently TOCTOU on a same-user-writable file. This round narrows it materially (mktemp'd unpredictable throwaway + PATH/LD pin so the sha256sum/bash themselves can't be swapped), but the residual window only fully closes with per-job runner isolation (ephemeral user/container), which is out of this PR's scope. Flagging it to the maintainer as the durable fix — see my summary note.

Comment on lines +5339 to +5340

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R4-15 (refs/replace): acknowledged residual, deferred to follow-up — a lower-severity workspace-ref surface no current PAT-git path reads (explicit refspecs throughout). Noting -c core.useReplaceRefs=false as the follow-up hardening.


if [[ "${OUTCOME}" == "fixed" ]]; then
NEXT_ROUND="$(( ROUND + 1 ))"
git config --local --unset-all http.https://github.com/.extraheader || true
Expand All @@ -4758,8 +4875,10 @@ jobs:
git config core.hooksPath /dev/null
# Authenticate push/fetch with a one-shot, host-scoped credential
# helper via a git_auth wrapper (see Publish PR) — nothing lands
# in .git/config, argv holds only the ${GITHUB_TOKEN} reference.
git_auth() { git -c credential."https://github.com".helper='!f(){ echo username=x-access-token; echo "password=${GITHUB_TOKEN}"; };f' "$@"; }
# in .git/config, argv holds only the ${GITHUB_TOKEN} reference,
# and the leading empty credential.helper resets the inherited
# helper list so a planted helper never answers first.
git_auth() { git -c credential.helper= -c credential."https://github.com".helper='!f(){ echo username=x-access-token; echo "password=${GITHUB_TOKEN}"; };f' "$@"; }
if [[ "${HEAD_REPO:-${REPO}}" != "${REPO}" ]]; then
# Push back to the FORK branch via allow-edits (PAT has push
# rights on the upstream, which GitHub extends to the fork's
Expand Down
Loading
Loading