Repository navigation
fix(api): only allow bookmark ids in DELETE /api/bookmarks/:id - #76
Conversation
The id went straight into the daily.dev URL, so a decoded `lists%2F<id>` or `..%2Ffeeds%2Fcustom%2F<id>` reached other Public API endpoints with the user's own token. The route now rejects ids outside [A-Za-z0-9_-] with a 400, and deleteBookmark() encodes the id as well. Part of #74. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🔇 Additional comments (3)
📝 SummarySummary by CodeRabbit
WalkthroughThe DELETE route now rejects missing bookmark IDs and IDs outside the allowed character pattern. The client URL-encodes IDs in DELETE paths. An end-to-end test checks 400 responses for invalid IDs. ChangesBookmark DELETE ID handling
Priority: ⬆️ High Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to Malformed dot-segment IDs are rejected before the external deletion request. No material merge-blocking behavior is established, so the change appears ready for normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the path at night Comment |
Summary
Part of #74 (reported by the daily.dev team while reviewing the marketplace submission).
DELETE /api/bookmarks/:idnow rejects any id that isn't^[\w-]+$with400 Invalid bookmark id, before anything is sent to daily.dev.deleteBookmark()also wraps the id inencodeURIComponent, so the client can't reach another endpoint even if a caller skips the route check.Before,
DELETE /api/bookmarks/lists%2F<id>reachedDELETE /public/v1/bookmarks/lists/<id>(and..%2Freached other resources) with the signed-in user's own token.Test plan
tests/e2e/api-bookmarks.spec.ts:lists%2F…,..%2Ffeeds%2F…,?and.ids all get 400. Without the route check they were forwarded to daily.dev (502)pnpm lint,pnpm format:check,pnpm build,pnpm test(32 passed)🤖 Generated with Claude Code