fix: prevent Resource.response() from mutating handler-owned attributes

list_response.go

@@ -44,9 +44,10 @@ func (p Page) rawResources() []interface{} {
 
 	var resources []interface{}
 	for _, v := range p.Resources {
-		attrs := v.Attributes
-		if attrs == nil {
-			attrs = ResourceAttributes{}
+		// Copy outer map to avoid mutating handler-owned attributes.
+		attrs := make(ResourceAttributes, len(v.Attributes)+3)
+		for k, val := range v.Attributes {
+			attrs[k] = val
 		}
 
 		attrs[schema.CommonAttributeID] = v.ID
@@ -55,10 +56,14 @@ func (p Page) rawResources() []interface{} {
 		}
 
 		// Merge Meta fields into the existing "meta" map if present, or create a new one.
+		// Copy the nested meta map too, to avoid mutating a handler-owned nested map.
 		var metaMap map[string]interface{}
 		if existing, ok := attrs[schema.CommonAttributeMeta]; ok {
 			if m, ok := existing.(map[string]interface{}); ok {
-				metaMap = m
+				metaMap = make(map[string]interface{}, len(m)+3)
+				for k, val := range m {
+					metaMap[k] = val
+				}
 			}
 		}
 		hasMeta := false

list_response_test.go

@@ -0,0 +1,60 @@
+package scim
+
+import (
+	"testing"
+	"time"
+
+	"github.com/elimity-com/scim/schema"
+)
+
+func TestRawResourcesDoesNotMutateHandlerAttributes(t *testing.T) {
+	t.Parallel()
+
+	t.Run("outer map not mutated", func(t *testing.T) {
+		t.Parallel()
+		attrs := ResourceAttributes{"displayName": "Test User"}
+		original := make(ResourceAttributes, len(attrs))
+		for k, v := range attrs {
+			original[k] = v
+		}
+		p := Page{Resources: []Resource{{ID: "42", Attributes: attrs}}}
+
+		_ = p.rawResources()
+
+		for k := range attrs {
+			if _, ok := original[k]; !ok {
+				t.Errorf("rawResources() injected %q into handler-owned attributes map", k)
+			}
+		}
+	})
+
+	t.Run("nested meta map not mutated", func(t *testing.T) {
+		t.Parallel()
+		// Handler provides a meta map (e.g. with resourceType pre-filled).
+		existingMeta := map[string]interface{}{"resourceType": "User"}
+		originalMeta := make(map[string]interface{}, len(existingMeta))
+		for k, v := range existingMeta {
+			originalMeta[k] = v
+		}
+		attrs := ResourceAttributes{
+			"displayName":              "Test User",
+			schema.CommonAttributeMeta: existingMeta,
+		}
+		now := time.Now()
+		p := Page{
+			Resources: []Resource{{
+				ID:         "42",
+				Attributes: attrs,
+				Meta:       Meta{Created: &now, LastModified: &now, Version: "W/\"1\""},
+			}},
+		}
+
+		_ = p.rawResources()
+
+		for k := range existingMeta {
+			if _, ok := originalMeta[k]; !ok {
+				t.Errorf("rawResources() injected %q into handler-owned meta map", k)
+			}
+		}
+	})
+}

resource_handler.go

@@ -46,9 +46,10 @@ type Resource struct {
 }
 
 func (r Resource) response(resourceType ResourceType, location string) ResourceAttributes {
-	response := r.Attributes
-	if response == nil {
-		response = ResourceAttributes{}
+	// Copy attributes to avoid mutating the map returned by the handler.
+	response := make(ResourceAttributes, len(r.Attributes))
+	for k, v := range r.Attributes {
+		response[k] = v
 	}
 
 	response[schema.CommonAttributeID] = r.ID

resource_response_mutation_test.go

@@ -0,0 +1,67 @@
+package scim_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/elimity-com/scim"
+	"github.com/elimity-com/scim/schema"
+)
+
+// TestResponseDoesNotMutateHandlerAttributes verifies that serving a GET request
+// does not modify the attributes map stored inside the resource handler.
+//
+// Resource.response() previously did:
+//
+//	response := r.Attributes  // reference copy, not a value copy
+//	response["id"] = r.ID     // writes back into the handler's stored map
+//
+// The result: a second GET on the same resource would find framework-injected
+// keys ("id", "schemas", "meta") already present in the attributes, leading to
+// duplicate or stale data in the response.
+func TestResponseDoesNotMutateHandlerAttributes(t *testing.T) {
+	handler := &testResourceHandler{
+		data: map[string]testData{
+			"0001": {
+				attributes: scim.ResourceAttributes{
+					"userName": "alice",
+				},
+			},
+		},
+		schema: schema.CoreUserSchema(),
+	}
+
+	s, err := scim.NewServer(&scim.ServerArgs{
+		ServiceProviderConfig: &scim.ServiceProviderConfig{},
+		ResourceTypes: []scim.ResourceType{
+			{
+				Name:     "User",
+				Endpoint: "/Users",
+				Schema:   schema.CoreUserSchema(),
+				Handler:  handler,
+			},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "/Users/0001", nil)
+	w := httptest.NewRecorder()
+	s.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+
+	// After the GET, the handler's stored map must not contain any of the
+	// framework-injected keys.  If response() mutated the map, these will be
+	// present and a subsequent GET would return them as user-supplied data.
+	stored := handler.data["0001"].attributes
+	for _, key := range []string{"id", "schemas", "meta"} {
+		if _, ok := stored[key]; ok {
+			t.Errorf("Resource.response() mutated handler attributes: key %q was injected into the stored map", key)
+		}
+	}
+}