Conversation
|
🚅 Deployed to the rivet-pr-4211 environment in rivet-frontend
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Graphite Automations"Test" took an action on this PR • (02/16/26)1 assignee was added to this PR based on Kacper Wojciechowski's automation. |
Pull Request Review: Database Inspector FeatureThis PR adds a database inspector feature to view and query SQLite databases in Rivet Actors. The implementation is solid overall, but there are several issues that should be addressed. Critical Issues1. SQL Injection Vulnerability 🔴Location: rivetkit-typescript/packages/rivetkit/src/inspector/actor-inspector.ts:122-128 The table name is being escaped with escapeDoubleQuotes() but this is insufficient protection against SQL injection. While double-quote escaping prevents breaking out of identifier quotes, in getDatabaseTableRows() the table name comes from user input via the frontend. Recommendation:
2. Race Condition in DbMutex 🟡Location: rivetkit-typescript/packages/rivetkit/src/db/drizzle/mod.ts:62-89 Multiple concurrent acquire() calls could check this.locked === false at the same time before any sets it to true, allowing multiple acquires. Recommendation: Use a more robust mutex implementation or ensure atomic check-and-set. 3. Missing Error Handling for Database Operations 🟡Location: rivetkit-typescript/packages/rivetkit/src/inspector/actor-inspector.ts:106-150 Database queries in getDatabaseSchema() don't have try-catch blocks. If a table query fails (corrupt table, permissions, etc.), the entire operation fails without graceful degradation. Recommendation: Wrap individual table queries in try-catch to handle corrupted or inaccessible tables gracefully. Performance Issues4. Inefficient Schema Discovery 🟡Location: rivetkit-typescript/packages/rivetkit/src/inspector/actor-inspector.ts:121-146 The schema discovery queries each table individually in a loop, making 2 queries per table. For databases with many tables, this creates significant overhead. Recommendations:
5. Missing Column Type Information 🟡Location: rivetkit-typescript/packages/rivetkit/src/inspector/actor-inspector.ts:136-143 Column types are hardcoded to empty strings. Users can't see actual column types, nullability, or primary key information. Recommendation: Use PRAGMA table_info(table_name) to get accurate column metadata. Code Quality Issues6. Duplicate DbMutex Implementation 🟡The DbMutex class is duplicated in two files with identical implementations. Recommendation: Extract to a shared utility file to maintain DRY principle. 7. Version Detection Logic Change
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| async getDatabaseTableRows( | ||
| table: string, | ||
| limit: number, | ||
| offset: number, | ||
| ): Promise<ArrayBuffer> { | ||
| if (!this.isDatabaseEnabled()) { | ||
| throw new actorErrors.DatabaseNotEnabled(); | ||
| } | ||
|
|
||
| const db = this.actor.db; | ||
| const safeLimit = Math.max(0, Math.min(Math.floor(limit), 500)); | ||
| const safeOffset = Math.max(0, Math.floor(offset)); | ||
| const quoted = `"${escapeDoubleQuotes(table)}"`; | ||
| const result = await db.execute( | ||
| `SELECT * FROM ${quoted} LIMIT ? OFFSET ?`, | ||
| safeLimit, | ||
| safeOffset, | ||
| ); | ||
| return bufferToArrayBuffer(cbor.encode(result)); | ||
| } |
There was a problem hiding this comment.
Security vulnerability: getDatabaseTableRows() accepts any table name from the client without validation. Unlike getDatabaseSchema() which filters out system tables (line 115), this method allows querying any table including SQLite system tables like sqlite_master, sqlite_sequence, etc.
Impact: Malicious clients could read sensitive system metadata or internal tables.
Fix: Add table name validation:
async getDatabaseTableRows(
table: string,
limit: number,
offset: number,
): Promise<ArrayBuffer> {
if (!this.isDatabaseEnabled()) {
throw new actorErrors.DatabaseNotEnabled();
}
// Validate table name - reject system and internal tables
if (table.startsWith('sqlite_') || table.startsWith('__drizzle_')) {
throw new Error('Cannot query system or internal tables');
}
const db = this.actor.db;
const safeLimit = Math.max(0, Math.min(Math.floor(limit), 500));
const safeOffset = Math.max(0, Math.floor(offset));
const quoted = `"${escapeDoubleQuotes(table)}"`;
const result = await db.execute(
`SELECT * FROM ${quoted} LIMIT ? OFFSET ?`,
safeLimit,
safeOffset,
);
return bufferToArrayBuffer(cbor.encode(result));
}| async getDatabaseTableRows( | |
| table: string, | |
| limit: number, | |
| offset: number, | |
| ): Promise<ArrayBuffer> { | |
| if (!this.isDatabaseEnabled()) { | |
| throw new actorErrors.DatabaseNotEnabled(); | |
| } | |
| const db = this.actor.db; | |
| const safeLimit = Math.max(0, Math.min(Math.floor(limit), 500)); | |
| const safeOffset = Math.max(0, Math.floor(offset)); | |
| const quoted = `"${escapeDoubleQuotes(table)}"`; | |
| const result = await db.execute( | |
| `SELECT * FROM ${quoted} LIMIT ? OFFSET ?`, | |
| safeLimit, | |
| safeOffset, | |
| ); | |
| return bufferToArrayBuffer(cbor.encode(result)); | |
| } | |
| async getDatabaseTableRows( | |
| table: string, | |
| limit: number, | |
| offset: number, | |
| ): Promise<ArrayBuffer> { | |
| if (!this.isDatabaseEnabled()) { | |
| throw new actorErrors.DatabaseNotEnabled(); | |
| } | |
| // Validate table name - reject system and internal tables | |
| if (table.startsWith('sqlite_') || table.startsWith('__drizzle_')) { | |
| throw new Error('Cannot query system or internal tables'); | |
| } | |
| const db = this.actor.db; | |
| const safeLimit = Math.max(0, Math.min(Math.floor(limit), 500)); | |
| const safeOffset = Math.max(0, Math.floor(offset)); | |
| const quoted = `"${escapeDoubleQuotes(table)}"`; | |
| const result = await db.execute( | |
| `SELECT * FROM ${quoted} LIMIT ? OFFSET ?`, | |
| safeLimit, | |
| safeOffset, | |
| ); | |
| return bufferToArrayBuffer(cbor.encode(result)); | |
| } |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| import { Button, Flex, ScrollArea, WithTooltip } from "@/components"; | ||
| import { | ||
| faChevronLeft, | ||
| faChevronRight, | ||
| faRefresh, | ||
| faTable, | ||
| faTableCells, | ||
| Icon, | ||
| } from "@rivet-gg/icons"; | ||
| import { useQuery } from "@tanstack/react-query"; | ||
| import { useState } from "react"; | ||
| import { ShimmerLine } from "../shimmer-line"; | ||
| import { | ||
| Select, | ||
| SelectContent, | ||
| SelectItem, | ||
| SelectTrigger, | ||
| SelectValue, | ||
| } from "../ui/select"; | ||
| import { useActorInspector } from "./actor-inspector-context"; |
There was a problem hiding this comment.
Imports need to be sorted according to Biome's rules. Run 'biome check --write .' to automatically fix import sorting.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| @@ -193,7 +193,7 @@ | |||
| ], | |||
| "scripts": { | |||
| "build": "tsup src/mod.ts src/client/mod.ts src/common/log.ts src/common/websocket.ts src/actor/errors.ts src/topologies/coordinate/mod.ts src/topologies/partition/mod.ts src/utils.ts src/driver-helpers/mod.ts src/driver-test-suite/mod.ts src/serve-test-suite/mod.ts src/test/mod.ts src/inspector/mod.ts src/workflow/mod.ts src/db/mod.ts src/db/drizzle/mod.ts", | |||
There was a problem hiding this comment.
Update the build script to run the schema generation step first by changing it to: "build": "npm run build:schema && tsup src/mod.ts src/client/mod.ts src/common/log.ts src/common/websocket.ts src/actor/errors.ts src/topologies/coordinate/mod.ts src/topologies/partition/mod.ts src/utils.ts src/driver-helpers/mod.ts src/driver-test-suite/mod.ts src/serve-test-suite/mod.ts src/test/mod.ts src/inspector/mod.ts src/workflow/mod.ts src/db/mod.ts src/db/drizzle/mod.ts",
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
1 participant
Description
Implemented the database inspector feature for actors, allowing users to browse and query SQLite databases within the actor inspector UI. This feature enables viewing table schemas, browsing data with pagination, and exploring foreign key relationships.
The implementation includes:
This feature requires RivetKit version 2.0.42 or higher, which includes the new actor inspector protocol v3 with database inspection capabilities.
Type of change
How Has This Been Tested?
Tested with various SQLite database schemas and data types, including tables with foreign key relationships. Verified pagination works correctly with large datasets and that the UI properly handles different column types.
Checklist: