DEV Community

Cover image for The SQL injection bug your code review keeps missing
Alok Rajasukumaran
Alok Rajasukumaran

Posted on • Originally published at github.com

The SQL injection bug your code review keeps missing

#node #typescript #opensource #webdev

Every TypeORM project I've worked on grows the same few dangerous lines. I got tired of catching them by hand, so I wrote a linter that does it for me.

I was reviewing a pull request a while back and nearly scrolled past this line:

await manager.query(`SELECT * FROM users WHERE id = ${req.params.id}`);
Enter fullscreen mode Exit fullscreen mode

Looks fine at a glance. It's a SQL injection hole. The id comes off the request and lands directly in the query string.

The thing that bugged me is that I only caught it because I happened to be reading carefully on that line, that day. Review catches this sort of thing a lot of the time. "A lot of the time" is how these end up in production.

It's usually not only injection either. The same projects tend to collect a few other habits:

  • synchronize: true in a data source config. It rewrites your schema on startup and can drop a column on the next deploy.
  • A QueryBuilder delete() or update() that hits .execute() with no .where(). Forget that one line and you've changed every row in the table.
  • Three or four writes in one function, none in a transaction, so if the second throws you're left with half-written data.
  • On multi-tenant apps, a query that forgets the tenant filter. That's how one customer sees another customer's data.

None of these really need a person to catch them. They're mechanical. A linter can do it on every commit.

So I built one.

eslint-plugin-typeorm-enterprise 

npm install --save-dev eslint eslint-plugin-typeorm-enterprise
Enter fullscreen mode Exit fullscreen mode 
// eslint.config.js
const typeormEnterprise = require('eslint-plugin-typeorm-enterprise');
module.exports = [typeormEnterprise.configs.recommended];
Enter fullscreen mode Exit fullscreen mode

Now those patterns are lint errors. Ten rules today, split into configs (recommended, strict, performance, multiTenant): raw SQL, interpolated and concatenated SQL, unsafe QueryBuilder deletes, EntityManager raw queries, writes outside a transaction, tenant scoping, and a nudge to stop counting rows when you only need to know one exists.

Two things I was picky about

It doesn't need any type setup. The rules read the syntax tree, so they work in a plain ESLint 9 config with nothing else to wire up, and they run under oxlint too. If you already lint with type information, there's an optional mode that checks the receiver is actually a TypeORM Repository or EntityManager instead of matching on a name.

And it tries hard not to be noisy. The fastest way to get a linter switched off is a wall of false positives on the first run. This one leaves req.query.id and other non-SQL calls alone.

Try it

Rule ideas and false-positive reports welcome, especially real TypeORM footguns you've hit.

Top comments (1)

Collapse
 
yune120 profile image
Yunetzi •

Spot on. Those recurring patterns bite every project. Add a quick checklist: parameterized queries, never build SQL strings, avoid JSON.parse on localStorage, and use tokens for auth instead of local state. Small reviews, big safety. 👍