Files
pad/internal/server
xarmian 8e16501e8a fix(security): scope webhook + token mutations by workspace to close cross-workspace IDOR (TASK-266) (#936)
* fix(security): scope webhook + token mutations by workspace to close cross-workspace IDOR (TASK-266)

handleDeleteWebhook, handleTestWebhook, and handleDeleteToken looked their
object up by ID with no workspace-ownership predicate. requireMinRole("owner")
only proves the caller owns the URL's workspace — not that the {webhookID} /
{tokenID} belongs to it — so any owner of any workspace could delete or test
another workspace's webhook, or revoke its API token, given the object ID
(cross-workspace IDOR / integrity + DoS + existence oracle).

Fix, matching the existing pre-fetch-and-compare idiom used by
handleDeleteWorkspaceAttachment / views / comments / links:
- webhooks: pre-fetch via GetWebhook and 404 unless hook.WorkspaceID matches
  the URL workspace (delete + test paths).
- tokens: new store.DeleteAPITokenScoped(id, workspaceID) doing
  DELETE ... WHERE id = ? AND workspace_id = ?, mirroring DeleteUserAPIToken.
  The unscoped DeleteAPIToken (its only caller) is removed.

Adds TestWebhookTokenCrossWorkspaceIDOR: an owner of workspace B cannot
delete/test A's webhook or revoke A's token via B's URL (404, objects survive),
while the legitimate owner still can within their own workspace. Verified
red-before/green-after.

Part of PLAN-259 (pre-open-source security audit). Closes the webhook+token
half of TASK-266; views/comments/links were already scoped.

Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra

* fix(security): atomic workspace-scoped webhook delete per Codex review (round 1)

handleDeleteWebhook pre-fetched via GetWebhook, which decrypts the HMAC
secret. A rotated/missing encryption key or corrupted ciphertext would make
the delete return 500, leaving a broken webhook undeletable. Replace the
pre-fetch-and-compare with an atomic store.DeleteWebhookScoped(id, workspaceID)
(DELETE ... WHERE id = ? AND workspace_id = ?) — same idiom as the token fix,
no decrypt on the delete path. The unscoped DeleteWebhook (its only caller) is
removed. handleTestWebhook keeps GetWebhook since it needs the decrypted hook
to dispatch.

Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra

* fix(security): scope test-webhook lookup before decrypt per Codex review (round 2)

handleTestWebhook fetched via GetWebhook (SELECT + decrypt by ID) and only then
compared workspace, so an undecryptable foreign webhook returned 500 rather than
404 — a residual cross-workspace existence oracle. Add store.GetWebhookScoped(id,
workspaceID) which applies the workspace_id predicate in SQL before decrypting;
a foreign/missing ID returns (nil,nil) → 404 without touching the ciphertext.
Strengthen the regression test's victim webhook with a non-empty secret so the
scoped-before-decrypt path is exercised.

Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra
2026-07-14 23:24:55 -04:00
..