fix: improve chat audit log descriptions and diff rendering (#25728)

Chat ACL audit diffs rendered as `[object Object]` because the diff
viewer called `.toString()` on object values. Common chat operations
(archive, share) showed generic "updated chat" descriptions instead of
semantic ones.

Add `chatAuditLogDescription` to derive semantic descriptions from the
audit diff for successful chat writes: "archived/unarchived chat" for
archive toggles, "updated sharing for chat" for ACL-only changes.
Extract diff value formatting into `formatAuditDiffValue`, which renders
object values as deterministic compact JSON with sorted keys, fixing the
`[object Object]` rendering for chat ACLs and any other object-valued
fields. The previous `determineIdPSyncMappingDiff` workaround for IdP
sync mappings was removed because the generic formatting handles it.

Closes CODAGT-513

> Generated by Coder Agents on behalf of @johnstcn

coderd/audit.go

@@ -303,6 +303,12 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
 		_, _ = b.WriteString("{user} ")
 	}
 
+	// Chat write operations get semantic descriptions derived from the diff.
+	if desc, ok := chatAuditLogDescription(alog); ok {
+		_, _ = b.WriteString(desc)
+		return b.String()
+	}
+
 	switch {
 	case alog.AuditLog.StatusCode == int32(http.StatusSeeOther):
 		_, _ = b.WriteString("was redirected attempting to ")
@@ -345,6 +351,56 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
 	return b.String()
 }
 
