Skip to content

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

Closed
anshul23102 wants to merge 942 commits into
utksh1:mainfrom
anshul23102:fix/1619-api-key-rotation
Closed

fix(security): add API key rotation and optional TTL expiry#1738
anshul23102 wants to merge 942 commits 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

Rakshak05 and others added 30 commits June 30, 2026 09:40
utksh1#1398

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
- New reportTemplates.ts service with ReportTemplate type, three built-in
  templates (executive, technical, compliance), and render/preview/export.
- ReportTemplatePicker.tsx slide-over component with type filtering, inline
  preview, and .md export.
- Integrate Templates button into Reports.tsx report cards.
- 21 unit tests covering template lifecycle, edge cases, and output.
…1546)

Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
…h1#1542)

Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
…odule (utksh1#1524)

The extract_target helper in executor.py is a pure function but lives in
a heavy import chain (FastAPI, cache, config). Per the maintainer's
approved extraction pattern (used for routes_json_helpers), this extracts
extract_target into a small import-safe executor_target_helpers module
and re-exports it from executor.py so existing call sites keep working.

Closes utksh1#1389.

Co-authored-by: tmdeveloper007 <tmdeveloper007@users.noreply.github.com>
i-OmSharma and others added 13 commits July 20, 2026 12:27
…-parser-validation

fix(plugins): enforce strict validation and integrity checks for missing custom parser utksh1#1812
…to ID generation

Addresses Bandit HIGH severity findings:
- finding_intelligence.py:75 - stable ID generation
- platform_resources.py:29 - asset ID generation

These SHA1 usages are for deterministic hashing to create stable IDs,
not for cryptographic security purposes. Adding usedforsecurity=False
clarifies the intent and resolves the security linter warnings.
The debug default was changed from True to False in the security fix.
Update the test to match the new secure default.
The saved_views_router now has require_api_key dependency.
Override it in tests to bypass authentication for unit testing.

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

NaitikVerma6776 and others added 6 commits July 21, 2026 10:50
Add shared time_utils helpers and use timezone-aware UTC with an
explicit offset for generated_at and discovered_at across reports,
findings API responses, and report generation.

Closes utksh1#1882
Default to_utc_iso to timespec=auto so finding intelligence tests can
compare against datetime.now(UTC). Update TLS verification mocks for
crawler client.stream() and stub crawl_target in API scanner tests.
…idable

_init_default_policies() built the entire network denylist from the
single Pydantic field settings.network_denylist. Pydantic replaces
(rather than merges) a list field's default when SECUSCAN_NETWORK_DENYLIST
is set via env var, so any operator adding even one custom denylist
entry silently dropped the built-in protection for cloud metadata
(169.254.169.254), loopback, RFC1918/CGNAT ranges, and IPv6
link-local/ULA space -- reopening SSRF to the metadata endpoint despite
the code comment claiming the denylist was 'always enforced'.

Fix: move those ranges into a new MANDATORY_DENYLIST module constant
that is not read from settings and is applied unconditionally in
_init_default_policies before any operator-configured entries. The
operator-facing network_denylist setting is now purely additive.

Also updates the existing default-denylist test and adds a regression
test reproducing the exact scenario from utksh1#1748.
…ne-standardize-9bb6

fix(backend): standardize timezone handling to UTC ISO-8601
…t-metadata-ssrf

Fix utksh1#1748: make cloud-metadata/private-range denylist non-overridable
Fix: add auth and owner isolation to saved views API (closes utksh1#1743)

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

This remains approved in principle, but the branch is behind main. Please rebase it on current main and rerun the API-key rotation/TTL test suite; it can be merged once the refreshed checks are clean.

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

This remains approved in principle, but the branch is behind main. Please rebase it on current main and rerun the API-key rotation/TTL test suite; it can be merged once the refreshed checks are clean.

Khanvilkarshravani27 and others added 4 commits July 24, 2026 15:10
Cover the scapy_recon plugin parser.py with targeted behavioural tests:

- Metadata contract: file existence, valid JSON, required fields, engine
  binary, target/type field declarations
- ARP output: host count, IP+MAC extraction, finding keys, category,
  severity, description content, metadata consistency, remediation
- ICMP output: host count, IP extraction, Unknown-MAC default
- Single-host edge case: IP+MAC in result and description
- Malformed/empty input: empty string, whitespace-only, no UP: lines,
  mixed noise lines, malformed UP: lines, missing MAC separator

No changes to backend source; test file only.
* fix: stop dashboard polling after health failure and add manual retry

* fix: skip pre-existing upstream auth tests that cannot pass with mocked auth

* fix: update postcss to resolve GHSA-r28c-9q8g-f849 high severity vulnerability

* fix: document localhost-only Docker binding, add opt-in network override
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
- Skip in-place key file migration when using custom SECUSCAN_API_KEY_FILE path
  (e.g. Docker secrets mounts which are read-only)
- Check write permissions before attempting plaintext-to-JSON migration
- Gracefully handle legacy key files on read-only mounts by using the key as-is
- Update README to show correct JSON format extraction (use jq to extract key)
- Fixes compatibility regression that broke deployments with read-only secret mounts
@anshul23102
anshul23102 force-pushed the fix/1619-api-key-rotation branch from f8dd87e to a9e5dec Compare July 30, 2026 07:22
@anshul23102

Copy link
Copy Markdown
Contributor Author

Addressed all feedback from your review:

  1. Fixed the critical read-only file issue by:

    • Detecting when SECUSCAN_API_KEY_FILE is set (custom path for Docker secrets)
    • Checking write permissions before attempting plaintext-to-JSON migration
    • Gracefully using legacy keys on read-only mounts without attempting write
  2. Updated README documentation to reflect the new JSON format:

    • Changed cat backend/data/.api_key to cat backend/data/.api_key | jq -r '.key'
    • Updated API examples to correctly extract the key from JSON
  3. Rebased on current main to pick up all recent changes

The PR now handles read-only secret mounts safely and operators using SECUSCAN_API_KEY_FILE will not encounter write errors on startup.

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