Never spread a database row into a sign-in response
One spread operator was enough to hand every client the password reset token hash and the last login IP. The fix is small, and so is the lesson.
A sign-in endpoint has one job that matters more than the rest: hand the client a token and enough profile to draw the first screen. In one of our NestJS backends, the function that built that response looked like this.
return {
user: {
...user,
token: this.generateJWT(user),
},
};
It is a natural thing to write. The user object is already in scope, the client needs most of its fields, and the spread keeps the diff to one line. It also meant the response contained whatever the user table contained.
What was in the response
The entity carries more than a profile. Alongside the name and email it holds the fields the password reset flow needs: the hash of the current reset token, when that token expires, and when it was used. It also holds the IP address of the last login.
None of those are secrets in the sense that a plaintext password is. The reset token is stored hashed, which is exactly what you want. But a hash that never leaves the server is a stronger position than a hash that rides along in every sign-in response, and an IP address is personal data the client has no reason to hold. As far as we could tell, no client screen used them. They were simply there because the row was there.
I want to be precise about the severity. I am not claiming anyone exploited this, and I have no evidence either way. The point is the shape of the mistake: the response shape was decided by the table, not by anyone.
Why the spread survives review
A reviewer reading ...user sees a tidy line. The diff gives no hint of what the object holds, because that knowledge lives in a different file. The mistake only shows when you open the entity and read down the column list, and nobody does that in a review of an auth change.
It also stays invisible in testing. Every test that signs in and checks for a token passes. Every screen renders. The extra fields are not wrong for the client, they are just unread.
And it ages badly. Each column added to the user table later joins the response without a decision. The response is wider today than the day it was written, and no commit says so.
The change we made
We added one small function and used it at the point where the response is built.
const PRIVATE_USER_FIELDS = [
"password",
"passwordResetTokenHash",
"passwordResetTokenExpiresAt",
"passwordResetTokenUsedAt",
"lastLoginIp",
] as const;
export function toPublicUser<T extends object>(user: T) {
const copy = { ...user } as Record<string, unknown>;
for (const field of PRIVATE_USER_FIELDS) delete copy[field];
return copy as Omit<T, (typeof PRIVATE_USER_FIELDS)[number]>;
}
The sign-in response now spreads toPublicUser(user) instead of user. The unit test builds a stored user with every private field set, asserts each one is gone, and then asserts that the profile fields the apps read are still there with their values. The second half matters as much as the first. A test that only checks removal will happily pass when the helper deletes everything.
The limit of this fix
This is a denylist, and I would not call it the best version. A denylist fails open: the next sensitive column someone adds to the table is public until someone remembers to extend the list. It is the smaller change: the apps read many profile fields, and a denylist cannot break any of them.
If I were writing the response from scratch, I would name the fields instead.
const { id, uuid, email, firstName, lastName, lastLoginAt } = user;
return { user: { id, uuid, email, firstName, lastName, lastLoginAt, token } };
That version fails closed. A new column stays private until a person decides to publish it, and the decision shows up in a diff that names the field. It costs a few more lines and one more edit when the profile grows, which is the right moment to be asked.
Checks worth adding to your own backend
Search for the spread first: ...user, ...entity, Object.assign(response, row), and any controller that returns an ORM object directly. Each hit is a response whose shape is owned by a table.
Then look at every response that carries credentials or identity: sign-in, token refresh, profile, invite acceptance. Those are the places where a stray column does the most harm.
Finally, write the test that pins the response, not just the happy path. Give it a row with every column populated and assert on the exact keys that come back. When a column is added, that test is what turns "it joined the response silently" into a failing build and a conversation.
0 comments