+// chatAuditLogDescription returns a description for successful chat write
+// operations based on the diff contents. It returns false for non-chat
+// resources, non-write actions, or error/redirect status codes, letting
+// the caller fall through to the generic description.
+func chatAuditLogDescription(alog database.GetAuditLogsOffsetRow) (string, bool) {
+	if alog.AuditLog.ResourceType != database.ResourceTypeChat ||
+		alog.AuditLog.Action != database.AuditActionWrite ||
+		alog.AuditLog.StatusCode >= 400 ||
+		alog.AuditLog.StatusCode == int32(http.StatusSeeOther) {
+		return "", false
+	}
+
+	var diff codersdk.AuditDiff
+	if err := json.Unmarshal(alog.AuditLog.Diff, &diff); err != nil {
+		return "", false
+	}
+
+	// Single "archived" field: archive or unarchive.
+	if len(diff) == 1 {
+		if field, ok := diff["archived"]; ok {
+			oldVal, oldOK := field.Old.(bool)
+			newVal, newOK := field.New.(bool)
+			if oldOK && newOK {
+				if !oldVal && newVal {
+					return "archived chat {target}", true
+				}
+				if oldVal && !newVal {
+					return "unarchived chat {target}", true
+				}
+			}
+		}
+	}
+
+	// All fields are ACL changes: sharing update.
+	if len(diff) > 0 {
+		aclOnly := true
+		for field := range diff {
+			if field != "user_acl" && field != "group_acl" {
+				aclOnly = false
+				break
+			}
+		}
+		if aclOnly {
+			return "updated sharing for chat {target}", true
+		}
+	}
+
+	return "", false
+}
+
 func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.GetAuditLogsOffsetRow) bool {
 	switch alog.AuditLog.ResourceType {
 	case database.ResourceTypeTemplate:

coderd/audit_internal_test.go

@@ -3,6 +3,7 @@ package coderd
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 	"testing"
 
 	"github.com/google/uuid"
@@ -14,6 +15,7 @@ import (
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbauthz"
 	"github.com/coder/coder/v2/coderd/database/dbmock"
+	"github.com/coder/coder/v2/codersdk"
 )
 
 func TestAuditLogIsResourceDeleted(t *testing.T) {
@@ -111,6 +113,91 @@ func TestAuditLogDescription(t *testing.T) {
 			},
 			want: "{user} deleted the git ssh key",
 		},
+		{
+			name: "chat_archived",
+			alog: chatAuditLogRow(t, codersdk.AuditDiff{
+				"archived": {Old: false, New: true},
+			}),
+			want: "{user} archived chat {target}",
+		},
+		{
+			name: "chat_unarchived",
+			alog: chatAuditLogRow(t, codersdk.AuditDiff{
+				"archived": {Old: true, New: false},
+			}),
+			want: "{user} unarchived chat {target}",
+		},
+		{
+			name: "chat_sharing_user_acl",
+			alog: chatAuditLogRow(t, codersdk.AuditDiff{
+				"user_acl": {Old: map[string]any{}, New: map[string]any{"user-1": map[string]any{"permissions": []string{"read"}}}},
+			}),
+			want: "{user} updated sharing for chat {target}",
+		},
+		{
+			name: "chat_sharing_group_acl",
+			alog: chatAuditLogRow(t, codersdk.AuditDiff{
+				"group_acl": {Old: map[string]any{}, New: map[string]any{"group-1": map[string]any{"permissions": []string{"read"}}}},
+			}),
+			want: "{user} updated sharing for chat {target}",
+		},
+		{
+			name: "chat_sharing_both_acls",
+			alog: chatAuditLogRow(t, codersdk.AuditDiff{
+				"user_acl":  {Old: map[string]any{}, New: map[string]any{"user-1": map[string]any{"permissions": []string{"read"}}}},
+				"group_acl": {Old: map[string]any{}, New: map[string]any{"group-1": map[string]any{"permissions": []string{"read"}}}},
+			}),
+			want: "{user} updated sharing for chat {target}",
+		},
+		{
+			name: "chat_mixed_diff_falls_through",
+			alog: chatAuditLogRow(t, codersdk.AuditDiff{
+				"archived":  {Old: false, New: true},
+				"pin_order": {Old: 1, New: 0},
+			}),
+			want: "{user} updated chat {target}",
+		},
+		{
+			name: "chat_acl_with_extra_field_falls_through",
+			alog: chatAuditLogRow(t, codersdk.AuditDiff{
+				"user_acl":  {Old: map[string]any{}, New: map[string]any{}},
+				"pin_order": {Old: 1, New: 0},
+			}),
+			want: "{user} updated chat {target}",
+		},
+		{
+			name: "chat_failed_write_no_override",
+			alog: func() database.GetAuditLogsOffsetRow {
+				row := chatAuditLogRow(t, codersdk.AuditDiff{
+					"archived": {Old: false, New: true},
+				})
+				row.AuditLog.StatusCode = 400
+				return row
+			}(),
+			want: "{user} unsuccessfully attempted to write chat {target}",
+		},
+		{
+			name: "chat_redirect_no_override",
+			alog: func() database.GetAuditLogsOffsetRow {
+				row := chatAuditLogRow(t, codersdk.AuditDiff{
+					"archived": {Old: false, New: true},
+				})
+				row.AuditLog.StatusCode = 303
+				return row
+			}(),
+			want: "{user} was redirected attempting to write chat {target}",
+		},
+		{
+			name: "chat_non_write_action_no_override",
+			alog: func() database.GetAuditLogsOffsetRow {
+				row := chatAuditLogRow(t, codersdk.AuditDiff{
+					"user_acl": {Old: map[string]any{}, New: map[string]any{"user-1": map[string]any{"permissions": []string{"read"}}}},
+				})
+				row.AuditLog.Action = database.AuditActionCreate
+				return row
+			}(),
+			want: "{user} created chat {target}",
+		},
 	}
 	// nolint: paralleltest // no longer need to reinitialize loop vars in go 1.22
 	for _, tc := range testCases {
@@ -121,3 +208,19 @@ func TestAuditLogDescription(t *testing.T) {
 		})
 	}
 }
+
+// chatAuditLogRow builds a GetAuditLogsOffsetRow for a successful chat write
+// with the given diff, suitable for testing auditLogDescription.
+func chatAuditLogRow(t *testing.T, diff codersdk.AuditDiff) database.GetAuditLogsOffsetRow {
+	t.Helper()
+	rawDiff, err := json.Marshal(diff)
+	require.NoError(t, err)
+	return database.GetAuditLogsOffsetRow{
+		AuditLog: database.AuditLog{
+			Action:       database.AuditActionWrite,
+			StatusCode:   200,
+			ResourceType: database.ResourceTypeChat,
+			Diff:         rawDiff,
+		},
+	}
+}

