Skip to content

Commit 6fc2ccb

Browse files
committed
fix: exercise verify=true in DoesNotMutateCallerMaps test
The test previously passed verify=false, but all delete calls that mutate maps are inside the `if verify` block, making the test a no-op. Use verify=true with a mock HTTP client that triggers tenant-not-found deletions so the test actually validates the maps.Clone protection. Made-with: Cursor
1 parent 3461a39 commit 6fc2ccb

File tree

1 file changed

+36
-10
lines changed

1 file changed

+36
-10
lines changed

pkg/detectors/azure_entra/serviceprincipal/v2/spv2_determinism_test.go

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ package v2
22

33
import (
44
"context"
5+
"io"
6+
"maps"
7+
"net/http"
8+
"strings"
59
"testing"
610

711
"github.com/stretchr/testify/assert"
@@ -109,7 +113,10 @@ func TestProcessData_DeterministicRawV2(t *testing.T) {
109113
}
110114

111115
// TestProcessData_DoesNotMutateCallerMaps verifies that ProcessData does not
112-
// modify the maps passed by the caller.
116+
// modify the maps passed by the caller. It uses verify=true with a mock HTTP
117+
// client so that the verification code path (which contains delete calls) is
118+
// actually exercised. With verify=false, no deletes occur and the test would
119+
// pass trivially even without maps.Clone.
113120
func TestProcessData_DoesNotMutateCallerMaps(t *testing.T) {
114121
const clientSecret = "abc4Q~fake-secret-that-is-long-enough1234567"
115122

@@ -118,20 +125,39 @@ func TestProcessData_DoesNotMutateCallerMaps(t *testing.T) {
118125
"a1b2c3d4-e5f6-7890-abcd-ef1234567890": {},
119126
"f9e8d7c6-b5a4-3210-fedc-ba9876543210": {},
120127
}
128+
// Use tenant IDs unique to this test to avoid polluting the package-level
129+
// tenantCache shared with other tests.
121130
tenantIDs := map[string]struct{}{
122-
"11111111-2222-3333-4444-555566667777": {},
123-
"aaaaaaaa-bbbb-cccc-dddd-eeeeffff0000": {},
131+
"cccccccc-dddd-eeee-ffff-000011112222": {},
132+
"dddddddd-eeee-ffff-0000-111122223333": {},
124133
}
125134

126-
origSecretLen := len(secrets)
127-
origClientLen := len(clientIDs)
128-
origTenantLen := len(tenantIDs)
135+
origSecrets := maps.Clone(secrets)
136+
origClientIDs := maps.Clone(clientIDs)
137+
origTenantIDs := maps.Clone(tenantIDs)
138+
139+
// Return 400 for every request so TenantExists returns false, triggering
140+
// delete(activeTenants, tenantId) inside the verify block.
141+
mockClient := &http.Client{
142+
Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
143+
return &http.Response{
144+
StatusCode: http.StatusBadRequest,
145+
Body: io.NopCloser(strings.NewReader("")),
146+
}, nil
147+
}),
148+
}
149+
150+
_ = ProcessData(context.Background(), secrets, clientIDs, tenantIDs, true, mockClient)
151+
152+
assert.Equal(t, origSecrets, secrets, "caller's secrets map must not be mutated")
153+
assert.Equal(t, origClientIDs, clientIDs, "caller's clientIDs map must not be mutated")
154+
assert.Equal(t, origTenantIDs, tenantIDs, "caller's tenantIDs map must not be mutated")
155+
}
129156

130-
_ = ProcessData(context.Background(), secrets, clientIDs, tenantIDs, false, nil)
157+
type roundTripFunc func(*http.Request) (*http.Response, error)
131158

132-
assert.Len(t, secrets, origSecretLen, "caller's secrets map must not be mutated")
133-
assert.Len(t, clientIDs, origClientLen, "caller's clientIDs map must not be mutated")
134-
assert.Len(t, tenantIDs, origTenantLen, "caller's tenantIDs map must not be mutated")
159+
func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
160+
return f(req)
135161
}
136162

137163
// TestProcessData_SameSecretDifferentRawV2 demonstrates the chain:

0 commit comments

Comments
 (0)