fix(security): remove hardcoded "secret" fallback and fail closed when JWT_SECRET is missing

[Xx-173]

Closes #97.

What

env.String("JWT_SECRET", "secret") silently fell back to the committed default secret whenever JWT_SECRET was unset or empty. Because that value lives in the source tree, anyone can mint an HS256 token that passes signature verification for any user_id / eid.

This PR removes every committed fallback signing secret and makes the affected paths fail closed.

Changes

1. The signing secret is now required — api/common/utils/jwt/jwt.go

• secretKey default changes from "secret" to ""
• adds ErrSecretNotConfigured and SecretConfigured()
• all four exported entry points now return ErrSecretNotConfigured instead of signing / verifying with an empty key:
  • UserGenerateJWT, UserParseJWT (jwt.go)
  • GenerateUploadDelegateJWT, ParseUploadDelegateJWT (upload_delegate.go) — the delegated batch-upload paths referenced in the issue; they share the same package-level key, so they needed the same guard

2. Sandbox download tokens — api/common/utils/sandboxdl/token.go

• no longer falls back to "secret" through JWT_SECRET; it still prefers its own SANDBOX_DOWNLOAD_TOKEN_SECRET
• GenerateDownloadToken / ValidateDownloadToken fail closed when neither is configured

3. Fail fast at startup — api/main.go

• checks jwt.SecretConfigured() before any other initialization and refuses to boot with an actionable message
• deliberately not via logger.FatalLog: shouldLog() returns false for every level when LOG_LEVEL=NONE, so the check would be silently swallowed. It writes to stderr and exits directly.

4. Env templates — api/.env.example, docker/.env.example, docker/.env, api/docker/.env

• document the previously undocumented JWT_SECRET and SANDBOX_DOWNLOAD_TOKEN_SECRET keys

⚠️ Breaking change

JWT_SECRET did not appear in any env template, so existing deployments were running on the public default. After this change they must set it:

openssl rand -hex 32

and assign the result to JWT_SECRET. A deployment that does not will refuse to start (the message says exactly this) instead of booting with a forgeable key. Sessions signed with the old default also stop validating — that is intended, since the exposed value must be rotated.

Verification

Reproduced against current main (da0d5e45) and against the patched tree, using a token forged with the public default secret:

tree	JWT_SECRET	UserParseJWT(forged)
before	unset	accepted — uid=1 eid=1 err=<nil>
after	unset	rejected — JWT_SECRET is not configured
after	set	rejected — token signature is invalid

With a real secret configured the legitimate paths still round-trip: UserGenerateJWT/UserParseJWT, GenerateDownloadToken/ValidateDownloadToken, GenerateUploadDelegateJWT/ParseUploadDelegateJWT.

gofmt clean, go vet clean, go build ./... passes on go1.25.9.

Related finding, intentionally not fixed here

api/config/encryption.go has the same shape of bug: 53AIHub_ENCRYPTION_KEY falls back to the committed "default-encryption-key-32-bytes-long". I left it out on purpose, because that key encrypts WeChat Pay configuration at rest (api/service/payment/wechatpay.go), so changing it needs a rotation / migration story for rows that were already encrypted with the default. It does not belong in this patch. Happy to send it as a separate PR if useful.

Summary by CodeRabbit

• Security

  • Replaced publicly known default secrets with required secure configuration for JWT authentication.
  • The service now refuses to start when JWT_SECRET is missing.
  • Added optional SANDBOX_DOWNLOAD_TOKEN_SECRET, which uses the JWT secret when not separately configured.
  • Token generation and validation now stop safely when required secrets are unavailable.

• Documentation

  • Updated environment templates with the new security settings and guidance for generating secure secrets.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b3587443-71a2-494e-854c-91d4c512390f

📥 Commits

Reviewing files that changed from the base of the PR and between da0d5e4 and 898b075.

📒 Files selected for processing (8)

• api/.env.example
• api/common/utils/jwt/jwt.go
• api/common/utils/jwt/upload_delegate.go
• api/common/utils/sandboxdl/token.go
• api/docker/.env
• api/main.go
• docker/.env
• docker/.env.example

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The change removes hardcoded JWT and sandbox token secrets, documents required environment variables, adds fail-closed token operations, and stops application startup when JWT_SECRET is missing.

Changes

Signing secret configuration

Layer / File(s)	Summary
Signing key defaults api/common/utils/sandboxdl/token.go, api/.env.example, api/docker/.env, docker/.env, docker/.env.example	Sandbox tokens no longer use the hardcoded secret fallback. Environment templates document JWT_SECRET and the optional SANDBOX_DOWNLOAD_TOKEN_SECRET.
JWT token guards api/common/utils/jwt/jwt.go, api/common/utils/jwt/upload_delegate.go	JWT generation and parsing return ErrSecretNotConfigured when JWT_SECRET is empty. Upload-delegate operations use the same guard.
Startup enforcement api/main.go	Startup checks jwt.SecretConfigured(), prints setup instructions, and exits with status 1 when JWT_SECRET is missing.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Suggested reviewers: liuzimu1995

Merge Risk: ⚪ Minimal · up to 898b0

The change removes the public signing fallback, requires JWT_SECRET at startup, and fails token operations closed when signing keys are absent; the documented deployment requirement introduces no identified current-head merge-blocking risk.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	Issue #97 requires removal of the predictable fallback and fail-closed JWT paths. The reviewed code sets secretKey from JWT_SECRET with an empty default, returns ErrSecretNotConfigured from JWT …	Validate JWT_SECRET against a documented minimum strength and reject weak or known values before startup. Keep the fail-closed behavior for all signing and verification paths. Add automated tests for missing and weak secrets, configured J…
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 6 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main security change: removing the hardcoded JWT secret fallback and failing closed when JWT_SECRET is missing.
Out of Scope Changes check	✅ Passed	The reviewed changes stay within Issue #97. They remove the JWT fallback, add fail-closed behavior, protect delegated upload and sandbox token operations from missing secrets, add the startup guard, a…

Full details: Linked Issues check

Explanation

Issue #97 requires removal of the predictable fallback and fail-closed JWT paths. The reviewed code sets secretKey from JWT_SECRET with an empty default, returns ErrSecretNotConfigured from JWT operations, guards delegated upload operations, and refuses startup when SecretConfigured() is false. Existing authentication checks remain untouched. However, SecretConfigured() only checks that the value is non-empty. It does not require minimum strength or reject weak values. The changed-file summary also lists no automated tests for missing-secret failures or configured token round-trips. The templates recommend openssl rand -hex 32, but they do not enforce the high-entropy requirement.

Resolution

Validate JWT_SECRET against a documented minimum strength and reject weak or known values before startup. Keep the fail-closed behavior for all signing and verification paths. Add automated tests for missing and weak secrets, configured JWT round-trips, delegated upload behavior, and the startup failure path.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 6 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit found a secret bare,
And tucked a safer setting there.
No token hops on defaults bright,
Startup stops without the right.
Secure keys now guide the flight.}

_{Comment @coderabbitai help to get the list of available commands.}