Skip to content

Commit 489f96a

Browse files
Copilotpelikhan
Copilot
and
pelikhan
committed
Rename detect-repo-visibility to determine-automatic-lockdown with custom token requirement
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent 0db7e40 commit 489f96a

5 files changed

Lines changed: 114 additions & 60 deletions

File tree

actions/setup/js/detect_repo_visibility.cjs renamed to actions/setup/js/determine_automatic_lockdown.cjs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
/// <reference types="@actions/github-script" />
33

44
/**
5-
* Detects repository visibility and sets lockdown mode for GitHub MCP server.
5+
* Determines automatic lockdown mode for GitHub MCP server based on repository visibility.
6+
*
7+
* This function only applies when a custom GitHub MCP server token is defined
8+
* (GH_AW_GITHUB_MCP_SERVER_TOKEN) and for public repositories.
69
*
710
* For public repositories, lockdown mode should be enabled (true) to prevent
811
* the GitHub token from accessing private repositories, which could leak
@@ -16,12 +19,12 @@
1619
* @param {any} core - GitHub Actions core library
1720
* @returns {Promise<void>}
1821
*/
19-
async function detectRepoVisibility(github, context, core) {
22+
async function determineAutomaticLockdown(github, context, core) {
2023
try {
21-
core.info("Detecting repository visibility for GitHub MCP lockdown configuration");
24+
core.info("Determining automatic lockdown mode for GitHub MCP server");
2225

2326
const { owner, repo } = context.repo;
24-
core.info(`Checking visibility for repository: ${owner}/${repo}`);
27+
core.info(`Checking repository: ${owner}/${repo}`);
2528

2629
// Fetch repository information
2730
const { data: repository } = await github.rest.repos.get({
@@ -39,21 +42,24 @@ async function detectRepoVisibility(github, context, core) {
3942
// Public repos should have lockdown enabled to prevent token from accessing private repos
4043
const shouldLockdown = !isPrivate;
4144

42-
core.info(`Setting GitHub MCP lockdown: ${shouldLockdown}`);
45+
core.info(`Automatic lockdown mode determined: ${shouldLockdown}`);
4346
core.setOutput("lockdown", shouldLockdown.toString());
4447
core.setOutput("visibility", visibility);
4548

4649
if (shouldLockdown) {
50+
core.info("Automatic lockdown mode enabled for public repository");
4751
core.warning("GitHub MCP lockdown mode enabled for public repository. " + "This prevents the GitHub token from accessing private repositories.");
52+
} else {
53+
core.info("Automatic lockdown mode disabled for private/internal repository");
4854
}
4955
} catch (error) {
5056
const errorMessage = error instanceof Error ? error.message : String(error);
51-
core.error(`Failed to detect repository visibility: ${errorMessage}`);
57+
core.error(`Failed to determine automatic lockdown mode: ${errorMessage}`);
5258
// Default to lockdown mode for safety
5359
core.setOutput("lockdown", "true");
5460
core.setOutput("visibility", "unknown");
55-
core.warning("Failed to detect repository visibility. Defaulting to lockdown mode for security.");
61+
core.warning("Failed to determine repository visibility. Defaulting to lockdown mode for security.");
5662
}
5763
}
5864

59-
module.exports = detectRepoVisibility;
65+
module.exports = determineAutomaticLockdown;

actions/setup/js/detect_repo_visibility.test.cjs renamed to actions/setup/js/determine_automatic_lockdown.test.cjs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { describe, it, expect, beforeEach, vi } from "vitest";
22

3-
describe("detect_repo_visibility", () => {
3+
describe("determine_automatic_lockdown", () => {
44
let mockContext;
55
let mockGithub;
66
let mockCore;
7-
let detectRepoVisibility;
7+
let determineAutomaticLockdown;
88

99
beforeEach(async () => {
1010
vi.resetModules();
@@ -35,7 +35,7 @@ describe("detect_repo_visibility", () => {
3535
};
3636

3737
// Import the module
38-
detectRepoVisibility = (await import("./detect_repo_visibility.cjs")).default;
38+
determineAutomaticLockdown = (await import("./determine_automatic_lockdown.cjs")).default;
3939
});
4040

4141
it("should set lockdown to true for public repository", async () => {
@@ -46,7 +46,7 @@ describe("detect_repo_visibility", () => {
4646
},
4747
});
4848

49-
await detectRepoVisibility(mockGithub, mockContext, mockCore);
49+
await determineAutomaticLockdown(mockGithub, mockContext, mockCore);
5050

5151
expect(mockGithub.rest.repos.get).toHaveBeenCalledWith({
5252
owner: "test-owner",
@@ -65,7 +65,7 @@ describe("detect_repo_visibility", () => {
6565
},
6666
});
6767

68-
await detectRepoVisibility(mockGithub, mockContext, mockCore);
68+
await determineAutomaticLockdown(mockGithub, mockContext, mockCore);
6969

7070
expect(mockGithub.rest.repos.get).toHaveBeenCalledWith({
7171
owner: "test-owner",
@@ -84,7 +84,7 @@ describe("detect_repo_visibility", () => {
8484
},
8585
});
8686

87-
await detectRepoVisibility(mockGithub, mockContext, mockCore);
87+
await determineAutomaticLockdown(mockGithub, mockContext, mockCore);
8888

8989
expect(mockCore.setOutput).toHaveBeenCalledWith("lockdown", "false");
9090
expect(mockCore.setOutput).toHaveBeenCalledWith("visibility", "internal");
@@ -94,12 +94,12 @@ describe("detect_repo_visibility", () => {
9494
const error = new Error("API request failed");
9595
mockGithub.rest.repos.get.mockRejectedValue(error);
9696

97-
await detectRepoVisibility(mockGithub, mockContext, mockCore);
97+
await determineAutomaticLockdown(mockGithub, mockContext, mockCore);
9898

99-
expect(mockCore.error).toHaveBeenCalledWith("Failed to detect repository visibility: API request failed");
99+
expect(mockCore.error).toHaveBeenCalledWith("Failed to determine automatic lockdown mode: API request failed");
100100
expect(mockCore.setOutput).toHaveBeenCalledWith("lockdown", "true");
101101
expect(mockCore.setOutput).toHaveBeenCalledWith("visibility", "unknown");
102-
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to detect repository visibility"));
102+
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to determine repository visibility"));
103103
});
104104

105105
it("should infer visibility from private field when visibility field is missing", async () => {
@@ -110,7 +110,7 @@ describe("detect_repo_visibility", () => {
110110
},
111111
});
112112

113-
await detectRepoVisibility(mockGithub, mockContext, mockCore);
113+
await determineAutomaticLockdown(mockGithub, mockContext, mockCore);
114114

115115
expect(mockCore.setOutput).toHaveBeenCalledWith("lockdown", "true");
116116
expect(mockCore.setOutput).toHaveBeenCalledWith("visibility", "public");
@@ -124,12 +124,13 @@ describe("detect_repo_visibility", () => {
124124
},
125125
});
126126

127-
await detectRepoVisibility(mockGithub, mockContext, mockCore);
127+
await determineAutomaticLockdown(mockGithub, mockContext, mockCore);
128128

129-
expect(mockCore.info).toHaveBeenCalledWith("Detecting repository visibility for GitHub MCP lockdown configuration");
130-
expect(mockCore.info).toHaveBeenCalledWith("Checking visibility for repository: test-owner/test-repo");
129+
expect(mockCore.info).toHaveBeenCalledWith("Determining automatic lockdown mode for GitHub MCP server");
130+
expect(mockCore.info).toHaveBeenCalledWith("Checking repository: test-owner/test-repo");
131131
expect(mockCore.info).toHaveBeenCalledWith("Repository visibility: public");
132132
expect(mockCore.info).toHaveBeenCalledWith("Repository is private: false");
133-
expect(mockCore.info).toHaveBeenCalledWith("Setting GitHub MCP lockdown: true");
133+
expect(mockCore.info).toHaveBeenCalledWith("Automatic lockdown mode determined: true");
134+
expect(mockCore.info).toHaveBeenCalledWith("Automatic lockdown mode enabled for public repository");
134135
});
135136
});

‎pkg/workflow/github_lockdown_autodetect_test.go‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,54 @@ func TestGitHubLockdownAutodetection(t *testing.T) {
1616
description string
1717
}{
1818
{
19-
name: "Auto-detection enabled when lockdown not specified",
19+
name: "Auto-determination enabled when lockdown not specified and custom token defined",
2020
workflow: `---
2121
on: issues
2222
engine: copilot
2323
tools:
2424
github:
2525
mode: local
26+
github-token: ${{ secrets.CUSTOM_TOKEN }}
2627
toolsets: [default]
2728
---
2829
2930
# Test Workflow
3031
31-
Test automatic lockdown detection.
32+
Test automatic lockdown determination with custom token.
3233
`,
3334
expectedDetectStep: true,
3435
expectedLockdown: "auto",
35-
description: "When lockdown is not specified, detection step should be added and lockdown should use step output",
36+
description: "When lockdown is not specified and custom token is defined, determination step should be added",
3637
},
3738
{
38-
name: "No auto-detection when lockdown explicitly set to true",
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",
56+
},
57+
{
58+
name: "No auto-determination when lockdown explicitly set to true",
3959
workflow: `---
4060
on: issues
4161
engine: copilot
4262
tools:
4363
github:
4464
mode: local
4565
lockdown: true
66+
github-token: ${{ secrets.CUSTOM_TOKEN }}
4667
toolsets: [default]
4768
---
4869
@@ -52,17 +73,18 @@ Test with explicit lockdown enabled.
5273
`,
5374
expectedDetectStep: false,
5475
expectedLockdown: "true",
55-
description: "When lockdown is explicitly true, no detection step and lockdown should be hardcoded",
76+
description: "When lockdown is explicitly true, no determination step and lockdown should be hardcoded",
5677
},
5778
{
58-
name: "No auto-detection when lockdown explicitly set to false",
79+
name: "No auto-determination when lockdown explicitly set to false",
5980
workflow: `---
6081
on: issues
6182
engine: copilot
6283
tools:
6384
github:
6485
mode: local
6586
lockdown: false
87+
github-token: ${{ secrets.CUSTOM_TOKEN }}
6688
toolsets: [default]
6789
---
6890
@@ -72,26 +94,27 @@ Test with explicit lockdown disabled.
7294
`,
7395
expectedDetectStep: false,
7496
expectedLockdown: "false",
75-
description: "When lockdown is explicitly false, no detection step and no lockdown setting",
97+
description: "When lockdown is explicitly false, no determination step and no lockdown setting",
7698
},
7799
{
78-
name: "Auto-detection with remote mode",
100+
name: "Auto-determination with remote mode and custom token",
79101
workflow: `---
80102
on: issues
81103
engine: copilot
82104
tools:
83105
github:
84106
mode: remote
107+
github-token: ${{ secrets.CUSTOM_TOKEN }}
85108
toolsets: [default]
86109
---
87110
88111
# Test Workflow
89112
90-
Test auto-detection with remote GitHub MCP.
113+
Test auto-determination with remote GitHub MCP and custom token.
91114
`,
92115
expectedDetectStep: true,
93116
expectedLockdown: "auto",
94-
description: "Auto-detection should work with remote mode too",
117+
description: "Auto-determination should work with remote mode when custom token is defined",
95118
},
96119
}
97120

@@ -125,9 +148,9 @@ Test auto-detection with remote GitHub MCP.
125148
yaml := string(lockContent)
126149

127150
// Check if detection step is present
128-
detectStepPresent := strings.Contains(yaml, "Detect repository visibility for GitHub MCP lockdown") &&
129-
strings.Contains(yaml, "detect-repo-visibility") &&
130-
strings.Contains(yaml, "detect_repo_visibility.cjs")
151+
detectStepPresent := strings.Contains(yaml, "Determine automatic lockdown mode for GitHub MCP server") &&
152+
strings.Contains(yaml, "determine-automatic-lockdown") &&
153+
strings.Contains(yaml, "determine_automatic_lockdown.cjs")
131154

132155
if detectStepPresent != tt.expectedDetectStep {
133156
t.Errorf("%s: Detection step presence = %v, want %v", tt.description, detectStepPresent, tt.expectedDetectStep)
@@ -137,7 +160,7 @@ Test auto-detection with remote GitHub MCP.
137160
switch tt.expectedLockdown {
138161
case "auto":
139162
// Should use step output expression
140-
if !strings.Contains(yaml, "steps.detect-repo-visibility.outputs.lockdown") {
163+
if !strings.Contains(yaml, "steps.determine-automatic-lockdown.outputs.lockdown") {
141164
t.Errorf("%s: Expected lockdown to use step output expression", tt.description)
142165
}
143166
case "true":
@@ -164,12 +187,13 @@ engine: claude
164187
tools:
165188
github:
166189
mode: local
190+
github-token: ${{ secrets.CUSTOM_TOKEN }}
167191
toolsets: [default]
168192
---
169193
170194
# Test Workflow
171195
172-
Test automatic lockdown detection with Claude.
196+
Test automatic lockdown determination with Claude and custom token.
173197
`
174198

175199
// Create temporary directory for test
@@ -200,15 +224,15 @@ Test automatic lockdown detection with Claude.
200224
yaml := string(lockContent)
201225

202226
// Check if detection step is present
203-
detectStepPresent := strings.Contains(yaml, "Detect repository visibility for GitHub MCP lockdown") &&
204-
strings.Contains(yaml, "detect-repo-visibility")
227+
detectStepPresent := strings.Contains(yaml, "Determine automatic lockdown mode for GitHub MCP server") &&
228+
strings.Contains(yaml, "determine-automatic-lockdown")
205229

206230
if !detectStepPresent {
207-
t.Error("Detection step should be present for Claude engine")
231+
t.Error("Determination step should be present for Claude engine with custom token")
208232
}
209233

210234
// Check if lockdown uses step output expression
211-
if !strings.Contains(yaml, "steps.detect-repo-visibility.outputs.lockdown") {
235+
if !strings.Contains(yaml, "steps.determine-automatic-lockdown.outputs.lockdown") {
212236
t.Error("Expected lockdown to use step output expression for Claude engine")
213237
}
214238
}

‎pkg/workflow/mcp_renderer.go‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,24 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github
4545

4646
// Get lockdown value - use detected value if lockdown wasn't explicitly set
4747
lockdown := getGitHubLockdown(githubTool)
48-
if !hasGitHubLockdownExplicitlySet(githubTool) {
48+
49+
// 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+
toplevelToken := workflowData.GitHubToken
53+
hasCustomToken := customGitHubToken != "" || toplevelToken != ""
54+
shouldUseStepOutput := !hasGitHubLockdownExplicitlySet(githubTool) && hasCustomToken
55+
56+
if shouldUseStepOutput {
4957
// Use the detected lockdown value from the step output
5058
// This will be evaluated at runtime based on repository visibility
5159
lockdown = true // This is a placeholder - actual value comes from step output
5260
}
5361

5462
toolsets := getGitHubToolsets(githubTool)
5563

56-
mcpRendererLog.Printf("Rendering GitHub MCP: type=%s, read_only=%t, lockdown=%t (explicit=%t), toolsets=%v, format=%s",
57-
githubType, readOnly, lockdown, hasGitHubLockdownExplicitlySet(githubTool), toolsets, r.options.Format)
64+
mcpRendererLog.Printf("Rendering GitHub MCP: type=%s, read_only=%t, lockdown=%t (explicit=%t, custom_token=%t, use_step=%t), toolsets=%v, format=%s",
65+
githubType, readOnly, lockdown, hasGitHubLockdownExplicitlySet(githubTool), hasCustomToken, shouldUseStepOutput, toolsets, r.options.Format)
5866

5967
if r.options.Format == "toml" {
6068
r.renderGitHubTOML(yaml, githubTool, workflowData)
@@ -76,7 +84,7 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github
7684
RenderGitHubMCPRemoteConfig(yaml, GitHubMCPRemoteOptions{
7785
ReadOnly: readOnly,
7886
Lockdown: lockdown,
79-
LockdownFromStep: !hasGitHubLockdownExplicitlySet(githubTool),
87+
LockdownFromStep: shouldUseStepOutput,
8088
Toolsets: toolsets,
8189
AuthorizationValue: authValue,
8290
IncludeToolsField: r.options.IncludeCopilotFields,
@@ -91,7 +99,7 @@ func (r *MCPConfigRendererUnified) RenderGitHubMCP(yaml *strings.Builder, github
9199
RenderGitHubMCPDockerConfig(yaml, GitHubMCPDockerOptions{
92100
ReadOnly: readOnly,
93101
Lockdown: lockdown,
94-
LockdownFromStep: !hasGitHubLockdownExplicitlySet(githubTool),
102+
LockdownFromStep: shouldUseStepOutput,
95103
Toolsets: toolsets,
96104
DockerImageVersion: githubDockerImageVersion,
97105
CustomArgs: customArgs,
@@ -481,9 +489,9 @@ func RenderGitHubMCPDockerConfig(yaml *strings.Builder, options GitHubMCPDockerO
481489
}
482490

483491
if options.LockdownFromStep {
484-
// Use lockdown value from step output (detected based on repository visibility)
492+
// Use lockdown value from step output (determined based on repository visibility)
485493
yaml.WriteString(" \"-e\",\n")
486-
yaml.WriteString(" \"GITHUB_LOCKDOWN_MODE=${{ steps.detect-repo-visibility.outputs.lockdown == 'true' && '1' || '0' }}\",\n")
494+
yaml.WriteString(" \"GITHUB_LOCKDOWN_MODE=${{ steps.determine-automatic-lockdown.outputs.lockdown == 'true' && '1' || '0' }}\",\n")
487495
} else if options.Lockdown {
488496
// Use explicit lockdown value from configuration
489497
yaml.WriteString(" \"-e\",\n")
@@ -579,8 +587,8 @@ func RenderGitHubMCPRemoteConfig(yaml *strings.Builder, options GitHubMCPRemoteO
579587

580588
// Add X-MCP-Lockdown header if lockdown mode is enabled
581589
if options.LockdownFromStep {
582-
// Use lockdown value from step output (detected based on repository visibility)
583-
headers["X-MCP-Lockdown"] = "${{ steps.detect-repo-visibility.outputs.lockdown }}"
590+
// Use lockdown value from step output (determined based on repository visibility)
591+
headers["X-MCP-Lockdown"] = "${{ steps.determine-automatic-lockdown.outputs.lockdown }}"
584592
} else if options.Lockdown {
585593
// Use explicit lockdown value from configuration
586594
headers["X-MCP-Lockdown"] = "true"

0 commit comments

Comments
 (0)