Fix dangling determine-automatic-lockdown step reference in static enclave sink-visibility policy

pkg/workflow/compiler_github_mcp_steps.go

@@ -23,6 +23,23 @@ 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.
+	//
+	// 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
+	}
 	if githubTool, hasGitHub := data.Tools["github"]; hasGitHub {
 		return githubTool != false
 	}

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.<id>.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,138 @@ Read the private repository's issues through the enclave.
 	assert.Contains(t, lock, `"forcePublicRepos": false`)
 }
 
+// stepOutputRefPattern matches steps.<id>.outputs.<name> references, capturing the step id.
+var stepOutputRefPattern = regexp.MustCompile(`steps\.([A-Za-z0-9_-]+)\.outputs\.[A-Za-z0-9_-]+`)
+
+// assertNoDanglingStepOutputReferences verifies that every `steps.<id>.outputs.*` reference
+// 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()
+
+	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
+			}
+		}
+	}
+}
+
+// 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.<id>.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 +566,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))
 

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) {
+		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 }}"
 		}
 	}
 
 	// 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, 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"
-	if githubToolEnabledInTools || enclaveDynamicRepositoryPolicyEnabled(workflowData) {
+	// 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.<id>.outputs.* reference).
+	if (githubToolEnabledInTools || githubBackendIsEnclaveOnly(workflowData)) && githubLockdownDetectionStepEnabled(workflowData) {
 		envVars[sinkVisibilityEnvVar] = "${{ steps.determine-automatic-lockdown.outputs.visibility }}"
 	}
 	if enclaveDynamicRepositoryPolicyEnabled(workflowData) {

pkg/workflow/mcp_github_config.go

@@ -141,18 +141,34 @@ 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
 	}
 	if workflowData == nil {
 		return true
 	}
+	if githubBackendIsEnclaveOnly(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.
-		// Only reference the lockdown step outputs when that step is actually generated.
+		// 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