diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index a3c38f8c67..7514dfdddf 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -450,20 +450,13 @@ jobs: { "description": "Add the 'smoked' label to the current pull request (can only be called once)", "inputSchema": { - "additionalProperties": false, + "additionalProperties": true, "properties": { - "labels": { - "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", - "type": "string" - }, - "number": { - "description": "The number of the issue or pull request.", + "payload": { + "description": "JSON-encoded payload to pass to the action", "type": "string" } }, - "required": [ - "labels" - ], "type": "object" }, "name": "add_smoked_label" @@ -1568,8 +1561,7 @@ jobs: env: GITHUB_TOKEN: ${{ github.token }} with: - labels: ${{ fromJSON(steps.process_safe_outputs.outputs.action_add_smoked_label_payload).labels }} - number: ${{ fromJSON(steps.process_safe_outputs.outputs.action_add_smoked_label_payload).number }} + payload: ${{ steps.process_safe_outputs.outputs.action_add_smoked_label_payload }} - name: Upload safe output items if: always() uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 diff --git a/actions/setup/js/determine_automatic_lockdown.cjs b/actions/setup/js/determine_automatic_lockdown.cjs index be17474d77..33ea82843c 100644 --- a/actions/setup/js/determine_automatic_lockdown.cjs +++ b/actions/setup/js/determine_automatic_lockdown.cjs @@ -15,9 +15,7 @@ * from the workflow's tools.github guard policy configuration. Pre-configured values * are never overridden. * - * Note: This step is NOT generated when tools.github.app is configured. GitHub App tokens - * are already scoped to specific repositories, so automatic guard policy detection is - * unnecessary. It is also NOT generated when both repos and min-integrity are explicitly + * Note: This step is NOT generated when both repos and min-integrity are explicitly * configured in the workflow. * * @param {any} github - GitHub API client diff --git a/docs/src/content/docs/reference/lockdown-mode.md b/docs/src/content/docs/reference/lockdown-mode.md index 9f3cb44dd9..c08b045928 100644 --- a/docs/src/content/docs/reference/lockdown-mode.md +++ b/docs/src/content/docs/reference/lockdown-mode.md @@ -21,7 +21,6 @@ For **public repositories** where the GitHub MCP server is configured **without* The automatic guard policy does **not** apply when: - An explicit `lockdown` or `min-integrity` value is set in the workflow frontmatter. -- A GitHub App token is configured (`tools.github.app`). To override or disable the automatic guard policy, set an explicit value: diff --git a/pkg/workflow/compiler_github_mcp_steps.go b/pkg/workflow/compiler_github_mcp_steps.go index cf6b0e00cb..fe35e78562 100644 --- a/pkg/workflow/compiler_github_mcp_steps.go +++ b/pkg/workflow/compiler_github_mcp_steps.go @@ -11,12 +11,12 @@ import ( // for GitHub MCP server based on repository visibility. // This step is added when: // - GitHub tool is enabled AND -// - guard policy (repos/min-integrity) is not fully configured in the workflow AND -// - tools.github.app is NOT configured (GitHub App tokens are already repo-scoped, so -// automatic guard policy detection is unnecessary and skipped) +// - guard policy (repos/min-integrity) is not fully configured in the workflow // // For public repositories, the step automatically sets min-integrity to "approved" and // repos to "all" if they are not already configured. +// This applies regardless of whether a GitHub App token is configured, because repo-scoping +// is not a substitute for author-integrity filtering inside a repository. func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder, data *WorkflowData) { // Check if GitHub tool is present githubTool, hasGitHub := data.Tools["github"] @@ -31,15 +31,6 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder, return } - // Skip automatic guard policy detection when a GitHub App is configured. - // GitHub App tokens are already scoped to specific repositories, so automatic - // guard policy detection is not needed — the token's access is inherently bounded - // by the app installation and the listed repositories. - if hasGitHubApp(githubTool) { - githubConfigLog.Print("GitHub App configured, skipping automatic guard policy determination (app tokens are already repo-scoped)") - return - } - githubConfigLog.Print("Generating automatic guard policy determination step for GitHub MCP server") // Resolve the latest version of actions/github-script diff --git a/pkg/workflow/github_mcp_app_token_test.go b/pkg/workflow/github_mcp_app_token_test.go index 0a55ddeb1a..146d3909d7 100644 --- a/pkg/workflow/github_mcp_app_token_test.go +++ b/pkg/workflow/github_mcp_app_token_test.go @@ -254,11 +254,11 @@ Test org-wide GitHub MCP app token. assert.Contains(t, lockContent, "app-id:", "Should include app-id field") } -// TestGitHubMCPAppTokenNoLockdownDetectionStep tests that determine-automatic-lockdown -// step is NOT generated when a GitHub App is configured. -// GitHub App tokens are already scoped to specific repositories, so automatic lockdown -// detection is unnecessary. -func TestGitHubMCPAppTokenNoLockdownDetectionStep(t *testing.T) { +// TestGitHubMCPAppTokenWithLockdownDetectionStep tests that determine-automatic-lockdown +// step IS generated even when a GitHub App is configured. +// Repo-scoping from a GitHub App token does not substitute for author-integrity filtering +// inside a repository; public repos still need automatic min-integrity: approved protection. +func TestGitHubMCPAppTokenWithLockdownDetectionStep(t *testing.T) { compiler := NewCompilerWithVersion("1.0.0") markdown := `--- @@ -280,7 +280,7 @@ tools: # Test Workflow -Test that determine-automatic-lockdown is not generated when app is configured. +Test that determine-automatic-lockdown is generated even when app is configured. ` tmpDir := t.TempDir() @@ -296,10 +296,14 @@ Test that determine-automatic-lockdown is not generated when app is configured. require.NoError(t, err, "Failed to read lock file") lockContent := string(content) - // The automatic lockdown detection step must NOT be present when app is configured - assert.NotContains(t, lockContent, "Determine automatic lockdown mode", "determine-automatic-lockdown step should not be generated when app is configured") - assert.NotContains(t, lockContent, "id: determine-automatic-lockdown", "determine-automatic-lockdown step ID should not be present") - assert.NotContains(t, lockContent, "steps.determine-automatic-lockdown.outputs.lockdown", "lockdown step output reference should not be present") + // The automatic lockdown detection step MUST be present even when app is configured. + // GitHub App repo-scoping does not replace author-integrity filtering for public repos. + assert.Contains(t, lockContent, "Determine automatic lockdown mode", "determine-automatic-lockdown step should be generated even when app is configured") + assert.Contains(t, lockContent, "id: determine-automatic-lockdown", "determine-automatic-lockdown step ID should be present") + + // Guard policy env vars must reference the lockdown step outputs + assert.Contains(t, lockContent, "GITHUB_MCP_GUARD_MIN_INTEGRITY: ${{ steps.determine-automatic-lockdown.outputs.min_integrity }}", "Guard min-integrity env var should reference lockdown step output") + assert.Contains(t, lockContent, "GITHUB_MCP_GUARD_REPOS: ${{ steps.determine-automatic-lockdown.outputs.repos }}", "Guard repos env var should reference lockdown step output") // App token should still be minted and used assert.Contains(t, lockContent, "id: github-mcp-app-token", "GitHub App token step should still be generated") diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 454cf949b4..dfa58c2afa 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -80,11 +80,11 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor } // Add guard policy env vars if the determine-automatic-lockdown step will be generated. - // Skip when a GitHub App is configured or when guard policy is already explicitly set — - // in those cases, the determine-automatic-lockdown step is not generated. + // Skip only when guard policy is already explicitly set — in that case, the + // determine-automatic-lockdown step is not generated. // Security: Pass step outputs through environment variables to prevent template injection. guardPoliciesExplicit := len(getGitHubGuardPolicies(githubTool)) > 0 - if !guardPoliciesExplicit && !appConfigured { + if !guardPoliciesExplicit { envVars["GITHUB_MCP_GUARD_MIN_INTEGRITY"] = "${{ steps.determine-automatic-lockdown.outputs.min_integrity }}" envVars["GITHUB_MCP_GUARD_REPOS"] = "${{ steps.determine-automatic-lockdown.outputs.repos }}" }