CogniPrep scores simulated psychometric tests. A completed test arrives at the server as a game state object, gets validated and sanitised, and is then handed to the scorer for that test.
The sanitiser is ordinary defensive code. Walk the object, strip control characters and null bytes out of strings, keep primitives, recurse into anything nested. It had been in the codebase for a long time and looked like this:
export function sanitizeGameState(gameState: any): any {
if (!gameState || typeof gameState !== 'object') return {};
const sanitized: any = {};
for (const key in gameState) {
const value = gameState[key];
if (typeof value === 'string') {
sanitized[key] = sanitizeString(value);
} else if (typeof value === 'object' && value !== null) {
sanitized[key] = sanitizeGameState(value);
} else {
sanitized[key] = value;
}
}
return sanitized;
}
Read the recursion. typeof [] === 'object', so a nested array goes into sanitizeGameState. Inside, const sanitized = {} and a for...in walks the array's indices as string keys.
An array goes in. An object comes out.
sanitizeGameState({ grid: [[1, 2], [3, 4]] })
// { grid: { 0: { 0: 1, 1: 2 }, 1: { 0: 3, 1: 4 } } }
For most of our tests this never mattered, because most game states are flat: a list of trials, each a small object of primitives. A one dimensional array of objects survives, by luck, because each element is an object and objects were the case the function was written for.
Then we built a test whose state holds a two dimensional array.
What the candidate saw
The scorer for that test starts with for (const row of state.grid). grid is now a plain object, which is not iterable, so that line throws TypeError: state.grid is not iterable.
The scoring engine has a catch around the whole dispatch, which is a reasonable thing to have. It logs the exception, then falls back:
} catch (error) {
logScoringError(`Scoring algorithm exception for ${gameId}`, error, context);
const fallbackScore = calculateFallbackScore(sanitizedGameState, context);
return {
gameId, userId, sessionId,
rawScore: fallbackScore,
metrics: { fallback: 1, scoringError: 1 },
completedAt: new Date(),
};
}
And the fallback looks for a trials array to compute basic accuracy from:
const trials = (gameState as any).trials;
if (!trials || !Array.isArray(trials) || trials.length === 0) {
return 0;
}
This test has no trials. So the candidate finished a full length simulation of a real employer assessment and was handed a raw score of zero, which then went through percentile normalisation and came out as a perfectly plausible looking "you are in the bottom percentile" result.
There is a log line. There is a scoringError: 1 metric. But nothing in the product treats a fallback score as suspect, and the number in front of the user is indistinguishable from a real one. Every test passed, the types were fine, and the build was green, because none of the existing tests used a nested array and TypeScript cannot see through any.
The other half of the same line
While fixing it, the same recursion turned up a second bug:
typeof null === 'object' // true
So every null inside an array became {}. In a game state, null usually means "not answered", and {} means whatever the scorer decides {} means, which is generally "answered, with something I do not recognise". That is not a sanitisation of the data, it is a silent change to it.
The fix
Two branches and a helper:
function sanitizeArrayItem(item: any): any {
if (item === null) return null;
if (typeof item === 'string') return sanitizeString(item);
if (typeof item === 'object') return sanitizeGameState(item);
return item;
}
export function sanitizeGameState(gameState: any): any {
if (!gameState || typeof gameState !== 'object') return {};
// An array has to be rebuilt as an array.
if (Array.isArray(gameState)) {
return gameState.map(sanitizeArrayItem);
}
// ...the object walk, unchanged, with arrays routed to sanitizeArrayItem
}
Array.isArray first, null checked explicitly before the typeof test, and strings inside arrays now sanitised too, which they previously were not. That last change is safe precisely because the string sanitiser only strips control characters and null bytes, and no legitimate answer contains those.
The lesson that outlives the bug
Three things here are worth carrying to other codebases.
typeof x === 'object' is three types, not one. It is objects, arrays and null. Any code path that branches on it and does not handle all three is choosing one of them silently. The two that bite are always the same two.
for...in on an array is legal. It gives you '0', '1', '2' as strings, plus anything on the prototype chain. It is almost never what you meant, and it never fails loudly.
A catch that substitutes a plausible value is worse than no catch. This is the real finding. Our scoring engine converts any exception into a score, so a state shape the pipeline cannot carry produces a result rather than an error. The failure had been changed from "this candidate's test did not score" into "this candidate is bad at this test", which is not a degraded version of the same answer. It is a different answer.
The narrow fix was in the sanitiser, and it is covered by tests now. The wider one is still open on our side, and it is the change I would make first in any codebase shaped like this: a score carrying fallback: 1 should not be rendered as a percentile at all. It should say that scoring failed. A fallback is a signal that your pipeline could not do its job, and displaying it as though it could is the part that turns a bug into a wrong result.
Where it lives
The affected test is McKinsey Solve, whose case exercise holds a grid rather than a flat trial list. Play it on the free tier and the score you get at the end is a real score.
Then go and grep your own codebase for for (const key in, and for any catch that returns a default value with the same shape as a success. You are looking for the places where a failure has been quietly promoted into an answer.
Top comments (0)