fix(identity): skip DKIM keygen on a domain reclaim - #906
Merged
jiashuoz merged 1 commit intoAug 17, 2026
Conversation
claimOrCreateDomain generated an RSA-2048 keypair and AEAD-sealed the private key before checking whether the domain row already existed. On a reclaim (the common idempotent path documented right above the function: "the DKIM keypair are minted on first INSERT and remain stable across re-claims") the SELECT branch returns the existing row and the freshly generated keypair is discarded, unused, on every call. Move the keygen/seal block into the pgx.ErrNoRows branch, right before the INSERT that is its only consumer, so it runs once per domain instead of once per claim call. The stored data is unchanged either way; only the wasted work on a reclaim goes away. Fixes tokencanopy#826 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
jiashuoz
approved these changes
Aug 17, 2026
Contributor
Author
|
Thanks for merging! |
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
claimOrCreateDomaingenerated a fresh RSA-2048 DKIM keypair (plus the AEADseal over its private key, #144) before checking whether the domain row
already existed. On a reclaim, the common idempotent path the function's own
docstring describes ("the DKIM keypair are minted on first INSERT and remain
stable across re-claims"), the SELECT branch returns the existing row and the
freshly generated keypair is discarded, unused. Every re-register of an
already-owned domain paid for a full RSA-2048 keygen for nothing.
The fix moves the keygen/seal block into the
pgx.ErrNoRowsbranch, rightbefore the INSERT that is its only consumer, so it runs once per domain
instead of once per call. Stored data is byte-for-byte identical either way;
this only removes the wasted work.
Operational risk
None. No stored data changes, no API response changes. The keygen/seal now
runs while holding the per-domain advisory lock (previously it ran before the
transaction started), which only extends lock hold time on the once-per-domain
new-row path; the existing
TestClaimOrCreateDomain_ConcurrentParentChildClaimsDoNotSplitOwnershipand
TestClaimOrCreateDomain_HierarchicalClaimsAreExclusiveAcrossAccountstests still pass unmodified.
Test plan
TestClaimOrCreateDomain_ReclaimSkipsDKIMKeygen(
internal/identity/store_test.go): swapscrypto/rand.Readerfor abyte-counter around a reclaim call only. Fails on unmodified
main(120977 bytes read, a real RSA-2048 keygen) and passes on the branch
(well under the 256-byte threshold, which is comfortably above
generateID's own unrelated 16-byte read on every call).go vet ./internal/identity/...andgofmt -lon both changed files:clean.
go test ./internal/identity/... ./internal/dkim/...: allpassing (505s/1s), including the pre-existing
TestClaimOrCreateDomain_StableOnReclaim(DKIM key/verification tokenstability across reclaim, unaffected) and the two concurrency tests named
above.
go-version: "1.26") against apostgres:16-alpineservice on:5433with the CI's own credentials, thesame shape
go-coverage/go-e2euse.make test-e2e(the separately tagged integration suite) and theweb/TS/Python/CLI/MCP jobs, none of which touch this file.
Fixes #826