Skip to content

fix(startup): reject malformed environment config (EN-1922) - #1847

Open
gfyrag wants to merge 2 commits into
release/v3.0from
fix/en-1922-malformed-env
Open

fix(startup): reject malformed environment config (EN-1922)#1847
gfyrag wants to merge 2 commits into
release/v3.0from
fix/en-1922-malformed-env

Conversation

@gfyrag

@gfyrag gfyrag commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Traceability

  • Jira: EN-1922
  • Native audit finding: process-boundary-recovery/malformed-env-falls-back-to-default
  • Challenge status: CONFIRMED

Before fix

At target base 23b4f498138b3ca222b52533fb683830fac38149, the production binary was started with GRPC_PORT=not-an-integer and otherwise valid bootstrap, bind, HTTP, WAL, and data arguments. The process remained running, reported effective grpcport: 0, opened gRPC on an ephemeral port and HTTP on the requested port, and later exited 0 after SIGTERM. The typed pflag setter had mutated the value to zero before returning its discarded parse error. A second production run with GRPC_COMPRESSION=not-a-bool preserved the valid default false and also reached running state.

BEFORE_FIX=BUG_REPRODUCED

Root cause and ownership

service.Execute uses the shared go-libs environment binder. That binder discarded errors returned by typed pflag.Set, allowing malformed selected environment configuration to continue. This shared primitive is the correct ownership boundary, so the implementation is in formancehq/go-libs#673. This PR is the minimal Ledger dependency update plus a production-boundary regression and deployment contract.

FIX_OWNERSHIP=GO_LIBS

Precedence contract

Configuration precedence remains explicit CLI flag, then environment, then default. A malformed environment value selected by that precedence fails. A valid explicit CLI value for the same flag wins and causes the malformed lower-precedence environment value to be ignored. This was verified with a real process: malformed GRPC_PORT plus --grpc-port 56735 started on 56735 with no environment error.

Fix and after-fix evidence

The pinned go-libs candidate propagates typed parse errors with environment and flag context while preserving early binding for command initialization and deferring selected-value failure until after Cobra parses explicit flags. The Ledger regression exercises server.NewRootCommand through service.Execute in a subprocess.

With the candidate binary and GRPC_PORT=not-an-integer, startup exited 1 with:

binding environment variable GRPC_PORT to flag --grpc-port: invalid argument "not-an-integer" for "--grpc-port" flag: strconv.ParseInt: parsing "not-an-integer": invalid syntax

No WAL or data directory was created and no application or gRPC startup marker appeared.

AFTER_FIX=PASS

Regression sensitivity

The shared binder was temporarily mutated to discard the parse error again. The Ledger process-boundary regression became red, observing grpcport: 0 followed by an unrelated missing cluster-id failure instead of the required binding diagnostic. The mutation was fully restored, both worktrees were confirmed clean of mutation artifacts, and the focused tests passed again.

TEST_SENSITIVITY=PASS

Validation

  • Nix Go 1.26.5 from /nix/store; hermetic caches outside the candidate worktree
  • go test -race . ./cmd/server -count=1
  • bash scripts/agent-check-full
  • bash scripts/ai-bugfix-gate pre-push
  • real-process malformed-env failure and explicit-CLI precedence proofs
  • clean exact candidate f8c649fb6c072e02e16ed8dec5f48960d1bb35fb

Shared-library validation in formancehq/go-libs#673 includes its focused race suite, full race-enabled suite, pre-commit, CI, and exact-head NumaryBot approval.

Dependency ordering

Merge formancehq/go-libs#673 first, then update this PR to the resulting release tag if maintainers publish one. This candidate pins the exact reviewed shared commit and deliberately excludes the unrelated pyroscope work in Ledger #1798.

The candidate was rebased onto the refreshed release/v3.0 tip 46b21f68b671ab70e3219bd54dcb721f74eea4c4; the final Ledger head is f8c649fb6c072e02e16ed8dec5f48960d1bb35fb, and the dependency now tracks go-libs 77516f9f3e01cd99633cdf55b223b1de24ff0e66.

@NumaryBot

NumaryBot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

✅ Approve — automated review

The dependency update, documentation, and subprocess regression test consistently enforce malformed environment configuration failure before startup. No actionable correctness issues were found.

No findings.

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.04%. Comparing base (46b21f6) to head (f8c649f).

Additional details and impacted files
@@               Coverage Diff                @@
##           release/v3.0    #1847      +/-   ##
================================================
+ Coverage         76.90%   77.04%   +0.14%     
================================================
  Files               472      472              
  Lines             50453    50453              
================================================
+ Hits              38800    38874      +74     
+ Misses             8252     8177      -75     
- Partials           3401     3402       +1     
Flag Coverage Δ
e2e 77.04% <ø> (+0.14%) ⬆️
scenario 77.04% <ø> (+0.14%) ⬆️
unit 77.04% <ø> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Azorlogh
Azorlogh previously approved these changes Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants