Skip to content
All posts
Auth4 min read

A short password answered 500

A deliberate, well-worded policy rejection reached the client as an internal server error, because the error class was missing one field.

A single brass key on an empty table

Somebody tried to sign up and got a 500. The same form with a different password worked. That combination is the useful clue, and the timing narrowed it further.

real email + strong password    ->  202  in 2.2s
throwaway email + 786ALlah     ->  500  in 0.49s

Two point two seconds is scrypt doing its work. Half a second is too fast for scrypt to have run at all, which places the failure before hashing, in the policy check. That is a narrow enough window to read the whole thing.

export class PasswordRejected extends Error {
  readonly code: string;
  // and no status.

  constructor(code: string, message: string) {
    super(message);
    this.name = "PasswordRejected";
    this.code = code;
  }
}

if (password.length < 10) {
  throw new PasswordRejected("too_short", "Use at least 10 characters. ...");
}

The message is good. It names the rule and says what to do. It never reached anyone.

The mapper duck-types on purpose

Errors from every package pass through one function that turns them into an API response. It duck-types rather than using `instanceof`, and that decision is right: these classes cross package boundaries, and two copies of a package in the tree make `instanceof` fail silently, turning a clean 400 into a 500 in production and nowhere else.

const status = typeof held.status === "number" ? held.status : undefined;

if (status !== undefined) {
  return new ApiError(mapStatus(status), message, { ... });
}

// No status. Could carry a connection string or a fragment of somebody's
// document, so the message goes to the log and not to the caller.
return new ApiError("INTERNAL_ERROR", "Something went wrong.", { detail: message });

The fallback is also right. An error with no status is an error nobody shaped for a user, and its message may well contain a connection string. Refusing to forward it is the correct default.

So every part behaved as designed. The contract is a `status` field, `PasswordRejected` did not have one, and a deliberate policy decision was classified as an unhandled crash. The whole fix is one line.

export class PasswordRejected extends Error {
  readonly code: string;
  readonly status = 400;   // the mapper duck-types on this
  // ...
}

Confirming the tests bite

Two tests went in with it, and then the field came back out to check they failed. They did. A test that passes against both the fixed and the broken version has told you nothing, and it costs about thirty seconds to find out which kind you wrote.

Worth noting what the user experienced. Not a form that said the password was too short, which is a small annoyance. A page that said something went wrong, which reads as the product being broken and is where people stop signing up.