Skip to content

Commit e89a32c

Browse files
wenshaoqwencoder
andauthored
fix(ci): neutralize legacy ##[ commands in autofix stdout echoes (#9871)
* fix(ci): neutralize legacy ##[ commands in autofix stdout echoes A review finding about workflow-command injection carried its payload strings (::error::forged, ##[add-matcher]) verbatim as example text. Prepare echoed feedback.md raw, the runner parsed the mid-line ##[add-matcher] and failed the step before the agent ran, and the consecutive-failure breaker burned the takeover window of #9761 in about 70 minutes while reporting it as a 100/100 round cap. The existing neutralization covered only the modern :: syntax; the runner also parses the legacy ##[name] form, even mid-line. Extend every untrusted-content echo in the autofix family (prepare feedback echo, both artifact dump loops, the gate's failure/handoff/no-action echoes, the deferred-findings dumps and upsert re-emit loops) to neutralize both prefixes, and pin the census plus a behavioral ##[ case in the contract tests. * fix(ci): unify the autofix neutralizer on one canonical spelling Review feedback on #9871: the two upsert re-emit loops neutralized with a hybrid spelling (bash expansion for :: plus a single-syntax sed for ##[) while the other fourteen sites used the canonical two-expression sed, establishing two implementations of one normalization. A future edit fixing one spelling could miss the other and recreate the single-syntax half-guard that let #9761 through. Unify both loops on the canonical sed and consolidate the pinned test shapes. The census test's comment also promised more than its assertions deliver: it pins the known enumeration, it does not auto-detect new un-neutralized echoes. Reword it to state the actual guarantee, ban the ##[-only sed and the bare bash expansion alongside the existing ::-only ban, and fold the script-side re-emit loop into the census. * test(ci): assert the neutralizer pairing property instead of banning shapes The three shape bans only matched the no--e spellings, so the canonical line's natural half-copies (sed -e 's/::/;;/g' alone, or the ##[-only -e mirror) passed every ban while guarding only one syntax. Assert the pairing property instead: every occurrence of either substitution expression must belong to the canonical two-expression pair, so a half-guard in ANY spelling unbalances the count and fails the census — verified by probe against all four half-guard shapes. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
1 parent 03dd858 commit e89a32c

5 files changed

Lines changed: 166 additions & 49 deletions

File tree

.github/scripts/autofix-push-and-report.sh

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,9 @@ run_deferred_upsert() {
324324
if [[ "${UPSERT_OUT}" != *'__upsert_child_live__'* ]]; then
325325
echo "::warning::deferred-findings upsert child never started (loader trace mode or exec failure); NOT persisted this round"
326326
fi
327-
# The child's output is agent-reachable content, so a line-start
328-
# `::` is neutralized before it reaches this step's stdout.
327+
# The child's output is agent-reachable content, so both workflow-command
328+
# syntaxes are neutralized before it reaches this step's stdout (`##[`
329+
# parses mid-line too — #9761).
329330
while IFS= read -r _upsert_line; do
330331
# Wrapper-authored lines carry a marker and are emitted
331332
# VERBATIM so they still render as GitHub annotations; the
@@ -335,7 +336,10 @@ run_deferred_upsert() {
335336
elif [[ "${_upsert_line}" == __upsert_trusted__* ]]; then
336337
printf '%s\n' "${_upsert_line#__upsert_trusted__}"
337338
else
338-
printf '%s\n' "${_upsert_line//::/;;}"
339+
# The canonical two-expression neutralizer, identical to every other
340+
# echo site — one spelling for the whole family, so a syntax change
341+
# cannot drift across two implementations.
342+
printf '%s\n' "${_upsert_line}" | sed -e 's/::/;;/g' -e 's/##\[/##[/g'
339343
fi
340344
done <<< "${UPSERT_OUT}"
341345
}

.github/scripts/run-autofix-review-verification.sh

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,10 @@ fi
236236
if [[ -f "${WORKDIR}/failure.md" && -n "$(git status --porcelain)" ]]; then
237237
echo "❌ Agent wrote failure.md after leaving a dirty workspace:"
238238
git status --short
239-
cat "${WORKDIR}/failure.md"
239+
# Agent-written content on step stdout: both workflow-command syntaxes
240+
# parse here (`##[` mid-line too — measured on #9761). Same reason
241+
# 'Show run artifacts' neutralizes these files.
242+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/failure.md"
240243
echo "outcome=failed" >> "${GITHUB_OUTPUT}"
241244
echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}"
242245
if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then
@@ -247,7 +250,7 @@ fi
247250

