Skip to content

fix(security): audit quick fixes — V2, V3, V13, V14, V15, V16 - #15

Merged
NicolasRitouet merged 7 commits into
mainfrom
security/audit-quick-fixes
Apr 9, 2026
Merged

fix(security): audit quick fixes — V2, V3, V13, V14, V15, V16#15
NicolasRitouet merged 7 commits into
mainfrom
security/audit-quick-fixes

Conversation

@NicolasRitouet

@NicolasRitouet NicolasRitouet commented Apr 9, 2026

Copy link
Copy Markdown
Member

Summary

Six quick security fixes from the AppSec audit, all low-risk and surgically scoped.

Changes

V2 — Stop logging token preview in auth middleware [Critical]

packages/backend/src/middleware/auth.ts

  • Replaced tokenPreview (first 20 + last 10 chars of bearer tokens) with tokenType + tokenLength
  • A 40-char GitHub PAT had 30/40 chars logged — enough for brute-force reconstruction
  • Ref: OWASP Logging Cheat Sheet — "never log access tokens"

V3 — Require authentication on /v1/github/repo-ids [High]

packages/backend/src/api/v1/routes/github.routes.ts

  • Added preHandler: [authenticateGitHub] to the route
  • Previously unauthenticated: anyone could probe private repo existence by name

V13 — Remove secret name enumeration from error messages [Medium]

packages/backend/src/api/v1/routes/secrets.routes.ts

  • Error response no longer lists all available secret names in the environment
  • Users with read access could discover secret names without knowing them

V14 — Generic error in Stripe webhook response [Medium]

packages/backend/src/api/v1/routes/billing.routes.ts

  • Changed error: error.message to error: "Processing failed" in webhook error response
  • Internal error details (DB errors, service names) were being sent to Stripe's infrastructure

V15 — Disable sendDefaultPii in all Sentry configs [Medium]

packages/dashboard/instrumentation-client.ts, sentry.edge.config.ts, sentry.server.config.ts

  • Set sendDefaultPii: false in all 3 Sentry configs (client, edge, server)
  • Was automatically sending cookies (including session tokens), IPs, and headers to Sentry

V16 — Map auth callback errors to predefined messages [Medium]

packages/dashboard/app/auth/callback/page.tsx

  • Error query parameter is now mapped to predefined safe messages instead of displayed verbatim
  • Prevented social engineering via crafted URLs (e.g. phishing messages in error param)
  • Analytics also normalized to only track known error codes

What's NOT in this PR

Test plan

  • Backend type-check passes
  • 935 backend tests pass (2 skipped, pre-existing)
  • Dashboard builds successfully
  • CI green (test, build, Vercel)
  • Verify /v1/github/repo-ids returns 401 without auth header
  • Verify secret not-found error no longer lists available keys
  • Verify Sentry events no longer contain cookies/IP

🤖 Generated with Claude Code

NicolasRitouet and others added 6 commits April 9, 2026 23:18
Replace tokenPreview (first 20 + last 10 chars of bearer tokens) with
tokenType + tokenLength. For a 40-char GitHub PAT, the preview leaked
30/40 characters — enough to reconstruct via brute force.

Ref: OWASP Logging Cheat Sheet — never log access tokens.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This endpoint was unauthenticated and allowed anyone to probe whether
private repositories exist by name, using the GitHub App installation
token. Now requires authenticateGitHub like other github routes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a requested secret key was not found, the error response listed
ALL available secret keys in that environment. This allowed any user
with read access to discover secret names without knowing them.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The webhook error handler was returning error.message to Stripe,
potentially leaking internal details (DB errors, service names).
Now returns a generic "Processing failed" — the full error is
already logged server-side.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sendDefaultPii: true was sending IP addresses, cookies (including
session tokens), and headers to Sentry. For a secrets management
platform, session cookies must never be sent to third-party services.

