Fix CLI proxy default policy to respect repo visibility (block private repos in public repos)#45158
Conversation
When no explicit repos/min-integrity is configured in the workflow frontmatter, the CLI proxy previously fell back to a hardcoded default that allowed all repos (including private repos), even when the workflow runs in a public repository. Changes: - determine_automatic_lockdown.cjs: set repos="public" (not "all") for public repos when no repos is explicitly configured. Also handle GH_AW_PRIVATE_TO_PUBLIC_FLOWS=allow to override back to "all" when user opts in. - compiler_github_mcp_steps.go: emit GH_AW_PRIVATE_TO_PUBLIC_FLOWS: allow env var when private-to-public-flows: allow is configured - compiler_difc_proxy.go: reference steps.determine-automatic-lockdown.outputs.repos and .min_integrity in the default CLI proxy policy template instead of hardcoded values - Update all workflow lock files via make recompile - Update tests for all changes Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Lock files will be recompiled separately after the source changes are merged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Pull request overview
Updates CLI proxy defaults to scope repository access by runtime visibility, addressing #45157.
Changes:
- Defaults public repositories to
repos=public. - Propagates the private-to-public flow opt-in.
- Uses lockdown outputs in generated proxy policy and updates tests.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/determine_automatic_lockdown.cjs |
Resolves visibility-aware policy defaults. |
actions/setup/js/determine_automatic_lockdown.test.cjs |
Tests public defaults and opt-in behavior. |
pkg/workflow/compiler_github_mcp_steps.go |
Propagates the opt-in environment variable. |
pkg/workflow/compiler_github_mcp_steps_test.go |
Tests opt-in propagation. |
pkg/workflow/compiler_difc_proxy.go |
Generates a runtime-aware CLI proxy policy. |
pkg/workflow/compiler_difc_proxy_test.go |
Tests dynamic policy generation. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/pi.golden |
Updates expected compiled workflow output. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 3
- Review effort level: Medium
| policyJSON := getDIFCProxyPolicyJSON(githubToolConfig, data, data.SandboxConfig.MCP) | ||
| if policyJSON == "" { | ||
| policyJSON = defaultCliProxyPolicyJSON | ||
| difcProxyLog.Print("No guard policy configured, using default CLI proxy policy") | ||
| policyJSON = defaultCliProxyPolicyJSONTemplate |
| // Default to safe guard policy for public repos on error | ||
| core.setOutput("min_integrity", "approved"); | ||
| core.setOutput("repos", "all"); | ||
| core.setOutput("repos", "public"); |
| } else { | ||
| core.info("Automatic guard policy determination complete for public repository"); | ||
| core.info("GitHub MCP guard policy automatically applied for public repository. " + "min-integrity='approved' and repos='all' ensure only approved-integrity content is accessible."); | ||
| core.info("GitHub MCP guard policy automatically applied for public repository. " + "min-integrity='approved' and repos='public' restrict access to public repositories only."); |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #45158 does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (60 additions across 7 files, threshold is 100). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
🧪 Test Quality Sentinel Report✅ Test Quality Score: 89/100 — Excellent
📊 Metrics (8 tests)
Verdict
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Fix CLI proxy default policy to respect repo visibility
The changes are correct, well-tested, and consistent with the existing MCP Gateway pattern.
Key observations:
- The core fix in
determine_automatic_lockdown.cjscorrectly closes the privilege escalation gap where a public-repo workflow could reach private repos via the CLI proxy. - The template expressions inside the single-quoted YAML string are evaluated by GitHub Actions at the expression layer before the shell sees the value, so the quoting is correct and safe.
- Step ordering is verified:
determine-automatic-lockdownruns beforeStart CLI proxyin the compiled output, so outputs are always available. - The opt-in path (
private-to-public-flows: allow) is correctly threaded: compiler emitsGH_AW_PRIVATE_TO_PUBLIC_FLOWS=allowin the step env, which the lockdown script reads to outputrepos=all. - Error fallback now uses
repos=publicinstead ofrepos=all— a security improvement for ambiguous-visibility failures. - Tests cover: public default, error fallback, explicit opt-in, and golden output regression.
No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 40.9 AIC · ⌖ 4.36 AIC · ⊞ 4.8K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — clean security fix with good test coverage; leaving comments on two actionable gaps.
📋 Key Themes & Highlights
Key Themes
- Error-path inconsistency (
determine_automatic_lockdown.cjs:120): Thecatchblock always falls back torepos=public, ignoringGH_AW_PRIVATE_TO_PUBLIC_FLOWS=allow. A workflow that explicitly opts into cross-visibility flows will have that opt-in silently dropped on transient API failure. - Silent typo swallowing (
determine_automatic_lockdown.cjs:67): The opt-in value is compared with strict equality against"allow"but unrecognised values produce no warning — a one-character typo in the frontmatter disables the opt-in without any signal to the author. - Test assertion gap (
determine_automatic_lockdown.test.cjs:307): The new opt-in test is present and correct, but omits themin_integrityassertion for the cross-visibility case.
Positive Highlights
- ✅ The core fix is correct and complete:
repos=publicdefault for public repos closes the private-repo exposure. - ✅ Excellent symmetry with the MCP Gateway: using step-output references in
defaultCliProxyPolicyJSONTemplatecleanly mirrors$GITHUB_MCP_GUARD_REPOS. - ✅
quoteYAMLEnvValuecorrectly handles the new template value with${{ }}expressions — the golden-file update confirms the output is well-formed. - ✅ Tests across all three layers (JS unit, Go unit, golden file) are updated and the new opt-in test case directly specifies the intended behaviour.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 50.1 AIC · ⌖ 4.79 AIC · ⊞ 6.6K
Comment /matt to run again
| // Default to safe guard policy for public repos on error | ||
| core.setOutput("min_integrity", "approved"); | ||
| core.setOutput("repos", "all"); | ||
| core.setOutput("repos", "public"); |
There was a problem hiding this comment.
[/diagnosing-bugs] Error fallback ignores GH_AW_PRIVATE_TO_PUBLIC_FLOWS — a workflow that opted into cross-visibility flows will silently fall back to repos=public on API failure, potentially breaking the workflow in a non-obvious way.
💡 Suggested fix
Read the env var in the catch block too:
} catch (error) {
const privateToPublicFlows = process.env.GH_AW_PRIVATE_TO_PUBLIC_FLOWS || "";
const fallbackRepos = privateToPublicFlows === "allow" ? "all" : "public";
core.setOutput("repos", fallbackRepos);
// ... rest of error handling
}A test covering GH_AW_PRIVATE_TO_PUBLIC_FLOWS=allow + API failure would lock this in.
@copilot please address this.
| // workflow author has explicitly opted in via private-to-public-flows: allow. | ||
| // Private/internal repos default to repos=all (no cross-visibility restriction). | ||
| const privateToPublicFlows = process.env.GH_AW_PRIVATE_TO_PUBLIC_FLOWS || ""; | ||
| const defaultRepos = isPrivate || privateToPublicFlows === "allow" ? "all" : "public"; |
There was a problem hiding this comment.
[/tdd] The opt-in condition privateToPublicFlows === "allow" silently ignores any other non-empty value (e.g. typos like "Allow" or "yes"). This should either be documented as intentional or validated with a warning.
💡 Suggested improvement
Emit a warning for unrecognised values so authors get immediate feedback on typos:
const privateToPublicFlows = process.env.GH_AW_PRIVATE_TO_PUBLIC_FLOWS || "";
if (privateToPublicFlows && privateToPublicFlows !== "allow") {
core.warning(`GH_AW_PRIVATE_TO_PUBLIC_FLOWS='${privateToPublicFlows}' is not recognised; expected 'allow'. Treating as unset.`);
}
const defaultRepos = isPrivate || privateToPublicFlows === "allow" ? "all" : "public";A test for GH_AW_PRIVATE_TO_PUBLIC_FLOWS=Allow (wrong case) would complete the specification.
@copilot please address this.
| expect(mockCore.setOutput).toHaveBeenCalledWith("repos", "all"); | ||
| expect(mockCore.setOutput).toHaveBeenCalledWith("min_integrity", "approved"); | ||
| expect(mockCore.setOutput).toHaveBeenCalledWith("visibility", "public"); | ||
| expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("repos not configured")); |
There was a problem hiding this comment.
[/tdd] The new opt-in test asserts that repos=all is output, but it doesn't assert that min_integrity is still approved for a public repo with the opt-in flag. Adding this assertion strengthens the spec for the cross-visibility case.
💡 Suggested addition
expect(mockCore.setOutput).toHaveBeenCalledWith("min_integrity", "approved");
expect(mockCore.setOutput).toHaveBeenCalledWith("visibility", "public");
expect(mockCore.setOutput).toHaveBeenCalledWith("repos", "all");
// Also assert info log mentions opt-in path for observability
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("repos not configured"));The last line is already present — just make min_integrity explicit too.
@copilot please address this.
There was a problem hiding this comment.
COMMENT — correct fix, two non-blocking concerns
The core change is correct and well-structured: switching the CLI proxy default from the hardcoded permissive repos=all to a visibility-aware runtime value via determine-automatic-lockdown step outputs closes a real gap (public-repo workflows could previously read private repos through the CLI proxy). The approach mirrors how the MCP Gateway already handles this.
Review themes
Concern 1 (medium): Unquoted env value — GH_AW_PRIVATE_TO_PUBLIC_FLOWS: allow is emitted as a bare scalar while every adjacent env is wrapped in quoteYAMLEnvValue. Not a bug today, but inconsistent and fragile. See inline comment on line 88.
Concern 2 (medium, security-adjacent): Silent opt-in failure — unrecognized private-to-public-flows values (typos, wrong case) are silently treated as disabled. A workflow author who misspells the value gets repos=public with no error or warning, believing cross-visibility flows are enabled when they are not. Compile-time validation of the enum value would catch this. See inline comment on line 69.
Both existing review comments (log message accuracy at line 91, missing opt-in check in error fallback at line 120) are valid and worth addressing before merge.
🔎 Code quality review by PR Code Quality Reviewer · 77.5 AIC · ⌖ 4.63 AIC · ⊞ 5.4K
Comment /review to run again
| fmt.Fprintf(yaml, " GH_AW_GITHUB_REPOS: %s\n", quoteYAMLEnvValue(configuredRepos)) | ||
| } | ||
| if privateToPublicFlowsAllow { | ||
| yaml.WriteString(" GH_AW_PRIVATE_TO_PUBLIC_FLOWS: allow\n") |
There was a problem hiding this comment.
Unquoted bare YAML scalar is inconsistent with the rest of this block and fragile if the value ever changes. Every adjacent env emission uses quoteYAMLEnvValue; this one does not.
💡 Suggested fix
yaml.WriteString(" GH_AW_PRIVATE_TO_PUBLIC_FLOWS: " + quoteYAMLEnvValue("allow") + "\n")allow is not a YAML boolean keyword, so this is not a bug today. But the inconsistency looks like an oversight and sets a fragile precedent — if this value ever changes to yes, on, or true, the unquoted scalar will be misinterpreted as a boolean by YAML 1.1 parsers (used by some Actions runners).
| // When set to "allow", the user has explicitly opted in to cross-visibility data | ||
| // flows, so the repos default should be "all" rather than "public" even for | ||
| // public repositories. | ||
| if ptpFlows, _ := toolConfig["private-to-public-flows"].(string); ptpFlows == "allow" { |
There was a problem hiding this comment.
Typos or casing variants in private-to-public-flows are silently ignored, making it look like the opt-in succeeded when it did not. This is a security-adjacent concern: a workflow author who writes private-to-public-flows: Allow or private-to-public-flows: yes will see no error, no warning, and will get repos=public — unexpectedly blocking data flows they believed they had enabled.
💡 Details
The compiler check (line 69) accepts only the exact string "allow" with no validation for other values. The runtime check in determine_automatic_lockdown.cjs mirrors this with a strict === 'allow' comparison.
Two improvements:
-
At compile time: validate that the value is exactly
"allow"(or a recognized set) and return a compile error for unrecognized values, rather than silently treating them as disabled. This is consistent with how other enum fields in the frontmatter are handled. -
At runtime: since the env var is compiler-emitted and only ever set to
allow, the JS check is fine as-is — but add a comment noting that the value is an enum, not a free-form string, so a future contributor does not add a.trim().toLowerCase()that weakens the gate without realizing it.
|
@copilot run pr-finisher skill |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Lock files will be recompiled separately after the source changes are merged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
📰 BREAKING: Smoke Copilot - AOAI (apikey) is now investigating this pull request. Sources say the story is developing... |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
Smoke test 29222894126
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "proxy.golang.org"See Network Configuration for more information.
|
Smoke Test Results: FAIL
Run URL: https://github.com/github/gh-aw/actions/runs/29222894208 Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
|
Caution agentic threat detected DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. Pull request created: #45169
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
|
Smoke: Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
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.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
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.
|
There was a problem hiding this comment.
Smoke review completed for run 29222934137.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist 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 · 98.9 AIC · ⌖ 4.8 AIC · ⊞ 19.1K
Comment /smoke-copilot to run again
Add label smoke to run again
💥 Smoke Test: Claude — Run 29222894214Core #1-12: all ✅ PR Review #13-19: 13✅ 14✅ 15✅ 16✅ 17✅ 18 Overall: PARTIAL — all executed tests passed; #18 blocked by pre-existing branch history, #19 skipped. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
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.
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist 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 · 122.7 AIC · ⌖ 14.7 AIC · ⊞ 8.4K
Comment /smoke-claude to run again
| // detect whether each field is already configured and avoid overriding it. | ||
| configuredMinIntegrity := "" | ||
| configuredRepos := "" | ||
| privateToPublicFlowsAllow := false |
There was a problem hiding this comment.
Nice — introducing the privateToPublicFlowsAllow flag here keeps the default-repos logic explicit and readable.
| // Public repos default to repos=public to block access to private repos unless the | ||
| // workflow author has explicitly opted in via private-to-public-flows: allow. | ||
| // Private/internal repos default to repos=all (no cross-visibility restriction). | ||
| const privateToPublicFlows = process.env.GH_AW_PRIVATE_TO_PUBLIC_FLOWS || ""; |
There was a problem hiding this comment.
Reading the private-to-public-flows env here is clear; consider a brief inline note that empty defaults to the restrictive path.
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Completed a pr-finisher pass and pushed updates in 3174273. I triaged PR state/CI, applied the actionable follow-up fixes, ran local validation ( |
|
🎉 This pull request is included in a new release. Release: |
The CLI proxy's fallback policy (
repos="all") allowed theghCLI inside AWF to read any repo the token could access — including private repos — when running in a public repository with no explicitreposguard policy configured. The MCP Gateway already handled this correctly via itsforcePublicReposruntime override; the CLI proxy had no equivalent.What changed
determine_automatic_lockdown.cjs— visibility-awarereposdefaultdefaultReposis nowisPrivate ? "all" : "public"instead of always"all"GH_AW_PRIVATE_TO_PUBLIC_FLOWSenv var support: when set to"allow", overrides the public-repo default back to"all"(user opted in to cross-visibility flows)repos="public"(consistent withvisibility="public"conservative default)compiler_github_mcp_steps.go— propagate opt-in flagtools.github.private-to-public-flows: allowis set, emitsGH_AW_PRIVATE_TO_PUBLIC_FLOWS: allowinto thedetermine-automatic-lockdownstep env so the script can outputrepos=allcompiler_difc_proxy.go— dynamic default policy templateReplaces the hardcoded
repos="all"fallback with a template referencing runtime step outputs:This mirrors how
GITHUB_MCP_GUARD_REPOSis already set from the same step output for the MCP Gateway path.Tests
determine_automatic_lockdown.test.cjs,compiler_difc_proxy_test.go, andcompiler_github_mcp_steps_test.gopi.goldenupdated to match new compiler outputBehavior matrix
reposin frontmatterreposvalue"public"← fixed"all""all"(explicit passthrough)["org/*"]["org/*"]private-to-public-flows: allow"all"(opted in)"all"Related
✨ PR Review Safe Output Test - Run 29222894214
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.comSee Network Configuration for more information.