fix: use shell env var for sink-visibility to prevent double-quoting in MCP gateway JSON config by Copilot · Pull Request #48632 · github/gh-aw

Copilot · 2026-07-28T13:49:12Z

PR #48589 introduced a double-quoting bug that caused MCP gateway startup to fail with "Configuration is not valid JSON" — the actual root cause of Impeccable Skills Reviewer failures, distinct from the hang-on-exit issue fixed in #48499.

The bug:

# PR #48589 set the env var via toJSON(), which embeds JSON quotes in the string value:
GH_AW_SINK_VISIBILITY: ${{ toJSON(steps.determine-automatic-lockdown.outputs.visibility) }}
#                      → GH_AW_SINK_VISIBILITY="\"public\""  (value has embedded quotes)

# Then the heredoc JSON wrapped it in more quotes:
"sink-visibility": "${GH_AW_SINK_VISIBILITY}"
#               → "sink-visibility": ""public""   ← invalid JSON

The fix: drop toJSON() so the env var holds the bare string, letting the JSON heredoc's surrounding "..." provide the only quoting layer:

GH_AW_SINK_VISIBILITY: ${{ steps.determine-automatic-lockdown.outputs.visibility }}
# → GH_AW_SINK_VISIBILITY="public"
# → "sink-visibility": "public"   ← valid JSON

Changes

mcp_renderer_guard.go: Change sinkVisibilityRuntimeExpr from guardExprSentinel + "${{ toJSON(...) }}" to "${GH_AW_SINK_VISIBILITY}". Also fix non-deterministic TOML rendering: sort map keys before iterating so the FNV heredoc delimiter hash is stable across compilations (was causing spurious drift failures).
mcp_environment.go: Emit GH_AW_SINK_VISIBILITY: ${{ steps.determine-automatic-lockdown.outputs.visibility }} without toJSON().
mcp_setup_gateway.go: Add -e GH_AW_SINK_VISIBILITY to docker run flags and dedup set.
Tests + golden files: Update assertions to check for ${GH_AW_SINK_VISIBILITY} shell-var reference instead of the old step-output expression; update 263 compiled lock files.

