COR-1861: Port SQL language support from the 0.1.2 fork onto main

[juangaitanv]

Linear: https://linear.app/corgea/issue/COR-1861/add-sql-language-support-to-sighthound

Read this first

SQL support already ships in production. main never had it.

The binary Fusion vendors and runs today is sighthound 0.1.2, built from commit 6dfc357.
6dfc357 is not an ancestor of main (git merge-base --is-ancestor 6dfc357 main → non-zero,
measured). main is 0.1.1. rules/sql never appeared anywhere in main's reachable history and
was never deleted from it — this is unported work, not a regression.

So this PR is a port, not new development. It takes the fork's SQL implementation and lands it
on main, with two deliberate false-positive downgrades and one engine fix that the port exposed.

---

1. The fork divergence, both directions

6dfc357 shares main's root commit but forked at 77e388e (2025-09-04). main has taken 135
commits since; the fork took seven. Both trees carry rule packs the other lacks
(measured with git ls-tree --name-only <ref> rules/):

	Rule packs
Only on the fork (6dfc357)	rules/sql, rules/xml, rules/properties, rules/config
Only on main	rules/csharp, rules/go, rules/java, rules/objectscript, rules/php, rules/ruby

SIGHTHOUND_SUPPORTED_LANGUAGES at fusion/fusion/config.py:363 lists exactly the fork's set —
javascript, typescript, tsx, html, xml, sql, properties, config. The Fusion gate is
built around the fork, not around main.

2. Do NOT bump main to 0.1.2

fusion/Dockerfile:40 hard-fails the image build unless the binary reports exactly 0.1.2:

if [ "$SIGHTHOUND_VERSION" != "0.1.2" ]; then \
    echo "Error: Expected sighthound version 0.1.2, but got $SIGHTHOUND_VERSION"; \
    exit 1; \
fi

Cargo.toml:3 stays at version = "0.1.1", deliberately. That pin is the only thing preventing a
re-vendor from main from silently dropping the fork-only languages, so it is left failing
loudly. Before this PR, four gated languages were at risk of vanishing on a re-vendor (xml,
sql, properties, config); after it, three.

No fusion/ changes are in this PR. The gate already contains "sql" — nothing there needs
changing. Merging this PR therefore cannot reach production on its own; that is intended.

---

3. The blocker: combined mode hard-errored on a search-only rule pack

This is the most important thing to check in this PR.

Fusion invokes the binary once per file, at fusion/fusion/agents/sighthound_scanner.py:71:

command = ["sighthound", "--output-format", "json", project_file.path]

No language argument, no --simple-analysis — default combined mode. run_selected_analysis
(src/main.rs:104-131) runs run_simple_analysis first, then appends the result of
run_taint_analysis_with_verbosity(…, verbose_mode = false).

All 11 SQL rules are mode: "search". run_taint_analysis_with_verbosity (src/scanner/modes.rs)
hard-errored whenever taint_rules_count == 0. So the first port build returned:

Error: No taint flow rules found. Please ensure your rules contain rules with mode='taint'.

on the one invocation that matters, where production 0.1.2 returned valid JSON. Two of the
ticket's acceptance criteria failed ("SQL files are included in the normal scan path" and
"produces valid findings in the expected output format").

The fix

if taint_rules_count == 0 {
    // Search-only rule packs (SQL, ObjectScript) are legitimate. In a combined scan the
    // simple pass has already produced the findings, so contribute nothing rather than
    // failing the whole scan. Only an explicit --taint-analysis request still errors,
    // because there the caller asked for taint flows specifically.
    if !verbose_mode {
        return Ok(Vec::new());
    }
    return Err(anyhow::anyhow!(
        "No taint flow rules found. Please ensure your rules contain rules with mode='taint'."
    ));
}

verbose_mode already distinguished the two callers: true = explicit --taint-analysis
(modes.rs:485), false = the combined second pass.

It must be Ok(Vec::new()) and not a search-pass re-run. run_selected_analysis appends this
result to the simple pass, so re-running search there would double-count every finding.

Measured after the fix

Fusion's exact invocation against tests/test_files/strictness_languages/sql/-style SQL, measured
this session against the vendored production binary (fusion/vendor/sighthound/osx/sighthound):

