mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
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
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -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": {
|
||||
|
||||
@@ -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"},
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user