Skip to content

fix: generate strong unique JWT key#1071

Open
graikhel-intel wants to merge 1 commit into
mainfrom
jwtkey
Open

fix: generate strong unique JWT key#1071
graikhel-intel wants to merge 1 commit into
mainfrom
jwtkey

Conversation

@graikhel-intel

@graikhel-intel graikhel-intel commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

JWT key generation and persistence

  • New install (no config file): auto-generate a 32-byte random hex key and write it to config.yml.
  • Existing config with empty jwtKey: auto-generate and persist without touching other fields.
  • Existing config with non-empty jwtKey: leave untouched.

Environment variable semantics (AUTH_JWT_KEY)

State Behaviour
Not set Auto-generate and persist
Set and non-empty Use as-is (warn if under 32 bytes)
Set but empty / whitespace Fail startup with ErrEmptyJWTKeyEnv

Weak key warning

  • Logged at startup if the effective key is under 32 bytes, regardless of source (env or config file).
  • Backward compatible — app continues to run, no hard rejection (reserved for a future major release).

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.38462% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.53%. Comparing base (fe3c5f0) to head (128ee76).

Files with missing lines Patch % Lines
config/config.go 65.38% 11 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1071      +/-   ##
==========================================
+ Coverage   43.39%   43.53%   +0.13%     
==========================================
  Files         141      141              
  Lines       13397    13447      +50     
==========================================
+ Hits         5814     5854      +40     
- Misses       7022     7024       +2     
- Partials      561      569       +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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to improve JWT signing-key safety by generating a strong unique key on first-run (when creating a new config), warning when the configured key appears weak, and adding a controller-side key-length validation plus related test updates.

Changes:

  • Generate and persist a random JWT signing key when initializing a brand-new config.yml (unless AUTH_JWT_KEY is set).
  • Add a startup warning when auth.jwtKey looks too short.
  • Add HS256 key-length validation in the v1 devices redirection JWT endpoint and extend test coverage around key length validation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
internal/controller/httpapi/v1/devices.go Adds HS256 key-length validation before signing redirection JWTs.
internal/controller/httpapi/v1/devices_test.go Updates test JWT key and adds unit tests for HS256 key-length validation helper.
config/config.go Generates a random JWT key for new configs and warns if key is weak.
config/config_test.go Adds tests for JWT key generation/persistence, env override behavior, and warning logging.

Comment thread internal/controller/httpapi/v1/devices.go Outdated
Comment thread internal/controller/httpapi/v1/devices_test.go Outdated
Comment thread config/config.go Outdated
Comment thread config/config.go Outdated
Comment thread config/config_test.go Outdated
Comment thread config/config_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comment thread config/config.go Outdated
Comment on lines +249 to +251
if _, isSetByEnv := os.LookupEnv("AUTH_JWT_KEY"); isSetByEnv {
return nil
}
Comment thread config/config.go Outdated

var pathErr *os.PathError
if errors.As(err, &pathErr) {
if _, isSetByEnv := os.LookupEnv("AUTH_JWT_KEY"); !isSetByEnv {
Comment thread .env.example Outdated
# AUTH_ADMIN_PASSWORD: If not set, a random password is generated
AUTH_ADMIN_PASSWORD=
AUTH_JWT_KEY=your_secret_jwt_key
# AUTH_JWT_KEY: If not set, a strong key is generated and saved on first run
Comment thread config/config_test.go
Comment on lines +205 to +210

func TestReadOrInitConfig_DoesNotMutateExistingNonEmptyJWTKey(t *testing.T) {
t.Parallel()

tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "config.yml")
Comment thread config/config.go
return nil, err
}

warnIfWeakJWTKey(ConsoleConfig)

@sudhir-intc sudhir-intc Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we not exit if the key length is < 32 than instead of logging a warning.
For a customer or developer they would hardly notice the warning in the log and see the console coming up successfuly

@graikhel-intel graikhel-intel force-pushed the jwtkey branch 3 times, most recently from 4c0be18 to c3c4411 Compare June 15, 2026 07:12
- Auto-generate a 32-byte random hex key on first run when jwtKey is
  empty in config and AUTH_JWT_KEY is not set
- Persist generated key to config.yml without touching other fields
- Fail startup when AUTH_JWT_KEY is set but empty (ErrEmptyJWTKeyEnv)
- Warn at startup when effective key is under 32 bytes (any source)
- Remove hardcoded placeholder jwtKey from config defaults and templates
- Parameterise docker-compose credentials via env variables
- Add tests for all key-handling paths
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.

3 participants