Disable forcePublicRepos for enclave-only GitHub MCP backends - #60016
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
✅ This PR is from a trusted core team contributor using the Copilot coding agent. Per the project's CONTRIBUTING.md, internal agentic development is the standard workflow. The change looks ready for team review. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
✅ Ponytail Reviewer completed successfully! Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"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.
Nice targeted fix overall; only trim opportunities are comment verbosity in code paths that can be documented once in specs/docs.
net: -20 lines possible.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
ab.chatgpt.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
Generated by ✂️ Ponytail Reviewer for #60016 · codex · gpt53codex · 3.18 AIC · ⌖ 4.03 AIC · ⊞ 12.8K
Comment /ponytail to run again
| // whole gateway, so it must stay enabled to protect the primary agent's read path. In a | ||
| // public repository that override rewrites the enclave's allow-only scope to repos="public", | ||
| // which silently discards enclaves[].agent.tools.github.allowed-repos and leaves the enclave | ||
| // unable to read the declared private repositories. |
There was a problem hiding this comment.
L363: shrink: 7-line explanatory warning prose in code comments. Move rationale to docs and keep a 1-line function comment plus concise warning string.
| } | ||
| } | ||
|
|
||
| // A static GitHub agent enclave is the mechanism that makes private-to-public |
There was a problem hiding this comment.
L221: yagni: long policy narrative embedded in runtime builder comment. Keep intent in docs/spec and reduce to a short guard comment next to the condition.
There was a problem hiding this comment.
🟡 Changes recommended
The warning path misreports explicit opt-outs and bypasses compiler warning accounting.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Disables the gateway’s public-repository override for enclave-only GitHub MCP backends while warning about shared primary-agent access.
Changes:
- Adds enclave-aware
forcePublicReposconfiguration. - Adds validation warnings and regression tests.
- Updates documentation and release notes.
File summaries
| File | Description |
|---|---|
pkg/workflow/mcp_gateway_config.go |
Disables the override for enclave-only backends. |
pkg/workflow/enclaves.go |
Adds the shared-backend warning. |
pkg/workflow/enclave_github_proxy_test.go |
Tests configuration and warnings. |
docs/src/content/docs/reference/mcp-gateway.md |
Documents gateway behavior. |
docs/src/content/docs/experimental/enclaves.md |
Documents enclave configuration. |
.changeset/enclave-force-public-repos.md |
Records the patch change. |
Review details
Suppressed comments (1)
pkg/workflow/enclaves.go:365
- When
tools.github.private-to-public-flows: allowis set,buildMCPGatewayConfigalready disablesforcePublicRepos, but this still emits a warning claiming the override remains enabled and the enclave reads nothing. Suppress the warning for that explicit opt-out and cover this combination in the test.
if !enclaveGitHubIssuesEnabled(workflowData) || !primaryGitHubMCPEnabled(workflowData) {
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if warning := staticEnclaveGitHubScopeOverrideWarning(workflowData); warning != "" { | ||
| fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warning)) |
|
|
||
| Set `tools.github: false` so the GitHub MCP server is rendered for the enclave identity only. When primary-agent GitHub access is disabled, the compiler emits `"forcePublicRepos": false` in the gateway config: the gateway's runtime public-repos override would otherwise rewrite the enclave's allow-only scope to `repos: "public"` in a public repository, silently discarding `allowed-repos` and leaving the enclave with nothing to read. Disclosure stays bounded by the enclave's sensitivity ledger, `max-output-bytes`, and `max-invocations`, and safe outputs keep their explicit `sink-visibility` enforcement. | ||
|
|
||
| If the primary agent also enables `tools.github`, the override must stay on to protect the primary read path, and the compiler warns that the enclave cannot read the declared private repositories in a public repository. |
| - API errors during visibility detection result in a non-fatal warning; the gateway/proxy falls back to the compiled policy. | ||
|
|
||
| **Opt-out**: Workflow authors who intentionally allow private→public data flows set `private-to-public-flows: allow` in frontmatter (Section 10.9). The compiler translates this to `gateway.forcePublicRepos: false` in the generated gateway JSON stdin config. For proxy mode, launchers should pass `--force-public-repos=false` when they need equivalent opt-out behavior. | ||
| **Opt-out**: Workflow authors who intentionally allow private→public data flows set `private-to-public-flows: allow` in frontmatter (Section 10.9). The compiler translates this to `gateway.forcePublicRepos: false` in the generated gateway JSON stdin config. The compiler also emits `gateway.forcePublicRepos: false` when the GitHub MCP server is rendered solely to serve a static agent enclave identity (`tools.github: false` plus `enclaves[].agent.tools.github`), because the override would otherwise discard the enclave's configured `allowed-repos`. For proxy mode, launchers should pass `--force-public-repos=false` when they need equivalent opt-out behavior. |
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 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
REQUEST_CHANGES
The enclave-only forcePublicRepos change itself makes sense, but the new compiler warning is not tied to the actual override state, so it now flags an explicitly supported opt-out path as broken.
Blocking theme
When a workflow combines a static GitHub enclave with primary tools.github and intentionally sets tools.github.private-to-public-flows: allow, buildMCPGatewayConfig already disables the runtime public-repos override. staticEnclaveGitHubScopeOverrideWarning does not account for that, so compilation still warns that the gateway will rewrite the scope to repos="public" and advises removing tools.github. That turns a valid configuration into a permanent false positive and makes the new diagnostic hard to trust.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 52 AIC · ⌖ 9.09 AIC · ⊞ 22.3K
Comment /review to run again
| if !enclaveGitHubIssuesEnabled(workflowData) || !primaryGitHubMCPEnabled(workflowData) { | ||
| return "" | ||
| } | ||
| return "enclaves: a static GitHub agent enclave is combined with primary 'tools.github'. In a public repository " + |
There was a problem hiding this comment.
This warning is a false positive for the documented tools.github.private-to-public-flows: allow opt-out, because buildMCPGatewayConfig already emits forcePublicRepos: false for that configuration and the enclave keeps its declared scope.
💡 Why this blocks the change
staticEnclaveGitHubScopeOverrideWarning only checks "static enclave + primary tools.github". It never checks whether the workflow has already disabled the runtime override via private-to-public-flows: allow, so the compiler will still print "the enclave reads nothing" and tell authors to remove tools.github even when the chosen configuration is the supported escape hatch.
Please key this warning off the effective forcePublicRepos state instead of the raw tool combination, and add a regression test for tools.github + static enclave + private-to-public-flows: allow so the warning only appears when the override really stays enabled.
| if !enclaveGitHubIssuesEnabled(workflowData) || !primaryGitHubMCPEnabled(workflowData) { | ||
| return "" | ||
| } | ||
| return "enclaves: a static GitHub agent enclave is combined with primary 'tools.github'. In a public repository " + |
There was a problem hiding this comment.
This warning is emitted unconditionally for any static-enclave-plus-primary-tools.github workflow, but the breakage you describe only occurs in public repositories, so private-repo compiles now get a false alarm too.
💡 Why this blocks the change
The warning text says the gateway rewrites the scope to repos="public" and leaves the enclave reading nothing, but that runtime override only does that on public repositories. On a private repository, this exact configuration does not hit the stated failure mode, so emitting the warning at compile time as a guaranteed problem is inaccurate and trains users to ignore it.
Please either gate the diagnostic on a public-repository context when that is knowable, or weaken the message so it clearly states this is a public-repo-only hazard instead of a universal failure.
There was a problem hiding this comment.
Reviewed with a correctness/security-first lens (backend compiler config change, not UI, so Impeccable's UI-focused modes weren't applicable — harden guidance on edge-state completeness was used instead).
Findings: none blocking.
forcePublicReposis only forced tofalsewhengithubBackendIsStaticEnclaveDelegationOnlyholds, and only when not already set fromprivate-to-public-flows— explicit author config still wins. ✅staticEnclaveGitHubScopeOverrideWarningcorrectly gates on bothenclaveGitHubIssuesEnabled(covers both the CLI-issues profile andtools.githubvariants) andprimaryGitHubMCPEnabled, and follows the existingconsole.FormatWarningMessageStderr+os.Stderrpattern used throughoutpkg/workflow. ✅- New tests cover enclave-only, primary-GitHub-enabled, and no-enclave cases, plus the warning path — all pass locally (
go test ./pkg/workflow/... -run 'TestBuildMCPGatewayConfigForcePublicReposForStaticEnclave|TestStaticEnclaveGitHubScopeOverrideWarning|TestGitHubGuardPoliciesFromStepSkipsEnclaveOnlyBackend|TestEnclaveGitHubToolsVersionGates'). ✅ - Docs (
experimental/enclaves.md,reference/mcp-gateway.md) and changeset accurately describe the new behavior. ✅
No inline comments needed.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 39.6 AIC · ⌖ 13.6 AIC · ⊞ 8.4K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd — approving with one non-blocking observation.
📋 Key Themes & Highlights
Key Themes
- Scope precisely bounded: The fix (
forcePublicRepos: falsefor enclave-only GitHub backends) is gated bygithubBackendIsStaticEnclaveDelegationOnly, which correctly requirestools.github: falseon the primary agent. This mirrors the existingprivate-to-public-flows: allowopt-out pattern already in the codebase, keeping the change consistent with prior art rather than introducing a new mechanism. - Fail-loud instead of fail-silent:
staticEnclaveGitHubScopeOverrideWarningcovers the case the compile-time fix can't resolve automatically (primary + enclave both wanting GitHub access) by naming the exact discarded config path (enclaves[].agent.tools.github.allowed-repos). This is a good instance of turning a previously silent failure into an actionable compile-time warning. - Test coverage is solid: Three explicit
buildMCPGatewayConfigcases (enclave-only, primary-enabled, no-enclave) plus a warning-emission test plus a lock-file substring assertion in the existing enclave-only compile test. All new/changed tests pass locally. - Minor gap (non-blocking): The dynamic-enclave delegation path (
githubBackendIsDynamicDelegationOnly) appears to share the sameforcePublicRepos-clobbers-scope risk, but this PR intentionally scopes the fix to the static enclave case. Left an inline comment suggesting a tracking issue.
Positive Highlights
- ✅
forcePublicRepos == nilguard ensures explicitprivate-to-public-flowsconfig always wins over the new auto-detection — no precedence surprises. - ✅ Docs (both
enclaves.mdand themcp-gateway.mdspec §4.1.3.8) were updated in the same PR, keeping the behavior change and its documentation in sync. - ✅ Changeset included.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 183.9 AIC · ⌖ 16.1 AIC · ⊞ 10.4K
Comment /matt to run again
| // which silently discards enclaves[].agent.tools.github.allowed-repos and leaves the enclave | ||
| // unable to read the declared private repositories. | ||
| func staticEnclaveGitHubScopeOverrideWarning(workflowData *WorkflowData) string { | ||
| if !enclaveGitHubIssuesEnabled(workflowData) || !primaryGitHubMCPEnabled(workflowData) { |
There was a problem hiding this comment.
[/codebase-design] Good targeted fix — this warning correctly names the discarded config path instead of failing silently, consistent with the codebase's existing pattern (see console.FormatWarningMessageStderr in agent_validation.go, compiler.go). One gap worth flagging for a follow-up: the dynamic-enclave delegation path (githubBackendIsDynamicDelegationOnly) shares the same forcePublicRepos-clobbers-enclave-scope risk as the static path fixed here, but neither buildMCPGatewayConfig nor this warning covers it — combining a dynamic GitHub enclave with primary tools.github would silently rewrite the delegated identity's allow-only scope the same way. Not blocking this PR (out of its stated scope), but worth a tracking issue so the dynamic path isn't left with the same silent-failure gap this PR just closed for static enclaves.
@copilot please address this.
|
@copilot address all review feedback |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
A static GitHub agent enclave declared in a public repository reads nothing: the gateway's
forcePublicRepossafety net rewrites the enclave'sallow-onlyguard torepos: "public"at startup, discarding the configuredallowed-repos. The enclave still starts, still debits the information budget, and still returns a schema-valid value — the failure is entirely silent, and the answer is ungrounded.The compiler already emits a correct per-server and per-agent policy for the enclave identity; it just never told the gateway not to clobber it.
Changes
pkg/workflow/mcp_gateway_config.go—buildMCPGatewayConfigemits"forcePublicRepos": falsewhengithubBackendIsStaticEnclaveDelegationOnly(workflowData)holds (tools.github: false+enclaves[].agent.tools.github). Only applied when the flag was not already derived fromtools.github.private-to-public-flows, so explicit author configuration wins.pkg/workflow/enclaves.go—staticEnclaveGitHubScopeOverrideWarning, emitted from enclave validation, covers the case the fix cannot: when the primary agent also enablestools.github, the override must stay on to protect the primary read path, so the enclave reads nothing. The warning names the discarded config path rather than failing silently.experimental/enclaves.mdand gateway spec §4.1.3.8 record the enclave exemption; changeset added."forcePublicRepos": falselock-file assertion in the existing enclave-only compile test, and a warning test.Why this is safe in the enclave-only case
forcePublicRepos: falseis gateway-global, so it is only applied when the GitHub MCP server exists solely for the enclave identity:tools.github: false);max-output-bytes, andmax-invocations;forcePublicRepos: falseonly removes mcpg's defaultsink-visibility="public", and every write-sink policy the compiler emits on this path already carries an explicitsink-visibility(${GH_AW_SINK_VISIBILITY}), so safe-outputs enforcement is unchanged.now compiles to a gateway config containing
"forcePublicRepos": false, so the enclave'sallow-onlyscope survives startup.Not addressed here
Issue items 2–5 (fail-closed logging in
overrideToPublicScope, enclave erroring on an empty tool inventory, ledger debits for read-free invocations, provenance derived from actual tool activity) and theagent-entrypoint.pystdout-artifact finding live ingithub/gh-aw-mcpgandgithub/gh-aw-firewall.