Skip to content

Commit 52cd33e

Browse files
authored
[test] Add tests for proxy.initGuardPolicy (#4333)
# Test Coverage Improvement: `proxy.initGuardPolicy` ## Function Analyzed - **Package**: `internal/proxy` - **Function**: `(*Server).initGuardPolicy` - **File**: `internal/proxy/proxy.go` (lines 150–213) - **Previous Coverage**: 0% (no direct tests existed) - **Complexity**: High — 6 distinct branches, DIFC enforcement, agent label application ## Why This Function? `initGuardPolicy` is the core method that connects a proxy `Server` to its WASM guard, sets up agent DIFC labels (secrecy/integrity), and optionally overrides the enforcement mode. Despite its criticality to the proxy security model, it had **zero direct unit tests** — it was only exercised indirectly through integration tests that require a real WASM binary. The function is non-trivial: - Parses and validates guard policy JSON - Rejects write-sink policies (proxy only supports allow-only) - Normalizes legacy `allowonly` key → `allow-only` before trusted-user injection - Calls `LabelAgent` on the guard and applies returned secrecy/integrity tags to the agent registry - Overrides the server's enforcement mode when the guard's response specifies one ## Tests Added A new test file `internal/proxy/init_guard_policy_test.go` (264 lines) covers: - ✅ **Invalid JSON** — `"not-valid-json"` rejected with "invalid policy JSON" error - ✅ **Validation failure** — empty `{}` rejected with "policy validation failed" error - ✅ **Write-sink rejected** — `{"write-sink":{...}}` rejected with "write-sink policies are not supported" - ✅ **LabelAgent error** — guard error propagates with "LabelAgent failed" wrapping - ✅ **Happy path** — successful initialization sets `guardInitialized = true` - ✅ **Agent label application** — secrecy and integrity tags from guard response applied to proxy agent registry - ✅ **DIFCMode override** — guard returning "strict" overrides server started in "filter" mode - ✅ **Invalid DIFCMode silently ignored** — unrecognized mode preserves original enforcement mode - ✅ **Empty DIFCMode** — skips mode-override block, initial mode preserved - ✅ **Legacy `allowonly` key** — accepted via policy-map normalization - ✅ **Legacy key + trusted users** — normalization enables trusted-user injection - ✅ **Trusted bots + users** — forwarded alongside policy without error - ✅ **Multiple secrecy tags** — all tags applied to agent registry ## Test Infrastructure Added a `labelAgentStubGuard` test double (implements `guard.Guard`) with configurable `LabelAgent` return values, alongside a `newTestServerForInitGuardPolicy` helper that assembles a minimal `*Server`. This pattern follows the existing `stubGuard` in `handler_difc_test.go`. ## Coverage Improvement ``` Before: 0% — initGuardPolicy had no direct tests After: ~100% branch coverage for initGuardPolicy ``` --- *Generated by Test Coverage Improver* *Next run will target the next most complex under-tested function* > [!WARNING] > **⚠️ Firewall blocked 1 domain** > > The following domain was blocked by the firewall during workflow execution: > > - `proxy.golang.org` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "proxy.golang.org" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > Generated by [Test Coverage Improver](https://github.com/github/gh-aw-mcpg/actions/runs/24776780413/agentic_workflow) · ● 24.1M · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+test-coverage-improver%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Test Coverage Improver, engine: copilot, model: auto, id: 24776780413, workflow_id: test-coverage-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/24776780413 --> <!-- gh-aw-workflow-id: test-coverage-improver -->
2 parents 1e210cc + 6d4d102 commit 52cd33e

1 file changed

Lines changed: 264 additions & 0 deletions

File tree

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
package proxy
2+
3+
import (
4+
"context"
5+
"errors"
6+
"net/http"
7+
"testing"
8+
9+
"github.com/github/gh-aw-mcpg/internal/difc"
10+
"github.com/github/gh-aw-mcpg/internal/guard"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// labelAgentStubGuard is a configurable test double for guard.Guard that supports
16+
// controlling the result of LabelAgent. Used specifically for initGuardPolicy tests.
17+
type labelAgentStubGuard struct {
18+
labelAgentResult *guard.LabelAgentResult
19+
labelAgentErr error
20+
}
21+
22+
func (g *labelAgentStubGuard) Name() string { return "stub-label-agent" }
23+
24+
func (g *labelAgentStubGuard) LabelAgent(_ context.Context, _ interface{}, _ guard.BackendCaller, _ *difc.Capabilities) (*guard.LabelAgentResult, error) {
25+
return g.labelAgentResult, g.labelAgentErr
26+
}
27+
28+
func (g *labelAgentStubGuard) LabelResource(_ context.Context, _ string, _ interface{}, _ guard.BackendCaller, _ *difc.Capabilities) (*difc.LabeledResource, difc.OperationType, error) {
29+
return difc.NewLabeledResource("stub"), difc.OperationRead, nil
30+
}
31+
32+
func (g *labelAgentStubGuard) LabelResponse(_ context.Context, _ string, _ interface{}, _ guard.BackendCaller, _ *difc.Capabilities) (difc.LabeledData, error) {
33+
return nil, nil
34+
}
35+
36+
// defaultLabelAgentStub returns a stub guard that succeeds with the given DIFC mode and agent labels.
37+
func defaultLabelAgentStub(difcMode string, secrecy, integrity []string) *labelAgentStubGuard {
38+
return &labelAgentStubGuard{
39+
labelAgentResult: &guard.LabelAgentResult{
40+
Agent: guard.AgentLabelsPayload{
41+
Secrecy: secrecy,
42+
Integrity: integrity,
43+
},
44+
DIFCMode: difcMode,
45+
},
46+
}
47+
}
48+
49+
// newTestServerForInitGuardPolicy creates a minimal proxy.Server for testing initGuardPolicy.
50+
func newTestServerForInitGuardPolicy(g guard.Guard, mode difc.EnforcementMode) *Server {
51+
return &Server{
52+
guard: g,
53+
evaluator: difc.NewEvaluatorWithMode(mode),
54+
agentRegistry: difc.NewAgentRegistryWithDefaults(nil, nil),
55+
capabilities: difc.NewCapabilities(),
56+
githubAPIURL: "https://api.github.com",
57+
httpClient: &http.Client{},
58+
enforcementMode: mode,
59+
}
60+
}
61+
62+
// validAllowOnlyPolicyJSON is a minimal valid allow-only guard policy.
63+
const validAllowOnlyPolicyJSON = `{"allow-only":{"repos":"public","min-integrity":"none"}}`
64+
65+
// validWriteSinkPolicyJSON is a valid write-sink guard policy.
66+
const validWriteSinkPolicyJSON = `{"write-sink":{"accept":["*"]}}`
67+
68+
// TestInitGuardPolicy_InvalidJSON verifies that non-JSON input is rejected immediately.
69+
func TestInitGuardPolicy_InvalidJSON(t *testing.T) {
70+
g := defaultLabelAgentStub(difc.ModeFilter, nil, nil)
71+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
72+
73+
err := s.initGuardPolicy(context.Background(), "not-valid-json", nil, nil)
74+
75+
require.Error(t, err)
76+
assert.Contains(t, err.Error(), "invalid policy JSON")
77+
assert.False(t, s.guardInitialized, "guardInitialized must stay false on error")
78+
}
79+
80+
// TestInitGuardPolicy_ValidationFailure verifies that a structurally valid JSON object
81+
// that does not constitute a valid guard policy is rejected.
82+
func TestInitGuardPolicy_ValidationFailure(t *testing.T) {
83+
g := defaultLabelAgentStub(difc.ModeFilter, nil, nil)
84+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
85+
86+
// An empty object has neither allow-only nor write-sink.
87+
err := s.initGuardPolicy(context.Background(), `{}`, nil, nil)
88+
89+
require.Error(t, err)
90+
assert.Contains(t, err.Error(), "policy validation failed")
91+
assert.False(t, s.guardInitialized)
92+
}
93+
94+
// TestInitGuardPolicy_WriteSinkRejected verifies that a write-sink policy is rejected because
95+
// the proxy only accepts allow-only policies during guard initialization.
96+
func TestInitGuardPolicy_WriteSinkRejected(t *testing.T) {
97+
g := defaultLabelAgentStub(difc.ModeFilter, nil, nil)
98+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
99+
100+
err := s.initGuardPolicy(context.Background(), validWriteSinkPolicyJSON, nil, nil)
101+
102+
require.Error(t, err)
103+
assert.Contains(t, err.Error(), "write-sink policies are not supported")
104+
assert.False(t, s.guardInitialized)
105+
}
106+
107+
// TestInitGuardPolicy_LabelAgentError verifies that an error from the guard's LabelAgent
108+
// call propagates correctly and leaves the server uninitialized.
109+
func TestInitGuardPolicy_LabelAgentError(t *testing.T) {
110+
g := &labelAgentStubGuard{
111+
labelAgentErr: errors.New("guard: wasm runtime error"),
112+
}
113+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
114+
115+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, nil, nil)
116+
117+
require.Error(t, err)
118+
assert.Contains(t, err.Error(), "LabelAgent failed")
119+
assert.Contains(t, err.Error(), "guard: wasm runtime error")
120+
assert.False(t, s.guardInitialized)
121+
}
122+
123+
// TestInitGuardPolicy_SuccessWithNoLabels verifies the happy path: a valid policy with no
124+
// agent labels sets guardInitialized to true and leaves the enforcement mode unchanged.
125+
func TestInitGuardPolicy_SuccessWithNoLabels(t *testing.T) {
126+
g := defaultLabelAgentStub(difc.ModeFilter, []string{}, []string{})
127+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
128+
129+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, nil, nil)
130+
131+
require.NoError(t, err)
132+
assert.True(t, s.guardInitialized)
133+
assert.Equal(t, difc.EnforcementFilter, s.enforcementMode)
134+
}
135+
136+
// TestInitGuardPolicy_SuccessAppliesAgentLabels verifies that secrecy and integrity tags
137+
// returned by LabelAgent are applied to the proxy agent in the registry.
138+
func TestInitGuardPolicy_SuccessAppliesAgentLabels(t *testing.T) {
139+
g := defaultLabelAgentStub(difc.ModeFilter, []string{"private:org/repo"}, []string{"approved"})
140+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
141+
142+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, nil, nil)
143+
144+
require.NoError(t, err)
145+
assert.True(t, s.guardInitialized)
146+
147+
labels := s.agentRegistry.GetOrCreate("proxy")
148+
require.NotNil(t, labels)
149+
assert.Contains(t, labels.GetSecrecyTags(), difc.Tag("private:org/repo"), "secrecy tag must be applied")
150+
assert.Contains(t, labels.GetIntegrityTags(), difc.Tag("approved"), "integrity tag must be applied")
151+
}
152+
153+
// TestInitGuardPolicy_DIFCModeOverride verifies that when the guard returns a DIFCMode in the
154+
// LabelAgent response, it overrides the server's current enforcement mode.
155+
func TestInitGuardPolicy_DIFCModeOverride(t *testing.T) {
156+
// Server starts in filter mode but guard wants strict.
157+
g := defaultLabelAgentStub(difc.ModeStrict, []string{}, []string{})
158+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
159+
160+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, nil, nil)
161+
162+
require.NoError(t, err)
163+
assert.True(t, s.guardInitialized)
164+
assert.Equal(t, difc.EnforcementStrict, s.enforcementMode,
165+
"guard response DIFCMode must override the server's enforcement mode")
166+
}
167+
168+
// TestInitGuardPolicy_InvalidDIFCModeIgnored verifies that an unrecognized DIFCMode in the
169+
// guard response is silently ignored and the original enforcement mode is preserved.
170+
func TestInitGuardPolicy_InvalidDIFCModeIgnored(t *testing.T) {
171+
g := defaultLabelAgentStub("not-a-real-mode", []string{}, []string{})
172+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
173+
174+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, nil, nil)
175+
176+
require.NoError(t, err)
177+
assert.True(t, s.guardInitialized)
178+
assert.Equal(t, difc.EnforcementFilter, s.enforcementMode,
179+
"unrecognized DIFCMode in guard response must not change the enforcement mode")
180+
}
181+
182+
// TestInitGuardPolicy_EmptyDIFCModePreservesMode verifies that an empty DIFCMode in
183+
// the guard response preserves the current enforcement mode without error.
184+
func TestInitGuardPolicy_EmptyDIFCModePreservesMode(t *testing.T) {
185+
g := defaultLabelAgentStub("", []string{}, []string{})
186+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementStrict)
187+
188+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, nil, nil)
189+
190+
require.NoError(t, err)
191+
assert.True(t, s.guardInitialized)
192+
// Empty DIFCMode causes the mode-override block to be skipped entirely, so the
193+
// server's initial strict mode is preserved.
194+
assert.Equal(t, difc.EnforcementStrict, s.enforcementMode)
195+
}
196+
197+
// TestInitGuardPolicy_LegacyAllowOnlyKey verifies that a policy using the legacy
198+
// "allowonly" key (without the hyphen) is accepted and the guard is initialized.
199+
// The normalization step in initGuardPolicy maps "allowonly" → "allow-only" so that
200+
// trusted-user injection via BuildLabelAgentPayload works correctly.
201+
func TestInitGuardPolicy_LegacyAllowOnlyKey(t *testing.T) {
202+
g := defaultLabelAgentStub(difc.ModeFilter, []string{}, []string{})
203+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
204+
205+
legacyPolicy := `{"allowonly":{"repos":"public","min-integrity":"none"}}`
206+
207+
err := s.initGuardPolicy(context.Background(), legacyPolicy, nil, nil)
208+
209+
require.NoError(t, err)
210+
assert.True(t, s.guardInitialized)
211+
}
212+
213+
// TestInitGuardPolicy_LegacyAllowOnlyKeyWithTrustedUsers verifies that a legacy "allowonly"
214+
// key is normalized before trusted-user injection so that trusted users are injected into
215+
// the correct "allow-only" slot in the payload.
216+
func TestInitGuardPolicy_LegacyAllowOnlyKeyWithTrustedUsers(t *testing.T) {
217+
g := defaultLabelAgentStub(difc.ModeFilter, []string{}, []string{})
218+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
219+
220+
legacyPolicy := `{"allowonly":{"repos":"public","min-integrity":"none"}}`
221+
trustedUsers := []string{"alice", "bob"}
222+
223+
// If the normalization works, trusted users are injected into "allow-only" and
224+
// the call succeeds. If it doesn't work, the payload would lack an "allow-only"
225+
// key and the trusted-user injection would be silently skipped (no error, but
226+
// we can at least confirm the guard initialized).
227+
err := s.initGuardPolicy(context.Background(), legacyPolicy, nil, trustedUsers)
228+
229+
require.NoError(t, err)
230+
assert.True(t, s.guardInitialized)
231+
}
232+
233+
// TestInitGuardPolicy_TrustedBotsAndUsers verifies that trusted bots and users can be
234+
// provided alongside a valid allow-only policy without causing an error.
235+
func TestInitGuardPolicy_TrustedBotsAndUsers(t *testing.T) {
236+
g := defaultLabelAgentStub(difc.ModeFilter, []string{}, []string{})
237+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
238+
239+
trustedBots := []string{"dependabot[bot]", "renovate[bot]"}
240+
trustedUsers := []string{"alice"}
241+
242+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, trustedBots, trustedUsers)
243+
244+
require.NoError(t, err)
245+
assert.True(t, s.guardInitialized)
246+
}
247+
248+
// TestInitGuardPolicy_MultipleSecrecyTags verifies that multiple secrecy tags are all applied.
249+
func TestInitGuardPolicy_MultipleSecrecyTags(t *testing.T) {
250+
secrecy := []string{"private:org/repo1", "private:org/repo2"}
251+
g := defaultLabelAgentStub(difc.ModeFilter, secrecy, []string{})
252+
s := newTestServerForInitGuardPolicy(g, difc.EnforcementFilter)
253+
254+
err := s.initGuardPolicy(context.Background(), validAllowOnlyPolicyJSON, nil, nil)
255+
256+
require.NoError(t, err)
257+
assert.True(t, s.guardInitialized)
258+
259+
labels := s.agentRegistry.GetOrCreate("proxy")
260+
require.NotNil(t, labels)
261+
for _, tag := range secrecy {
262+
assert.Contains(t, labels.GetSecrecyTags(), difc.Tag(tag), "secrecy tag %q must be applied", tag)
263+
}
264+
}

0 commit comments

Comments
 (0)