Skip to content

fix(sqlite): bound .dump output cumulatively across all tables#1880

Merged
chaliy merged 1 commit into
mainfrom
fix/issue-1869-sqlite-dump-bounded
Jun 5, 2026
Merged

fix(sqlite): bound .dump output cumulatively across all tables#1880
chaliy merged 1 commit into
mainfrom
fix/issue-1869-sqlite-dump-bounded

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented Jun 5, 2026

Summary

  • .dump dot-command previously built the full output string before the caller applied max_output_bytes, allowing memory to grow unboundedly across many tables (DeepSec [DeepSec][MEDIUM] SQLite .dump builds full output before enforcing output cap #1869)
  • Added max_output_bytes parameter to dispatch() and dump(), enforced via a bounded_append() helper that checks the cumulative budget after each schema line and each row INSERT
  • Added DotError::OutputCap variant for clean, Display-formatted error messages (no debug shapes)
  • run_statements now passes remaining = max_output_bytes - stdout.len() to dispatch() so the cap is applied against overall invocation output

Test plan

  • dump_respects_output_cap — verifies .dump returns DotError::OutputCap mid-construction when a tight budget is set across multiple tables with rows
  • dump_empty_db_under_cap — verifies a small dump still succeeds within a reasonable budget
  • All 103 existing sqlite tests pass (cargo test --features sqlite -p bashkit --lib)
  • cargo build --features sqlite -p bashkit clean build
  • No {:?} debug format strings in builtin code (error messages use Display)

Closes #1869

Copilot AI review requested due to automatic review settings June 5, 2026 01:04
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Jun 5, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit 1bba0ab Commit Preview URL

Branch Preview URL
Jun 05 2026, 01:14 AM

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses DeepSec #1869 by ensuring the SQLite .dump dot-command enforces max_output_bytes during output construction (cumulatively across all tables), preventing unbounded growth of the dump output buffer before the global output cap is applied.

Changes:

  • Passes the remaining output budget into dot-command dispatch so .dump can enforce the cap cumulatively across schema + data.
  • Adds DotError::OutputCap and a bounded_append() helper to stop .dump construction as soon as it would exceed the budget.
  • Adds regression tests validating .dump stops mid-construction under a tight cap and still succeeds for a small empty DB.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/bashkit/src/builtins/sqlite/mod.rs Computes remaining output budget and passes it into dot-command dispatch so .dump can enforce a cumulative cap.
crates/bashkit/src/builtins/sqlite/dot_commands.rs Implements bounded .dump construction via bounded_append(), adds OutputCap error variant, and adds tests for the new behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +55 to +56
#[error("sqlite output exceeds byte cap ({0} > {1})")]
OutputCap(usize, usize),
Comment on lines +288 to +290
/// Append `chunk` to `buf`, returning `Err(OutputCap)` if doing so would
/// exceed `max`. This enforces the output budget *during* construction so
/// memory never grows beyond the cap even for large multi-table dumps.
@chaliy chaliy force-pushed the fix/issue-1869-sqlite-dump-bounded branch from 42c9559 to 1694110 Compare June 5, 2026 01:09
Pass the remaining output budget into dump() via dispatch(), and enforce
it during construction with bounded_append(). Previously each per-table
SELECT was bounded by max_result_bytes, but cumulative dump output could
grow unboundedly before the caller applied push_stdout_bounded. An
attacker controlling many tables/rows could exhaust memory.

Changes:
- DotError::OutputCap variant for clean Display-formatted cap errors
- bounded_append() helper: checks cumulative budget before each push
- dispatch() gains max_output_bytes param; run_statements passes remaining
- New unit tests: dump_respects_output_cap, dump_empty_db_under_cap
- New integration test: dump_output_cap_enforced_across_multiple_tables
- THREAT[TM-DOS-091] annotation at mitigation point
- Updated specs/threat-model.md (TM-DOS-091) and specs/sqlite-builtin.md (TM-SQL-013)

Closes #1869
@chaliy chaliy force-pushed the fix/issue-1869-sqlite-dump-bounded branch from 1694110 to 1bba0ab Compare June 5, 2026 01:14
@chaliy chaliy merged commit 56067e8 into main Jun 5, 2026
35 checks passed
@chaliy chaliy deleted the fix/issue-1869-sqlite-dump-bounded branch June 5, 2026 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DeepSec][MEDIUM] SQLite .dump builds full output before enforcing output cap

2 participants