DEV Community

Daniel Pertu
Daniel Pertu

Posted on

Somebody else's report is a 404, and the user id is in the where clause rather than in an if

A saved attempt at one of our assessment centre exercises now has its own URL, /dashboard/exercises/report/<attemptId>. It is somebody's own work, their own answers and their own feedback, which makes it the least interesting page in the product and one of the few worth being careful about.

Here is everything that protects it, and the reasoning for each piece.

1. The user id is part of the lookup

There is no getAttempt(id) in the codebase. The only function that reads one attempt takes two arguments:

/** One attempt, only when it belongs to `userId`. */
export async function getExerciseAttemptForUser(
  attemptId: string,
  userId: string
): Promise<SelectExerciseAttempt | null> {
  const [row] = await db
    .select()
    .from(exerciseAttemptsTable)
    .where(
      and(
        eq(exerciseAttemptsTable.id, attemptId),
        eq(exerciseAttemptsTable.user_id, userId)
      )
    )
    .limit(1);
  return row ?? null;
}
Enter fullscreen mode Exit fullscreen mode

The distinction worth caring about is not whether the check happens, it is whether the check can be skipped. A fetch-then-compare version has two statements, and the day somebody writes a new page and copies only the first one, the bug is silent: the data is correct, the owner is wrong. With the id in the where clause there is no intermediate value to forget to compare. The repository function cannot be called in a way that returns a row you are not allowed to see.

It also means the route handler has nothing to decide. It calls one function and renders what comes back.

2. A miss is notFound(), and so is a match on somebody else's row

const attempt = await getExerciseAttemptForUser(attemptId, user.id);
if (!attempt) notFound();
Enter fullscreen mode Exit fullscreen mode

Two different situations collapse into one answer: the id does not exist, and the id exists but belongs to another account. That is on purpose. A 403 on the second case is a confirmation that the row is real, which turns a random id into a probe. 404 for both tells you nothing you did not already know.

This also removes a validation branch. attemptId arrives as a string from the URL and is never parsed or checked against a uuid pattern, because a malformed id simply fails to match anything and falls into the same notFound(). One path for nonsense, missing rows and other people's rows.

The sign-in check is separate and comes first:

const user = await getAuthenticatedUser();
if (!user) redirect('/login');
Enter fullscreen mode Exit fullscreen mode

redirect, not notFound, because for a signed-out visitor the page genuinely might be theirs once they sign in.

3. A noindex tag on a page no crawler can reach

export const metadata: Metadata = {
  title: 'Exercise Report',
  robots: { index: false, follow: false },
};
Enter fullscreen mode Exit fullscreen mode

Every /dashboard/... path is behind a prefix check in our proxy and answers a signed-out request with a redirect to /login, so a crawler cannot see this page in the first place. The meta tag is still there for the case the prefix check does not cover: a URL that somebody pastes somewhere, opens in their own signed-in browser, and which then gets picked up by something with a session attached to it. Crawl rules protect the path. The tag travels with the document.

You can check the first half live with one request against the current site:

$ curl -sI https://cogniprep.app/dashboard/exercises | head -2
HTTP/2 307
location: https://cogniprep.app/login
Enter fullscreen mode Exit fullscreen mode

That redirect is served for every path under /dashboard, whether or not a page exists there, which is also why probing for routes under it tells you nothing.

4. Force-dynamic, because the answer is per-request by definition

export const dynamic = 'force-dynamic';
export const revalidate = 0;
Enter fullscreen mode Exit fullscreen mode

Two reasons, one obvious and one not. The obvious one: the page is per-user, and any caching layer that keys on the URL alone would be a cache that serves one candidate's report to another. The less obvious one: this page polls itself while an AI review is still running, by re-rendering the server component every few seconds. A cached render would make that poll a no-op and the feedback would never appear, which is a correctness bug that looks exactly like a slow model.

See it

The exercises this belongs to are at cogniprep.app/assessment-centre-practice, where one e-tray scenario is free to play in full, and the assessment centre guide is free to read without an account. If you care about what the product stores against your account and how to get it out or delete it, that is written out in plain terms on the privacy page.

The report route itself ships with the next release. The redirect behaviour in point 3 is live right now, and the curl above is the whole test.

The general version

For any row that belongs to one person, the question to ask is not "does this endpoint check ownership" but "is there a code path where it could be read without the owner being part of the query". Put the owner in the where clause and that path stops existing. Then answer the failure with the least informative status you can get away with, and resist the urge to be helpful about why.

Top comments (0)