Security audit fixes for issue #449

[joryirving]

Security Audit Fixes for Issue #449

Changes Made

Finding	Severity	Fix
CSP unsafe-inline	HIGH	Replaced with nonce-based script-src 'nonce-xxx'
Mobile auth token logged	MEDIUM	Removed console.log statements from /api/mobile-auth/consume
SSE endpoint unprotected	MEDIUM	Added rate limiter (10 req/min per IP)
Auth endpoint unprotected	LOW	Added rate limiter (20 req/15min per IP)
Health endpoint exposed internal state	MEDIUM	Removed gateway WS status, reconnect attempts, uptime

Not Fixed (Declined)

• CORS origin check bypass - non-browser mobile clients (Capacitor) require no-origin requests

Testing

All 22 tests pass.

Note

The nonce-based CSP requires inline <script> tags in HTML to include the nonce attribute. The nonce is stored in res.locals.cspNonce for template injection if needed.

[its-miso]

✅ Automated recommendation: APPROVE

Analysis engine: MiniMax-M2.7@https://api.minimax.io/v1

PR Review: Security audit fixes for issue #449

Recommendation: Approve

This PR addresses all documented security findings from issue #449 with targeted, well-implemented fixes. The changes are internally consistent and follow security best practices.

---

Change-by-Change Findings

1. AGENTS.md (identity/approval authority removal)

• Finding: Removed "Identity" and "Approval Authority" sections (-28 lines)
• Assessment: Intentional cleanup (see git log commit 6658668), appropriate for open-source repository. No security or functional impact.

2. security.js — CSP nonce-based implementation

• Finding: Replaced unsafe-inline script-src with per-request nonce using crypto.randomBytes(16).toString('base64')
• Assessment: ✅ Correct implementation
  • Uses cryptographically secure crypto.randomBytes(16) for nonce generation
  • Nonce stored in res.locals.cspNonce for template injection
  • CSP also expanded with additional restrictive directives: base-uri 'self', frame-ancestors 'none', img-src 'self' data:, connect-src 'self' ws: wss:, form-action 'self'
• Test updated: tests/security.test.js assertion updated to /script-src 'self' 'nonce-\w+/

3. server.js — Rate limiters

• Finding: Added sseLimiter (10 req/min) for /api/events SSE endpoint and authLimiter (20 req/15min) for /login
• Assessment: ✅ Consistent with existing limiter pattern
  • Proper IP extraction from cf-connecting-ip and x-forwarded-for headers
  • authLimiter correctly uses skip to avoid limiting non-local auth modes
  • Error messages are descriptive and consistent

4. server.js — Removed sensitive logging

• Finding: Removed 8 console.log statements from /api/mobile-auth/consume that logged token/body/headers/user/session state
• Assessment: ✅ Correct. These logs could expose auth tokens and session data. Only console.error for actual errors retained.

5. server.js — Health endpoint sanitization

• Finding: Removed uptime, gatewayWsConnected, gatewayWsReconnectAttempts, gatewayWsLastError, gatewayWsLastClose from /api/health
• Assessment: ✅ Correct. Internal operational state should not be exposed in health checks.

---

Standards Compliance

Requirement	Status
Security first — don't expose secrets	✅ Addressed (removed token logging)
Be direct and practical	✅ Targeted fixes without scope creep
Research before task	✅ Based on documented audit findings (#449)
Tests pass	✅ All 22 tests pass (per PR body)

---

Declined Finding: CORS Origin Bypass

• Rationale: Non-browser Capacitor mobile clients require no-origin requests
• Assessment: ✅ Reasonable tradeoff documented in PR body

---

Unknowns / Needs Verification

Item	Note
HTML nonce injection	PR notes state inline <script> tags need nonce attribute for CSP to work. The nonce is stored in res.locals.cspNonce. If this PR only modifies server-side security headers but the frontend HTML is served separately, templates using inline scripts should be verified to inject nonce="${res.locals.cspNonce}" on their <script> tags.
No version/changelog update	Package.json version unchanged; typical for security patches. No CHANGELOG entry observed. May be intentional for patch release workflow.

---

Sources

• PR: Security audit fixes for issue #449 #451
• Git log confirms security audit context: dea42b0 Security audit fixes for issue #449
• Rate limiter pattern follows existing limiter in server.js (line 288)
• AGENTS.md removal rationale: commit 6658668 Remove identity/approval authority from AGENTS.md (open source repo)