248251
if [[ -f "${WORKDIR}/failure.md" ]]; then
249252
echo "🛑 Agent aborted intentionally:"
250-
cat "${WORKDIR}/failure.md"
253+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/failure.md"
251254
echo "outcome=failed" >> "${GITHUB_OUTPUT}"
252255
echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}"
253256
if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then
@@ -277,7 +280,7 @@ if [[ -s "${WORKDIR}/handoff.md" && -n "$(git status --porcelain)" \
277280
&& "${AUDIT_VERDICT:-}" != 'conflict' ]]; then
278281
echo "❌ Agent wrote handoff.md after leaving a dirty workspace:"
279282
git status --short
280-
sed 's/::/;;/g' "${WORKDIR}/handoff.md"
283+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/handoff.md"
281284
echo "outcome=dirty_handoff" >> "${GITHUB_OUTPUT}"
282285
echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}"
283286
if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then
@@ -299,7 +302,7 @@ if [[ -s "${WORKDIR}/handoff.md" && "${committed_rc:-0}" -eq 1 \
299302
&& "${AUDIT_VERDICT:-}" != 'conflict' ]]; then
300303
echo "❌ Agent wrote handoff.md but the round HAS a commit — a brake violation:"
301304
git log --oneline "origin/${BRANCH}..${BRANCH}"
302-
sed 's/::/;;/g' "${WORKDIR}/handoff.md"
305+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/handoff.md"
303306
echo "outcome=committed_handoff" >> "${GITHUB_OUTPUT}"
304307
echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}"
305308
if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then
@@ -321,10 +324,11 @@ if git diff --quiet "origin/${BRANCH}...${BRANCH}" \
321324
&& [[ -s "${WORKDIR}/handoff.md" ]] \
322325
&& [[ "${AUDIT_VERDICT:-}" != 'conflict' ]]; then
323326
echo "🤝 Branch unchanged with a handoff — the agent stopped under instruction and deferred this item to a human:"
324-
# Agent-written content: a line-start `::` would be parsed as a workflow
325-
# command (::error::, ::add-mask::), the same reason 'Show run artifacts'
326-
# neutralizes these files.
327-
sed 's/::/;;/g' "${WORKDIR}/handoff.md"
327+
# Agent-written content: both workflow-command syntaxes parse on step
328+
# stdout — a line-start `::` (::error::, ::add-mask::) AND `##[` even
329+
# mid-line (a quoted `##[add-matcher]` fails the step; measured on
330+
#9761). The same reason 'Show run artifacts' neutralizes these files.
331+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/handoff.md"
328332
echo "outcome=handoff" >> "${GITHUB_OUTPUT}"
329333
echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}"
330334
if [[ "${AUDIT_VERDICT_RECORDED:-false}" == 'true' ]]; then
@@ -586,7 +590,9 @@ if git diff --quiet "origin/${BRANCH}...${BRANCH}"; then
586590
# no-commit handoff was classified before the structural checks above.
587591
if [[ -s "${WORKDIR}/no-action.md" ]]; then
588592
echo "🟰 No action needed:"
589-
cat "${WORKDIR}/no-action.md"
593+
# Both command syntaxes, like every other echo of agent-written files
594+
# (`##[` parses mid-line too — #9761).
595+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/no-action.md"
590596
echo "verified_head=$(git rev-parse HEAD)" >> "${GITHUB_OUTPUT}"
591597
echo "outcome=noop" >> "${GITHUB_OUTPUT}"
592598
echo "kiss_audit=${KISS_AUDIT:-false}" >> "${GITHUB_OUTPUT}"