$ ./target/release/sighthound --output-format json tautology.sql   # port, exit 0
[{"finding_type":"Authentication Bypass","severity":"Critical","confidence":"High","cwe_id":"cwe-89","line":1},
 {"finding_type":"Performance Issue","severity":"Low","confidence":"High","cwe_id":"cwe-1240","line":1}]

$ fusion/vendor/sighthound/osx/sighthound --output-format json tautology.sql   # 0.1.2, exit 0
[{"finding_type":"Authentication Bypass","severity":"Critical","confidence":"High","cwe_id":"cwe-89","line":1},
 {"finding_type":"Performance Issue","severity":"Low","confidence":"High","cwe_id":"cwe-1240","line":1}]

2 findings, byte-identical to production, exit 0. And the other half still fails honestly:

$ ./target/release/sighthound --output-format json --taint-analysis case.sql
Error: No taint flow rules found. Please ensure your rules contain rules with mode='taint'.
exit 1

Blast radius and side effect

Blast radius is exactly "rule sets with zero taint rules", which previously always hard-errored.
The change can only turn an error into a result; it cannot change any finding.

Side effect (measured): an ObjectScript-only scan now exits 0 instead of erroring.
rules/objectscript/ and rules/html/ are also search-only, so they tripped the same guard. Same
root cause, same class of fix.

Regression coverage

Two tests in tests/strictness/language_coverage.rs pin both halves:

• sql_default_combined_mode_succeeds_without_taint_rules — asserts combined mode produces
  findings and that its finding set equals --simple-analysis's, so a future "fix" that
  re-runs the search pass and doubles the findings fails here.
• sql_explicit_taint_analysis_still_errors_without_taint_rules — asserts non-zero exit and the
  No taint flow rules found message.

Validated by reverting the fix: with the if !verbose_mode { return Ok(Vec::new()); } block
removed, the first test fails with the original No taint flow rules found error.

The two no-op lines in the same hunk

The diff also replaces show_progress && verbose_mode with report in two places. report is a
pre-existing binding on main (let report = show_progress && verbose_mode;, already passed to
ScanContext::new), so these are a pure rename with zero behavior change. They exist because
reusing the binding brought run_taint_analysis_with_verbosity back to CCN 13
(measured: run_taint_analysis_with_verbosity@524-605@src/scanner/modes.rs, CCN 13).

---

4. The two deliberate rule divergences from production (D2)

rules/sql/sql_security.ron differs from git show 6dfc357:rules/sql/sql_security.ron by
exactly three hunks — verified with
git diff --no-index <(git show 6dfc357:rules/sql/sql_security.ron) rules/sql/sql_security.ron.
Everything else in the pack is a byte-verbatim port.

These two are the only intended findings-behavior differences between this PR and the 0.1.2
binary. Both are the same shape: keep the detection, stop it overstating severity.

D2a — sql-injection-001 fired Critical on a bare "||"

Production reports SELECT first_name || ' ' || last_name FROM employees; — ordinary column
concatenation — as a Critical SQL Injection, at Medium confidence.

             patterns: Some([
                 "CONCAT(*",
-                "||",
                 "SELECT * WHERE",
                 "SELECT * FROM * WHERE",
             ]),

"||" moves to a new rule at Low severity / Low confidence:

        (
            id: Some("sql-injection-002"),
            name: Some("String Concatenation Operator"),
            category: Some("sql-injection"),
            mode: "search",
            patterns: Some([
                "||",
            ]),
            finding_type: Some("SQL Injection"),
            severity: Some("Low"),
            confidence: Some("Low"),
            cwe_id: Some("cwe-89"),
            description: Some("The || operator may be building dynamic SQL, but substring matching cannot distinguish that from ordinary column concatenation"),
            file_types: Some((extensions: Some([".sql"]))),
            tags: Some(["sql-injection", "database", "cwe-89"])
        ),

sql-injection-001 keeps its three glob patterns at Critical / Medium. The finding is downgraded,
not deleted.

Rejected: deleting "||" outright — the remaining patterns are globs anchored at byte 0, which
would leave the rule nearly inert. Also rejected: narrowing to "' ||" — verified not to work, the
benign case contains that substring too.

D2b — sql-comment-001 reported ordinary comments as SQL Injection

Measured against the production binary this session — a file containing
-- inline comment test / CREATE TABLE t (id INT); -- trailing comment / /* block */:

