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
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