Conversation
📝 WalkthroughWalkthroughThe change reworks ChangesURL Marker Matching Hardening
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/drive/drive_add_comment.go (1)
1262-1280: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSolid hardening — path-anchored matching with scheme/host validation.
The rewrite correctly rejects non-URL inputs (no scheme/host), and since
u.Pathexcludes query/fragment, markers can no longer be spoofed via?next=/wiki/...style strings. RequiringHasPrefix(path, marker)also prevents markers from matching mid-path (e.g./space/wiki/...), which is confirmed intentional by the new test cases indrive_add_comment_test.go.One optional nit: consider adding a short doc comment above
extractURLTokendocumenting the new invariants (absolute URL required; marker must anchor at path start) — mirrors the comment update already made forpermApplyURLMarkersindrive_apply_permission.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/drive/drive_add_comment.go` around lines 1262 - 1280, Add a short doc comment for extractURLToken describing its invariants: it only accepts absolute URLs with scheme and host, and the marker must match at the start of u.Path. Keep the comment aligned with the updated behavior in extractURLToken and the related documentation style used for permApplyURLMarkers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@shortcuts/drive/drive_add_comment.go`:
- Around line 1262-1280: Add a short doc comment for extractURLToken describing
its invariants: it only accepts absolute URLs with scheme and host, and the
marker must match at the start of u.Path. Keep the comment aligned with the
updated behavior in extractURLToken and the related documentation style used for
permApplyURLMarkers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 577994f2-65e5-419e-b41e-8487d766217c
📒 Files selected for processing (4)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goshortcuts/drive/drive_apply_permission.goshortcuts/drive/drive_apply_permission_test.go
Summary
Drive URL token inference now parses the URL path rather than scanning raw input, so query strings and non-URL paths cannot spoof supported Drive resource markers.
Changes
Test Plan
Related Issues
Summary by CodeRabbit