Skip to content

fix: move sink-visibility to env var to prevent zizmor template-injection - #48589

Merged
pelikhan merged 8 commits into
mainfrom
copilot/compiler-fix-sink-visibility
Jul 28, 2026
Merged

fix: move sink-visibility to env var to prevent zizmor template-injection#48589
pelikhan merged 8 commits into
mainfrom
copilot/compiler-fix-sink-visibility

Conversation

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Compiling a workflow with GitHub MCP + safe-outputs emitted ${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }} directly inside the run: heredoc for the sink-visibility guard policy field. zizmor --persona=auditor flags any ${{ }} expression in a heredoc body as template injection.

Changes

Eliminate ${{ }} from the run: heredoc

  • sinkVisibilityRuntimeExpr changed from a sentinel-based raw expression to ${GH_AW_SINK_VISIBILITY} — a plain shell variable reference expanded by the unquoted heredoc

Move the expression to env:

  • collectMCPEnvironmentVariables now emits GH_AW_SINK_VISIBILITY: ${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }} on the Start MCP Gateway step's env: block — safe because GitHub Actions evaluates env: expressions outside the shell execution context
  • The condition uses tools["github"] directly (not mcpTools) so mode: gh-proxy workflows — where GitHub is excluded from mcpTools but determine-automatic-lockdown still runs — also get the env var

Pass env var into the gateway container

  • Added -e GH_AW_SINK_VISIBILITY to the docker run flags and dedup set in mcp_setup_gateway.go

Harden internal zizmor scan

  • Added --persona auditor to the zizmor Docker invocation so internal CI catches this class of finding going forward

Before / After

Before (in the run: heredoc — flagged by zizmor):

"sink-visibility": ${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }}

After (shell variable in heredoc, value set via env:):

# In env: block
GH_AW_SINK_VISIBILITY: ${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }}

# In run: heredoc
"sink-visibility": "${GH_AW_SINK_VISIBILITY}"

263 lock files regenerated; 8 golden test files updated to reflect the new env var and docker flag.



✨ PR Review Safe Output Test - Run 30366142424

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · sonnet46 · 87.4 AIC · ⌖ 19.3 AIC · ⊞ 6.1K ·
Comment /smoke-claude to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 9.86 AIC · ⌖ 6.19 AIC · ⊞ 7.2K ·
Comment /souschef to run again

Copilot AI and others added 2 commits July 28, 2026 12:07
…tion

- Change sinkVisibilityRuntimeExpr from a raw ${{ }} expression in the
  run: heredoc to ${GH_AW_SINK_VISIBILITY} shell variable reference
- Add GH_AW_SINK_VISIBILITY env var to Start MCP Gateway step's env:
  block, set from ${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }}
- Add -e GH_AW_SINK_VISIBILITY to MCP gateway Docker container flags
- Add GH_AW_SINK_VISIBILITY to buildAddedGatewayEnvVarSet to prevent
  duplicate emission
- Add --persona auditor to internal zizmor invocation
- Update test to check for GH_AW_SINK_VISIBILITY instead of old expression
- Regenerate 263 lock files via make recompile

Fixes #48565

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
The initial fix only added GH_AW_SINK_VISIBILITY to the env block when
'github' was present in mcpTools. For gh-proxy mode workflows, GitHub is
excluded from mcpTools (because it's handled via CLI proxy instead of MCP
server), but determine-automatic-lockdown still runs and sink-visibility
is still emitted in the MCP gateway config JSON.

Move the GH_AW_SINK_VISIBILITY env var assignment to check tools["github"]
directly (same condition as generateGitHubMCPLockdownDetectionStep), so
all workflows with the GitHub tool get the env var regardless of mode.

Also update golden test files to reflect the new env var and docker flag.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix sink-visibility runtime expression triggering template injection fix: move sink-visibility to env var to prevent zizmor template-injection Jul 28, 2026
Copilot AI requested a review from pelikhan July 28, 2026 12:23
@pelikhan
pelikhan marked this pull request as ready for review July 28, 2026 12:35
Copilot AI review requested due to automatic review settings July 28, 2026 12:35
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ failed during design decision gate check.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel failed during test quality analysis.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

