Skip to content

fix(security): critical security fixes (Phase 1) - #67

Open
saadamjad wants to merge 1 commit into
mainfrom
fix/security-phase-1
Open

fix(security): critical security fixes (Phase 1)#67
saadamjad wants to merge 1 commit into
mainfrom
fix/security-phase-1

Conversation

@saadamjad

Copy link
Copy Markdown
Collaborator

What does this PR do?

Phase 1 of the security audit remediation plan — fixes the CRITICAL and highest-impact HIGH findings that would cause embarrassing failures on day-1 of public launch.

  • OTP plaintext logging: no longer logs the OTP code when SMTP is unconfigured in production; raises instead
  • CORS wildcard + credentials: allow_origins=["*"] + allow_credentials=True is an invalid combination browsers silently reject. Origins now come from CORS_ORIGINS env var
  • User enumeration: POST /v1/auth/request-otp returned 409 for existing emails and 404 for unknown ones. Both cases now return an identical 200 response
  • No restart policies: added restart: unless-stopped to every Docker Compose service
  • Services bound to 0.0.0.0: Postgres, Qdrant, Redis ports now bind to 127.0.0.1 only
  • Shallow health check in deploy script: infra/deploy-production.sh now polls /health/deep (real dependency check) instead of /health (always 200)
  • NEXT_PUBLIC_DEV_MODE footgun: deploy script now refuses to deploy if this is still true (bypasses dashboard OTP login)
  • Dockerfile ran as root: added non-root USER + core/.dockerignore
  • client_ip() spoofing: X-Forwarded-For is now only trusted from a configured trusted proxy address, closing a rate-limit bypass
  • Dev API key footgun: refuses to boot if DEV_API_KEY is set while ENV=production; removed the compose default that could silently re-enable the bypass
  • Added CHANGELOG.md and .github/PULL_REQUEST_TEMPLATE.md

How to test

# Lint
/Users/apple/Library/Python/3.14/bin/ruff check core/ sdk/python/ mcp/ integrations/

# Core unit tests (includes updated OTP-enumeration and rate-limit tests)
pytest core/tests/ -m "not integration" -v

# SDK + MCP + TS SDK
pytest sdk/python/tests/ -v
pytest mcp/tests/ -v
cd sdk/typescript && npm test

# Dashboard
cd dashboard && npm run lint && npm run build

Manual checks:

  • curl -s -o /dev/null -w "%{http_code}" -X POST http://localhost:8000/v1/auth/request-otp -H "Content-Type: application/json" -d '{"email":"nobody@example.com"}' → 200
  • Set ENV=production + DEV_API_KEY=zizkadb_dev_local → app refuses to start

Checklist

  • Tests pass (pytest core/tests/ -m "not integration" -v — 136 passed)
  • Lint passes (ruff check core/ sdk/python/ mcp/ integrations/)
  • Dashboard lints clean (npm run lint)
  • No new allow_origins=["*"] + allow_credentials=True combinations
  • Schema changes are idempotent — N/A, no schema changes in this PR
  • Auth dependency is correct — no auth-dependency routing changed
  • CHANGELOG.md updated under [Unreleased]

🤖 Generated with Claude Code

Addresses the CRITICAL and highest-impact HIGH findings from the
production-grade security audit:

- Fix OTP plaintext logging in production when SMTP is unconfigured
- Fix CORS wildcard + credentials (invalid combo, silently rejected by
  browsers); origins now read from CORS_ORIGINS
- Fix user enumeration in POST /v1/auth/request-otp (was 409 vs 404)
- Add restart: unless-stopped to every Docker Compose service
- Bind Postgres/Qdrant/Redis ports to 127.0.0.1 instead of 0.0.0.0
- Fix deploy script to poll /health/deep instead of /health (always 200)
- Add NEXT_PUBLIC_DEV_MODE=true production deploy guard
- Add non-root USER to core/Dockerfile + core/.dockerignore
- Fix client_ip() to only trust X-Forwarded-For from a trusted proxy,
  closing a rate-limit bypass via header spoofing
- Refuse to start if DEV_API_KEY is set while ENV=production; remove the
  compose default that could silently re-enable the dev auth bypass
- Add CHANGELOG.md and PR template

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@saadamjad saadamjad changed the title fix(security): critical security fixes before public launch (Phase 1) fix(security): critical security fixes (Phase 1) Jul 16, 2026
@saadamjad
saadamjad requested a review from Zizka-ai July 17, 2026 08:38
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