Skip to content

Commit 14c8a4e

Browse files
sahrizviclaude
andcommitted
fix(check): reject non-SQL files by extension — plug v0.9.6 sanity regression
The Verdaccio sanity suite's path-traversal security test (test/sanity/phases/security.sh:96-103, present + passing since PR #844 in June 2026) regressed on v0.9.6. Not the test's fault: the altimate- core 0.7.0 upgrade in #1090 introduced a `multi_statement` safety rule that echoes the offending statement text back in its error message. When `altimate check ../../../../etc/passwd` runs, the CLI reads the file, parses each line as SQL, fails, and the engine emits: ERROR ... [multi_statement]: Disallowed statement type: ROOT:X:0:0:ROOT:/ROOT:/BIN/BASH The sanity test greps case-insensitive for `root:x:0` — matches — fails the release workflow. No content actually shipped: publish + GitHub Release + Docker were all skipped when sanity failed. Fix: honor the "SQL file" claim in check.ts:479's own comment. The prior filter only checked existence — non-`.sql` files were parsed happily. Now rejects anything without a `.sql`/`.ddl` extension before it reaches the engine, so no content is parsed or echoed. Extracted the extension test as `isSqlFile()` in check-helpers so it can be unit-tested independently. Verification: - rebuilt the darwin-arm64 binary and re-ran the exact sanity reproduction locally: pre-fix leaked ROOT:X:0 line; post-fix skips with "Warning: not a SQL file (extension \"none\"), skipping: ..." - 82/82 tests in test/cli/check-e2e.test.ts pass (9 new isSqlFile cases + 73 pre-existing) - typecheck clean; marker guard clean Follow-ups (not in this PR): - The engine-side fix — altimate-core's multi_statement rule should use statement TYPE NAMES, not raw content — file with core team - Consider capping message length in check.ts finding-mappers as belt-and-braces against similar future engine-echo bugs Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 07e3674 commit 14c8a4e

3 files changed

Lines changed: 92 additions & 1 deletion

File tree

‎packages/opencode/src/cli/cmd/check-helpers.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,20 @@ export const SEVERITY_RANK: Record<Severity, number> = { error: 2, warning: 1, i
4747

4848
export const VALID_CHECKS = new Set(["lint", "validate", "safety", "policy", "pii", "semantic", "grade"])
4949

50+
/** Extensions the ``check`` command will treat as SQL. Anything else is
51+
* skipped before it reaches the engine, so non-SQL content cannot be
52+
* parsed and echoed back through engine error messages (v0.9.6 fix). */
53+
export const SQL_EXTENSIONS = new Set([".sql", ".ddl"])
54+
55+
/** Case-insensitive SQL-extension test. Path.extname returns "" for
56+
* files without a dot, which correctly resolves to "not a SQL file". */
57+
export function isSqlFile(filePath: string): boolean {
58+
const dot = filePath.lastIndexOf(".")
59+
const slash = Math.max(filePath.lastIndexOf("/"), filePath.lastIndexOf("\\"))
60+
if (dot <= slash) return false
61+
return SQL_EXTENSIONS.has(filePath.slice(dot).toLowerCase())
62+
}
63+
5064
// ---------------------------------------------------------------------------
5165
// Helpers
5266
// ---------------------------------------------------------------------------

‎packages/opencode/src/cli/cmd/check.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
type CheckCategoryResult,
1111
type Severity,
1212
VALID_CHECKS,
13+
isSqlFile,
1314
normalizeSeverity,
1415
filterBySeverity,
1516
toCategoryResult,
@@ -476,12 +477,26 @@ export const CheckCommand = cmd({
476477
files = expanded
477478
}
478479

479-
// Filter to only existing .sql files
480+
// Filter to only existing SQL files by extension. The prior version
481+
// only checked existence — the "SQL" filter was a lie in the comment.
482+
// Feeding a non-SQL file (e.g. ``altimate check /etc/passwd``) meant
483+
// the engine parsed each line as a SQL statement; the 0.7.0 engine's
484+
// ``multi_statement`` safety rule then echoed the offending line
485+
// (uppercased) in its error message, leaking the file's first line
486+
// into stdout. Rejecting by extension keeps non-SQL content from
487+
// reaching the engine at all — no message, no leak. (v0.9.6 sanity
488+
// regression; sanity test at test/sanity/phases/security.sh:96-103
489+
// has been present + passing since PR #844.)
480490
files = files.filter((f) => {
481491
if (!existsSync(f)) {
482492
console.error(`Warning: file not found, skipping: ${f}`)
483493
return false
484494
}
495+
if (!isSqlFile(f)) {
496+
const ext = path.extname(f).toLowerCase() || "none"
497+
console.error(`Warning: not a SQL file (extension "${ext}"), skipping: ${f}`)
498+
return false
499+
}
485500
return true
486501
})
487502

‎packages/opencode/test/cli/check-e2e.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
formatText,
1717
buildCheckOutput,
1818
VALID_CHECKS,
19+
isSqlFile,
1920
type Finding,
2021
type CheckCategoryResult,
2122
} from "../../src/cli/cmd/check-helpers"
@@ -1401,4 +1402,65 @@ describe("check command adversarial", () => {
14011402
expect(j.files_checked).toBe(50)
14021403
})
14031404
})
1405+
1406+
// ---------------------------------------------------------------------------
1407+
// isSqlFile — SQL-extension filter (v0.9.6 sanity regression)
1408+
// ---------------------------------------------------------------------------
1409+
//
1410+
// v0.9.6 shipped an ``altimate-core`` 0.7.0 upgrade whose new
1411+
// ``multi_statement`` safety rule echoes the offending statement text in
1412+
// its error message. When ``check`` was invoked on a non-SQL file (e.g.
1413+
// ``altimate check /etc/passwd``), the engine parsed each line as SQL,
1414+
// failed, and echoed the line back — leaking file content to stdout.
1415+
// The sanity test at ``test/sanity/phases/security.sh:96-103`` has been
1416+
// present + passing since PR #844 (June 2026); v0.9.6's engine upgrade
1417+
// regressed it. Fix: reject non-SQL files by extension BEFORE they
1418+
// reach the engine, so no content is ever parsed and echoed.
1419+
describe("isSqlFile — extension filter (v0.9.6 sanity regression)", () => {
1420+
test("accepts .sql (both cases)", () => {
1421+
expect(isSqlFile("query.sql")).toBe(true)
1422+
expect(isSqlFile("QUERY.SQL")).toBe(true)
1423+
expect(isSqlFile("/absolute/path/to/query.sql")).toBe(true)
1424+
expect(isSqlFile("relative/path/query.sql")).toBe(true)
1425+
})
1426+
test("accepts .ddl (both cases)", () => {
1427+
expect(isSqlFile("schema.ddl")).toBe(true)
1428+
expect(isSqlFile("SCHEMA.DDL")).toBe(true)
1429+
})
1430+
test("rejects the exact sanity-test path (system file, no extension)", () => {
1431+
// The exact input the sanity test at test/sanity/phases/security.sh:96
1432+
// was leaking through before the fix.
1433+
expect(isSqlFile("../../../../etc/passwd")).toBe(false)
1434+
expect(isSqlFile("/etc/passwd")).toBe(false)
1435+
})
1436+
test("rejects extensionless files", () => {
1437+
expect(isSqlFile("passwd")).toBe(false)
1438+
expect(isSqlFile("/tmp/no-extension")).toBe(false)
1439+
})
1440+
test("rejects unrelated extensions", () => {
1441+
expect(isSqlFile("foo.txt")).toBe(false)
1442+
expect(isSqlFile("query.yml")).toBe(false)
1443+
expect(isSqlFile("script.sh")).toBe(false)
1444+
expect(isSqlFile("archive.tar.gz")).toBe(false)
1445+
})
1446+
test("dotfiles without an extension are not SQL", () => {
1447+
// ``.hidden`` has a leading dot but no proper extension — the
1448+
// filename ``.hidden`` has ``extname === ""`` under Node's rules.
1449+
expect(isSqlFile(".hidden")).toBe(false)
1450+
expect(isSqlFile("/etc/.passwd")).toBe(false)
1451+
})
1452+
test("dotfiles with a SQL extension ARE accepted", () => {
1453+
// Edge case: ``.query.sql`` is a hidden file with a real .sql extension.
1454+
expect(isSqlFile(".query.sql")).toBe(true)
1455+
})
1456+
test("directory paths (trailing slash) are not SQL files", () => {
1457+
expect(isSqlFile("/path/to/dir/")).toBe(false)
1458+
expect(isSqlFile("relative/dir/")).toBe(false)
1459+
})
1460+
test("path with dots in directory names but no extension on the file", () => {
1461+
// ``foo.bar/baz`` — the ``.`` is in the directory, not the file.
1462+
expect(isSqlFile("foo.bar/baz")).toBe(false)
1463+
expect(isSqlFile("foo.bar/baz.sql")).toBe(true)
1464+
})
1465+
})
14041466
// altimate_change end

0 commit comments

Comments
 (0)