Conversation
railway-app
bot
temporarily deployed
to
rivet-frontend / rivet-pr-4211
|
🚅 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 for browsing SQLite databases inside Rivet Actors. The overall structure is sound, but there are several important issues to address before merging. Critical Issues1. Column Schema Uses Sample Row Instead of PRAGMA — Empty Types and No Foreign Keys
const sample = await db.execute(`SELECT * FROM ${quoted} LIMIT 1`);
const columnNames = sample?.[0] ? Object.keys(sample[0]) : [];
columns: columnNames.map((name, cid) => ({
cid, name,
type: "", // always empty
notnull: 0, // always zero
dflt_value: null,
pk: 0, // always zero
})),
foreignKeys: [], // always emptyTwo problems with this approach:
SQLite provides exactly the right pragmas for this: PRAGMA table_info("tablename"); -- cid, name, type, notnull, dflt_value, pk
PRAGMA foreign_key_list("tablename"); -- id, seq, table, from, to, ...The backend should use these instead of the sample-row approach. 2. Indentation Bug in
|
![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.
NathanFlurry
force-pushed
the
02-11-feat_add_xyflow
branch
from
7b2929d to
b13ac06
Compare
NathanFlurry
force-pushed
the
02-16-feat_add_database_inspector
branch
from
6bbc16d to
73e303e
Compare
| }, | ||
| close: async () => { | ||
| await waDb.close(); |
There was a problem hiding this comment.
Race condition: close() is not protected by the mutex while execute() is. If close() is called while a query is running, it will cause database corruption or "file is not a database" errors.
close: async () => {
await mutex.run(() => waDb.close());
},Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Merge activity
|
# 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: - New database table component with column resizing and sorting capabilities - Database schema retrieval API in the actor inspector - Table row pagination with configurable limits - Foreign key relationship visualization - UI integration in the actor details panel 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 - [x] New feature (non-breaking change which adds functionality) ## 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: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes
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: