fix: persist generated JWT secret across restarts - #65
Conversation
A missing JWT_SECRET used to be invented on every boot, so hub sessions and dcc-bus daemons diverged after a restart. Generate the secret once and write it into loco-server.conf so the default hub image is production-safe without operator action. Co-authored-by: Cursor <cursoragent@cursor.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This Pull Request provides a robust solution for persisting the JWT secret, which is a significant improvement for session stability across restarts. The new PersistJWTSecret function correctly handles file creation, updates existing configurations, and enforces secure file permissions (0600). The updated precedence logic for resolving the JWT secret is clearly documented and implemented, ensuring a consistent and secure approach to secret management. The addition of comprehensive tests for the persistence logic further strengthens the change.
| // PersistJWTSecret writes JWT_SECRET=<secret> into path, replacing a | ||
| // commented or empty JWT_SECRET line and creating the file when missing. | ||
| // The file mode is 0600 because it now holds a signing key. | ||
| func PersistJWTSecret(path, secret string) error { |
There was a problem hiding this comment.
✨ [POSITIVE] POSITIVE: The PersistJWTSecret function is well-implemented, handling file creation, updates, and ensuring secure file permissions (0600). The os.Chmod call after os.WriteFile is a good defensive measure to guarantee the correct permissions are set, regardless of whether the file was newly created or already existed.
| return os.Chmod(path, 0o600) | ||
| } | ||
|
|
||
| func upsertDotenvKey(text, key, value string) string { |
There was a problem hiding this comment.
✨ [POSITIVE] POSITIVE: The upsertDotenvKey and isDotenvKeyLine helper functions are robust, correctly handling commented lines, case-insensitivity for keys, and ensuring proper newline handling when appending.
| t.Fatalf("DefaultPath() = %q, want %q", got, want) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
✨ [POSITIVE] POSITIVE: The new tests provide excellent coverage for the JWT secret persistence logic, including scenarios for replacing commented lines, creating new files, and verifying file permissions. This ensures the reliability of the new functionality.
| cmd.Flags().StringVar(&f.JWTSecret, "jwt-secret", "", | ||
| "hex/base64 secret used to sign session JWTs. Falls back to BIGFRED_JWT_SECRET "+ | ||
| "env var; a random per-run secret is generated when empty (sessions don't survive restarts).") | ||
| "hex/base64 secret used to sign session JWTs. Falls back to loco-server.conf JWT_SECRET, "+ |
There was a problem hiding this comment.
✨ [POSITIVE] POSITIVE: The updated flag description and the comment for resolveJWTSecret clearly articulate the new precedence order for JWT secret resolution, which now includes persistence to loco-server.conf. This improves clarity and maintainability.
| secret := hex.EncodeToString(buf) | ||
| path := config.DefaultPath() | ||
| if err := config.PersistJWTSecret(path, secret); err != nil { | ||
| log.WithError(err).WithField("path", path).Warn("generated JWT secret but could not persist it; sessions will not survive a restart") |
There was a problem hiding this comment.
✨ [POSITIVE] POSITIVE: The logging for both successful persistence and cases where persistence fails is very helpful for debugging and operational monitoring. It clearly indicates the state of the JWT secret generation and storage.
1 participant
Summary
--jwt-secret/JWT_SECRET/BIGFRED_JWT_SECRETwere empty, so hub sessions (and dcc-bus daemons spawned with--jwt-secret) died after a restart./data/etc/loco-server.conf(JWT_SECRET=…, mode 0600) on first boot and reused afterwards.Test plan
go test ./pkgs/bigfred/server/cli/...JWT_SECRET, start loco-server once, confirm the file now containsJWT_SECRET=<hex>and logsgenerated and persisted JWT secretMade with Cursor