Merge remote-tracking branch 'origin/dev/v2.1' into feat/warn-unknown-sockguard-env - #469
Conversation
Viper's AutomaticEnv only consults a variable for a key it already knows, so SOCKGUARD_LISTEN_SOCKT was read by nothing: the listener stayed on its config-file or default value and no output said so. On a default-deny proxy that silence is the dangerous direction, because the operator believes they tightened something. serve and validate now log one line per unrecognized variable, naming it and, when the spelling is within two edits of a real one, the variable it was probably meant to be. Only the name is logged, never the value. What counts as recognized is reconstructed from the loader's own Viper state, the registerDefaults walk over Config's mapstructure tags plus the config file, so it cannot drift from what Viper binds and a key that exists only because the YAML declares it is not reported as ignored.
|
Deployment failed for project sockguard-website with the following error: Learn More: https://vercel.com/codeswhat?upgradeToPro=build-rate-limit |
biggest-littlest
left a comment
There was a problem hiding this comment.
Reviewed against the CHANGELOG entry and the diff; CI green outside the qlty/Vercel quota noise.
ALARGECOMPANY
left a comment
There was a problem hiding this comment.
Reviewed against the CHANGELOG entry and the diff; CI green outside the qlty/Vercel quota noise.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe change detects unrecognized ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Viper's
AutomaticEnvonly consults a variable for a key it already knows about, soSOCKGUARD_LISTEN_SOCKT=/run/sockguard.sockwas read by nothing: the listener stayed on its config-file or default value and no output said so. That's the dangerous direction for a default-deny proxy, because the operator believes they tightened something.sockguard serveandsockguard validatenow both check the environment at startup and log one line per unrecognizedSOCKGUARD_*variable, naming the variable and, when the spelling is within two edits of a real one, the variable it was probably meant to be. Only the name is logged, never the value, since a typo lands on a variable holding a credential as easily as on one holding a socket path. The warning is advisory and never fails startup. What counts as recognized is reconstructed from the loader's own Viper state, theregisterDefaultswalk overConfig's mapstructure tags plus the config file, so it can't drift from what Viper binds, and a key that exists only because the YAML declares it isn't reported as ignored. Tecnativa compat vars carry noSOCKGUARD_prefix and are never flagged.New tests. In
app/internal/cmd/serve_warn_test.go,TestWarnUnknownEnvVarsNamesEachUnknownOncecovers one line per unknown variable, deduplication of a repeat, and that the value never reaches the log;TestWarnUnknownEnvVarsIgnoresKnownAndCompatVariablescovers correctly spelled keys at three nesting depths plus the unprefixed Tecnativa vars staying silent;TestWarnUnknownEnvVarsSuggestsNearestKnownVariableis a table over a dropped letter, a missing underscore, a singular for a plural, and a name with nothing close enough to suggest.TestRunServeWarnsOnUnknownEnvVarandTestRunValidateWarnsOnUnknownEnvVar(inapp/internal/cmd/validate_test.go) prove each command actually emits it rather than just exposing the helper. Inapp/internal/config/env_unknown_test.go,TestUnknownEnvVarsAcceptsKeysTheConfigFileDeclarespins the case that drove the file-aware derivation, since a pointer block declared in YAML really is overridable from the environment, andTestUnknownEnvVarsSurvivesAnUnusableConfigFilecovers a missing or malformed file falling back to the schema half instead of accepting everything. Every one of them fails with the warning stubbed out.CHANGELOG entry went under
### Added.Changelog
✨ Added warnings for unknown
SOCKGUARD_*variables inserveandvalidate.✨ Added typo suggestions with
did_you_meanfor close variable names.✨ Added warning deduplication and value omission.
✨ Added recognition of schema keys, defaults, mapstructure tags, and YAML-declared keys.
✨ Added tests for warnings, suggestions, compatibility variables, config files, and command integration.
✨ Added configuration documentation.
Verify warning detection does not widen accepted socket or configuration access.
Verify malformed or unreadable configuration files cannot suppress unknown-variable warnings.
Verify warning output remains stable and does not expose environment values.