diff --git a/docs/audit_events.md b/docs/audit_events.md index 1bb4c8b72..5fadc521f 100644 --- a/docs/audit_events.md +++ b/docs/audit_events.md @@ -174,6 +174,19 @@ make generate-graphql Audit events are stored to the database. +Kubernetes Secret apply changes must record field paths or key names only, never old or +new values. This applies to all Secret diff fields, including annotations that may +contain credential snapshots. Apply diffs discard values before they are returned +or stored. Historical entries are not redacted by the application. + +Use [the inspection query](../sketchpad/list_sensitive_activity_logs.sql) to list +historical Secret diffs containing values, without displaying the values +themselves. After reviewing the affected rows, run +[the cleanup query](../sketchpad/redact_sensitive_activity_logs.sql) to remove +old/new values in place. This cleanup is manual, not a database migration. Run it +after older API instances have stopped writing unredacted diffs. Rotate credentials +where exposure is established; cleanup does not remove backups or exports. + Use the `Auditor` instance [internal/audit/auditor.go](../internal/audit/auditor.go) to store events: ```go diff --git a/integration_tests/apply_secret.lua b/integration_tests/apply_secret.lua new file mode 100644 index 000000000..ee230e607 --- /dev/null +++ b/integration_tests/apply_secret.lua @@ -0,0 +1,131 @@ +local user = User.new() +local outsider = User.new() +local team = Team.new("apply-secret-team", "Secret apply testing", "#apply-secret-team") +team:addMember(user) + +Test.rest("create Secret via apply", function(t) + t.addHeader("x-user-email", user:email()) + t.send("POST", "/api/v1/teams/apply-secret-team/environments/dev/apply", [[ + {"resources":[{ + "apiVersion":"v1", + "kind":"Secret", + "metadata":{"name":"apply-secret","annotations":{"snapshot":"OLD_SNAPSHOT"}}, + "data":{"PASSWORD":"T0xEX01BUktFUg=="}, + "stringData":{"TOKEN":"OLD_PLAINTEXT_MARKER"} + }]} + ]]) + t.check(200, { + results = { + { resource = "Secret/apply-secret", environmentName = "dev", status = "created" }, + }, + }) +end) + +local update = [[ + {"resources":[{ + "apiVersion":"v1", + "kind":"Secret", + "metadata":{"name":"apply-secret","annotations":{"snapshot":"NEW_SNAPSHOT"}}, + "data":{"PASSWORD":"TkVXX01BUktFUg=="}, + "stringData":{"TOKEN":"NEW_PLAINTEXT_MARKER"} + }]} +]] + +Test.rest("Secret apply response contains field paths only", function(t) + t.addHeader("x-user-email", user:email()) + t.send("POST", "/api/v1/teams/apply-secret-team/environments/dev/apply", update) + t.check(200, { + results = { + { + resource = "Secret/apply-secret", + environmentName = "dev", + status = "applied", + changedFields = { + { field = "data.PASSWORD" }, + { field = "metadata.annotations.snapshot" }, + { field = "stringData.TOKEN" }, + }, + }, + }, + }) +end) + +Test.k8s("Secret apply still updates the Kubernetes object", function(t) + t.check("v1", "secrets", "dev", team:slug(), "apply-secret", { + apiVersion = "v1", + kind = "Secret", + data = { PASSWORD = "TkVXX01BUktFUg==" }, + stringData = { TOKEN = "NEW_PLAINTEXT_MARKER" }, + metadata = { + name = "apply-secret", + namespace = team:slug(), + annotations = { snapshot = "NEW_SNAPSHOT" }, + }, + }) +end) + +Test.rest("unchanged Secret apply has no changed fields", function(t) + t.addHeader("x-user-email", user:email()) + t.send("POST", "/api/v1/teams/apply-secret-team/environments/dev/apply", update) + t.check(200, { + results = { + { resource = "Secret/apply-secret", environmentName = "dev", status = "applied" }, + }, + }) +end) + +Test.sql("Secret apply persists field paths but no values", function(t) + t.queryRow([[ + SELECT + resource_type, + CONVERT_FROM(data, 'UTF8')::JSONB -> 'changedFields' AS changed_fields + FROM activity_log_entries + WHERE resource_name = 'apply-secret' AND action = 'UPDATED' + ]]) + t.check { + resource_type = "SECRET", + changed_fields = { + { field = "data.PASSWORD" }, + { field = "metadata.annotations.snapshot" }, + { field = "stringData.TOKEN" }, + }, + } +end) + +Test.gql("other tenant users see Secret field names only", function(t) + t.addHeader("x-user-email", outsider:email()) + t.query [[ + query { + activityLog(first: 10, filter: { activityTypes: [SECRET_UPDATED] }) { + nodes { + __typename + resourceType + resourceName + ... on SecretUpdatedActivityLogEntry { + data { updatedFields { field oldValue newValue } } + } + } + } + } + ]] + t.check { + data = { + activityLog = { + nodes = { + { + __typename = "SecretUpdatedActivityLogEntry", + resourceType = "SECRET", + resourceName = "apply-secret", + data = { + updatedFields = { + { field = "data.PASSWORD", oldValue = Null, newValue = Null }, + { field = "metadata.annotations.snapshot", oldValue = Null, newValue = Null }, + { field = "stringData.TOKEN", oldValue = Null, newValue = Null }, + }, + }, + }, + }, + }, + }, + } +end) diff --git a/internal/apply/diff.go b/internal/apply/diff.go index 364f6d66c..cae308087 100644 --- a/internal/apply/diff.go +++ b/internal/apply/diff.go @@ -31,6 +31,7 @@ var ignoredMetadataFields = map[string]bool{ // If before is nil, all fields in after are considered "added". // If after is nil, all fields in before are considered "removed". // Server-managed fields (status, metadata.resourceVersion, etc.) are excluded. +// Secret changes include field names only, never values. func Diff(before, after *unstructured.Unstructured) []activitylog.ResourceChangedField { var beforeMap, afterMap map[string]any @@ -42,6 +43,11 @@ func Diff(before, after *unstructured.Unstructured) []activitylog.ResourceChange } changes := diffMaps(beforeMap, afterMap, "") + if (before != nil && before.GetKind() == "Secret") || (after != nil && after.GetKind() == "Secret") { + for i := range changes { + changes[i] = activitylog.ResourceChangedField{Field: changes[i].Field} + } + } // Sort for deterministic output slices.SortFunc(changes, func(a, b activitylog.ResourceChangedField) int { diff --git a/internal/apply/diff_test.go b/internal/apply/diff_test.go index 9fd0ca184..b3b98c097 100644 --- a/internal/apply/diff_test.go +++ b/internal/apply/diff_test.go @@ -1,7 +1,10 @@ package apply import ( + "encoding/json" "fmt" + "slices" + "strings" "testing" "github.com/nais/api/internal/activitylog" @@ -15,6 +18,84 @@ func TestDiff_BothNil(t *testing.T) { } } +func TestDiff_SecretValuesRedacted(t *testing.T) { + before := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]any{ + "annotations": map[string]any{ + "kubectl.kubernetes.io/last-applied-configuration": "OLD_SECRET_SNAPSHOT", + }, + }, + "data": map[string]any{ + "updated": "OLD_SECRET_VALUE", + "removed": "REMOVED_SECRET_VALUE", + }, + "stringData": map[string]any{ + "password": "OLD_PLAINTEXT_VALUE", + }, + }} + after := before.DeepCopy() + after.Object["data"] = map[string]any{ + "updated": "NEW_SECRET_VALUE", + "added": "ADDED_SECRET_VALUE", + } + after.Object["stringData"] = map[string]any{"password": "NEW_PLAINTEXT_VALUE"} + after.SetAnnotations(map[string]string{ + "kubectl.kubernetes.io/last-applied-configuration": "NEW_SECRET_SNAPSHOT", + }) + + for _, test := range []struct { + name string + before *unstructured.Unstructured + after *unstructured.Unstructured + }{ + {name: "update", before: before, after: after}, + {name: "creation", after: after}, + {name: "deletion", before: before}, + } { + t.Run(test.name, func(t *testing.T) { + changes := Diff(test.before, test.after) + if len(changes) == 0 { + t.Fatal("expected field names to remain visible") + } + for _, field := range changes { + if field.OldValue != nil || field.NewValue != nil { + t.Fatalf("Secret field %q contains values", field.Field) + } + } + response, err := json.Marshal(Response{Results: []ResourceResult{{ChangedFields: changes}}}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(response), "VALUE") || strings.Contains(string(response), "SNAPSHOT") { + t.Fatalf("Secret values were serialized: %s", response) + } + }) + } + + var paths []string + for _, field := range Diff(before, after) { + paths = append(paths, field.Field) + } + want := []string{ + "data.added", + "data.removed", + "data.updated", + "metadata.annotations.kubectl.kubernetes.io/last-applied-configuration", + "stringData.password", + } + if !slices.Equal(paths, want) { + t.Fatalf("expected changed paths %v, got %v", want, paths) + } + if changes := Diff(after, after.DeepCopy()); len(changes) != 0 { + t.Fatalf("an unchanged Secret produced changes: %+v", changes) + } + if before.Object["data"].(map[string]any)["updated"] != "OLD_SECRET_VALUE" { + t.Fatal("diff modified the original Secret") + } +} + func TestDiff_BeforeNilCreatesAdded(t *testing.T) { after := &unstructured.Unstructured{ Object: map[string]any{ diff --git a/internal/workload/secret/activitylog.go b/internal/workload/secret/activitylog.go index 634fe0147..a884fb7b0 100644 --- a/internal/workload/secret/activitylog.go +++ b/internal/workload/secret/activitylog.go @@ -15,6 +15,7 @@ const ( ) func init() { + activitylog.RegisterKindResourceType("Secret", activityLogEntryResourceTypeSecret) activitylog.RegisterTransformer(activityLogEntryResourceTypeSecret, func(entry activitylog.GenericActivityLogEntry) (activitylog.ActivityLogEntry, error) { switch entry.Action { case activitylog.ActivityLogEntryActionCreated: @@ -22,12 +23,21 @@ func init() { GenericActivityLogEntry: entry.WithMessage("Created secret"), }, nil case activitylog.ActivityLogEntryActionUpdated: - data, err := activitylog.TransformData(entry, func(data *SecretUpdatedActivityLogEntryData) *SecretUpdatedActivityLogEntryData { - return data - }) + payload, err := activitylog.UnmarshalData[struct { + SecretUpdatedActivityLogEntryData + ChangedFields []activitylog.ResourceChangedField `json:"changedFields"` + }](entry) if err != nil { return nil, err } + data := &payload.SecretUpdatedActivityLogEntryData + for _, field := range payload.ChangedFields { + data.UpdatedFields = append(data.UpdatedFields, &SecretUpdatedActivityLogEntryDataUpdatedField{ + Field: field.Field, + OldValue: field.OldValue, + NewValue: field.NewValue, + }) + } return SecretUpdatedActivityLogEntry{ GenericActivityLogEntry: entry.WithMessage("Updated secret"),