Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions .github/workflows/qwen-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,12 @@ jobs:
rm -rf "${WORKDIR}"
mkdir -p "${WORKDIR}"

# Same staging as the review-address job: the verify gate always runs the
# trusted checkout's copy of the schema gate, never a working-tree copy.
- name: 'Stage trusted schema gate'
run: |-
Comment on lines +292 to +294

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.

[Critical] This copy remains writable by branch-controlled host commands before it is invoked. In issue-autofix, verification checks out the agent-created branch and runs its npm run build, npm run typecheck, and npm run lint entry points with RUNNER_TEMP inherited before executing this file. A prompt-injected change to any of those entry points can replace the staged script with a successful no-op, so a stale schema passes and the later publish step pushes the unverified branch. Run branch-controlled commands inside a boundary that cannot mutate the gate, then materialize the trusted script from the pinned base immediately before invocation and verify its integrity instead of exposing a long-lived same-user-writable copy.

— Codex GPT-5 via Qwen Code /review

cp .github/scripts/check-settings-schema.sh "${RUNNER_TEMP}/check-settings-schema.sh"

- name: 'Check bot credentials'
env:
GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}'
Expand Down Expand Up @@ -787,7 +793,11 @@ jobs:
# verify step so the two copies cannot drift (rationale + the
# generator crash guard live in the script). On failure it writes
# outcome=failed to GITHUB_OUTPUT and exits 1.
bash .github/scripts/check-settings-schema.sh
# Run the copy staged from the trusted base checkout: a PR branch
# that predates the script does not contain it (bash would exit 127
# and kill the gate with no outcome), and the gate logic must come
# from the trusted base, not the branch under verification.
bash "${RUNNER_TEMP}/check-settings-schema.sh"

# Run changed/related tests for the packages this fix touches.
# --changed follows the import graph so transitive breakage is caught.
Expand Down Expand Up @@ -1196,6 +1206,16 @@ jobs:
rm -rf "${WORKDIR}"
mkdir -p "${WORKDIR}"

# Stage the schema gate script from the TRUSTED BASE checkout before
# "Prepare branch and feedback" switches the working tree to the PR
# branch. The verify gate must run the trusted copy: a PR branch that
# predates this script does not contain it (bash exits 127 and the gate
# dies without an outcome), and an in-branch copy would let branch code
# define its own gate.
- name: 'Stage trusted schema gate'
run: |-
Comment on lines +1214 to +1216

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.

[Critical] Staging before checkout does not keep this destination trusted: after switching to the PR branch, Triage and address directly executes that branch's .qwen/skills/autofix/scripts/run-agent.mjs on the host with RUNNER_TEMP inherited. A bot-owned branch can replace this script with a no-op and create no-action.md; verification then executes the replacement and records outcome=noop, bypassing the freshness gate. Invoke a runner staged from the trusted base, isolate all PR-controlled execution from the gate path, and materialize or integrity-check the trusted gate immediately before use.

— Codex GPT-5 via Qwen Code /review

cp .github/scripts/check-settings-schema.sh "${RUNNER_TEMP}/check-settings-schema.sh"

- name: 'Check runner environment'
env:
RUNNER_ENVIRONMENT: '${{ runner.environment }}'
Expand Down Expand Up @@ -1474,7 +1494,11 @@ jobs:
# the script); the write is on a tracked file compared by `git status`,
# not the commit-level no-op git-diff below, and it is restored on
# failure. On failure it writes outcome=failed and exits 1.
bash .github/scripts/check-settings-schema.sh
# Run the copy staged from the trusted base checkout: a PR branch
# that predates the script does not contain it (bash would exit 127
# and kill the gate with no outcome), and the gate logic must come
# from the trusted base, not the branch under verification.
bash "${RUNNER_TEMP}/check-settings-schema.sh"

if git diff --quiet "origin/${BRANCH}...${BRANCH}"; then
# No new commit. That is only legitimate as a deliberate no-action.
Expand Down
37 changes: 33 additions & 4 deletions scripts/tests/qwen-autofix-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -958,14 +958,43 @@ describe('qwen-autofix workflow', () => {
expect(step).toContain('npm run typecheck');
expect(step).toContain('npm run lint');
// The settings-schema freshness gate is extracted to a shared script so the
// two gates cannot drift; each verify step just invokes it.
expect(step).toContain('bash .github/scripts/check-settings-schema.sh');
// two gates cannot drift. Each verify step MUST invoke the copy staged from
// the trusted base checkout, NOT the working-tree path: after "Prepare
// branch and feedback" the tree is the PR branch, and a branch that predates
// the script does not contain it (bash exits 127 and the gate dies with no
// outcome), while an in-branch copy would let branch code define its own
// gate.
expect(step).toContain('bash "${RUNNER_TEMP}/check-settings-schema.sh"');
expect(step).not.toContain('bash .github/scripts/check-settings-schema.sh');
expect(step).toContain(
'No package changes detected; skipping package tests.',
);
expect(step).not.toContain('Fix does not touch any package');
expect(step).not.toContain('PR does not touch any package');
}
// Both jobs must stage the trusted copy before any branch switch.
expect(
workflow.match(
/cp \.github\/scripts\/check-settings-schema\.sh "\$\{RUNNER_TEMP\}\/check-settings-schema\.sh"/g,
) ?? [],
).toHaveLength(2);
// In the issue-autofix job the staging must happen BEFORE the verify gate's
// `git checkout "${BRANCH}"` (first occurrence in the file is the issue
// job's): the agent's commits can touch .github/scripts, so a post-checkout
// copy would stage the agent's version of the gate instead of the trusted
// base's. indexOf resolves to the issue job's staging (first occurrence).
expect(
workflow.indexOf("- name: 'Stage trusted schema gate'"),
).toBeGreaterThanOrEqual(0);
expect(
workflow.indexOf("- name: 'Stage trusted schema gate'"),
).toBeLessThan(workflow.indexOf('git checkout "${BRANCH}"'));
// In the review-address job the staging must happen BEFORE the branch switch
// ("Prepare branch and feedback" exists only in that job; the job's staging
// step is the last occurrence of the staging step name in the file).
expect(
workflow.lastIndexOf("- name: 'Stage trusted schema gate'"),
).toBeLessThan(workflow.indexOf("- name: 'Prepare branch and feedback'"));
// The shared script mirrors CI's freshness gate: regenerate + `git status
// --porcelain` (version-agnostic — the generator's --check was reverted from
// main by #7031 and must NOT be relied on), with a generator-crash guard, and
Expand Down Expand Up @@ -994,10 +1023,10 @@ describe('qwen-autofix workflow', () => {
);
expect(reviewVerifyGate).toBeTruthy();
expect(
reviewVerifyGate.indexOf('bash .github/scripts/check-settings-schema.sh'),
reviewVerifyGate.indexOf('bash "${RUNNER_TEMP}/check-settings-schema.sh"'),
).toBeGreaterThanOrEqual(0);
expect(
reviewVerifyGate.indexOf('bash .github/scripts/check-settings-schema.sh'),
reviewVerifyGate.indexOf('bash "${RUNNER_TEMP}/check-settings-schema.sh"'),
).toBeLessThan(reviewVerifyGate.indexOf('outcome=noop'));
});

Expand Down
Loading