site/src/pages/AuditPage/AuditLogRow/AuditLogDiff/AuditLogDiff.tsx

@@ -1,43 +1,6 @@
 import type { FC } from "react";
 import type { AuditDiff } from "#/api/typesGenerated";
-
-const getDiffValue = (value: unknown): string => {
-	if (typeof value === "string") {
-		return `"${value}"`;
-	}
-
-	if (isTimeObject(value)) {
-		if (!value.Valid) {
-			return "null";
-		}
-
-		return new Date(value.Time).toLocaleString();
-	}
-
-	if (Array.isArray(value)) {
-		const values = value.map((v) => getDiffValue(v));
-		return `[${values.join(", ")}]`;
-	}
-
-	if (value === null || value === undefined) {
-		return "null";
-	}
-
-	return String(value);
-};
-
-const isTimeObject = (
-	value: unknown,
-): value is { Time: string; Valid: boolean } => {
-	return (
-		value !== null &&
-		typeof value === "object" &&
-		"Time" in value &&
-		typeof value.Time === "string" &&
-		"Valid" in value &&
-		typeof value.Valid === "boolean"
-	);
-};
+import { formatAuditDiffValue } from "./auditUtils";
 
 interface AuditLogDiffProps {
 	diff: AuditDiff;
@@ -58,7 +21,9 @@ export const AuditLogDiff: FC<AuditLogDiffProps> = ({ diff }) => {
 						<div>
 							{attrName}:{" "}
 							<span className="rounded p-px bg-red-800">
-								{valueDiff.secret ? "••••••••" : getDiffValue(valueDiff.old)}
+								{valueDiff.secret
+									? "••••••••"
+									: formatAuditDiffValue(valueDiff.old)}
 							</span>
 						</div>
 					</div>
@@ -74,7 +39,9 @@ export const AuditLogDiff: FC<AuditLogDiffProps> = ({ diff }) => {
 						<div>
 							{attrName}:{" "}
 							<span className="rounded p-px bg-green-800">
-								{valueDiff.secret ? "••••••••" : getDiffValue(valueDiff.new)}
+								{valueDiff.secret
+									? "••••••••"
+									: formatAuditDiffValue(valueDiff.new)}
 							</span>
 						</div>
 					</div>

site/src/pages/AuditPage/AuditLogRow/AuditLogDiff/auditUtils.test.ts

@@ -1,4 +1,4 @@
-import { determineGroupDiff } from "./auditUtils";
+import { determineGroupDiff, formatAuditDiffValue } from "./auditUtils";
 
 const auditDiffForNewGroup = {
 	id: {
@@ -120,3 +120,70 @@ describe("determineAuditDiff", () => {
 		expect(determineGroupDiff(AuditDiffForDeletedGroup)).toEqual(result);
 	});
 });
+
+describe("formatAuditDiffValue", () => {
+	it.each([
+		{ name: "string", value: "hello", expected: '"hello"' },
+		{
+			name: "string containing double quotes",
+			value: 'he said "hello"',
+			expected: '"he said \\"hello\\""',
+		},
+		{
+			name: "array of primitives",
+			value: ["admin", "auditor"],
+			expected: '["admin", "auditor"]',
+		},
+		{ name: "boolean true", value: true, expected: "true" },
+		{ name: "boolean false", value: false, expected: "false" },
+		{ name: "number", value: 42, expected: "42" },
+		{ name: "null", value: null, expected: "null" },
+		{ name: "undefined", value: undefined, expected: "null" },
+		{
+			name: "invalid SQL time",
+			value: { Time: "0001-01-01T00:00:00Z", Valid: false },
+			expected: "null",
+		},
+	])("preserves current behavior for $name", ({ value, expected }) => {
+		expect(formatAuditDiffValue(value)).toBe(expected);
+	});
+
+	it("preserves current behavior for valid SQL time objects", () => {
+		const value = { Time: "2024-10-22T09:03:23.961702Z", Valid: true };
+
+		expect(formatAuditDiffValue(value)).toBe(
+			new Date(value.Time).toLocaleString(),
+		);
+	});
+
+	it("formats plain objects as deterministic compact JSON", () => {
+		expect(
+			formatAuditDiffValue({
+				z: ["read"],
+				a: { permissions: ["read"] },
+			}),
+		).toBe('{"a":{"permissions":["read"]},"z":["read"]}');
+	});
+
+	it("formats chat ACL objects as deterministic compact JSON", () => {
+		expect(
+			formatAuditDiffValue({
+				"user-2": { permissions: ["read"] },
+				"user-1": { permissions: ["read"] },
+			}),
+		).toBe(
+			'{"user-1":{"permissions":["read"]},"user-2":{"permissions":["read"]}}',
+		);
+	});
+
+	it("formats arrays containing objects without object string coercion", () => {
+		expect(
+			formatAuditDiffValue([
+				{ user_id: "user-2", permissions: ["read"] },
+				{ permissions: ["read"], user_id: "user-1" },
+			]),
+		).toBe(
+			'[{"permissions":["read"],"user_id":"user-2"}, {"permissions":["read"],"user_id":"user-1"}]',
+		);
+	});
+});

site/src/pages/AuditPage/AuditLogRow/AuditLogDiff/auditUtils.ts

@@ -25,27 +25,68 @@ export const determineGroupDiff = (auditLogDiff: AuditDiff): AuditDiff => {
 };
 
 /**
- *
- * @param auditLogDiff
- * @returns a diff with the 'mappings' as a JSON string. Otherwise, it is [Object object]
+ * Formats an audit diff value for display. Strings are quoted, nullish values
+ * become "null", SQL time objects are localized, arrays are recursed, and plain
+ * objects are serialized as compact JSON with sorted keys.
  */
-export const determineIdPSyncMappingDiff = (
-	auditLogDiff: AuditDiff,
-): AuditDiff => {
-	const old = auditLogDiff.mapping?.old as Record<string, string[]> | undefined;
-	const new_ = auditLogDiff.mapping?.new as
-		| Record<string, string[]>
-		| undefined;
-	if (!old || !new_) {
-		return auditLogDiff;
+export const formatAuditDiffValue = (value: unknown): string => {
+	if (typeof value === "string") {
+		return JSON.stringify(value);
 	}
 
-	return {
-		...auditLogDiff,
-		mapping: {
-			old: JSON.stringify(old),
-			new: JSON.stringify(new_),
-			secret: auditLogDiff.mapping?.secret,
-		},
-	};
+	if (isTimeObject(value)) {
+		if (!value.Valid) {
+			return "null";
+		}
+
+		return new Date(value.Time).toLocaleString();
+	}
+
+	if (Array.isArray(value)) {
+		const values = value.map((v) => formatAuditDiffValue(v));
+		return `[${values.join(", ")}]`;
+	}
+
+	if (value === null || value === undefined) {
+		return "null";
+	}
+
+	if (isPlainObject(value)) {
+		return JSON.stringify(sortObjectKeys(value));
+	}
+
+	return String(value);
+};
+
+const isTimeObject = (
+	value: unknown,
+): value is { Time: string; Valid: boolean } => {
+	return (
+		value !== null &&
+		typeof value === "object" &&
+		"Time" in value &&
+		typeof value.Time === "string" &&
+		"Valid" in value &&
+		typeof value.Valid === "boolean"
+	);
+};
+
+const isPlainObject = (value: unknown): value is Record<string, unknown> => {
+	return Object.prototype.toString.call(value) === "[object Object]";
+};
+
+const sortObjectKeys = (value: unknown): unknown => {
+	if (Array.isArray(value)) {
+		return value.map(sortObjectKeys);
+	}
+
+	if (!isPlainObject(value)) {
+		return value;
+	}
+
+	const sorted: Record<string, unknown> = {};
+	for (const key of Object.keys(value).sort()) {
+		sorted[key] = sortObjectKeys(value[key]);
+	}
+	return sorted;
 };

site/src/pages/AuditPage/AuditLogRow/AuditLogRow.stories.tsx

@@ -1,4 +1,5 @@
 import type { Meta, StoryObj } from "@storybook/react-vite";
+import type { AuditLog } from "#/api/typesGenerated";
 import { Table, TableBody } from "#/components/Table/Table";
 import { chromatic } from "#/testHelpers/chromatic";
 import {
@@ -187,3 +188,87 @@ export const WithConnectionType: Story = {
 		},
 	},
 };
+
+const MockChatAuditLog: AuditLog = {
+	...MockAuditLog,
+	resource_type: "chat",
+	resource_id: "c542b43f-4375-421a-a7e0-b39187e35131",
+	resource_target: "c542b43f",
+	resource_icon: "",
+	resource_link: "/agents/c542b43f-4375-421a-a7e0-b39187e35131",
+	description: "{user} updated chat {target}",
+	additional_fields: {},
+};
+
+export const WithChatACLDiff: Story = {
+	parameters: { chromatic },
+	args: {
+		auditLog: {
+			...MockChatAuditLog,
+			id: "1d718c45-5dfb-4f24-9546-4f61fa8e3402",
+			action: "write",
+			description: "{user} updated sharing for chat {target}",
+			diff: {
+				user_acl: {
+					old: {},
+					new: {
+						"9a68e35d-bf3a-43bd-8e68-130df721cc71": {
+							permissions: ["read"],
+						},
+					},
+					secret: false,
+				},
+				group_acl: {
+					old: {},
+					new: {
+						"6d130d81-017e-44ff-8fca-3a38623dcb14": {
+							permissions: ["read"],
+						},
+					},
+					secret: false,
+				},
+			},
+		},
+		defaultIsDiffOpen: true,
+	},
+};
+
+export const WithArchivedChatDescription: Story = {
+	args: {
+		auditLog: {
+			...MockChatAuditLog,
+			id: "57329396-084a-4074-9930-385a7eed858a",
+			action: "write",
+			description: "{user} archived chat {target}",
+			diff: {
+				archived: {
+					old: false,
+					new: true,
+					secret: false,
+				},
+			},
+		},
+	},
+};
+
+export const WithUpdatedChatSharingDescription: Story = {
+	args: {
+		auditLog: {
+			...MockChatAuditLog,
+			id: "8f26cabf-8867-4d2f-942d-77e759a16c1c",
+			action: "write",
+			description: "{user} updated sharing for chat {target}",
+			diff: {
+				user_acl: {
+					old: {},
+					new: {
+						"9a68e35d-bf3a-43bd-8e68-130df721cc71": {
+							permissions: ["read"],
+						},
+					},
+					secret: false,
+				},
+			},
+		},
+	},
+};

site/src/pages/AuditPage/AuditLogRow/AuditLogRow.tsx

@@ -22,10 +22,7 @@ import { cn } from "#/utils/cn";
 import { buildReasonLabels } from "#/utils/workspace";
 import { AuditLogDescription } from "./AuditLogDescription/AuditLogDescription";
 import { AuditLogDiff } from "./AuditLogDiff/AuditLogDiff";
-import {
-	determineGroupDiff,
-	determineIdPSyncMappingDiff,
-} from "./AuditLogDiff/auditUtils";
+import { determineGroupDiff } from "./AuditLogDiff/auditUtils";
 
 interface AuditLogRowProps {
 	auditLog: AuditLog;
@@ -53,14 +50,6 @@ export const AuditLogRow: FC<AuditLogRowProps> = ({
 		auditDiff = determineGroupDiff(auditLog.diff);
 	}
 
-	if (
-		auditLog.resource_type === "idp_sync_settings_organization" ||
-		auditLog.resource_type === "idp_sync_settings_group" ||
-		auditLog.resource_type === "idp_sync_settings_role"
-	) {
-		auditDiff = determineIdPSyncMappingDiff(auditLog.diff);
-	}
-
 	const toggle = () => {
 		if (shouldDisplayDiff) {
 			setIsDiffOpen((v) => !v);