Skip to content

Commit adb344b

Browse files
Copilotpelikhangithub-actions[bot]
authored
Add unit tests for StripANSI in pkg/stringutil (#44277)
* Initial plan * Initial plan - no changes yet Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Add focused unit tests for StripANSI in pkg/stringutil/ansi_test.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: always generate determine-automatic-lockdown step when GitHub tool is enabled The step was incorrectly skipped when min-integrity or repos were explicitly configured in the github tool. This broke workflows at runtime because sink-visibility in safe-outputs and other MCP server guard policies still referenced steps.determine-automatic-lockdown.outputs.visibility even though the step was missing. The step serves two purposes: 1. Output repository visibility (always needed for sink-visibility) 2. Auto-configure guard policies when not explicitly set in the workflow Remove the skip condition so the step always runs when GitHub tool is enabled. The existing env-var passing logic already correctly passes GH_AW_GITHUB_MIN_INTEGRITY/GH_AW_GITHUB_REPOS when those values are explicitly configured, so the script will respect them and not override. Update tests to reflect the corrected behavior and recompile all workflows. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent 448f8ee commit adb344b

4 files changed

Lines changed: 187 additions & 17 deletions

File tree

‎pkg/stringutil/ansi_test.go‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
//go:build !integration
2+
3+
package stringutil
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestStripANSI(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
input string
15+
want string
16+
}{
17+
{
18+
name: "empty string",
19+
input: "",
20+
want: "",
21+
},
22+
{
23+
name: "plain text unchanged",
24+
input: "hello world",
25+
want: "hello world",
26+
},
27+
{
28+
name: "basic color code",
29+
input: "\x1b[31mred text\x1b[0m",
30+
want: "red text",
31+
},
32+
{
33+
name: "bold color code",
34+
input: "\x1b[1;32mBold Green\x1b[0m",
35+
want: "Bold Green",
36+
},
37+
{
38+
name: "reset code only",
39+
input: "text\x1b[mmore",
40+
want: "textmore",
41+
},
42+
{
43+
name: "multiple ANSI sequences",
44+
input: "\x1b[31mdoes important\x1b[0m things\x1b[m",
45+
want: "does important things",
46+
},
47+
{
48+
name: "description with embedded ANSI",
49+
input: "This workflow \x1b[31mdoes important\x1b[0m things\x1b[m",
50+
want: "This workflow does important things",
51+
},
52+
{
53+
name: "bold ANSI in description",
54+
input: "Workflow with \x1b[1mANSI\x1b[0m codes",
55+
want: "Workflow with ANSI codes",
56+
},
57+
{
58+
name: "file path with ANSI color",
59+
input: "path/to/\x1b[32mfile1.md\x1b[0m",
60+
want: "path/to/file1.md",
61+
},
62+
{
63+
name: "stop-time with ANSI codes",
64+
input: "2026-12-31\x1b[31mT23:59:59Z\x1b[0m",
65+
want: "2026-12-31T23:59:59Z",
66+
},
67+
{
68+
name: "environment name with ANSI bold",
69+
input: "production-\x1b[1menv\x1b[0m",
70+
want: "production-env",
71+
},
72+
{
73+
name: "OSC sequence with BEL terminator",
74+
input: "before\x1b]0;title\x07after",
75+
want: "beforeafter",
76+
},
77+
{
78+
name: "OSC sequence with ST terminator",
79+
input: "before\x1b]0;title\x1b\\after",
80+
want: "beforeafter",
81+
},
82+
{
83+
name: "G0 character set selection",
84+
input: "before\x1b(Bafter",
85+
want: "beforeafter",
86+
},
87+
{
88+
name: "G1 character set selection",
89+
input: "before\x1b)0after",
90+
want: "beforeafter",
91+
},
92+
{
93+
name: "application keypad mode",
94+
input: "before\x1b=after",
95+
want: "beforeafter",
96+
},
97+
{
98+
name: "normal keypad mode",
99+
input: "before\x1b>after",
100+
want: "beforeafter",
101+
},
102+
{
103+
name: "reset sequence",
104+
input: "before\x1bcafter",
105+
want: "beforeafter",
106+
},
107+
{
108+
name: "cursor save (two-char sequence)",
109+
input: "before\x1b7after",
110+
want: "beforeafter",
111+
},
112+
{
113+
name: "cursor restore (two-char sequence)",
114+
input: "before\x1b8after",
115+
want: "beforeafter",
116+
},
117+
{
118+
name: "ESC at end of string",
119+
input: "text\x1b",
120+
want: "text",
121+
},
122+
{
123+
name: "ESC with no following character stripped",
124+
input: "text\x1b[",
125+
want: "text",
126+
},
127+
{
128+
name: "multiline text with ANSI codes",
129+
input: "Line 1 with \x1b[32mgreen\x1b[0m text\nLine 2 with \x1b[31mred\x1b[0m text",
130+
want: "Line 1 with green text\nLine 2 with red text",
131+
},
132+
{
133+
name: "256-color foreground",
134+
input: "\x1b[38;5;196mred256\x1b[0m",
135+
want: "red256",
136+
},
137+
{
138+
name: "256-color background",
139+
input: "\x1b[48;5;21mblue256bg\x1b[0m",
140+
want: "blue256bg",
141+
},
142+
{
143+
name: "hyperlink OSC sequence",
144+
input: "\x1b]8;;https://example.com\x07click\x1b]8;;\x07",
145+
want: "click",
146+
},
147+
{
148+
name: "no ESC but brackets preserved",
149+
input: "array[0] and [key]",
150+
want: "array[0] and [key]",
151+
},
152+
{
153+
name: "source path with ANSI",
154+
input: "\x1b[33mowner/repo\x1b[0m@v1.2.3",
155+
want: "owner/repo@v1.2.3",
156+
},
157+
}
158+
159+
for _, tt := range tests {
160+
t.Run(tt.name, func(t *testing.T) {
161+
got := StripANSI(tt.input)
162+
assert.Equal(t, tt.want, got)
163+
})
164+
}
165+
}