0.1.2 : SQL Injection  / Medium / Medium @ line 2
port  : Code Smell     / Low    / Low    @ line 1

An ordinary trailing -- comment reported as SQL Injection. Nearly every real .sql file carries a
comment, so this fires at far higher volume than D2a — mislabelled as injection. (The line moves
from 2 to 1 because of Δ1 in §6, not because of this change.)

-            finding_type: Some("SQL Injection"),
-            severity: Some("Medium"),
+            finding_type: Some("Code Smell"),
+            severity: Some("Low"),
             confidence: Some("Low"),

Patterns (["--", "/*", "#"]) unchanged; confidence was already Low in the fork.

Note on the unless clause (-- Note:, -- TODO, …): it is dead code in both trees.
main's UnifiedRule (src/models.rs:153-236) has no unless field and serde ignores it, and
6dfc357:src/ never references one. It disarms nothing. It is kept as ported —
rules/javascript/frontend_security.ron:20 carries the same inert field, so removing it here would
be an undeclared divergence.

Rejected: deleting the rule outright — customers filtering on it would see findings vanish rather
than drop.

---

5. Honest limitations, now documented in README.md

The ticket asks for documented dialect limitations. The new SQL row (README.md:42) and the
limitations section under the language table state them plainly, because they are severe:

• No SQL grammar, no dataflow, no taint. SQL is matched textually. Rules match against the
  whole file, not against parsed statements. The tree-sitter JavaScript grammar is wired in as a
  no-op host so call_node_types() names the parse-tree root, yielding one node per file whose
  "function name" is the entire file text.
• Pattern semantics. Patterns without * match as substrings anywhere in the file. Patterns
  containing * are globs evaluated against the whole file, so they are anchored at byte 0 —
  EXEC(@sql); on line 1 is reported; the identical statement on line 2 is not.
• end_line is always EOF. Start lines are accurate.
• Tautology-oriented rules. Measured against the production binary — zero findings on all
  three: dynamic SQL inside a stored-procedure body, CREATE USER … IDENTIFIED BY '…', and
  GRANT ALL … TO PUBLIC are not detected.
• Dialect-specific syntax (T-SQL, PL/SQL, PL/pgSQL) is not understood.
• .sql / .ddl / .dml: all three are detected as SQL (src/scanner/utils.rs:266) and
  discovered in explicit mode (src/scanner/vulnerability_scanner.rs:109), but only .sql
  produces findings. Every rule in the pack carries
  file_types: Some((extensions: Some([".sql"]))), and rule_applies_to_file
  (src/scanner/utils.rs:145-192) rejects the others. Measured under the production binary:
  identical EXEC(@sql); in a.ddl, b.sql, c.dml → only b.sql fired. The gate arms are kept
  as a faithful port (the fork has the should_include_file arm at 6dfc357:src/scanner/core.rs:1324),
  the limitation is documented, and sql_explicit_mode_scans_ddl_and_dml_but_only_sql_reports
  pins the zero result so a future reader does not "fix" the gate.
• SKIP_DIRS excludes files beneath it, matched by whole-path-component equality. Measured: the
  constant at src/config.rs:27 has 18 entries; the ones a SQL user will hit are tests, test,
  vendor, build, dist, target, node_modules. --include-test-fixtures reopens tests/
  and test/ only.

Expect both false positives (a benign || is reported, now at Low) and false negatives (dynamic
SQL that is not the first thing in the file is missed).

---

6. Measured engine-drift delta vs the production 0.1.2 binary (D3)

main's engine is 135 commits ahead of the fork (scanning_logic.rs/modes.rs split, AST
provenance), so ported rules can produce different findings than production. Same 17-file probe
corpus, --output-format json, both columns. Recorded for review, not chased — per the locked
decision, a delta is information, not a failed port.

