Add complete local SQLite snapshots - #1884
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a ChangesDatabase snapshot workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant SnapshotScript
participant createDatabaseSnapshot
participant RemoteReplica
participant SQLiteSnapshot
Developer->>SnapshotScript: run snapshot with output path
SnapshotScript->>createDatabaseSnapshot: pass validated request
createDatabaseSnapshot->>RemoteReplica: synchronize temporary replica
RemoteReplica-->>createDatabaseSnapshot: return synchronized database
createDatabaseSnapshot->>SQLiteSnapshot: checkpoint and integrity-check database
SQLiteSnapshot-->>createDatabaseSnapshot: return verification and WAL results
createDatabaseSnapshot-->>Developer: publish snapshot and report output path
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9badf5f8b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/database-snapshot-lib.ts`:
- Around line 104-109: Update requireRemoteDatabaseUrl to allow http only for
loopback hostnames (localhost, 127.0.0.1, and ::1), while requiring https for
all other remote hosts. Preserve acceptance of existing supported secure
protocols and reject non-loopback plain-HTTP URLs before returning the value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 54620bfd-631e-40de-9ba0-2f44773c186e
📒 Files selected for processing (7)
AGENTS.mdREADME.mddeno.jsonscripts/database-snapshot-lib.tsscripts/database-snapshot.tstest/scripts/database-snapshot-options.test.tstest/scripts/database-snapshot.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/database-snapshot-lib.ts (1)
251-257: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSensitive Data Exposure (CWE-732): Incorrect Permission Assignment for Critical Resource
Reachability: External
● Entry scripts/database-snapshot.ts:15 createDatabaseSnapshot │ ▼ ● Sink scripts/database-snapshot-lib.tsSet the temp database to 0600 before
Deno.linkThe hard link preserves the temporary inode’s mode, so a default umask can publish the snapshot as group/world-readable on a shared host. Add a regression test that asserts the published file is owner-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/database-snapshot-lib.ts` around lines 251 - 257, Update publishSnapshot to set the temporary database file’s permissions to owner-only (0600) before calling Deno.link, ensuring the published hard link inherits the restricted mode. Add a regression test covering publishSnapshot that verifies the resulting output file is not group- or world-readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@scripts/database-snapshot-lib.ts`:
- Around line 251-257: Update publishSnapshot to set the temporary database
file’s permissions to owner-only (0600) before calling Deno.link, ensuring the
published hard link inherits the restricted mode. Add a regression test covering
publishSnapshot that verifies the resulting output file is not group- or
world-readable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b699d070-7903-4d3f-a57b-7fe4090066e0
⛔ Files ignored due to path filters (1)
deno.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
AGENTS.mdREADME.mddeno.jsonscripts/database-snapshot-lib.tsscripts/database-snapshot.tstest/scripts/database-snapshot-options.test.tstest/scripts/database-snapshot.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e1f0a7eaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Resolved CodeRabbit’s owner-only file finding in 3b8f226. The verified temporary database is now changed to |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/scripts/database-snapshot-safety.test.ts`:
- Around line 63-67: Extract the shared directory-listing helper in
test-utils/files.ts, using Array.fromAsync over Deno.readDir followed by mapping
entry names and sorting them. In test/scripts/database-snapshot-safety.test.ts,
replace the local directoryNames implementation with the shared helper; in
test/scripts/database-snapshot.test.ts, replace the inline directory-entry
listing block with that same helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9736f825-bf41-4d7e-8ce6-1d30685cacd3
📒 Files selected for processing (3)
scripts/database-snapshot-lib.tstest/scripts/database-snapshot-safety.test.tstest/scripts/database-snapshot.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c4036cd0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
deno task snapshot --out <path.sqlite>for developer machinesDB_URLandDB_TOKENfrom.env, preferring them over existing shell valuesSafety
Remote connections must use TLS. Plain HTTP and
libsql:URLs with a finaltls=0value are accepted only for exact loopback hosts. The task builds its replica in a temporary directory beside the requested output. It closes the syncing client, reopens the file locally, requires a fully truncated WAL and a clean integrity check, sets the database to owner-only0600, then publishes through an atomic no-replace hard link. It checks the destination main file and its WAL/SHM sidecars before syncing and again before publication. The temporary replica and every sidecar are removed on success or failure. Existing files and files created during sync are never overwritten or deleted.Interactive terminals refresh the active stage with elapsed time once per second. Redirected output stays as stable one-line events. The progress timer is stopped on both success and failure.
Verification
nix develop -c deno task precommit.env; it published a complete 110 MB SQLite file and passed its built-in integrity check