fix(check): reject non-SQL files by extension — unblock v0.9.6 release

Merged
SA
2mo ago
+293-34

Summary

Fixes the v0.9.6 release-workflow sanity failure. The Sanity (Verdaccio) job's Phase 5 security test failed → Publish to npm and Create GitHub Release were skipped. Nothing shipped; the release is in limbo. This PR unblocks it.

What actually happened — the test existed all along, altimate-core 0.7.0 regressed it

The failing test is at test/sanity/phases/security.sh:96-103, introduced in #844 (June 2026), and has been present + passing on every release since — including v0.9.5 on Aug 10. It runs:

altimate check "../../../../etc/passwd"

and greps case-insensitive for root:x:0 in the output. Historically that output was Warning: file not found, skipping: /... (on macOS) or No SQL files found (on Linux) — no leak.

v0.9.6 shipped the altimate-core 0.5.1 → 0.7.0 upgrade in #1090. One of its new safety rules — multi_statement — echoes the offending statement text back inside its error message. When the CLI reads /etc/passwd on Linux CI, parses each line as SQL, and fails, the engine emits:

ERROR ... [multi_statement]: Disallowed statement type: ROOT:X:0:0:ROOT:/ROOT:/BIN/BASH
    suggestion: Statement type 'ROOT:X:0:0:ROOT:/ROOT:/BIN/BASH' is not in the allowed list: ["SELECT", "WITH"]

The sanity grep matches → test fails → publish skipped. Not the test's fault; the engine change regressed it. No customer would have seen this in practice (they're unlikely to altimate check /etc/passwd), but the sanity net catches the class of "CLI leaks file content to stdout" cleanly and it's right to fail-closed here.

The fix

The prior file filter at packages/opencode/src/cli/cmd/check.ts had a comment that said "Filter to only existing .sql files" — but the code only checked existence, not extension. That was the actual bug: altimate check /etc/passwd was accepted, parsed, and echoed. This PR makes the filter honor its own comment:

files = files.filter((f) => {
  if (!existsSync(f)) { console.error(`Warning: file not found, skipping: ${f}`); return false }
  if (!isSqlFile(f)) {
    const ext = path.extname(f).toLowerCase() || "none"
    console.error(`Warning: not a SQL file (extension "${ext}"), skipping: ${f}`)
    return false
  }
  return true
})

isSqlFile() accepts .sql / .ddl (case-insensitive) — extracted to check-helpers.ts so it's unit-testable independently.

Non-SQL content no longer reaches the engine at all — no parse, no error message, no leak. The sanity test now passes because the CLI output for altimate check ../../../../etc/passwd becomes:

Warning: not a SQL file (extension "none"), skipping: /.../etc/passwd
No SQL files found to check.

Verification

  • Rebuilt the darwin-arm64 binary with the fix and re-ran the exact sanity reproduction locally — pre-fix output leaked ROOT:X:0:..., post-fix skips cleanly with the warning above.
  • 82/82 tests in test/cli/check-e2e.test.ts pass — 9 new isSqlFile cases (accept .sql/.ddl in both cases; reject the exact sanity-test path, extensionless files, unrelated extensions, dotfiles, directory paths, dot-in-dir paths) + 73 pre-existing.
  • bun turbo typecheck clean.
  • Marker Guard (strict, vs origin/main) clean.

Follow-ups (not in this PR)

  • Engine-side fix: altimate-core's multi_statement rule should surface statement TYPE NAMES (e.g. SELECT, INSERT), not the raw offending content. Will file with the core team.
  • Belt-and-braces: cap message length across all check-finding mappers so a future engine change that echoes user content can't leak this way again.

Release plan (after this merges)

  1. Delete broken v0.9.6 tag on remote (safe — nothing was published)
  2. Tag v0.9.6-beta.1 at the new main HEAD → release.yml re-runs, sanity now passes, publishes to npm beta dist-tag (existing latest users unaffected)
  3. Once beta.1 is clean, tag v0.9.6 at the same SHA → publishes to latest

Test plan

  • Local rebuild + sanity-repro passes
  • 82/82 check-e2e.test.ts (9 new) pass
  • typecheck clean
  • Marker Guard clean
  • CI on this PR
  • Once merged: release.yml on v0.9.6-beta.1 — full sanity path
  • release.yml on v0.9.6 — final promote to latest

🤖 Generated with Claude Code

Summary by cubic

Blocks non-SQL, non-regular, and all symlinked files in altimate check to prevent parsing arbitrary content and leaking it via engine errors. Previously any existing path was parsed; now only regular .sql/.ddl files are processed, and symlinks are rejected to close a TOCTOU race.

  • Accepts only case-insensitive .sql/.ddl; rejects bare .sql/.ddl dotfiles, directories or symlinks, and any non-SQL paths. Non-SQL paths warn and are skipped; if none remain, prints “No SQL files found to check.” Default discovery now scans **/*.{sql,ddl}.
  • Adds isSqlFile() and SQL_EXTENSIONS in packages/opencode/src/cli/cmd/check-helpers.ts. In packages/opencode/src/cli/cmd/check.ts, adds statSync(...).isFile() and an lstatSync symlink gate that blanket-rejects links (TOCTOU-safe), dual-glob discovery, and clearer warnings.
  • Tests add handler-level adversarial cases (mixed inputs, dotfiles, .sql directories, symlinks now rejected) and isSqlFile coverage; fixes spy restoration and removes test-only telemetry env mutations. All tests pass. No API changes beyond stricter file acceptance.
  • Migration required: rename extensionless or non-.sql/.ddl SQL files, and replace symlinked inputs with their resolved file paths.
  • Release: delete the broken v0.9.6 tag, tag v0.9.6-beta.1 to verify, then tag v0.9.6 to publish to latest.

Summary by CodeRabbit

  • New Features

    • The check command recognizes .sql and .ddl files regardless of capitalization.
    • SQL files with names such as .query.sql are supported.

  • Bug Fixes

    • Bare .sql and .ddl dotfiles, non-SQL inputs, SQL-named directories, symbolic links, and other non-file paths are rejected before parsing.
    • Broken links are handled safely, with diagnostic paths reported without exposing file contents.
    • Valid SQL files remain discoverable and checkable when referenced through their direct paths.