HYPERFLEET-1133 - fix: make production the default runtime environment#191
HYPERFLEET-1133 - fix: make production the default runtime environment#191ldornele wants to merge 3 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR changes the default runtime environment constant from DevelopmentEnv to ProductionEnv, adds a test asserting EnvironmentDefault is ProductionEnv (with Gomega dot-import), and updates docs to state the binary defaults to production (JWT/TLS enabled) and how to run with authentication disabled for local development. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Risk Score: 2 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 13 lines | +0 |
| Sensitive paths | cmd/ | +2 |
| Test coverage | Tests cover changed packages | +0 |
Computed by hyperfleet-risk-scorer
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/hyperfleet-api/environments/framework_test.go (1)
56-59: ⚡ Quick winConsider a behavioral test, not just constant equality.
This guards the constant, which is a fine regression check. But the actual security guarantee from this PR is that an unset
HYPERFLEET_ENVresolves to a config with JWT and TLS enabled. A test that initializes the environment with noHYPERFLEET_ENVset and asserts JWT/TLS-enabled on the resolved config would catch regressions inOverrideConfig()wiring that a const comparison cannot.Want me to draft a test that loads defaults with
HYPERFLEET_ENVunset and asserts JWT/TLS are enabled?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/hyperfleet-api/environments/framework_test.go` around lines 56 - 59, Add a behavioral test that unsets HYPERFLEET_ENV, constructs the environment/config via the same codepath used in production (call the function that resolves/overrides config—e.g., OverrideConfig or the environment loading function used in tests) and assert that the resulting config has JWT and TLS enabled (e.g., cfg.JWT.Enabled == true and cfg.TLS.Enabled == true). Use os.Unsetenv("HYPERFLEET_ENV") at the start of the test, then call the environment resolution function referenced in the codebase (OverrideConfig / the environment loader used alongside EnvironmentDefault) and use the existing test helpers (RegisterTestingT/Expect) to check JWT and TLS flags so regressions in wiring are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/hyperfleet-api/environments/framework_test.go`:
- Around line 56-59: Add a behavioral test that unsets HYPERFLEET_ENV,
constructs the environment/config via the same codepath used in production (call
the function that resolves/overrides config—e.g., OverrideConfig or the
environment loading function used in tests) and assert that the resulting config
has JWT and TLS enabled (e.g., cfg.JWT.Enabled == true and cfg.TLS.Enabled ==
true). Use os.Unsetenv("HYPERFLEET_ENV") at the start of the test, then call the
environment resolution function referenced in the codebase (OverrideConfig / the
environment loader used alongside EnvironmentDefault) and use the existing test
helpers (RegisterTestingT/Expect) to check JWT and TLS flags so regressions in
wiring are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6f9ed444-ec1b-44d5-a58d-687c1d311a52
📒 Files selected for processing (1)
cmd/hyperfleet-api/environments/framework_test.go
Summary
Changes the default runtime environment from
developmenttoproductionto enforce secure-by-default behavior.Problem: When
HYPERFLEET_ENVis not set, the server currently defaults todevelopmentmode, which silently disables JWT authentication and TLS. This creates a security risk — any deployment that omitsthe environment variable runs without authentication.
Solution: Set
EnvironmentDefault = ProductionEnvso that missing configuration results in a secured service, not an open one.Changes
EnvironmentDefaultfromDevelopmentEnvtoProductionEnvHow Environment Configuration Works
The application follows a two-stage configuration process:
OverrideConfig()to adjust settings per environmentThis means the codebase is secure by default — environments must explicitly opt-out of security features.
Environment Configuration Differences
e_production.go(new default):