diff --git a/backend/plugin.go b/backend/plugin.go index d7a0c85..693193f 100644 --- a/backend/plugin.go +++ b/backend/plugin.go @@ -225,30 +225,24 @@ func (p *examplePlugin) deleteHello(req *plugin.Request, res *plugin.Response) { return } - rows, err := p.db.Query( - "SELECT id, project_id, task_id, name, message, created_by, created_at, updated_at FROM hello_messages WHERE id = $1", - id, + // Scoped by project_id directly in the DELETE itself, rather than a + // separate SELECT-then-check: returning a distinguishable 403 for "this + // id exists but isn't yours" vs. 404 for "this id doesn't exist" is an + // existence oracle, letting a caller enumerate valid ids across every + // project. A foreign id and a nonexistent id must look identical. + affected, err := p.db.Exec( + "DELETE FROM hello_messages WHERE id = $1 AND project_id = $2", + id, req.Caller.ProjectID, ) if err != nil { - res.Error(500, "failed to read hello message") + res.Error(500, "failed to delete hello message") return } - if len(rows.Rows) == 0 { + if affected == 0 { res.Error(404, "hello message not found") return } - msg := rowToMessage(rows.Rows[0]) - if msg.ProjectID != req.Caller.ProjectID { - res.Error(403, "hello message belongs to a different project") - return - } - - if _, err := p.db.Exec("DELETE FROM hello_messages WHERE id = $1", id); err != nil { - res.Error(500, "failed to delete hello message") - return - } - res.NoContent() } diff --git a/backend/plugin_test.go b/backend/plugin_test.go index 03756e1..21c51f8 100644 --- a/backend/plugin_test.go +++ b/backend/plugin_test.go @@ -107,6 +107,62 @@ func TestUpdateAndDeleteHello(t *testing.T) { } } +// TestDeleteHello_ForeignProjectMatchesNonexistentResponse pins the fix for +// an existence oracle: deleteHello used to return a distinguishable 403 for +// "this id exists but belongs to another project" vs. 404 for "this id +// doesn't exist at all", letting a caller enumerate valid ids across every +// project one probe at a time. Both cases must now look identical. +func TestDeleteHello_ForeignProjectMatchesNonexistentResponse(t *testing.T) { + tc := setupPlugin(t) + + create := tc.Call("POST", "/hello", plugintest.Request{ + Caller: plugin.CallerIdentity{ProjectID: "other-project", CallerID: "member-2", CallerRole: "PROJECT_MEMBER"}, + }.WithJSONBody(map[string]any{"name": "Not yours"})) + if create.StatusCode != 201 { + t.Fatalf("expected 201, got %d: %s", create.StatusCode, create.BodyString()) + } + var createEnv struct { + Data helloMessage `json:"data"` + } + _ = json.Unmarshal(create.Body, &createEnv) + + foreign := tc.Call("DELETE", "/hello/:id", plugintest.Request{ + Caller: req().Caller, + PathParams: map[string]string{"id": createEnv.Data.ID}, + }) + nonexistent := tc.Call("DELETE", "/hello/:id", plugintest.Request{ + Caller: req().Caller, + PathParams: map[string]string{"id": "does-not-exist"}, + }) + + if foreign.StatusCode != 404 { + t.Fatalf("expected 404 for a foreign-project id, got %d: %s", foreign.StatusCode, foreign.BodyString()) + } + if foreign.StatusCode != nonexistent.StatusCode || foreign.BodyString() != nonexistent.BodyString() { + t.Fatalf("foreign-project and nonexistent responses must be identical: foreign=%d %q, nonexistent=%d %q", + foreign.StatusCode, foreign.BodyString(), nonexistent.StatusCode, nonexistent.BodyString()) + } + + // And the foreign message must genuinely survive (not actually deleted). + list := tc.Call("GET", "/hello", plugintest.Request{ + Caller: plugin.CallerIdentity{ProjectID: "other-project", CallerID: "member-2", CallerRole: "PROJECT_MEMBER"}, + Query: map[string]string{}, + }) + var listEnv struct { + Data []helloMessage `json:"data"` + } + _ = json.Unmarshal(list.Body, &listEnv) + found := false + for _, m := range listEnv.Data { + if m.ID == createEnv.Data.ID { + found = true + } + } + if !found { + t.Fatal("foreign message was deleted despite the 404") + } +} + func TestTaskDeletedEventRemovesTaskMessages(t *testing.T) { tc := setupPlugin(t) diff --git a/plugin.json b/plugin.json index 5183897..2acf2ff 100644 --- a/plugin.json +++ b/plugin.json @@ -2,9 +2,17 @@ "id": "com.paca.example", "displayName": "Plugin SDK Hello World", "description": "Hello world examples for every Paca plugin SDK feature.", - "version": "0.1.2", + "version": "0.1.3", "minCoreVersion": "v0.13.3", "permissions": ["db.read", "db.write", "events.subscribe", "events.emit"], + "customPermissions": [ + { + "key": "example.manage", + "label": "Manage Hello settings", + "description": "View this project's Hello Project Settings tab. Demonstrates a plugin-specific custom permission gating a project.settings.tab registration — does not affect the sidebar/task-detail/view Hello surfaces, which stay visible to any project member on purpose (the backend read routes they share with this settings tab are intentionally left ungated for that reason). Creating, editing, or deleting a Hello message from any of those surfaces still requires the built-in projects.write (Owner/Manager-tier), same as this settings tab's own write actions — an ordinary member can view but not write, on every surface alike.", + "scope": "project" + } + ], "backend": { "eventSubscriptions": ["task.deleted"], "routes": [ @@ -122,7 +130,8 @@ { "point": "project.settings.tab", "component": "HelloProjectSettingsTab", - "order": 10 + "order": 10, + "requiredPermission": "example.manage" }, { "point": "view",