Skip to content

fix: security hardening (7 findings from audit) - #904

Open
pointunbalance wants to merge 1 commit into
midday-ai:mainfrom
pointunbalance:security-fixes
Open

fix: security hardening (7 findings from audit)#904
pointunbalance wants to merge 1 commit into
midday-ai:mainfrom
pointunbalance:security-fixes

Conversation

@pointunbalance

Copy link
Copy Markdown

Summary

This PR addresses 7 security findings identified in a comprehensive security audit of the midday codebase.

Changes

Severity Finding Fix
MEDIUM Polar webhook signature bypass fallback Removed fallback that bypasses signature verification on SDK parse errors
MEDIUM Content-Disposition header injection Added sanitizeFilename() to strip special characters from filenames
LOW Path traversal incomplete sanitization Added explicit rejection of .. and . path segments
LOW Worker admin dashboard optional auth Added startup warning when BOARD_USERNAME/BOARD_PASSWORD not set
LOW Inbox webhook auth bypass Added startup warning when INBOX_WEBHOOK_USERNAME/INBOX_WEBHOOK_PASSWORD not set
INFO Env var non-null assertions Replaced ! assertions with proper null checks for STRIPE_SECRET_KEY
INFO Slack response_url validation Added URL validation ensuring response URLs point to *.slack.com

Files Changed

  • apps/api/src/rest/routers/webhooks/polar/index.ts
  • apps/api/src/rest/routers/files/download.ts
  • apps/api/src/rest/routers/files/utils.ts
  • apps/worker/src/index.ts
  • apps/api/src/rest/routers/webhooks/inbox/index.ts
  • apps/api/src/rest/routers/webhooks/stripe/index.ts
  • apps/api/src/rest/routers/apps/slack/interactions.ts

Testing

All changes are defensive hardening with no behavioral changes to the happy path. The fixes follow existing code patterns in the codebase.

- Fix Polar webhook signature bypass fallback (MEDIUM)
- Sanitize Content-Disposition filename to prevent header injection (MEDIUM)
- Add path traversal rejection for '..' sequences (LOW)
- Add startup warning for Worker admin dashboard without auth (LOW)
- Add startup warning for Inbox webhook without Basic Auth (LOW)
- Replace non-null assertions with proper env var checks (INFO)
- Add Slack response_url validation as defense-in-depth (INFO)
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