From cccdfeab0a025f7f63d19d1140a87eea9e7bf26b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:10:46 +0000 Subject: [PATCH 1/9] Initial plan From b458fe254a6492f4c3f729d99114d7a77c041249 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:27:52 +0000 Subject: [PATCH 2/9] Fix dangling determine-automatic-lockdown reference for static enclave sink-visibility Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/compiler_github_mcp_steps.go | 7 ++ pkg/workflow/enclave_github_proxy_test.go | 116 +++++++++++++++++++++- pkg/workflow/mcp_environment.go | 21 ++-- pkg/workflow/mcp_github_config.go | 10 +- 4 files changed, 142 insertions(+), 12 deletions(-) diff --git a/pkg/workflow/compiler_github_mcp_steps.go b/pkg/workflow/compiler_github_mcp_steps.go index 9d22add76b9..e8b0a8b3285 100644 --- a/pkg/workflow/compiler_github_mcp_steps.go +++ b/pkg/workflow/compiler_github_mcp_steps.go @@ -23,6 +23,13 @@ func githubLockdownDetectionStepEnabled(data *WorkflowData) bool { if enclaveDynamicRepositoryPolicyEnabled(data) { return true } + // A static enclave-only GitHub backend still needs the target repository's visibility + // for its safe-outputs write-sink policy, even when the primary agent has no GitHub + // MCP access at all (tools.github: false). Generate the step in that case too, so the + // write-sink policy's sink-visibility field always has a valid producer step. + if staticEnclaveWriteSinkGuardPolicy(data) != nil { + return true + } if githubTool, hasGitHub := data.Tools["github"]; hasGitHub { return githubTool != false } diff --git a/pkg/workflow/enclave_github_proxy_test.go b/pkg/workflow/enclave_github_proxy_test.go index ec738befe83..352a9c3d591 100644 --- a/pkg/workflow/enclave_github_proxy_test.go +++ b/pkg/workflow/enclave_github_proxy_test.go @@ -5,6 +5,7 @@ package workflow import ( "os" "path/filepath" + "regexp" "strings" "testing" @@ -358,12 +359,18 @@ Read the private repository's issues through the enclave. var doc any require.NoError(t, yaml.Unmarshal(lockBytes, &doc), "generated lock file must be valid YAML") - // The determine-automatic-lockdown step is not generated, so its outputs must not be - // referenced by the server-level guard policy or the gateway step environment. - assert.NotContains(t, lock, "Determine automatic lockdown mode") + // The determine-automatic-lockdown step IS generated in this configuration, solely to + // supply the target repository's visibility for the static enclave's write-sink policy + // (GH_AW_SINK_VISIBILITY). Its min_integrity/repos outputs must still not be referenced, + // because the server-level guard policy for this enclave-only backend is derived + // statically from the enclave declaration, not from the step outputs. + assert.Contains(t, lock, "Determine automatic lockdown mode") + assert.Contains(t, lock, "id: determine-automatic-lockdown") + assert.Contains(t, lock, "GH_AW_SINK_VISIBILITY: ${{ steps.determine-automatic-lockdown.outputs.visibility }}") assert.NotContains(t, lock, "$GITHUB_MCP_GUARD_MIN_INTEGRITY") assert.NotContains(t, lock, "$GITHUB_MCP_GUARD_REPOS") assert.NotContains(t, lock, "GITHUB_MCP_GUARD_MIN_INTEGRITY: ${{ steps.determine-automatic-lockdown.outputs.min_integrity }}") + assert.NotContains(t, lock, "GITHUB_MCP_GUARD_REPOS: ${{ steps.determine-automatic-lockdown.outputs.repos }}") // The server-level guard policy mirrors the enclave identity policy, so it never // broadens access beyond what the enclave identity already allows. @@ -372,6 +379,9 @@ Read the private repository's issues through the enclave. assert.Contains(t, lock, `"write-sink"`) assert.Contains(t, lock, `"private:octo-org/private-service"`) assert.Contains(t, lock, `"sink-visibility": "${GH_AW_SINK_VISIBILITY}"`) + // The sink-visibility env var must reference the step actually generated above — no + // dangling steps..outputs.* reference. + assert.Contains(t, lock, "steps.determine-automatic-lockdown.outputs.visibility") assert.Contains(t, lock, `"${AWF_ENCLAVE_GITHUB_MCP_AGENT_ID}":{"servers":["github"],"tools":{"github":["list_issues","issue_read"]},"allow-only":{"min-integrity":"none","repos":["octo-org/private-service"]}}`) assert.Contains(t, lock, `export GH_AW_MCP_GITHUB_CHECK_AGENT_ID="${AWF_ENCLAVE_GITHUB_MCP_AGENT_ID}"`) assert.NotContains(t, lock, `"${MCP_GATEWAY_AGENT_ID}":{"servers":["awf-enclave","github"`) @@ -382,6 +392,100 @@ Read the private repository's issues through the enclave. assert.Contains(t, lock, `"forcePublicRepos": false`) } +// stepOutputRefPattern matches steps..outputs. references, capturing the step id. +var stepOutputRefPattern = regexp.MustCompile(`steps\.([A-Za-z0-9_-]+)\.outputs\.[A-Za-z0-9_]+`) + +// stepIDPattern matches `id: ` lines in a GitHub Actions step definition, capturing +// the step id. +var stepIDPattern = regexp.MustCompile(`(?m)^\s*id:\s*([A-Za-z0-9_-]+)\s*$`) + +// assertNoDanglingStepOutputReferences verifies that every `steps..outputs.*` reference +// in the generated lock file corresponds to a step id that is actually emitted somewhere in +// the file. This guards against the class of bug described in gh-aw#60336, where a consumer +// (e.g. an environment variable or guard policy) references a step's outputs even though the +// producer step itself was never generated, expanding to an empty string at runtime. +func assertNoDanglingStepOutputReferences(t *testing.T, lock string) { + t.Helper() + emittedIDs := make(map[string]bool) + for _, match := range stepIDPattern.FindAllStringSubmatch(lock, -1) { + emittedIDs[match[1]] = true + } + for _, match := range stepOutputRefPattern.FindAllStringSubmatch(lock, -1) { + stepID := match[1] + assert.True(t, emittedIDs[stepID], "reference %q has no corresponding emitted step id %q", match[0], stepID) + } +} + +// TestCompileStaticEnclaveOnlyGitHubDisabledSinkVisibility is a regression test for +// gh-aw#60336: a static GitHub enclave combined with `tools.github: false` and safe-outputs +// must not emit GH_AW_SINK_VISIBILITY (or any other value) referencing the +// determine-automatic-lockdown step unless that step is actually generated. Before the fix, +// the compiler emitted `GH_AW_SINK_VISIBILITY: ${{ steps.determine-automatic-lockdown.outputs.visibility }}` +// and `"sink-visibility": "${GH_AW_SINK_VISIBILITY}"` even though no `determine-automatic-lockdown` +// step existed, so the env var resolved to an empty string and MCP Gateway rejected the +// resulting `sink-visibility: ""` as invalid. +func TestCompileStaticEnclaveOnlyGitHubDisabledSinkVisibility(t *testing.T) { + tmp := t.TempDir() + workflowPath := filepath.Join(tmp, "roadmap-triage-enclave.md") + content := `--- +on: workflow_dispatch +strict: false +network: defaults +engine: copilot +tools: + github: false +enclaves: + - agent: + model: gpt-5 + tools: + github: + allowed: [list_issues, issue_read] + allowed-repos: [githubnext/gh-aw-enclave-demo-private] + min-integrity: none + repos: + - repo: githubnext/gh-aw-enclave-demo-private + sensitivity: confidential +safe-outputs: + add-comment: + max: 1 +--- + +Read the private repository's issues through the enclave and post a triage comment. +` + require.NoError(t, os.WriteFile(workflowPath, []byte(content), 0o600)) + compiler := NewCompiler() + compiler.SetSkipValidation(true) + require.NoError(t, compiler.CompileWorkflow(workflowPath)) + lockBytes, err := os.ReadFile(strings.TrimSuffix(workflowPath, ".md") + ".lock.yml") + require.NoError(t, err) + lock := string(lockBytes) + + var doc any + require.NoError(t, yaml.Unmarshal(lockBytes, &doc), "generated lock file must be valid YAML") + + // General invariant: every steps..outputs.* reference must correspond to an + // emitted step id somewhere in the generated workflow. + assertNoDanglingStepOutputReferences(t, lock) + + // The determine-automatic-lockdown step and its producer for GH_AW_SINK_VISIBILITY must + // either both be present, or both be absent — never a dangling reference to one without + // the other. Here, the step IS generated (solely to supply visibility for the static + // enclave's write-sink policy). + assert.Contains(t, lock, "id: determine-automatic-lockdown") + assert.Contains(t, lock, "GH_AW_SINK_VISIBILITY: ${{ steps.determine-automatic-lockdown.outputs.visibility }}") + assert.Contains(t, lock, `"sink-visibility": "${GH_AW_SINK_VISIBILITY}"`) + + // The primary agent must have no GitHub access: its own guard policy must not be + // automatically derived from the lockdown step's min_integrity/repos outputs. + assert.NotContains(t, lock, "GITHUB_MCP_GUARD_MIN_INTEGRITY: ${{ steps.determine-automatic-lockdown.outputs.min_integrity }}") + assert.NotContains(t, lock, "GITHUB_MCP_GUARD_REPOS: ${{ steps.determine-automatic-lockdown.outputs.repos }}") + + // The enclave identity retains its scoped repository access, and the static write-sink + // retains its narrow accepted secrecy labels. + assert.Contains(t, lock, `"private:githubnext/gh-aw-enclave-demo-private"`) + assert.Contains(t, lock, `"allow-only":{"min-integrity":"none","repos":["githubnext/gh-aw-enclave-demo-private"]}`) +} + // TestBuildMCPGatewayConfigForcePublicReposForStaticEnclave verifies that the gateway's // runtime public-repos override is disabled when the GitHub MCP server exists solely to // serve a static enclave agent identity, and left at its default when the primary agent @@ -424,7 +528,11 @@ func TestGitHubGuardPoliciesFromStepSkipsEnclaveOnlyBackend(t *testing.T) { delete(data.Tools, "github") data.ExplicitlyDisabledTools = map[string]struct{}{"github": {}} - assert.False(t, githubLockdownDetectionStepEnabled(data)) + // The determine-automatic-lockdown step is still generated for an enclave-only static + // backend, solely to supply GH_AW_SINK_VISIBILITY for the write-sink policy — but its + // min_integrity/repos outputs must never drive the primary GitHub MCP server's guard + // policy, which stays derived statically from the enclave declaration. + assert.True(t, githubLockdownDetectionStepEnabled(data)) assert.False(t, githubGuardPoliciesFromStep(data, nil)) assert.True(t, githubBackendIsStaticEnclaveDelegationOnly(data)) diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index e2abf65f4f6..7cf0eed4147 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -94,24 +94,33 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor envVars["GITHUB_MCP_SERVER_TOKEN"] = effectiveToken } - // Add guard policy env vars if the determine-automatic-lockdown step will be generated. - // Skip only when guard policy is already explicitly set — in that case, the - // determine-automatic-lockdown step is not generated. + // Add guard policy env vars if the determine-automatic-lockdown step will be generated + // and its outputs are actually used to render the primary GitHub MCP server's guard + // policy. Skip when guard policy is already explicitly set, or when the GitHub MCP + // server exists solely to serve an enclave identity — enclave-only backends always + // derive their guard policy from the enclave declaration, never from the step outputs + // (see staticEnclaveGitHubGuardPolicies / dynamicEnclaveGitHubGuardPolicies), even + // though the step may still be generated to supply GH_AW_SINK_VISIBILITY. // Security: Pass step outputs through environment variables to prevent template injection. guardPoliciesExplicit := len(getGitHubGuardPolicies(toolConfig)) > 0 - if githubToolEnabledInTools && !guardPoliciesExplicit && githubLockdownDetectionStepEnabled(workflowData) { + enclaveOnlyGitHubBackend := githubBackendIsStaticEnclaveDelegationOnly(workflowData) || githubBackendIsDynamicDelegationOnly(workflowData) + if githubToolEnabledInTools && !guardPoliciesExplicit && !enclaveOnlyGitHubBackend && githubLockdownDetectionStepEnabled(workflowData) { envVars["GITHUB_MCP_GUARD_MIN_INTEGRITY"] = "${{ steps.determine-automatic-lockdown.outputs.min_integrity }}" envVars["GITHUB_MCP_GUARD_REPOS"] = "${{ steps.determine-automatic-lockdown.outputs.repos }}" } } // Emit GH_AW_SINK_VISIBILITY for all workflows where the determine-automatic-lockdown step - // runs (i.e., any workflow with a GitHub tool or a dynamic enclave). This avoids + // runs (i.e., any workflow with a GitHub tool, a dynamic enclave, or a static enclave-only + // GitHub backend whose write-sink policy needs the destination visibility). This avoids // embedding a ${{ }} expression directly in the run: heredoc, which zizmor flags as // template injection. The value is the raw step output (no toJSON), and the surrounding // JSON double-quotes in the heredoc produce a valid JSON string at runtime: // "sink-visibility": "${GH_AW_SINK_VISIBILITY}" → "sink-visibility": "public" - if githubToolEnabledInTools || enclaveDynamicRepositoryPolicyEnabled(workflowData) { + // Gating on githubLockdownDetectionStepEnabled ensures this never references a step that + // isn't actually generated (a dangling steps..outputs.* reference). + sinkVisibilityRelevant := githubToolEnabledInTools || enclaveDynamicRepositoryPolicyEnabled(workflowData) || githubBackendIsStaticEnclaveDelegationOnly(workflowData) + if sinkVisibilityRelevant && githubLockdownDetectionStepEnabled(workflowData) { envVars[sinkVisibilityEnvVar] = "${{ steps.determine-automatic-lockdown.outputs.visibility }}" } if enclaveDynamicRepositoryPolicyEnabled(workflowData) { diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index c52273dd3ac..8e5b82cbe73 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -152,8 +152,14 @@ func githubGuardPoliciesFromStep(workflowData *WorkflowData, explicitGuardPolici if !hasGitHub { // Default-tool resolution removes the "github" key when tools.github is false, so an // absent key can still mean the GitHub MCP server is rendered for enclave delegation. - // Only reference the lockdown step outputs when that step is actually generated. - return githubLockdownDetectionStepEnabled(workflowData) + // The determine-automatic-lockdown step may still run in that case (to supply + // GH_AW_SINK_VISIBILITY for the write-sink policy), but its min_integrity/repos + // outputs must never drive an enclave-only backend's guard policy — that policy is + // always derived statically from the enclave declaration instead. Checking the + // explicit-disable marker directly (rather than githubLockdownDetectionStepEnabled) + // keeps this decision independent of whether the step happens to be generated. + _, explicitlyDisabled := workflowData.ExplicitlyDisabledTools["github"] + return !explicitlyDisabled } return githubTool != false } From 6658113b84d186fda17ce15795df03fd563f622e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:38:57 +0000 Subject: [PATCH 3/9] Address code review feedback: scope regex and clarify guard-policy exclusions Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../gh-aw_pydantic.md | 575 ++++++++++++++++++ pkg/workflow/enclave_github_proxy_test.go | 9 +- pkg/workflow/mcp_github_config.go | 25 +- 3 files changed, 596 insertions(+), 13 deletions(-) create mode 100644 pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md diff --git a/pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md b/pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md new file mode 100644 index 00000000000..2438433bbff --- /dev/null +++ b/pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md @@ -0,0 +1,575 @@ +--- +runtimes: + python: + version: "3.12" +pre-agent-steps: + - name: Preinstall Pydantic AI coder agent + run: | + # This step runs on the host runner with the checkout as its working + # directory, before the AWF sandbox exists. -P keeps that directory off + # sys.path, so a repo-local pip.py or pydantic_ai_harness/ cannot be + # imported in place of the installed packages. + # + # 2.36.0 is the first pydantic-ai-slim release carrying `pai --mcp-config`, + # which is how the gateway's MCP servers reach the agent. + # + # The anthropic extra is what an `anthropic/` model runs on: that backend of + # the api-proxy serves the Messages API, not Chat Completions. + python3 -P -m pip install --quiet --user --disable-pip-version-check "pydantic-ai-harness[cli]==$GH_AW_ENGINE_VERSION" "pydantic-ai-slim[anthropic,openai,mcp]>=2.36.0" + "$HOME/.local/bin/pai" --version + python3 -P -c "from pydantic_ai_harness import Coder" +engine: + id: pydantic-ai + version: "0.26.0" + display-name: Pydantic AI + description: Pydantic AI CLI (pai) running the pydantic-ai-harness coder agent with MCP tool support + experimental: true + mcp: true + provider: + name: github + behaviors: + secret-strategy: universal-llm-consumer + # Repository paths gh-aw treats as this engine's configuration: it protects them + # from pull-request modification and derives the inline sub-agent and skill + # directories from the first prefix (.pydantic-ai/agents, .pydantic-ai/skills). + # The engine itself writes nothing into the checkout. + manifest: + files: + - AGENTS.md + path-prefixes: + - .pydantic-ai/ + network: + defaults: + - host.docker.internal + - github.com + - raw.githubusercontent.com + - api.github.com + - objects.githubusercontent.com + - pypi.org + - files.pythonhosted.org + provider-domains: + copilot: api.githubcopilot.com + anthropic: api.anthropic.com + openai: api.openai.com + codex: api.openai.com + execution: + command-name: pai + step-name: Execute Pydantic AI CLI + model-env-var: PAI_MODEL + write-timestamp: true + provider-env-mode: universal-llm-consumer + harness-script: | + const { spawnSync } = require("child_process"); + const { chmodSync, existsSync, mkdtempSync, readFileSync, writeFileSync } = require("fs"); + const { homedir, tmpdir } = require("os"); + const { join } = require("path"); + const { fetchAWFReflect, resolveProviderEndpointFromReflect, deriveBaseUrlFromModelsURL } = require("./awf_reflect.cjs"); + + // gh-aw passes `execution.command-name` (or a workflow's `engine.command`) + // first, then `execution.args`. The name is not spawned -- the CLI is started + // by the interpreter that owns the install, see LAUNCHER below -- so only the + // arguments after it are forwarded. + const commandArgs = process.argv.slice(3); + const log = message => process.stderr.write(`[pydantic-ai] ${message}\n`); + + // `pai -a` takes one target, either an import path or a JSON/YAML agent + // spec, and the spec format resolves capability names through a closed + // registry that the harness capabilities are not part of, so the coder + // composition cannot be expressed as a spec. It is written as a Python + // module instead: `Coder()` supplies the filesystem, shell, planning and + // sub-agent tools. + // + // The gateway's MCP servers are deliberately not part of the module. + // `pai --mcp-config` reads the same Claude-shaped config file through the + // same `pydantic_ai.mcp.load_mcp_toolsets`, `${VAR}` expansion included, so + // routing them through the CLI is what lets a `PAI_AGENT` agent receive + // them on identical terms. + const AGENT_MODULE = `from pydantic_ai import Agent + from pydantic_ai_harness import Coder + + agent = Agent(name="coder", capabilities=[Coder()]) + `; + const DEFAULT_AGENT = "gh_aw_agent:agent"; + + // The CLI runs inside the interpreter that owns the install rather than as a + // separate `pai` process, so the agent module is imported once, in the process + // that runs it. Three things follow from that. + // + // `pai` reduces any failed `-a` load to one line naming the target, so an + // agent that raises on import would reach the step log without its traceback. + // The import here happens before the CLI starts, and an unhandled exception is + // the step's failure, traceback included. + // + // `pydantic_ai._cli.load_agent` prepends the working directory -- the checkout + // -- to sys.path before resolving the target, ahead of PYTHONPATH. A + // repository file named `gh_aw_agent.py` would therefore be loaded in place of + // the generated module. Importing the target first settles which file the name + // means, because an import of a module already in sys.modules does not search + // the path again. + // + // A separate preflight process could do neither: it would import the module in + // one interpreter and leave the CLI to import it again in another, running any + // module-level work in the agent twice. + // + // The residual is `load_agent`'s insert itself: everything the agent imports + // after that point still sees the checkout first on sys.path. That is the + // CLI's documented behavior for its own users, and not something this file + // can change from the outside. + // + // `-P` keeps the `-c` invocation from putting the working directory on + // sys.path on its own account; PYTHONPATH below is what makes the agent + // importable. A spec file, and the dotted `module.attribute` form the CLI also + // accepts, are left to the CLI as before. + const LAUNCHER = `import runpy + import sys + + target, *cli_args = sys.argv[1:] + module, separator, attribute = target.rpartition(":") + if separator and not target.lower().endswith((".yml", ".yaml", ".json")): + import importlib + + from pydantic_ai import Agent + + loaded = getattr(importlib.import_module(module), attribute) + if not isinstance(loaded, Agent): + raise TypeError(f"{target} is {type(loaded).__name__}, not pydantic_ai.Agent") + + sys.argv = ["pai", *cli_args] + runpy.run_module("pydantic_ai", run_name="__main__", alter_sys=True) + `; + + const main = async () => { + const workspace = process.env.GITHUB_WORKSPACE; + if (!workspace) throw new Error("GITHUB_WORKSPACE is required"); + const promptFile = process.env.GH_AW_PROMPT; + if (!promptFile) throw new Error("GH_AW_PROMPT is required"); + + // Neither the generated module nor the gateway's MCP config is written into + // the checkout. A file committed at a path the engine reads is + // repository-controlled input to a process that runs with the gateway's + // credentials: an `mcp.json` there can name a stdio server for the CLI to + // spawn, and a package there shadows an installed one for the whole run. The + // module goes to a private directory created inside the sandbox; the config + // adapter writes on the host into the `${RUNNER_TEMP}/gh-aw` tree that the + // agent step mounts read-only, where gh-aw's own Claude and Codex converters + // write theirs. + // + // `PAI_AGENT` runs an agent the repository defines, in whichever form + // `pai -a` accepts. The generated module is not written in that case: + // nothing would load it, and a stale copy on disk is worse than none. + const configuredAgent = process.env.PAI_AGENT; + const agentTarget = configuredAgent || DEFAULT_AGENT; + const moduleDir = configuredAgent ? "" : mkdtempSync(join(tmpdir(), "gh-aw-pydantic-ai-")); + if (moduleDir) { + const agentModulePath = join(moduleDir, "gh_aw_agent.py"); + writeFileSync(agentModulePath, AGENT_MODULE, { mode: 0o600 }); + chmodSync(agentModulePath, 0o600); + } + + const env = { ...process.env }; + // `pip install --user` puts `pai` here. The runner tool cache that holds + // `uv` and the interpreter's own bin directory is under /opt, which the + // sandbox exposes read-only, but the home directory is where the CLI and + // its user site-packages actually live. + // + // Which interpreter owns those user site-packages matters: only the one + // that ran the pre-agent `pip install --user` can import them, and the + // sandbox prelude prepends every `bin` directory under the runner tool + // cache — which caches several Python versions — so a bare `python3` + // there resolves by `find` order rather than to the installing + // interpreter. `actions/setup-python` names that one in `pythonLocation`; + // putting its `bin` on PATH also gives the agent's own shell tool a + // `python3` that can see the installed packages. + const pythonBin = process.env.pythonLocation ? join(process.env.pythonLocation, "bin") : ""; + const python = pythonBin ? join(pythonBin, "python3") : "python3"; + env.PATH = [join(homedir(), ".local", "bin"), pythonBin, process.env.PATH || ""].filter(Boolean).join(":"); + // The module is reached through PYTHONPATH rather than by importing it as a + // package, and prepending keeps a caller-supplied PYTHONPATH usable. + // + // The checkout itself joins the path only under `PAI_AGENT`. That is the + // opt-in: it makes repository code importable, which is the whole point + // of running your own agent, and it is exactly what `-P` on the install + // step keeps off the path for the default composition. + env.PYTHONPATH = [moduleDir, configuredAgent ? workspace : "", process.env.PYTHONPATH || ""].filter(Boolean).join(":"); + delete env.COPILOT_GITHUB_TOKEN; + + const provider = process.env.GH_AW_LLM_PROVIDER; + const configuredBaseUrl = process.env.PAI_BASE_URL; + + // `pai` sends the model name verbatim, minus the provider marker that + // selects one of its clients, so the bare model ID reaches the api-proxy — + // which steers to the configured provider by the port it is reached on, not + // by a prefix in the model name: Copilot rejects `copilot/` with + // `model_not_supported`. + // Only the first segment is the provider. Stripping greedily would eat an + // org namespace out of ids like `meta-llama/Llama-3.1`, so this mirrors the + // `SplitN(model, "/", 2)` gh-aw itself uses to read the provider off. + if (!env.PAI_MODEL) throw new Error("PAI_MODEL is required"); + const modelProvider = env.PAI_MODEL.split("/", 1)[0].trim().toLowerCase(); + const requestedModel = env.PAI_MODEL.replace(/^[^/]*\//, ""); + // The api-proxy's Anthropic backend forwards the request path to + // api.anthropic.com unchanged and rewrites Messages-shaped bodies; it does + // not translate Chat Completions into Messages. So `anthropic/` is addressed + // with the Messages API: `anthropic:` on `-m`, and ANTHROPIC_BASE_URL for + // the endpoint. The Copilot and Codex backends are OpenAI-shaped and stay on + // Chat Completions, and `PAI_BASE_URL` names a Chat Completions endpoint by + // definition, so it keeps every provider there too. + const useMessagesAPI = !configuredBaseUrl && modelProvider === "anthropic"; + // The dotted-alias rewrite describes the api-proxy's Copilot backend, + // which publishes Copilot's Claude models under dotted IDs. Every other + // destination — the anthropic and openai backends, or an endpoint named + // by PAI_BASE_URL — gets the id the workflow wrote: a model actually + // called `claude-sonnet-4-5` there has to arrive as that. + const model = !configuredBaseUrl && modelProvider === "copilot" + ? requestedModel.replace(/^(claude-(?:haiku|sonnet|opus)-\d+)-(\d+)$/, "$1.$2") + : requestedModel; + + // `PAI_BASE_URL` points the engine at an OpenAI-compatible endpoint of the + // workflow's choosing instead of the AWF api-proxy. Two constraints shape + // it. + // + // It has to be a variable of this definition's own, because AWF sets the + // backend's own base URL variable on this step itself (OPENAI_BASE_URL, or + // ANTHROPIC_BASE_URL for the anthropic backend), pointing at the api-proxy + // on host.docker.internal whenever the firewall is enabled, so its presence + // cannot carry the workflow's intent, and reading it as intent is what + // made the pre-#52843 definition pick the wrong endpoint. + // + // There is deliberately no matching key knob. gh-aw excludes any + // `engine.env` value holding a secret from the agent sandbox + // (`awf --exclude-env`), so a credential cannot be delivered here at all + // and the API key below stays the placeholder. The endpoint therefore + // has to accept that placeholder, or be fronted by something upstream of + // the agent that adds the real credential. + let baseUrl = configuredBaseUrl || (useMessagesAPI ? process.env.ANTHROPIC_BASE_URL : process.env.OPENAI_BASE_URL); + if (!configuredBaseUrl) { + // Only /reflect discovery needs the provider: it selects which of the + // api-proxy's configured endpoints to use. A caller-supplied base URL + // names the endpoint outright, so demanding a provider alongside it + // would reject a complete configuration. + if (!provider) throw new Error("GH_AW_LLM_PROVIDER is required"); + if (process.env.AWF_REFLECT_ENABLED === "1") { + const result = await fetchAWFReflect({ logger: log }); + if (!result.ok || !result.reflectData) { + throw new Error(`Unable to discover the Pydantic AI LLM endpoint from /reflect: ${result.reason || "empty response"}`); + } + const endpoint = resolveProviderEndpointFromReflect({ + provider, + reflectData: result.reflectData, + logger: log, + }); + if (!endpoint?.baseUrl) { + throw new Error(`No configured /reflect endpoint found for provider ${provider}`); + } + baseUrl = endpoint.baseUrl; + const reflectedEndpoint = result.reflectData.endpoints?.find( + entry => entry?.configured === true && entry.provider === endpoint.endpointProvider + ); + if (!useMessagesAPI && typeof reflectedEndpoint?.models_url === "string") { + // `endpoint.baseUrl` is the models-listing origin, while the + // OpenAI-compatible client posts to `/chat/completions`, so the + // path prefix carried by models_url (`/v1` on some providers) has to + // come along — and this helper applies the same api-proxy -> + // host.docker.internal rewrite. + // + // The Anthropic client keeps the origin instead: it appends + // `/v1/messages` itself, so carrying the prefix over would post to + // `/v1/v1/messages`. + baseUrl = deriveBaseUrlFromModelsURL(reflectedEndpoint.models_url); + } + } + } + if (!baseUrl) { + throw new Error( + `Pydantic AI requires AWF endpoint discovery, PAI_BASE_URL or ${useMessagesAPI ? "ANTHROPIC_BASE_URL" : "OPENAI_BASE_URL"}` + ); + } + // The AWF api-proxy injects the real upstream credentials and ignores the + // inbound key, but neither client constructs itself without one. Setting it + // also replaces whatever key this step inherited, so the agent process holds + // the placeholder rather than a provider credential. + if (useMessagesAPI) { + env.ANTHROPIC_BASE_URL = baseUrl; + env.ANTHROPIC_API_KEY = "awf-anthropic-proxy"; + } else { + env.OPENAI_BASE_URL = baseUrl; + env.OPENAI_API_KEY = "awf-copilot-proxy"; + } + + // `-m` is always passed: the composed agent carries no model, and without + // the flag `pai` silently falls back to its own `openai:gpt-5` default, + // billing a model the workflow never asked for. gh-aw validates + // `provider/model` at compile time, so PAI_MODEL is set for every compiled + // workflow, and the throw above covers any other invocation. + // + // An explicit `-m` also replaces the model a loaded agent declares, so a + // `PAI_AGENT` agent runs on the workflow's `engine.model` whatever it was + // constructed with. That is what routes it through the endpoint above. + const cliArgs = [...commandArgs, "-a", agentTarget]; + // The config adapter writes this file only for a workflow that configures + // MCP tools, and `--mcp-config` fails on a path that is not there, so its + // absence has to mean "no servers" rather than an error. The + // `RUNNER_TEMP || "/tmp"` fallback is the one gh-aw's own converters use, and + // the adapter resolves this path by the same expression. + const mcpConfig = join(process.env.RUNNER_TEMP || "/tmp", "gh-aw", "mcp-config", "mcp-servers.json"); + if (existsSync(mcpConfig)) cliArgs.push("--mcp-config", mcpConfig); + cliArgs.push("-m", `${useMessagesAPI ? "anthropic" : "openai-chat"}:${model}`, readFileSync(promptFile, "utf8")); + log( + `provider=${configuredBaseUrl ? "(PAI_BASE_URL)" : provider} model=${model} baseUrl=${baseUrl}` + + (configuredAgent ? ` agent=${configuredAgent}` : "") + ); + // The target is passed twice on purpose: once for LAUNCHER, which imports it + // and hands the CLI a module already in sys.modules, and once as the `-a` + // the CLI parses for itself. + const result = spawnSync(python, ["-P", "-c", LAUNCHER, agentTarget, ...cliArgs], { cwd: workspace, env, stdio: "inherit" }); + if (result.error) throw result.error; + if (result.status !== 0) { + const error = new Error(`Pydantic AI execution failed with exit code ${result.status ?? "unknown"}`); + // Surface the child's own status so the step fails with the same code. + error.exitCode = typeof result.status === "number" && result.status !== 0 ? result.status : 1; + throw error; + } + }; + + main().catch(error => { + log(error instanceof Error ? error.message : String(error)); + process.exitCode = typeof error?.exitCode === "number" && error.exitCode !== 0 ? error.exitCode : 1; + }); + mcp: + config-path: ${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json + config-adapter: | + // Renders the MCP gateway's configuration as the Claude-style + // `mcpServers` document that `pydantic_ai.mcp.load_mcp_toolsets` reads, + // which the harness script hands to `pai --mcp-config`. Only HTTP entries + // are carried: `load_mcp_toolsets` can host stdio + // servers too, but the gateway already fronts every configured server + // over HTTP, and CLI-mounted servers are excluded because the agent + // reaches those as executables on PATH instead. + const fs = require("fs"); + const path = require("path"); + + const requireEnvVar = name => { + const value = process.env[name]; + if (!value) throw new Error(`${name} environment variable is required`); + return value; + }; + + const gatewayOutputPath = requireEnvVar("MCP_GATEWAY_OUTPUT"); + const gatewayDomain = process.env.MCP_GATEWAY_DOMAIN || "host.docker.internal"; + const gatewayPort = requireEnvVar("MCP_GATEWAY_PORT"); + const gatewayURL = `http://${gatewayDomain}:${gatewayPort}`; + + let cliServers; + try { + cliServers = new Set(JSON.parse(process.env.GH_AW_MCP_CLI_SERVERS || "[]")); + } catch (error) { + throw new Error(`Failed to parse GH_AW_MCP_CLI_SERVERS: ${error instanceof Error ? error.message : String(error)}`); + } + + const gatewayOutput = JSON.parse(fs.readFileSync(gatewayOutputPath, "utf8")); + const rawServers = gatewayOutput.mcpServers; + const servers = rawServers && typeof rawServers === "object" && !Array.isArray(rawServers) ? rawServers : {}; + + const mcpServers = {}; + for (const [name, entry] of Object.entries(servers)) { + if (cliServers.has(name) || !entry || typeof entry !== "object") continue; + if (typeof entry.url !== "string") { + console.log(`Skipping MCP server ${name}: the Pydantic AI engine only supports HTTP MCP servers`); + continue; + } + const server = { url: entry.url.replace(/^http:\/\/[^/]+\/mcp\//, `${gatewayURL}/mcp/`) }; + if (entry.headers && typeof entry.headers === "object") server.headers = entry.headers; + mcpServers[name] = server; + } + + // This script runs on the host runner, in the Start MCP Gateway step, so it + // writes where that step already created a directory and where the agent step + // mounts `${RUNNER_TEMP}/gh-aw` read-only -- the same file the built-in Claude + // converter produces, which is also the path gh-aw's log redaction scans for + // the gateway bearer token. The harness script resolves it by the same + // expression. Keeping it out of the checkout is what stops a committed + // `mcp.json` from reaching `pai --mcp-config`; see the harness script. + const configPath = path.join(process.env.RUNNER_TEMP || "/tmp", "gh-aw", "mcp-config", "mcp-servers.json"); + fs.mkdirSync(path.dirname(configPath), { recursive: true, mode: 0o700 }); + fs.writeFileSync(configPath, JSON.stringify({ mcpServers }, null, 2), { mode: 0o600 }); + fs.chmodSync(configPath, 0o600); + console.log(`Wrote ${Object.keys(mcpServers).length} MCP server(s) to ${configPath}`); + log-parser: | + function parseLog(logContent) { + const lines = logContent.split("\n"); + const logEntries = []; + const mcpFailures = []; + let maxTurnsHit = false; + const AWF_INFRA_RE = /^\[(INFO|WARN|SUCCESS|ERROR|entrypoint|health-check|pydantic-ai)\]|^ (?:Container|Network|Volume) |^Process exiting with code:/; + let inputTokens = 0; + let outputTokens = 0; + let toolCallIndex = 0; + let turnCount = 0; + let pendingText = []; + + function flushText() { + if (pendingText.length === 0) return; + const text = pendingText.join("\n").trim(); + if (text) { + logEntries.push({ type: "assistant", message: { content: [{ type: "text", text }] } }); + turnCount++; + } + pendingText = []; + } + + logEntries.push({ type: "system", subtype: "init", model: null, session_id: null }); + + for (const line of lines) { + if (!line.trim()) continue; + if (AWF_INFRA_RE.test(line)) continue; + if (/max.?turns|maximum.*turns.*reached|turn limit/i.test(line)) maxTurnsHit = true; + if (/MCP server .* failed|MCP.*connection.*error|Failed to connect to MCP/i.test(line)) { + const serverMatch = line.match(/MCP server ['"]?([^\s'"]+)['"]?/i); + mcpFailures.push(serverMatch ? serverMatch[1] : line.trim()); + } + + let parsed = null; + try { + if (line.trim().startsWith("{")) parsed = JSON.parse(line.trim()); + } catch (e) { /* not JSON */ } + + if (parsed) { + if (parsed.input_tokens) inputTokens += parsed.input_tokens; + if (parsed.output_tokens) outputTokens += parsed.output_tokens; + const entryType = parsed.type != null ? String(parsed.type) : "log"; + const msg = parsed.msg || parsed.message || parsed.content || ""; + + if (/tool[._]call|tool[._]use/i.test(entryType)) { + flushText(); + const toolId = `pai_tool_${toolCallIndex++}`; + const toolName = parsed.tool || parsed.name || entryType; + logEntries.push({ type: "assistant", message: { content: [{ type: "tool_use", id: toolId, name: toolName, input: {} }] } }); + logEntries.push({ type: "user", message: { content: [{ type: "tool_result", tool_use_id: toolId, content: msg }] } }); + } else if (msg) { + pendingText.push(msg); + } else if (!parsed.input_tokens && !parsed.output_tokens) { + // A JSON line carrying none of the text fields is still assistant output -- + // a reply that is bare JSON, say -- so it is kept as written. A usage record + // is not: its numbers were just added to the totals. + pendingText.push(line.trim()); + } + } else { + pendingText.push(line.trim()); + } + } + flushText(); + + const usage = {}; + if (inputTokens) usage.input_tokens = inputTokens; + if (outputTokens) usage.output_tokens = outputTokens; + logEntries.push({ type: "result", num_turns: turnCount, usage }); + const parts = [`**Turns:** ${turnCount}`, `**Tool calls:** ${toolCallIndex}`]; + if (inputTokens || outputTokens) parts.push(`**Tokens:** ${((inputTokens ?? 0) + (outputTokens ?? 0)).toLocaleString()}`); + if (mcpFailures.length) parts.push(`**MCP failures:** ${mcpFailures.length}`); + if (maxTurnsHit) parts.push("**Max turns reached**"); + return { markdown: parts.join(" · "), logEntries, mcpFailures, maxTurnsHit }; + } +--- + + diff --git a/pkg/workflow/enclave_github_proxy_test.go b/pkg/workflow/enclave_github_proxy_test.go index 352a9c3d591..feeb8ef11e1 100644 --- a/pkg/workflow/enclave_github_proxy_test.go +++ b/pkg/workflow/enclave_github_proxy_test.go @@ -395,9 +395,12 @@ Read the private repository's issues through the enclave. // stepOutputRefPattern matches steps..outputs. references, capturing the step id. var stepOutputRefPattern = regexp.MustCompile(`steps\.([A-Za-z0-9_-]+)\.outputs\.[A-Za-z0-9_]+`) -// stepIDPattern matches `id: ` lines in a GitHub Actions step definition, capturing -// the step id. -var stepIDPattern = regexp.MustCompile(`(?m)^\s*id:\s*([A-Za-z0-9_-]+)\s*$`) +// stepIDPattern matches `id: ` lines under a GitHub Actions step definition (emitted +// with indentation deeper than top-level job/workflow keys, e.g. +// " id: determine-automatic-lockdown"), capturing the step id. The minimum indent +// avoids false-positive matches from unrelated `id:` keys at the workflow/job level, while +// tolerating reasonable indentation changes in the generator. +var stepIDPattern = regexp.MustCompile(`(?m)^ {6,}id:\s*([A-Za-z0-9_-]+)\s*$`) // assertNoDanglingStepOutputReferences verifies that every `steps..outputs.*` reference // in the generated lock file corresponds to a step id that is actually emitted somewhere in diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index 8e5b82cbe73..62aa7fc433a 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -148,18 +148,23 @@ func githubGuardPoliciesFromStep(workflowData *WorkflowData, explicitGuardPolici if workflowData == nil { return true } + // Enclave-only GitHub backends (static or dynamic) always derive their guard policy + // statically from the enclave declaration, never from the determine-automatic-lockdown + // step outputs — even though that step may still be generated for such backends solely + // to supply GH_AW_SINK_VISIBILITY for the write-sink policy. + if githubBackendIsStaticEnclaveDelegationOnly(workflowData) || githubBackendIsDynamicDelegationOnly(workflowData) { + return false + } githubTool, hasGitHub := workflowData.Tools["github"] if !hasGitHub { - // Default-tool resolution removes the "github" key when tools.github is false, so an - // absent key can still mean the GitHub MCP server is rendered for enclave delegation. - // The determine-automatic-lockdown step may still run in that case (to supply - // GH_AW_SINK_VISIBILITY for the write-sink policy), but its min_integrity/repos - // outputs must never drive an enclave-only backend's guard policy — that policy is - // always derived statically from the enclave declaration instead. Checking the - // explicit-disable marker directly (rather than githubLockdownDetectionStepEnabled) - // keeps this decision independent of whether the step happens to be generated. - _, explicitlyDisabled := workflowData.ExplicitlyDisabledTools["github"] - return !explicitlyDisabled + // Default-tool resolution removes the "github" key when tools.github is false. The + // static/dynamic enclave-only backends that could otherwise cause + // githubLockdownDetectionStepEnabled to return true here (via its own enclave checks) + // were already excluded above, so deferring to it below only covers the remaining + // case: a plain (non-enclave) GitHub MCP server whose "github" key was removed by + // default-tool resolution. This keeps the result in lockstep with whether the + // determine-automatic-lockdown step is actually generated. + return githubLockdownDetectionStepEnabled(workflowData) } return githubTool != false } From 66cdcf4a43f21721a779a433749eaeb69335a3a6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:41:47 +0000 Subject: [PATCH 4/9] Extract shared githubBackendIsEnclaveOnly helper; remove stray test artifact Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../gh-aw_pydantic.md | 575 ------------------ pkg/workflow/mcp_environment.go | 3 +- pkg/workflow/mcp_github_config.go | 15 +- 3 files changed, 11 insertions(+), 582 deletions(-) delete mode 100644 pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md diff --git a/pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md b/pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md deleted file mode 100644 index 2438433bbff..00000000000 --- a/pkg/workflow/.github/aw/imports/pydantic/pydantic-ai-harness/8e863b5b88c9e41e638f0dc416b8946135b584b7/gh-aw_pydantic.md +++ /dev/null @@ -1,575 +0,0 @@ ---- -runtimes: - python: - version: "3.12" -pre-agent-steps: - - name: Preinstall Pydantic AI coder agent - run: | - # This step runs on the host runner with the checkout as its working - # directory, before the AWF sandbox exists. -P keeps that directory off - # sys.path, so a repo-local pip.py or pydantic_ai_harness/ cannot be - # imported in place of the installed packages. - # - # 2.36.0 is the first pydantic-ai-slim release carrying `pai --mcp-config`, - # which is how the gateway's MCP servers reach the agent. - # - # The anthropic extra is what an `anthropic/` model runs on: that backend of - # the api-proxy serves the Messages API, not Chat Completions. - python3 -P -m pip install --quiet --user --disable-pip-version-check "pydantic-ai-harness[cli]==$GH_AW_ENGINE_VERSION" "pydantic-ai-slim[anthropic,openai,mcp]>=2.36.0" - "$HOME/.local/bin/pai" --version - python3 -P -c "from pydantic_ai_harness import Coder" -engine: - id: pydantic-ai - version: "0.26.0" - display-name: Pydantic AI - description: Pydantic AI CLI (pai) running the pydantic-ai-harness coder agent with MCP tool support - experimental: true - mcp: true - provider: - name: github - behaviors: - secret-strategy: universal-llm-consumer - # Repository paths gh-aw treats as this engine's configuration: it protects them - # from pull-request modification and derives the inline sub-agent and skill - # directories from the first prefix (.pydantic-ai/agents, .pydantic-ai/skills). - # The engine itself writes nothing into the checkout. - manifest: - files: - - AGENTS.md - path-prefixes: - - .pydantic-ai/ - network: - defaults: - - host.docker.internal - - github.com - - raw.githubusercontent.com - - api.github.com - - objects.githubusercontent.com - - pypi.org - - files.pythonhosted.org - provider-domains: - copilot: api.githubcopilot.com - anthropic: api.anthropic.com - openai: api.openai.com - codex: api.openai.com - execution: - command-name: pai - step-name: Execute Pydantic AI CLI - model-env-var: PAI_MODEL - write-timestamp: true - provider-env-mode: universal-llm-consumer - harness-script: | - const { spawnSync } = require("child_process"); - const { chmodSync, existsSync, mkdtempSync, readFileSync, writeFileSync } = require("fs"); - const { homedir, tmpdir } = require("os"); - const { join } = require("path"); - const { fetchAWFReflect, resolveProviderEndpointFromReflect, deriveBaseUrlFromModelsURL } = require("./awf_reflect.cjs"); - - // gh-aw passes `execution.command-name` (or a workflow's `engine.command`) - // first, then `execution.args`. The name is not spawned -- the CLI is started - // by the interpreter that owns the install, see LAUNCHER below -- so only the - // arguments after it are forwarded. - const commandArgs = process.argv.slice(3); - const log = message => process.stderr.write(`[pydantic-ai] ${message}\n`); - - // `pai -a` takes one target, either an import path or a JSON/YAML agent - // spec, and the spec format resolves capability names through a closed - // registry that the harness capabilities are not part of, so the coder - // composition cannot be expressed as a spec. It is written as a Python - // module instead: `Coder()` supplies the filesystem, shell, planning and - // sub-agent tools. - // - // The gateway's MCP servers are deliberately not part of the module. - // `pai --mcp-config` reads the same Claude-shaped config file through the - // same `pydantic_ai.mcp.load_mcp_toolsets`, `${VAR}` expansion included, so - // routing them through the CLI is what lets a `PAI_AGENT` agent receive - // them on identical terms. - const AGENT_MODULE = `from pydantic_ai import Agent - from pydantic_ai_harness import Coder - - agent = Agent(name="coder", capabilities=[Coder()]) - `; - const DEFAULT_AGENT = "gh_aw_agent:agent"; - - // The CLI runs inside the interpreter that owns the install rather than as a - // separate `pai` process, so the agent module is imported once, in the process - // that runs it. Three things follow from that. - // - // `pai` reduces any failed `-a` load to one line naming the target, so an - // agent that raises on import would reach the step log without its traceback. - // The import here happens before the CLI starts, and an unhandled exception is - // the step's failure, traceback included. - // - // `pydantic_ai._cli.load_agent` prepends the working directory -- the checkout - // -- to sys.path before resolving the target, ahead of PYTHONPATH. A - // repository file named `gh_aw_agent.py` would therefore be loaded in place of - // the generated module. Importing the target first settles which file the name - // means, because an import of a module already in sys.modules does not search - // the path again. - // - // A separate preflight process could do neither: it would import the module in - // one interpreter and leave the CLI to import it again in another, running any - // module-level work in the agent twice. - // - // The residual is `load_agent`'s insert itself: everything the agent imports - // after that point still sees the checkout first on sys.path. That is the - // CLI's documented behavior for its own users, and not something this file - // can change from the outside. - // - // `-P` keeps the `-c` invocation from putting the working directory on - // sys.path on its own account; PYTHONPATH below is what makes the agent - // importable. A spec file, and the dotted `module.attribute` form the CLI also - // accepts, are left to the CLI as before. - const LAUNCHER = `import runpy - import sys - - target, *cli_args = sys.argv[1:] - module, separator, attribute = target.rpartition(":") - if separator and not target.lower().endswith((".yml", ".yaml", ".json")): - import importlib - - from pydantic_ai import Agent - - loaded = getattr(importlib.import_module(module), attribute) - if not isinstance(loaded, Agent): - raise TypeError(f"{target} is {type(loaded).__name__}, not pydantic_ai.Agent") - - sys.argv = ["pai", *cli_args] - runpy.run_module("pydantic_ai", run_name="__main__", alter_sys=True) - `; - - const main = async () => { - const workspace = process.env.GITHUB_WORKSPACE; - if (!workspace) throw new Error("GITHUB_WORKSPACE is required"); - const promptFile = process.env.GH_AW_PROMPT; - if (!promptFile) throw new Error("GH_AW_PROMPT is required"); - - // Neither the generated module nor the gateway's MCP config is written into - // the checkout. A file committed at a path the engine reads is - // repository-controlled input to a process that runs with the gateway's - // credentials: an `mcp.json` there can name a stdio server for the CLI to - // spawn, and a package there shadows an installed one for the whole run. The - // module goes to a private directory created inside the sandbox; the config - // adapter writes on the host into the `${RUNNER_TEMP}/gh-aw` tree that the - // agent step mounts read-only, where gh-aw's own Claude and Codex converters - // write theirs. - // - // `PAI_AGENT` runs an agent the repository defines, in whichever form - // `pai -a` accepts. The generated module is not written in that case: - // nothing would load it, and a stale copy on disk is worse than none. - const configuredAgent = process.env.PAI_AGENT; - const agentTarget = configuredAgent || DEFAULT_AGENT; - const moduleDir = configuredAgent ? "" : mkdtempSync(join(tmpdir(), "gh-aw-pydantic-ai-")); - if (moduleDir) { - const agentModulePath = join(moduleDir, "gh_aw_agent.py"); - writeFileSync(agentModulePath, AGENT_MODULE, { mode: 0o600 }); - chmodSync(agentModulePath, 0o600); - } - - const env = { ...process.env }; - // `pip install --user` puts `pai` here. The runner tool cache that holds - // `uv` and the interpreter's own bin directory is under /opt, which the - // sandbox exposes read-only, but the home directory is where the CLI and - // its user site-packages actually live. - // - // Which interpreter owns those user site-packages matters: only the one - // that ran the pre-agent `pip install --user` can import them, and the - // sandbox prelude prepends every `bin` directory under the runner tool - // cache — which caches several Python versions — so a bare `python3` - // there resolves by `find` order rather than to the installing - // interpreter. `actions/setup-python` names that one in `pythonLocation`; - // putting its `bin` on PATH also gives the agent's own shell tool a - // `python3` that can see the installed packages. - const pythonBin = process.env.pythonLocation ? join(process.env.pythonLocation, "bin") : ""; - const python = pythonBin ? join(pythonBin, "python3") : "python3"; - env.PATH = [join(homedir(), ".local", "bin"), pythonBin, process.env.PATH || ""].filter(Boolean).join(":"); - // The module is reached through PYTHONPATH rather than by importing it as a - // package, and prepending keeps a caller-supplied PYTHONPATH usable. - // - // The checkout itself joins the path only under `PAI_AGENT`. That is the - // opt-in: it makes repository code importable, which is the whole point - // of running your own agent, and it is exactly what `-P` on the install - // step keeps off the path for the default composition. - env.PYTHONPATH = [moduleDir, configuredAgent ? workspace : "", process.env.PYTHONPATH || ""].filter(Boolean).join(":"); - delete env.COPILOT_GITHUB_TOKEN; - - const provider = process.env.GH_AW_LLM_PROVIDER; - const configuredBaseUrl = process.env.PAI_BASE_URL; - - // `pai` sends the model name verbatim, minus the provider marker that - // selects one of its clients, so the bare model ID reaches the api-proxy — - // which steers to the configured provider by the port it is reached on, not - // by a prefix in the model name: Copilot rejects `copilot/` with - // `model_not_supported`. - // Only the first segment is the provider. Stripping greedily would eat an - // org namespace out of ids like `meta-llama/Llama-3.1`, so this mirrors the - // `SplitN(model, "/", 2)` gh-aw itself uses to read the provider off. - if (!env.PAI_MODEL) throw new Error("PAI_MODEL is required"); - const modelProvider = env.PAI_MODEL.split("/", 1)[0].trim().toLowerCase(); - const requestedModel = env.PAI_MODEL.replace(/^[^/]*\//, ""); - // The api-proxy's Anthropic backend forwards the request path to - // api.anthropic.com unchanged and rewrites Messages-shaped bodies; it does - // not translate Chat Completions into Messages. So `anthropic/` is addressed - // with the Messages API: `anthropic:` on `-m`, and ANTHROPIC_BASE_URL for - // the endpoint. The Copilot and Codex backends are OpenAI-shaped and stay on - // Chat Completions, and `PAI_BASE_URL` names a Chat Completions endpoint by - // definition, so it keeps every provider there too. - const useMessagesAPI = !configuredBaseUrl && modelProvider === "anthropic"; - // The dotted-alias rewrite describes the api-proxy's Copilot backend, - // which publishes Copilot's Claude models under dotted IDs. Every other - // destination — the anthropic and openai backends, or an endpoint named - // by PAI_BASE_URL — gets the id the workflow wrote: a model actually - // called `claude-sonnet-4-5` there has to arrive as that. - const model = !configuredBaseUrl && modelProvider === "copilot" - ? requestedModel.replace(/^(claude-(?:haiku|sonnet|opus)-\d+)-(\d+)$/, "$1.$2") - : requestedModel; - - // `PAI_BASE_URL` points the engine at an OpenAI-compatible endpoint of the - // workflow's choosing instead of the AWF api-proxy. Two constraints shape - // it. - // - // It has to be a variable of this definition's own, because AWF sets the - // backend's own base URL variable on this step itself (OPENAI_BASE_URL, or - // ANTHROPIC_BASE_URL for the anthropic backend), pointing at the api-proxy - // on host.docker.internal whenever the firewall is enabled, so its presence - // cannot carry the workflow's intent, and reading it as intent is what - // made the pre-#52843 definition pick the wrong endpoint. - // - // There is deliberately no matching key knob. gh-aw excludes any - // `engine.env` value holding a secret from the agent sandbox - // (`awf --exclude-env`), so a credential cannot be delivered here at all - // and the API key below stays the placeholder. The endpoint therefore - // has to accept that placeholder, or be fronted by something upstream of - // the agent that adds the real credential. - let baseUrl = configuredBaseUrl || (useMessagesAPI ? process.env.ANTHROPIC_BASE_URL : process.env.OPENAI_BASE_URL); - if (!configuredBaseUrl) { - // Only /reflect discovery needs the provider: it selects which of the - // api-proxy's configured endpoints to use. A caller-supplied base URL - // names the endpoint outright, so demanding a provider alongside it - // would reject a complete configuration. - if (!provider) throw new Error("GH_AW_LLM_PROVIDER is required"); - if (process.env.AWF_REFLECT_ENABLED === "1") { - const result = await fetchAWFReflect({ logger: log }); - if (!result.ok || !result.reflectData) { - throw new Error(`Unable to discover the Pydantic AI LLM endpoint from /reflect: ${result.reason || "empty response"}`); - } - const endpoint = resolveProviderEndpointFromReflect({ - provider, - reflectData: result.reflectData, - logger: log, - }); - if (!endpoint?.baseUrl) { - throw new Error(`No configured /reflect endpoint found for provider ${provider}`); - } - baseUrl = endpoint.baseUrl; - const reflectedEndpoint = result.reflectData.endpoints?.find( - entry => entry?.configured === true && entry.provider === endpoint.endpointProvider - ); - if (!useMessagesAPI && typeof reflectedEndpoint?.models_url === "string") { - // `endpoint.baseUrl` is the models-listing origin, while the - // OpenAI-compatible client posts to `/chat/completions`, so the - // path prefix carried by models_url (`/v1` on some providers) has to - // come along — and this helper applies the same api-proxy -> - // host.docker.internal rewrite. - // - // The Anthropic client keeps the origin instead: it appends - // `/v1/messages` itself, so carrying the prefix over would post to - // `/v1/v1/messages`. - baseUrl = deriveBaseUrlFromModelsURL(reflectedEndpoint.models_url); - } - } - } - if (!baseUrl) { - throw new Error( - `Pydantic AI requires AWF endpoint discovery, PAI_BASE_URL or ${useMessagesAPI ? "ANTHROPIC_BASE_URL" : "OPENAI_BASE_URL"}` - ); - } - // The AWF api-proxy injects the real upstream credentials and ignores the - // inbound key, but neither client constructs itself without one. Setting it - // also replaces whatever key this step inherited, so the agent process holds - // the placeholder rather than a provider credential. - if (useMessagesAPI) { - env.ANTHROPIC_BASE_URL = baseUrl; - env.ANTHROPIC_API_KEY = "awf-anthropic-proxy"; - } else { - env.OPENAI_BASE_URL = baseUrl; - env.OPENAI_API_KEY = "awf-copilot-proxy"; - } - - // `-m` is always passed: the composed agent carries no model, and without - // the flag `pai` silently falls back to its own `openai:gpt-5` default, - // billing a model the workflow never asked for. gh-aw validates - // `provider/model` at compile time, so PAI_MODEL is set for every compiled - // workflow, and the throw above covers any other invocation. - // - // An explicit `-m` also replaces the model a loaded agent declares, so a - // `PAI_AGENT` agent runs on the workflow's `engine.model` whatever it was - // constructed with. That is what routes it through the endpoint above. - const cliArgs = [...commandArgs, "-a", agentTarget]; - // The config adapter writes this file only for a workflow that configures - // MCP tools, and `--mcp-config` fails on a path that is not there, so its - // absence has to mean "no servers" rather than an error. The - // `RUNNER_TEMP || "/tmp"` fallback is the one gh-aw's own converters use, and - // the adapter resolves this path by the same expression. - const mcpConfig = join(process.env.RUNNER_TEMP || "/tmp", "gh-aw", "mcp-config", "mcp-servers.json"); - if (existsSync(mcpConfig)) cliArgs.push("--mcp-config", mcpConfig); - cliArgs.push("-m", `${useMessagesAPI ? "anthropic" : "openai-chat"}:${model}`, readFileSync(promptFile, "utf8")); - log( - `provider=${configuredBaseUrl ? "(PAI_BASE_URL)" : provider} model=${model} baseUrl=${baseUrl}` + - (configuredAgent ? ` agent=${configuredAgent}` : "") - ); - // The target is passed twice on purpose: once for LAUNCHER, which imports it - // and hands the CLI a module already in sys.modules, and once as the `-a` - // the CLI parses for itself. - const result = spawnSync(python, ["-P", "-c", LAUNCHER, agentTarget, ...cliArgs], { cwd: workspace, env, stdio: "inherit" }); - if (result.error) throw result.error; - if (result.status !== 0) { - const error = new Error(`Pydantic AI execution failed with exit code ${result.status ?? "unknown"}`); - // Surface the child's own status so the step fails with the same code. - error.exitCode = typeof result.status === "number" && result.status !== 0 ? result.status : 1; - throw error; - } - }; - - main().catch(error => { - log(error instanceof Error ? error.message : String(error)); - process.exitCode = typeof error?.exitCode === "number" && error.exitCode !== 0 ? error.exitCode : 1; - }); - mcp: - config-path: ${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json - config-adapter: | - // Renders the MCP gateway's configuration as the Claude-style - // `mcpServers` document that `pydantic_ai.mcp.load_mcp_toolsets` reads, - // which the harness script hands to `pai --mcp-config`. Only HTTP entries - // are carried: `load_mcp_toolsets` can host stdio - // servers too, but the gateway already fronts every configured server - // over HTTP, and CLI-mounted servers are excluded because the agent - // reaches those as executables on PATH instead. - const fs = require("fs"); - const path = require("path"); - - const requireEnvVar = name => { - const value = process.env[name]; - if (!value) throw new Error(`${name} environment variable is required`); - return value; - }; - - const gatewayOutputPath = requireEnvVar("MCP_GATEWAY_OUTPUT"); - const gatewayDomain = process.env.MCP_GATEWAY_DOMAIN || "host.docker.internal"; - const gatewayPort = requireEnvVar("MCP_GATEWAY_PORT"); - const gatewayURL = `http://${gatewayDomain}:${gatewayPort}`; - - let cliServers; - try { - cliServers = new Set(JSON.parse(process.env.GH_AW_MCP_CLI_SERVERS || "[]")); - } catch (error) { - throw new Error(`Failed to parse GH_AW_MCP_CLI_SERVERS: ${error instanceof Error ? error.message : String(error)}`); - } - - const gatewayOutput = JSON.parse(fs.readFileSync(gatewayOutputPath, "utf8")); - const rawServers = gatewayOutput.mcpServers; - const servers = rawServers && typeof rawServers === "object" && !Array.isArray(rawServers) ? rawServers : {}; - - const mcpServers = {}; - for (const [name, entry] of Object.entries(servers)) { - if (cliServers.has(name) || !entry || typeof entry !== "object") continue; - if (typeof entry.url !== "string") { - console.log(`Skipping MCP server ${name}: the Pydantic AI engine only supports HTTP MCP servers`); - continue; - } - const server = { url: entry.url.replace(/^http:\/\/[^/]+\/mcp\//, `${gatewayURL}/mcp/`) }; - if (entry.headers && typeof entry.headers === "object") server.headers = entry.headers; - mcpServers[name] = server; - } - - // This script runs on the host runner, in the Start MCP Gateway step, so it - // writes where that step already created a directory and where the agent step - // mounts `${RUNNER_TEMP}/gh-aw` read-only -- the same file the built-in Claude - // converter produces, which is also the path gh-aw's log redaction scans for - // the gateway bearer token. The harness script resolves it by the same - // expression. Keeping it out of the checkout is what stops a committed - // `mcp.json` from reaching `pai --mcp-config`; see the harness script. - const configPath = path.join(process.env.RUNNER_TEMP || "/tmp", "gh-aw", "mcp-config", "mcp-servers.json"); - fs.mkdirSync(path.dirname(configPath), { recursive: true, mode: 0o700 }); - fs.writeFileSync(configPath, JSON.stringify({ mcpServers }, null, 2), { mode: 0o600 }); - fs.chmodSync(configPath, 0o600); - console.log(`Wrote ${Object.keys(mcpServers).length} MCP server(s) to ${configPath}`); - log-parser: | - function parseLog(logContent) { - const lines = logContent.split("\n"); - const logEntries = []; - const mcpFailures = []; - let maxTurnsHit = false; - const AWF_INFRA_RE = /^\[(INFO|WARN|SUCCESS|ERROR|entrypoint|health-check|pydantic-ai)\]|^ (?:Container|Network|Volume) |^Process exiting with code:/; - let inputTokens = 0; - let outputTokens = 0; - let toolCallIndex = 0; - let turnCount = 0; - let pendingText = []; - - function flushText() { - if (pendingText.length === 0) return; - const text = pendingText.join("\n").trim(); - if (text) { - logEntries.push({ type: "assistant", message: { content: [{ type: "text", text }] } }); - turnCount++; - } - pendingText = []; - } - - logEntries.push({ type: "system", subtype: "init", model: null, session_id: null }); - - for (const line of lines) { - if (!line.trim()) continue; - if (AWF_INFRA_RE.test(line)) continue; - if (/max.?turns|maximum.*turns.*reached|turn limit/i.test(line)) maxTurnsHit = true; - if (/MCP server .* failed|MCP.*connection.*error|Failed to connect to MCP/i.test(line)) { - const serverMatch = line.match(/MCP server ['"]?([^\s'"]+)['"]?/i); - mcpFailures.push(serverMatch ? serverMatch[1] : line.trim()); - } - - let parsed = null; - try { - if (line.trim().startsWith("{")) parsed = JSON.parse(line.trim()); - } catch (e) { /* not JSON */ } - - if (parsed) { - if (parsed.input_tokens) inputTokens += parsed.input_tokens; - if (parsed.output_tokens) outputTokens += parsed.output_tokens; - const entryType = parsed.type != null ? String(parsed.type) : "log"; - const msg = parsed.msg || parsed.message || parsed.content || ""; - - if (/tool[._]call|tool[._]use/i.test(entryType)) { - flushText(); - const toolId = `pai_tool_${toolCallIndex++}`; - const toolName = parsed.tool || parsed.name || entryType; - logEntries.push({ type: "assistant", message: { content: [{ type: "tool_use", id: toolId, name: toolName, input: {} }] } }); - logEntries.push({ type: "user", message: { content: [{ type: "tool_result", tool_use_id: toolId, content: msg }] } }); - } else if (msg) { - pendingText.push(msg); - } else if (!parsed.input_tokens && !parsed.output_tokens) { - // A JSON line carrying none of the text fields is still assistant output -- - // a reply that is bare JSON, say -- so it is kept as written. A usage record - // is not: its numbers were just added to the totals. - pendingText.push(line.trim()); - } - } else { - pendingText.push(line.trim()); - } - } - flushText(); - - const usage = {}; - if (inputTokens) usage.input_tokens = inputTokens; - if (outputTokens) usage.output_tokens = outputTokens; - logEntries.push({ type: "result", num_turns: turnCount, usage }); - const parts = [`**Turns:** ${turnCount}`, `**Tool calls:** ${toolCallIndex}`]; - if (inputTokens || outputTokens) parts.push(`**Tokens:** ${((inputTokens ?? 0) + (outputTokens ?? 0)).toLocaleString()}`); - if (mcpFailures.length) parts.push(`**MCP failures:** ${mcpFailures.length}`); - if (maxTurnsHit) parts.push("**Max turns reached**"); - return { markdown: parts.join(" · "), logEntries, mcpFailures, maxTurnsHit }; - } ---- - - diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 7cf0eed4147..644110ebdf9 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -103,8 +103,7 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor // though the step may still be generated to supply GH_AW_SINK_VISIBILITY. // Security: Pass step outputs through environment variables to prevent template injection. guardPoliciesExplicit := len(getGitHubGuardPolicies(toolConfig)) > 0 - enclaveOnlyGitHubBackend := githubBackendIsStaticEnclaveDelegationOnly(workflowData) || githubBackendIsDynamicDelegationOnly(workflowData) - if githubToolEnabledInTools && !guardPoliciesExplicit && !enclaveOnlyGitHubBackend && githubLockdownDetectionStepEnabled(workflowData) { + if githubToolEnabledInTools && !guardPoliciesExplicit && !githubBackendIsEnclaveOnly(workflowData) && githubLockdownDetectionStepEnabled(workflowData) { envVars["GITHUB_MCP_GUARD_MIN_INTEGRITY"] = "${{ steps.determine-automatic-lockdown.outputs.min_integrity }}" envVars["GITHUB_MCP_GUARD_REPOS"] = "${{ steps.determine-automatic-lockdown.outputs.repos }}" } diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index 62aa7fc433a..4520eb1b247 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -141,6 +141,15 @@ func githubBackendIsStaticEnclaveDelegationOnly(workflowData *WorkflowData) bool return enclaveGitHubIssuesEnabled(workflowData) && !primaryGitHubMCPEnabled(workflowData) } +// githubBackendIsEnclaveOnly reports whether the GitHub MCP server is rendered solely to +// serve an enclave agent identity (static or dynamic), with no primary-agent GitHub access. +// Backends in this state always derive their guard/write-sink policies statically from the +// enclave declaration, never from the determine-automatic-lockdown step outputs, even though +// that step may still be generated for them solely to supply GH_AW_SINK_VISIBILITY. +func githubBackendIsEnclaveOnly(workflowData *WorkflowData) bool { + return githubBackendIsStaticEnclaveDelegationOnly(workflowData) || githubBackendIsDynamicDelegationOnly(workflowData) +} + func githubGuardPoliciesFromStep(workflowData *WorkflowData, explicitGuardPolicies map[string]any) bool { if len(explicitGuardPolicies) > 0 { return false @@ -148,11 +157,7 @@ func githubGuardPoliciesFromStep(workflowData *WorkflowData, explicitGuardPolici if workflowData == nil { return true } - // Enclave-only GitHub backends (static or dynamic) always derive their guard policy - // statically from the enclave declaration, never from the determine-automatic-lockdown - // step outputs — even though that step may still be generated for such backends solely - // to supply GH_AW_SINK_VISIBILITY for the write-sink policy. - if githubBackendIsStaticEnclaveDelegationOnly(workflowData) || githubBackendIsDynamicDelegationOnly(workflowData) { + if githubBackendIsEnclaveOnly(workflowData) { return false } githubTool, hasGitHub := workflowData.Tools["github"] From ba848b6a1b0df9378c13b02d7f8fef71ef6f3a27 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:44:10 +0000 Subject: [PATCH 5/9] Use staticEnclaveWriteSinkGuardPolicy predicate directly to keep step gen/reference in sync Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/mcp_environment.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 644110ebdf9..4178a95708d 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -116,9 +116,12 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor // template injection. The value is the raw step output (no toJSON), and the surrounding // JSON double-quotes in the heredoc produce a valid JSON string at runtime: // "sink-visibility": "${GH_AW_SINK_VISIBILITY}" → "sink-visibility": "public" + // The static-enclave check here uses staticEnclaveWriteSinkGuardPolicy directly — the same + // predicate generateGitHubMCPLockdownDetectionStep's caller (githubLockdownDetectionStepEnabled) + // uses to decide whether to generate the step — so the two can never drift out of sync. // Gating on githubLockdownDetectionStepEnabled ensures this never references a step that // isn't actually generated (a dangling steps..outputs.* reference). - sinkVisibilityRelevant := githubToolEnabledInTools || enclaveDynamicRepositoryPolicyEnabled(workflowData) || githubBackendIsStaticEnclaveDelegationOnly(workflowData) + sinkVisibilityRelevant := githubToolEnabledInTools || enclaveDynamicRepositoryPolicyEnabled(workflowData) || staticEnclaveWriteSinkGuardPolicy(workflowData) != nil if sinkVisibilityRelevant && githubLockdownDetectionStepEnabled(workflowData) { envVars[sinkVisibilityEnvVar] = "${{ steps.determine-automatic-lockdown.outputs.visibility }}" } From f4769afea03fea2bef39e9c708d3f3ded9288035 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:46:16 +0000 Subject: [PATCH 6/9] Name step-id indent constant with explanatory comment for regression test Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/enclave_github_proxy_test.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/workflow/enclave_github_proxy_test.go b/pkg/workflow/enclave_github_proxy_test.go index feeb8ef11e1..306da34953c 100644 --- a/pkg/workflow/enclave_github_proxy_test.go +++ b/pkg/workflow/enclave_github_proxy_test.go @@ -3,6 +3,7 @@ package workflow import ( + "fmt" "os" "path/filepath" "regexp" @@ -395,12 +396,18 @@ Read the private repository's issues through the enclave. // stepOutputRefPattern matches steps..outputs. references, capturing the step id. var stepOutputRefPattern = regexp.MustCompile(`steps\.([A-Za-z0-9_-]+)\.outputs\.[A-Za-z0-9_]+`) -// stepIDPattern matches `id: ` lines under a GitHub Actions step definition (emitted -// with indentation deeper than top-level job/workflow keys, e.g. -// " id: determine-automatic-lockdown"), capturing the step id. The minimum indent -// avoids false-positive matches from unrelated `id:` keys at the workflow/job level, while -// tolerating reasonable indentation changes in the generator. -var stepIDPattern = regexp.MustCompile(`(?m)^ {6,}id:\s*([A-Za-z0-9_-]+)\s*$`) +// stepIDMinIndent is the minimum indentation (in spaces) of an `id:` key that belongs to a +// GitHub Actions step definition rather than a top-level workflow/job/matrix key. Steps are +// nested at "jobs: : steps: - name: ...", which the compiler currently renders with 8 +// spaces of indentation for step-level keys (see e.g. the " id: ..." lines written by +// generateGitHubMCPLockdownDetectionStep). A lower floor is used here so the check still +// matches if the generator's exact nesting depth changes slightly, while still excluding +// unrelated `id:` keys declared at the workflow or job level (which use less indentation). +const stepIDMinIndent = 6 + +// stepIDPattern matches `id: ` lines under a GitHub Actions step definition (e.g. +// " id: determine-automatic-lockdown"), capturing the step id. +var stepIDPattern = regexp.MustCompile(fmt.Sprintf(`(?m)^ {%d,}id:\s*([A-Za-z0-9_-]+)\s*$`, stepIDMinIndent)) // assertNoDanglingStepOutputReferences verifies that every `steps..outputs.*` reference // in the generated lock file corresponds to a step id that is actually emitted somewhere in From 913af052eb19bbb9e506a90a9153f56c79990ede Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:48:46 +0000 Subject: [PATCH 7/9] Simplify sink-visibility gating to reuse shared enclave-only helper Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/mcp_environment.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 4178a95708d..8514ecfddbe 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -110,19 +110,17 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor } // Emit GH_AW_SINK_VISIBILITY for all workflows where the determine-automatic-lockdown step - // runs (i.e., any workflow with a GitHub tool, a dynamic enclave, or a static enclave-only - // GitHub backend whose write-sink policy needs the destination visibility). This avoids + // runs (i.e., any workflow with a GitHub tool, or an enclave-only GitHub backend — static + // or dynamic — whose write-sink policy needs the destination visibility). This avoids // embedding a ${{ }} expression directly in the run: heredoc, which zizmor flags as // template injection. The value is the raw step output (no toJSON), and the surrounding // JSON double-quotes in the heredoc produce a valid JSON string at runtime: // "sink-visibility": "${GH_AW_SINK_VISIBILITY}" → "sink-visibility": "public" - // The static-enclave check here uses staticEnclaveWriteSinkGuardPolicy directly — the same - // predicate generateGitHubMCPLockdownDetectionStep's caller (githubLockdownDetectionStepEnabled) - // uses to decide whether to generate the step — so the two can never drift out of sync. + // githubBackendIsEnclaveOnly is the same shared helper used to decide whether the primary + // agent's automatic guard-policy env vars apply, keeping both decisions in sync. // Gating on githubLockdownDetectionStepEnabled ensures this never references a step that // isn't actually generated (a dangling steps..outputs.* reference). - sinkVisibilityRelevant := githubToolEnabledInTools || enclaveDynamicRepositoryPolicyEnabled(workflowData) || staticEnclaveWriteSinkGuardPolicy(workflowData) != nil - if sinkVisibilityRelevant && githubLockdownDetectionStepEnabled(workflowData) { + if (githubToolEnabledInTools || githubBackendIsEnclaveOnly(workflowData)) && githubLockdownDetectionStepEnabled(workflowData) { envVars[sinkVisibilityEnvVar] = "${{ steps.determine-automatic-lockdown.outputs.visibility }}" } if enclaveDynamicRepositoryPolicyEnabled(workflowData) { From 3cf873653a529d39ee6d63d22ec574127f9d4f70 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:51:50 +0000 Subject: [PATCH 8/9] Document why step-generation uses precise predicates instead of githubBackendIsEnclaveOnly Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/compiler_github_mcp_steps.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/workflow/compiler_github_mcp_steps.go b/pkg/workflow/compiler_github_mcp_steps.go index e8b0a8b3285..8e7f87ecff4 100644 --- a/pkg/workflow/compiler_github_mcp_steps.go +++ b/pkg/workflow/compiler_github_mcp_steps.go @@ -27,6 +27,16 @@ func githubLockdownDetectionStepEnabled(data *WorkflowData) bool { // for its safe-outputs write-sink policy, even when the primary agent has no GitHub // MCP access at all (tools.github: false). Generate the step in that case too, so the // write-sink policy's sink-visibility field always has a valid producer step. + // + // This intentionally checks staticEnclaveWriteSinkGuardPolicy(data) != nil rather than + // the broader githubBackendIsEnclaveOnly(data) helper: the latter is true whenever an + // enclave-only backend exists at all, even one with no allowed repos (in which case + // staticEnclaveWriteSinkGuardPolicy returns nil because there is no write-sink policy to + // populate). Using the broader helper here would generate this step needlessly for those + // repo-less configurations. githubBackendIsEnclaveOnly remains the right check for the + // separate question of "does the primary GitHub MCP server's own guard policy come from + // this step's outputs" (see githubGuardPoliciesFromStep and collectMCPEnvironmentVariables), + // which is unrelated to whether the step itself needs to exist for sink-visibility. if staticEnclaveWriteSinkGuardPolicy(data) != nil { return true } From 336af1098f3381cf88e5acc2e259a9b62e828f1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:22:56 +0000 Subject: [PATCH 9/9] Scope dangling step-output assertions to each job Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/workflow/enclave_github_proxy_test.go | 80 +++++++++++++++-------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/pkg/workflow/enclave_github_proxy_test.go b/pkg/workflow/enclave_github_proxy_test.go index 306da34953c..97df7876bd4 100644 --- a/pkg/workflow/enclave_github_proxy_test.go +++ b/pkg/workflow/enclave_github_proxy_test.go @@ -3,7 +3,6 @@ package workflow import ( - "fmt" "os" "path/filepath" "regexp" @@ -394,35 +393,64 @@ Read the private repository's issues through the enclave. } // stepOutputRefPattern matches steps..outputs. references, capturing the step id. -var stepOutputRefPattern = regexp.MustCompile(`steps\.([A-Za-z0-9_-]+)\.outputs\.[A-Za-z0-9_]+`) - -// stepIDMinIndent is the minimum indentation (in spaces) of an `id:` key that belongs to a -// GitHub Actions step definition rather than a top-level workflow/job/matrix key. Steps are -// nested at "jobs: : steps: - name: ...", which the compiler currently renders with 8 -// spaces of indentation for step-level keys (see e.g. the " id: ..." lines written by -// generateGitHubMCPLockdownDetectionStep). A lower floor is used here so the check still -// matches if the generator's exact nesting depth changes slightly, while still excluding -// unrelated `id:` keys declared at the workflow or job level (which use less indentation). -const stepIDMinIndent = 6 - -// stepIDPattern matches `id: ` lines under a GitHub Actions step definition (e.g. -// " id: determine-automatic-lockdown"), capturing the step id. -var stepIDPattern = regexp.MustCompile(fmt.Sprintf(`(?m)^ {%d,}id:\s*([A-Za-z0-9_-]+)\s*$`, stepIDMinIndent)) +var stepOutputRefPattern = regexp.MustCompile(`steps\.([A-Za-z0-9_-]+)\.outputs\.[A-Za-z0-9_-]+`) // assertNoDanglingStepOutputReferences verifies that every `steps..outputs.*` reference -// in the generated lock file corresponds to a step id that is actually emitted somewhere in -// the file. This guards against the class of bug described in gh-aw#60336, where a consumer -// (e.g. an environment variable or guard policy) references a step's outputs even though the -// producer step itself was never generated, expanding to an empty string at runtime. +// in the generated lock file corresponds to a step id that is actually emitted in the same +// job. For references made from a step field, the producer step must also appear earlier in +// that job's step list. This guards against the class of bug described in gh-aw#60336, where a +// consumer (e.g. an environment variable or guard policy) references a step's outputs even +// though the producer step itself was never generated, expanding to an empty string at runtime. func assertNoDanglingStepOutputReferences(t *testing.T, lock string) { t.Helper() - emittedIDs := make(map[string]bool) - for _, match := range stepIDPattern.FindAllStringSubmatch(lock, -1) { - emittedIDs[match[1]] = true - } - for _, match := range stepOutputRefPattern.FindAllStringSubmatch(lock, -1) { - stepID := match[1] - assert.True(t, emittedIDs[stepID], "reference %q has no corresponding emitted step id %q", match[0], stepID) + + var workflow map[string]any + require.NoError(t, yaml.Unmarshal([]byte(lock), &workflow), "generated lock file must be valid YAML") + + jobs, ok := workflow["jobs"].(map[string]any) + require.True(t, ok, "generated lock file must contain jobs") + + for jobID, jobValue := range jobs { + job, ok := jobValue.(map[string]any) + require.True(t, ok, "job %q must be an object", jobID) + + steps, _ := job["steps"].([]any) + allStepIDs := make(map[string]bool) + for _, stepValue := range steps { + step, ok := stepValue.(map[string]any) + if !ok { + continue + } + if stepID, ok := step["id"].(string); ok { + allStepIDs[stepID] = true + } + } + + jobBytes, err := yaml.Marshal(jobValue) + require.NoError(t, err, "job %q must marshal for step reference checks", jobID) + for _, match := range stepOutputRefPattern.FindAllStringSubmatch(string(jobBytes), -1) { + stepID := match[1] + assert.True(t, allStepIDs[stepID], "job %q reference %q has no corresponding emitted step id %q", jobID, match[0], stepID) + } + + previousStepIDs := make(map[string]bool) + for stepIndex, stepValue := range steps { + step, ok := stepValue.(map[string]any) + if !ok { + continue + } + + stepBytes, err := yaml.Marshal(stepValue) + require.NoError(t, err, "job %q step %d must marshal for step reference checks", jobID, stepIndex) + for _, match := range stepOutputRefPattern.FindAllStringSubmatch(string(stepBytes), -1) { + stepID := match[1] + assert.True(t, previousStepIDs[stepID], "job %q step %d reference %q must refer to a previously emitted step id %q", jobID, stepIndex, match[0], stepID) + } + + if stepID, ok := step["id"].(string); ok { + previousStepIDs[stepID] = true + } + } } }