Skip to content

fix(security): add API key rotation and optional TTL expiry#1738

Open
anshul23102 wants to merge 1 commit into
utksh1:mainfrom
anshul23102:fix/1619-api-key-rotation
Open

fix(security): add API key rotation and optional TTL expiry#1738
anshul23102 wants to merge 1 commit into
utksh1:mainfrom
anshul23102:fix/1619-api-key-rotation

Conversation

@anshul23102

Copy link
Copy Markdown
Contributor

Problem

backend/data/.api_key stores 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 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 -- 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/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's older than the TTL, forcing rotation.
  • Documented the new 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 test_auth.py and test_api_auth.py: 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 -- all checks passed

Fixes #1619

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 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 utksh1 added level:advanced 55 pts difficulty label for advanced contributor PRs type:security Security work category bonus label type:feature Feature work category bonus label area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests labels Jul 9, 2026

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests level:advanced 55 pts difficulty label for advanced contributor PRs type:feature Feature work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: API key stored in plaintext with no rotation or expiry

2 participants