Skip to content

security: harden raw and literal variable rendering against injection#1

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775755468-security-hardening
Open

security: harden raw and literal variable rendering against injection#1
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775755468-security-hardening

Conversation

@devin-ai-integration
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot commented Apr 9, 2026

Summary

Adds input validation guards to the two unsafe variable rendering paths (/*!name*/ raw variables and /*^name*/ literal variables) to reduce the risk of SQL injection through these escape hatches.

render-raw-variable (! sigil): Previously performed zero validation — any string was interpolated directly into SQL. Now rejects values containing:

  • ; (multi-statement injection)
  • -- (line comment sequences)
  • /* (block comment sequences)

render-literal-variable (^ sigil): Previously only rejected single quotes. Now also rejects:

  • \ (backslash — escape-based injection in databases with non-standard string escaping, relevant for planned MySQL support per roadmap)
  • NUL bytes (truncation attacks in some drivers)

Five new tests cover all added validation rules.

Review & Testing Checklist for Human

  • The blocklist for raw variables is intentionally narrow and not exhaustive. Patterns like UNION SELECT, subqueries, and xp_cmdshell are not blocked. This is defense-in-depth, not a security boundary — verify this aligns with your expectations for the ! sigil's threat model.
  • Backslash rejection in literal variables may be overly strict for PostgreSQL. With standard_conforming_strings = on (default since PG 9.1), backslashes are literal characters, so legitimate values like file paths (C:\data) would now be rejected. Decide if this trade-off is acceptable or if the check should be deferred until non-PostgreSQL support is added.
  • -- rejection in raw variables could false-positive on edge cases. While -- is universally a SQL line comment, verify there's no legitimate raw-variable use case in your codebase that would contain this sequence (e.g., unusual identifier patterns).
  • No opt-out mechanism exists. These checks are unconditional. If a user needs to pass a semicolon in a raw variable for a legitimate reason (unlikely but possible), they are blocked with no workaround. Consider whether a :unsafe? true option should exist.
  • Run bb test locally to confirm all 81+ tests pass (the new tests exercise exception paths, so they should be deterministic).

Notes

  • The ! sigil is documented in the spec as "safety is intentionally delegated to the developer" — these guards are best-effort defense-in-depth, not a replacement for developer diligence. The blocklist targets patterns that have no legitimate use in typical raw-variable contexts (ORDER BY columns, table/column names, etc.).
  • The checks are consistent with the library's existing error-handling pattern (throwing ex-info with :parameter and :value in the data map).

Link to Devin session: https://app.devin.ai/sessions/c4ff79c8728b4771ad2ab36bfe3e71cf
Requested by: @hatappo

@devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@hatappo hatappo force-pushed the main branch 2 times, most recently from 1182d98 to 3ab939e Compare April 9, 2026 18:15
- render-raw-variable: reject values containing semicolons, line comment
  sequences (--), and block comment sequences (/*) to prevent
  multi-statement injection and comment-based query manipulation

- render-literal-variable: reject backslash characters and NUL bytes
  in string values in addition to existing single-quote check, to
  prevent escape-based injection in non-PostgreSQL databases

- Add tests for all new validation rules
@devin-ai-integration devin-ai-integration bot force-pushed the devin/1775755468-security-hardening branch from 1e8c5e2 to 7085cfe Compare April 9, 2026 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant