Skip to content

fix: adversary 跨仓重定位 env 间接化(zizmor high) - #101

Merged
randypanding merged 1 commit into
mainfrom
zizmor-xt-repo
Aug 25, 2026
Merged

fix: adversary 跨仓重定位 env 间接化(zizmor high)#101
randypanding merged 1 commit into
mainfrom
zizmor-xt-repo

Conversation

@randypanding

@randypanding randypanding commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

PR#98 引入的 XT_REPO 行内插值触发 zizmor template-injection high(run 块内 ${{ }})。改 env 间接(XT_REPO_IN),语义零变化。CIW hygiene exit 14 实测暴露。

Summary by CodeRabbit

  • 安全性改进
    • 优化跨仓库目标路径的配置读取方式,减少配置注入风险。
    • 保持跨仓库目标统一定位至 external-target/,路径校验行为不变。

Copilot AI lite review requested due to automatic review settings August 25, 2026 00:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 90385fb2-0e29-405a-a6c1-30c074c975f9

📥 Commits

Reviewing files that changed from the base of the PR and between b480224 and 761fef1.

📒 Files selected for processing (1)
  • .github/workflows/adversary.yml

📝 Walkthrough

Walkthrough

Changes

跨仓目标路径处理

Layer / File(s) Summary
基于环境变量的目标解析
.github/workflows/adversary.yml
跨仓目标仓配置通过 XT_REPO_IN 环境变量传入。目标仓存在时,目标路径仍添加 external-target/ 前缀。

Suggested labels: security, bug

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch zizmor-xt-repo

Comment @coderabbitai help to get the list of available commands.

@randypanding
randypanding merged commit bab6a00 into main Aug 25, 2026
22 of 24 checks passed
@randypanding
randypanding deleted the zizmor-xt-repo branch August 25, 2026 00:59
@coderabbitai coderabbitai Bot added bug Something isn't working security labels Aug 25, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix zizmor template-injection in adversary workflow via env indirection

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove inline ${{ }} interpolation from the run block to satisfy zizmor template-injection
 checks
• Introduce XT_REPO_IN env indirection for cross-repo target relocation without behavior change
• Keep ADR-0085 external-target prefix rewrite semantics and existing path validation intact
Diagram

graph TD
  A["Workflow inputs / payload"] --> B["Step env (XT_REPO_IN)"] --> C["Bash: parse target"] --> D["GITHUB_OUTPUT target"]

  subgraph Legend
    direction LR
    _cfg["Workflow config"] ~~~ _proc["Run step"]
  end
Loading
High-Level Assessment

The chosen approach (env indirection) is the lowest-risk fix: it removes ${{ }} from the bash body to satisfy zizmor while keeping ADR-0085 semantics unchanged. Alternatives like moving logic into a checked-in script/action or rewriting control flow purely in YAML would add churn without improving security meaningfully beyond the current mitigation.

Files changed (1) +3 / -2

Bug fix (1) +3 / -2
adversary.ymlMove XT_REPO expression into env to avoid run-block interpolation +3/-2

Move XT_REPO expression into env to avoid run-block interpolation

• Adds 'XT_REPO_IN' in the step environment and switches the cross-repo relocation check to test that env var. Removes the inline '${{ ... }}' assignment from the 'run' script to address zizmor template-injection (high) without changing behavior.

.github/workflows/adversary.yml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Misleading “no interpolation” comment 🐞 Bug ⚙ Maintainability
Description
The new comment claims the run body has “正文零插值”, but the script still contains a ${{
github.event_name }} template expansion in the run block, so the statement is false and can
re-trigger zizmor template-injection under stricter personas (e.g., pedantic). This mismatch
increases future maintenance risk and weakens the intended “no template in run” invariant.
Code

.github/workflows/adversary.yml[R118-119]

+          # zizmor template-injection:表达式一律经 env 间接(XT_REPO_IN),正文零插值
+          if [[ -n "${XT_REPO_IN:-}" ]]; then
Relevance

●● Moderate

Comment is misleading but the ${{ github.event_name }} expansion is a pre-existing unrelated line,
not introduced here.

PR-#92

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds a comment asserting “正文零插值”, but the same run block still contains a `${{
github.event_name }}` expansion. zizmor’s documentation shows that under the pedantic persona, even
non-attacker-controlled ${{ }} expansions inside run: are flagged as template-injection,
making the comment and intended invariant brittle.

.github/workflows/adversary.yml[111-121]
.github/workflows/adversary.yml[113-116]
🌐 Documents that the pedantic persona flags template-injection for any template expansion inside a run: block, even for ${{ github.event_name }}.

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A newly added comment states the `run:` body contains no `${{ }}` template expansions, but the script still uses `${{ github.event_name }}` inside the run block. This is misleading and can re-surface `template-injection` findings if zizmor is run in a stricter persona.

### Issue Context
This workflow already follows the pattern “template values via env, not interpolated in shell” elsewhere in the repo, and the comment suggests this step follows the same invariant.

### Fix Focus Areas
- .github/workflows/adversary.yml[108-122]

### Suggested change
1. Add an env var like `EVENT_NAME: ${{ github.event_name }}` in the step’s `env:`.
2. Replace `if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then` with `if [[ "$EVENT_NAME" == "workflow_dispatch" ]]; then`.
3. Either remove or reword the comment to match reality (e.g., “untrusted expressions are env-indirected”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
  +2 more
Review mode: ⚖️ Balanced: This is a security-sensitive GitHub Actions workflow change addressing template injection; despite the tiny, localized diff, it warrants a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +118 to +119
# zizmor template-injection:表达式一律经 env 间接(XT_REPO_IN),正文零插值
if [[ -n "${XT_REPO_IN:-}" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Misleading “no interpolation” comment 🐞 Bug ⚙ Maintainability

The new comment claims the run body has “正文零插值”, but the script still contains a ${{
github.event_name }} template expansion in the run block, so the statement is false and can
re-trigger zizmor template-injection under stricter personas (e.g., pedantic). This mismatch
increases future maintenance risk and weakens the intended “no template in run” invariant.
Agent Prompt
### Issue description
A newly added comment states the `run:` body contains no `${{ }}` template expansions, but the script still uses `${{ github.event_name }}` inside the run block. This is misleading and can re-surface `template-injection` findings if zizmor is run in a stricter persona.

### Issue Context
This workflow already follows the pattern “template values via env, not interpolated in shell” elsewhere in the repo, and the comment suggests this step follows the same invariant.

### Fix Focus Areas
- .github/workflows/adversary.yml[108-122]

### Suggested change
1. Add an env var like `EVENT_NAME: ${{ github.event_name }}` in the step’s `env:`.
2. Replace `if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then` with `if [[ "$EVENT_NAME" == "workflow_dispatch" ]]; then`.
3. Either remove or reword the comment to match reality (e.g., “untrusted expressions are env-indirected”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants