feat(security): support Fernet key rotation via MultiFernet - #69
Merged
Conversation
FERNET_KEY encrypts the GitHub and Claude credentials in the database, and replacing it made every one of them unreadable — so in practice it could not be replaced, which is a bad property for a key whose exposure is the whole reason it exists. The crypto helpers now take an ordered keyset and go through MultiFernet: encrypt with the first key, decrypt with whichever one matches. A new FERNET_KEY_FALLBACKS setting holds retired keys, so a rotation is a deploy rather than an outage. Fallbacks are validated at boot alongside the primary key. A malformed retired key must not surface at first decrypt — the row that would break is exactly the credential the rotation is trying not to lose. `helprs.scripts.rotate_credentials` finishes the job by re-encrypting stored rows under the primary key, using MultiFernet.rotate (no plaintext needed). Without it the fallback list grows forever and a "retired" key stays as sensitive as the live one. It rewrites in a single transaction, is safe to re-run, and a row no configured key can read is reported and left intact rather than overwritten. The keyset is typed `list[str]` rather than `Sequence[str]` deliberately: `str` satisfies `Sequence[str]`, so a caller still passing a single key would have type-checked cleanly. mypy caught all six call sites this way. `settings.fernet_keys` is a plain property, not a computed_field — the latter would put every key back into model_dump() output and undo the SecretStr work from #65. Docs: a rotation runbook in self-hosting.md, the new setting in .env.example, and the stale "in development mode, any password is accepted" line next to ADMIN_PASSWORD removed — that bypass was fixed in #60.
|
helPRs session created for this PR. Skill: |
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.
FERNET_KEYencrypts the GitHub OAuth tokens and Claude BYOK credentials stored in the database. Replacing it made all of them unreadable at once — so in practice it could not be replaced, which is an unfortunate property for a key whose possible exposure is the entire reason it exists.What changed
The crypto helpers take an ordered keyset and go through
MultiFernet: encrypt with the first key, decrypt with whichever one matches. A newFERNET_KEY_FALLBACKSsetting holds retired keys, so rotation is a deploy rather than an outage:Fallbacks are validated at boot alongside the primary key, and the error names the offending index. A malformed retired key must not surface at first decrypt — the row that would break is precisely the credential the rotation is trying not to lose.
The script is the half that makes it finish
uv run python -m helprs.scripts.rotate_credentialsre-encrypts every stored credential under the primary key, usingMultiFernet.rotateso it never needs the plaintext. Without it the fallback list grows forever and a "retired" key stays exactly as sensitive as the live one — the key is only genuinely retired once nothing is still encrypted with it.list_all_byok_configsdeliberately ignores the_active()soft-delete filter — a soft-deleted installation's ciphertext still pins the old key.Two design notes worth the review
The keyset is
list[str], notSequence[str]. I wroteSequence[str]first and the test suite exploded while mypy stayed silent — becausestrsatisfiesSequence[str], so every call site still passing a single key type-checked cleanly and only failed at runtime, insideFernet(key.encode())on a one-character string.list[str]closes that: mypy found all six call sites itself.settings.fernet_keysis a plain@property, not acomputed_field. A computed field would put every key back intomodel_dump()output and quietly undo theSecretStrwork from #65. There is a test asserting a fallback key does not appear inrepr/str/model_dump.Verification
ruff+ruff format+mypycleanrotateis idempotent;rotatefails loudly when no key matches; an empty keyset is rejectedDocs
A rotation runbook in
docs/self-hosting.md, the new setting in.env.example, and the keyset invariant in CLAUDE.md.Also removed a stale line sitting next to
ADMIN_PASSWORDin.env.example: "In development mode, any password is accepted." That was true, it was the auth bypass fixed in #60, and it was still telling self-hosters the panel is open in development.