[test] Add tests for proxy.handleWithDIFC uncovered branches#3059
Conversation
Adds handler_difc_test.go with 12 comprehensive tests covering the previously untested branches of the handleWithDIFC DIFC pipeline in internal/proxy/handler.go: - Phase 1: LabelResource error → 502 - Phase 2: write operation blocked by coarse check → 403 - Phase 4: LabelResponse error with coarse allowed → original response - Phase 4: LabelResponse error with coarse blocked → empty response - Phase 5: no fine-grained labels + coarse blocked → empty response - Phase 5: strict mode with filtered items → 403 - Phase 5: filter mode with filtered collection items → accessible items returned - Phase 5: GraphQL + collection + no items filtered → original body preserved - Phase 5: GraphQL + collection + filtered items → rebuilt response - Phase 5: SimpleLabeledData (non-GraphQL) → ToResult - Phase 5: SimpleLabeledData (GraphQL) → original body preserved - Phase 6: propagate mode accumulates secrecy labels on agent Introduces stubGuard, a configurable test double implementing guard.Guard that allows controlling LabelResource and LabelResponse return values to exercise each code path. All existing tests used guard.NewNoopGuard() which always returns nil from LabelResponse, limiting coverage to the happy path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds targeted unit tests to increase branch coverage of the DIFC enforcement pipeline in internal/proxy by exercising previously untested error/fallback and filtering behaviors in handleWithDIFC.
Changes:
- Introduces a configurable
stubGuardto deterministically controlLabelResource/LabelResponseoutcomes. - Adds 12 new tests covering LabelResource/LabelResponse failure paths, strict vs filter mode behavior, GraphQL collection rebuild behavior, and propagate-mode label accumulation.
- Adds small test helpers for building servers with specific enforcement modes and agent default labels.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestHandleWithDIFC_WriteOperationBlocked(t *testing.T) { | ||
| // The agent carries a private secrecy tag; the resource (public) has no secrecy. | ||
| // For a WRITE: agent secrecy must be a subset of resource secrecy. | ||
| // Agent has "private:test-org/test-repo", resource is empty → write is denied. | ||
| upstream := mockUpstream(t, http.StatusOK, map[string]interface{}{"id": 1}) | ||
| defer upstream.Close() | ||
|
|
||
| g := &stubGuard{ | ||
| labelResourceResult: publicResource(), | ||
| labelResourceOp: difc.OperationWrite, | ||
| } | ||
| s := newTestServerWithPrivateAgent(t, upstream.URL, g, difc.EnforcementFilter) | ||
| h := &proxyHandler{server: s} | ||
|
|
||
| req := httptest.NewRequest(http.MethodPost, "/repos/org/repo/issues", | ||
| bytes.NewBufferString(`{"title":"new"}`)) | ||
| w := httptest.NewRecorder() | ||
| h.handleWithDIFC(w, req, "/repos/org/repo/issues", "create_issue", | ||
| map[string]interface{}{"owner": "org", "repo": "repo"}, nil) |
There was a problem hiding this comment.
TestHandleWithDIFC_WriteOperationBlocked drives the write-block branch by calling handleWithDIFC with a REST POST. In the actual request path, ServeHTTP bypasses handleWithDIFC for non-GraphQL write methods (it calls passthrough), so this scenario is unreachable and can give misleading coverage. Consider rewriting this test to exercise the same Phase 2 write-deny behavior through a GraphQL POST /graphql that the guard labels as OperationWrite (e.g., a mutation), or otherwise route through ServeHTTP in a way that matches production control flow.
| for i, t := range tags { | ||
| result[i] = string(t) |
There was a problem hiding this comment.
In tagsAsStrings, the loop variable name t is easy to confuse with the conventional t *testing.T identifier used throughout tests. Renaming it to something like tag would make the helper clearer and reduce cognitive overhead when reading failures.
| for i, t := range tags { | |
| result[i] = string(t) | |
| for i, tag := range tags { | |
| result[i] = string(tag) |
Test Coverage Improvement:
handleWithDIFCFunction Analyzed
internal/proxyhandleWithDIFCinternal/proxy/handler.go(lines 116–302)Why This Function?
handleWithDIFCis the core enforcement function of the proxy server. It implements the full DIFC (Decentralized Information Flow Control) pipeline. Despite its complexity, all 6 existing tests usedguard.NewNoopGuard(), which always returnsnilfromLabelResponse. This means only the "no fine-grained labels + coarse allowed" path was tested — leaving the vast majority of branches uncovered.What Was Added
internal/proxy/handler_difc_test.go— 12 new tests covering every previously untested branch:TestHandleWithDIFC_LabelResourceErrorTestHandleWithDIFC_WriteOperationBlockedTestHandleWithDIFC_LabelResponseError_CoarseAllowedTestHandleWithDIFC_LabelResponseError_CoarseBlockedTestHandleWithDIFC_NoFineGrainedLabels_CoarseBlockedTestHandleWithDIFC_StrictMode_FiltersBlockTestHandleWithDIFC_Collection_FilterMode_ItemsFilteredTestHandleWithDIFC_GraphQL_Collection_NoItemsFilteredTestHandleWithDIFC_GraphQL_Collection_ItemsFilteredTestHandleWithDIFC_SimpleLabeledData_NonGraphQLSimpleLabeledData(REST) → ToResult pathTestHandleWithDIFC_SimpleLabeledData_GraphQLSimpleLabeledData(GraphQL) → original bodyTestHandleWithDIFC_PropagateMode_AccumulatesLabelsKey Infrastructure
stubGuard: A configurableguard.Guardtest double that allows tests to control whatLabelResourceandLabelResponsereturn, enabling precise branch targeting.newTestServerWithStub/newTestServerWithPrivateAgent: Server factory helpers that set up servers with specific guards and enforcement modes.privateResource()/publicResource(): Factory helpers for creating labeled resources to trigger coarse check denials.Notes
package proxy(same package), following the project convention.require/assert) as per AGENTS.md conventions.handler_test.gopatterns (mockUpstream,proxyHandler,httptest).gofmt) and logically correct (verified against the handler implementation).Generated by Test Coverage Improver