chore(sonarqube): keep interpolation out of query call sites, and clear the last hotspot #294

New Issue

2026-09-04T09:13:30-05:00

bermudalamb commented

2026-09-04 09:13:30 -05:00

The quality gate is red on one condition and one condition only: new_security_hotspots_reviewed: 75.0 against a threshold of 100. Everything else reads 0 bugs, 0 vulnerabilities, 0 duplicated lines, 0 minutes of debt.

The outstanding hotspot is backend/src/routes/admin.ts:230, typescript:S2077 — "Make sure that executing SQL queries is safe here."

What is actually true of that line

const { rows: full } = await pool.query<AdminItemRow>(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [itemId]);

The value is already parameterized. itemId is bound as $1, goes to Postgres through the driver's separate parameter channel, and never enters the query text. What is interpolated is ADMIN_ITEM_SELECT, a module constant in itemSelect.ts that contains no caller data.

So there is no injection here, and "parameterize it" is not a change that can be made — it is already done. S2077 fires on the template literal, because the rule cannot tell a module constant from a request value, and asks a human to confirm.

That leaves two ways to a green gate. Marking the hotspot reviewed in the dashboard is one. Removing the interpolation is the other, and it is better: it makes the property structural rather than a human assertion that has to be repeated every time the line moves.

The rule this adopts

No interpolation at a query call site. Where a query's shape is fixed, the whole SQL string becomes a named constant and the call passes an identifier. That way the reviewer's question never arises, and a future edit cannot quietly turn a constant into a value.

Fixed shape — becomes a constant

Where	Today
`admin.ts:176`, `admin.ts:230`	`${ADMIN_ITEM_SELECT} WHERE i.id = $1` — identical, one constant serves both
`items.ts:97`	`${PUBLIC_ITEM_SELECT} WHERE i.id = $1 AND ${EXCLUDE_PENDING}`
`adminItemDrafts.ts:62,63`	two fixed `DRAFT_SELECT` variants
`adminUploadLinks.ts:72`	`LINK_SELECT` plus a fixed `ORDER BY`

Genuinely dynamic — stays, and stays explained

admin.ts:146 and items.ts:86 build their WHERE at run time from buildItemFilterSql. These cannot become constants and should not pretend to be. What makes them safe is already written down at admin.ts:132-143: the clause fragments are string literals in itemFilters.ts, the only interpolations inside them are placeholder indices ($${next}), and every value goes in params. Worth confirming that reasoning still holds rather than assuming it, and leaving the comment pointing at it.

Cannot be parameterized at all

draftingWorker.ts:77 interpolates a table name:

const { rows } = await pool.query<{ name: string }>(`SELECT name FROM ${table} ORDER BY name`);

Postgres does not allow a parameterized identifier — $1 can be a value, never a table. What makes this safe is that table is typed 'categories' | 'tags' and is never caller input. That is the correct mitigation and it should be stated as such rather than left looking like an oversight.

Also in this pass

The project's only open Sonar issue, introduced by #260:

[MAJOR] backend/src/routes/adminUploadLinks.ts:126 — typescript:S1854 Remove this useless assignment to variable "outcome". Effort 15min.

outcome is initialised to 'skipped-unconfigured' at its declaration and assigned the same value again in the catch. The final review of #260 called it Minor; Sonar rates it higher, and it is worth more than tidiness — a loadStoredTemplate or renderTemplate failure currently reports as "SMTP not configured", which is a different untruth from the SMTP-rejection conflation that was deliberately approved.

Done when

The gate is green, no query call site interpolates a value, and the two that must build SQL at run time say plainly why they are safe.

