Skip to content

test(integration): TestIdentityLink flakes on identity_already_bound - 4-digit suffix from UnixNano%10000 collides across runs #653

Description

@ericfitz

Summary

TestIdentityLink fails intermittently with:

identity_link_test.go:129: Step 3: driveIdentityLinkCallback failed:
  redirect location "http://localhost:8079/?error=identity_already_bound" has no link_pending param

Cause

test/integration/workflows/identity_link_test.go:57 derives its per-run user ids from the low four
digits of the nanosecond clock:

suffix := fmt.Sprintf("%04d", time.Now().UnixNano()%10000)
aliceID := "il-alice-" + suffix
aliceAltID := "il-alt-" + suffix

That is only 10,000 distinct suffixes, and the comment above it ("Use unique suffixes so repeated
runs don't clash") assumes uniqueness the expression does not provide — UnixNano() % 10000 is
effectively a random draw, not a counter.

Each successful run leaves il-alt-<suffix> permanently bound to its alice account, and nothing
deletes it. So the pool of poisoned suffixes grows monotonically with every run against a given
database, and the collision probability rises with it. When a run draws an already-used suffix,
POST /oauth2/callback correctly refuses with identity_already_bound and the test fails.

TestIdentityLink_SecondConfirmRejected (line 463) has the same construction with the il2- prefix.

Evidence

Same suite, same commit range, two consecutive runs on 2026-07-31:

  • run 1 drew suffix 5000FAIL: TestIdentityLink (81 passed, 1 failed, 9 skipped)
  • run 2 drew a fresh suffix → 82 passed, 0 failed, 9 skipped

Confirmed unrelated to the branch under test: the failing suite passed 82/82 on the clean base
commit and 82/82 again with the branch's changes restored.

Fix options

  1. Widen the suffix. login_hint must match ^[a-zA-Z0-9-]{3,20}$, and il-alice- is 9 chars,
    leaving 11 for the suffix — plenty for a full nanosecond timestamp or a random base36 string.
    The 4-digit limit was never necessary.
  2. Clean up in teardown. DELETE /me/identities/{id} already exists and
    TestIdentityLink_SecondConfirmRejected calls it; TestIdentityLink does not. Adding a
    t.Cleanup would bound the growth but still leaves a window on failure.

Option 1 alone fixes it; doing both is better hygiene.

Note

This is latent flakiness in a test that guards identity-linking security behavior, so a spurious
red here trains people to re-run rather than investigate. Worth fixing before it masks a real
regression.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions