Security Hardening + AI Pentest Framework#31
Conversation
…ulnerability report SECURITY CHANGES (convex/webhooks.ts): - listFailedIntegrationEvents: query → internalQuery - replayIntegrationEvent: mutation → internalMutation - replayFailedIntegrationEvents: mutation → internalMutation These admin functions were accidentally exposed to external clients. They are now internal-only, callable only from other Convex functions. FULL REPORT (PR-SECURITY-HARDENING-2026-03-26.md): Comprehensive security audit covering: - CRITICAL: 6 IDOR vulnerabilities in calendar/notifications - HIGH: 6 vulnerabilities (Math.random, GPS exposure, auth gaps) - MEDIUM: 18 issues (zero rate limiting, schema permissions missing) - LOW: 16 issues Audit by: 10 parallel subagents (5 preliminary + 5 deep-dive) Method: Passive static analysis Files audited: All convex/*.ts, key src/ files
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Roast 🔥
Verdict: 1 Issue Found | Recommendation: Address before merge
Overview
| Severity | Count |
|---|---|
| 🚨 critical | 0 |
| 1 | |
| 💡 suggestion | 0 |
| 🤏 nitpick | 0 |
Issue Details (click to expand)
| File | Line | Roast |
|---|---|---|
PR-SECURITY-HARDENING-2026-03-26.md |
- | Security theater: 3 fixes for 46 documented vulnerabilities |
🔥 The Roast: This PR has better documentation than code changes — 1,426 lines of vulnerability reports for a grand total of 3 one-line type changes. The PR claims "Security Hardening" but it's more like "Security Hardening Brochure." You documented 6 CRITICAL IDOR vulnerabilities (including exposing OAuth tokens for any user), 6 HIGH severity issues (like Math.random() for session tokens), and 18 MEDIUM issues — then fixed exactly 3 functions to use internalQuery/internalMutation instead of query/mutation.
Oh wait, those 3 changes ARE good. Making listFailedIntegrationEvents, replayIntegrationEvent, and replayFailedIntegrationEvents internal-only is the right call. But if someone reads this PR title and merges it thinking "security hardened," they've got a false sense of security while 43 vulnerabilities still exist.
🩹 The Fix:
Option A (Honest PR): Rename this PR to "Security Audit Report + 3 Quick Wins" and make it clear the remaining 43 vulnerabilities need separate PRs.
Option B (Actually Hardened): Fix all 6 CRITICAL IDOR vulnerabilities in this PR before merging. CR-1 through CR-6 are the real doozies — they allow any authenticated user to fetch OAuth tokens, push tokens, and full schedules of any other user. That's not theoretical; that's CVSS 8-9 territory.
📏 Severity: warning
🏆 Best part: The 3 code changes that WERE made (query→internalQuery, mutation→internalMutation for webhook admin functions) are correct and targeted. Whoever reviewed these knew what they were doing.
💀 Worst part: The PR title promises "46 Vulnerabilities" fixed, but only 3 actual code changes exist. This creates a dangerous "check the box" illusion where someone could think security work is done when the hard stuff (IDOR authorization checks, rate limiting, GPS exposure removal) hasn't been touched.
📊 Overall: Like finding a 50-page car repair estimate and then only changing the windshield wipers. The diagnosis is excellent; the actual repairs are left for another day.
Files Reviewed (2 files)
PR-SECURITY-HARDENING-2026-03-26.md- Documentation only (audit report)convex/webhooks.ts- 3 actual code changes (good fixes)
Adds comprehensive pentest infrastructure for AI agents to actively scan and exploit vulnerabilities against a staging Convex deployment. NEW FILES (.agents/pentest/): - harnesses/convex-agent.ts — Convex API client for AI agents - harnesses/attack-suite.ts — Attack templates (IDOR, rate-limit, auth, etc.) - index.ts — CI/CD pentest runner - inventory/CONVEX_INVENTORY.md — Pre-built attack surface inventory - templates/ — Attack templates per vulnerability class ATTACK CLASSES: - IDOR attacks: OAuth token theft, push token theft, schedule recon - Rate limiting: OTP flooding, job spam, notification flooding - Auth bypass: Missing auth checks, role confusion - Data exposure: GPS, financial data, internal notes - Payment attacks: Amount manipulation, double-spending - Session hijack: Math.random() token prediction USAGE: CONVEX_DEPLOYMENT_URL=... CONVEX_ADMIN_KEY=... node index.js --target=idor This enables AI agents to run real active pentests against staging environments with proper authentication and attack templates.
DerpcatMusic
changed the title
DerpcatMusic
changed the title
| const results = await attacker.attack({ | ||
| functionName: "getGoogleIntegrationForUser", | ||
| vulnerableArg: "userId", | ||
| victimId: await convex.query("users", { email: "victim@test.com" }), |
There was a problem hiding this comment.
🔥 The Roast: This import path is so wrong it might actually work if you believe in UFOs. ./templates/idorttack doesn't exist — IDORAttacker is actually exported from ./harnesses/attack-suite. Also, the file is named idorttack.md (typo in filename).
🩹 The Fix:
| victimId: await convex.query("users", { email: "victim@test.com" }), | |
| import { IDORAttacker } from "./harnesses/attack-suite"; |
📏 Severity: warning
Code Review Roast 🔥Verdict: 1 Issue Found | Recommendation: Fix before merge Overview
Issue Details (click to expand)
🏆 Best part: The pentest framework is well-structured with proper security controls — it validates environment variables, requires admin keys, and is appropriately scoped to 💀 Worst part: The README imports 📊 Overall: Like a secure vault with a typo on the welcome sign — the structure is solid, but someone's going to walk into the wrong room. Files Reviewed (3 files)
Note: The PR description mentions "Security Hardening" work (CRITICAL IDOR fixes, rate limiting), but those fixes appear to be in a separate vulnerability report ( Reviewed by minimax-m2.7 · 784,328 tokens |
CRITICAL IDOR FIXES (user can access any user's data):
1. convex/calendar.ts - getGoogleIntegrationForUser
- Removed userId arg → uses getCurrentUserDoc(ctx)
- OAuth tokens (accessToken/refreshToken) NO LONGER returned to client
- Fixes: Attacker can steal Google OAuth tokens for any user
2. convex/calendar.ts - getCalendarProfileForUser
- Removed userId arg → uses getCurrentUserDoc(ctx)
- Fixes: Attacker can view any user's calendar config
3. convex/calendar.ts - getCalendarTimelineForUser
- Removed userId arg → uses getCurrentUserDoc(ctx)
- Fixes: Attacker can view any user's full lesson schedule
4. convex/calendar.ts - syncGoogleCalendarForUser
- Removed userId arg → derives from ctx.auth.getUserIdentity()
- Fixes: Attacker can manipulate any user's calendar
5. convex/calendar.ts - getEventMappingsForIntegration
- Added ownership validation: throws if integration.userId !== currentUser
- Fixes: Attacker can map Google events to internal job IDs
6. convex/calendar.ts - disconnectGoogleIntegrationLocally
- Removed userId arg → uses getCurrentUserDoc(ctx)
- Fixes: Attacker can disconnect any user's calendar
7. convex/notificationsCore.ts - getPushRecipientForUser
- Removed userId arg → uses getCurrentUserDoc(ctx)
- Fixes: Attacker can steal any user's Expo push token
8. convex/paymentsRead.ts - getPaymentForInvoicingRead
- Added requireCurrentUser() + ownership check
- Fixes: Attacker can view payment details for any payment ID
HIGH SEVERITY FIXES:
9. convex/jobs.ts - checkInstructorConflicts
- Removed instructorId arg → uses requireInstructorProfile(ctx)
- Fixes: Attacker can view any instructor's schedule conflicts
10. convex/users.ts - createUploadSessionToken
- Replaced Math.random() with crypto.randomUUID()
- Fixes: Upload session tokens were predictable (PRNG)
11. convex/userPushNotifications.ts - sendUserPushNotification
- Refactored to directly query user DB instead of calling vulnerable helper
- Fixes: Push tokens no longer exposed via internal query
CALLEE UPDATES (convex/calendarNode.ts):
- Updated 8 call sites to not pass userId/integrationId args
- Internal queries now derive user from auth context
Referenced PRs:
- PR #31: Security Hardening + AI Pentest Framework
1 participant
See PR-SECURITY-HARDENING-2026-03-26.md for full vulnerability report.
PR Contents
1. Security Hardening (Already Applied)
2. Vulnerability Report (PR-SECURITY-HARDENING-2026-03-26.md)
46 findings: 6 CRITICAL, 6 HIGH, 18 MEDIUM, 16 LOW
3. AI Pentest Framework (.agents/pentest/)
Enables AI agents to run active pentests against staging environments.
Usage:
CONVEX_DEPLOYMENT_URL=... CONVEX_ADMIN_KEY=... node .agents/pentest/index.js --target=idor
Attack Classes:
Test Plan