‎pkg/workflow/compiler_github_mcp_steps.go‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,12 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder,
2828
return
2929
}
3030

31-
// Skip the detection step when guard policies are already explicitly configured.
32-
// Explicit guard policies mean the compiler already knows the min-integrity and repos values,
33-
// so there is nothing for the runtime detection script to determine.
34-
if toolConfig, ok := githubTool.(map[string]any); ok {
35-
if len(getGitHubGuardPolicies(toolConfig)) > 0 {
36-
githubConfigLog.Print("Skipping GitHub MCP lockdown detection step: guard policy explicitly configured")
37-
return
38-
}
39-
}
40-
31+
// NOTE: Do NOT skip this step when guard policies are explicitly configured.
32+
// Even when min-integrity/repos are hardcoded, the step must still run to output
33+
// the repository visibility via steps.determine-automatic-lockdown.outputs.visibility,
34+
// which is referenced as sink-visibility in safe-outputs and other MCP server guard
35+
// policies. Removing the step while leaving those references in place breaks workflows
36+
// at runtime with undefined step output errors.
4137
githubConfigLog.Print("Generating automatic guard policy determination step for GitHub MCP server")
4238

4339
// Resolve the latest version of actions/github-script

‎pkg/workflow/compiler_github_mcp_steps_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestQuoteYAMLEnvValue(t *testing.T) {
3939
assert.Equal(t, `'["a","b"]'`, quoteYAMLEnvValue(`["a","b"]`))
4040
}
4141

42-
func TestGenerateGitHubMCPLockdownDetectionStepSkipsWhenGuardPolicyExplicit(t *testing.T) {
42+
func TestGenerateGitHubMCPLockdownDetectionStepGeneratedWithExplicitGuardPolicy(t *testing.T) {
4343
t.Parallel()
4444

4545
var yaml strings.Builder
@@ -55,8 +55,14 @@ func TestGenerateGitHubMCPLockdownDetectionStepSkipsWhenGuardPolicyExplicit(t *t
5555
NewCompiler().generateGitHubMCPLockdownDetectionStep(&yaml, data)
5656
output := yaml.String()
5757

58-
// When guard policies are explicitly configured, the detection step must not be generated.
59-
assert.Empty(t, output, "detection step should be skipped when guard policy is explicitly configured")
58+
// The detection step must still be generated even when guard policies are explicitly
59+
// configured because it outputs repository visibility used by sink-visibility in
60+
// safe-outputs and other MCP server guard policies.
61+
assert.Contains(t, output, "determine-automatic-lockdown", "detection step should be generated even when guard policy is explicitly configured")
62+
// The configured min-integrity and repos values must be passed as env vars to the step
63+
// so the script can respect them and avoid overriding explicit config.
64+
assert.Contains(t, output, "GH_AW_GITHUB_MIN_INTEGRITY", "env var should be present when min-integrity is explicitly set")
65+
assert.Contains(t, output, "GH_AW_GITHUB_REPOS", "env var should be present when allowed-repos is explicitly set")
6066
}
6167

6268
func TestGenerateGitHubMCPLockdownDetectionStepGeneratedWhenNoGuardPolicy(t *testing.T) {

‎pkg/workflow/github_lockdown_autodetect_test.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ Test with explicit lockdown enabled.
5959
description: "When lockdown is explicitly true but no guard policy, auto detection step should still run",
6060
},
6161
{
62-
name: "No auto detection when guard policy explicitly configured",
62+
name: "Detection step still generated when guard policy explicitly configured",
6363
workflow: `---
6464
on: issues
6565
engine: copilot
@@ -75,9 +75,12 @@ tools:
7575
7676
Test with explicit guard policy configured.
7777
`,
78-
expectedGuardPolicy: "static",
79-
expectAutoDetectionStep: false,
80-
description: "When guard policy is explicitly configured, no auto detection step",
78+
expectedGuardPolicy: "static",
79+
// The detection step must still be generated even when guard policies are explicit
80+
// because it outputs repository visibility used by sink-visibility in safe-outputs
81+
// and other MCP server guard policies. Removing the step breaks those references.
82+
expectAutoDetectionStep: true,
83+
description: "When guard policy is explicitly configured, detection step still generated for visibility output",
8184
},
8285
{
8386
name: "Automatic detection with remote mode when not specified",

0 commit comments

Comments
 (0)