Where AI coding assistants get security wrong

What we see

The login function returns the same error for an invalid email and an invalid password — correct, that prevents user enumeration. But the same function does not handle a database timeout, a connection failure, or an unexpected exception the same way. Under those conditions it may return a generic 500 (acceptable), return a stack trace (unacceptable), or — worst case — allow the login to proceed because the exception was caught before the return statement.

Why an assistant produces it

The prompt was "add authentication". The success path and the wrong-password path are both in the training data thousands of times over. The path where Postgres stops answering mid-request is not, and nothing in the prompt asked for it.

The pattern

export async function login(email: string, password: string) {
  let user;
  try {
    user = await db.user.findUnique({ where: { email } });
    if (!user) return { error: "Invalid credentials" };
    const ok = await bcrypt.compare(password, user.passwordHash);
    if (!ok) return { error: "Invalid credentials" };
  } catch (err) {
    logger.error("login failed", err);
    // falls through — no return, no throw
  }

  // reached even when the try block threw
  await createSession(user!.id);
  return { ok: true };
}

The fix

export async function login(email: string, password: string) {
  try {
    const user = await db.user.findUnique({ where: { email } });
    if (!user) return { error: "Invalid credentials" };

    const ok = await bcrypt.compare(password, user.passwordHash);
    if (!ok) return { error: "Invalid credentials" };

    // Session creation is INSIDE the guarded path, and is the last
    // thing that happens — nothing authenticated exists before here.
    await createSession(user.id);
    return { ok: true };
  } catch (err) {
    // Anything unexpected is a rejection, logged as an incident, not a
    // credential failure — an operator needs to be able to tell these apart.
    logger.error({ event: "auth.error" }, err);
    return { error: "Authentication unavailable" };
  }
}

The correct implementation

If anything unexpected happens during authentication, the correct outcome is a clear, logged rejection. No session is created, no authenticated state is set, and the log line distinguishes "wrong password" from "the auth system is broken" so the second one pages somebody. The error handling on the unhappy path must be as deliberate as the logic on the happy path.

What we see

The organisation ID is applied as a filter in the route handler or the service layer. Single-record lookups — GET /controls/:id — include it correctly. The failure appears in aggregates. COUNT(*), GROUP BY and SUM() implementations frequently aggregate across all organisations and then filter the result set that gets returned. The user sees the right data, so the bug is invisible; but the totals, the pagination counts, and the timing of the response are all computed over other tenants' rows.

Why an assistant produces it

Tenant scoping is applied where the developer was looking — the route handler — because that is where the prompt was pointed. Aggregates get added later, often by a different prompt, and the filter is a WHERE clause rather than a property of the data access layer, so there is nothing to inherit it from.

The pattern

// Route-level scoping: correct for the read, wrong for the count.
const rows = await db.control.findMany({
  where: { orgId, status },
  take: 10,
  skip: 0,
});
// Nothing scopes this one — the total leaks every tenant's row count.
const total = await db.control.count({ where: { status } });

The fix

// Scoping is a property of the repository, not of the caller.
export function controlsFor(orgId: string) {
  const scope = { orgId };
  return {
    list: (where: ControlWhere, page: Page) =>
      db.control.findMany({ where: { ...where, ...scope }, ...page }),
    count: (where: ControlWhere) =>
      db.control.count({ where: { ...where, ...scope } }),
    // Every method spreads `scope` last, so a caller cannot override it.
  };
}

The correct implementation

Isolation belongs in a typed repository that every query goes through, not in the route handler. The organisation ID should be a constructor argument, applied last in every WHERE clause so a caller cannot accidentally override it — and a caller who wants unscoped data should have to reach for a differently-named function that is grep-able in review.

What we see

The application disables an AI feature for an organisation by checking a feature flag in the route handler. The check is correct and the model call is blocked. But the AI interaction history, the model outputs, and the evidence fed to the model are stored without the same organisation-level access controls that protect everything else.

Why an assistant produces it

AI features get built as a vertical slice — route, model call, store the transcript — and the transcript table is new, so it inherits nothing. The prompt said "gate this feature behind a flag", and gating it at the route is a complete answer to that prompt.

The correct implementation

The feature flag check, the rate limit check, and the evidence fetch must all be implemented at the data layer, in a repository that enforces the organisation ID on every query — not in the route handler, not in middleware. Anyone who reaches the database through a misconfigured admin panel, an injection elsewhere in the application, or a compromised elevated account should still be unable to read another tenant's model outputs.

What we see

Token verification using === or ==. These operators are not constant-time — they return false as soon as they find a character mismatch. An attacker who can make many requests and measure response times can use the difference to determine how many characters of a valid token they have correct.

Why an assistant produces it

This one is in the Node.js documentation and it is still consistently missed. Assistants reach for === because it is what appears in the overwhelming majority of training code; timingSafeEqual appears rarely enough that the model does not produce it without being explicitly prompted.

The pattern

