Repository navigation
Configuration: a file that does not parse stops the service - #316
Merged
Merged
Conversation
get_configuration fell back to Settings::default() whenever the configuration failed to deserialize, silently: auth at localhost, the database at 127.0.0.1. A typo in a production configuration.yaml would start a service on those defaults. A configuration that is there but does not parse is now an error. With no configuration file at all the defaults still apply: CI and the test harness run that way (configuration.yaml.dist is not read - `with_name` does not recognise the `.dist` extension). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found by the
user.rolesecurity audit. Companion of the OAuth fallback PR.Problem.
get_configuration(src/configuration.rs) started fromSettings::default()and replaced it onlyif let Ok(loaded) = settings.try_deserialize(). A configuration that failed to parse, for example a typo or a wrong type in a productionconfiguration.yaml, was silently ignored. The service then ran with auth athttp://localhost:8080/meand the database at 127.0.0.1, with nothing in the log to explain why.Change. New
settings_from(config), which holds the same code as before: deserialize, then the environment overlays.mainstops with it.configuration.yaml.distis never actually loaded:config::File::with_namedoes not recognise the.distextension. That is why CI has always run on the defaults. This PR does not change that.Tests
a_configuration_that_does_not_parse_is_an_error: failed against the old code (fell back to the default settings), passes now.no_configuration_at_all_runs_on_the_defaults.cargo test --libpasses (2305).tests/bake_channels.rspasses both with a localconfiguration.yamland without one (only the.distfile present, as in CI).Production is not affected: the database is not read from the environment, so production could not run on the defaults, which means its configuration does parse.
🤖 Generated with Claude Code