Skip to content

fix(config): remove hardcoded JWT signing key; resolve securely#1134

Open
madhavilosetty-intel wants to merge 1 commit into
mainfrom
fix/jwt-key-secure-handling
Open

fix(config): remove hardcoded JWT signing key; resolve securely#1134
madhavilosetty-intel wants to merge 1 commit into
mainfrom
fix/jwt-key-secure-handling

Conversation

@madhavilosetty-intel

Copy link
Copy Markdown
Contributor

The JWT signing key defaulted to the public literal your_secret_jwt_key, allowing anyone to forge HS256 JWTs and bypass authentication for both REST APIs and KVM/SOL/IDER WebSocket endpoints. Because cleanenv accepts the non-empty default, the application starts with the insecure key even when AUTH_JWT_KEY is unset.

Remove the hardcoded default and resolve the key securely (Vault → OS keyring → generate a random 256-bit key), never persisting it to config.yml. Refuse to start with an empty or known-insecure key, tighten config.yml permissions (0600/0700), and remove the literal from .env.example and config/config.yml.

Refs: #1067

@madhavilosetty-intel
madhavilosetty-intel requested a review from a team as a code owner July 10, 2026 23:40
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 61.03896% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.88%. Comparing base (036fc8b) to head (e840c17).

Files with missing lines Patch % Lines
cmd/app/main.go 50.00% 19 Missing and 6 partials ⚠️
config/config.go 40.00% 1 Missing and 2 partials ⚠️
cmd/app/secretstore.go 90.90% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1134      +/-   ##
==========================================
+ Coverage   43.59%   43.88%   +0.28%     
==========================================
  Files         143      144       +1     
  Lines       13621    13634      +13     
==========================================
+ Hits         5938     5983      +45     
+ Misses       7118     7078      -40     
- Partials      565      573       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@madhavilosetty-intel
madhavilosetty-intel force-pushed the fix/jwt-key-secure-handling branch from 383532a to 0c93db7 Compare July 10, 2026 23:42
The JWT signing key defaulted to the public literal "your_secret_jwt_key"
in defaultConfig, .env.example and config/config.yml. Because every JWT
(REST middleware and the KVM/SOL/IDER WebSocket) is validated with HS256
against this key, anyone could forge a token and bypass authentication.
The env-required tag gave false assurance: cleanenv is satisfied by the
non-empty default, so the app booted with the insecure key.

Resolve the key with no hardcoded default, mirroring the AMT encryption
key: secret store (Vault) -> OS keyring -> generate a 256-bit key, stored
securely and never written to config.yml. Refuse to start when the key is
empty or the known-insecure literal. Tighten config.yml to 0600 / dir 0700
(it persists the admin password in plaintext) and scrub the literal from
.env.example and config/config.yml.

This is Phase 1 of #1067 (secure credential storage); admin-password
migration and the disable-plaintext toggle follow in Phase 2.

Refs: #1067
@madhavilosetty-intel
madhavilosetty-intel force-pushed the fix/jwt-key-secure-handling branch from 0c93db7 to e840c17 Compare July 11, 2026 00:10
@sudhir-intc

Copy link
Copy Markdown
Contributor

@madhavilosetty-intel : This issue needs to be resolved independant of the secret store related support as described in #1067 or if you think that a secret store support is needed to fix this issue then we need to first address #1067 before having the issue fixed in parts.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants