Skip to content

feat(config): unknown WALGIT__ overrides fail closed - #52

Merged
yordis merged 2 commits into
mainfrom
yordis/feat-config-fail-closed-overrides
Oct 29, 2026
Merged

yordis merged 2 commits into
mainfrom
yordis/feat-config-fail-closed-overrides

Conversation

@yordis

@yordis yordis commented Oct 29, 2026

Copy link
Copy Markdown
Member
  • A mistyped or wrong-typed WALGIT__ override was dropped with a warning and the server silently ran on the default, while the TOML file already refuses unknown keys.
  • Overrides for another binary's sections (D52) are still skipped silently, so one environment can keep configuring both binaries.
  • Brings in feat(config): unknown WALGIT__ overrides fail closed tobi/walgit#100 by jonasgrosch, cherry-picked with original authorship; the upstream test fails on our main and passes with this change.

An environment override this build could not apply was dropped with a
WARN: `WALGIT__SERVER__LSITEN=0.0.0.0:8080` left the server on the default
127.0.0.1:8080, a wrong-typed value likewise ran the default. The file
itself already refuses unknown keys (`deny_unknown_fields`); the env path
now does the same.

A malformed name, an unknown section or key, or a value of the wrong type
is an error at startup and in `walgit config check`, naming every such
variable at once and, for an unknown key, the nearest known one:

  WALGIT__SERVER__LSITEN: unknown key `lsiten` in [server]; did you mean WALGIT__SERVER__LISTEN?

Nothing of a refused batch is applied. The nearest key comes from serde's
own `expected one of` list (Levenshtein over the siblings, no new
dependency).

No escape hatch. The warn-and-ignore loader existed for config/image skew
(2026-08-21: a host on an older image crash-looped on a newer key), not
for foreign variables sharing the prefix. That case is answered by
ordering (image before the env that needs it) and by `config check
--env-file` run by the binary that will serve; `--strict` and
`apply_env_report` are removed, as the check is now always strict. An
override under another binary's FOREIGN path (D52) is still skipped
silently. Recorded as D53.

(cherry picked from commit 1eca457a)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The default endpoint is the rustfs/MinIO rig, which needs path-style addressing; the example showed AWS values. Pinned by a test.

(cherry picked from commit ee5f506)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Rollouts that relied on ignored unknown env keys will now fail startup until the image or env is fixed; operators should gate deploys with walgit config check --env-file on the binary that will run.

Overview
WALGIT__ environment overrides now fail closed (D53), matching TOML’s deny_unknown_fields behavior instead of warning and partially applying known keys. A malformed name, unknown key, or wrong type aborts startup and walgit config check with a single error listing every bad variable; nothing from that batch is applied, even valid overrides mixed in with typos.

Errors name each WALGIT__… variable and, for unknown keys, suggest the nearest real key (e.g. LSITEN → LISTEN). Overrides under another binary’s FOREIGN sections are still skipped silently (D52 unchanged).

CLI: walgit config check drops --strict and the “ignored overrides” messaging; --env-file now fails the same way startup would. New integration tests cover server exit, config check, and env files.

Docs and walgit.example.toml are updated (validation notes; [store.s3] defaults aligned with code defaults for local MinIO/rustfs).

Reviewed by Cursor Bugbot for commit 0817c13. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Oct 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 07a2b82b-f8f4-469c-94eb-321e51b65f96
Commits

Reviewing files that changed from the base of the PR and between 2668870 and 0817c13.

Files selected for processing: 10
  • AGENTS.md
  • crates/walgit-cli/src/config_cmd.rs
  • crates/walgit-cli/src/lib.rs
  • crates/walgit-cli/tests/config_env.rs
  • crates/walgit-config/src/lib.rs
  • crates/walgit-config/src/nats.rs
  • crates/walgit-config/src/settings.rs
  • docs/EVENTS.md
  • docs/PACKFILE_MIGRATION.md
  • walgit.example.toml
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@yordis yordis closed this Oct 29, 2026
@yordis yordis reopened this Oct 29, 2026
@yordis yordis closed this Oct 29, 2026
@yordis yordis reopened this Oct 29, 2026
@yordis
yordis merged commit 4a4db2b into main Oct 29, 2026
10 of 15 checks passed
@yordis
yordis deleted the yordis/feat-config-fail-closed-overrides branch October 29, 2026 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants