Repository navigation
fix(stack-kms-vendor): let a key router hold at most one vendor by type - #1562
Draft
freshtonic wants to merge 1 commit into
Draft
freshtonic wants to merge 1 commit into
freshtonic wants to merge 1 commit into
Conversation
ADR-0009 allows ZeroKMS and at most one key vendor backend in a cipher. The decision log said the KeyRouter type held exactly one vendor. It did not: the ZeroKMS side took any DataKeySource, and a KeyRouter is one. So KeyRouter<KeyRouter<ZeroKMS, Aws>, Gcp> compiled. RecordError::SecondVendor catches two vendors only inside one VendorKeySource. Add the marker trait stack_kms::ZeroKmsKeySource. StackKms, the two test sources of stack-kms and the guest's MaybeZeroKms implement it. A router does not. KeyRouter::new and KeyRouter::zerokms_only now ask for it, so a nested router does not compile. A compile_fail doc test shows this, and it compiles when the bound is removed. The decision log now says what the type holds and that the first claim was wrong.
🦋 Changeset detectedLatest commit: 5f12aa4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true |
This branch has not been deployed
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.
Summary
A cipher may use ZeroKMS (CipherStash's key service) plus at most one key vendor backend: a customer's own key service, such as AWS KMS. The decision log said the
KeyRoutertype enforced this. It did not: the ZeroKMS side of the router took any key source, and a router is one. So a router that held a second router, and a second vendor, compiled. This PR makes the type hold the rule. No runtime behaviour changes.Changes
stack-kms: new marker traitZeroKmsKeySource, implemented forStackKms(the real ZeroKMS client) and its two test sources.stack-kms-vendor:KeyRouter::newandKeyRouter::zerokms_onlyask for it. No router implements it. Acompile_faildoc test shows a nested router does not compile.MaybeZeroKmsimplements it.@cipherstash/authpatch.Verification
compile_faildoc test passes with the bound. With the bound removed it fails ("Test compiled successfully, but it's markedcompile_fail"), so it fails for the right reason.cargo clippy --workspace --all-features --all-targets -- -D warnings, and the guest's clippy (native andwasm32-wasip1): pass.mise x --env test -- cargo nextest run --workspace --all-features: 1401 of 1402 pass. The one failure isstack-encrypt::ui derive_diagnostics(trybuild), which fails on this machine for every snapshot because of the local toolchain setup. CI is the check for it.mise run test:doc: pass.mise run wasm:guest:test: 80 of 80 pass. All four Go guests rebuilt, thengo test -count=1 ./...: pass.Related
Refs #1561. Review finding on the key vendor backend stack.