skip to content

Which security weaknesses deserve a named line on a review checklist for generated code?

level: seniorimportance: should knowfreq 40%

answer

  1. A short list, never a taxonomy
  2. Two tests decide each line
  3. No scanner can decide it
  4. Correctness resting on something outside the file

basics

~10 s

The ones whose correctness depends on something outside the file the generator was working in: the caller's authority, the trust boundary the input crossed, how credentials are obtained, and what the failure path discloses.

solid answer

~50 s

Not a vulnerability catalogue - a checklist with a taxonomy on it does not get read. A line earns its place when two things hold: no automated check can decide it, and the generator's missing context tends to produce it. That yields a short list. Is the caller's authority checked, or is the operation merely performed correctly? Did the input cross a trust boundary that the surrounding file does not reveal? Are credentials obtained through the team's mechanism rather than a plausible-looking shortcut? Does the failure path disclose what the success path is careful about? And one that surprises people: a suggestion is fitted to the code around it, so an unsafe idiom already in the repository propagates with every change that copies its shape. None of this claims generated code is more dangerous. It claims these weaknesses arrive looking finished, which is a different and more useful claim.

code

pseudocode · 15 lines
pseudocode
# the generated change: reads clearly, types check, tests pass
function handleGetInvoice(request):
    id = request.path.invoiceId
    invoice = invoices.findById(id)
    if invoice == null:
        return notFound()
    return ok(render(invoice))

# the rule it could not see - applied in every other handler
# in this codebase, and stated nowhere in the file it edited:
#
#     invoice = invoices.findByIdForAccount(id, request.caller.accountId)
#
# the added test asks for the caller's OWN invoice and passes;
# nothing in the suite ever asks for somebody else's.

go deeper

for a junior

Be able to say that a change can perform an operation perfectly and still never check who is asking, and that a clean scanner run will not tell you so.

for a middle

Explain the selection rule: a line belongs on the list when no automated check can decide it and the tool could not have seen what makes it wrong.

for a senior

Show the worked case - the handler that loads by identifier, passes every check, and never scopes the read to its caller - and how you would surface it in review.

for a principal

Own which of these lines you convert into an enforced check at the framework or pipeline level, so that review stops carrying them one change at a time.

## A selection rule, not a catalogue The instinct after an incident is to paste a list of common weakness classes onto the review checklist. It fails for a mechanical reason: a list that long is not read, so it displaces the lines that were being read before. What is needed is a **selection rule** that keeps the list to a handful of items. A security item earns a named line when **both** of these are true: 1. **No automated check can decide it.** If a scanner or a build rule can, buy that instead - it then applies to every change, including the ones nobody reviews carefully. 2. **The generator's missing context tends to produce it.** The weakness has to be one that follows from working inside a file without the rest of the system in view. Everything else belongs in the team's general security work rather than on this list. Vulnerability taxonomies and threat modelling are their own subject with their own depth; this list is the thin slice where a reviewer of a generated change has leverage. ## Why the blind spot produces these particular classes The mechanism is the same one that explains most of this subject: **what the tool could see determines what it could get right.** A suggestion is shaped by the file being edited and whatever else the tool was given. Security properties that are enforced *locally* - a bounds check, a null guard - are visible in that window. Security properties enforced *elsewhere* - by a guard layer, by a convention every other handler follows, by a policy about where secrets come from - are not. So the generated code can be right about what it could see and is silent about what it could not, and silence reads as completeness. ## The lines - **Authority of the caller.** The code performs the operation correctly and never asks whether this caller may perform it. Performing and permitting are separate concerns, and only the first is local. - **The trust boundary the input crossed.** The value is handled correctly for an internal caller, and this path is reachable from outside. Nothing in the edited file says which. - **Where credentials come from.** A plausible-looking literal, a configuration read that bypasses the team's mechanism, or a log line that prints the value the mechanism exists to protect. - **What the failure path reveals.** The generic handler that returns the underlying error to the caller, after a success path that was careful about exactly that information. - **Imitation of a local unsafe idiom.** The context a suggestion is fitted to is the code around it. If one handler in the repository forgot to scope its read, the next generated handler will look like that one. The fix is not a checklist line at all: correct the exemplar, or make the safe path the only reachable one. ## The change that passes everything the team runs The worked case is worth carrying in your head, because it is what "arrives looking finished" means. A request handler takes an identifier from the path, loads the record, returns not-found when it is missing, and renders it. It compiles. It is typed. It matches the surrounding handlers. The added test asks for the caller's own record and passes. Every check the team runs is green, and the read was never scoped to the caller - because the scoping rule lives in every *other* handler and in none of the text the tool was shown. | Line | What a check can decide | What is left for a person | |---|---|---| | Authority of the caller | That a scoping helper was called at all | Whether the scope applied is the right one | | Trust boundary | That untrusted input reached a sink unvalidated | Whether this path is reachable from outside | | Credentials | That a literal looks like a secret | Whether this is the team's sanctioned source | | Failure path | That an error object is returned to a caller | Whether what it reveals matters here | | Local unsafe idiom | That a known bad pattern recurs | Whether the exemplar should be removed | ## What this is not claiming It is **not** a claim that generated code is more insecure than hand-written code; that would be an empirical assertion nobody in a review can check. The claim is narrower and more useful: these weaknesses **survive** the automated checks and arrive in a shape that reads as finished work. ## Making the lines cheaper over time The best outcome for any line here is that it stops being a line: 1. Require every request-path data read to go through a scoping helper, and fail a change that adds a raw one. Most instances become a build failure and the reviewer is left with only the judgement. 2. Make the sanctioned credential source the only one that works in a running service, so a plausible shortcut fails at startup rather than in review. 3. Remove the unsafe exemplar rather than warning about it, because the exemplar is what the next suggestion will copy. ## What an interviewer is listening for The selection rule, stated before any list. A candidate who opens with a taxonomy is answering a different question; one who says *"a line belongs here when no check can decide it and the tool could not have known it"* has given a rule that generates the list and survives the next tool change.

  • Why does an unsafe pattern already in the repository spread faster with assistance?
    Because the code around it is what a suggestion is fitted to. Asked to add another handler, a tool tends to mirror the handlers it can see - including the one that forgot to scope its read. The remedy is not a checklist line but removing the exemplar: fix the original, or make the safe path the only one that works.
  • How would you stop the authority line becoming an unenforceable ritual?
    Give it a mechanical half. Require every request-path read to go through a scoping helper and fail a change that adds a raw one. That turns most instances into a build failure and leaves the reviewer only the part a check cannot make: whether the scope that was applied is the correct one for this operation.

saying these in an interview costs you the question

  • Says a clean scanner run means the change is secure
  • Wants the full vulnerability catalogue on the review checklist
  • Believes code matching the surrounding style is therefore safe
  • Thinks the tool applied an authorisation rule it was never shown
  • Treats a test that reads the caller's own data as an authorisation test