Skip to content

fix(magic-link): reject ambiguous revoke prefixes - #2996

Open
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/magic-link-revoke-ambiguous-prefix
Open

fix(magic-link): reject ambiguous revoke prefixes#2996
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/magic-link-revoke-ambiguous-prefix

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Why this matters

The admin API revokes links by the hash prefix displayed in the list UI. If two active hashes share that prefix, the current loop revokes whichever record happens to appear first, potentially disabling the wrong user's invite while leaving the intended one valid.

Root cause and invariant

The endpoint mutated and returned on the first prefix match without proving uniqueness. The invariant is that a shortened identifier may authorize a destructive state change only when it resolves to exactly one active record.

The endpoint now collects active matches under the store lock, returns 409 with guidance when the prefix is ambiguous, and writes only after a unique match is established.

Overlap check

Searched open and closed PRs for magic link revoke prefix collision, ambiguous token hash prefix, and the changed router/test files. No semantic match or open same-file PR was found. This is independent from token generation, redemption, and store durability.

Regression coverage

A FastAPI boundary test persists two active records with the same displayed eight-character prefix, calls the authenticated DELETE endpoint, asserts 409, and reloads the store to prove neither record was revoked.

Validation

  • pytest -q ods/extensions/services/dashboard-api/tests/test_magic_link.py -x ? 67 passed
  • python -m py_compile ods/extensions/services/dashboard-api/routers/magic_link.py ods/extensions/services/dashboard-api/tests/test_magic_link.py
  • git diff --check

Tradeoffs and rollback

Prefix collisions are rare, but the check is O(n) over an already capped local store and happens inside the existing lock. Operators can retry with a longer prefix; exact hashes remain unambiguous. Revert restores first-match mutation with no migration.

@tang-vu

tang-vu commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Batch compatibility receipt (2026-08-23)

Validated merge order: #2989 ? #2990 ? #2993 ? #2991 ? #2992 ? #2994 ? #2995 ? #2996 ? #2997 ? #2998. The changes are independently useful; this order only reconciles shared model-router and magic-link files.

Synthetic integration head: origin/batch/quality-ten-20260822-round2-integration at 91eb730d. The only textual conflict was the two model-router tests inserting at the same class boundary; the integration resolution retains both contracts. Magic-link changes merged cleanly.

Combined validation on that exact head:

  • remote-provider egress contract: 16 passed
  • Token Spy suites: 22 passed, 1 skipped (live PostgreSQL availability)
  • model-router suite: 55 passed
  • APE suite: 29 passed
  • dashboard OAuth + magic-link suites: 118 passed, 2 platform skips
  • Brave Search loopback E2E: all checks passed
  • git diff --check: passed

All required GitHub checks are green across the batch. #2992 initially hit a transient openSUSE repository/mirror failure while installing rsync; a clearly labeled empty retry commit reran the unchanged tree, and openSUSE plus the full matrix passed.

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