Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 10 additions & 16 deletions backend/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
56 changes: 56 additions & 0 deletions backend/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
13 changes: 11 additions & 2 deletions plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -122,7 +130,8 @@
{
"point": "project.settings.tab",
"component": "HelloProjectSettingsTab",
"order": 10
"order": 10,
"requiredPermission": "example.manage"
},
{
"point": "view",
Expand Down
Loading