How SQL Injection happens in Node.js migration scripts and how to fix it

Introduction

The scripts/setup-d1.mjs file manages Cloudflare D1 database migrations for this application — reading .sql files from a migrations directory, executing them, and recording each applied migration by name. It's the kind of infrastructure code that rarely gets a second look after it's written. But buried inside the applyMigrations function was a SQL injection vulnerability that could let an attacker execute arbitrary SQL against the database simply by controlling a filename.

The culprit was a helper called escapeSqlString and a single line of template-literal SQL construction:

const mergedSql = `${sql}\n\nINSERT INTO d1_migrations (name) VALUES ('${escapeSqlString(file)}');`;

This pattern — trusting a sanitization function to make string interpolation safe — is one of the most common sources of SQL injection in real-world codebases.

The Vulnerability Explained

What went wrong in escapeSqlString

The function that was supposed to protect the INSERT statement looked like this:

function escapeSqlString(value) {
  return String(value || '').replaceAll("'", "''");
}

It only escapes single quotes by doubling them. That's a partial defense at best. A filename containing a semicolon, a SQL comment sequence (--), or other metacharacters passes through completely unmodified.

The dangerous construction in applyMigrations

In applyMigrations (around line 234), the migration SQL and the tracking INSERT were merged into a single string:

const mergedSql = `${sql}\n\nINSERT INTO d1_migrations (name) VALUES ('${escapeSqlString(file)}');`;
await executeD1Sql(databaseId, mergedSql);

The variable file comes from a directory listing of the migrations folder. If an attacker can write a file to that directory, they control the value of file directly.

Concrete attack scenario

Consider a file named:

'); DROP TABLE users; --

After passing through escapeSqlString, the single quote is doubled, but the rest survives intact. The assembled SQL becomes:

-- (migration SQL here)

INSERT INTO d1_migrations (name) VALUES (''); DROP TABLE users; --');

The D1 query endpoint receives a multi-statement payload. The DROP TABLE users executes as a legitimate SQL command, and the trailing -- comments out the rest. The d1_migrations table itself, application data tables, or any other accessible object could be targeted.

Even without dropping tables, an attacker could:
- Exfiltrate data by crafting a UNION-based payload embedded in the filename
- Corrupt migration state by inserting fake records into d1_migrations
- Escalate privileges if the D1 API token has broader permissions than migration tracking requires

The Fix

The fix makes two targeted, surgical changes that together eliminate the injection surface.

1. Remove escapeSqlString entirely

The broken helper is deleted. There is no safe way to use it for the purpose it was serving — the entire approach of sanitizing values before interpolation is replaced with something that doesn't require sanitization at all.

2. Separate the INSERT and use a parameterized query

Before:

const mergedSql = `${sql}\n\nINSERT INTO d1_migrations (name) VALUES ('${escapeSqlString(file)}');`;
await executeD1Sql(databaseId, mergedSql);

After:

await executeD1Sql(databaseId, sql);
await executeD1Sql(databaseId, 'INSERT INTO d1_migrations (name) VALUES (?);', [file]);

The migration SQL runs on its own. The tracking INSERT is a separate call with a ? placeholder and file passed as a bound parameter in the params array.

3. executeD1Sql now accepts parameters

The function signature was updated to support the params argument:

// Before
async function executeD1Sql(databaseId, sql) {
  const data = await cfApiRequest(`/d1/database/${encodeURIComponent(databaseId)}/query`, 'POST', { sql });
  return data.result;
}

// After
async function executeD1Sql(databaseId, sql, params) {
  const body = params ? { sql, params } : { sql };
  const data = await cfApiRequest(`/d1/database/${encodeURIComponent(databaseId)}/query`, 'POST', body);
  return data.result;
}

When params is provided, it's included in the POST body alongside sql. Cloudflare's D1 API handles the binding server-side, ensuring the filename value is never parsed as SQL syntax — regardless of what characters it contains.

Why this works where escaping didn't

Parameterized queries enforce a hard separation between the SQL structure (the query template) and the data (the bound parameters). The database driver transmits them separately; the query planner sees the ? as a typed data slot, not as SQL text to be parsed. No amount of SQL metacharacters in file can change the shape of the query.

