Skip to content

Fix/fallback backtick identifiers - #103

Merged
KARTIKrocks merged 3 commits into
mainfrom
fix/fallback-backtick-identifiers
Oct 14, 2026
Merged

KARTIKrocks merged 3 commits into
mainfrom
fix/fallback-backtick-identifiers

Conversation

@KARTIKrocks

@KARTIKrocks KARTIKrocks commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Summary

The default parser blanked string literals but not backtick-quoted identifiers, so a table or column named like a keyword was read as a clause: DELETE FROM `where` counted a WHERE, and a MySQL column named limit counted a LIMIT. That hid delete-without-where, update-without-where and select-without-limit findings on MySQL/MariaDB queries.

Backtick runs are now reduced to `x` before the clause scan. x rather than nothing keeps a quoted column visible to the function-on-column check, so WHERE YEAR(`d`) = 1 still reports non-sargable-predicate. An unterminated backtick is left alone instead of swallowing the rest of the statement. IsMultiStatement deliberately keeps its own reading (it does not blank backticks): a backtick is no quote in PostgreSQL, and a run of them must not hide a ;.

Closes #99

Type of change

  • Bug fix
  • New detection rule
  • New integration / parser
  • Feature / enhancement
  • Docs only
  • Refactor / chore

Checklist

  • make ci passes (fmt-check, vet, lint, vuln, test-race, lint-docs) across all modules
  • Added/updated tests (and, where practical, a failure-mode check)
  • Updated docs under website/docs/ with a version marker for anything new (n/a: no public API or documented behaviour change)
  • Updated AGENTS.md / .sqlguard.example.yml if a convention or config key changed (n/a: no convention changed)
  • Added an entry under ## [Unreleased] in CHANGELOG.md
  • No new third-party deps in analyzer / middleware / reporter
  • Findings stay redaction-safe (no raw literals leak into a Result)

Notes for reviewers

  • TestFallbackParserBacktickIdentifiers covers keyword-named tables and columns, doubled and unterminated backticks, and a backtick inside a string; TestFallbackParserBacktickNonSargable covers a function over a quoted column. The identifier cases fail without the fix.
  • Known gap, same as on main and not changed here: stripComments is not backtick-aware, so a -- inside a backtick name (FROM `a--b` ) still starts a comment. Copying backtick runs verbatim there would also reach Redact, where a stray backtick could leave a comment's contents in the output, so it needs its own change.
  • Double-quoted columns under a function have the same non-sargable gap as before (Make double-quoted literal redaction dialect-aware #62 territory).

A table or column named like a keyword (`where`, `limit`) was read as a
clause by the fallback parser. IsMultiStatement keeps its own reading, since a
backtick is no quote in PostgreSQL.

Fixes #99
@codeant-ai

codeant-ai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR da56ea9 Oct 14, 2026 · 05:53 06:00

@codeant-ai

codeant-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Repository: KARTIKrocks/sqlguard/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 46fdb7a5-08bc-4b7e-9dc9-0404ba0d7817
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codeant-ai

codeant-ai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: 9dd5354d
Scan Time: 2026-10-14 06:07:12 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets ✅ PASSED 0 secrets found
SAST ✅ PASSED No security issues
SCA (Dependencies) ✅ PASSED Rating S: No vulnerabilities

View Full Results

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Oct 14, 2026
Blank a backtick run to `x` rather than nothing so YEAR(`d`) still reads as a
function on a column, and leave an unterminated backtick alone.
@codeant-ai

codeant-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown

CodeAnt Nitpicks

1 custom suggestion

1. Replace select-without-where with a registered rule name, or remove it; sqlguard does not register this rule.

Custom_rule · CHANGELOG.md:33

@codeant-ai

codeant-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown

CodeAnt PR Risk: Low Risk

  • The PR appears safe to merge; the change is narrowly scoped to fallback-parser handling of backtick-quoted identifiers.
  • Tests cover keyword-named identifiers, real clauses, doubled backticks, and function predicates on quoted columns.

Assessed commit: 9dd5354d73db

@KARTIKrocks
KARTIKrocks merged commit ec6ddcc into main Oct 14, 2026
32 checks passed
@KARTIKrocks
KARTIKrocks deleted the fix/fallback-backtick-identifiers branch October 14, 2026 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: backtick-quoted identifiers survive the fallback sanitizer and read as clauses

1 participant