The quality gate is red on one condition and one condition only: `new_security_hotspots_reviewed: 75.0` against a threshold of 100. Everything else reads 0 bugs, 0 vulnerabilities, 0 duplicated lines, 0 minutes of debt. The outstanding hotspot is `backend/src/routes/admin.ts:230`, `typescript:S2077` — "Make sure that executing SQL queries is safe here." ## What is actually true of that line ```ts const { rows: full } = await pool.query<AdminItemRow>(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [itemId]); ``` The **value is already parameterized**. `itemId` is bound as `$1`, goes to Postgres through the driver's separate parameter channel, and never enters the query text. What is interpolated is `ADMIN_ITEM_SELECT`, a module constant in `itemSelect.ts` that contains no caller data. So there is no injection here, and "parameterize it" is not a change that can be made — it is already done. S2077 fires on the *template literal*, because the rule cannot tell a module constant from a request value, and asks a human to confirm. That leaves two ways to a green gate. Marking the hotspot reviewed in the dashboard is one. Removing the interpolation is the other, and it is better: it makes the property structural rather than a human assertion that has to be repeated every time the line moves. ## The rule this adopts **No interpolation at a query call site.** Where a query's shape is fixed, the whole SQL string becomes a named constant and the call passes an identifier. That way the reviewer's question never arises, and a future edit cannot quietly turn a constant into a value. ## Fixed shape — becomes a constant | Where | Today | |---|---| | `admin.ts:176`, `admin.ts:230` | `` `${ADMIN_ITEM_SELECT} WHERE i.id = $1` `` — identical, one constant serves both | | `items.ts:97` | `` `${PUBLIC_ITEM_SELECT} WHERE i.id = $1 AND ${EXCLUDE_PENDING}` `` | | `adminItemDrafts.ts:62,63` | two fixed `DRAFT_SELECT` variants | | `adminUploadLinks.ts:72` | `LINK_SELECT` plus a fixed `ORDER BY` | ## Genuinely dynamic — stays, and stays explained `admin.ts:146` and `items.ts:86` build their `WHERE` at run time from `buildItemFilterSql`. These cannot become constants and should not pretend to be. What makes them safe is already written down at `admin.ts:132-143`: the clause fragments are string literals in `itemFilters.ts`, the only interpolations inside them are placeholder *indices* (`$${next}`), and every value goes in `params`. Worth confirming that reasoning still holds rather than assuming it, and leaving the comment pointing at it. ## Cannot be parameterized at all `draftingWorker.ts:77` interpolates a **table name**: ```ts const { rows } = await pool.query<{ name: string }>(`SELECT name FROM ${table} ORDER BY name`); ``` Postgres does not allow a parameterized identifier — `$1` can be a value, never a table. What makes this safe is that `table` is typed `'categories' | 'tags'` and is never caller input. That is the correct mitigation and it should be stated as such rather than left looking like an oversight. ## Also in this pass The project's only open Sonar issue, introduced by #260: > `[MAJOR] backend/src/routes/adminUploadLinks.ts:126` — `typescript:S1854` Remove this useless assignment to variable "outcome". Effort 15min. `outcome` is initialised to `'skipped-unconfigured'` at its declaration and assigned the same value again in the catch. The final review of #260 called it Minor; Sonar rates it higher, and it is worth more than tidiness — a `loadStoredTemplate` or `renderTemplate` failure currently reports as "SMTP not configured", which is a different untruth from the SMTP-rejection conflation that was deliberately approved. ## Done when The gate is green, no query call site interpolates a value, and the two that must build SQL at run time say plainly why they are safe.

bermudalamb referenced this issue from a commit

2026-09-04 09:36:09 -05:00

chore(sonarqube): keep interpolation out of query call sites (#294)

bermudalamb referenced a pull request that will close this issue

2026-09-04 09:36:33 -05:00

chore(sonarqube): keep interpolation out of query call sites (#294) #295

bermudalamb closed this issue

2026-09-04 09:36:40 -05:00

bermudalamb referenced this issue from a commit

2026-09-04 09:36:40 -05:00

Merge pull request 'chore(sonarqube): keep interpolation out of query call sites (#294)' (#295) from chore/294-no-interpolation-at-query-sites into main

bermudalamb referenced this issue