Prevention & Best Practices

Always use parameterized queries for user- or filesystem-controlled data

If your SQL API supports bound parameters — and virtually all modern ones do — use them. This includes:
- Cloudflare D1: { sql: 'SELECT * FROM t WHERE id = ?', params: [id] }
- better-sqlite3: stmt.run(value)
- pg (node-postgres): client.query('SELECT $1', [value])
- Prisma / Drizzle / Kysely: parameterization is the default

Treat filenames as untrusted input

Filenames sourced from readdir, glob, or any filesystem operation can be influenced by anyone with write access to that directory. They are not safe to interpolate into SQL, shell commands, or HTML without proper handling.

Validate migration filenames before use

Even with parameterized queries in place, consider adding an allowlist check on migration filenames:

if (!/^\d{4}[-_][\w-]+\.sql$/.test(file)) {
  throw new Error(`Unexpected migration filename: ${file}`);
}

This adds defense-in-depth and makes migration state easier to reason about.

Never combine migration SQL with tracking SQL in one string

Merging two SQL statements with template literals (as the original code did) creates a multi-statement payload that bypasses the intent of parameterized queries. Keep statements separate and parameterize each independently.

Use static analysis to catch injection patterns early

Tools that perform taint analysis — tracing data from a source (filesystem read) through transformations to a sink (SQL execution) — can catch this class of bug before it reaches production. Semgrep rules for SQL injection in JavaScript are available at https://semgrep.dev/r?q=sql-injection+javascript.

Relevant standards:
- OWASP SQL Injection Prevention Cheat Sheet
- CWE-89: Improper Neutralization of Special Elements used in an SQL Command

Key Takeaways

• escapeSqlString was a false sense of security: Escaping only single quotes while leaving semicolons, comment sequences, and other metacharacters unhandled made the function actively misleading. If you see a custom SQL-escaping helper in a codebase, treat it as a red flag.
• Filenames are attacker-controlled input: In applyMigrations, the file variable came from readdir — not from a trusted constant. Any value read from the filesystem should be treated with the same suspicion as HTTP input.
• Merging SQL statements via template literals breaks parameterization: The original design concatenated the migration SQL and the tracking INSERT into one string, making it impossible to safely parameterize the filename. Separating the two calls was a prerequisite for the fix.
• The D1 params array is the right tool: Cloudflare's D1 query API natively supports bound parameters. There was no need for manual escaping; the correct API was available all along.
• Deleting escapeSqlString was the right call: Leaving a broken helper in place invites future misuse. Removing it entirely ensures no new code can accidentally rely on it.

How Orbis AppSec Detected This

• Source: The file variable populated by readdirSync in applyMigrations — a filesystem-controlled string that can be set by anyone with write access to the migrations directory.
• Sink: The executeD1Sql(databaseId, mergedSql) call at line ~234, where the assembled string (including the unsanitized filename) was POST-ed to the Cloudflare D1 query endpoint.
• Missing control: The escapeSqlString function only neutralized single-quote characters. No validation of the filename format, no use of bound parameters, and no separation between SQL structure and data values.
• CWE: CWE-89 — Improper Neutralization of Special Elements used in an SQL Command (SQL Injection)
• Fix: Replaced the single concatenated executeD1Sql call with two separate calls — one for the migration SQL, one for the tracking INSERT using a ? placeholder and [file] as a bound parameter array.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This vulnerability is a textbook example of why custom sanitization functions are dangerous: escapeSqlString looked like a solution, but it only addressed one of many possible injection vectors. The real fix wasn't a better escaping function — it was eliminating the need for escaping entirely by using the parameterized query API that was already available.

For developers writing migration scripts, database seeders, or any infrastructure code that touches SQL: treat every value that isn't a hard-coded string literal as untrusted, and let your database driver handle the separation between code and data. That's the only reliable defense against SQL injection.

References

• CWE-89: SQL Injection
• OWASP SQL Injection Prevention Cheat Sheet
• Cloudflare D1 Parameterized Queries Documentation
• Semgrep SQL Injection Rules for JavaScript
• fix: add parameterized queries in setup-d1.mjs