Repository navigation
test: add regression guard for chat title masking - #24584
Conversation
Chat titles are masked in audit diffs via ActionSecret (PR #24564). Add a Test_diff case so flipping title back to ActionTrack fails loudly, and document the rationale inline in coderd/audit/request.go and enterprise/audit/table.go. Coder Agents generated.
There was a problem hiding this comment.
Clean follow-up that does exactly what it promises: a regression test for the ActionSecret annotation on Chat.title, plus two inline comments documenting the security rationale. The test is solid, exercises the real production audit table via diffValues, and catches both the ActionTrack and ActionIgnore regression vectors in a single assertion.
1 P3, 2 Nit. No blockers.
The commit scope enterprise/audit doesn't cover coderd/audit/request.go. The PR title correctly omits scope, so this only affects git log readers. The commit subject "regression-guard" reads better in imperative: "add regression guard for chat title masking."
"Two failure modes guarded with one assertion. That's efficient." — Bisky
🤖 This review was automatically generated with Coder Agents.
Follow-up to #24564 addressing unresolved review findings.
Test_diff/Chat/TitleMaskedtoenterprise/audit/diff_internal_test.goso flippingtitleback toActionTrackfails loudly. Verified: the case passes today, fails with a clear diff after flipping toActionTrack, passes again after reverting.coderd/audit/request.go:138explaining whyResourceTargetfordatabase.Chatreturns a UUID prefix instead of the title.enterprise/audit/table.gotitleentry, matching the surroundingActionSecretcomment style.Won't-fix, with rationale (per user):
resource_targetis a display hint, not an identifier; the full UUID lives inresource_id.[:8]): single call site; extracting would be ceremony.Implementation plan
See
.agents/plans/derem-24564-followups.mdin the branch history (not committed).Key insight from the refine-plan pass: a first-draft test in
coderd/exp_chats_test.gowould not have guarded the regression becauseMockAuditor.diffis a no-op stub. The real diff logic lives inenterprise/audit/diff.go::diffValues, andTest_diffis the right home, next to the existingGitSSHKey.private_keyandUser.hashed_passwordmasking cases.