Repository navigation
feat(security): audit account routes + allowlist cleanup - #887
Conversation
|
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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThese changes add audit logging to account-related API endpoints (drives status and verification checks) and remove audit exemptions for previously exempt routes, requiring them to implement audit instrumentation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bfeece2d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ['ai/ollama/models', 'Local Ollama model discovery, no user data'], | ||
| ['ai/lmstudio/models', 'Local LMStudio model discovery, no user data'], | ||
|
|
||
| // --- Drive sub-routes (read-only data fetches, covered by parent drive audit) --- | ||
| // TODO: Add audit coverage in follow-up PR | ||
| ['drives/[driveId]/access', 'Read-only access check — follow-up'], | ||
| ['drives/[driveId]/agents', 'Agent list for drive — follow-up'], | ||
| ['drives/[driveId]/assignees', 'Assignee list for drive — follow-up'], | ||
| ['drives/[driveId]/history', 'Drive history view — follow-up'], | ||
| ['drives/[driveId]/integrations', 'Integration list for drive — follow-up'], | ||
| ['drives/[driveId]/integrations/audit', 'Integration audit log — follow-up'], | ||
| ['drives/[driveId]/pages', 'Page list for drive — follow-up'], | ||
| ['drives/[driveId]/permissions-tree', 'Permissions tree view — follow-up'], | ||
| ['drives/[driveId]/search/glob', 'Glob search within drive — follow-up'], | ||
| ['drives/[driveId]/search/regex', 'Regex search within drive — follow-up'], | ||
| ['drives/[driveId]/trash', 'Trash list for drive — follow-up'], | ||
|
|
||
| // --- Page sub-routes (read-only data fetches, covered by parent page audit) --- | ||
| // TODO: Add audit coverage in follow-up PR | ||
| ['pages/[pageId]/agent-config', 'Page agent config — follow-up'], | ||
| ['pages/[pageId]/ai-usage', 'AI usage stats — follow-up'], | ||
| ['pages/[pageId]/breadcrumbs', 'Breadcrumb navigation — follow-up'], | ||
| ['pages/[pageId]/children', 'Child page list — follow-up'], | ||
| ['pages/[pageId]/history', 'Page history view — follow-up'], | ||
| ['pages/[pageId]/permissions/check', 'Permission check — follow-up'], | ||
| ['pages/[pageId]/processing-status', 'Processing status poll — follow-up'], | ||
| ['pages/[pageId]/reprocess', 'Reprocess trigger — follow-up'], | ||
| ['pages/[pageId]/tasks/[taskId]', 'Individual task CRUD — follow-up'], | ||
| ['pages/[pageId]/tasks/reorder', 'Task reorder — follow-up'], | ||
| ['pages/[pageId]/tasks/statuses', 'Task status list — follow-up'], | ||
| ['pages/[pageId]/versions/compare', 'Version comparison — follow-up'], | ||
| ['pages/[pageId]/view', 'Page view endpoint — follow-up'], | ||
| ['pages/tree', 'Page tree navigation — follow-up'], | ||
|
|
||
| // --- Account sub-routes (status checks) --- | ||
| // TODO: Add audit coverage in follow-up PR | ||
| ['account/drives-status', 'Drive status check — follow-up'], | ||
| ['account/verification-status', 'Email verification status — follow-up'], | ||
|
|
||
| // --- Monitoring with admin auth (already audited via withAdminAuth wrapper) --- | ||
| ['monitoring/[metric]', 'Uses withAdminAuth which includes audit — verify after merge'], | ||
| ]); |
There was a problem hiding this comment.
This change removes the drive/page follow-up entries from AUDIT_EXEMPT_ROUTES, but this commit only adds logAuditEvent to two account routes; at this snapshot, the coverage gate still finds 25 unaudited non-exempt routes (for example drives/[driveId]/access, pages/[pageId]/view, and pages/tree). Running security-audit-coverage.test.ts on commit 63f88df therefore fails immediately, so the branch is not green unless those other route handlers are updated in the same change.
Useful? React with 👍 / 👎.
Add SecurityAuditService coverage to account/drives-status and account/verification-status routes. Remove all 34 TODO/follow-up entries from AUDIT_EXEMPT_ROUTES allowlist (drives, pages, account, monitoring sections) now that sibling PRs provide actual coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
0bfeece to
a92c281
Compare
This branch was previously deployed
1 participant
Summary
Test plan
pnpm --filter web vitest run src/app/api/__tests__/security-audit-coverage.test.tspasses (after sibling PRs merge)Note: This PR must merge after the 3 sibling audit PRs.
🤖 Generated with Claude Code
Summary by CodeRabbit