-
Notifications
You must be signed in to change notification settings - Fork 344
fix: always emit CLI_PROXY_POLICY env var for CLI proxy #25419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -693,3 +693,68 @@ func TestGenerateSetGHRepoAfterDIFCProxyStep(t *testing.T) { | |
| assert.NotContains(t, result, "GH_HOST", "should not touch GH_HOST") | ||
| }) | ||
| } | ||
|
|
||
| // TestBuildStartCliProxyStepYAML verifies that the CLI proxy step always emits | ||
| // CLI_PROXY_POLICY, using the default permissive policy when no guard policy is | ||
| // configured in the frontmatter. | ||
| func TestBuildStartCliProxyStepYAML(t *testing.T) { | ||
| c := &Compiler{} | ||
|
|
||
| t.Run("emits default policy when no guard policy is configured", func(t *testing.T) { | ||
| data := &WorkflowData{ | ||
| Tools: map[string]any{ | ||
| "github": map[string]any{"toolsets": []string{"default"}}, | ||
| }, | ||
| } | ||
|
|
||
| result := c.buildStartCliProxyStepYAML(data) | ||
| require.NotEmpty(t, result, "should emit CLI proxy step even without guard policy") | ||
| assert.Contains(t, result, "CLI_PROXY_POLICY", "should always emit CLI_PROXY_POLICY") | ||
| assert.Contains(t, result, `"allow-only"`, "default policy should contain allow-only") | ||
| assert.Contains(t, result, `"repos":"all"`, "default policy should allow all repos") | ||
| assert.Contains(t, result, `"min-integrity":"none"`, "default policy should have min-integrity none") | ||
| }) | ||
|
Comment on lines
+710
to
+716
|
||
|
|
||
| t.Run("emits default policy when github tool is nil", func(t *testing.T) { | ||
| data := &WorkflowData{ | ||
| Tools: map[string]any{}, | ||
| } | ||
|
|
||
| result := c.buildStartCliProxyStepYAML(data) | ||
| require.NotEmpty(t, result, "should emit CLI proxy step even without github tool") | ||
| assert.Contains(t, result, "CLI_PROXY_POLICY", "should always emit CLI_PROXY_POLICY") | ||
| assert.Contains(t, result, `"min-integrity":"none"`, "should use default min-integrity") | ||
| }) | ||
|
|
||
| t.Run("uses configured guard policy when present", func(t *testing.T) { | ||
| data := &WorkflowData{ | ||
| Tools: map[string]any{ | ||
| "github": map[string]any{ | ||
| "min-integrity": "approved", | ||
| "allowed-repos": "owner/*", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| result := c.buildStartCliProxyStepYAML(data) | ||
| require.NotEmpty(t, result, "should emit CLI proxy step") | ||
| assert.Contains(t, result, "CLI_PROXY_POLICY", "should emit CLI_PROXY_POLICY") | ||
| assert.Contains(t, result, `"min-integrity":"approved"`, "should use configured min-integrity") | ||
| assert.Contains(t, result, `"repos":"owner/*"`, "should use configured repos") | ||
| }) | ||
|
|
||
| t.Run("emits correct step structure", func(t *testing.T) { | ||
| data := &WorkflowData{ | ||
| Tools: map[string]any{ | ||
| "github": map[string]any{"toolsets": []string{"default"}}, | ||
| }, | ||
| } | ||
|
|
||
| result := c.buildStartCliProxyStepYAML(data) | ||
| assert.Contains(t, result, "name: Start CLI proxy", "should have correct step name") | ||
| assert.Contains(t, result, "GH_TOKEN:", "should include GH_TOKEN") | ||
| assert.Contains(t, result, "GITHUB_SERVER_URL:", "should include GITHUB_SERVER_URL") | ||
| assert.Contains(t, result, "CLI_PROXY_IMAGE:", "should include CLI_PROXY_IMAGE") | ||
| assert.Contains(t, result, "start_cli_proxy.sh", "should reference the start script") | ||
| }) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultCliProxyPolicyJSONis defined with backslash-escaped quotes inside a raw string literal, producing{\"allow-only\":...}at runtime. That string is not valid JSON and will be passed verbatim tostart_cli_proxy.sh→--policy, likely causing the proxy to reject the policy. Define the constant as actual JSON (no backslashes) or build it viajson.Marshalto match the format returned bygetDIFCProxyPolicyJSONand the PR description example.