Sequential IDs Are a Security Decision

A convenient helper, of a kind that exists in most codebases:

function findByRef(ref) {
  if (/^\d+$/.test(ref)) {
    return db.get("SELECT * FROM runs WHERE id = ?", Number(ref));
  }
  return db.get("SELECT * FROM runs WHERE public_id = ?", ref);
}

Take an id or a public handle, return the row. Scripts pass the numeric id, the web passes the handle. It reads as a kindness.

Now route an HTTP path segment through it:

app.get("/api/runs/:id", (req, res) => {
  const run = findByRef(req.params.id);
  res.json({ run, leads: leadsOf(run.id), shareUrl: shareFor(run.id) });
});

Every URL the application generates contains a random handle with 72 bits of entropy. Nobody guesses one. But the route also accepts 1, and 2, and 3, and now the entire table is walkable with a for loop.

Why the gate did not catch it

The route was deliberately unauthenticated, and for a defensible reason: the page for a run polls its own status, and running that poll through the authenticated, rate-limited path would have made a page consume its own quota. Public reads were an intentional design decision.

That decision is completely fine, and it is only fine while the identifier is unguessable. The auth rule and the identifier scheme were two separate choices made by two different people at two different times, and the security property lived in the space between them where nothing tested it.

The matcher did not help either. It was anchored:

const COSTLY = /^\/api\/(runs|run|brief)\/?$/;

That matches /api/runs and nothing below it. The collection was rate limited. Every individual resource under it was not.

The fix is a second function, not a check

The instinct is to add a validation branch to the handler. Better to make the unsafe capability unreachable from the network:

/** The only lookup a request off the network may use. */
function findByPublicId(ref) {
  return db.get("SELECT * FROM runs WHERE public_id = ?", ref);
}

Internal callers keep the flexible helper. HTTP handlers get the narrow one. The rule is now expressed in the type of thing a handler can call, rather than in a check somebody has to remember to write in each new route.

Write the test against the vulnerability

A test asserting the new function works is nearly worthless, because it would pass before the bug existed and after it was reintroduced somewhere else. Assert the distinction instead:

assert.ok(findByRef(String(row.id)), "the numeric path still exists internally");
assert.equal(findByPublicId(String(row.id)), undefined, "and is unreachable publicly");

That test documents why the two functions exist. Delete the second one and it fails.

Two things worth taking away

First, a helper that accepts several kinds of identifier is a fine internal convenience and a poor public boundary. The flexibility is the vulnerability.

Second, when you deliberately leave something unauthenticated, write down what is carrying the security instead. If the answer is "the identifier is unguessable", then any code path that accepts a guessable identifier for the same resource is a bug, and it is worth a test that says so.