fix(settings): reject KEY_HMAC_SECRET dev default in all environments#34
Open
Shaivpidadi wants to merge 1 commit intodevfrom
Open
fix(settings): reject KEY_HMAC_SECRET dev default in all environments#34Shaivpidadi wants to merge 1 commit intodevfrom
Shaivpidadi wants to merge 1 commit intodevfrom
Conversation
Cipher's review of GOV-573 (precheck#31) flagged that key_hmac_secret's default shifted from "" to the public _DEFAULT_KEY_HMAC_SECRET marker. In DEBUG=true the service therefore signed API-key HMACs with a known string, making any hash trivially forgeable. Because KEY_HMAC_SECRET is the API-key identity boundary (not a recoverable webhook signature), the dev marker must be rejected unconditionally. - Document the _DEFAULT_KEY_HMAC_SECRET constant and the key_hmac_secret field: debug HMAC keys are deterministic and public; never restore a debug database into a non-debug environment. - Promote the default-marker check for KEY_HMAC_SECRET out of the non-debug branch so it runs in every environment, including DEBUG=true. - Add parametrized tests covering both debug=true and debug=false, plus a positive test that non-default dev values are still accepted. Refs: GOV-1486
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
1 participant
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.
Summary
Follow-up to Cipher's GOV-573 review of precheck#31. The default for
key_hmac_secretshifted from\"\"to_DEFAULT_KEY_HMAC_SECRET, which meant the service signed API-key HMACs with a publicly-known marker string underDEBUG=true. BecauseKEY_HMAC_SECRETis the API-key identity boundary (not a recoverable webhook signature), the dev marker is now rejected in every environment.Changes
app/settings.py— Added explanatory comment on_DEFAULT_KEY_HMAC_SECRETand a docstring on thekey_hmac_secretfield noting the debug-marker danger.app/settings.py— Promoted the default-marker check forKEY_HMAC_SECRETout of theif not self.debug:branch so it runs unconditionally.tests/test_settings.py— Added a parametrized test overDEBUG=trueandDEBUG=falseconfirming the dev marker is rejected in both, plus a positive test that a non-default dev value still starts cleanly.Breaking change
Operators running with
DEBUG=trueand the defaultKEY_HMAC_SECRETwill now fail fast at startup. The repo'stests/conftest.pyalready sets a unique value (test-hmac-secret-for-ci-only), andenv.examplealready flags the variable as required — any local dev env that relied on the silent default must now set one.GovernsAI Tracker issue
GOV-1486
Reviewers
Tagging Nexus (code quality) and Cipher (security/arch) — both approvals required.
Test plan