netfelix-audio-fix/AUDIT.md

Security & Reliability Audit (2026-04-15)

Scope

Reviewed server and client codepaths with focus on:

• exposed attack surface
• secret handling
• destructive endpoints
• job execution safety
• input validation consistency

Findings (Highest Severity First)

1. Critical: Real service credentials are committed in the repository

• Evidence:
  • .env.development:6 (JELLYFIN_API_KEY=...)
  • .env.development:10 (RADARR_API_KEY=...)
  • .env.development:15 (SONARR_API_KEY=...)
• Impact:
  • Anyone with repository access can use these keys against the referenced services.
  • Secrets are now considered compromised and must be rotated.
• Fix:
  • Rotate all exposed API keys immediately.
  • Remove .env.development from Git history or sanitize it.
  • Keep only .env.example in version control.

2. High: No authentication/authorization on privileged API routes

• Evidence:
  • server/index.tsx:37 to server/index.tsx:43 mounts all admin routes without auth middleware.
  • Destructive/control endpoints are publicly callable by any client that can reach port 3000, e.g.:
    • server/api/settings.ts:204 (POST /api/settings/reset)
    • server/api/settings.ts:183 (POST /api/settings/clear-scan)
    • server/api/execute.ts:150 (POST /api/execute/start)
    • server/api/execute.ts:209 (POST /api/execute/stop)
• Impact:
  • Unauthorized users can start/stop jobs, wipe state, and alter processing behavior.
  • In containerized deployments with published ports, this is remotely exploitable on the network.
• Fix:
  • Add auth middleware at /api/* (at minimum: token-based admin auth).
  • Gate destructive routes with explicit admin authorization checks.

3. High: Settings endpoint leaks secrets in cleartext

• Evidence:
  • server/api/settings.ts:11 to server/api/settings.ts:15 returns getAllConfig() directly.
  • getAllConfig() includes API keys and passwords from DB/env via server/db/index.ts:123 to server/db/index.ts:134.
• Impact:
  • Any caller with API access can retrieve Jellyfin/Radarr/Sonarr API keys and MQTT password.
• Fix:
  • Redact secrets in responses (e.g. *** with optional last-4 chars).
  • Add a separate write-only secret update flow.

4. Medium: Inconsistent route ID validation in execute API

• Evidence:
  • server/lib/validate.ts:8 provides strict numeric parseId.
  • server/api/execute.ts:142 defines a looser local parser using Number.parseInt.
• Impact:
  • Values like "12abc" are accepted as ID 12 on execute routes, which can target unintended jobs.
• Fix:
  • Reuse server/lib/validate.ts parseId in server/api/execute.ts.
  • Add route tests for mixed alphanumeric IDs ("42abc", "+1", etc.).

Testing & Verification Gaps

• Could not run bun test / bun lint in this environment because bun is not installed.
• Existing tests cover some parser behavior in server/lib/__tests__/validate.test.ts, but execute-route param parsing has no dedicated regression test.

Recommended Remediation Order

1. Rotate leaked credentials and sanitize repository history.
2. Introduce API authentication and enforce it on all /api/* routes.
3. Redact secret fields from settings responses.
4. Replace execute-local parseId with shared strict validator and add tests.