diff --git a/cmd/pad/chatgpt_projection_test.go b/cmd/pad/chatgpt_projection_test.go index d478017a9..e8e5f118a 100644 --- a/cmd/pad/chatgpt_projection_test.go +++ b/cmd/pad/chatgpt_projection_test.go @@ -37,12 +37,19 @@ type projectionFixture struct { } func newProjectionFixture(t *testing.T) projectionFixture { + t.Helper() + return newProjectionFixtureNamed(t, "Olivia Owner") +} + +// newProjectionFixtureNamed is the fixture with the owner's display name set +// to ownerName (an email-shaped one exercises the name mask). +func newProjectionFixtureNamed(t *testing.T, ownerName string) projectionFixture { t.Helper() s := storetest.NewSQLite(t) api := padserver.New(s) t.Cleanup(api.Stop) - owner, err := s.CreateUser(models.UserCreate{Email: "owner@example.com", Name: "Olivia Owner", Password: "correct-horse-battery-staple"}) + owner, err := s.CreateUser(models.UserCreate{Email: "owner@example.com", Name: ownerName, Password: "correct-horse-battery-staple"}) if err != nil { t.Fatal(err) } @@ -93,11 +100,11 @@ func newProjectionFixture(t *testing.T) projectionFixture { if _, err := s.CreateItemLink(ws.ID, models.ItemLinkCreate{TargetID: t2.ID, LinkType: "blocks"}, t1.ID); err != nil { t.Fatal(err) } - c, err := s.CreateComment(ws.ID, t1.ID, owner.ID, models.CommentCreate{Body: "Seen in prod.", Author: "Olivia Owner"}) + c, err := s.CreateComment(ws.ID, t1.ID, owner.ID, models.CommentCreate{Body: "Seen in prod.", Author: ownerName}) if err != nil { t.Fatal(err) } - if _, err := s.CreateComment(ws.ID, t1.ID, owner.ID, models.CommentCreate{Body: "Fix incoming.", Author: "Olivia Owner", ParentID: c.ID}); err != nil { + if _, err := s.CreateComment(ws.ID, t1.ID, owner.ID, models.CommentCreate{Body: "Fix incoming.", Author: ownerName, ParentID: c.ID}); err != nil { t.Fatal(err) } @@ -171,7 +178,14 @@ func leaks(path string, v any, allowedIDKeys map[string]bool) []string { } func TestChatGPTProjection_NoIdentifiersOrEmailsLeak(t *testing.T) { - f := newProjectionFixture(t) + // Twice: with an ordinary display name, and with one that IS an email + // address (a user can set that), which every person field must mask. + for _, name := range []string{"Olivia Owner", "olivia.owner@example.com"} { + t.Run(name, func(t *testing.T) { sweepAllTools(t, newProjectionFixtureNamed(t, name)) }) + } +} + +func sweepAllTools(t *testing.T, f projectionFixture) { ws := f.ws calls := []struct { tool string @@ -321,3 +335,20 @@ func TestChatGPTProjection_ErrorPathsDoNotLeak(t *testing.T) { } var uuidAnywhere = regexp.MustCompile(`[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`) + +// item_history names who made each change (the display name #1714 added), +// and masks a name that is an email address. +func TestChatGPTProjection_HistoryNamesTheEditor(t *testing.T) { + for name, want := range map[string]string{"Olivia Owner": "Olivia Owner", "olivia.owner@example.com": "a Pad user"} { + f := newProjectionFixtureNamed(t, name) + f.call(t, "update_item", map[string]any{"workspace": f.ws, "ref": f.task1, "content": "Edited through ChatGPT."}) + _, v := f.call(t, "item_history", map[string]any{"workspace": f.ws, "ref": f.task1}) + rows, _ := v.(map[string]any)["items"].([]any) + if len(rows) == 0 { + t.Fatalf("%s: no history rows", name) + } + if got := rows[0].(map[string]any)["actor_name"]; got != want { + t.Errorf("owner %q: newest row actor_name = %v, want %q", name, got, want) + } + } +} diff --git a/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json b/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json index 5a0460889..750353269 100644 --- a/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json +++ b/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json @@ -237,7 +237,7 @@ "idempotentHint": true, "openWorldHint": false }, - "description": "List an item's saved versions, newest first: when each was saved, and whether a person, an agent or ChatGPT made it.", + "description": "List an item's saved versions, newest first: when each was saved and who made it, with whether that was a person, an agent or ChatGPT.", "inputSchema": { "properties": { "limit": { diff --git a/internal/mcp/chatgpt_catalog.go b/internal/mcp/chatgpt_catalog.go index fea2bb2e5..fe85649b5 100644 --- a/internal/mcp/chatgpt_catalog.go +++ b/internal/mcp/chatgpt_catalog.go @@ -225,7 +225,7 @@ var ChatGPTCatalog = []ChatGPTTool{ }, { Name: "item_history", - Description: "List an item's saved versions, newest first: when each was saved, and whether a person, an agent or ChatGPT made it.", + Description: "List an item's saved versions, newest first: when each was saved and who made it, with whether that was a person, an agent or ChatGPT.", Source: ChatGPTSource{"pad_item", "history"}, Params: []string{chatGPTWorkspaceParam, "ref", "limit"}, Required: []string{chatGPTWorkspaceParam, "ref"}, diff --git a/internal/mcp/chatgpt_project.go b/internal/mcp/chatgpt_project.go index e956357d2..79c45242f 100644 --- a/internal/mcp/chatgpt_project.go +++ b/internal/mcp/chatgpt_project.go @@ -189,7 +189,7 @@ var chatGPTResponseShapes = map[string]shape{ "restore_item": itemShape, "add_comment": commentShape, "list_comments": shapeList(commentShape), - "item_history": shapeList(shapeKeys("created_at", "created_by", "source", "change_summary")), + "item_history": shapeList(shapeKeys("created_at", "created_by", "source", "change_summary", "actor_name")), "item_dependencies": shapeList(linkShape), "link_items": linkShape, "project_dashboard": dashboardShape, @@ -239,6 +239,7 @@ func projectChatGPTResult(t ChatGPTTool, in map[string]any, res *CallToolResult) } projected := s.apply(value) rewriteRelationValues(projected, value) + maskEmailShapedNames(projected, "") b, err := json.Marshal(projected) if err != nil { return NewErrorResult(ErrorPayload{Code: ErrServerError, Message: "could not encode the response"}) @@ -457,3 +458,51 @@ func projectChatGPTError(res *CallToolResult) *CallToolResult { out.IsError = true return out } + +// personNameKeys are the keys whose values are a person's display name. +// "name" counts only inside a "user" object; elsewhere it names a +// workspace, collection or role. +var personNameKeys = map[string]bool{ + "author": true, "agent_name": true, "actor_name": true, "assigned_user": true, + "assigned_user_name": true, "owner_username": true, "assigned_users": true, +} + +// emailLike matches a value containing an email address. +var emailLike = regexp.MustCompile(`[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}`) + +// maskedPersonName replaces a display name that is (or contains) an email +// address. A user can set their display name to their email, and a name is +// shown wherever a person is; OpenAI's review forbids emails in responses. +// It is not replaced by the local part, which would be a new identifier +// (lead's ruling on TASK-3321). +const maskedPersonName = "a Pad user" + +// maskEmailShapedNames replaces email-shaped person names in place. +func maskEmailShapedNames(v any, parent string) { + switch t := v.(type) { + case map[string]any: + for k, e := range t { + isName := personNameKeys[k] || (k == "name" && parent == "user") + if isName { + switch x := e.(type) { + case string: + if emailLike.MatchString(x) { + t[k] = maskedPersonName + } + case []any: + for i, el := range x { + if s, ok := el.(string); ok && emailLike.MatchString(s) { + x[i] = maskedPersonName + } + } + } + continue + } + maskEmailShapedNames(e, k) + } + case []any: + for _, e := range t { + maskEmailShapedNames(e, parent) + } + } +} diff --git a/internal/mcp/chatgpt_project_test.go b/internal/mcp/chatgpt_project_test.go index 36b5534a0..a96b3c9e4 100644 --- a/internal/mcp/chatgpt_project_test.go +++ b/internal/mcp/chatgpt_project_test.go @@ -112,3 +112,25 @@ func TestChatGPTProjection_TextFallbackIsProjected(t *testing.T) { t.Errorf("text fallback not projected: %s", text) } } + +// A display name that is an email address reaches ChatGPT as "a Pad user" +// in every person field, and nowhere else is a name touched (TASK-3321). +func TestChatGPTProjection_EmailShapedNamesAreMasked(t *testing.T) { + v := map[string]any{ + "user": map[string]any{"name": "jane@example.com"}, + "workspace": map[string]any{"name": "Acme Launch"}, + "author": "jane@example.com", + "actor_name": "Jane Doe", + "by_role": []any{map[string]any{"role_name": "Dev", "assigned_users": []any{"jane@example.com", "Sam"}}}, + } + maskEmailShapedNames(v, "") + b, _ := json.Marshal(v) + if strings.Contains(string(b), "@") { + t.Errorf("an email-shaped name survived: %s", b) + } + for _, keep := range []string{"Acme Launch", "Jane Doe", `"Sam"`} { + if !strings.Contains(string(b), keep) { + t.Errorf("an ordinary name %s was masked: %s", keep, b) + } + } +}