#39Add invoice listing and CSV exportChanges requested
iamapoorv476 wants to mergedemo-invoice-export→main0 files+0−0reviewed 17d ago
The agent requested changes — it recorded 2 critical and 2 warning-level findings across 0 files.
¶ Marginalia · model claude-haiku-4-5 · 18955 tokens
findings 5 · steps 29
tools called 24
tools called 24
criticalapp/routes/invoices.py : 10security
app/routes/invoices.py : 10
The get_invoices endpoint accepts a customer_id parameter but does not verify that the authenticated user owns or has access to that customer. An attacker can access any customer's invoices by simply changing the customer_id in the URL. The user object is obtained from get_current_user but never used to validate ownership.
+ Add authorization check to verify the authenticated user owns the customer_id being requested. For example: if user["customer_id"] != customer_id: raise HTTPException(status_code=403, detail="Unauthorized"). Apply the same check to the export_invoices endpoint.
criticalapp/routes/invoices.py : 15security
app/routes/invoices.py : 15
The export_invoices endpoint has the same authorization vulnerability as get_invoices. It does not verify that the authenticated user owns the customer_id being exported. An attacker can export any customer's invoices.
+ Add authorization check: if user["customer_id"] != customer_id: raise HTTPException(status_code=403, detail="Unauthorized")
warningapp/services/invoices.py : 13performance
app/services/invoices.py : 13
The function executes one query to fetch invoices, then for each invoice (in a loop), executes another query to fetch its items. This results in 1 + N database queries where N is the number of invoices. With pagination of 50 items, this means 51 queries per request. This will cause significant performance degradation as the number of invoices grows.
+ Use a single JOIN query to fetch invoices and their items together, or use a separate query to fetch all items for the invoice IDs in one batch. Example: SELECT invoice_id, description, quantity, unit_price FROM invoice_items WHERE invoice_id IN (SELECT id FROM invoices WHERE customer_id = ?)"
warningapp/services/notifications.py : 3security
app/services/notifications.py : 3
The webhook URL is hardcoded as "https://hooks.example.com/exports" in the source code. This should be configurable via environment variables to allow different URLs for different environments (dev, staging, production) and to avoid exposing infrastructure details in the codebase.
+ Move the webhook URL to an environment variable. Use: WEBHOOK_URL = os.getenv("WEBHOOK_URL", "https://hooks.example.com/exports")
suggestionapp/services/notifications.py : 7quality
app/services/notifications.py : 7
The notify_export_ready function catches all exceptions with a bare except clause and silently ignores them with pass. This makes debugging difficult and could hide real errors like network failures, timeouts, or invalid webhook URLs. It also prevents proper logging of failures.
+ Add proper error handling and logging. Use: except requests.RequestException as e: logger.error(f"Failed to notify export ready: {e}") or at minimum log the error before passing. Consider whether the export should fail if the webhook notification fails.