Fixture	Production 0.1.2	main (verbatim port)	Δ
proc.sql	0	0	—
execfirst.sql	SQL Injection / High / Medium @L1	same	—
execlater.sql	0	0	—
concat.sql	Performance Issue / Low / High @L1	same	—
tautology.sql	Authentication Bypass / Critical / High @L1; Performance Issue / Low / High @L1	same	—
pipe.sql	SQL Injection / Critical / Medium @L1; Performance Issue / Low / High @L1	same	—
clean.sql	Performance Issue / Low / High @L1	same	—
cmt.sql	0	SQL Injection / Medium / Low @L1	Δ1
cmt2.sql	SQL Injection / Medium / Medium @L1	SQL Injection / Medium / Low @L1	Δ2
drop.sql	Dangerous Operation / High / High @L1	same	—
trunc.sql	Dangerous Operation / High / High @L1	same	—
union.sql	Performance Issue / Low / High @L1; SQL Injection / High / Medium @L1	same	—
sleep.sql	Performance Issue / Low / High @L1; SQL Injection / High / Medium @L1	same	—
immediate.sql	SQL Injection / High / Medium @L1	same	—
loc.sql	SQL Injection @L6; Dangerous Operation @L7	same	—
probe.ddl	0	0	—
probe.dml	0	0	—
total	17	18	+1

17 of 17 fixtures reproduce production. The two deltas:

• Δ1 — a leading -- comment fires sql-comment-001 under main's engine; it did not under the
  fork's. Production returns 0 for cmt.sql (-- a plain comment + SELECT id FROM t;); main
  returns one finding. unless is inert in both trees, so it is not the cause. This makes D2b more
  valuable than its own rationale assumed: main mislabels more ordinary SQL than production did.
• Δ2 — main honors the rule's declared confidence; the fork's engine did not.
  sql-comment-001 declares confidence: Some("Low"). Production reports Medium; main reports
  Low. main is the more faithful reader of the rule data.

