fix(security): add API key rotation and optional TTL expiry#1738
fix(security): add API key rotation and optional TTL expiry#1738anshul23102 wants to merge 1 commit into
Conversation
Fixes utksh1#1619: backend/data/.api_key stored the single authentication secret as a plaintext file with no TTL, rotation endpoint, or revocation mechanism. Any process with read access to that directory (a co-located container, a path-traversal exploit, or a leaked backup) obtained a credential that was valid indefinitely. Changes: - Key file is now JSON ({"key": ..., "created_at": <epoch seconds>}) instead of bare plaintext, so the key's age can be tracked. A pre-existing plaintext key file is migrated in place on next startup -- the key itself is kept, just wrapped with a fresh created_at. - POST /api/v1/admin/api-key/rotate generates a new key and invalidates the old one immediately (there is no grace period -- the old key stops authenticating the instant this returns). Gated by the separate, statically-configured admin API key so a leaked client key can never be used to mint itself a replacement. - GET /api/v1/admin/api-key/status reports the key's age and, if configured, its expiry, without exposing the key itself. - New SECUSCAN_API_KEY_TTL_SECONDS setting (default 0 = disabled, so existing deployments are unaffected until an operator opts in). When set, both require_api_key and the session-creation endpoint reject the client key once it is older than the TTL, forcing rotation. - Documented the new rotation/expiry flow in docs/api-authentication.md. Testing: - testing/backend/unit/test_api_key_rotation.py: 15 new tests covering the JSON key format, legacy-plaintext migration, rotate/status endpoint auth gating, immediate old-key invalidation, and TTL expiry on both the main auth dependency and session creation. - Updated testing/backend/unit/test_auth.py and test_api_auth.py: the existing init_api_key tests asserted the key file was bare plaintext; updated to read the new JSON format (the key value itself is unchanged). - pytest testing/backend/unit -q -m 'not benchmark' -- 2240 passed, 1 pre-existing failure unrelated to this change (parser sandbox timeout test, flaky in this environment) - ruff check backend/secuscan/auth.py backend/secuscan/routes.py backend/secuscan/config.py testing/backend/unit/test_api_key_rotation.py testing/backend/unit/test_api_auth.py testing/backend/unit/test_auth.py -- all checks passed
utksh1
left a comment
There was a problem hiding this comment.
The rotation/TTL idea is valuable, but there's a compatibility regression: init_api_key() now treats any JSON parse failure as legacy plaintext and immediately rewrites the file. For operators using SECUSCAN_API_KEY_FILE pointing at a read-only secret mount (Docker/K8s), the first upgraded boot will fail with a write error.
Fix: skip the in-place rewrite when the file is read-only or when a custom key-file path is set. Also update the README's cat backend/data/.api_key instruction since the file format changed to JSON.
utksh1
left a comment
There was a problem hiding this comment.
Excellent security enhancement! Addresses #1619 by adding API key rotation and optional TTL expiry.
Key improvements:
✅ Key file now JSON format with created_at timestamp
✅ POST /api/v1/admin/api-key/rotate endpoint (admin-gated)
✅ GET /api/v1/admin/api-key/status for monitoring
✅ Optional SECUSCAN_API_KEY_TTL_SECONDS setting
✅ Backward compatible - migrates plaintext keys automatically
✅ Comprehensive test coverage (15 new tests)
Next step: This PR is from July 8th. Please rebase with main to pick up recent changes (#2037, #2038, #2039, #2040, #2041, #2042) and verify all tests still pass. Once rebased, I'll merge immediately!
Problem
backend/data/.api_keystores the single authentication secret as a plaintext file with no TTL, rotation endpoint, or revocation mechanism. Any process with read access to that directory (a co-located container, a path-traversal exploit, or a leaked backup) obtains a credential that is valid indefinitely.Fix
{"key": ..., "created_at": <epoch seconds>}) instead of bare plaintext, so the key's age can be tracked. A pre-existing plaintext key file is migrated in place on next startup -- the key itself is kept, just wrapped with a freshcreated_at.POST /api/v1/admin/api-key/rotategenerates a new key and invalidates the old one immediately -- no grace period. Gated by the separate, statically-configured admin API key so a leaked client key can never be used to mint itself a replacement.GET /api/v1/admin/api-key/statusreports the key's age and, if configured, its expiry, without exposing the key itself.SECUSCAN_API_KEY_TTL_SECONDSsetting (default0= disabled, so existing deployments are unaffected until an operator opts in). When set, bothrequire_api_keyand the session-creation endpoint reject the client key once it's older than the TTL, forcing rotation.docs/api-authentication.md.Testing
testing/backend/unit/test_api_key_rotation.py: 15 new tests covering the JSON key format, legacy-plaintext migration, rotate/status endpoint auth gating, immediate old-key invalidation, and TTL expiry on both the main auth dependency and session creationtest_auth.pyandtest_api_auth.py: existinginit_api_keytests asserted the key file was bare plaintext; updated to read the new JSON format (the key value itself is unchanged)pytest testing/backend/unit -q -m "not benchmark"-- 2240 passed, 1 pre-existing failure unrelated to this change (parser sandbox timeout test, flaky in this environment)ruff check-- all checks passedFixes #1619