fix(auth): enforce required JWT key to prevent forged token attacks

[nbmaiti]

Console shipped with a hardcoded default JWT signing key (your_secret_jwt_key), so any deployment that didn't override it could have session tokens forged by anyone who read the source. This removes the default and makes auth.jwtKey a required setting — NewConfig() now fails fast at startup with ErrJWTKeyMissing if AUTH_JWT_KEY (or jwtKey in config.yml) isn't set.

Also updates existing tests that called NewConfig() without a key (they broke once it became required) and adds coverage for the new validation branch.

[Copilot]

Pull request overview

This PR hardens authentication configuration by removing an insecure default JWT signing key and introducing startup-time validation to prevent running with an unset JWT key.

Changes:

• Removed the hardcoded default auth.jwtKey value (your_secret_jwt_key) from the in-memory default config.
• Added config validation to fail fast when auth.jwtKey is empty.
• Introduced a dedicated config error (ErrJWTKeyMissing) for missing JWT key configuration.

Suppressed comments (2)

config/config.go:423

• Current jwtKey validation only rejects an empty string. Existing deployments (and the repo's config/config.yml) may still use the placeholder value "your_secret_jwt_key", which would pass validation and keeps the forged-token risk this PR intends to prevent. Consider explicitly rejecting that placeholder value as well.

	if c.JWTKey == "" {
		return ErrJWTKeyMissing
	}

config/config.go:423

• New startup validation for auth.jwtKey should be covered by unit tests (e.g., NewConfig/validate returns ErrJWTKeyMissing when jwtKey is empty or set to the placeholder). This helps prevent regressions in config loading precedence (defaults vs YAML vs env).

	if c.JWTKey == "" {
		return ErrJWTKeyMissing
	}

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.83%. Comparing base (ec2b707) to head (2e45f09).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1223   +/-   ##
=======================================
  Coverage   50.82%   50.83%           
=======================================
  Files         149      149           
  Lines       13872    13874    +2     
=======================================
+ Hits         7051     7053    +2     
  Misses       6217     6217           
  Partials      604      604           

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

internal/controller/ws/v1/redirect_test.go:25

• Removing the function-level //nolint:paralleltest reintroduces a paralleltest lint violation (this test does not call t.Parallel() and uses shared env/global config + mock logger expectations). Re-add the nolint on the test function to keep CI linting consistent with the existing non-parallel design.

func TestWebSocketHandler(t *testing.T) {

internal/controller/ws/v1/redirect_test.go:108

• Removing the function-level //nolint:paralleltest reintroduces a paralleltest lint violation (this test does not call t.Parallel() and relies on shared env/global config). Re-add the nolint on the test function.

func TestWebSocketHandlerDeviceBinding(t *testing.T) {

internal/controller/ws/v1/redirect_test.go:236

• Removing the function-level //nolint:paralleltest reintroduces a paralleltest lint violation (this test does not call t.Parallel() and relies on shared env/global config). Re-add the nolint on the test function.

func TestWebSocketHandlerTokenValidation(t *testing.T) {

internal/app/app_test.go:101

• os.Setenv in a parallel test can leak configuration into other tests in the same package and is not automatically restored. Prefer t.Setenv, and set env vars before calling t.Parallel() so the setup happens while the test is still running serially.

	os.Setenv("AUTH_JWT_KEY", "test-jwt-key")

	cfg, _ := config.NewConfig()

config/config_test.go:25

• clearEnv() is used to isolate config/env tests, but it doesn’t currently unset AUTH_JWT_KEY. With the new required JWT key, this can leak between tests and make failures/order-dependence hard to debug. Update clearEnv() to unset AUTH_JWT_KEY, and prefer t.Setenv for per-test setup so values are automatically restored.

func TestNewConfig_Defaults(t *testing.T) { //nolint:paralleltest // cannot have simultaneous tests modifying environment variables
	clearEnv() // Clear environment variables to ensure defaults are tested
	os.Setenv("AUTH_JWT_KEY", "test-jwt-key-for-default-testing")

[Copilot]

Pull request overview

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

Suppressed comments (5)

config/config_test.go:52

• This test mutates process-wide environment variables but no longer has a //nolint:paralleltest suppression. With paralleltest enabled (.golangci.yml:36), that will raise a lint error unless the test calls t.Parallel() (which isn't safe here).

func TestNewConfig_EnvVars(t *testing.T) {

config/config_test.go:310

• This test performs filesystem writes (fixed relative path) and mutates environment variables, so it can't be safely parallelized. With paralleltest enabled (.golangci.yml:36), it should either call t.Parallel() or keep a //nolint:paralleltest suppression with an explanation.

func TestNewConfig_FileAndEnvVars(t *testing.T) {

internal/controller/ws/v1/redirect_test.go:25

• This test uses t.Setenv(), which makes it incompatible with t.Parallel(). With paralleltest enabled (.golangci.yml:36), the function needs a //nolint:paralleltest suppression (or a refactor that avoids env mutation).

func TestWebSocketHandler(t *testing.T) {

internal/controller/ws/v1/redirect_test.go:108

• This test uses t.Setenv(), which is not compatible with t.Parallel(). With paralleltest enabled (.golangci.yml:36), re-add a //nolint:paralleltest suppression on the test function (the existing suppressions on subtests/loops don't cover the top-level function).

func TestWebSocketHandlerDeviceBinding(t *testing.T) {

internal/controller/ws/v1/redirect_test.go:236

• This test uses t.Setenv(), which prevents safely calling t.Parallel(). With paralleltest enabled (.golangci.yml:36), add a //nolint:paralleltest suppression on the test function signature to avoid a lint failure.

func TestWebSocketHandlerTokenValidation(t *testing.T) {

[Copilot]

🟡 Changes recommended

Config validation currently requires a JWT key even when auth is disabled, which can block valid AUTH_DISABLED deployments and is a functional regression.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

internal/app/app_test.go:101

• This test discards the error from config.NewConfig(). If config construction fails, cfg may be nil and later assignments will panic; asserting NoError keeps the failure message actionable.

func TestRun(t *testing.T) { //nolint:tparallel // t.Setenv is incompatible with t.Parallel, so this test can't mark itself parallel even though its subtest does
	t.Setenv("AUTH_JWT_KEY", "test-jwt-key")

	ctrl := gomock.NewController(t)

	t.Cleanup(ctrl.Finish)
	defer ctrl.Finish()

	mockDB := mocks.NewMockDB(ctrl)
	mockHTTPServer := mocks.NewMockHTTPServer(ctrl)

	cfg, _ := config.NewConfig()
	cfg.Provider = ProviderPostgres
	cfg.DB.URL = "postgres://testuser:testpass@localhost/testdb"

internal/controller/ws/v1/redirect_test.go:116

• The test ignores the error from config.NewConfig(). If NewConfig fails, ConsoleConfig may be nil and subsequent mutations will panic; asserting NoError makes failures clearer and avoids nil dereferences.

	t.Setenv("AUTH_JWT_KEY", "test-jwt-key")

	_, _ = config.NewConfig()

	config.ConsoleConfig.Disabled = false

internal/controller/ws/v1/redirect_test.go:244

• The test ignores the error from config.NewConfig(). If NewConfig fails, ConsoleConfig may be nil and the test will panic later; assert the error to make failures deterministic and easier to diagnose.

	t.Setenv("AUTH_JWT_KEY", "test-jwt-key")

	_, _ = config.NewConfig()

	config.ConsoleConfig.Disabled = false

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

[sudhir-intc]

@nbmaiti - Please update the config/config.yaml and .env.example to remove the default values

Changes implemented

[RosieAMT]

🎉 This PR is included in version 1.40.1 🎉

The release is available on:

• GitHub release
• v1.40.1

Your semantic-release bot 📦🚀

[RosieAMT]

🎉 This PR is included in version 1.40.1 🎉

The release is available on:

• GitHub release
• v1.40.1

Your semantic-release bot 📦🚀

[RosieAMT]

🎉 This PR is included in version 1.40.1 🎉

The release is available on:

• GitHub release
• v1.40.1

Your semantic-release bot 📦🚀