After D2 is applied on top, three lines move and the total stays at 18 — no finding is lost:
pipe.sql Critical/Medium → Low/Low (D2a); cmt.sql and cmt2.sql SQL Injection/Medium → Code
Smell/Low (D2b). concat.sql is unchanged, which proves removing "||" did not disturb CONCAT(*.

Existing behavior is unchanged

Full tests/test_files corpus, non-.sql findings only: 471 before, 471 after, empty diff.
Re-verified after the D2 edits.

Caveat worth recording: finding order is nondeterministic. Two consecutive scans of the same
unchanged corpus on clean main return the same 471 findings in a different array order — the scan
path is rayon-parallel, and jq -S sorts keys, not array elements. Any before/after comparison
must sort (sort_by(.file, .line, .finding_type, .snippet)) to be meaningful. Pre-existing and
unrelated to SQL, but it will bite the next person who diffs two runs.

---

7. Verification

Gate	Result
make ci	exit 0, 263 tests passed. Clippy (strict), format check, dep audit, acceptance, coverage, arch all green
make acceptance	green, 9 scenarios (was 8) — the new one is SQL rules catch the unsafe fixture
make complexity	red with exactly one warning: build_sarif_log@189-305@src/scanner/output.rs, length 117 (threshold > 100). This is the inherited clean-main baseline at 529cbd7, not caused here, and deliberately not fixed. No second warning appeared
make agents-md-drift	red on clean main too (diverges at line 43) — inherited. No CLAUDE.md or AGENTS.md edit in this PR. It happens to pass inside this worktree only because the git-ignored CLAUDE.md mirror is absent here; that is a worktree artifact, not a fix
make check	red on clean main too — cmd_check (harness.rs:1023-1058) includes the drift gate in its results array, so make check cannot be green until make sync-agents-md runs. make ci never runs the drift gate, so make ci is the gate held green
cargo build --release --no-default-features --features sql	compiles — this is what sql = ["tree-sitter-javascript"] buys over the fork's sql = []. Its warnings match the --features python and --features html controls exactly, so they are the inherited single-feature-build pattern
Output formats	All four (text, json, csv, sarif) verified on the 18-finding probe corpus. The four finding types SQL introduces with no counterpart in main's corpus — Dangerous Operation, Performance Issue, Authentication Bypass, Code Smell — plus Low severity and cwe-1240 all round-trip. CSV parsed with a real CSV reader (snippets contain newlines, so wc -l overcounts)

Two things worth flagging so a reviewer is not surprised — they are different sets of four:

1. make ci reports CRAP: 4 function(s) exceed 30 (advisory, non-blocking):
   sink_patterns_for_finding@671-720@src/scanner/scanning_logic.rs (70.4),
   is_js_bundler_config@40-47@src/scanner/vulnerability_scanner.rs (42.0),
   extract_parameter_name@1602-1630@src/scanner/scanning_logic.rs (42.0),
   fmt@380-405@src/scanner/prefilter.rs (33.7). None is touched by this PR.
2. make ci runs clippy without --all-targets. A separate cargo clippy --all-targets -- -D warnings
   run shows 4 pre-existing lints, all in files this PR does not touch (measured):
   tests/unit/django_xss_prevention_tests.rs:113 (length comparison to one),
   tests/unit/django_xss_prevention_tests.rs:128 (unneeded return),
   harness.rs:1515 (unnecessary clone), src/scanner/output.rs:318 (items after a test module).

SARIF note, so a reviewer does not read it as a bug: rule_id_for (src/scanner/output.rs:193)
keys SARIF rule objects on the lowercased cwe_id, and ten of the eleven SQL rules are cwe-89
(only sql-wildcard-001 is cwe-1240). SARIF therefore emits two rule objects, not eleven,
whose properties.security-severity is the group maximum. Inherited production behavior. Severity
assertions in the new tests are made at the result level accordingly — a test asserting "a new
rule object appears for sql-injection-002" would fail on a correct implementation.

8. Zero complexity headroom — flag for future work

load_embedded_rules (src/rules.rs) is now at CCN 15 against a make complexity ceiling of
15 (measured: uvx lizard@1.22.2 -l rust src/rules.rs → load_embedded_rules@136-175@src/rules.rs,
CCN 15). It was 14 before; the new "sql" => match arm's ? costs the +1. It reads at exactly the
ceiling, which is why it produces no warning — the gate fires on > 15, so the separate lizard
run is the only way to see it.

Language #12 cannot be added without refactoring load_embedded_rules's match arms into a table
first. That is what blocks porting the fork's other three packs.

---

9. Open question for the requester

The requester (Ibrahim, via Slack) asked for SQL support that already ships in production. We do
not know which trigger applies, and the two readings need different fixes:

• (a) They looked at main or the OSS repo and saw no rules/sql. This PR is the complete fix.
• (b) A customer is not seeing SQL findings in production. This PR fixes nothing for them —
  production already runs the SQL-capable binary, and main at 0.1.1 cannot get past Fusion's
  version pin. That would need a separate investigation into Fusion routing or rule narrowness.

Please confirm which reading applies.

---

10. Follow-ups, explicitly not in this PR

1. --code-type backend silently scans zero SQL files. src/code_type_detector.rs:250-252 has
   no "sql" arm, so .sql falls through to CodeType::Unknown while every other backend language
   is matched. Real defect; needs a ticket. Not a production issue today — Fusion does not pass
   --code-type; its command is exactly ["sighthound", "--output-format", "json", path]
   (fusion/fusion/agents/sighthound_scanner.py:71).
2. Port the other three fork-only packs (xml, properties, config) — blocked on the
   load_embedded_rules refactor in §8.
3. Update the external agent skill. src/models.rs:297's positional [LANGUAGE] help string
   changed, so per AGENTS.md:46 the skill at Corgea/skills →
   plugins/sighthound/skills/sighthound/SKILL.md needs sql added. It enumerates languages in
   three places (frontmatter description, the Language values: list, and ## Limitations).
   Different repo, out of scope here.
4. src/config.rs:21 ESTIMATED_LANGUAGES: usize = 6 is stale — the registry now has 12
   languages. Pre-existing, deliberately untouched.

Scope fence

No tree-sitter SQL grammar. No dialect choice. No new or improved rules beyond D2a/D2b. No
prefilter change. No new control surface. No version bump. No re-vendoring. No fusion/ edits. No
build_sarif_log fix. No ESTIMATED_LANGUAGES fix. No AGENTS.md or CLAUDE.md edit.

[leenk7991]

i think we should also add "sql" to the bakend type in detect_code_type

can we also make sure that SIGHTHOUND_SUPPORTED_LANGUAGES in fusion includes "sql"

[juangaitanv]

Done — added "sql" to the backend arm in detect_code_type; a .sql file previously fell through to Unknown and was silently dropped under --code-type backend. SIGHTHOUND_SUPPORTED_LANGUAGES doesn't exist in this repo — it lives in fusion, so that half needs a fusion-side change.