⚠️ PR Code Quality Reviewer failed during code quality review.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer failed during the skills-based review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Moves MCP sink visibility through an environment variable to avoid zizmor template-injection findings.

Changes:

  • Routes repository visibility through GH_AW_SINK_VISIBILITY.
  • Passes the variable into the MCP gateway container.
  • Enables zizmor’s auditor persona and regenerates tests/workflows.
Show a summary per file
File Description
pkg/workflow/mcp_environment.go Adds sink-visibility environment value.
pkg/workflow/mcp_renderer_guard.go Uses shell-variable indirection.
pkg/workflow/mcp_setup_gateway.go Forwards the variable into Docker.
pkg/workflow/non_github_mcp_guard_policy_test.go Updates guard-policy assertions.
pkg/cli/zizmor.go Enables auditor persona.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden Updates golden output.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden Updates golden output.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden Updates golden output.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden Updates golden output.
pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden Updates Gemini output.
pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden Updates Copilot output.
pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden Updates Codex output.
pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden Updates Claude output.
.github/workflows/unbloat-docs.lock.yml Regenerates workflow lock.
.github/workflows/tidy.lock.yml Regenerates workflow lock.
.github/workflows/step-name-alignment.lock.yml Regenerates workflow lock.
.github/workflows/spec-enforcer.lock.yml Regenerates workflow lock.
.github/workflows/smoke-pi.lock.yml Regenerates workflow lock.
.github/workflows/smoke-github-claude.lock.yml Regenerates workflow lock.
.github/workflows/smoke-copilot-mai.lock.yml Regenerates workflow lock.
.github/workflows/smoke-copilot-auto.lock.yml Regenerates workflow lock.
.github/workflows/smoke-claude-on-copilot.lock.yml Regenerates workflow lock.
.github/workflows/skillet.lock.yml Regenerates workflow lock.
.github/workflows/schema-consistency-checker.lock.yml Regenerates workflow lock.
.github/workflows/refiner.lock.yml Regenerates workflow lock.
.github/workflows/pr-description-caveman.lock.yml Regenerates workflow lock.
.github/workflows/plan.lock.yml Regenerates workflow lock.
.github/workflows/objective-impact-report.lock.yml Regenerates workflow lock.
.github/workflows/lint-monster.lock.yml Regenerates workflow lock.
.github/workflows/jsweep.lock.yml Regenerates workflow lock.
.github/workflows/instructions-janitor.lock.yml Regenerates workflow lock.
.github/workflows/hourly-ci-cleaner.lock.yml Regenerates workflow lock.
.github/workflows/go-logger.lock.yml Regenerates workflow lock.
.github/workflows/eslint-refiner.lock.yml Regenerates workflow lock.
.github/workflows/eslint-monster.lock.yml Regenerates workflow lock.
.github/workflows/eslint-miner.lock.yml Regenerates workflow lock.
.github/workflows/designer-drift-audit.lock.yml Regenerates workflow lock.
.github/workflows/design-decision-gate.lock.yml Regenerates workflow lock.
.github/workflows/delight.lock.yml Regenerates workflow lock.
.github/workflows/daily-windows-terminal-integration-builder.lock.yml Regenerates workflow lock.
.github/workflows/daily-team-status.lock.yml Regenerates workflow lock.
.github/workflows/daily-security-red-team.lock.yml Regenerates workflow lock.
.github/workflows/daily-safeoutputs-git-simulator.lock.yml Regenerates workflow lock.
.github/workflows/daily-github-docs-seo-optimizer.lock.yml Regenerates workflow lock.
.github/workflows/daily-elixir-credo-snippet-audit.lock.yml Regenerates workflow lock.
.github/workflows/daily-doc-updater.lock.yml Regenerates workflow lock.
.github/workflows/daily-doc-healer.lock.yml Regenerates workflow lock.
.github/workflows/daily-code-metrics.lock.yml Regenerates workflow lock.
.github/workflows/daily-choice-test.lock.yml Regenerates workflow lock.
.github/workflows/craft.lock.yml Regenerates workflow lock.
.github/workflows/copilot-session-insights.lock.yml Regenerates workflow lock.
.github/workflows/copilot-pr-merged-report.lock.yml Regenerates workflow lock.
.github/workflows/copilot-agent-analysis.lock.yml Regenerates workflow lock.
.github/workflows/ci-doctor.lock.yml Regenerates workflow lock.
.github/workflows/avenger.lock.yml Regenerates workflow lock.
.github/workflows/approach-validator.lock.yml Regenerates workflow lock.
.github/workflows/agentic-token-optimizer.lock.yml Regenerates workflow lock.
.github/workflows/ab-testing-advisor.lock.yml Regenerates workflow lock.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 186/276 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread pkg/workflow/mcp_environment.go Outdated
@pelikhan

Copy link
Copy Markdown
Collaborator

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan

Copy link
Copy Markdown
Collaborator

/matt

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot there are encoding issues with the json https://github.com/github/gh-aw/actions/runs/30359620031/job/90276252896?pr=48589

Fixed in commit fix: remove toJSON() from GH_AW_SINK_VISIBILITY to fix double-encoding.

The issue was that toJSON() already wraps the value in JSON string quotes (e.g. "private"), but the heredoc also surrounds it with quotes: "sink-visibility": "${GH_AW_SINK_VISIBILITY}". This resulted in "sink-visibility": ""private"" — invalid JSON.

The fix removes toJSON() and passes the raw step output (${{ steps.determine-automatic-lockdown.outputs.visibility }}), so the shell var expands to the bare string and the heredoc quotes produce valid JSON: "sink-visibility": "private".

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — commenting with two targeted suggestions, no blocking issues.

📋 Key Themes & Highlights

Key Themes

  • Test coverage gap: the tools["github"] = false and gh-proxy mode branches of GH_AW_SINK_VISIBILITY emission are not directly unit-tested in mcp_environment_test.go; the gh-proxy regression from commit de750a8 could silently recur without them
  • Comment clarity: sinkVisibilityRuntimeExpr comment explains the zizmor motivation but doesn't clarify why the guardExprSentinel is no longer needed, leaving two parallel patterns unexplained side-by-side

Positive Highlights

  • ✅ Clean root-cause fix: expression moved to env: (evaluated outside shell), not just suppressed
  • toJSON() removal in the follow-up commit correctly avoids double-encoding; the comment in mcp_environment.go explains the reasoning clearly
  • ✅ Condition checks tools["github"] directly (not mcpTools) to cover gh-proxy mode — good domain knowledge applied
  • ✅ Dedup set in buildAddedGatewayEnvVarSet updated atomically with the docker flags — no partial state
  • ✅ Lock files regenerated and golden tests updated in the same PR

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 108.6 AIC · ⌖ 5.02 AIC · ⊞ 6.7K
Comment /matt to run again

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Smoke OTEL completed successfully!

@github-actions

Copy link
Copy Markdown
Contributor

Agent Container Tool Check

Tool Status Version
bash 5.2.21
sh available
git 2.54.0
jq 1.7
yq v4.53.3
curl 8.5.0
gh 2.96.0
node v24.18.0
python3 3.11.15
go 1.24.13
java 21.0.11
dotnet 10.0.302

Result: 12/12 tools available ✅ PASS

🔧 Tool validation by Agent Container Smoke Test · sonnet50 · 14 AIC · ⌖ 4.25 AIC · ⊞ 7.7K ·
Comment /smoke-test-tools to run again

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results

  • GitHub MCP Testing: ✅
  • Web Fetch Testing: ✅
  • File Writing Testing: ✅
  • Bash Tool Testing: ✅
  • Build gh-aw: ❌

Overall Status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

Smoke Gemini — Powered by Gemini · age00 · 9.39 AIC · ⌖ 1.55 AIC · ⊞ 10.2K ·
Comment /smoke-gemini to run again

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test summary: workflow reliability, security/dependency work, and docs updates dominated the last 24h.
Overall status: PASS
Bash tool test: ✅

