The caveat, first
This one could not happen today, and it is worth saying that before anything else rather than at the end where it reads as a retraction.
Firebase user ids are alphanumeric. Our document slugs are kebab-case. Neither contains an underscore, so the ambiguity described below has no way to occur with the inputs the system actually receives. Nobody was ever affected, and there is nothing here to exploit.
It is published because the reasoning generalises, because a test caught it before it shipped, and because the interesting part is not the bug but why we refused to reason our way past it.
What the record is for
Staff acknowledge policies. Someone reads the security policy or the contractor terms, presses a button, and a document records that this person accepted that policy at that version. It is evidence about a named individual, which is the entire reason it exists.
The id is built from three fields joined with an underscore: the user id, the policy slug, and the version. Encoding the version is deliberate. A policy that changes needs a fresh acknowledgement, and putting the version in the id makes a stale record impossible to mistake for a current one. It also makes the write idempotent, so a double click cannot create two rows, and it turns "has this person read the current version" into a single document read instead of a query.
That design is good and we kept it. The problem is in the join.
The ambiguity
Joining fields with a separator is only reversible if no field can contain the separator. Ours could, in principle:
("u1_x", "p", "1") -> "u1_x_p_1"
("u1", "x_p", "1") -> "u1_x_p_1"Two different triples, one string. Different user, different policy, same document id.
Now consider what that document is. There is no other uniqueness guard anywhere: no compound index, no constraint, no check. The id is the constraint. So if two triples collide, one person's acknowledgement satisfies another person's requirement, and the record says a person accepted a policy they never saw.
Why nobody would ever find out
This is the part that made it worth rejecting rather than reasoning around, and it has nothing to do with how likely the collision is.
The write is idempotent on purpose, so that a double click does not create two rows. That means writing to an id that already exists is the normal, expected, correct case. It is what happens every time somebody presses the button twice.
So a collision would look exactly like a repeat press. Same operation, same result, no error, no conflict, no log line, nothing to alert on. The system cannot tell the two apart, because from its position they are the same event.
A collision here is indistinguishable from a legitimate repeat press. There is no detection to build, because the two are the same operation. That is what moves it from unlikely to unacceptable.
Compare that to a bug that throws. A loud failure is a bug with a deadline attached. A silent one produces a record that looks correct, gets relied on, and is wrong in a way that only surfaces when someone disputes it, which is the worst possible moment for evidence to be wrong.
The fix, and the option we turned down
The builder now refuses any segment that is empty or contains the separator, and throws:
if (value.includes(SEPARATOR)) {
throw new Error(
`acknowledgementId: ${name} contains "${SEPARATOR}", ` +
`which would make the id ambiguous`,
);
}The obvious alternative was to escape the separator instead of rejecting it. That works, and we turned it down for a reason worth stating: escaping changes the id shape for exactly the records that trigger it. The register would then hold two id formats, one common and one rare, and nobody would know which was which or why. A rare format that appears only under conditions nobody remembers is a future confusion with no owner.
Failing loudly costs one exception on a code path that has never been taken. Escaping quietly costs a permanent inconsistency in a record that exists to be trusted.
The test that pins this failed when it was first written, which is the only reason we know the guard was needed rather than assumed.
What to take from it
- Find out what your uniqueness constraint actually is. If it is a constructed id, then the construction is the constraint, and it deserves the scrutiny you would give a database index.
- Joining fields with a separator is only safe when no field can contain the separator. That is a property of your inputs, and inputs are not yours to control forever.
- "Cannot happen with our current inputs" is a statement about today. Identity providers change formats, imports arrive from elsewhere, and slugs get generated from user text.
- Weigh silence, not just likelihood. A rare loud failure is cheap. A rare silent one that produces a plausible wrong record is expensive, and the two deserve different thresholds.
- Prefer rejecting an input over transforming it when the transformation would change the shape of stored data. A rare alternate format is a question nobody will be able to answer later.
The general version: idempotency and collision detection pull against each other. Making a repeat write harmless is usually right, and it removes the signal that would have told you two different things had landed in one place. Where that record is evidence about a person, the id has to carry the uniqueness by construction, because nothing downstream is going to notice.
What this does not cover
- This was never exploitable and never affected anyone. Firebase uids are alphanumeric and our slugs are kebab-case, so no segment can contain the separator. It is published as a design lesson, not as a vulnerability.
- The guard rejects inputs it considers ambiguous. It does not prove the id scheme is collision free for every conceivable future input, only that the one ambiguity we identified is now refused rather than encoded.
- The reasoning about silent failure is about our own system, where the write is idempotent by design. A system that errors on a duplicate id has a different risk profile and would weigh this differently.
- We have not audited every other constructed id in this codebase against the same standard. This one was examined because a test was written for it.
Sources
- lib/dashboard/acknowledgement-id.ts and tests/acknowledgement-id.test.ts in the whatscene.in repository. The id builder, its documented reasoning, and the assertion that refuses an ambiguous segment. The test's comment records that it failed when first written. Run: commit 2b2ce13, staff policy acknowledgement feature.
Revisions
- 24 August 2026 First published. The guard shipped with the feature rather than as a later fix.
This page is revised in place rather than replaced, so its address does not change.