function verifyToken(supplied: string, expected: string) {
  return supplied === expected; // short-circuits on first mismatch
}

The fix

import { timingSafeEqual } from "crypto";

function verifyToken(supplied: string, expected: string): boolean {
  const suppliedBuf = Buffer.from(supplied);
  const expectedBuf = Buffer.from(expected);

  // timingSafeEqual throws on length mismatch. Comparing lengths first
  // leaks only the length, which is not secret — skipping this check
  // and letting it throw is a different vulnerability.
  if (suppliedBuf.length !== expectedBuf.length) return false;

  return timingSafeEqual(suppliedBuf, expectedBuf);
}

The correct implementation

Use crypto.timingSafeEqual, with an explicit length check before the comparison. Note the ordering: timingSafeEqual requires buffers of equal length, so if the lengths differ and you proceed to the comparison you have swapped a timing leak for a thrown exception on a hot path.

What we see

Structured logging is generated correctly. The problem is the default verbosity: debug statements that include request bodies, query parameters, or whole error objects. At DEBUG level in development these are useful. When the level is misconfigured in production, or an error path triggers a statement nobody expected to reach production logs, API keys, passwords and session tokens land in the log store — which usually has a longer retention period and a wider access list than the database does.

Why an assistant produces it

"Add logging so we can debug this" produces logging optimised for debugging. Logging the whole object is strictly more useful at 2am in development, and nothing in the prompt described the production access model for the log store.

The pattern

logger.debug("request", { headers: req.headers, body: req.body });
logger.error("query failed", { sql, params, error });

The fix

// Log the shape, not the content.
logger.debug({
  event: "request",
  route: req.route.path,
  bodyKeys: Object.keys(req.body),
  hasAuth: Boolean(req.headers.authorization),
});
logger.error({ event: "query.failed", queryId, code: error.code });

The correct implementation

Log the shape of data, not the content: that a request arrived at an endpoint, that a query was executed, that an error occurred — not what the request contained or what the parameters were. Three specific places to check: error handlers that log error.message or error.stack (these can contain query strings with credentials); database query loggers that log the full statement (parameterised queries are safe, but the parameter values appear in some log formats); and HTTP request loggers that log all headers, because Authorization and Cookie are headers.

What we see

CSRF middleware applied globally with an exclusion list. The list is written for the routes that legitimately cannot use tokens — webhook receivers, public endpoints. Over time, as routes are added and the list is copied from a template, state-changing routes that should be protected end up exempt. The specific mechanism: exemptions written as path prefixes. Add an internal state-changing route under /api/ and it silently inherits the exemption.

Why an assistant produces it

A prefix is the shortest correct-looking answer to "exempt the webhooks from CSRF", and it stays correct until someone adds a route. Nothing in the codebase fails when that happens.

The pattern

app.use(csrf({ ignorePaths: ["/api/", "/webhook/"] }));
// Every future /api/ route is exempt. Including the dangerous ones.

The fix

// Exempt exact routes, and make the list a decision, not a prefix.
const CSRF_EXEMPT = new Set([
  "/webhook/stripe",   // signature-verified instead
  "/webhook/github",   // signature-verified instead
]);
app.use(csrf({ ignorePaths: (path) => CSRF_EXEMPT.has(path) }));

The correct implementation

Exemptions should be exact paths, never prefixes, and each one should carry a comment naming the control that replaces CSRF for that route — usually signature verification. A webhook without either is not exempt, it is unprotected.

What we see

Upload middleware that writes the file to a temporary directory before the application processes it. This is the standard behaviour of several popular Node.js upload libraries. The file is unencrypted; it may not be deleted if the application crashes or returns early; the path often appears in log output; and on shared hosting other processes may be able to read the directory.

Why an assistant produces it

Disk-backed temporary storage is the library default, and defaults are what generated code inherits. The cleanup path is written for the success case because that is the case the developer described.

The pattern

const upload = multer({ dest: "/tmp/uploads" });

app.post("/import", upload.single("file"), async (req, res) => {
  const rows = await parse(req.file.path);
  await save(rows);
  await fs.unlink(req.file.path); // never runs if parse() throws
  res.json({ ok: true });
});

The fix

const upload = multer({
  storage: multer.memoryStorage(),
  limits: { fileSize: 5 * 1024 * 1024 },
});

app.post("/import", upload.single("file"), async (req, res, next) => {
  // Validate from the bytes, not the client's Content-Type.
  if (!isCsv(req.file.buffer)) return res.status(415).json({ error: "…" });
  try {
    await save(await parse(req.file.buffer));
    res.json({ ok: true });
  } catch (err) {
    next(err);
  } finally {
    req.file.buffer.fill(0); // don't leave it in the heap for a dump to find
  }
});

The correct implementation

Process the file in memory where the size allows it. If it must be written to disk, encrypt before writing, use a secure random filename, and implement cleanup in a finally block so it runs when processing fails. Separately: validate the file type from the file's magic bytes, not the Content-Type header — that header is client-supplied and trivially spoofable.