.github/scripts/upsert-deferred-issue.sh

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ gh_reason() {
4949
local r=''
5050
[[ -n "${GH_ERR}" && -s "${GH_ERR}" ]] &&
5151
r="$(tr '\r\n\t' ' ' < "${GH_ERR}" | head -c 200)"
52-
# `::` neutralized like every other agent/API-derived echo: an API error
53-
# body is not trusted to be free of workflow-command syntax.
54-
r="$(printf '%s' "${r}" | sed 's/::/;;/g')"
52+
# Both workflow-command syntaxes neutralized like every other
53+
# agent/API-derived echo: an API error body is not trusted to be free of
54+
# them (`##[` parses mid-line too — #9761).
55+
r="$(printf '%s' "${r}" | sed -e 's/::/;;/g' -e 's/##\[/##[/g')"
5556
[[ -n "${r// /}" ]] && printf '%s' "${r}" || printf 'no stderr captured'
5657
}
5758
gh_err_reset() { [[ -n "${GH_ERR}" ]] && : > "${GH_ERR}"; }
@@ -67,9 +68,10 @@ OWN_FINDINGS="${WORKDIR}/deferred-findings.json"
6768
# filters this round's feedback out of every later round, and the next run's
6869
# workspace reset deletes the file — nothing re-derives them. So each abort
6970
# says so and dumps what it had, for manual recovery from the run log.
70-
# `::` is neutralized in the dump: the content is agent-influenced and a
71-
# raw `::` at line start would be parsed as a workflow command (same reason
72-
# `<!--` is neutralized at every publish site).
71+
# Both workflow-command syntaxes are neutralized in the dump: the content is
72+
# agent-influenced and would otherwise be parsed as a command — `::` at line
73+
# start AND `##[` even mid-line (measured on #9761). Same reason `<!--` is
74+
# neutralized at every publish site.
7375
dump_file() {
7476
[[ -s "$1" ]] || return 0
7577
local size
@@ -85,7 +87,7 @@ dump_file() {
8587
else
8688
echo "--- $1"
8789
fi
88-
head -c 4000 "$1" | sed 's/::/;;/g'
90+
head -c 4000 "$1" | sed -e 's/::/;;/g' -e 's/##\[/##[/g'
8991
echo
9092
}
9193
lost() {

.github/workflows/qwen-autofix.yml

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,13 +1318,15 @@ jobs:
13181318
if [[ -f "${WORKDIR}/failure.md" && -n "$(git status --porcelain)" ]]; then
13191319
echo "❌ Agent wrote failure.md after leaving a dirty workspace:"
13201320
git status --short
1321-
cat "${WORKDIR}/failure.md"
1321+
# Agent-written content on step stdout: neutralize both command
1322+
# syntaxes (`##[` parses mid-line too — #9761).
1323+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/failure.md"
13221324
exit 1
13231325
fi
13241326
13251327
if [[ -f "${WORKDIR}/failure.md" ]]; then
13261328
echo "🛑 Agent aborted intentionally:"
1327-
cat "${WORKDIR}/failure.md"
1329+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/failure.md"
13281330
exit 1
13291331
fi
13301332
@@ -1395,12 +1397,14 @@ jobs:
13951397
for f in decision.json pr-title.txt pr-body.md e2e-report.md failure.md failure.zh.md fix.diff; do
13961398
if [[ -f "${WORKDIR}/${f}" ]]; then
13971399
echo "=============== ${f} ==============="
1398-
# Agent-written content on step STDOUT: a line-start `::` would
1399-
# be parsed as a workflow command (::error::, ::add-mask::), the
1400-
# same reason the PR-lane dump loop neutralizes it. (The two
1401-
# step-SUMMARY loops write to a file, where `::` is not
1400+
# Agent-written content on step STDOUT: both workflow-command
1401+
# syntaxes parse here — a line-start `::` (::error::,
1402+
# ::add-mask::) AND `##[` even mid-line (a quoted
1403+
# `##[add-matcher]` fails THIS step; measured on #9761). The
1404+
# same reason the PR-lane dump loop neutralizes both. (The two
1405+
# step-SUMMARY loops write to a file, where commands are not
14021406
# parsed.)
1403-
sed 's/::/;;/g' "${WORKDIR}/${f}"
1407+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/${f}"
14041408
echo
14051409
fi
14061410
done
@@ -4699,7 +4703,17 @@ jobs:
46994703
fi
47004704
} > "${WORKDIR}/feedback.md"
47014705
echo '--- feedback.md ---'
4702-
cat "${WORKDIR}/feedback.md"
4706+
# Reviewer/bot comment bodies ride this file VERBATIM and this echo
4707+
# puts them on step stdout, which the runner scans for workflow
4708+
# commands in BOTH syntaxes: `::name::` AND the legacy `##[name]`,
4709+
# the latter parsed MID-line too — a quoted `##[add-matcher]` makes
4710+
# the runner load the rest of the line as a matcher file and fail
4711+
# THIS step before the agent runs. Measured on #9761: a review
4712+
# finding about that injection channel carried the payload strings
4713+
# as its example text, and five consecutive pre-agent crashes
4714+
# burned the takeover window in ~70 minutes. Neutralize both
4715+
# prefixes like every other untrusted echo.
4716+
cat "${WORKDIR}/feedback.md" | sed -e 's/::/;;/g' -e 's/##\[/##[/g'
47034717
47044718
# The agent below runs for up to 130 minutes and the verification gate adds
47054719
# more, but nothing reaches the PR thread until "Push and report" at the
@@ -5072,7 +5086,9 @@ jobs:
50725086
else
50735087
echo "::warning::could not merge carried deferrals across the repair (one of the two sets is unparseable); keeping the carried set and preserving this round's as deferred-findings.unmerged.json. Raw content follows:"
50745088
fi
5075-
head -c 4000 "${WORKDIR}/deferred-findings.json" | sed 's/::/;;/g'
5089+
# Both command syntaxes, like every other untrusted echo
5090+
# (`##[` parses mid-line too — #9761).
5091+
head -c 4000 "${WORKDIR}/deferred-findings.json" | sed -e 's/::/;;/g' -e 's/##\[/##[/g'
50765092
echo
50775093
rm -f "${WORKDIR}/deferred-findings.carry.next"
50785094
# Keep the discarded set ON DISK so the warning's pointer at
@@ -5300,10 +5316,12 @@ jobs:
53005316
for f in feedback.md address-summary.md no-action.md failure.md failure.zh.md handoff.md gate-rejection.md gate-advisories.md growth-audit.json agent-api-error agent-api-error-kind agent-timeout resolved-comments.txt comment-replies.json deferred-findings.json deferred-findings.carry.json deferred-findings.unmerged.json pr.diff; do
53015317
if [[ -f "${WORKDIR}/${f}" ]]; then
53025318
echo "=============== ${f} ==============="
5303-
# Agent-written content: a line-start `::` would be parsed as a
5304-
# workflow command (::error::, ::add-mask::), the same reason
5305-
# every other echo of these files neutralizes it.
5306-
sed 's/::/;;/g' "${WORKDIR}/${f}"
5319+
# Agent/reviewer-written content: both workflow-command syntaxes
5320+
# parse here — a line-start `::` (::error::, ::add-mask::) AND
5321+
# `##[` even mid-line (a quoted `##[add-matcher]` fails THIS
5322+
# step; measured on #9761 via the prepare echo of this same
5323+
# file). Neutralize both, like every other echo of these files.
5324+
sed -e 's/::/;;/g' -e 's/##\[/##[/g' "${WORKDIR}/${f}"
53075325
echo
53085326
fi
53095327
done
@@ -5995,7 +6013,11 @@ jobs:
59956013
elif [[ "${_upsert_line}" == __upsert_trusted__* ]]; then
59966014
printf '%s\n' "${_upsert_line#__upsert_trusted__}"
59976015
else
5998-
printf '%s\n' "${_upsert_line//::/;;}"
6016+
# The canonical two-expression neutralizer, identical to
6017+
# every other echo site — one spelling for the whole
6018+
# family, so a syntax change cannot drift across two
6019+
# implementations (`##[` parses mid-line too — #9761).
6020+
printf '%s\n' "${_upsert_line}" | sed -e 's/::/;;/g' -e 's/##\[/##[/g'
59996021
fi
60006022
done <<< "${UPSERT_OUT}"
60016023
fi

0 commit comments

Comments
 (0)