From 38f5d3f0b2b75c07affa9af1d83ecefcb2ce1a50 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 22 Apr 2026 11:52:52 +0100 Subject: [PATCH] test: add regression guard for chat title masking (#24584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #24564 addressing unresolved review findings. - **DEREM-1**: Add `Test_diff/Chat/TitleMasked` to `enterprise/audit/diff_internal_test.go` so flipping `title` back to `ActionTrack` fails loudly. Verified: the case passes today, fails with a clear diff after flipping to `ActionTrack`, passes again after reverting. - **DEREM-4**: Inline comment at `coderd/audit/request.go:138` explaining why `ResourceTarget` for `database.Chat` returns a UUID prefix instead of the title. - **DEREM-5**: Trailing comment on `enterprise/audit/table.go` `title` entry, matching the surrounding `ActionSecret` comment style. Won't-fix, with rationale (per user): - **DEREM-2** (8-char prefix collision risk): `resource_target` is a display hint, not an identifier; the full UUID lives in `resource_id`. - **DEREM-3** (named constant for `[:8]`): single call site; extracting would be ceremony. - **DEREM-6** (PR title misleading): merged PR title is immutable. - **DEREM-7** (historical log redaction): the offending version only shipped to dogfood for a couple of hours and not to customers. > 🤖 --- coderd/audit/request.go | 6 ++++++ coderd/audit/request_test.go | 14 ++++++++++++++ enterprise/audit/diff_internal_test.go | 23 +++++++++++++++++++++++ enterprise/audit/table.go | 6 +++--- 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/coderd/audit/request.go b/coderd/audit/request.go index a1bc0a4be9..b9bf307ab4 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -135,6 +135,12 @@ func ResourceTarget[T Auditable](tgt T) string { case database.AiSeatState: return "AI Seat" case database.Chat: + // Chat titles can contain sensitive content (secrets, internal + // project names), so we use a short UUID prefix as a display + // hint instead. The full UUID is still recorded in resource_id, + // which is what the audit UI links on. An 8-char prefix is fine + // for display; collisions affect the display label and search + // filter but not the primary resource identifier. return typed.ID.String()[:8] default: panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt)) diff --git a/coderd/audit/request_test.go b/coderd/audit/request_test.go index e0040425d4..9bdf4718d3 100644 --- a/coderd/audit/request_test.go +++ b/coderd/audit/request_test.go @@ -4,10 +4,12 @@ import ( "context" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/propagation" "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/database" ) func TestBaggage(t *testing.T) { @@ -31,3 +33,15 @@ func TestBaggage(t *testing.T) { require.Equal(t, expected, got) } + +func TestResourceTarget_ChatTitleNotLeaked(t *testing.T) { + t.Parallel() + + chat := database.Chat{ + ID: uuid.UUID{1}, + Title: "sensitive-project-name", + } + target := audit.ResourceTarget(chat) + require.NotContains(t, target, chat.Title, + "ResourceTarget for Chat must not contain the title; it should use a UUID prefix") +} diff --git a/enterprise/audit/diff_internal_test.go b/enterprise/audit/diff_internal_test.go index afbd1b3784..5e8e30492d 100644 --- a/enterprise/audit/diff_internal_test.go +++ b/enterprise/audit/diff_internal_test.go @@ -367,6 +367,29 @@ func Test_diff(t *testing.T) { }, }) + runDiffTests(t, []diffTest{ + { + // Chat titles can contain sensitive content, so they must be + // masked in audit diffs via ActionSecret. This case guards + // against a regression where title is flipped back to + // ActionTrack in enterprise/audit/table.go. + name: "TitleMasked", + left: audit.Empty[database.Chat](), + right: database.Chat{ + ID: uuid.UUID{1}, + OwnerID: uuid.UUID{2}, + WorkspaceID: uuid.NullUUID{UUID: uuid.UUID{3}, Valid: true}, + Title: "a very secret chat title", + }, + exp: audit.Map{ + "id": audit.OldNew{Old: "", New: uuid.UUID{1}.String()}, + "owner_id": audit.OldNew{Old: "", New: uuid.UUID{2}.String()}, + "workspace_id": audit.OldNew{Old: "null", New: uuid.UUID{3}.String()}, + "title": audit.OldNew{Old: "", New: "", Secret: true}, + }, + }, + }) + runDiffTests(t, []diffTest{ { name: "Create", diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 890f3aa9e8..7b3354d6c5 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -386,18 +386,18 @@ var auditableResourcesTypes = map[any]map[string]Action{ "workspace_id": ActionTrack, "build_id": ActionIgnore, // Internal lifecycle. "agent_id": ActionIgnore, // Internal lifecycle. - "title": ActionSecret, + "title": ActionSecret, // May contain sensitive content. "status": ActionIgnore, // Churns every message. "worker_id": ActionIgnore, // Internal. "started_at": ActionIgnore, - "heartbeat_at": ActionIgnore, + "heartbeat_at": ActionIgnore, // Internal. "created_at": ActionIgnore, // Never changes. "updated_at": ActionIgnore, // Bumped on every mutation. "parent_chat_id": ActionIgnore, // Immutable after creation. "root_chat_id": ActionIgnore, // Immutable after creation. "last_model_config_id": ActionIgnore, // Churns every message. "archived": ActionTrack, - "last_error": ActionIgnore, + "last_error": ActionIgnore, // Internal. "mode": ActionTrack, "mcp_server_ids": ActionTrack, "labels": ActionTrack,