From 0f0a899737d56bb0e9d32c05cb90050b7dd34bdd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Jul 2026 17:01:25 +0000 Subject: [PATCH 1/5] Initial plan From 92434fb19b84200ad55e11b094f101535157d5f9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Jul 2026 17:23:39 +0000 Subject: [PATCH 2/5] feat: implement private-to-public-flows as tools.github frontmatter field - Add `private-to-public-flows` to the workflow frontmatter JSON schema under `tools.github`, accepting either `"allow"` (blanket opt-out) or an array of server ID strings (selective exemption) - Add `PrivateToPublicFlows any` field to `GitHubToolConfig` struct - Parse `private-to-public-flows` in `parseGitHubTool` (string and []string/[]any forms) - Add `ForcePublicRepos *bool` and `SinkVisibilityExemptServers []string` to `MCPGatewayRuntimeConfig` - In `buildMCPGatewayConfig`, translate `PrivateToPublicFlows` to the gateway config fields - Emit `"forcePublicRepos": false` and `"sinkVisibilityExemptServers"` in the gateway JSON section of the renderer - Reject `private-to-public-flows: allow` in strict mode (list form allowed per MCP Gateway Spec Section 10.9.4) - Add documentation section to github-tools.md - Add tests: parsing, gateway config building, emission, strict mode Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../content/docs/reference/github-tools.md | 43 +++ pkg/parser/schemas/main_workflow_schema.json | 100 +++--- pkg/workflow/mcp_gateway_config.go | 41 ++- pkg/workflow/mcp_renderer.go | 18 ++ pkg/workflow/private_to_public_flows_test.go | 301 ++++++++++++++++++ .../strict_mode_network_validation.go | 20 ++ pkg/workflow/tools_parser.go | 22 ++ pkg/workflow/tools_types.go | 16 + 8 files changed, 511 insertions(+), 50 deletions(-) create mode 100644 pkg/workflow/private_to_public_flows_test.go diff --git a/docs/src/content/docs/reference/github-tools.md b/docs/src/content/docs/reference/github-tools.md index 122fc8cce0d..a42798d2761 100644 --- a/docs/src/content/docs/reference/github-tools.md +++ b/docs/src/content/docs/reference/github-tools.md @@ -175,6 +175,49 @@ permissions: Alternatively, you can authenticate with a PAT or GitHub App. If using a GitHub App, add `vulnerability-alerts: read` to your workflow's `permissions:` field and ensure the GitHub App is configured with this permission. +## Cross-Visibility Opt-Out (`private-to-public-flows`) + +By default, the MCP Gateway enforces _cross-visibility protections_ that prevent private repository data from flowing into public-repository sinks. These protections are active whenever the workflow runs in a public repository: + +- **`forcePublicRepos`** — restricts the GitHub MCP server to public repositories only at runtime, preventing private-data secrecy tags from accumulating in the agent's context. +- **`sink-visibility` enforcement** — blocks any output to a sink whose visibility is `public` when the agent carries non-empty secrecy tags from private-repo reads. + +When you intentionally need private-to-public data flows, declare `private-to-public-flows` under `tools.github`. + +### Blanket opt-out (`allow`) + +```yaml +tools: + github: + private-to-public-flows: allow +``` + +This disables both `forcePublicRepos` and the default `sink-visibility` enforcement for **all** MCP servers. The MCP Gateway emits `"forcePublicRepos": false` in its startup config. + +:::caution +`private-to-public-flows: allow` is **not compatible with strict mode**. Strict mode workflows that require this opt-out must use the list form instead. +::: + +### Selective exemption (list of server IDs) + +```yaml +tools: + github: + private-to-public-flows: + - github-mcp-server + - my-custom-server +``` + +This exempts only the listed MCP server IDs from the default `sink-visibility` enforcement. `forcePublicRepos` is **not** disabled; private repo access still requires the allow-only policy to permit it. This form is compatible with strict mode. + +The compiler emits `"sinkVisibilityExemptServers": ["github-mcp-server", "my-custom-server"]` in the gateway config. + +### Security implications + +Opting out of cross-visibility protections means the agent may read from private repositories and write that data to public sinks (e.g., a public GitHub issue, a Slack channel). Only use this when your workflow explicitly requires it and you understand the data-flow implications. + +See [MCP Gateway Specification Section 10.9](../mcp-gateway#109-cross-visibility-opt-out-private-to-public-flows) for full protocol details. + ## Related Documentation - [Tools Reference](/gh-aw/reference/tools/) - All tool configurations diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 93313a4ae4d..49e36f5f333 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -23,7 +23,7 @@ "emoji": { "type": "string", "description": "Optional emoji to represent the workflow visually in listings and UI surfaces.", - "examples": ["🤖", "🔍", "🚀"] + "examples": ["\ud83e\udd16", "\ud83d\udd0d", "\ud83d\ude80"] }, "source": { "type": "string", @@ -687,7 +687,7 @@ { "type": "array", "minItems": 1, - "description": "Array of label names — any of these labels will trigger the workflow.", + "description": "Array of label names \u2014 any of these labels will trigger the workflow.", "items": { "type": "string", "minLength": 1, @@ -708,7 +708,7 @@ { "type": "array", "minItems": 1, - "description": "Array of label names — any of these labels will trigger the workflow.", + "description": "Array of label names \u2014 any of these labels will trigger the workflow.", "items": { "type": "string", "minLength": 1, @@ -1867,7 +1867,7 @@ "oneOf": [ { "type": "null", - "description": "Bare key with no value — equivalent to true. Skips workflow execution if any CI checks on the target branch are currently failing." + "description": "Bare key with no value \u2014 equivalent to true. Skips workflow execution if any CI checks on the target branch are currently failing." }, { "type": "boolean", @@ -1963,12 +1963,12 @@ } }, "roles": { - "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (⚠️ security consideration).", + "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (\u26a0\ufe0f security consideration).", "oneOf": [ { "type": "string", "enum": ["admin", "maintainer", "maintain", "write", "triage", "read", "all"], - "description": "Single repository permission level that can trigger the workflow. Use 'all' to allow any authenticated user (⚠️ disables permission checking entirely - use with caution)" + "description": "Single repository permission level that can trigger the workflow. Use 'all' to allow any authenticated user (\u26a0\ufe0f disables permission checking entirely - use with caution)" }, { "type": "array", @@ -1993,7 +1993,7 @@ } }, "labels": { - "description": "Filter workflows triggered by pull_request_target (or other labeled events) to only fire when the triggering label matches one of these names. Generates a job-level if: condition on the pre-activation job so unmatched label events show as Skipped (⊘) rather than Failed (❌).", + "description": "Filter workflows triggered by pull_request_target (or other labeled events) to only fire when the triggering label matches one of these names. Generates a job-level if: condition on the pre-activation job so unmatched label events show as Skipped (\u2298) rather than Failed (\u274c).", "oneOf": [ { "$ref": "#/$defs/non_empty_string", @@ -2012,7 +2012,7 @@ }, "allow-bot-authored-trigger-comment": { "type": "boolean", - "description": "Allow the bot-posted-menu / user-checks-box pattern: when a workflow posts a checkbox-menu comment as a GitHub App bot and a human maintainer edits it to tick a box (issue_comment:edited where actor ≠ comment.user.login), treat this as safe and skip the confused-deputy check. When false (default), the check applies to all issue_comment events. The Dependabot confused-deputy attack vector (issue_comment:created) is unaffected." + "description": "Allow the bot-posted-menu / user-checks-box pattern: when a workflow posts a checkbox-menu comment as a GitHub App bot and a human maintainer edits it to tick a box (issue_comment:edited where actor \u2260 comment.user.login), treat this as safe and skip the confused-deputy check. When false (default), the check applies to all issue_comment events. The Dependabot confused-deputy attack vector (issue_comment:created) is unaffected." }, "manual-approval": { "type": "string", @@ -3314,7 +3314,7 @@ }, "network": { "$comment": "Strict mode requirements: When strict=true, the 'network' field must be present (not null/undefined) and cannot contain standalone wildcard '*' in allowed domains (but patterns like '*.example.com' ARE allowed). This is validated in Go code (pkg/workflow/strict_mode_validation.go) via validateStrictNetwork().", - "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' — 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", + "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' \u2014 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", "examples": [ "defaults", { @@ -3463,7 +3463,7 @@ }, "runtime": { "type": "string", - "description": "Container runtime for the agent container. Use 'gvisor' to run the agent under gVisor's runsc runtime for additional kernel-level isolation. Use 'docker-sbx' to run the agent inside a Docker sbx microVM with KVM hypervisor-level isolation — requires sandbox.agent.sudo: true, DOCKER_PAT and DOCKER_USERNAME secrets, and a KVM-capable runner. Incompatible with runner.topology: arc-dind.", + "description": "Container runtime for the agent container. Use 'gvisor' to run the agent under gVisor's runsc runtime for additional kernel-level isolation. Use 'docker-sbx' to run the agent inside a Docker sbx microVM with KVM hypervisor-level isolation \u2014 requires sandbox.agent.sudo: true, DOCKER_PAT and DOCKER_USERNAME secrets, and a KVM-capable runner. Incompatible with runner.topology: arc-dind.", "enum": ["gvisor", "docker-sbx"], "examples": ["gvisor", "docker-sbx"] }, @@ -3833,7 +3833,7 @@ [ { "name": "Verify Post-Steps Execution", - "run": "echo \"✅ Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" + "run": "echo \"\u2705 Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" }, { "name": "Upload Test Results", @@ -3889,7 +3889,7 @@ "type": "integer", "minimum": 1, "default": 5, - "description": "Maximum number of consecutive AWF cache misses allowed before the API proxy blocks further requests. Maps to `apiProxy.maxCacheMisses`. Precedence: frontmatter value → `GH_AW_DEFAULT_MAX_TURN_CACHE_MISSES` env override → built-in default `5`." + "description": "Maximum number of consecutive AWF cache misses allowed before the API proxy blocks further requests. Maps to `apiProxy.maxCacheMisses`. Precedence: frontmatter value \u2192 `GH_AW_DEFAULT_MAX_TURN_CACHE_MISSES` env override \u2192 built-in default `5`." }, "max-daily-ai-credits": { "$ref": "#/$defs/max_daily_ai_credits_limit", @@ -4211,6 +4211,26 @@ "github-app": { "$ref": "#/$defs/github_app", "description": "GitHub App configuration for token minting. When configured, a GitHub App installation access token is minted at workflow start and used instead of the default token. This token overrides any custom github-token setting and provides fine-grained permissions matching the agent job requirements." + }, + "private-to-public-flows": { + "description": "Opts out of cross-visibility protections for private-to-public data flows. Set to 'allow' for a blanket opt-out (disables forcePublicRepos and default sink-visibility enforcement), or provide an array of MCP server IDs to exempt only those servers from default sink-visibility enforcement. Incompatible with strict mode when set to 'allow'. See MCP Gateway Specification Section 10.9.", + "oneOf": [ + { + "type": "string", + "enum": ["allow"], + "description": "Blanket opt-out: disables forcePublicRepos and sink-visibility enforcement for all servers. Not allowed in strict mode." + }, + { + "type": "array", + "description": "Selective exemption: disables default sink-visibility enforcement for the listed MCP server IDs only. Compatible with strict mode.", + "items": { + "type": "string", + "minLength": 1, + "description": "MCP server ID to exempt from sink-visibility enforcement" + }, + "minItems": 1 + } + ] } }, "additionalProperties": false, @@ -4364,7 +4384,7 @@ }, "mode": { "type": "string", - "description": "Integration mode: 'cli' (recommended) installs @playwright/cli via npm for token-efficient CLI invocations — use playwright-cli commands in bash and localhost to reach local servers; 'mcp' (deprecated) runs a Docker-based MCP server.", + "description": "Integration mode: 'cli' (recommended) installs @playwright/cli via npm for token-efficient CLI invocations \u2014 use playwright-cli commands in bash and localhost to reach local servers; 'mcp' (deprecated) runs a Docker-based MCP server.", "enum": ["cli", "mcp"] } }, @@ -4875,7 +4895,7 @@ }, "lsp": { "type": "object", - "description": "⚠️ Experimental. Top-level Language Server Protocol (LSP) configuration for Copilot CLI. Each key is a language identifier and each value defines the server command, args, and file extension mappings. Using this field emits a compile-time warning.", + "description": "\u26a0\ufe0f Experimental. Top-level Language Server Protocol (LSP) configuration for Copilot CLI. Each key is a language identifier and each value defines the server command, args, and file extension mappings. Using this field emits a compile-time warning.", "patternProperties": { "^[a-zA-Z0-9_-]+$": { "type": "object", @@ -5165,7 +5185,7 @@ "$ref": "#/$defs/templatable_bool_or_int" } ], - "description": "Title-based deduplication for create-issue. Set to true for exact title matching, or provide a non-negative integer (0–100) to deduplicate by Levenshtein edit distance (e.g., 1 allows one-character differences). Accepts a GitHub Actions expression that resolves to a boolean or integer at runtime. Applies within-run and against open/recently-closed repository issues." + "description": "Title-based deduplication for create-issue. Set to true for exact title matching, or provide a non-negative integer (0\u2013100) to deduplicate by Levenshtein edit distance (e.g., 1 allows one-character differences). Accepts a GitHub Actions expression that resolves to a boolean or integer at runtime. Applies within-run and against open/recently-closed repository issues." }, "target-repo": { "type": "string", @@ -5251,7 +5271,7 @@ }, "normalize-closing-keywords": { "type": "boolean", - "description": "When true, strip backticks from recognized issue-closing keywords (e.g. `Closes #1` → Closes #1) in body fields for this output type.", + "description": "When true, strip backticks from recognized issue-closing keywords (e.g. `Closes #1` \u2192 Closes #1) in body fields for this output type.", "examples": [true, false] } }, @@ -6587,7 +6607,7 @@ }, "normalize-closing-keywords": { "type": "boolean", - "description": "When true, strip backticks from recognized issue-closing keywords (e.g. `Closes #1` → Closes #1) in body fields for this output type.", + "description": "When true, strip backticks from recognized issue-closing keywords (e.g. `Closes #1` \u2192 Closes #1) in body fields for this output type.", "examples": [true, false] } }, @@ -6912,14 +6932,14 @@ "description": "Object form for granular control over the protected-file set. Use the exclude list to remove specific files from the default protection while keeping the rest." } ], - "description": "Controls protected-file protection. String form: request_review (default), blocked, allowed, or fallback-to-issue — or a GitHub Actions expression for reusable workflows. Object form: { policy, exclude } to customise the protected-file set." + "description": "Controls protected-file protection. String form: request_review (default), blocked, allowed, or fallback-to-issue \u2014 or a GitHub Actions expression for reusable workflows. Object form: { policy, exclude } to customise the protected-file set." }, "allowed-files": { "type": "array", "items": { "type": "string" }, - "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern — files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern \u2014 files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." }, "preserve-branch-name": { "type": "boolean", @@ -6988,7 +7008,7 @@ }, "normalize-closing-keywords": { "type": "boolean", - "description": "When true, strip backticks from recognized issue-closing keywords (e.g. `Closes #1` → Closes #1) in body fields for this output type.", + "description": "When true, strip backticks from recognized issue-closing keywords (e.g. `Closes #1` \u2192 Closes #1) in body fields for this output type.", "examples": [true, false] } }, @@ -9022,14 +9042,14 @@ "description": "Object form for granular control over the protected-file set. Use the exclude list to remove specific files from the default protection while keeping the rest." } ], - "description": "Controls protected-file protection. String form: blocked (default), allowed, or fallback-to-issue — or a GitHub Actions expression for reusable workflows. Object form: { policy, exclude } to customise the protected-file set." + "description": "Controls protected-file protection. String form: blocked (default), allowed, or fallback-to-issue \u2014 or a GitHub Actions expression for reusable workflows. Object form: { policy, exclude } to customise the protected-file set." }, "allowed-files": { "type": "array", "items": { "type": "string" }, - "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern — files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern \u2014 files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." }, "excluded-files": { "type": "array", @@ -10353,7 +10373,7 @@ }, "scripts": { "type": "object", - "description": "Inline JavaScript script handlers that run inside the consolidated safe-outputs job handler loop. Unlike 'jobs' (which create separate GitHub Actions jobs), scripts execute in-process alongside the built-in handlers. Users write only the body of the main function — the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };' automatically. Script names containing dashes will be automatically normalized to underscores (e.g., 'post-slack-message' becomes 'post_slack_message').", + "description": "Inline JavaScript script handlers that run inside the consolidated safe-outputs job handler loop. Unlike 'jobs' (which create separate GitHub Actions jobs), scripts execute in-process alongside the built-in handlers. Users write only the body of the main function \u2014 the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };' automatically. Script names containing dashes will be automatically normalized to underscores (e.g., 'post-slack-message' becomes 'post_slack_message').", "patternProperties": { "^[a-zA-Z_][a-zA-Z0-9_-]*$": { "type": "object", @@ -10411,7 +10431,7 @@ }, "script": { "type": "string", - "description": "JavaScript handler body. Write only the code that runs inside the handler for each item — the compiler generates the full outer wrapper including config input destructuring (`const { channel, message } = config;`) and the handler function (`return async function handleX(item, resolvedTemporaryIds) { ... }`). The body has access to `item` (runtime message with input values), `resolvedTemporaryIds` (map of temporary IDs), and config-destructured local variables for each declared input." + "description": "JavaScript handler body. Write only the code that runs inside the handler for each item \u2014 the compiler generates the full outer wrapper including config input destructuring (`const { channel, message } = config;`) and the handler function (`return async function handleX(item, resolvedTemporaryIds) { ... }`). The body has access to `item` (runtime message with input values), `resolvedTemporaryIds` (map of temporary IDs), and config-destructured local variables for each declared input." } }, "required": ["script"], @@ -10446,8 +10466,8 @@ }, "staged-title": { "type": "string", - "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '🎭 Preview: {operation}'", - "examples": ["🎭 Preview: {operation}", "## Staged Mode: {operation}"] + "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '\ud83c\udfad Preview: {operation}'", + "examples": ["\ud83c\udfad Preview: {operation}", "## Staged Mode: {operation}"] }, "staged-description": { "type": "string", @@ -10461,18 +10481,18 @@ }, "run-success": { "type": "string", - "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '✅ Agentic [{workflow_name}]({run_url}) completed successfully.'", - "examples": ["✅ Agentic [{workflow_name}]({run_url}) completed successfully.", "✅ [{workflow_name}]({run_url}) finished."] + "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.'", + "examples": ["\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.", "\u2705 [{workflow_name}]({run_url}) finished."] }, "run-failure": { "type": "string", - "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", - "examples": ["❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "❌ [{workflow_name}]({run_url}) {status}."] + "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", + "examples": ["\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "\u274c [{workflow_name}]({run_url}) {status}."] }, "detection-failure": { "type": "string", - "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", - "examples": ["⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "⚠️ Detection job failed in [{workflow_name}]({run_url})."] + "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", + "examples": ["\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "\u26a0\ufe0f Detection job failed in [{workflow_name}]({run_url})."] }, "agent-failure-issue": { "type": "string", @@ -10502,7 +10522,7 @@ "body-header": { "type": "string", "description": "Custom header text prepended to every message body generated by safe outputs (issues, comments, pull requests, discussions). Applied after any threat-detection caution alert and before the agent-generated content. Available placeholders: {workflow_name}, {run_url}.", - "examples": ["> ⚠️ This content was generated by [{workflow_name}]({run_url}).", "> 🤖 AI-generated output — please review before acting."] + "examples": ["> \u26a0\ufe0f This content was generated by [{workflow_name}]({run_url}).", "> \ud83e\udd16 AI-generated output \u2014 please review before acting."] }, "disclosure-header": { "oneOf": [ @@ -10513,10 +10533,10 @@ { "type": "string", "description": "Custom AI authorship disclosure header prepended to every message body. Available placeholders: {workflow_name}, {run_url}.", - "examples": ["> 🤖 This content was generated by [{workflow_name}]({run_url}) and was not written by the account owner."] + "examples": ["> \ud83e\udd16 This content was generated by [{workflow_name}]({run_url}) and was not written by the account owner."] } ], - "description": "AI authorship disclosure header prepended to every message body. Set to true for built-in default text, or provide a custom template string. Insertion order from top to bottom: threat-detection caution alert (if any) → disclosure-header → body-header → agent-generated content. Available placeholders: {workflow_name}, {run_url}." + "description": "AI authorship disclosure header prepended to every message body. Set to true for built-in default text, or provide a custom template string. Insertion order from top to bottom: threat-detection caution alert (if any) \u2192 disclosure-header \u2192 body-header \u2192 agent-generated content. Available placeholders: {workflow_name}, {run_url}." }, "append-only-comments": { "type": "boolean", @@ -10563,7 +10583,7 @@ }, "allowed-teams": { "type": "array", - "description": "List of team slugs whose members are always allowed to be mentioned. Accepts 'team-slug' (resolved against the current org) or 'org/team-slug' format. Team members are fetched from the GitHub API at runtime; bots are excluded. IMPORTANT: requires read:org scope — not available with the default GITHUB_TOKEN. Use a classic PAT with read:org, a fine-grained PAT with Members:Read, or a GitHub App with the Members:Read permission. Without the required scope, team lookups fail with a warning and those members are skipped.", + "description": "List of team slugs whose members are always allowed to be mentioned. Accepts 'team-slug' (resolved against the current org) or 'org/team-slug' format. Team members are fetched from the GitHub API at runtime; bots are excluded. IMPORTANT: requires read:org scope \u2014 not available with the default GITHUB_TOKEN. Use a classic PAT with read:org, a fine-grained PAT with Members:Read, or a GitHub App with the Members:Read permission. Without the required scope, team lookups fail with a warning and those members are skipped.", "items": { "type": "string", "minLength": 1 @@ -10876,11 +10896,11 @@ }, { "type": "object", - "description": "Configuration for replacing one label with another on issues/PRs in a single atomic operation. Enables clear state transitions (e.g. 'in-progress' → 'done').", + "description": "Configuration for replacing one label with another on issues/PRs in a single atomic operation. Enables clear state transitions (e.g. 'in-progress' \u2192 'done').", "properties": { "allowed-transitions": { "type": "array", - "description": "Optional list of allowed label state transitions. Each entry specifies a (from, to) pair that is permitted. When specified, the agent may ONLY perform the listed transitions, regardless of allowed-add/allowed-remove. Useful for enforcing a strict state machine (e.g. only 'in-review' → 'approved' is allowed).", + "description": "Optional list of allowed label state transitions. Each entry specifies a (from, to) pair that is permitted. When specified, the agent may ONLY perform the listed transitions, regardless of allowed-add/allowed-remove. Useful for enforcing a strict state machine (e.g. only 'in-review' \u2192 'approved' is allowed).", "items": { "type": "object", "properties": { @@ -11642,7 +11662,7 @@ ] }, "evals": { - "description": "⚠️ Experimental. BinEval binary evaluation questions to run after safe-outputs and before the conclusion job. Can be a plain list of questions (shorthand) or an object with questions, model, and runs-on fields.", + "description": "\u26a0\ufe0f Experimental. BinEval binary evaluation questions to run after safe-outputs and before the conclusion job. Can be a plain list of questions (shorthand) or an object with questions, model, and runs-on fields.", "oneOf": [ { "type": "array", @@ -11892,7 +11912,7 @@ ] }, "templatable_bool_or_int": { - "description": "A boolean or non-negative integer (0–100) value that may also be specified as a GitHub Actions expression string that resolves to a boolean or integer at runtime (e.g. '${{ inputs.dedup }}').", + "description": "A boolean or non-negative integer (0\u2013100) value that may also be specified as a GitHub Actions expression string that resolves to a boolean or integer at runtime (e.g. '${{ inputs.dedup }}').", "oneOf": [ { "type": "boolean" diff --git a/pkg/workflow/mcp_gateway_config.go b/pkg/workflow/mcp_gateway_config.go index 8f7440df976..42b1c517a9d 100644 --- a/pkg/workflow/mcp_gateway_config.go +++ b/pkg/workflow/mcp_gateway_config.go @@ -155,17 +155,38 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig sessionTimeout = workflowData.EngineConfig.MCPSessionTimeout toolTimeout = workflowData.EngineConfig.MCPToolTimeout } + + // Derive ForcePublicRepos and SinkVisibilityExemptServers from tools.github.private-to-public-flows. + var forcePublicRepos *bool + var sinkVisibilityExemptServers []string + if workflowData.ParsedTools != nil && workflowData.ParsedTools.GitHub != nil { + switch v := workflowData.ParsedTools.GitHub.PrivateToPublicFlows.(type) { + case string: + if v == "allow" { + // Blanket opt-out: disable the runtime public-repos override. + falseVal := false + forcePublicRepos = &falseVal + } + case []string: + if len(v) > 0 { + sinkVisibilityExemptServers = v + } + } + } + return &MCPGatewayRuntimeConfig{ - Port: int(DefaultMCPGatewayPort), // Will be formatted as "${MCP_GATEWAY_PORT}" in renderer - Domain: "${MCP_GATEWAY_DOMAIN}", // Gateway variable expression - APIKey: "${MCP_GATEWAY_API_KEY}", // Gateway variable expression - PayloadDir: "${MCP_GATEWAY_PAYLOAD_DIR}", // Gateway variable expression for payload directory - PayloadPathPrefix: workflowData.SandboxConfig.MCP.PayloadPathPrefix, // Optional path prefix for agent containers - PayloadSizeThreshold: payloadSizeThreshold, // Size threshold in bytes - TrustedBots: workflowData.SandboxConfig.MCP.TrustedBots, // Additional trusted bot identities from frontmatter - KeepaliveInterval: workflowData.SandboxConfig.MCP.KeepaliveInterval, // Keepalive interval from frontmatter (0=default, -1=disabled, >0=custom) - SessionTimeout: sessionTimeout, // Session timeout from engine.mcp.session-timeout (empty = gateway default 6h) - ToolTimeout: toolTimeout, // Tool timeout from engine.mcp.tool-timeout (empty = gateway built-in default 60s) + Port: int(DefaultMCPGatewayPort), // Will be formatted as "${MCP_GATEWAY_PORT}" in renderer + Domain: "${MCP_GATEWAY_DOMAIN}", // Gateway variable expression + APIKey: "${MCP_GATEWAY_API_KEY}", // Gateway variable expression + PayloadDir: "${MCP_GATEWAY_PAYLOAD_DIR}", // Gateway variable expression for payload directory + PayloadPathPrefix: workflowData.SandboxConfig.MCP.PayloadPathPrefix, // Optional path prefix for agent containers + PayloadSizeThreshold: payloadSizeThreshold, // Size threshold in bytes + TrustedBots: workflowData.SandboxConfig.MCP.TrustedBots, // Additional trusted bot identities from frontmatter + KeepaliveInterval: workflowData.SandboxConfig.MCP.KeepaliveInterval, // Keepalive interval from frontmatter (0=default, -1=disabled, >0=custom) + SessionTimeout: sessionTimeout, // Session timeout from engine.mcp.session-timeout (empty = gateway default 6h) + ToolTimeout: toolTimeout, // Tool timeout from engine.mcp.tool-timeout (empty = gateway built-in default 60s) + ForcePublicRepos: forcePublicRepos, // nil = default (true); &false = disable runtime public-repos override + SinkVisibilityExemptServers: sinkVisibilityExemptServers, // Server IDs exempt from default sink-visibility enforcement // OTLPEndpoint and OTLPHeaders are set from workflowData by injectOTLPConfig, which is // the fully resolved OTLP config (including imports). Using these fields ensures gateway // OTLP config honours observability defined in imported shared workflows. diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index 7fa438e1b66..795ed811c8d 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -207,6 +207,24 @@ func RenderJSONMCPConfig( } fmt.Fprintf(&configBuilder, ",\n \"toolTimeout\": %d", toolTimeoutSeconds) } + // Emit forcePublicRepos: false when private-to-public-flows: allow is declared. + // Only emitted when explicitly set to false; omitting the field lets the gateway default (true). + // See MCP Gateway Specification Section 4.1.3.8. + if options.GatewayConfig.ForcePublicRepos != nil && !*options.GatewayConfig.ForcePublicRepos { + configBuilder.WriteString(",\n \"forcePublicRepos\": false") + } + // Emit sinkVisibilityExemptServers when private-to-public-flows lists specific server IDs. + // See MCP Gateway Specification Section 10.9. + if len(options.GatewayConfig.SinkVisibilityExemptServers) > 0 { + configBuilder.WriteString(",\n \"sinkVisibilityExemptServers\": [") + for i, serverID := range options.GatewayConfig.SinkVisibilityExemptServers { + if i > 0 { + configBuilder.WriteString(", ") + } + fmt.Fprintf(&configBuilder, "%q", serverID) + } + configBuilder.WriteString("]") + } // When OTLP tracing is configured, add the opentelemetry section directly to the // gateway config. The endpoint is passed via the OTEL_EXPORTER_OTLP_ENDPOINT env var // (injected by injectOTLPConfig) so that secrets are never interpolated directly into diff --git a/pkg/workflow/private_to_public_flows_test.go b/pkg/workflow/private_to_public_flows_test.go new file mode 100644 index 00000000000..edaa075689d --- /dev/null +++ b/pkg/workflow/private_to_public_flows_test.go @@ -0,0 +1,301 @@ +//go:build !integration + +package workflow + +import ( + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestParseGitHubToolPrivateToPublicFlows tests parsing of tools.github.private-to-public-flows +// from frontmatter into GitHubToolConfig.PrivateToPublicFlows. +func TestParseGitHubToolPrivateToPublicFlows(t *testing.T) { + tests := []struct { + name string + input any + wantVal any + wantNil bool + }{ + { + name: "string allow", + input: map[string]any{"private-to-public-flows": "allow"}, + wantVal: "allow", + }, + { + name: "array of server IDs ([]any)", + input: map[string]any{"private-to-public-flows": []any{"github-mcp-server", "custom-server"}}, + wantVal: []string{"github-mcp-server", "custom-server"}, + }, + { + name: "array of server IDs ([]string)", + input: map[string]any{"private-to-public-flows": []string{"srv-a", "srv-b"}}, + wantVal: []string{"srv-a", "srv-b"}, + }, + { + name: "field absent", + input: map[string]any{}, + wantNil: true, + }, + { + name: "unsupported type ignored", + input: map[string]any{"private-to-public-flows": 42}, + wantNil: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := parseGitHubTool(tt.input) + require.NotNil(t, cfg, "parseGitHubTool should not return nil") + if tt.wantNil { + assert.Nil(t, cfg.PrivateToPublicFlows) + } else { + assert.Equal(t, tt.wantVal, cfg.PrivateToPublicFlows) + } + }) + } +} + +// TestBuildMCPGatewayConfigPrivateToPublicFlows verifies that buildMCPGatewayConfig correctly +// translates tools.github.private-to-public-flows into ForcePublicRepos / SinkVisibilityExemptServers +// on the returned MCPGatewayRuntimeConfig. +func TestBuildMCPGatewayConfigPrivateToPublicFlows(t *testing.T) { + falseVal := false + + tests := []struct { + name string + privateToPublicFlows any + wantForcePublicRepos *bool + wantSinkVisibilityExemptSvr []string + }{ + { + name: "allow: sets ForcePublicRepos to false", + privateToPublicFlows: "allow", + wantForcePublicRepos: &falseVal, + }, + { + name: "list: sets SinkVisibilityExemptServers", + privateToPublicFlows: []string{"github-mcp-server", "custom-srv"}, + wantSinkVisibilityExemptSvr: []string{"github-mcp-server", "custom-srv"}, + }, + { + name: "nil: no override fields set", + privateToPublicFlows: nil, + wantForcePublicRepos: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wd := &WorkflowData{ + ParsedTools: &Tools{ + GitHub: &GitHubToolConfig{ + ReadOnly: true, + PrivateToPublicFlows: tt.privateToPublicFlows, + }, + }, + SandboxConfig: &SandboxConfig{ + MCP: &MCPGatewayRuntimeConfig{}, + }, + } + + cfg := buildMCPGatewayConfig(wd) + require.NotNil(t, cfg) + + if tt.wantForcePublicRepos == nil { + assert.Nil(t, cfg.ForcePublicRepos, "ForcePublicRepos should be nil when not set") + } else { + require.NotNil(t, cfg.ForcePublicRepos, "ForcePublicRepos should be set") + assert.Equal(t, *tt.wantForcePublicRepos, *cfg.ForcePublicRepos) + } + + assert.Equal(t, tt.wantSinkVisibilityExemptSvr, cfg.SinkVisibilityExemptServers) + }) + } +} + +// TestPrivateToPublicFlowsGatewayEmission compiles a minimal workflow with +// tools.github.private-to-public-flows and checks that the rendered MCP gateway +// JSON contains the expected forcePublicRepos / sinkVisibilityExemptServers fields. +func TestPrivateToPublicFlowsGatewayEmission(t *testing.T) { + makeWorkflow := func(ptpFlows string) string { + return `--- +on: + workflow_dispatch: +strict: false +permissions: + contents: read +engine: copilot +tools: + github: + private-to-public-flows: ` + ptpFlows + ` +--- + +# Test workflow +` + } + + makeWorkflowListForm := func() string { + return `--- +on: + workflow_dispatch: +strict: false +permissions: + contents: read +engine: copilot +tools: + github: + private-to-public-flows: + - github-mcp-server + - custom-server +--- + +# Test workflow +` + } + + tests := []struct { + name string + content string + wantContains []string + wantNotContains []string + }{ + { + name: "allow form emits forcePublicRepos false", + content: makeWorkflow("allow"), + wantContains: []string{`"forcePublicRepos": false`}, + wantNotContains: []string{`"sinkVisibilityExemptServers"`}, + }, + { + name: "list form emits sinkVisibilityExemptServers", + content: makeWorkflowListForm(), + wantContains: []string{`"sinkVisibilityExemptServers": ["github-mcp-server", "custom-server"]`}, + wantNotContains: []string{`"forcePublicRepos"`}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpFile, err := os.CreateTemp("", "test-ptp-*.md") + require.NoError(t, err, "failed to create temp file") + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.WriteString(tt.content) + require.NoError(t, err) + tmpFile.Close() + + compiler := NewCompiler() + compiler.SetSkipValidation(true) + + workflowData, err := compiler.ParseWorkflowFile(tmpFile.Name()) + require.NoError(t, err, "parsing should succeed") + require.NotNil(t, workflowData) + + yaml, _, _, err := compiler.generateYAML(workflowData, tmpFile.Name()) + require.NoError(t, err, "YAML generation should succeed") + + for _, want := range tt.wantContains { + assert.Contains(t, yaml, want, + "expected YAML to contain %q\n\nYAML snippet:\n%s", + want, extractGatewaySection(yaml)) + } + for _, notWant := range tt.wantNotContains { + assert.NotContains(t, yaml, notWant, + "expected YAML NOT to contain %q\n\nYAML snippet:\n%s", + notWant, extractGatewaySection(yaml)) + } + }) + } +} + +// TestPrivateToPublicFlowsStrictModeValidation verifies that strict mode rejects +// private-to-public-flows: allow but permits the list form. +func TestPrivateToPublicFlowsStrictModeValidation(t *testing.T) { + makeWorkflow := func(ptpFlows string) string { + return `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +network: + allowed: + - github.com +tools: + github: + private-to-public-flows: ` + ptpFlows + ` +--- + +# Test workflow +` + } + + makeWorkflowListForm := func() string { + return `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +network: + allowed: + - github.com +tools: + github: + private-to-public-flows: + - my-server +--- + +# Test workflow +` + } + + t.Run("allow is rejected in strict mode", func(t *testing.T) { + tmpFile, err := os.CreateTemp("", "test-ptp-strict-*.md") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.WriteString(makeWorkflow("allow")) + require.NoError(t, err) + tmpFile.Close() + + compiler := NewCompiler() + compiler.SetStrictMode(true) + compiler.SetSkipValidation(false) + + err = compiler.CompileWorkflow(tmpFile.Name()) + require.Error(t, err, "strict mode should reject private-to-public-flows: allow") + assert.Contains(t, err.Error(), "private-to-public-flows") + }) + + t.Run("list form is accepted in strict mode", func(t *testing.T) { + tmpFile, err := os.CreateTemp("", "test-ptp-strict-list-*.md") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.WriteString(makeWorkflowListForm()) + require.NoError(t, err) + tmpFile.Close() + + compiler := NewCompiler() + compiler.SetStrictMode(true) + compiler.SetSkipValidation(false) + + err = compiler.CompileWorkflow(tmpFile.Name()) + require.NoError(t, err, "strict mode should accept list form of private-to-public-flows") + }) +} + +// extractGatewaySection extracts the gateway JSON section from compiled YAML for test diagnostics. +func extractGatewaySection(yaml string) string { + start := strings.Index(yaml, `"gateway"`) + if start == -1 { + return "(no gateway section found)" + } + end := min(start+500, len(yaml)) + return yaml[start:end] +} diff --git a/pkg/workflow/strict_mode_network_validation.go b/pkg/workflow/strict_mode_network_validation.go index 7c0e42a3721..14301feea0b 100644 --- a/pkg/workflow/strict_mode_network_validation.go +++ b/pkg/workflow/strict_mode_network_validation.go @@ -111,6 +111,26 @@ func (c *Compiler) validateStrictTools(frontmatter map[string]any) error { return nil } + // Reject private-to-public-flows: allow in strict mode. + // Per MCP Gateway Specification Section 10.9.4, the blanket "allow" value is incompatible + // with strict mode because it disables both forcePublicRepos and sink-visibility enforcement. + // The list form (specific server IDs) is allowed in strict mode. + if githubValue, hasGitHub := toolsMap["github"]; hasGitHub { + if githubMap, ok := githubValue.(map[string]any); ok { + if ptpFlows, exists := githubMap["private-to-public-flows"]; exists { + if ptpStr, ok := ptpFlows.(string); ok && ptpStr == "allow" { + strictModeValidationLog.Printf("private-to-public-flows: allow rejected in strict mode") + return NewValidationError( + "tools.github.private-to-public-flows", + ptpStr, + "strict mode: 'private-to-public-flows: allow' is not allowed; it disables forcePublicRepos and sink-visibility enforcement, which is incompatible with strict mode", + "To exempt specific MCP servers from sink-visibility enforcement in strict mode, use the list form:\n\ntools:\n github:\n private-to-public-flows:\n - my-server-id\n - other-server-id", + ) + } + } + } + } + // Check if cache-memory is configured with scope: repo cacheMemoryValue, hasCacheMemory := toolsMap["cache-memory"] if hasCacheMemory { diff --git a/pkg/workflow/tools_parser.go b/pkg/workflow/tools_parser.go index 3a59a7b703e..f8b246b15d1 100644 --- a/pkg/workflow/tools_parser.go +++ b/pkg/workflow/tools_parser.go @@ -389,6 +389,28 @@ func parseGitHubTool(val any) *GitHubToolConfig { config.EndorserMinIntegrity = endorserMinIntegrity } + // Parse private-to-public-flows: accepts "allow" (string) or []string of server IDs. + if rawPtP, ok := configMap["private-to-public-flows"]; ok { + switch v := rawPtP.(type) { + case string: + // "allow" is the only valid string value + config.PrivateToPublicFlows = v + case []any: + // Array of server ID strings + servers := make([]string, 0, len(v)) + for _, item := range v { + if s, ok := item.(string); ok { + servers = append(servers, s) + } + } + config.PrivateToPublicFlows = servers + case []string: + config.PrivateToPublicFlows = v + default: + toolsParserLog.Printf("Warning: private-to-public-flows has unsupported type %T, ignoring", rawPtP) + } + } + return config } diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index e4bf58fa86c..accfb69d4b1 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -366,6 +366,13 @@ type GitHubToolConfig struct { // and MCPG >= v0.2.18. // Valid values: "approved", "unapproved", "merged" EndorserMinIntegrity string `yaml:"endorser-min-integrity,omitempty"` + // PrivateToPublicFlows opts out of cross-visibility protections for private→public data flows. + // Accepts either the string "allow" (blanket opt-out) or a []string of MCP server IDs + // (selective exemption for those servers only). + // - "allow" → compiler emits gateway.forcePublicRepos: false; rejected in strict mode. + // - []string → compiler emits gateway.sinkVisibilityExemptServers with the listed IDs. + // See MCP Gateway Specification Section 10.9. + PrivateToPublicFlows any `yaml:"-"` } // PlaywrightToolConfig represents the configuration for the Playwright tool @@ -468,6 +475,15 @@ type MCPGatewayRuntimeConfig struct { ToolTimeout string `yaml:"tool-timeout,omitempty"` // Timeout for individual MCP tool calls as a Go duration string (e.g. "2m", "30s"); empty = gateway built-in default (60s) OTLPEndpoint string `yaml:"-"` // OTLP collector endpoint (derived from observability.otlp, not user-settable) OTLPHeaders string `yaml:"-"` // Raw OTLP HTTP headers string (derived from observability.otlp, not user-settable) + // ForcePublicRepos controls the gateway's runtime public-repo override. + // When set to a pointer to false, the compiler emits "forcePublicRepos": false in the gateway + // JSON config, disabling the runtime check that restricts repos to "public" when the + // workflow runs in a public repository. Set from tools.github.private-to-public-flows: allow. + ForcePublicRepos *bool `yaml:"-"` + // SinkVisibilityExemptServers is the list of MCP server IDs exempt from default + // sink-visibility="public" enforcement. Emitted as gateway.sinkVisibilityExemptServers. + // Set from tools.github.private-to-public-flows: [server-ids...]. + SinkVisibilityExemptServers []string `yaml:"-"` } // HasTool checks if a tool is present in the configuration From 95bc57e407609391ca2e59c8e4c6780044bca774 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Jul 2026 17:25:37 +0000 Subject: [PATCH 3/5] feat: address code review - improve warning message clarity Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/tools_parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/tools_parser.go b/pkg/workflow/tools_parser.go index f8b246b15d1..eb1d70ac554 100644 --- a/pkg/workflow/tools_parser.go +++ b/pkg/workflow/tools_parser.go @@ -407,7 +407,7 @@ func parseGitHubTool(val any) *GitHubToolConfig { case []string: config.PrivateToPublicFlows = v default: - toolsParserLog.Printf("Warning: private-to-public-flows has unsupported type %T, ignoring", rawPtP) + toolsParserLog.Printf("Warning: private-to-public-flows has unsupported type %T (expected string \"allow\" or array of server IDs), ignoring", rawPtP) } } From f5fcfe9357d29a8c18e42f9331f4dc37fc030701 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Jul 2026 17:59:25 +0000 Subject: [PATCH 4/5] fix: address code review - propagate allow into sink-visibility, validate server IDs, fix shell safety, update docs Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../content/docs/reference/github-tools.md | 10 +- .../src/content/docs/reference/mcp-gateway.md | 26 ++-- pkg/workflow/compiler_validators.go | 1 + pkg/workflow/mcp_github_config.go | 27 +++- pkg/workflow/mcp_renderer.go | 18 +++ pkg/workflow/private_to_public_flows_test.go | 116 ++++++++++++++++-- .../strict_mode_network_validation.go | 34 +++++ 7 files changed, 206 insertions(+), 26 deletions(-) diff --git a/docs/src/content/docs/reference/github-tools.md b/docs/src/content/docs/reference/github-tools.md index a42798d2761..f46a786ec22 100644 --- a/docs/src/content/docs/reference/github-tools.md +++ b/docs/src/content/docs/reference/github-tools.md @@ -204,19 +204,23 @@ This disables both `forcePublicRepos` and the default `sink-visibility` enforcem tools: github: private-to-public-flows: - - github-mcp-server + - github - my-custom-server +mcp-servers: + my-custom-server: + type: http + url: "http://localhost:9000/mcp" ``` This exempts only the listed MCP server IDs from the default `sink-visibility` enforcement. `forcePublicRepos` is **not** disabled; private repo access still requires the allow-only policy to permit it. This form is compatible with strict mode. -The compiler emits `"sinkVisibilityExemptServers": ["github-mcp-server", "my-custom-server"]` in the gateway config. +The compiler emits `"sinkVisibilityExemptServers": ["github", "my-custom-server"]` in the gateway config. The built-in GitHub MCP server ID is `github`. Custom server IDs match the key used in `mcp-servers`. ### Security implications Opting out of cross-visibility protections means the agent may read from private repositories and write that data to public sinks (e.g., a public GitHub issue, a Slack channel). Only use this when your workflow explicitly requires it and you understand the data-flow implications. -See [MCP Gateway Specification Section 10.9](../mcp-gateway#109-cross-visibility-opt-out-private-to-public-flows) for full protocol details. +See [MCP Gateway Specification Section 10.9](./mcp-gateway#109-cross-visibility-opt-out-private-to-public-flows) for full protocol details. ## Related Documentation diff --git a/docs/src/content/docs/reference/mcp-gateway.md b/docs/src/content/docs/reference/mcp-gateway.md index 77f0dde0d8b..ba1b035bcd5 100644 --- a/docs/src/content/docs/reference/mcp-gateway.md +++ b/docs/src/content/docs/reference/mcp-gateway.md @@ -1647,7 +1647,7 @@ As a defense-in-depth measure, the gateway forces `sink-visibility="public"` for ### 10.9 Cross-Visibility Opt-Out (`private-to-public-flows`) -Workflow authors who intentionally allow private→public data flows can opt out of cross-visibility protections by declaring `private-to-public-flows` in workflow frontmatter. It accepts either `allow` for a blanket opt-out that disables both `forcePublicRepos` and sink-visibility enforcement, or a list of server IDs to exempt only those servers from default sink-visibility enforcement. +Workflow authors who intentionally allow private→public data flows can opt out of cross-visibility protections by declaring `private-to-public-flows` under `tools.github` in workflow frontmatter. It accepts either `allow` for a blanket opt-out that disables both `forcePublicRepos` and sink-visibility enforcement, or a list of server IDs to exempt only those servers from default sink-visibility enforcement. #### 10.9.1 Workflow Frontmatter @@ -1655,9 +1655,8 @@ Workflow authors who intentionally allow private→public data flows can opt out ```yaml --- tools: - - github - - safe-outputs -private-to-public-flows: allow + github: + private-to-public-flows: allow --- ``` @@ -1665,25 +1664,28 @@ private-to-public-flows: allow ```yaml --- tools: - - github - - safe-outputs - - playwright -private-to-public-flows: - - playwright + github: + private-to-public-flows: + - github + - my-custom-server +mcp-servers: + my-custom-server: + type: http + url: "http://localhost:9000/mcp" --- ``` #### 10.9.2 Constraints -Blanket `allow` is incompatible with `guards_mode: strict`, so the compiler MUST reject that combination at compile time. The list form remains compatible with `strict` because it relaxes only the default sink-visibility for named servers while leaving other protections in place. In list form, every server ID MUST map to an actual MCP server declared in the workflow's `tools` list; unknown IDs MUST be rejected at compile time. With non-strict mode (`filter` or `propagate`), the blanket form disables both the forced `repos="public"` override (Section 4.1.3.8) and the `sink-visibility` runtime-verification override (Section 10.8.4). +Blanket `allow` is incompatible with `guards_mode: strict`, so the compiler MUST reject that combination at compile time. The list form remains compatible with `strict` because it relaxes only the default sink-visibility for named servers while leaving other protections in place. In list form, every server ID MUST map to an actual MCP server declared in the workflow's `tools` or `mcp-servers` list; unknown IDs MUST be rejected at compile time. With non-strict mode (`filter` or `propagate`), the blanket form disables both the forced `repos="public"` override (Section 4.1.3.8) and the `sink-visibility` runtime-verification override (Section 10.8.4). #### 10.9.3 Compiler Responsibilities When the compiler encounters `private-to-public-flows`, it MUST validate the chosen form, emit the matching gateway configuration, and record the opt-out in the audit trail. -For blanket `allow`, the compiler MUST reject `guards_mode: strict`, set `gateway.forcePublicRepos: false` in the generated JSON stdin config, and skip setting `sink-visibility: "public"` even if the target repo is public at compile time. +For blanket `allow`, the compiler MUST reject `guards_mode: strict`, set `gateway.forcePublicRepos: false` in the generated JSON stdin config, and skip setting `sink-visibility` in write-sink guard policies even if the target repo is public at compile time. -For list form, the compiler MUST verify that every listed server ID exists in the workflow's declared `tools` list, set `gateway.sinkVisibilityExemptServers` to that list in the generated JSON stdin config, and leave `forcePublicRepos` unchanged. +For list form, the compiler MUST verify that every listed server ID exists in the workflow's declared `tools` or `mcp-servers` list, set `gateway.sinkVisibilityExemptServers` to that list in the generated JSON stdin config, and leave `forcePublicRepos` unchanged. #### 10.9.4 Interaction Matrix diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go index 9339ef58906..32c75b3e2b4 100644 --- a/pkg/workflow/compiler_validators.go +++ b/pkg/workflow/compiler_validators.go @@ -182,6 +182,7 @@ func (c *Compiler) validateCoreToolConfiguration(workflowData *WorkflowData, mar {logMessage: "Validating labels", validateFn: func() error { return validateLabels(workflowData) }}, {logMessage: "Validating workflow_dispatch input requirements for command triggers", validateFn: func() error { return validateCommandWorkflowDispatchInputs(workflowData) }}, {logMessage: "Validating max-daily-ai-credits frontmatter", validateFn: func() error { return validateMaxDailyAICFrontmatter(workflowData) }}, + {logMessage: "Validating private-to-public-flows server IDs", validateFn: func() error { return validatePrivateToPublicFlowsServerIDs(workflowData) }}, } // This validation is intentionally outside the table below because strict mode // turns the same validation result into either an error or a warning. diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index a4a49bdc2a7..f84c899bae3 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -541,6 +541,10 @@ func transformRepoPattern(pattern string) string { // a GitHub App configured, auto-lockdown detection will set repos=all at runtime, so a // write-sink policy with accept=["*"] is returned to match that runtime behaviour. // +// When private-to-public-flows: allow is declared, sink-visibility is omitted from the returned +// policy per MCP Gateway Specification Section 10.9.3: the blanket allow disables both +// forcePublicRepos and sink-visibility enforcement. +// // Returns nil when workflowData is nil, when no GitHub tool is present, or when a GitHub App is // configured (auto-lockdown is skipped for GitHub App tokens, which are already repo-scoped). func deriveWriteSinkGuardPolicyFromWorkflow(workflowData *WorkflowData) map[string]any { @@ -554,9 +558,21 @@ func deriveWriteSinkGuardPolicyFromWorkflow(workflowData *WorkflowData) map[stri toolConfig, _ := rawGithubTool.(map[string]any) + // Detect blanket opt-out: private-to-public-flows: allow. + // Per Section 10.9.3, allow disables sink-visibility enforcement in addition to forcePublicRepos. + blanketAllow := workflowData.ParsedTools != nil && + workflowData.ParsedTools.GitHub != nil && + workflowData.ParsedTools.GitHub.PrivateToPublicFlows == "allow" + // Try to derive from explicit guard policy first policy := deriveSafeOutputsGuardPolicyFromGitHub(toolConfig) if policy != nil { + if blanketAllow { + // Strip sink-visibility from write-sink when blanket allow is declared. + if writeSink, ok := policy["write-sink"].(map[string]any); ok { + delete(writeSink, "sink-visibility") + } + } return policy } @@ -566,11 +582,14 @@ func deriveWriteSinkGuardPolicyFromWorkflow(workflowData *WorkflowData) map[stri // sink-visibility is set as a runtime expression so that the write-sink guard can enforce // public/private/internal semantics based on the actual repository visibility at workflow execution time. if rawGithubTool != false && len(getGitHubGuardPolicies(toolConfig)) == 0 && !hasGitHubApp(toolConfig) { + writeSink := map[string]any{ + "accept": []string{"*"}, + } + if !blanketAllow { + writeSink["sink-visibility"] = sinkVisibilityRuntimeExpr + } return map[string]any{ - "write-sink": map[string]any{ - "accept": []string{"*"}, - "sink-visibility": sinkVisibilityRuntimeExpr, - }, + "write-sink": writeSink, } } diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index 795ed811c8d..d7a0d77d6f9 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -47,6 +47,7 @@ package workflow import ( "fmt" "os" + "regexp" "strings" "time" @@ -55,6 +56,17 @@ import ( var mcpRendererLog = logger.New("workflow:mcp_renderer") +// safeMCPServerIDRE matches MCP server IDs that are safe to embed inside an unquoted bash heredoc. +// The pattern allows alphanumeric characters, hyphens, and underscores — the same characters used +// for built-in tool names and typical custom mcp-server keys. +var safeMCPServerIDRE = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + +// isSafeMCPServerID reports whether id can be safely embedded inside an unquoted bash heredoc +// without triggering shell expansion. +func isSafeMCPServerID(id string) bool { + return id != "" && safeMCPServerIDRE.MatchString(id) +} + func durationStringToSeconds(durationValue string) (int, error) { parsedDuration, err := time.ParseDuration(durationValue) if err != nil { @@ -221,6 +233,12 @@ func RenderJSONMCPConfig( if i > 0 { configBuilder.WriteString(", ") } + // Validate against safe-identifier pattern before writing into the heredoc. + // The config is rendered inside an unquoted bash heredoc; unvalidated IDs + // containing shell metacharacters (e.g. $(cmd), `cmd`) would be expanded. + if !isSafeMCPServerID(serverID) { + return fmt.Errorf("private-to-public-flows: server ID %q contains characters that are unsafe for shell heredoc emission; IDs must match [A-Za-z0-9_-]+", serverID) + } fmt.Fprintf(&configBuilder, "%q", serverID) } configBuilder.WriteString("]") diff --git a/pkg/workflow/private_to_public_flows_test.go b/pkg/workflow/private_to_public_flows_test.go index edaa075689d..0c2e3c316c2 100644 --- a/pkg/workflow/private_to_public_flows_test.go +++ b/pkg/workflow/private_to_public_flows_test.go @@ -27,8 +27,8 @@ func TestParseGitHubToolPrivateToPublicFlows(t *testing.T) { }, { name: "array of server IDs ([]any)", - input: map[string]any{"private-to-public-flows": []any{"github-mcp-server", "custom-server"}}, - wantVal: []string{"github-mcp-server", "custom-server"}, + input: map[string]any{"private-to-public-flows": []any{"github", "custom-server"}}, + wantVal: []string{"github", "custom-server"}, }, { name: "array of server IDs ([]string)", @@ -79,8 +79,8 @@ func TestBuildMCPGatewayConfigPrivateToPublicFlows(t *testing.T) { }, { name: "list: sets SinkVisibilityExemptServers", - privateToPublicFlows: []string{"github-mcp-server", "custom-srv"}, - wantSinkVisibilityExemptSvr: []string{"github-mcp-server", "custom-srv"}, + privateToPublicFlows: []string{"github", "custom-srv"}, + wantSinkVisibilityExemptSvr: []string{"github", "custom-srv"}, }, { name: "nil: no override fields set", @@ -150,8 +150,12 @@ engine: copilot tools: github: private-to-public-flows: - - github-mcp-server + - github - custom-server +mcp-servers: + custom-server: + type: http + url: "http://localhost:9000/mcp" --- # Test workflow @@ -173,7 +177,7 @@ tools: { name: "list form emits sinkVisibilityExemptServers", content: makeWorkflowListForm(), - wantContains: []string{`"sinkVisibilityExemptServers": ["github-mcp-server", "custom-server"]`}, + wantContains: []string{`"sinkVisibilityExemptServers": ["github", "custom-server"]`}, wantNotContains: []string{`"forcePublicRepos"`}, }, } @@ -247,7 +251,7 @@ network: tools: github: private-to-public-flows: - - my-server + - github --- # Test workflow @@ -299,3 +303,101 @@ func extractGatewaySection(yaml string) string { end := min(start+500, len(yaml)) return yaml[start:end] } + +// TestPrivateToPublicFlowsAllowSuppressesSinkVisibility verifies that +// private-to-public-flows: allow suppresses sink-visibility in the write-sink guard policy. +func TestPrivateToPublicFlowsAllowSuppressesSinkVisibility(t *testing.T) { + t.Run("allow suppresses sink-visibility in auto-lockdown path", func(t *testing.T) { + wd := &WorkflowData{ + Tools: map[string]any{ + "github": map[string]any{}, + }, + ParsedTools: &Tools{ + GitHub: &GitHubToolConfig{ + ReadOnly: false, + PrivateToPublicFlows: "allow", + }, + }, + } + policy := deriveWriteSinkGuardPolicyFromWorkflow(wd) + require.NotNil(t, policy, "policy should be derived when github tool is present") + writeSink, ok := policy["write-sink"].(map[string]any) + require.True(t, ok, "policy should have write-sink") + assert.NotContains(t, writeSink, "sink-visibility", + "sink-visibility should be absent when private-to-public-flows: allow is set") + assert.Contains(t, writeSink, "accept", + "accept should still be present") + }) + + t.Run("no allow preserves sink-visibility in auto-lockdown path", func(t *testing.T) { + wd := &WorkflowData{ + Tools: map[string]any{ + "github": map[string]any{}, + }, + ParsedTools: &Tools{ + GitHub: &GitHubToolConfig{ + ReadOnly: false, + }, + }, + } + policy := deriveWriteSinkGuardPolicyFromWorkflow(wd) + require.NotNil(t, policy) + writeSink, ok := policy["write-sink"].(map[string]any) + require.True(t, ok) + assert.Contains(t, writeSink, "sink-visibility", + "sink-visibility should be present when private-to-public-flows is not set") + }) +} + +// TestValidatePrivateToPublicFlowsServerIDs tests that unknown server IDs are rejected. +func TestValidatePrivateToPublicFlowsServerIDs(t *testing.T) { + t.Run("valid declared servers accepted", func(t *testing.T) { + wd := &WorkflowData{ + Tools: map[string]any{"github": map[string]any{}, "my-server": map[string]any{}}, + ParsedTools: &Tools{ + GitHub: &GitHubToolConfig{PrivateToPublicFlows: []string{"github", "my-server"}}, + }, + } + assert.NoError(t, validatePrivateToPublicFlowsServerIDs(wd)) + }) + + t.Run("unknown server ID rejected", func(t *testing.T) { + wd := &WorkflowData{ + Tools: map[string]any{"github": map[string]any{}}, + ParsedTools: &Tools{ + GitHub: &GitHubToolConfig{PrivateToPublicFlows: []string{"github", "undeclared-server"}}, + }, + } + err := validatePrivateToPublicFlowsServerIDs(wd) + require.Error(t, err, "undeclared server ID should be rejected") + assert.Contains(t, err.Error(), "undeclared-server") + }) + + t.Run("allow string form skipped", func(t *testing.T) { + wd := &WorkflowData{ + Tools: map[string]any{"github": map[string]any{}}, + ParsedTools: &Tools{ + GitHub: &GitHubToolConfig{PrivateToPublicFlows: "allow"}, + }, + } + assert.NoError(t, validatePrivateToPublicFlowsServerIDs(wd), + "string allow form should not trigger server ID validation") + }) + + t.Run("nil parsed tools returns nil", func(t *testing.T) { + assert.NoError(t, validatePrivateToPublicFlowsServerIDs(&WorkflowData{})) + }) +} + +// TestIsSafeMCPServerID tests the shell-safety identifier check. +func TestIsSafeMCPServerID(t *testing.T) { + safe := []string{"github", "my-server", "my_server", "server123", "UPPER-CASE"} + for _, id := range safe { + assert.True(t, isSafeMCPServerID(id), "expected %q to be safe", id) + } + + unsafe := []string{"", "srv with space", "srv$(cmd)", "srv`cmd`", "srv;rm", "srv|pipe", "srv&bg", "srv>out"} + for _, id := range unsafe { + assert.False(t, isSafeMCPServerID(id), "expected %q to be unsafe", id) + } +} diff --git a/pkg/workflow/strict_mode_network_validation.go b/pkg/workflow/strict_mode_network_validation.go index 14301feea0b..94e743e2bc1 100644 --- a/pkg/workflow/strict_mode_network_validation.go +++ b/pkg/workflow/strict_mode_network_validation.go @@ -172,3 +172,37 @@ func (c *Compiler) validateStrictTools(frontmatter map[string]any) error { return nil } + +// validatePrivateToPublicFlowsServerIDs validates that every server ID in the list form of +// private-to-public-flows matches a declared MCP server in the workflow's tools list. +// Per MCP Gateway Specification Section 10.9.2, unknown IDs must be rejected at compile time. +func validatePrivateToPublicFlowsServerIDs(workflowData *WorkflowData) error { + if workflowData == nil || workflowData.ParsedTools == nil || workflowData.ParsedTools.GitHub == nil { + return nil + } + servers, ok := workflowData.ParsedTools.GitHub.PrivateToPublicFlows.([]string) + if !ok || len(servers) == 0 { + return nil + } + // Collect valid server IDs from the merged tools map. + validIDs := make(map[string]bool, len(workflowData.Tools)) + for id := range workflowData.Tools { + validIDs[id] = true + } + var unknown []string + for _, id := range servers { + if !validIDs[id] { + unknown = append(unknown, id) + } + } + if len(unknown) == 0 { + return nil + } + return NewValidationError( + "tools.github.private-to-public-flows", + fmt.Sprintf("%v", unknown), + fmt.Sprintf("unknown MCP server ID(s) %v; every ID in private-to-public-flows must match a server declared in tools or mcp-servers", unknown), + "Check that each server ID in private-to-public-flows matches an entry in tools or mcp-servers. "+ + "The built-in GitHub MCP server ID is \"github\". Custom servers use the key from mcp-servers.", + ) +} From 560c207dd1158b45c7809019d65a8c7bf0ab1444 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Jul 2026 19:30:49 +0000 Subject: [PATCH 5/5] fix: use absolute path for mcp-gateway link in github-tools.md Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- docs/src/content/docs/reference/github-tools.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/content/docs/reference/github-tools.md b/docs/src/content/docs/reference/github-tools.md index f46a786ec22..1dc83bb8cd0 100644 --- a/docs/src/content/docs/reference/github-tools.md +++ b/docs/src/content/docs/reference/github-tools.md @@ -220,7 +220,7 @@ The compiler emits `"sinkVisibilityExemptServers": ["github", "my-custom-server" Opting out of cross-visibility protections means the agent may read from private repositories and write that data to public sinks (e.g., a public GitHub issue, a Slack channel). Only use this when your workflow explicitly requires it and you understand the data-flow implications. -See [MCP Gateway Specification Section 10.9](./mcp-gateway#109-cross-visibility-opt-out-private-to-public-flows) for full protocol details. +See [MCP Gateway Specification Section 10.9](/gh-aw/reference/mcp-gateway/#109-cross-visibility-opt-out-private-to-public-flows) for full protocol details. ## Related Documentation