Skip to content

fix(#856): the identity, keys and certificates family of identifier draw sites - #1287

Merged
scttfrdmn merged 1 commit into
mainfrom
fix/856-identity-keys-ids
Sep 26, 2026
Merged

scttfrdmn merged 1 commit into
mainfrom
fix/856-identity-keys-ids

Conversation

@scttfrdmn

Copy link
Copy Markdown
Owner

Tier 4 of #856. Eight generators across seven services move onto IDMint: Cognito (both the
user-pool and the identity-pool API), IAM Identity Center, KMS, ACM, Secrets Manager, WAFv2, and
API Gateway's API keys. rand.Read draw sites go from 23 to 15.

Every rendering is byte-for-byte what the crypto/rand version produced — the width, the case and
the alphabet — so an identifier a previous substrate recorded is still the shape this one mints.
Where the choice was between IDMint.UUID (RFC 4122 version and variant nibbles set) and
IDMint.HexUUID (the same grouping without them), the API model was consulted each time and settled
nothing: API_ApiKey publishes no length and no pattern on either id or value,
API_KeyMetadata.KeyId publishes a length of 1–2048 and no pattern, and CertificateArn's pattern
admits either form. So the existing rendering stays, per #671's "only what the API model states".
WAFv2 is the exception where the model does speak — API_WebACLSummary publishes
^[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}$, which is indifferent to those nibbles — and UUID()
preserves what was recorded.

Three of these are not identifiers

They are in scope because a caller hands them back, so a re-minted one turns a recorded success
into a refusal or an incomplete response:

  • A WAFv2 LockToken must be returned to UpdateWebACL. A replay that re-minted it answered the
    recorded update with WAFOptimisticLockException instead of the recorded success. One minter
    serves the web ACL ID, the IP set ID and both lock tokens, because the published constraint is
    identical on each.
  • A Secrets Manager version ID is the quietest break in the family. GetSecretValue reports an
    unknown VersionId by omitting SecretString rather than by refusing, so a replay answered a
    recorded read with a 200 that had silently lost a member.
  • A KMS GenerateDataKey stub data key is observed twice in one response: the bytes are the
    Plaintext, and the same bytes are wrapped into the CiphertextBlob. A recorded Decrypt was
    never affected — substrate's ciphertext carries its own plaintext, so a replayed decrypt answers
    from the recorded blob regardless — so the break was in the create, not the read.

One service was minting another's identifiers

CreateApiKey reached into generateACMCertID for both UUID-shaped strings it returns, so a change
to ACM's rendering would have silently moved API Gateway's. They now have their own minters, and the
id and value draw in turn so they differ. generateACMCertID no longer returns an error: the
mint cannot fail where rand.Read could, so the caller's dead error branch went with it.

Tests

emulator/ids_test.go gains the ordinal test — one CreateUserPoolClient with GenerateSecret
draws three times, and the client ID, the first half of the secret and its second half are pairwise
distinct — and the wire assertion: a recorded stream across all seven services replays with zero
Differences and StateValid true under ValidateState. Each recorded read is one a re-minted
value would break, and the assertion was shown non-vacuous by reverting generateVersionID to
randomHex, which produces 21 differences: VersionId majors on three operations, the lost
SecretString, and a state-hash cascade through the rest of the stream.

Gaps filed rather than folded in

Keeping the tier shape-preserving is what lets its tests rest on "the rendering is unchanged", so two
fidelity gaps it turned up became issues:

Both are recorded in docs/services.md beside the existing member-name gap (#1136) rather than left
only on the tracker.

Verification

make lint (0 issues), make test (race, green), and make docs-reference-check docs-versions version-check discarded-unmarshal-check wire-bookkeeping-check with the ratchet still at 330.
Patch coverage intersected against git diff -U0 main: 43/43 changed statement lines covered.

Refs #856.

Eight generators across seven services move onto IDMint: Cognito (both the
user-pool and the identity-pool API), IAM Identity Center, KMS, ACM, Secrets
Manager, WAFv2, and API Gateway's API keys.

Every rendering is byte-for-byte what crypto/rand produced, so an identifier a
previous substrate recorded is still the shape this one mints. Nothing in any of
the seven API models distinguishes the two renderings of a UUID-shaped value,
which is why #671 leaves them alone.

Three of the minted values are not identifiers, and are in scope because a
caller hands them back. A WAFv2 LockToken must be returned to UpdateWebACL, so a
re-minted one answered a recorded update with WAFOptimisticLockException. A
Secrets Manager version ID is the quietest of the family: GetSecretValue reports
an unknown VersionId by omitting SecretString rather than refusing, so a replay
answered a recorded read with a 200 that had lost a member. A KMS stub data key
is observed twice in one response, as Plaintext and inside CiphertextBlob.

An API Gateway API key's id and value were drawn from ACM's certificate-ID
generator, so a change to ACM's rendering would have silently moved API
Gateway's. They now have their own minter.

Two fidelity gaps the family turned up are filed rather than folded in: a secret
version ID is half the published minimum width and ignores the caller's
ClientRequestToken (#1285), and CreateUserPool reports the pool id as
UserPool.UserPoolId where UserPoolType publishes Id (#1286).

15 draw sites remain on crypto/rand.

Refs #856.
@scttfrdmn
scttfrdmn merged commit f56717d into main Sep 26, 2026
17 checks passed
@scttfrdmn
scttfrdmn deleted the fix/856-identity-keys-ids branch September 26, 2026 14:26
@codecov

codecov Bot commented Sep 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant