fix(sql-utils): harden validateRowId + post-review test fixes by zknpr · Pull Request #456 · zknpr/SQLite-Explorer

zknpr · 2026-06-08T17:18:47Z

Follow-up to the 24-PR bot batch review. After merging that batch, a read-only Codex pass over the cumulative delta found no blockers but flagged one SHOULD-FIX and two test NITs. This PR addresses them.

Changes

1. Harden validateRowId (src/core/sql-utils.ts) — the SHOULD-FIX
Previously just Number() + Number.isFinite, which silently accepted:

'' / ' ' → 0 (a missing rowid became "row 0")
'123.45' → 123.45, '1e3' → 1000 (non-integer "rowids")

These flowed into WHERE rowid = ? paths (bound params, so no injection — but malformed input was treated as a real rowid instead of erroring). Now requires a canonical integer string form (/^[+-]?\d+$/) and Number.isSafeInteger. All production callers pass real integer rowids, so no runtime impact; this just makes malformed input fail loudly.

2. tests/unit/sql-utils.test.ts — flipped the characterization tests (added in #446) that blessed the weak behavior to instead assert rejection for blank/whitespace/fractional/scientific-notation rowids. Existing accept/reject cases (incl. MAX/MIN_SAFE_INTEGER, bigint, 'abc', NaN/Infinity) unchanged.

3. tests/unit/workerFactory_browser.test.ts (NIT) — dropped the stale name arg from the test-local FakeEndpoint.exportDatabase signature (leftover from #452's arity migration; harmless but inconsistent).

4. tests/unit/databaseModel.test.ts (NIT) — the serializeDatabase test was tautological (asserted a value it set itself). Replaced with call-count + zero-arity assertions, so it now actually fails if save() regresses to passing a filename.

Verification

npx tsc --noEmit -p tsconfig.json — clean
npm test — 428 pass / 0 fail / 1 skip (pre-existing build-artifact skip)

Out of scope (noted)

There's a parallel webview validateRowId in core/ui/modules/utils.js (used by edit.js/clipboard.js) that's still lenient. It's a secondary gate — the worker re-validates — and hardening it requires rebuilding the committed minified viewer.html bundles, so it's left for a separate, build-inclusive change.

🤖 Generated with Claude Code

validateRowId only did Number()+isFinite, so '' / ' ' coerced to 0 and '123.45' / '1e3' were accepted — silently turning malformed input into plausible-but-wrong rowids on WHERE rowid = ? paths. Now require a canonical integer string form and Number.isSafeInteger (rejects blank/whitespace, fractional, scientific-notation, NaN/Infinity, and >2^53 magnitudes). Also addresses two test-quality findings from the post-batch Codex review: - workerFactory_browser.test.ts: drop stale 'name' arg from the local FakeEndpoint.exportDatabase signature (leftover from #452's arity change) - databaseModel.test.ts: replace the tautological serializeDatabase assertion (asserted a value it set itself) with call-count + zero-arity checks Tests flipped to assert rejection for blank/whitespace/fractional/scientific rowids. tsc -p tsconfig.json clean; 428 unit tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

vercel · 2026-06-08T17:18:53Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
sq-lite-explorer	Ready	Preview, Comment	Jun 8, 2026 5:24pm

gemini-code-assist

Code Review

This pull request refines SQLite row ID validation in validateRowId to reject empty, fractional, or scientific-notation strings, as well as unsafe-magnitude integers, and updates the corresponding unit tests. It also refactors DatabaseDocument save tests to avoid tautological assertions. The review feedback correctly identifies that non-string/non-numeric types (like null, boolean, and arrays) can still bypass the validation and silently coerce to 0. It suggests adding strict type checks to validateRowId and adding test cases to verify that these invalid types are rejected.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-08T17:20:11Z

 export function validateRowId(rowId: RecordId): number {
+  // For string inputs, require a canonical integer form (optional sign + digits).
+  // This rejects '', '   ', '123.45' and '1e3' up front — none are valid rowids,
+  // even though Number() would happily turn them into 0 / 123.45 / 1000.
+  if (typeof rowId === 'string' && !/^[+-]?\d+$/.test(rowId.trim())) {
+    throw new Error(`Invalid rowid: ${rowId}`);
+  }


The current implementation of validateRowId allows non-string/non-numeric types like null, boolean (true/false), and arrays ([]) to bypass the string check. Since Number(null) is 0, Number(false) is 0, and Number([]) is 0, these values will be silently coerced to 0 and accepted as valid row IDs (which violates the goal of preventing silent coercion to row ID 0).

We should strictly enforce that the input is a number, string, or bigint to prevent these unexpected type coercions.

Suggested change

export function validateRowId(rowId: RecordId): number {

// For string inputs, require a canonical integer form (optional sign + digits).

// This rejects '', ' ', '123.45' and '1e3' up front — none are valid rowids,

// even though Number() would happily turn them into 0 / 123.45 / 1000.

if (typeof rowId === 'string' && !/^[+-]?\d+$/.test(rowId.trim())) {

throw new Error(`Invalid rowid: ${rowId}`);

}

export function validateRowId(rowId: RecordId): number {

if (typeof rowId !== 'number' && typeof rowId !== 'string' && typeof rowId !== 'bigint') {

throw new Error('Invalid rowid: ' + rowId);

}

// For string inputs, require a canonical integer form (optional sign + digits).

// This rejects '', ' ', '123.45' and '1e3' up front — none are valid rowids,

// even though Number() would happily turn them into 0 / 123.45 / 1000.

if (typeof rowId === 'string' && !/^[+-]?\\d+$/.test(rowId.trim())) {

throw new Error('Invalid rowid: ' + rowId);

}

gemini-code-assist · 2026-06-08T17:20:12Z

+    it('should reject fractional numbers and unsafe-magnitude integers', () => {
+      assert.throws(() => validateRowId(123.45), /Invalid rowid: 123\.45/);
+      assert.throws(() => validateRowId(Number.MAX_SAFE_INTEGER + 1), /Invalid rowid:/);
    });


Add test cases to verify that validateRowId correctly rejects unexpected types like null, boolean, and arrays, which would otherwise be silently coerced to 0 or 1 by Number().

Suggested change

it('should reject fractional numbers and unsafe-magnitude integers', () => {

assert.throws(() => validateRowId(123.45), /Invalid rowid: 123\.45/);

assert.throws(() => validateRowId(Number.MAX_SAFE_INTEGER + 1), /Invalid rowid:/);

});

it('should reject fractional numbers, unsafe-magnitude integers, and invalid types', () => {

assert.throws(() => validateRowId(123.45), /Invalid rowid: 123\\.45/);

assert.throws(() => validateRowId(Number.MAX_SAFE_INTEGER + 1), /Invalid rowid:/);

// @ts-ignore

assert.throws(() => validateRowId(null), /Invalid rowid:/);

// @ts-ignore

assert.throws(() => validateRowId(true), /Invalid rowid:/);

// @ts-ignore

assert.throws(() => validateRowId([]), /Invalid rowid:/);

});

coderabbitai · 2026-06-08T17:20:26Z

Warning

Review limit reached

@zknpr, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 11 minutes and 13 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1c7aaf91-43d8-46e7-b8a8-3d959d86ae63

📥 Commits

Reviewing files that changed from the base of the PR and between 3eb7512 and aa90fa8.

📒 Files selected for processing (4)

src/core/sql-utils.ts
tests/unit/databaseModel.test.ts
tests/unit/sql-utils.test.ts
tests/unit/workerFactory_browser.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/rowid-hardening-review-nits

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Addresses Gemini's review on #456: the prior guard still let null/boolean/ array slip through — Number(null)/Number(false)/Number([]) are 0, Number(true) is 1, Number([5]) is 5, all passing isSafeInteger. Add an explicit typeof guard (string/number/bigint only) before coercion, plus tests asserting null/undefined/boolean/array/object are rejected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

vercel Bot deployed to Preview June 8, 2026 17:18 View deployment

gemini-code-assist Bot reviewed Jun 8, 2026

View reviewed changes

vercel Bot deployed to Preview June 8, 2026 17:24 View deployment

zknpr merged commit 4251c39 into main Jun 8, 2026
7 checks passed

zknpr deleted the fix/rowid-hardening-review-nits branch June 8, 2026 17:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sql-utils): harden validateRowId + post-review test fixes#456

fix(sql-utils): harden validateRowId + post-review test fixes#456
zknpr merged 2 commits into
mainfrom
fix/rowid-hardening-review-nits

zknpr commented Jun 8, 2026

Uh oh!

vercel Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 8, 2026

Uh oh!

gemini-code-assist Bot Jun 8, 2026

Uh oh!

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-    it('should reject fractional numbers and unsafe-magnitude integers', () => {
-      assert.throws(() => validateRowId(123.45), /Invalid rowid: 123\.45/);
-      assert.throws(() => validateRowId(Number.MAX_SAFE_INTEGER + 1), /Invalid rowid:/);
-    });
+    it('should reject fractional numbers, unsafe-magnitude integers, and invalid types', () => {
+      assert.throws(() => validateRowId(123.45), /Invalid rowid: 123\\.45/);
+      assert.throws(() => validateRowId(Number.MAX_SAFE_INTEGER + 1), /Invalid rowid:/);
+      // @ts-ignore
+      assert.throws(() => validateRowId(null), /Invalid rowid:/);
+      // @ts-ignore
+      assert.throws(() => validateRowId(true), /Invalid rowid:/);
+      // @ts-ignore
+      assert.throws(() => validateRowId([]), /Invalid rowid:/);
+    });

Uh oh!

Conversation

zknpr commented Jun 8, 2026

Changes

Verification

Out of scope (noted)

Uh oh!

vercel Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Jun 8, 2026 •

edited

Loading

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading