mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-23 15:45:13 +00:00
* fix(ci): make autofix verification gates hermetic to runner git config A leaked global exec knob on the persistent pool (run 31516789251: diff.external=global-driver in the runner user's ~/.gitconfig) failed four per-hunk probe tests in packages/cli on #8613. The rejection was 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 — attempt 4 of the failure window, with nothing pushed. Three layers against that class: - Both verification gates (the review script and the issue-fix inline twin) now export a per-run throwaway GIT_CONFIG_GLOBAL (seeded with the workspace safe.directory) and GIT_CONFIG_SYSTEM=/dev/null before their first git command, so every check they spawn — vitest fixture repos included — is hermetic to the host, and a branch-authored `git config --global` dies with the run instead of poisoning the next one. - The sanitize step (all three byte-identical copies) now also scrubs the runner USER's global config — denylist of the command-execution families only, so infra-owned routing/credential keys survive. This self-heals the live pollution on the affected runner on its next job and removes (not merely bypasses) a planted global core.hooksPath. - test-efficacy.integration.test.ts gets the same GIT_CONFIG_GLOBAL / HOME isolation as git.integration.test.ts: the code under test pins --no-ext-diff, but the test scaffolding's plain `git diff` did not, so a hostile user git config could fail the suite anywhere. Contract tests pin the gate redirects (before the first git command, truncated per run) and functionally run the extracted scrub pipeline against a poisoned fixture config, asserting the kept/removed sets. * fix(ci): widen the config scrub and re-sanitize before PAT-bearing steps Address the #8961 review findings (2 Critical, 8 Suggestions across two reviews), all probe-verified by the reviewers: - Denylist regex: subsection slots are .+ (git subsection names may contain dots — diff."a.b".command flattened past [^.]+), and the missing exec families are added: gpg.(*.)program, pager.*, interactive.diffFilter, difftool./mergetool., remote.*.uploadpack/ receivepack. The functional fixture now covers every alternation plus dotted subsections, non-exec/corrupt/missing-config arms pin the two load-bearing '|| true' guards. - The global scrub moved above the .git early-exit: host hygiene owes nothing to the workspace existing. - New resanitize-git-config.sh (staged from the trusted base) re-runs the local allowlist sweep and the global denylist scrub at the top of both PAT-bearing git steps — the gates run branch test code on the host after the job-start sanitize, and the env redirect is not a filesystem boundary. Contract tests pin script/step lists equal, the staging in both jobs, the call-before-credential ordering, and run the script functionally against planted local+global keys. - All three one-shot credential helpers lead with -c credential.helper= (empty resets the helper list; the first helper to answer wins, so a planted one must never run first). Count-pinned in the contract test. - comment-status.integration.test.ts gets the same git-config isolation as its siblings; test-efficacy gains an isolation tripwire test that goes red if the redirect is removed, instead of only on hostile hosts. - Comment fixes: the copies are cross-referenced as contract-test-pinned, and the system-config bypass is documented in both gates. * fix(ci): close the XDG/env/transport bypasses around the config scrub Address the #8961 round-2 review findings (5 Critical + 8 Suggestions, probe-verified by the reviewers; the XDG listing gap independently reproduced on git 2.55): - The global scrub sweeps BOTH files of the global scope: with ~/.gitconfig and $XDG_CONFIG_HOME/git/config both present, `git config --global` lists/unsets only the former, so keys planted in the XDG file survived every copy. The scrub is now a loop that redirects GIT_CONFIG_GLOBAL at each file in turn. - Denylist adds url.*.insteadOf/pushInsteadOf (transport rewrite of the PAT push/fetch; rest of url.* stays) and http.*.sslVerify/sslCAInfo (turns a kept http.proxy into a TLS-terminating interceptor); the three PAT helper chains lead with -c http.sslVerify=true. - The staged resanitize script's provenance holds at cp time only — RUNNER_TEMP is writable by the branch code that runs in between — so the staging steps record its sha256 in GITHUB_OUTPUT and the PAT steps verify before executing. - Both gates and both PAT steps export GIT_CONFIG_COUNT=0: GITHUB_ENV-injected GIT_CONFIG_KEY/VALUE entries apply at command-line precedence and outrank every file-level guard. - Gates emit a ::notice when /etc/gitconfig exists (bypassed by the redirect — replicate needed settings via per-job env). - Tests: the scrub's functional harness drives HOME/XDG fixtures and covers the new families; the resanitize run plants worktree-scoped config (deleting the rm -f line previously stayed green); the gate redirect block is executed against a hostile HOME and an env-planted GIT_CONFIG_* key; the isolation tripwire pins the NOSYSTEM leg and probes system-scope leakage. - The process-env git isolation pattern is extracted into isolateHostGitConfig() in review/lib/test-utils.ts and adopted by all five suites that duplicated it; comment-status gains the same tripwire. * fix(ci): take PAT git steps off host scopes and close the env channels Address the #8961 round-3 review (5 Critical + 6 Suggestions, probe-verified by the reviewer): - Both PAT-bearing steps now run fully hermetic, same shape as the gates: a per-run throwaway GIT_CONFIG_GLOBAL + GIT_CONFIG_SYSTEM= /dev/null, so a concurrent job rewriting the shared ~/.gitconfig in the sweep->push window (max-parallel, one HOME across ~27 runner registrations) can no longer steer the push, and a URL-scoped sslVerify=false there can no longer override the -c pin. Both steps and both gates also strip the git ENV channels that outrank file config: GIT_CONFIG_PARAMETERS, GIT_SSL_NO_VERIFY/CAINFO, GIT_PROXY_COMMAND, GIT_EXEC_PATH, GIT_DIR/WORK_TREE, GIT_ASKPASS, GIT_SSH/_COMMAND, plus GIT_CONFIG_COUNT=0. - The push-race salvage merge runs -c commit.gpgsign=false: a global commit.gpgsign=true with no key would exit 128 and be misread as a content conflict, discarding a verified round (R2-10). - The maintainer-fork fetch, the one PAT-bearing network site the round-2 rollout skipped, leads with -c http.sslVerify=true -c credential.helper= (anonymous; public fork heads need no auth, so it fails closed on a 401 instead of feeding a planted helper the PAT). - Denylist widens protocol.ext.allow to protocol.(ext.)?allow (the top-level fallback policy arms ext:: too) in all four copies. - Tests: the two PAT hermetic blocks and the two gate blocks are pinned equal; the sha256 verify line is pinned verbatim and asserted to carry no bypass; the resanitize fixture plants a live XDG exec key (drops of the loop's XDG leg now fail); the gate redirect functional exec adds the env-channel unsets; diff-plan adopts isolateHostGitConfig (sixth suite) keeping its GIT_TERMINAL_PROMPT delta; comment-status tripwire gains the GIT_CONFIG_GLOBAL assertion. * fix(ci): pin PATH, seal repo-redirect and env channels, harden all PAT sites Address the #8961 round-4 review (6 Critical + suggestions, probe-verified by the reviewer): - PATH is pinned to a value the stage step records before any branch code runs, and LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH are dropped, at the top of every PAT step and both gate steps — a $GITHUB_ENV-planted PATH or preload would otherwise swap the git/sha256sum/bash the digest gate itself runs on. - The Prepare step (PAT-bearing, previously unhardened) now takes the same hermetic preamble as the push steps; all three PAT preambles are pinned identical by the contract test. - The throwaway global config is created with mktemp, not a fixed literal path a same-user watcher could re-plant into after the seed. - The env-strip list gains GIT_ALLOW_PROTOCOL (env twin of protocol.allow), GIT_COMMON_DIR / GIT_OBJECT_DIRECTORY / GIT_ALTERNATE_OBJECT_DIRECTORIES / GIT_SHALLOW_FILE (repo-redirect twins), across all PAT and gate copies; the salvage/fork fetches carry -c fetch.recurseSubmodules=false -c protocol.ext.allow=never so a planted submodule cannot execute an ext:: URL with the PAT. - resanitize removes .git/commondir and .git/shallow (the file twins of GIT_COMMON_DIR/GIT_SHALLOW_FILE), and Push-and-report refuses to push a HEAD that is not the gate's recorded verified_head — closing the repo-redirect path that pushed attacker content. - The gate runner (run-autofix-review-verification.sh) is now digest- verified before both gate passes, like resanitize already was: the branch runs its own build/test between the passes, so an unverified copy would let it define its own verdict. - Contract tests pin every new surface: the three identical PAT preambles, the full unset var set, the mktemp path, the trusted-PATH wiring, the two gate-runner digest checks, the recurse/protocol pins, the HEAD==verified_head guard, and the commondir/shallow removal. * fix(ci): pin gh env channels, harmonize allowlist subsection slots Round-4 follow-ups: - Pin GH_HOST=github.com and unset GH_TOKEN/GH_ENTERPRISE_TOKEN/ GH_CONFIG_DIR before the first gh call in all three PAT steps, so a $GITHUB_ENV-planted GH_HOST cannot spoof the identity check and a planted GH_TOKEN cannot outrank the inline one. - Harmonize the local allowlist's remote/submodule subsection slots to .+ (matching the denylist comment and preventing a dotted-name remote from silently losing its url/fetch on every resanitize). * fix(ci): pin gh config dir and push the exact verified object Round-5 closable findings before landing: - R5-7: pin GH_CONFIG_DIR to a fresh mktemp -d instead of unsetting it, so PAT-bearing gh calls no longer fall back to the attacker-writable ~/.config/gh (whose config.yml can carry http_unix_socket and other transport reroutes) on the shared HOME. - R5-8: push the exact verified commit object (PUSH_SHA:refs/heads/...), not symbolic HEAD which the push would re-resolve — closing the check-then-use race the verified-HEAD guard was added to close. PUSH_SHA is pinned to VERIFIED_HEAD under the guard and re-pinned to the merge result after each salvage merge. The remaining round-5 Criticals (BASH_ENV/BASH_FUNC_* and LD_PRELOAD executing at step-shell startup before any unset runs; GITHUB_OUTPUT writable by gate-run branch code) are not closable from inside a Actions step — they require runner-level isolation and are tracked as a follow-up.
54 lines
3.9 KiB
Bash
54 lines
3.9 KiB
Bash
#!/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
|
|
# Repo-scope redirect files first. `.git/commondir` (the file twin of
|
|
# GIT_COMMON_DIR) repoints local config, refs AND objects — a plant makes
|
|
# the very --local sweep below act on the ATTACKER's config, and lets the
|
|
# PAT push deliver attacker content; `.git/shallow` (twin of
|
|
# GIT_SHALLOW_FILE) narrows the object graph. A normal actions/checkout is
|
|
# not a linked worktree, so neither file legitimately exists here —
|
|
# removing them cannot break a real checkout, only defuse a plant. Then
|
|
# config.worktree (can carry core.hooksPath, invisible to `git config
|
|
# --local`), then the local allowlist sweep.
|
|
GIT_DIR_PATH="$(git rev-parse --git-dir 2>/dev/null || echo .git)"
|
|
rm -f "${GIT_DIR_PATH}/commondir" "${GIT_DIR_PATH}/shallow" 2>/dev/null || true
|
|
rm -f "$(git rev-parse --git-path config.worktree 2>/dev/null || echo /nonexistent)" 2>/dev/null || true
|
|
git config --local --unset-all extensions.worktreeConfig 2>/dev/null || true
|
|
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; } \
|
|
| while IFS= read -r key; do git config --local --unset-all "$key" 2>/dev/null || true; done
|
|
fi
|
|
# The GLOBAL scope spans TWO files — ~/.gitconfig and
|
|
# ${XDG_CONFIG_HOME:-~/.config}/git/config — but with both present,
|
|
# `git config --global` lists and unsets ONLY ~/.gitconfig (probed on
|
|
# git 2.43 and 2.55: the listing omits the XDG keys and --unset-all
|
|
# exits 5 with them live), so sweep each file explicitly by pointing
|
|
# GIT_CONFIG_GLOBAL at it — the env var replaces the whole global
|
|
# scope with exactly that file, for reads and writes alike.
|
|
for global_file in "${HOME}/.gitconfig" "${XDG_CONFIG_HOME:-${HOME}/.config}/git/config"; do
|
|
{ GIT_CONFIG_GLOBAL="${global_file}" 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$|url\..+\.(insteadof|pushinsteadof)$|http\.(.+\.)?(sslverify|sslcainfo)$|include\.|includeif\.|protocol\.(ext\.)?allow$)' || true; } \
|
|
| while IFS= read -r key; do GIT_CONFIG_GLOBAL="${global_file}" git config --global --unset-all "$key" 2>/dev/null || true; done
|
|
done
|