Disabled in all 3 Sentry configs: client, edge, and server.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The error query parameter was displayed verbatim in the UI, allowing
attackers to craft URLs with social engineering messages (e.g.
"Your account has been compromised, click here..."). Now maps error
codes to predefined safe messages.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Apr 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
keyway-dashboard Ready Ready Preview, Comment Apr 9, 2026 9:38pm

Request Review

@coderabbitai

coderabbitai Bot commented Apr 9, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

API routes, middleware, and dashboard configs updated: GitHub auth added to /repo-ids; Stripe webhook error response made generic; secrets NotFound messages simplified; auth middleware logs tokenType/tokenLength instead of previews; OAuth callback maps known errors to friendly text; Sentry sendDefaultPii disabled.

Changes

Cohort / File(s) Summary
Billing & GitHub & Secrets routes
packages/backend/src/api/v1/routes/billing.routes.ts, packages/backend/src/api/v1/routes/github.routes.ts, packages/backend/src/api/v1/routes/secrets.routes.ts
Billing webhook now returns a constant "Processing failed" message for non-signature errors while preserving { received: true }. GitHub /repo-ids route now uses authenticateGitHub preHandler. Secrets /view route no longer includes computed "available secrets" text in NotFoundError messages.
Authentication middleware
packages/backend/src/middleware/auth.ts
Removed logging of token content preview; logs now emit tokenType and tokenLength. JWT verification and GitHub-token fallback logs updated to include tokenType instead of token preview.
Dashboard OAuth callback
packages/dashboard/app/auth/callback/page.tsx
Added errorMessages map; normalized tracking payload to known codes or 'unknown'; displayed error text comes from mapped messages with a generic fallback.
Sentry configuration (client, edge, server)
packages/dashboard/instrumentation-client.ts, packages/dashboard/sentry.edge.config.ts, packages/dashboard/sentry.server.config.ts
Disabled sendDefaultPii (changed from true to false) across client, edge, and server Sentry initializations; no other config surface or exports changed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐇 I nibble bytes beneath the moon,

Tokens trimmed, errors tuned in tune,
Auth gates set and secrets pruned,
Sentry hushes footprint rune,
Hop—new code blooms by morning soon.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title references specific audit fix items (V2, V3, V13-V16) that directly correspond to the security changes implemented across the PR, clearly identifying the main purpose as addressing identified security vulnerabilities.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/audit-quick-fixes

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/dashboard/app/auth/callback/page.tsx`:
- Around line 23-25: The code currently sends the raw, user-controlled
errorParam to analytics; update the error handling in the auth callback to
normalize errorParam against the allowed set (keys of errorMessages or an
explicit knownErrors list), map any unrecognized or empty value to
'unknown_error', use that normalized value when calling
trackEvent(AnalyticsEvents.AUTH_CALLBACK_ERROR, ...), and continue to use
setError(errorMessages[normalized] || 'An unexpected error occurred...') so the
UI still shows a friendly message while telemetry only receives the normalized
code.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: badb50cc-b4f0-4199-b9a7-285e1ed24588

📥 Commits

Reviewing files that changed from the base of the PR and between 608c2c3 and 9b4341b.

📒 Files selected for processing (8)
  • packages/backend/src/api/v1/routes/billing.routes.ts
  • packages/backend/src/api/v1/routes/github.routes.ts
  • packages/backend/src/api/v1/routes/secrets.routes.ts
  • packages/backend/src/middleware/auth.ts
  • packages/dashboard/app/auth/callback/page.tsx
  • packages/dashboard/instrumentation-client.ts
  • packages/dashboard/sentry.edge.config.ts
  • packages/dashboard/sentry.server.config.ts

Comment thread packages/dashboard/app/auth/callback/page.tsx
CodeRabbit review: the raw errorParam from the URL was still sent to
PostHog analytics, allowing user-controlled values into telemetry.
Now only sends known error codes; unknown values are tracked as
"unknown" to prevent high-cardinality noise and data injection.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@NicolasRitouet
NicolasRitouet merged commit dd34606 into main Apr 9, 2026
5 checks passed
@NicolasRitouet
NicolasRitouet deleted the security/audit-quick-fixes branch April 9, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant