db import: Reject dot-commands in SQLite dump files - #340
Conversation
📝 WalkthroughWalkthroughSQLite imports now reject files containing SQLite dot-commands before preprocessing or execution. A regression scenario verifies the expected CLI error and confirms that the dot-command does not create a side-effect file. ChangesSQLite import validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Not ready to approve
There are a couple of correctness gaps (robust file read error handling and a missing failure assertion in the new acceptance test) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR adds a security hardening check to the SQLite implementation of wp db import by rejecting SQLite CLI “dot-commands” in dump files, and introduces a Behat scenario to validate the new behavior.
Changes:
- Detect and abort imports when the SQLite dump contains dot-commands (e.g.,
.shell). - Add an acceptance test scenario ensuring dot-commands are rejected and no side effect file is created.
File summaries
| File | Description |
|---|---|
| src/DB_Command_SQLite.php | Rejects SQLite dot-commands during import to prevent sqlite3 CLI meta-command execution. |
| features/db-import.feature | Adds Behat coverage to ensure dot-command dumps are rejected. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| $contents = (string) file_get_contents( $file ); | ||
| } | ||
|
|
||
| if ( preg_match( '/^\s*\.[a-zA-Z]+/m', $contents ) ) { | ||
| WP_CLI::error( 'SQLite dot-commands are not allowed in import files.' ); |
| When I try `wp db import malicious_sqlite.sql` | ||
| Then STDERR should contain: | ||
| """ | ||
| SQLite dot-commands are not allowed in import files. | ||
| """ | ||
| And the side_effect_sqlite.txt file should not exist |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
features/db-import.feature (1)
260-265: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert that the import stops before SQL execution.
The fixture creates
wp_cli_sqlite_metaon Line 256 before.shellon Line 257, but the scenario checks only the side-effect file. An implementation could create the table, skip.shell, and still pass these assertions. Assert a non-zero return code and querysqlite_masterto confirm thatwp_cli_sqlite_metawas not created.Suggested assertions
When I try `wp db import malicious_sqlite.sql` + Then the return code should not be 0 Then STDERR should contain: """ SQLite dot-commands are not allowed in import files. """ And the side_effect_sqlite.txt file should not exist + + When I run `wp db query "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'wp_cli_sqlite_meta';" --skip-column-names` + Then STDOUT should be empty🤖 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 `@features/db-import.feature` around lines 260 - 265, Strengthen the malicious SQLite import scenario after the import command by asserting a non-zero return code, then query sqlite_master to verify wp_cli_sqlite_meta was not created. Keep the existing STDERR and side_effect_sqlite.txt assertions, ensuring the import stops before executing any SQL.
🤖 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 `@src/DB_Command_SQLite.php`:
- Around line 457-458: Update the docblock for sqlite_import() to document that
import files containing SQLite dot-commands are rejected before preprocessing
and execution. Keep the existing generic SQL import documentation intact and
describe this user-facing restriction alongside it.
- Around line 457-460: Update the dot-command validation in the import handling
around the preg_match check to follow SQLite CLI rules: recognize commands only
when the dot starts at the left margin, on its own statement line, and outside
SQL comments or ordinary SQL statements. Replace the current broad regex with a
stateful scanner, or explicitly document and preserve the intentionally stricter
import policy if that behavior is required.
---
Nitpick comments:
In `@features/db-import.feature`:
- Around line 260-265: Strengthen the malicious SQLite import scenario after the
import command by asserting a non-zero return code, then query sqlite_master to
verify wp_cli_sqlite_meta was not created. Keep the existing STDERR and
side_effect_sqlite.txt assertions, ensuring the import stops before executing
any SQL.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7562bb9-b0eb-4a40-a15a-c90d78bc51f6
📒 Files selected for processing (2)
features/db-import.featuresrc/DB_Command_SQLite.php
| if ( preg_match( '/^\s*\.[a-zA-Z]+/m', $contents ) ) { | ||
| WP_CLI::error( 'SQLite dot-commands are not allowed in import files.' ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the new import restriction.
sqlite_import() now rejects SQLite dot-commands, but its docblock at Line 423 only describes generic SQL import behavior. Update it to document the rejection before preprocessing and execution.
As per coding guidelines, update relevant inline code documentation when changes affect user-facing functionality.
Suggested docblock update
/**
* Import SQL into SQLite database.
+ *
+ * Rejects SQLite dot-commands before preprocessing and execution.🤖 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 `@src/DB_Command_SQLite.php` around lines 457 - 458, Update the docblock for
sqlite_import() to document that import files containing SQLite dot-commands are
rejected before preprocessing and execution. Keep the existing generic SQL
import documentation intact and describe this user-facing restriction alongside
it.
Source: Coding guidelines
| if ( preg_match( '/^\s*\.[a-zA-Z]+/m', $contents ) ) { | ||
| WP_CLI::error( 'SQLite dot-commands are not allowed in import files.' ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
command -v sqlite3 >/dev/null
command -v python3 >/dev/null
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
side_effect="$tmpdir/side_effect"
cat >"$tmpdir/dump.sql" <<EOF
/*
.shell touch "$side_effect"
*/
CREATE TABLE test_table (id INTEGER);
EOF
sqlite3 "$tmpdir/test.db" <"$tmpdir/dump.sql"
test ! -e "$side_effect"
python3 - "$tmpdir/dump.sql" <<'PY'
import re
import sys
contents = open(sys.argv[1], encoding="utf-8").read()
assert re.search(r'^\s*\.[a-zA-Z]+', contents, re.MULTILINE)
PYRepository: wp-cli/db-command
Length of output: 155
🏁 Script executed:
#!/bin/sh
set -eu
echo "== repo files =="
git ls-files | sed -n '1,80p'
echo "== target file outline/size =="
wc -l src/DB_Command_SQLite.php
ast-grep outline src/DB_Command_SQLite.php --view expanded 2>/dev/null | sed -n '1,220p' || true
echo "== relevant target lines =="
cat -n src/DB_Command_SQLite.php | sed -n '400,490p'
echo "== imports/import methods references =="
rg -n "SQLite dot-commands|import files|\.import|import|DB_Command_SQLite" src tests -SRepository: wp-cli/db-command
Length of output: 8372
🌐 Web query:
SQLite command-line shell dot commands SQL comments dot command at left margin documentation
💡 Result:
In the SQLite command-line interface (CLI), dot-commands—also known as meta-commands—are special instructions interpreted directly by the sqlite3 program rather than by the SQLite library [1][2]. The official rules governing dot-commands are as follows: 1. Left Margin Requirement: A dot-command must begin with a "." at the absolute left margin of the input line, with no preceding whitespace [1][3]. 2. Single-Line Constraint: Dot-commands must be entirely contained on a single input line [1][4]. 3. SQL Interaction: Dot-commands cannot be used in the middle of an ordinary SQL statement and cannot be executed at a continuation prompt [1][4]. 4. Comment Syntax: There is no comment syntax available for dot-commands; they do not recognize or support comments [1][5]. While dot-commands themselves do not support comments, the SQLite CLI does support whole-line comments for SQL scripts [4]. These comments must begin with a '#' character at the left margin, with no preceding whitespace, and extend to the end of the line [4]. For a complete list of available dot-commands in your current version, you can execute the.help command within the sqlite3 shell [1][6].
Citations:
- 1: https://sqlite.org/cli.html
- 2: https://github.com/sqlite/sqlite/blob/master/sqlite3.1
- 3: https://sqlite.org/draft/cli.html
- 4: https://www.sqlite.org/draft/cli.html
- 5: https://www.w3resource.com/sqlite/sqlite-dot-commands.php
- 6: https://sqlite.org/docsrc/artifact/5d0b18c208bdcd7f
🏁 Script executed:
#!/bin/sh
set -eu
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
bin="$(command -v sqlite3 || echo '"sqlite3 unavailable"')"
echo "sqlite3=$bin"
if [ "$bin" = 'sqlite3 unavailable' ]; then
exit 1
fi
sqlite3 "$tmpdir/out.db" <<'SQL' | sed -n '1,80p'
-- test CLI behavior against representative inputs used by the raw regex
.create abc
select ".x" as q;
CREATE TABLE test_table (id INTEGER);
/*
.shell touch "$tmpdir/sideeffect"
*/
INSERT INTO test_table VALUES (1);
SQL
test -e "$tmpdir/out.db" || exit 2
echo "== raw regex matches =="
python3 - <<'PY'
import re, sys
patterns = [
('leading-dot', re.compile(r'^\s*\.[a-zA-Z]+', re.MULTILINE)),
('dot-at-left', re.compile(r'^\.[a-zA-Z]+', re.MULTILINE)),
]
for name, pat in patterns:
hits = []
with open(sys.argv[1]) as f:
for i,line in enumerate(f,1):
if pat.search(line):
hits.append((i,line.rstrip()))
print(name, hits)
PY "$tmpdir/dump.sql" 2>/dev/null || true
echo "== CLI behavior with exact regex-like leading whitespace dot command =="
cat >"$tmpdir/exact.sql" <<EOF
.shell touch "$tmpdir/regexmatched"
CREATE TABLE d (id INTEGER);
EOF
rm -f "$tmpdir/regexmatched"
sqlite3 "$tmpdir/exact.db" <"$tmpdir/exact.sql" | sed -n '1,40p' || true
if [ -e "$tmpdir/regexmatched" ]; then
echo "regexmatched exists"
else
echo "regexmatched does not exist"
fi
cat >"$tmpdir/exact2.sql" <<EOF
# comment before
CREATE TABLE e (id INTEGER);
EOF
sqlite3 "$tmpdir/exact2.db" <"$tmpdir/exact2.sql" || trueRepository: wp-cli/db-command
Length of output: 227
🌐 Web query:
site:sqlite.org/cli.html dot-commands left margin SQLite docs
💡 Result:
In SQLite, dot-commands (special commands used in the command-line shell) must begin with a "." character at the left margin of the input line [1][2]. This means there can be no preceding whitespace before the dot [1][2]. According to the official SQLite documentation, the following rules apply to dot-command syntax [1][2]: - The command must begin with a "." at the left margin with no preceding whitespace [1][2]. - It must be entirely contained on a single input line [1][2]. - Dot-commands cannot be used in the middle of an ordinary SQL statement or at a continuation prompt [1][2]. - There is no comment syntax available for dot-commands [1][2]. Similarly, whole-line comments that begin with a "#" character also require that there be no whitespace prior to the initial "#" [1][2].
Citations:
Align dot-command detection with SQLite CLI rules.
The current ^\s*\.[a-zA-Z]+ pattern rejects tokens after leading whitespace and inside SQL comments. SQLite requires dot-commands to start at the left margin, be single-line, and not appear in the middle of an ordinary SQL statement. Use a stateful scanner matching that state, or document this import policy as intentionally stricter than SQLite CLI behavior.
🤖 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 `@src/DB_Command_SQLite.php` around lines 457 - 460, Update the dot-command
validation in the import handling around the preg_match check to follow SQLite
CLI rules: recognize commands only when the dot starts at the left margin, on
its own statement line, and outside SQL comments or ordinary SQL statements.
Replace the current broad regex with a stateful scanner, or explicitly document
and preserve the intentionally stricter import policy if that behavior is
required.
Source: MCP tools
Applying some slight hardening
Summary by CodeRabbit