security: remove passwords from all logs#1368
Conversation
Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@riderx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThree files updated to prevent sensitive data leakage in logging and error reporting. Password fields are stripped from logs in backend handlers, and Discord utilities now include comprehensive sanitization functions for headers and request bodies before external transmission. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 684e45b9dd
ℹ️ 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".
| app.post('/', async (c) => { | ||
| const rawBody = await parseBody<AcceptInvitation>(c) | ||
| cloudlog({ requestId: c.get('requestId'), context: 'accept_invitation raw body', rawBody }) | ||
| const { password: _password, ...rawBodyWithoutPassword } = rawBody |
There was a problem hiding this comment.
Guard against null body before destructuring
If a client sends a valid JSON null body, parseBody returns null, and destructuring (const { password: _password, ...rawBodyWithoutPassword } = rawBody) throws a TypeError before schema validation runs. This changes the behavior from a 400 with invalid_json_body to a 500 error, which is a regression for malformed input. Consider moving the log after safeParse or defaulting rawBody to {} when it’s null/undefined.
Useful? React with 👍 / 👎.
| app.post('/', async (c) => { | ||
| const rawBody = await parseBody<ValidatePasswordCompliance>(c) | ||
| cloudlog({ requestId: c.get('requestId'), context: 'validate_password_compliance raw body (password redacted)', rawBody: { ...rawBody, password: '***' } }) | ||
| const { password: _password, ...rawBodyWithoutPassword } = rawBody |
There was a problem hiding this comment.
Destructuring rawBody can throw on JSON null
In this handler, a request body of JSON null will cause rawBody to be null, and destructuring it throws before validation, resulting in a 500 instead of the intended 400 error response. This is a regression introduced by the new logging redaction. Guard rawBody or move the log after safeParse to preserve the previous error handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
supabase/functions/_backend/utils/discord.ts (1)
8-64: LGTM! Comprehensive sanitization implementation.The sanitization functions provide defense-in-depth protection:
REMOVED_FIELDScompletely removes password from logsPARTIALLY_REDACTED_FIELDSshows first/last 4 characters for debugging while protecting secretspartialRedactcorrectly handles short valuessanitizeSensitiveFromStringuses regex to sanitize JSON-like stringssanitizeSensitiveHeadersfilters sensitive headersRegarding the static analysis warnings about ReDoS: These are false positives. The regex patterns are constructed from hardcoded constants (
REMOVED_FIELDSandPARTIALLY_REDACTED_FIELDS), not user input, so there's no risk of malicious regex injection.Optional note: The regex patterns assume properly formatted JSON with double quotes. While this handles the vast majority of cases, escaped quotes within values (e.g.,
"password":"my\"secret") might not be fully sanitized. However, this edge case is unlikely and the current implementation is still a significant security improvement.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/private/validate_password_compliance.tssupabase/functions/_backend/utils/discord.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/utils/discord.tssupabase/functions/_backend/private/validate_password_compliance.ts
supabase/functions/_backend/**
📄 CodeRabbit inference engine (CLAUDE.md)
Backend logic should be organized in
supabase/functions/_backend/with subdirectories for plugins, private endpoints, public endpoints, triggers, and utilities
Files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/utils/discord.tssupabase/functions/_backend/private/validate_password_compliance.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode with path aliases mapping
~/tosrc/
Files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/utils/discord.tssupabase/functions/_backend/private/validate_password_compliance.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Supabase Edge Functions use Deno runtime
Files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/utils/discord.tssupabase/functions/_backend/private/validate_password_compliance.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/utils/discord.tssupabase/functions/_backend/private/validate_password_compliance.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/utils/discord.tssupabase/functions/_backend/private/validate_password_compliance.ts
🧠 Learnings (3)
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Check `c.get('auth')?.authType` to determine authentication type ('apikey' vs 'jwt') in backend endpoints
Applied to files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/private/validate_password_compliance.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use structured logging with `cloudlog({ requestId: c.get('requestId'), message: '...' })` for all backend logging
Applied to files:
supabase/functions/_backend/private/accept_invitation.tssupabase/functions/_backend/utils/discord.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `middlewareAPISecret` for internal API endpoints and `middlewareKey` for external API keys; validate against `owner_org` in the `apikeys` table
Applied to files:
supabase/functions/_backend/utils/discord.ts
🧬 Code graph analysis (2)
supabase/functions/_backend/private/accept_invitation.ts (2)
supabase/functions/_backend/utils/logging.ts (1)
cloudlog(3-15)scripts/snippet/cloudflare-snippet-filter-appid.js (1)
body(65-65)
supabase/functions/_backend/private/validate_password_compliance.ts (1)
supabase/functions/_backend/utils/logging.ts (1)
cloudlog(3-15)
🪛 ast-grep (0.40.3)
supabase/functions/_backend/utils/discord.ts
[warning] 27-27: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp("${field}"\\s*:\\s*"[^"]*"\\s*,?\\s*, 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 37-37: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(("${field}"\\s*:\\s*)"([^"]*)", 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (3)
supabase/functions/_backend/private/accept_invitation.ts (1)
66-67: LGTM! Password successfully removed from logs.The destructuring pattern correctly removes the password field before logging, preventing sensitive data leakage while preserving it for business logic. This implementation aligns with the PR objective to ensure passwords never reach Cloudflare logs.
Also applies to: 128-129
supabase/functions/_backend/private/validate_password_compliance.ts (1)
58-59: LGTM! Password successfully removed from logs.The destructuring pattern correctly excludes the password field from logs while preserving it for validation and authentication logic. This change aligns with the PR objective and follows the same pattern as
accept_invitation.ts.supabase/functions/_backend/utils/discord.ts (1)
107-113: LGTM! Sanitization correctly integrated into error alerts.The implementation properly applies sanitization to both headers and request body before sending to Discord:
- Line 107-108: Headers extracted and sanitized
- Line 113: Body sanitized with defense-in-depth comment
- Line 135: Sanitized body used in Discord embed
This ensures that passwords are never transmitted to Discord, while other sensitive fields (api_key, token) remain partially visible for debugging. The changes align with the PR objective and test plan.
Also applies to: 135-135
Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
* security: remove passwords from all logs Ensure passwords are never logged to Cloudflare, Supabase, or Discord by: - Removing password field from cloudlog calls in accept_invitation and validate_password_compliance - Sanitizing Discord alerts to completely remove password field and partially redact other sensitive fields 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: move password redaction after validation to handle null body Address PR feedback - if a client sends JSON null, destructuring before validation throws TypeError (500) instead of returning 400. Move cloudlog calls after safeParse validation to ensure body is valid before destructuring. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore: remove deno.lock from commit 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>



Summary
Ensure passwords are never logged to Cloudflare, Supabase, or Discord alerts by removing the password field entirely from all logging contexts.
Changes
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.