fix: reject duplicate recipient policy keys in BuildPolicyJson#48
Merged
Conversation
Two recipients sharing an email collapsed to the same policy key and one silently overwrote the other, dropping a recipient from the sealed policy. Throw ArgumentException(nameof(recipients)) on collision, mirroring the duplicate-entry guard in ZipHelper.CreateZip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
Contributor
Author
There was a problem hiding this comment.
VERDICT: approve
Rules Dobby 2 — sign-off (cycle 1)
Reviewed against the applicable rule set (conventional-commit PR title, Closes keyword, reviewer assignment) plus the correctness of the diff. No blocking or non-blocking findings.
SealPipeline.BuildPolicyJsonnow rejects a duplicate policy key withArgumentException(nameof(recipients))viaDictionary.TryAdd, mirroring the duplicate-entry guard documented forZipHelper.CreateZip— instead of silently overwriting and dropping a recipient.- The throw reuses the existing
nameof(recipients)param-name convention already present in the same method's unknown-base-type guard. BuildPolicyJsonTests.DuplicatePolicyKey_Throwscovers two recipients colliding on the same email and assertsParamName == "recipients".- Title is conventional-commit compliant;
Closes #47links the issue;@rubenhensenis requested as reviewer. - Tests: 83/83 pass on net10.0; both TFMs build (net8.0 runtime absent in workspace by design — CI covers it).
No security advisory referenced — not embargoed.
rubenhensen
approved these changes
Jul 24, 2026
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.
SealPipeline.BuildPolicyJsonkeyed the policy dictionary by recipient email withpolicy[r.Email] = .... Two recipients that resolve to the same email collapsed to one key, so one silently overwrote the other and was dropped from the sealed policy.This mirrors the duplicate-entry guard already in
ZipHelper.CreateZip: on collision it now throwsArgumentExceptionwithnameof(recipients)as the parameter name instead of overwriting.A regression test in
BuildPolicyJsonTestscovers two recipients colliding on the same email; it fails before the change and passes after.Tests run on net10.0 (83 passed); net8.0 builds but its runtime is absent in the workspace, so CI covers that TFM.
Closes #47