This repository was archived by the owner on Nov 7, 2026. It is now read-only.
Repository navigation
fix(core): prevent SQLite permit leaks and restrict DB files - #395
Merged
Merged
Conversation
1. Cherry-pick the two pinned upstream donors with -x, retaining only the assigned behaviors and the fork's Effect beta.83 adapters and database contracts. 2. Make permit acquisition cancellation-safe in the Bun, Core Node, and standalone Node adapters without changing retry or transaction semantics. 3. Prepare POSIX database, WAL, and SHM modes at Database.layerFromPath execution, preserving in-memory operation, existing-file locks, EventWriter registration, auto-vacuum ordering, and db path purity. 4. Add bounded real-client regressions for queued reserve and withTransaction plus producer-driven file-mode and live-lock tests; demonstrate focused red/green results. 5. Record partial-port provenance in CHANGESET.md and hand off the verified branch through the existing draft PR after focused controls, lint, and identity checks.
Author
|
Session ID: unknown |
Partial port of anomalyco#60651 (c6056f3) into packages/core/src/database/sqlite.bun.ts, packages/core/src/database/sqlite.node.ts, and packages/effect-sqlite-node/src/index.ts. Use a Scope-owned child with the fork-pinned Semaphore.withPermits API rather than upgrading Effect or importing Semaphore.takeScoped. Keep the same-connection retry/backoff gate, adapters, and transaction behavior; exclude workerd, generated API changes, and unrelated compatibility edits. The reserve leg of the adapted sweep in packages/core/test/sqlite-busy.test.ts is the acquisition-interruption red/green proof on Bun, Core Node, and standalone Node. The queued withTransaction leg is a preservation control: the pinned SqlClient.makeWithTransaction masks acquireConnection, so this port does not claim to change its wait-cancellation semantics. (cherry picked from commit c6056f3)
Partial adapted port of anomalyco#52150 (89d37a0) at the fork's Database.layerFromPath boundary, in packages/core/src/database/file-permissions.ts and packages/core/src/database/database.ts. Create missing files exclusively with owner-only permissions, tighten existing database/WAL/SHM files before SQLite opens, and never open or close an existing database during preparation. Keep in-memory operation filesystem-free, Windows inherited ACLs, EventWriter registration, fresh-store auto-vacuum ordering, and pure db path selection. Exclude the donor's replaced layer(options) API and its Global path-resolution flow. Adapt the donor database regression in packages/core/test/database.test.ts to exercise Core opening, SQLite-produced existing sidecars and data, and same-process lock preservation against an independent contender. Cover explicit --database selection through the real online-prune CLI in packages/opencode/test/cli/db/prune.test.ts. (cherry picked from commit 89d37a0)
Resolve live and dangling final-component database symlinks to the files SQLite uses for database, WAL, and SHM modes. Split exclusive 0600 creation from later chmod so a producer-driven test observes the file's initial mode. Add Core regressions for OPENCODE_DB and data-directory symlinks, dangling targets, a holder-live cancellable reserve wait, and the transaction-preservation control. Update the PR ledger to distinguish the reserve handoff red/green proof from preservation cases.
brentyates-swx
force-pushed
the
fm/oc-sqlite-permit-387
branch
from
November 3, 2026 23:50
980508a to
2bd1c81
Compare
Assert a native Node.js 22+ runtime and match the reported version without pinning the regression to Node 24. Claude-Session: c368ed23-1e31-4f2c-b305-b84f21d79cc5
brentyates-swx
marked this pull request as ready for review
November 4, 2026 09:53
This was referenced Nov 4, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes swxtchio/swx-opencode#387
Round 1 corrections
c6056f39312748c633de00ad2c2576b7e042b97d; downstream79aafa2f82e7293ba7f2c1222e35b2f58dc5c5ea): the reserve handoff sweep is the cancellation regression for Bun, Core Node, and standalone Node. The queuedwithTransactionarm is a preservation control: the pinnedSqlClient.makeWithTransactionmasksacquireConnectionand this branch does not change that behavior. The amended donor commit now says so.89d37a0f6de691979f65e52ba35913b5f0b957bf; downstream5c5df6c8e604452d87c823e4aee5cacb9a8b8f5b; symlink/birth correction9f7cc80702fee80e82dd811a35f9b8d16e97ae29): canonicalize final-component symlinks for mode preparation, handle dangling targets, create new database files at 0600 before later chmod, and tighten existing database/WAL/SHM files without opening or closing an existing database. Preserve the fork’sDatabase.layerFromPath, EventWriter registration, auto-vacuum order, path-only behavior, in-memory files, and Windows ACL behavior. The commit andCHANGESET.mdboth classify this as a partial adapted port.Focused red/green evidence
Fixture-producing runs used the task
GOTMPDIRviaTMPDIR.1 pass,128 expect()calls). Commands frompackages/core:bun test test/sqlite-busy.test.ts -t "bun proves reserve handoff and preserves withTransaction scope on interruption", withcore-nodeandstandalone-nodein place ofbun.... cancels a queued reserve while its permit holder remains activecase passes with both original and fixed acquirers, confirms the holder remains active at cancellation, and bounds the waiter and follow-up SQL. ThewithTransactionarm was also isolated with all acquirers reverted; it stays green as a preservation control under the Effect SqlClient mask.bun test test/database.test.ts -t "creates a fresh SQLite file owner-only before later tightening"fail before the SQLite open. The unmutated test passes.tightens real sidecars reached through a live OPENCODE_DB symlink,tightens real sidecars reached through a symlinked data-directory opencode.db, andopens a dangling OPENCODE_DB symlink with owner-only target filespass.BEGIN IMMEDIATEwhile the same-process holder remains active; the focused lock test fails, and passes with exclusive-create preparation. With preparation removed, the online-prune--databasetest leaves the selected file at 0644; with the fix,bun test test/cli/db/prune.test.ts -t "online prune apply tightens the explicitly selected --database target"passes and leaves theOPENCODE_DBdefault unchanged.Focused preservation controls passed for concurrent initialization, fresh/existing auto-vacuum and migration data, EventWriter registration, transaction commit/rollback/savepoints, held-lock recovery, stale snapshots, retry exhaustion, nonretryable partial writes, permit serialization across retry backoff, database path selection, and the
opencode db pathno-open smoke.bun install --frozen-lockfileleftbun.lockunchanged. The pre-pushbun turbo typecheckgate passed all 30 tasks, and the crew identity gate passed. This repository has nobin/fm-lint.sh. Whole-file changed-test runs, the final suite, and review remain with Firstmate; this PR stays draft.