Enforce note ownership check in DeleteBlock

[egeozcan]

Fixes a vulnerability where DeleteBlock API callers could bypass the parent note context verification by omitting noteId in the request parameters.

---

PR created automatically by Jules for task 2895326477337872354 started by @egeozcan

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[egeozcan]

Review and fixes (pushed as 79eafe3; base was already current master, no rebase needed).

Findings, all fixed:

• mr note-block delete <id> sent no noteId and would now fail with 400. It now takes a required --note-id; help text, generated CLI docs and the CLI E2E spec are updated.
• public/dist/main.js was not rebuilt, so the shipped block editor still did not send noteId on delete. Rebuilt.
• OpenAPI (deleteBlock, deleteBlockPost) and docs-site pages did not mark noteId required (TestCommittedOpenAPISpecIsFresh was failing after the handler change). Updated and regenerated.
• Callers/tests that deleted without noteId: e2e apiClient.deleteBlock (now resolves noteId from the block), block_api_test.go, delete_block_mention_sync_test.go, status_code_consistency_test.go. Fixed; stale 'verify the damage' comments in the cross-note test tidied.
• Added a test that the POST /v1/note/block/delete form route enforces the same rule (missing and foreign noteId, then success).

Checked, no change needed: a scoped principal gets 404 from GetBlock/DeleteBlock via NoteVisible; a block's note_id is immutable so the check-then-delete has no race, and no Postgres-specific behaviour.

Overlap with #73: it edits the same handler (DeleteBlockHandler and the Update/State handlers), blockEditor.js and the same api_tests files (block_api_test.go, delete_block_mention_sync_test.go, status_code_consistency_test.go), so whichever merges second will conflict. #73 is a superset on the handler. It also does not touch the CLI, OpenAPI, docs, e2e client or dist, so those fixes from this PR are still needed if #73 lands. Suggest merging one, then rebasing the other (rebuild public/dist once).

[google-labs-jules]

Review and fixes (pushed as 79eafe3; base was already current master, no rebase needed).

Findings, all fixed:

• mr note-block delete <id> sent no noteId and would now fail with 400. It now takes a required --note-id; help text, generated CLI docs and the CLI E2E spec are updated.
• public/dist/main.js was not rebuilt, so the shipped block editor still did not send noteId on delete. Rebuilt.
• OpenAPI (deleteBlock, deleteBlockPost) and docs-site pages did not mark noteId required (TestCommittedOpenAPISpecIsFresh was failing after the handler change). Updated and regenerated.
• Callers/tests that deleted without noteId: e2e apiClient.deleteBlock (now resolves noteId from the block), block_api_test.go, delete_block_mention_sync_test.go, status_code_consistency_test.go. Fixed; stale 'verify the damage' comments in the cross-note test tidied.
• Added a test that the POST /v1/note/block/delete form route enforces the same rule (missing and foreign noteId, then success).

Checked, no change needed: a scoped principal gets 404 from GetBlock/DeleteBlock via NoteVisible; a block's note_id is immutable so the check-then-delete has no race, and no Postgres-specific behaviour.

Overlap with #73: it edits the same handler (DeleteBlockHandler and the Update/State handlers), blockEditor.js and the same api_tests files (block_api_test.go, delete_block_mention_sync_test.go, status_code_consistency_test.go), so whichever merges second will conflict. #73 is a superset on the handler. It also does not touch the CLI, OpenAPI, docs, e2e client or dist, so those fixes from this PR are still needed if #73 lands. Suggest merging one, then rebasing the other (rebuild public/dist once).

Acknowledged. I see that the necessary fixes for the CLI, OpenAPI spec, E2E client, and tests have been pushed directly. Since this comment is informational and closes out the work with the mentioned fixes, no further action is required from me at this time. I am stopping work on this task.

[egeozcan]

Closing: superseded by #73 (merged), which requires noteId on all block mutation handlers including DeleteBlock.

[google-labs-jules]

Closing: superseded by #73 (merged), which requires noteId on all block mutation handlers including DeleteBlock.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.