Fixes [Agent Perf] Impeccable Skills Reviewer still failing after CLI hang-on-exit fix merged (PR #48499) #48622

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

github-actions · 2026-07-28T14:39:44Z

✅ PR Code Quality Reviewer completed the code quality review.

github-actions · 2026-07-28T14:40:13Z

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #48632 does not have the 'implementation' label and has only 76 new lines of code in business logic directories (threshold: 100).

github-actions · 2026-07-28T14:40:20Z

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

github-actions · 2026-07-28T14:40:25Z

✅ Test Quality Sentinel completed test quality analysis.

Copilot

Pull request overview

Fixes MCP gateway JSON generation by passing repository visibility as a bare environment value. It also stabilizes TOML rendering.

Changes:

Replaces embedded toJSON() output with ${GH_AW_SINK_VISIBILITY}.
Passes the variable through gateway environments and Docker.
Sorts TOML policy keys and updates tests/generated outputs.

Show a summary per file

File	Description
`pkg/workflow/mcp_renderer_guard.go`	Renders shell visibility references and deterministic TOML.
`pkg/workflow/mcp_environment.go`	Defines the bare visibility environment value.
`pkg/workflow/mcp_setup_gateway.go`	Passes visibility into the gateway container.
`pkg/workflow/non_github_mcp_guard_policy_test.go`	Updates guard-policy assertions.
`pkg/workflow/template_injection_validation_test.go`	Covers safe heredoc shell expansion.
`pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden`	Updates compiled fixture output.
`pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden`	Updates compiled fixture output and policies.
`pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden`	Updates compiled fixture output.
`pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden`	Updates compiled fixture 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 gateway configuration.
`.github/workflows/tidy.lock.yml`	Regenerates gateway configuration.
`.github/workflows/step-name-alignment.lock.yml`	Regenerates gateway configuration.
`.github/workflows/spec-enforcer.lock.yml`	Regenerates gateway configuration.
`.github/workflows/smoke-pi.lock.yml`	Regenerates gateway configuration.
`.github/workflows/smoke-github-claude.lock.yml`	Regenerates gateway configuration.
`.github/workflows/smoke-copilot-mai.lock.yml`	Regenerates gateway configuration.
`.github/workflows/smoke-copilot-auto.lock.yml`	Regenerates gateway configuration.
`.github/workflows/smoke-claude-on-copilot.lock.yml`	Regenerates gateway configuration.
`.github/workflows/skillet.lock.yml`	Regenerates gateway configuration.
`.github/workflows/schema-consistency-checker.lock.yml`	Regenerates gateway configuration.
`.github/workflows/refiner.lock.yml`	Regenerates gateway configuration.
`.github/workflows/pr-description-caveman.lock.yml`	Regenerates gateway configuration.
`.github/workflows/plan.lock.yml`	Regenerates gateway configuration.
`.github/workflows/objective-impact-report.lock.yml`	Regenerates gateway configuration.
`.github/workflows/lint-monster.lock.yml`	Regenerates gateway configuration.
`.github/workflows/jsweep.lock.yml`	Regenerates gateway configuration.
`.github/workflows/instructions-janitor.lock.yml`	Regenerates gateway configuration.
`.github/workflows/hourly-ci-cleaner.lock.yml`	Regenerates gateway configuration.
`.github/workflows/go-logger.lock.yml`	Regenerates gateway configuration.
`.github/workflows/eslint-refiner.lock.yml`	Regenerates gateway configuration.
`.github/workflows/eslint-monster.lock.yml`	Regenerates gateway configuration.
`.github/workflows/eslint-miner.lock.yml`	Regenerates gateway configuration.
`.github/workflows/designer-drift-audit.lock.yml`	Regenerates gateway configuration.
`.github/workflows/delight.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-windows-terminal-integration-builder.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-team-status.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-security-red-team.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-safeoutputs-git-simulator.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-github-docs-seo-optimizer.lock.yml`	Updates the gateway Docker environment.
`.github/workflows/daily-elixir-credo-snippet-audit.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-doc-updater.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-doc-healer.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-code-metrics.lock.yml`	Regenerates gateway configuration.
`.github/workflows/daily-choice-test.lock.yml`	Regenerates gateway configuration.
`.github/workflows/copilot-session-insights.lock.yml`	Regenerates gateway configuration.
`.github/workflows/copilot-pr-merged-report.lock.yml`	Updates the gateway Docker environment.
`.github/workflows/copilot-agent-analysis.lock.yml`	Regenerates gateway configuration.
`.github/workflows/ci-doctor.lock.yml`	Regenerates gateway configuration.
`.github/workflows/avenger.lock.yml`	Regenerates gateway configuration.
`.github/workflows/approach-validator.lock.yml`	Regenerates gateway configuration.
`.github/workflows/agentic-token-optimizer.lock.yml`	Regenerates gateway configuration.
`.github/workflows/ab-testing-advisor.lock.yml`	Regenerates gateway configuration.

Review details

Tip

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

Files reviewed: 185/276 changed files
Comments generated: 0
Review effort level: Medium

github-actions

The fix correctly eliminates the double-quoting bug. Moving GH_AW_SINK_VISIBILITY to a plain env var (no toJSON()) and interpolating via "${GH_AW_SINK_VISIBILITY}" in the heredoc produces valid JSON. The change is consistent across all affected lock files. ✅

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 21.4 AIC · ⌖ 5.23 AIC · ⊞ 5K

github-actions · 2026-07-28T14:45:19Z

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 92/100 — Excellent

Analyzed 2 modified test(s): 2 design, 0 implementation, 0 violation(s).

📊 Metrics (2 modified tests)

Metric	Value
Analyzed	2 (Go: 2, JS: 0)
✅ Design	2 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	2 (100%)
Duplicate clusters	0
Inflation	No (test: +6 lines, prod: +51 lines)
🚨 Violations	0

Test	File	Classification	Issues
`TestAllNonGitHubMCPServersGetWriteSinkWhenGitHubHasAllowOnly`	`non_github_mcp_guard_policy_test.go:481`	`design_test` / `behavioral_contract`	None
`TestScanRunContentExpressionsHeredoc`	`template_injection_validation_test.go:1445`	`design_test` / `behavioral_contract`	None

Verdict

✅ passed. 0% implementation tests (threshold: 30%).

🧪 Test quality analysis by Test Quality Sentinel · sonnet46 · 32.7 AIC · ⌖ 10.4 AIC · ⊞ 8.1K · ◷
_{Comment /review to run again}

github-actions

✅ Test Quality Sentinel: 92/100. 0% implementation tests (threshold: 30%).

github-actions

Skills-Based Review 🧠

Applied /diagnosing-bugs — the fix is correct, well-scoped, and properly addresses the root cause.

📋 Key Themes & Highlights

Root-Cause Analysis

The bug was introduced by #48589 using toJSON(steps.determine-automatic-lockdown.outputs.visibility) directly inside a heredoc JSON value. toJSON() wraps the string in double-quotes, producing "public" — which, when embedded inside an already double-quoted JSON string, yields "\"public\"" and breaks JSON parsing.

The fix is architecturally correct: move the value out of the heredoc via GH_AW_SINK_VISIBILITY env var, pass the raw step output (no toJSON), and use the shell variable reference ${GH_AW_SINK_VISIBILITY} inside the heredoc. The surrounding JSON "..." provides the JSON string delimiters at runtime.

The compiler-level fix is in pkg/workflow/mcp_environment.go (line 117) and is consistent with the existing GITHUB_MCP_GUARD_MIN_INTEGRITY / GITHUB_MCP_GUARD_REPOS pattern used right above it — ✅ good symmetry.

Positive Highlights

✅ Root cause fully addressed in the compiler, not just in generated files
✅ sinkVisibilityEnvVar / sinkVisibilityRuntimeExpr constants cleanly defined in mcp_renderer_guard.go
✅ Existing tests in template_injection_validation_test.go and non_github_mcp_guard_policy_test.go already assert the ${GH_AW_SINK_VISIBILITY} form
✅ Zero lock files left using the old toJSON(...) form (all 100 affected workflows recompiled)
✅ No non-lock source files changed — consistent with a pure recompile

Minor Observation (non-blocking)

mcp_environment_test.go does not yet have a targeted test asserting that GH_AW_SINK_VISIBILITY is emitted into envVars for hasGitHubInTools workflows. Coverage comes indirectly via integration-style tests. A unit test there would make future regressions easier to pinpoint.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 56.2 AIC · ⌖ 4.74 AIC · ⊞ 6.7K
_{Comment /matt to run again}

github-actions

REQUEST_CHANGES — dangling env var reference for tools: {github: false}

Core double-quoting fix (shell env var + surrounding JSON quotes) is sound, and the sorted-map-iteration fix for deterministic TOML output is a solid correctness improvement. However, the new hasGitHubInTools check in mcp_environment.go uses key-presence instead of value-truthiness, so GH_AW_SINK_VISIBILITY gets set to reference a determine-automatic-lockdown step that generateGitHubMCPLockdownDetectionStep never emits when tools.github is explicitly false. That leaves a dangling/undefined step-output reference in compiled workflows — the same class of bug this PR fixes elsewhere. Verified via a targeted unit test.

Themes

Positive: eliminating toJSON() double-encoding and relying on the JSON heredoc's own quoting is correct and minimal.
Positive: sorting policy/field names before iterating TOML guard-policies fixes non-deterministic compiler output (stable FNV heredoc delimiters).
Blocking: hasGitHubInTools truthiness bug (see inline comment) needs a one-line fix (hasGitHubKey && githubToolVal != false) plus a regression test for tools: {github: false}.

🔎 Code quality review by PR Code Quality Reviewer · sonnet50 · 64 AIC · ⌖ 4.47 AIC · ⊞ 7.4K
_{Comment /review to run again}

github-actions · 2026-07-28T14:48:09Z

@@ -103,6 +107,16 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor
 		}
 	}

