DEV Community

Bedirhan Elibol
Bedirhan Elibol

Posted on

We ran our security scanner on real SaaS code. Here is what it got wrong.

#security #javascript #webdev #opensource

A scanner that cries wolf gets switched off. So we pointed Zelsis at the code of several mature open-source SaaS products and read every high-severity finding by hand.

Most of them were wrong. The code was protected, just in ways our rules did not understand. Below are the patterns, and the fix we made for each one. If you write a scanner, a linter or a review checklist, you will probably hit the same ones.

1. The guard is in a wrapper 

export const GET = withCron(async () => {
  await renewDomains();
});
Enter fullscreen mode Exit fullscreen mode

Our cron rule looked for CRON_SECRET inside the file. It was not there, because withCron checks the signature before the handler runs. The rule now counts wrappers and auth helpers named for the job, such as withCron or isAuthorizedCronOrInternalRequest. It also flags only files that export a route, so helpers in a cron/ folder are no longer treated as endpoints.

2. The id comes from the session 

export const getAccount = actionClient.action(async ({ ctx: { emailAccountId } }) => {
  return prisma.emailAccount.findUnique({ where: { id: emailAccountId } });
});
Enter fullscreen mode Exit fullscreen mode

This looks like a record loaded by id with no owner check. It is not. The action client builds ctx from the signed-in session, so the id belongs to the caller. The IDOR rule now treats values destructured from a server action's ctx as the caller's own.

The same rule also missed prefixed owner keys. where: { id, emailAccountId } is scoped to the caller, but the rule only knew bare names like userId.

3. The HTML was sanitized first 

<div dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(text) }} />
Enter fullscreen mode Exit fullscreen mode

Flagging every dangerouslySetInnerHTML is easy and mostly noise. The rule now skips values sanitized at the sink, names like safeHtml in files that call a sanitizer, and translations looked up with a literal key such as t("intro.paragraph"). That markup comes from the app's own locale files.

4. The SQL is built from constants 

for (const table of ["messages", "threads"]) {
  await tx.execute(`DELETE FROM ${table} WHERE account_id = ?`, [accountId]);
}
Enter fullscreen mode Exit fullscreen mode

String interpolation in SQL is a real risk, but here every interpolated value is a constant. The rule now follows loops over literal arrays, helpers that are only called with literals, placeholder lists such as ids.map(() => "?").join(","), and cursor clauses that choose between fixed SQL and an empty string.

5. The shell helper only gets literals 

function git(command: string) {
  return execSync(`git ${command}`).toString();
}
git("status --porcelain");
Enter fullscreen mode Exit fullscreen mode

A private helper that puts its parameter into a shell command is a real sink. But if it is not exported and every call in the file passes a literal, nothing outside the file can reach it. The rule checks the call sites before it flags.

6. SHA-1, for a reason

Checking passwords against the HaveIBeenPwned range API requires SHA-1, and only a five-character prefix leaves the process. The weak-hash rule now recognises that case.

How we keep fixes honest

Every change above ships with two fixtures: the vulnerable version must still be caught, and the safe version must stay quiet. The projects are also in the clean set our benchmark runs on each release. A rule that starts firing on clean code gets marked experimental and stops failing builds until someone reviews it.

The planted flaws in our test corpus are all still caught. We checked.

If you want to see what the scanner says about your own code, public repositories are free at zelsis.com. If it flags something that is actually fine, tell us. That is how this list was written.

Top comments (0)