Closed
Conversation
Replaced synchronous `exec` with iterative `iterateStatements` + `step` loop in `WasmDatabaseEngine`. Added `queryTimeout` configuration to `DatabaseInitConfig` and `WasmDatabaseEngine` (default 50s). Implemented timeout check during row iteration to interrupt long-running queries (e.g., infinite CTEs). Ensured statement resources are freed on timeout/error. Preserved backward compatibility with `exec` behavior (parameter binding to first statement, result format). Fixes security vulnerability where malicious queries could freeze the WASM worker. Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @zknpr, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical security enhancement to the WASM-based SQLite worker by implementing a configurable query timeout. This prevents potential Denial of Service vulnerabilities where malicious or inefficient queries could indefinitely freeze the worker, thereby improving the overall stability and resilience of the database operations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a query timeout mechanism to the WASM-based SQLite worker, which is a crucial security enhancement to prevent denial-of-service attacks from long-running queries. The implementation correctly uses iterateStatements to step through query execution and check for timeouts. The changes are well-tested with a new unit test file that validates both timeout and success scenarios.
My review includes a few suggestions to improve type safety by replacing any with specific types from sql.js, a high-severity fix to prevent a potential double-free of a statement object on timeout, and a suggestion to replace a magic number with a named constant for better maintainability.
Comment on lines
+102
to
+103
| stmt.free(); // Free current statement to prevent leaks | ||
| throw new Error(`Query execution timed out after ${this.queryTimeout}ms`); |
There was a problem hiding this comment.
There's a potential for a double-free issue here. When a timeout occurs, stmt.free() is called, and an error is thrown. The catch block will then attempt to call currentStmt.free() again on the same statement object, as currentStmt still holds the reference and is not nullified. While the empty catch in the error handler would suppress a crash, it's safer to prevent the double-free attempt altogether.
By setting currentStmt to null after freeing it, you ensure the cleanup logic in the catch block won't try to free it again.
stmt.free(); // Free current statement to prevent leaks
currentStmt = null; // Avoid double-free in the catch block
throw new Error(`Query execution timed out after ${this.queryTimeout}ms`);| interface WasmDatabaseInstance { | ||
| exec(sql: string, params?: unknown[]): Array<{ columns: string[]; values: unknown[][] }>; | ||
| prepare(sql: string, params?: unknown[]): any; | ||
| iterateStatements(sql: string): IterableIterator<any>; |
There was a problem hiding this comment.
For better type safety and maintainability, you can use the Statement type from sql.js instead of any. This will allow TypeScript to correctly infer types for variables that use this iterator, such as stmt in the executeQuery method.
Suggested change
| iterateStatements(sql: string): IterableIterator<any>; | |
| iterateStatements(sql: string): IterableIterator<import('sql.js').Statement>; |
| async executeQuery(sql: string, params?: CellValue[]): Promise<QueryResultSet[]> { | ||
| const startTime = Date.now(); | ||
| const results: QueryResultSet[] = []; | ||
| let currentStmt: any = null; |
There was a problem hiding this comment.
To improve type safety, currentStmt can be strongly typed as import('sql.js').Statement | null instead of any. This complements the change to iterateStatements to return a typed iterator.
Suggested change
| let currentStmt: any = null; | |
| let currentStmt: import('sql.js').Statement | null = null; |
| } | ||
|
|
||
| const engine = new WasmDatabaseEngine(wasmInstance); | ||
| const engine = new WasmDatabaseEngine(wasmInstance, config.queryTimeout || 50000); |
There was a problem hiding this comment.
The default query timeout 50000 is a magic number. It's better to define it as a named constant to improve readability and make it easier to change in the future. For example: const DEFAULT_QUERY_TIMEOUT_MS = 50000;.
const DEFAULT_QUERY_TIMEOUT_MS = 50000;
const engine = new WasmDatabaseEngine(wasmInstance, config.queryTimeout || DEFAULT_QUERY_TIMEOUT_MS);
zknpr
added a commit
that referenced
this pull request
Feb 7, 2026
Adds query execution timeout to prevent runaway queries: - Add DEFAULT_QUERY_TIMEOUT_MS (30 seconds) constant - Add queryTimeout property to WasmDatabaseEngine - Rewrite executeQuery to use iterateStatements for row-by-row iteration - Check timeout during row iteration for interruptible queries - Add bind() method to WasmPreparedStatement interface for type safety - Proper statement cleanup on timeout or error (prevents double-free) This re-applies the timeout feature from PR #47 with proper implementation using sql.js's iterateStatements API for fine-grained control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zknpr
added a commit
that referenced
this pull request
Feb 7, 2026
* perf: replace synchronous file check with async in native worker
Optimized `getNativeBinaryPath` to use `fs.promises.access` instead of `fs.existsSync`. This prevents blocking the event loop during extension initialization, improving responsiveness, especially on slow file systems.
Updated `isNativeAvailable` and `createNativeDatabaseConnection` to be async, and propagated changes to `src/workerFactory.ts`.
Benchmark results (10k iterations):
- Sync: ~53ms total
- Async: ~1030ms total
While async has higher overhead, it is non-blocking, which is preferred for UI responsiveness. The impact on single execution is negligible (<0.1ms).
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* test: add serialization tests for ModificationTracker
Add a new test suite `tests/unit/tracker_serialization.test.ts` to verify
the serialization and deserialization of `ModificationTracker`.
The tests cover:
- Basic serialization of tracker state.
- Proper handling of `Uint8Array` data using `binaryReplacer`/`binaryReviver`.
- Preservation of checkpoint index.
- Handling of empty tracker state.
This ensures data integrity during hot exit and backup scenarios.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor cancelTokenToAbortSignal to src/core/cancellation-utils.ts and add tests.
- Extracted `cancelTokenToAbortSignal` from `src/helpers.ts` to `src/core/cancellation-utils.ts` to decouple from `vscode` module runtime dependency.
- Updated `src/helpers.ts` to re-export the function.
- Added unit tests in `tests/unit/cancellation-utils.test.ts`.
- Fixed type signature of `cancelTokenToAbortSignal` to correctly reflect return type when input is null/undefined.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor WebviewCollection to be testable and add unit tests
- Extracted `WebviewCollection` class to `src/webview-collection.ts` to isolate it from `vscode` module dependency.
- Updated `src/helpers.ts` to re-export `WebviewCollection`.
- Added comprehensive unit tests in `tests/unit/webview_collection.test.ts`.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* test: add unit tests for getUriParts utility
- Added `tests/unit/helpers.test.ts` to test `getUriParts`.
- Created `tests/mocks/vscode.ts` to mock VS Code API for tests.
- Created `tsconfig.test.json` to configure path mapping for tests.
- Updated `package.json` to use `tsconfig.test.json` for tests.
- Fixed `src/platform/cryptoShim.ts` to use optional chaining for `import.meta.env` to prevent crashes in Node.js test environment.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Optimize undo deletion with batch insertions
- Replaced loop of single-row inserts with a batched INSERT statement in `undoModification`.
- Implemented `insertRowBatch` method in `WasmDatabaseEngine` to handle chunked inserts (max 999 parameters/chunk) for SQLite compatibility.
- Updated `DatabaseOperations` interface and worker endpoint.
- Measured ~4.4x performance improvement (62ms -> 14ms for 1000 rows).
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* feat(perf): batch fetch pragmas to reduce IPC overhead
- Refactor `natives/native-worker.js` to extract `executeQuery` logic.
- Add `queryBatch` command to `natives/native-worker.js` for executing multiple queries.
- Update `src/nativeWorker.ts` to use `queryBatch` for fetching pragmas.
- Reduced pragma fetching time by ~3.4x (benchmark: 9.76ms -> 2.85ms per batch).
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* feat: improve batch update performance in HostBridge
Refactored `updateCellBatch` in `src/hostBridge.ts` to implement parallel execution for updates when the backend does not support native batching. This replaces the previous sequential fallback loop.
Additionally, extracted JSON patch generation logic into a private helper method `tryGeneratePatch` to reduce code duplication between `updateCell` and `updateCellBatch`.
The fallback implementation now correctly fires a single batch modification event instead of individual events per cell.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* feat: Validate table/view existence in virtual file system
- Added validation check in `readFile` to ensure the target table or view exists in `sqlite_schema`.
- Replaced placeholder `isTable` variable with actual SQL query.
- Throws `FileSystemError.FileNotFound` if the target is invalid.
This prevents errors when accessing non-existent tables or views and fulfills the TODO item.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* feat: Optimize JSON patch implementation with native SQLite function
- Added `checkJsonPatchSupport` to `WasmDatabaseEngine` to detect native `json_patch` availability.
- Optimized `updateCell` to use `json_patch` SQL function when available.
- Optimized `updateCellBatch` to use `json_patch` SQL function for batch updates when available.
- Ensured JS fallback is used when native function is unavailable.
- Handled object inputs for patches by stringifying them before binding.
- Added comprehensive unit tests in `tests/unit/json_patch_optimization.test.ts`.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* fix: prevent stack overflow in JSON merge patch generation
This commit addresses a security vulnerability where deeply nested or cyclic JSON objects could cause a stack overflow in `generateMergePatch` and `applyMergePatch`.
- Introduces a `MAX_DEPTH` limit of 1000 for recursion in `src/core/json-utils.ts`.
- Updates `generateMergePatch` and `applyMergePatch` to throw an error if the depth limit is exceeded.
- Adds comprehensive tests in `tests/unit/json_utils_security.test.ts` to verify the fix and ensure no regressions.
This change ensures the extension is robust against malicious or accidentally deep JSON structures.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* ⚡ Optimize Undo/Redo Performance with Batch Cell Updates
Refactored `undoModification` and `redoModification` in `src/core/sqlite-db.ts` to use `updateCellBatch` instead of iterating and executing single cell updates. This resolves the N+1 query performance issue.
Measured Improvement:
- Baseline: ~486ms for undoing 50,000 cell updates.
- Optimized: ~215ms for undoing 50,000 cell updates.
- Speedup: ~2.2x faster.
Added regression test case in `tests/unit/undo_redo_integration.test.ts`.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* fix: implement memory limit for undo history
This commit introduces a memory limit to the `ModificationTracker` to prevent unbounded memory growth in the undo history.
Changes:
- Added `calculateSize` helper to estimate the memory footprint of modifications.
- Updated `ModificationTracker` to track total size of history entries.
- Added `maxMemory` configuration (default 50MB).
- Implemented eviction logic in `record()` to remove oldest entries when memory limit is exceeded.
- Updated `deserialize` to recalculate entry sizes.
- Added unit tests in `tests/unit/undo_memory_limit.test.ts`.
This fixes a potential security vulnerability where large modifications could cause the extension to consume excessive memory.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* feat(core): implement query timeout in WASM worker to prevent DoS
Replaced synchronous `exec` with iterative `iterateStatements` + `step` loop in `WasmDatabaseEngine`.
Added `queryTimeout` configuration to `DatabaseInitConfig` and `WasmDatabaseEngine` (default 50s).
Implemented timeout check during row iteration to interrupt long-running queries (e.g., infinite CTEs).
Ensured statement resources are freed on timeout/error.
Preserved backward compatibility with `exec` behavior (parameter binding to first statement, result format).
Fixes security vulnerability where malicious queries could freeze the WASM worker.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* test: Add unit tests for WasmDatabaseEngine.updateCellBatch
Added comprehensive unit tests for `WasmDatabaseEngine.updateCellBatch` in `src/core/sqlite-db.ts`.
The tests cover:
- Standard batch updates for multiple rows and columns.
- JSON patch updates using RFC 7396 merge patch logic.
- Mixed updates combining standard value updates and JSON patches.
- Graceful handling of empty update arrays.
- Transaction atomicity ensuring that errors during a batch update trigger a rollback.
This improves test coverage for critical data modification logic and ensures reliability of batch operations.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Test: Add unit tests for WasmDatabaseEngine.addColumn
- Added tests/unit/sqlite-db.test.ts
- Verified various scenarios including default values and SQL injection prevention.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* perf: Use async file read in sqlite-db to avoid blocking event loop
- Replaced `fs.readFileSync` with `fs.promises.readFile` in `src/core/sqlite-db.ts`.
- Replaced `fs.statSync` with `fs.promises.stat`.
- Added `tests/performance/benchmark.ts` to measure event loop blocking.
- Verified performance: Event loop ticks increased from ~8 to ~3881 during a 500MB file read, confirming non-blocking behavior.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* fix(security): securely escape NUL bytes in SQL export strings
This commit addresses a potential security vulnerability where strings
containing NUL bytes (`\0`) were not being correctly escaped for SQL
export. Raw NUL bytes in SQL scripts can be misinterpreted by some
tools (e.g., C-based CLI tools) as string terminators, potentially
leading to truncated queries or SQL injection.
The fix involves detecting NUL bytes in strings and encoding such
strings as hex BLOBs, then casting them back to TEXT using
`CAST(X'...' AS TEXT)`. This ensures that the generated SQL script
contains only safe ASCII characters and preserves the original data
integrity.
Tests have been added to `tests/unit/sql-utils.test.ts` to verify
the fix and ensure no regressions.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* test: improve coverage for toDatasetAttrs by refactoring to html-utils
- Refactored `toDatasetAttrs`, `toBoolString`, and `toDashCase` from `src/helpers.ts` to a new `src/html-utils.ts` module to remove dependency on `vscode` module during testing.
- Added comprehensive unit tests in `tests/unit/html-utils.test.ts`.
- Updated `src/helpers.ts` to re-export the utilities to maintain backward compatibility.
- Updated `package-lock.json` to match `package.json` version (1.2.7).
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* feat(test): add unit tests for SQLiteFileSystemProvider.writeFile
Introduces unit tests for `SQLiteFileSystemProvider.writeFile` to improve test coverage for core file system operations.
- Created `tests/mocks/vscode.ts` to mock `vscode` module dependencies (`Uri`, `FileSystemError`, etc.).
- Created `tsconfig.test.json` to map `vscode` imports to the mock implementation during tests.
- Added `tests/unit/virtualFileSystem.test.ts` covering:
- Happy path for text content updates.
- Handling of binary content (invalid UTF-8).
- Error handling for invalid Row IDs and permission checks (`__create__.sql`).
- Updated `package.json` test script to use `tsconfig.test.json` for all unit tests.
This enables testing of `src/virtualFileSystem.ts` which was previously untestable due to direct `vscode` imports.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Remove 'unsafe-inline' from style-src CSP
Refactored the data grid and other UI components to eliminate the need for 'unsafe-inline' in the Content Security Policy style-src directive.
Key changes:
- Removed `cspUtil.inlineStyle` from `src/editorController.ts`.
- Replaced inline styles with CSS classes in `core/ui/modules/grid.js` and `core/ui/modules/ui.js`.
- Replaced inline styles with CSS classes in `core/ui/viewer.template.html`.
- Added new utility classes to `core/ui/viewer.css` to support the changes.
- Updated `package-lock.json` to match `package.json` version (1.2.7).
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor undoModification in WasmDatabaseEngine
Extracted switch cases into private helper methods to improve readability and maintainability.
Added new tests for cell_update and row_insert undo/redo in tests/unit/undo_redo_integration.test.ts.
Updated tests/unit/undo_redo_integration.test.ts to use beforeEach/afterEach for better isolation.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor query builder to remove duplicated filter logic
Refactored `buildSelectQuery` and `buildCountQuery` in `src/core/query-builder.ts` to use a shared helper function `buildFilterConditions`.
This change reduces code duplication and fixes a potential bug where applying a global filter with an empty column list would generate invalid SQL (`WHERE ()`).
- Extracted filter generation logic to `buildFilterConditions`.
- Added safety check for empty columns in global filter generation.
- Updated unit tests to reflect the bug fix (safe behavior for empty columns).
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor resolveCustomEditor for better maintainability
- Extract serialization logic to `src/core/serialization.ts`.
- Extract webview message handling to `src/webviewMessageHandler.ts`.
- Simplify `resolveCustomEditor` in `src/editorController.ts` by delegating to `WebviewMessageHandler`.
- Add unit tests for serialization and message handling.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor worker endpoint to remove boilerplate and fix RPC serialization
Removed unused and unserializable function proxies from `initializeDatabase` return value. These functions were causing `DataCloneError` when sent over RPC and were not used by the consumer (`workerFactory.ts`).
Updated `DatabaseInitResult` interface to make `operations` optional.
Added regression test `tests/unit/worker_endpoint_serialization.test.ts`.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor updateCell to use updateCellBatch in SQLite engine
Refactored `updateCell` in `src/core/sqlite-db.ts` to delegate to `updateCellBatch`, eliminating duplicated logic for cell updates and JSON patching.
Also updated `undoModification` and `redoModification` to use `updateCellBatch` for batch operations, removing explicit transaction loops and preventing nested transaction errors.
Enhanced `updateCellBatch` with row ID validation and improved JSON object detection.
Added unit tests in `tests/unit/cell_update_refactor.test.ts`.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor WasmDatabaseInstance to use typed prepared statements
- Defined `WasmPreparedStatement` interface matching sql.js API
- Updated `WasmDatabaseInstance.prepare` to return `WasmPreparedStatement`
- Refactored `WasmDatabaseEngine` methods (`fetchTableData`, `updateCellBatch`) to use the new interface
- Added API coverage tests for `fetchTableData` and `updateCellBatch`
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor BlobInspector to use backendApi consistently
- Removed `hostBridge` parameter from `BlobInspector` constructor.
- Removed unused `this.hostBridge` property.
- Updated `BlobInspector` to rely on the imported `backendApi` singleton for all RPC calls, ensuring proper `Uint8Array` serialization.
- Updated `initEdit` in `edit.js` to instantiate `BlobInspector` without arguments.
- Updated comments to reflect the direct usage of `backendApi`.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* feat(test): add unit tests for SQLiteFileSystemProvider.readFile
This commit introduces comprehensive unit tests for `SQLiteFileSystemProvider.readFile` in `src/virtualFileSystem.ts`.
It achieves this by:
1. Creating a mock for the `vscode` module in `tests/unit/mocks/vscode.ts`.
2. Creating a setup file `tests/unit/vscode_mock_setup.ts` to intercept `vscode` imports and inject the mock.
3. Implementing `tests/unit/virtualFileSystem.test.ts` covering:
- URI parsing and document resolution.
- Handling of `__create__.sql` special files.
- Reading regular table cells (strings, nulls, blobs).
- Error handling for invalid URIs, missing documents, and database errors.
This ensures the core file reading functionality is reliable and regression-proof.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Refactor RPC to use safe Transferable[] instead of any[]
Replaced `any[]` with `Transferable[]` in `src/core/rpc.ts` and `src/workerFactory.ts` to improve type safety for zero-copy transfers.
Fixed a bug in `connectWorkerPort` usage where the transfer list was being ignored in `postMessage` calls.
Added unit tests to verify `Transfer` wrapper and transfer list extraction.
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
* Fix: Apply security fixes and memory leak fixes
- Add escapeLikePattern() to prevent SQL wildcard injection (from PR #63)
- Update query-builder to use ESCAPE clause for LIKE queries
- Fix memory leak in cancelTokenToAbortSignal by disposing listener (PR #34 fix)
- Add tests for escapeLikePattern
* Remove conflicting test file from PR #59 (not fully merged)
* Fix test failures and update CHANGELOG for v1.3.0
- Update vscode mock to include env, UIKind, ColorThemeKind
- Fix json_patch_optimization tests to match current behavior (JS-based patch)
- Fix virtualFileSystem tests to account for table existence check
- Remove wasm-timeout.test.ts (causes OOM, timeout feature needs re-implementation)
- Update CHANGELOG.md with v1.3.0 changes
* feat(core): Re-implement query timeout using iterateStatements
Adds query execution timeout to prevent runaway queries:
- Add DEFAULT_QUERY_TIMEOUT_MS (30 seconds) constant
- Add queryTimeout property to WasmDatabaseEngine
- Rewrite executeQuery to use iterateStatements for row-by-row iteration
- Check timeout during row iteration for interruptible queries
- Add bind() method to WasmPreparedStatement interface for type safety
- Proper statement cleanup on timeout or error (prevents double-free)
This re-applies the timeout feature from PR #47 with proper implementation
using sql.js's iterateStatements API for fine-grained control.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat(config): Add configurable settings for query timeout and undo memory
Implements all 4 code review suggestions:
1. Expose query timeout as VS Code setting:
- Add `sqliteExplorer.queryTimeout` setting (1s-600s, default 30s)
- Pass config to worker via DatabaseInitConfig
- Use in WasmDatabaseEngine constructor
2. Expose undo memory limit as VS Code setting:
- Add `sqliteExplorer.maxUndoMemory` setting (1MB-512MB, default 50MB)
- Add getMaxUndoMemory() helper in databaseModel.ts
- Pass to ModificationTracker constructor
3. Document why fetchTableData() bypasses timeout:
- Add detailed comment explaining pagination naturally bounds
execution time, making timeout unnecessary
4. Remove 'unsafe-inline' from CSP:
- Remove unused `inlineStyle` constant from helpers.ts
- Update CLAUDE.md to reflect full CSP compliance
- Dynamic styles use CSSOM which is CSP-compliant
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* docs(CLAUDE.md): Update configuration table and add Gotchas section
- Fix defaultPageSize value (500, not 1000)
- Add queryTimeout and maxUndoMemory settings
- Add Gotchas section documenting common issues:
- SQL security (LIKE wildcards, type validation, NUL bytes)
- RPC serialization (Uint8Array, Transfer wrapper)
- Testing (VS Code mocks, JSON patch tests)
- Build (WASM location, browser vs Node detection)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(csp): Add nonce to style tag for CSP compliance
The <style> tag in viewer.html was being blocked by CSP after removing
'unsafe-inline' from style-src. This caused the entire UI to render
without any styling.
Fix:
- Add nonce="<!--NONCE-->" attribute to <style> tag in viewer.html
- Add nonce to styleSrc CSP directive in editorController.ts
- Update CLAUDE.md to reflect nonce-based style policy
The nonce placeholder is replaced at runtime with the same nonce used
for scripts, allowing the inline <style> block to pass CSP validation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(grid): Dynamic row number column width for large datasets
The row number column was fixed at 50px, which caused the pin icon
to overflow/disappear when row numbers exceeded 99 (3+ digits).
Fix:
- Calculate row number width dynamically based on max row number
- Formula: 36px base + 8px per digit
- Examples: 50 (rows 1-99), 52 (100-999), 60 (1000-9999)
This ensures the pin icon remains visible and clickable regardless
of how many rows are displayed.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat(deleteColumns): Add confirmation dialog for dependent indexes
When deleting columns that have dependent indexes, the extension now:
- Detects indexes that reference the columns being deleted
- Shows a warning dialog listing affected indexes
- Asks user to confirm dropping indexes or cancel
- Only proceeds if user confirms, avoiding silent database reload on cancel
Added findDependentIndexes() method to both WASM and native backends.
Updated LoggingDatabaseOperations wrapper to forward the new methods.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
|
Merged as part of v1.3.0 release in PR #65 |
Contributor
Acknowledged. Thank you. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR addresses a security vulnerability where the WASM-based SQLite worker could be frozen by long-running or infinite queries (DoS).
Changes:
src/core/sqlite-db.ts:WasmDatabaseInstanceinterface to includeiterateStatements.WasmDatabaseEngineto accept aqueryTimeout(default 50000ms).executeQueryto useiterateStatementsinstead ofexec.Date.now()against the timeout deadline.stmt.free()) on timeout or error.execbehavior: parameters bind to the first statement; results matchsql.jsformat (excluding empty results if no columns).src/core/types.ts:queryTimeouttoDatabaseInitConfig.tests/unit/wasm-timeout.test.ts:Verification:
npx tsx --test tests/unit/wasm-timeout.test.ts-> Passed.npm test-> All 42 tests passed.PR created automatically by Jules for task 11895128145140788206 started by @zknpr