From 2dc5ac7058efb6ae0ff476ad46dddaa06259a9de Mon Sep 17 00:00:00 2001 From: xarmian Date: Fri, 2 Oct 2026 02:42:25 -0400 Subject: [PATCH] feat(mcp): ChatGPT item_history names who made each change; email-shaped names are masked (TASK-3321) (#1716) #1714 put actor_name on every history summary row. The ChatGPT item_history allow-list now keeps it, and the description says "who" again. Per the lead's ruling, a display name that is (or contains) an email address reaches ChatGPT as "a Pad user" in every person field (author, agent_name, actor_name, assigned_user(_name), owner_username, assigned_users, and user.name), never as the local part, which would be a new identifier. Other names (workspace, collection, role) are untouched. The leak sweep now runs twice, the second time with an owner whose display name is an email address. It fails with the mask off, as does the history leg. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC --- cmd/pad/chatgpt_projection_test.go | 39 ++++++++++++-- .../testdata/mcp_wire/chatgpt_tools_list.json | 2 +- internal/mcp/chatgpt_catalog.go | 2 +- internal/mcp/chatgpt_project.go | 51 ++++++++++++++++++- internal/mcp/chatgpt_project_test.go | 22 ++++++++ 5 files changed, 109 insertions(+), 7 deletions(-) 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) + } + } +}