fix(analyzer): honor sqlguard:ignore only inside SQL comments - #92
Conversation
The in-SQL directive only had to follow a comment marker somewhere earlier in the text, and the marker could itself sit inside a string, so SELECT * FROM users WHERE note = '-- sqlguard:ignore' reported nothing. Where a query embeds user input, a value could switch every rule off. parseIgnoreDirective now lexes the SQL and matches the directive only in comment spans, skipping quoted runs and dollar-quoted bodies. Where dialects disagree about literals (backslash escapes, $$) every reading is lexed and a directive counts only if all agree. Comment markers vary too: # is XOR in Postgres and MySQL's -- needs trailing whitespace, so a directive is dropped if the strict markers put it inside a literal. // inside SQL, never a SQL comment, is no longer honored. Closes #66
…ctives Comparing only "all markers on" against "all markers off" missed MySQL's actual mix, where # is a comment but --x is not. In 1 # '\n--x' -- sqlguard:ignore ' the directive sits inside a MySQL string yet still suppressed every rule. A directive found in a comment is now dropped if any combination of the # and -- readings puts it inside a literal.
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: KARTIKrocks/sqlguard/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| // Genuine directives. | ||
| {"line comment", "SELECT * FROM t -- sqlguard:ignore", true, nil}, | ||
| {"block comment scoped", "SELECT * FROM t /* sqlguard:ignore:select-star */", false, []string{"select-star"}}, | ||
| {"hash comment", "SELECT * FROM t # sqlguard:ignore", true, nil}, |
There was a problem hiding this comment.
Suggestion: Change the expected result to no suppression because # is not a comment marker in every supported dialect, so the directive must be rejected under intersection semantics.
Severity Level: Major Custom_rule
Rule source 📖
.codeant/review.json line 22 (rule "suppression-comment-only")
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** analyzer/suppress_test.go
**Line:** 21:21
**Comment:**
*Custom Rule: Change the expected result to no suppression because `#` is not a comment marker in every supported dialect, so the directive must be rejected under intersection semantics.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Declining: # sqlguard:ignore is a documented, working form (a MySQL comment), so this case must keep suppressing. The intersection covers the backslash and $$ literal readings only. The # and -- markers are handled differently: comments are found with every marker, and a directive is dropped only if some marker combination puts it inside a literal (the hash before a literal, mysql double minus and mysql marker mix cases). Here there is no literal, so it counts under every reading. The rule text read as if the intersection also covered the markers; d0a4b9e makes that explicit in .codeant/review.json and the other reviewer configs.
There was a problem hiding this comment.
✅ Customized review instruction saved!
Instruction:
Treat documented SQL comment markers such as MySQL '#' as valid suppression markers; apply intersection semantics only to literal interpretations, not to whether supported comment markers are recognized.
Applied to:
analyzer/suppress_test.go
💡 To manage or update this instruction, visit: CodeAnt AI Settings
| // (# is XOR in Postgres; MySQL's -- needs trailing whitespace) puts it inside | ||
| // a literal. | ||
| func commentDirective(sql string, backslash, dollar bool) (all bool, rules map[string]bool) { | ||
| comments, _ := lexSQL(sql, backslash, dollar, true, true) |
There was a problem hiding this comment.
Suggestion: The candidate scan always treats # and tight -- as comments, even when a dialect treats them as operators, so malformed or dialect-specific SQL can suppress findings outside a comment.
Assessment: 🟠 Major · 🔁 Occurrence: Rarely · 🏷️ Incorrect condition logic
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** analyzer/suppress.go
**Line:** 47:47
**Comment:**
*Incorrect Condition Logic: The candidate scan always treats `#` and tight `--` as comments, even when a dialect treats them as operators, so malformed or dialect-specific SQL can suppress findings outside a comment.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Declining, this is deliberate. Finding comments with every marker is what keeps a genuine MySQL # sqlguard:ignore working. The threat #66 closes is text inside a value, and that is covered: a directive is dropped if any #/-- combination, under any backslash/$$ reading, puts it inside a literal. For # or a tight -- to produce a suppression otherwise, the directive would have to appear as bare SQL tokens outside every literal, e.g. SELECT a # b FROM t sqlguard:ignore. That is not valid SQL in a dialect where # is an operator, a value cannot produce it, and whoever can write raw SQL can already write a real -- sqlguard:ignore.
There was a problem hiding this comment.
✅ Customized review instruction saved!
Instruction:
In the SQL suppression candidate scan, continue recognizing all dialect comment markers and discard directives found inside literals under any marker interpretation; do not flag this deliberate behavior as incorrect condition logic.
Applied to:
analyzer/suppress.go
💡 To manage or update this instruction, visit: CodeAnt AI Settings
parseIgnoreDirective runs on every analyzed query, and its pre-check lowercased the whole query, allocating whenever it held an uppercase byte. containsFold does the same case-insensitive search in place. Also make the reviewer rules say which readings must all agree: the backslash and $$ literal readings. The # and -- markers are handled by dropping a directive a marker combination puts in a literal, so a genuine # sqlguard:ignore still counts.
1 participant
User description
Closes #66.
What was wrong
The in-SQL
sqlguard:ignoredirective only had to follow a comment marker somewhere earlier in the text, and that marker could itself sit inside a string. SoSELECT * FROM users WHERE note = '-- sqlguard:ignore'reported nothing. Where a query embeds user input, a value could switch every rule off for the statement. The docs and the Greptile rationale both claimed the marker anchoring prevented exactly this.What changed
parseIgnoreDirectivenow lexes the SQL and matches the directive only inside comment spans (--,/* */,#), skipping quoted runs, quoted identifiers and dollar-quoted bodies whole.$$), every reading is lexed and a directive counts only if all of them agree. This is the opposite side fromRedact's union, on purpose: a spurious suppression hides findings, a missed one only reports a finding.#is XOR in PostgreSQL, and MySQL's--needs trailing whitespace. A directive found in a comment is dropped if any combination of those two readings puts it inside a literal (coversa # 4 AND note = '...',1--'...', and MySQL's own mix).//inside SQL text is no longer honored; it is not a SQL comment in any dialect and was never documented. The Go-source// sqlguard:ignoreread by the scanner (ParseIgnoreComment) is unaffected.Known cost: a
#comment containing an apostrophe (# don't flag: sqlguard:ignore) is not honored, since under the readings where#is not a comment the apostrophe opens a literal.--or/* */work there.Tests
TestParseIgnoreDirective: genuine directives, directives inside each kind of literal, both literal-ambiguity cases, and the#/--/ MySQL-mix cases. The literal cases fail onmain.TestIgnoreDirectiveInLiteralDoesNotSuppress: the sqlguard:ignore inside a string literal silences every rule #66 reproduction end to end.make fmt-check vet lint test-race lint-docsgreen across all nine modules.Docs
website/docs/suppressions.md(_Changed in 0.6._), CHANGELOG, AGENTS.md, and all reviewer configs:.greptile/rules.mdrewritten (it stated the false claim), and a newsuppression-comment-onlyrule in.greptile/config.json,.codeant/review.jsonand.coderabbit.yaml.CodeAnt-AI Description
Prevent SQL text from disabling analysis through false
sqlguard:ignoredirectivesWhat Changed
// sqlguard:ignoreis no longer treated as a SQL comment, while Go-source suppression remains supportedImpact
✅ Prevented user-supplied SQL values from hiding analyzer findings✅ Fewer false suppressions across SQL dialects✅ Clearer and safer suppression behavior💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.