You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(deps): bump knex to 3.2.9 for the search_path SQL injection - #1024
Bumps knex from 2.4.2 to 3.2.9 for AIKIDO-2026-10469: knex 0.0.1–3.2.8 interpolates values passed to searchPath for the Postgres dialect into generated SQL without validating or escaping them, so a value an attacker influences becomes an injected identifier. This repository is a Postgres user and declares knex 2.4.2, so the image ships a vulnerable copy.
Version chosen: 3.2.9
3.2.9 is the smallest published version that contains the fix — #6411, "escape double quotes in searchPath to prevent SQL injection" — and the version the advisory names. 3.2.10 and 3.3.0 add no further fix for this advisory, so 3.2.9 is the smallest change that removes it.
Breaking changes read from the release notes:
3.0.0 — drops Node < 16 support, plus typing fixes for raw bindings and mixed modules. The repo requires node >=24.14.1, so nothing to do here.
3.2.0 — tightens the types (RawBinding, "types no longer allow knex to be called without tablename", first on 0 matches documented, migration lifecycle hooks). One compile error surfaced, fixed below.
3.2.9 — the fix above, plus a SQLite RETURNING fix and DELETE … LIMIT support in MySQL. Neither applies to this repo.
Nothing in src/ or the migrations needed a runtime change: pnpm build:check, the full unit suite and the integration suite (CI) pass unchanged apart from the typing fix.
The one code change
src/scripts/benchmark-queries.ts passed bindings as readonly unknown[] to db.raw(). An assertion to unknown no longer satisfies knex 3's RawBinding, so the assertion is dropped:
toSQL().bindings is already the accepted type, so this removes an assertion rather than adding one. It is a compile-time change only: the emitted JavaScript is byte-identical with and without the assertion, verified by building both ways and comparing sha256sum dist/src/scripts/benchmark-queries.js — df144e2b…df1dda in both builds.
No match, in src/ or anywhere in the repository outside node_modules. nostream never passes an identifier to search_path, so the vulnerable code path is not reachable through this codebase. The bump is still required: the vulnerable dependency should not ship, and reachability can change with a dependency, not just with our source.
The docker:test:integration suite (cucumber against a real Postgres) is not runnable from this sandbox; CI's integration job covers it and this PR does not change its inputs beyond the dependency.
Note on the lockfile
knex 3.2.9 declares an optional peer dependency on pg-query-stream ^4.14.0; this repo pins pg-query-stream 4.3.0 as a direct dependency, so pnpm resolves the peer to 4.3.0. That version is optional (used only by knex's .stream()), the resolution installs cleanly with --frozen-lockfile, and nothing here calls it. Bumping pg-query-stream is out of scope for a security bump and is not done here.
rewrite/main still needs the same bump
Checked separately, as requested. rewrite/main still declares "knex": "2.4.2" and still imports knex at runtime:
It also still ships the TypeScript server and its test:unit suite in checks.yml. The same bump is therefore being opened against rewrite/main as its own PR rather than mixed into this branch.
AIKIDO-2026-10469: knex 0.0.1-3.2.8 interpolates values passed to
searchPath for the Postgres dialect into generated SQL without validating or
escaping them. nostream declares knex 2.4.2, so the image ships a vulnerable
copy.
3.2.9 is the smallest published version that contains the fix (#6411 escapes
double quotes in searchPath), and it is the version Aikido names. 3.2.10 and
3.3.0 carry no further security fix; 3.3.0 also lands a fix to something other
than this advisory, so the smallest bump above the fixed version is the one that
changes the least.
The repo never sets searchPath: grep -rn "search_path" src/ finds no match, so
the vulnerable code path is not reachable here. The bump is still taken, because
the vulnerable dependency should not ship.
knex 3.0.0 drops Node < 16 (the repo requires >=24.14.1) and 3.2.0 tightens the
RawBinding type. That surfaced one compile error: benchmark-queries.ts passed
"bindings as readonly unknown[]" to db.raw(), and an assertion to unknown no
longer satisfies RawBinding. Dropping the assertion is the fix, not a new one -
toSQL().bindings is already the accepted type, and the emitted JavaScript is
byte-identical with and without the assertion.
No actionable comments were generated in the recent review.
Recent review info
Automatic approval policy
Commit: eb304b80c9cb.
Setting
Effective value
Source
Automatic approvals
Enabled
Global configuration
Merge risk
Minimal
Global configuration
Security risk
Not set
Global configuration
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: c25dbeea-03a6-485b-9085-23bf60fac8f5
Commits
Reviewing files that changed from the base of the PR and between bc22677 and eb304b8.
Files ignored due to path filters: 1
pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
Files selected for processing: 3
.changeset/knex-3-2-9-security.md
package.json
src/scripts/benchmark-queries.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
Walkthrough
The Knex dependency changes from 2.4.2 to 3.2.9. The benchmark query passes SQL bindings to db.raw without a readonly-array assertion. A changeset documents the upgrade and related compatibility details.
Changes: Knex upgrade
Layer / File(s)
Summary
Knex upgrade and query compatibility .changeset/knex-3-2-9-security.md, package.json, src/scripts/benchmark-queries.ts
The Knex version changes to 3.2.9. The benchmark query passes bindings directly to db.raw. The changeset records the upgrade details, including the reported searchPath issue and Node requirement.
No merge-blocking issue is established for the dependency upgrade or benchmark binding change. Normal checks remain appropriate before merging.
Architecture risk: Low · up to eb304
The change affects 2 systems.
Changed systems:src, package.json
Architecture concerns
No architecture-level concerns identified.
Review details
Systems and components
observed — src (service) was modified; 1 changed file maps to changed impact.
observed — package.json (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
observed — Modified behavior in package.json: The knex dependency version changes from 2.4.2 to 3.2.9.
observed — Modified behavior in src/scripts/benchmark-queries.ts: db.raw now receives bindings directly instead of receiving them cast as readonly unknown[].
observed — Modified behavior in .changeset/knex-3-2-9-security.md: Adds patch changeset metadata for nostream and a title describing the Knex 3.2.9 dependency update.
observed — Modified behavior in .changeset/knex-3-2-9-security.md: Adds text describing the reported searchPath interpolation issue, the 3.2.9 escaping change, the repository’s declared Knex versions, and the claim that nostream does not set searchPath.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely identifies the main change: upgrading Knex to 3.2.9 to address the search_path SQL injection.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (2 skipped: 2 unsupported.)
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the bindings with care, A newer Knex hops into the burrow. No readonly cast blocks the path, The changeset records the upgrade, Then quiet paws return to the meadow.
[Medium impact] The PR appears safe to merge; no actionable issues were found.
Summary
Updates knex from 2.4.2 to 3.2.9 for the reported searchPath SQL injection fix.
The relay's PostgreSQL callers now use Knex 3.2.9.
Acknowledged by chappie-daemon: the optional pg-query-stream peer mismatch is intentionally left unchanged because streaming is unused. The separate rewrite/main update is deferred to its own PR.
Brings 142 commits from main onto the rewrite line, which had 30 of its own.
Three files conflicted and each resolution keeps the side that matches the code
this branch actually runs:
- .github/workflows/checks.yml — union. The rewrite's path filters for
resources/ and crates/ stay; main's two globs (.github/workflows/** and
.github/scripts/**) are added to the src filter, because a unit test reads
those files and #965 skipped the suite a release.yml change broke.
- CONFIGURATION.md — the rewrite's mirroring.static[] rows stay. main removed
the setting along with its TypeScript worker; this branch still parses it in
nostream-config (settings.rs, guided.rs, validate.rs and the mirror_filters
test), so main's "removed" row describes a worker that never existed here.
- resources/default-settings.yaml — the rewrite's connection.maxConnectionsPerIp
block stays, and main's comment about period units is kept above the
rateLimits list it describes.
The knex 3.2.9 bump from #1024 arrives with the merge, so the standalone PR that
carried the same fix onto this branch is superseded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Bumps
knexfrom 2.4.2 to 3.2.9 for AIKIDO-2026-10469: knex 0.0.1–3.2.8 interpolates values passed tosearchPathfor the Postgres dialect into generated SQL without validating or escaping them, so a value an attacker influences becomes an injected identifier. This repository is a Postgres user and declaresknex2.4.2, so the image ships a vulnerable copy.Version chosen: 3.2.9
3.2.9 is the smallest published version that contains the fix — #6411, "escape double quotes in searchPath to prevent SQL injection" — and the version the advisory names. 3.2.10 and 3.3.0 add no further fix for this advisory, so 3.2.9 is the smallest change that removes it.
Breaking changes read from the release notes:
node >=24.14.1, so nothing to do here.RawBinding, "types no longer allow knex to be called without tablename",firston 0 matches documented, migration lifecycle hooks). One compile error surfaced, fixed below.RETURNINGfix andDELETE … LIMITsupport in MySQL. Neither applies to this repo.Nothing in
src/or the migrations needed a runtime change:pnpm build:check, the full unit suite and the integration suite (CI) pass unchanged apart from the typing fix.The one code change
src/scripts/benchmark-queries.tspassedbindings as readonly unknown[]todb.raw(). An assertion tounknownno longer satisfies knex 3'sRawBinding, so the assertion is dropped:const { rows } = await db.raw<{ rows: { 'QUERY PLAN': ExplainResult[] }[] }>( `EXPLAIN (ANALYZE, BUFFERS, VERBOSE, FORMAT JSON) ${sql}`, - bindings as readonly unknown[], + bindings, )toSQL().bindingsis already the accepted type, so this removes an assertion rather than adding one. It is a compile-time change only: the emitted JavaScript is byte-identical with and without the assertion, verified by building both ways and comparingsha256sum dist/src/scripts/benchmark-queries.js—df144e2b…df1ddain both builds.search_path: not usedNo match, in
src/or anywhere in the repository outsidenode_modules. nostream never passes an identifier tosearch_path, so the vulnerable code path is not reachable through this codebase. The bump is still required: the vulnerable dependency should not ship, and reachability can change with a dependency, not just with our source.Verification
The 3 warnings are pre-existing
lint/style/useBlockStatementswarnings intest/unit/ci/*.spec.ts; none are in a file this PR touches.Exits 0 with no output. Resolved dependency:
The
docker:test:integrationsuite (cucumber against a real Postgres) is not runnable from this sandbox; CI's integration job covers it and this PR does not change its inputs beyond the dependency.Note on the lockfile
knex 3.2.9 declares an optional peer dependency on
pg-query-stream ^4.14.0; this repo pinspg-query-stream4.3.0 as a direct dependency, so pnpm resolves the peer to 4.3.0. That version is optional (used only by knex's.stream()), the resolution installs cleanly with--frozen-lockfile, and nothing here calls it. Bumpingpg-query-streamis out of scope for a security bump and is not done here.rewrite/mainstill needs the same bumpChecked separately, as requested.
rewrite/mainstill declares"knex": "2.4.2"and still imports knex at runtime:It also still ships the TypeScript server and its
test:unitsuite inchecks.yml. The same bump is therefore being opened againstrewrite/mainas its own PR rather than mixed into this branch.Changeset
.changeset/knex-3-2-9-security.md—"nostream": patch.This is dependency hygiene: the advisory is not reachable in this codebase today.
Summary by CodeRabbit
Bug Fixes
Maintenance