Generated by ⚡ Smoke Copilot MAI · mai10 · 3.18 AIC · ⌖ 1.67 AIC · ⊞ 18.8K ·
Comment /smoke-copilot-mai to run again
Add label smoke to run again

@github-actions

Copy link
Copy Markdown
Contributor

Commit pushed: 55ea9aa

Generated by Changeset Generator · gpt54 · 5.21 AIC · ⌖ 3.48 AIC · ⊞ 11.7K

@github-actions

Copy link
Copy Markdown
Contributor

Comment Memory

Code wakes at sunrise
Silent checks keep the path clear
Smoke drifts into green

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex · gpt54 · 9.92 AIC · ⌖ 1.19 AIC · ⊞ 11.2K ·
Comment /smoke-codex to run again

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Smoke Test Run 30366142424 — PR Review Results

Core Tests #1-12: ✅✅✅✅✅✅✅✅✅✅✅✅
PR Review Tests:

Overall: PARTIAL

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · sonnet46 · 87.4 AIC · ⌖ 19.3 AIC · ⊞ 6.1K ·
Comment /smoke-claude to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💥 Automated smoke test review - all systems nominal! Run 30366142424 complete.

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · sonnet46 · 87.4 AIC · ⌖ 19.3 AIC · ⊞ 6.1K
Comment /smoke-claude to run again

Comment thread pkg/workflow/mcp_environment.go Outdated
// the GitHub tool is present in tools (regardless of mode — gh-proxy workflows also get this
// step even though github is excluded from mcpTools).
rawGitHubValue, hasGitHubInTools := tools["github"]
if hasGitHubInTools && rawGitHubValue != false {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 The condition hasGitHubInTools && rawGitHubValue != false correctly handles both the presence and truthiness of the github key in tools. This ensures GH_AW_SINK_VISIBILITY is set for gh-proxy mode workflows too — solid guard! Consider adding a test case for the rawGitHubValue == false branch to ensure full coverage.

Comment thread pkg/cli/zizmor.go
dockerArgs := []string{
"run",
"--rm",
"-v", gitRoot + ":/workdir",
"-w", "/workdir",
"ghcr.io/zizmorcore/zizmor:latest",
"--persona", "auditor",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Adding --persona auditor is a great improvement — this ensures the internal zizmor scan catches template-injection findings like the one this PR fixes. This makes the CI pipeline self-enforcing for future similar regressions.

@github-actions

Copy link
Copy Markdown
Contributor

Comment Memory

Code smoke tests hum
Bots trace checks through shifting branches
Tiny green lights bloom

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

📰 BREAKING: Report filed by Smoke Copilot · gpt54 · 53.5 AIC · ⌖ 5.56 AIC · ⊞ 20.8K ·
Comment /smoke-copilot to run again
Add label smoke to run again

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot rebase fixes from #48632 into this PR

…d policy iteration, and case string handler

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot rebase fixes from #48632 into this PR

Rebased in commit fix: rebase PR 48632 fixes. Changes incorporated from #48632:

  • sinkVisibilityEnvVar = "GH_AW_SINK_VISIBILITY" constant in mcp_renderer_guard.go, used everywhere instead of the inline string literal
  • Sorted policy/field iteration in renderGuardPoliciesToml for deterministic TOML output (fixes spurious drift failures)
  • case string: handler in the TOML renderer so sink-visibility values are emitted correctly
  • Simplified mcp_environment.go to use _, hasGitHubInTools := tools["github"] (no rawGitHubValue != false check)
  • 263 lock files regenerated to reflect the sorted-output change

@pelikhan

Copy link
Copy Markdown
Collaborator

/smoke-copilot

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot is now investigating this issue comment. Sources say the story is developing...

@pelikhan

Copy link
Copy Markdown
Collaborator

/smoke-claude

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

💥 WHOOSH! Smoke Claude springs into action on this issue comment! [Panel 1 begins...]

@pelikhan
pelikhan merged commit b7c02ea into main Jul 28, 2026
30 checks passed
@pelikhan
pelikhan deleted the copilot/compiler-fix-sink-visibility branch July 28, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler: sink-visibility runtime expression triggers zizmor template-injection

3 participants