+	// Emit GH_AW_SINK_VISIBILITY for all workflows where the determine-automatic-lockdown step
+	// runs (i.e., any workflow with a GitHub tool, including mode:gh-proxy). This avoids


hasGitHubInTools checks key existence only, so tools: {github: false} still emits GH_AW_SINK_VISIBILITY referencing a determine-automatic-lockdown step output that is never generated for that config — producing a broken/empty expression at runtime.

💡 Details and fix

The step generator (generateGitHubMCPLockdownDetectionStep in compiler_github_mcp_steps.go) explicitly skips step generation when githubTool == false:

githubTool, hasGitHub := data.Tools["github"] if !hasGitHub || githubTool == false { return // step NOT generated }

But here, _, hasGitHubInTools := tools["github"] is true even when the value is false, because it only tests map-key presence, not truthiness. I verified this with a quick unit test: collectMCPEnvironmentVariables(map[string]any{"github": false}, ...) returns GH_AW_SINK_VISIBILITY = "${{ steps.determine-automatic-lockdown.outputs.visibility }}" even though the corresponding step is never emitted into the workflow YAML — this is the exact class of bug (dangling step-output reference) this PR is trying to fix for the JSON-quoting case.

Fix: check the value, not just presence:

githubToolVal, hasGitHubKey := tools["github"] hasGitHubInTools := hasGitHubKey && githubToolVal != false

Initial plan

bab7e85

Copilot AI assigned Copilot and pelikhan Jul 28, 2026

Copilot started work on behalf of pelikhan July 28, 2026 13:49 View session

Copilot AI linked an issue Jul 28, 2026 that may be closed by this pull request

[Agent Perf] Impeccable Skills Reviewer still failing after CLI hang-on-exit fix merged (PR #48499) #48622

Open

github-actions Bot mentioned this pull request Jul 28, 2026

Smoke Test: Copilot - 30365373087 #48637

Closed

fix: use shell env var for sink-visibility to prevent double-quoting …

cc6ecbc

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

Copilot AI changed the title ~~[WIP] Fix Impeccable Skills Reviewer failing after CLI hang-on-exit fix~~ fix: use shell env var for sink-visibility to prevent double-quoting in MCP gateway JSON config Jul 28, 2026

Copilot AI requested a review from pelikhan July 28, 2026 14:36

Copilot finished work on behalf of pelikhan July 28, 2026 14:36

pelikhan marked this pull request as ready for review July 28, 2026 14:37

Copilot AI review requested due to automatic review settings July 28, 2026 14:37

Copilot started reviewing on behalf of pelikhan July 28, 2026 14:39 View session

pelikhan mentioned this pull request Jul 28, 2026

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

Merged

Copilot AI reviewed Jul 28, 2026

View reviewed changes

github-actions Bot approved these changes Jul 28, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jul 28, 2026

[aw] No-Op Runs #43499

Open

github-actions Bot approved these changes Jul 28, 2026

View reviewed changes

pelikhan closed this Jul 28, 2026

github-actions Bot requested changes Jul 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: use shell env var for sink-visibility to prevent double-quoting in MCP gateway JSON config - #48632

fix: use shell env var for sink-visibility to prevent double-quoting in MCP gateway JSON config#48632
pelikhan with Copilot wants to merge 2 commits into
mainfrom
copilot/gh-aw-issue-48499-fix

Copilot AI commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Copilot AI commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 28, 2026

🧪 Test Quality Sentinel Report

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Root-Cause Analysis

Positive Highlights

Minor Observation (non-blocking)

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

REQUEST_CHANGES — dangling env var reference for tools: {github: false}

Uh oh!

github-actions Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading