Skip to content

Commit c79b157

Browse files
Copilotpelikhan
andcommitted
Add runtime if condition for custom token check in determination step
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent b9c1398 commit c79b157

File tree

3 files changed

+34
-59
lines changed

3 files changed

+34
-59
lines changed

pkg/workflow/github_lockdown_autodetect_test.go

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,46 +13,28 @@ func TestGitHubLockdownAutodetection(t *testing.T) {
1313
workflow string
1414
expectedDetectStep bool
1515
expectedLockdown string // "auto" means use step output expression, "true" means hardcoded true, "false" means not present
16+
expectIfCondition bool // true if step should have if: condition
1617
description string
1718
}{
1819
{
19-
name: "Auto-determination enabled when lockdown not specified and custom token defined",
20+
name: "Auto-determination enabled when lockdown not specified",
2021
workflow: `---
2122
on: issues
2223
engine: copilot
2324
tools:
2425
github:
2526
mode: local
26-
github-token: ${{ secrets.CUSTOM_TOKEN }}
2727
toolsets: [default]
2828
---
2929
3030
# Test Workflow
3131
32-
Test automatic lockdown determination with custom token.
32+
Test automatic lockdown determination.
3333
`,
3434
expectedDetectStep: true,
3535
expectedLockdown: "auto",
36-
description: "When lockdown is not specified and custom token is defined, determination step should be added",
37-
},
38-
{
39-
name: "No auto-determination when no custom token",
40-
workflow: `---
41-
on: issues
42-
engine: copilot
43-
tools:
44-
github:
45-
mode: local
46-
toolsets: [default]
47-
---
48-
49-
# Test Workflow
50-
51-
Test without custom token - should not add determination step.
52-
`,
53-
expectedDetectStep: false,
54-
expectedLockdown: "false",
55-
description: "When no custom token is defined, no determination step should be added",
36+
expectIfCondition: true,
37+
description: "When lockdown is not specified, determination step should be added with if condition",
5638
},
5739
{
5840
name: "No auto-determination when lockdown explicitly set to true",
@@ -63,7 +45,6 @@ tools:
6345
github:
6446
mode: local
6547
lockdown: true
66-
github-token: ${{ secrets.CUSTOM_TOKEN }}
6748
toolsets: [default]
6849
---
6950
@@ -73,6 +54,7 @@ Test with explicit lockdown enabled.
7354
`,
7455
expectedDetectStep: false,
7556
expectedLockdown: "true",
57+
expectIfCondition: false,
7658
description: "When lockdown is explicitly true, no determination step and lockdown should be hardcoded",
7759
},
7860
{
@@ -84,7 +66,6 @@ tools:
8466
github:
8567
mode: local
8668
lockdown: false
87-
github-token: ${{ secrets.CUSTOM_TOKEN }}
8869
toolsets: [default]
8970
---
9071
@@ -94,27 +75,28 @@ Test with explicit lockdown disabled.
9475
`,
9576
expectedDetectStep: false,
9677
expectedLockdown: "false",
78+
expectIfCondition: false,
9779
description: "When lockdown is explicitly false, no determination step and no lockdown setting",
9880
},
9981
{
100-
name: "Auto-determination with remote mode and custom token",
82+
name: "Auto-determination with remote mode",
10183
workflow: `---
10284
on: issues
10385
engine: copilot
10486
tools:
10587
github:
10688
mode: remote
107-
github-token: ${{ secrets.CUSTOM_TOKEN }}
10889
toolsets: [default]
10990
---
11091
11192
# Test Workflow
11293
113-
Test auto-determination with remote GitHub MCP and custom token.
94+
Test auto-determination with remote GitHub MCP.
11495
`,
11596
expectedDetectStep: true,
11697
expectedLockdown: "auto",
117-
description: "Auto-determination should work with remote mode when custom token is defined",
98+
expectIfCondition: true,
99+
description: "Auto-determination should work with remote mode",
118100
},
119101
}
120102

@@ -156,6 +138,13 @@ Test auto-determination with remote GitHub MCP and custom token.
156138
t.Errorf("%s: Detection step presence = %v, want %v", tt.description, detectStepPresent, tt.expectedDetectStep)
157139
}
158140

141+
// Check if the step has the if condition when expected
142+
if tt.expectIfCondition && detectStepPresent {
143+
if !strings.Contains(yaml, "if: secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN != ''") {
144+
t.Errorf("%s: Expected if condition for GH_AW_GITHUB_MCP_SERVER_TOKEN", tt.description)
145+
}
146+
}
147+
159148
// Check lockdown configuration based on expected value
160149
switch tt.expectedLockdown {
161150
case "auto":
@@ -187,13 +176,12 @@ engine: claude
187176
tools:
188177
github:
189178
mode: local
190-
github-token: ${{ secrets.CUSTOM_TOKEN }}
191179
toolsets: [default]
192180
---
193181
194182
# Test Workflow
195183
196-
Test automatic lockdown determination with Claude and custom token.
184+
Test automatic lockdown determination with Claude.
197185
`
198186

199187
// Create temporary directory for test
@@ -228,7 +216,12 @@ Test automatic lockdown determination with Claude and custom token.
228216
strings.Contains(yaml, "determine-automatic-lockdown")
229217

230218
if !detectStepPresent {
231-
t.Error("Determination step should be present for Claude engine with custom token")
219+
t.Error("Determination step should be present for Claude engine")
220+
}
221+
222+
// Check if the step has the if condition
223+
if !strings.Contains(yaml, "if: secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN != ''") {
224+
t.Error("Expected if condition for GH_AW_GITHUB_MCP_SERVER_TOKEN in determination step")
232225
}
233226

234227
// Check if lockdown uses step output expression

pkg/workflow/mcp_renderer.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,8 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github
4747
lockdown := getGitHubLockdown(githubTool)
4848

4949
// Check if automatic lockdown determination step will be generated
50-
// This requires: lockdown not explicitly set AND custom token configured
51-
customGitHubToken := getGitHubToken(githubTool)
52-
var toplevelToken string
53-
if workflowData != nil {
54-
toplevelToken = workflowData.GitHubToken
55-
}
56-
hasCustomToken := customGitHubToken != "" || toplevelToken != ""
57-
shouldUseStepOutput := !hasGitHubLockdownExplicitlySet(githubTool) && hasCustomToken
50+
// The step is always generated when lockdown is not explicitly set
51+
shouldUseStepOutput := !hasGitHubLockdownExplicitlySet(githubTool)
5852

5953
if shouldUseStepOutput {
6054
// Use the detected lockdown value from the step output
@@ -64,8 +58,8 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github
6458

6559
toolsets := getGitHubToolsets(githubTool)
6660

67-
mcpRendererLog.Printf("Rendering GitHub MCP: type=%s, read_only=%t, lockdown=%t (explicit=%t, custom_token=%t, use_step=%t), toolsets=%v, format=%s",
68-
githubType, readOnly, lockdown, hasGitHubLockdownExplicitlySet(githubTool), hasCustomToken, shouldUseStepOutput, toolsets, r.options.Format)
61+
mcpRendererLog.Printf("Rendering GitHub MCP: type=%s, read_only=%t, lockdown=%t (explicit=%t, use_step=%t), toolsets=%v, format=%s",
62+
githubType, readOnly, lockdown, hasGitHubLockdownExplicitlySet(githubTool), shouldUseStepOutput, toolsets, r.options.Format)
6963

7064
if r.options.Format == "toml" {
7165
r.renderGitHubTOML(yaml, githubTool, workflowData)

pkg/workflow/mcp_servers.go

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -771,11 +771,10 @@ func replaceExpressionsInPlaywrightArgs(args []string, expressions map[string]st
771771
}
772772

773773
// generateGitHubMCPLockdownDetectionStep generates a step to determine automatic lockdown mode
774-
// for GitHub MCP server based on repository visibility. This step is only added when:
774+
// for GitHub MCP server based on repository visibility. This step is added when:
775775
// - GitHub tool is enabled AND
776-
// - lockdown field is not explicitly specified in the workflow configuration AND
777-
// - A custom GitHub MCP server token is defined (GH_AW_GITHUB_MCP_SERVER_TOKEN exists) AND
778-
// - Repository is public
776+
// - lockdown field is not explicitly specified in the workflow configuration
777+
// The step includes a runtime condition that only executes if GH_AW_GITHUB_MCP_SERVER_TOKEN is defined
779778
func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder, data *WorkflowData) {
780779
// Check if GitHub tool is present
781780
githubTool, hasGitHub := data.Tools["github"]
@@ -789,19 +788,6 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder,
789788
return
790789
}
791790

792-
// Check if custom GitHub MCP server token is defined
793-
// The step only applies when GH_AW_GITHUB_MCP_SERVER_TOKEN is explicitly configured
794-
customGitHubToken := getGitHubToken(githubTool)
795-
toplevelToken := data.GitHubToken
796-
797-
// Determine if a custom token is being used (not the default fallback)
798-
hasCustomToken := customGitHubToken != "" || toplevelToken != ""
799-
800-
if !hasCustomToken {
801-
mcpServersLog.Print("No custom GitHub MCP server token defined, skipping automatic lockdown determination")
802-
return
803-
}
804-
805791
mcpServersLog.Print("Generating automatic lockdown determination step for GitHub MCP server")
806792

807793
// Resolve the latest version of actions/github-script
@@ -816,8 +802,10 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder,
816802
}
817803

818804
// Generate the step using the determine_automatic_lockdown.cjs action
805+
// The step only runs if GH_AW_GITHUB_MCP_SERVER_TOKEN secret is defined
819806
yaml.WriteString(" - name: Determine automatic lockdown mode for GitHub MCP server\n")
820807
yaml.WriteString(" id: determine-automatic-lockdown\n")
808+
yaml.WriteString(" if: secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN != ''\n")
821809
fmt.Fprintf(yaml, " uses: %s\n", pinnedAction)
822810
yaml.WriteString(" with:\n")
823811
yaml.WriteString(" script: |\n")

0 commit comments

Comments
 (0)