Summary
#3445 (parameterize update_user_policy keys) and #3451 (drop f-string LIMIT in get_simulations) were filed in isolation from a targeted bug audit. Neither was found via a systematic grep of the repo's SQL surface. A repo-wide audit of f-string/.format() SQL construction is overdue to flush out any remaining patterns before the next incident.
What goes wrong
Today's workflow assumes contributors notice string-interpolated SQL during review. There is no:
- Automated check (ruff, bandit, semgrep rule) for
execute(f"..." or execute("..." % ...) patterns.
- Pre-commit hook blocking string-interpolated SQL.
- Documented guideline in
CONTRIBUTING.md about parameterization.
The result is that new endpoints can quietly reintroduce injection vectors; #3445 and #3451 both survived multiple reviews.
Suggested fix
- Audit: grep the repo for all SQL execution sites (
cursor.execute, db.execute, connection.execute, etc.) and verify every dynamic segment is a ?/:param placeholder. Cover at minimum:
policyengine_api/endpoints/ (all files)
policyengine_api/services/
policyengine_api/data/
policyengine_api/utils/
- Codify: add a
bandit config or semgrep rule to CI that fails on execute(f"..." and similar patterns. Example rule:
- id: fstring-sql
pattern-either:
- pattern: $CONN.execute(f"...")
- pattern: $CONN.execute("..." % $X)
- pattern: $CONN.execute("..." + $X)
message: Don't build SQL with string interpolation — use parameterized queries.
severity: ERROR
languages: [python]
- Document in
CONTRIBUTING.md: "SQL must use parameterized queries; never f"..." or .format() into a SQL string."
Severity
Medium — no known active vulnerability, but the class of bug has now resurfaced twice in the same repo.
Relates to
Fixes #3445, #3451 (both closed).
Summary
#3445 (parameterize
update_user_policykeys) and #3451 (drop f-stringLIMITinget_simulations) were filed in isolation from a targeted bug audit. Neither was found via a systematic grep of the repo's SQL surface. A repo-wide audit of f-string/.format()SQL construction is overdue to flush out any remaining patterns before the next incident.What goes wrong
Today's workflow assumes contributors notice string-interpolated SQL during review. There is no:
execute(f"..."orexecute("..." % ...)patterns.CONTRIBUTING.mdabout parameterization.The result is that new endpoints can quietly reintroduce injection vectors; #3445 and #3451 both survived multiple reviews.
Suggested fix
cursor.execute,db.execute,connection.execute, etc.) and verify every dynamic segment is a?/:paramplaceholder. Cover at minimum:policyengine_api/endpoints/(all files)policyengine_api/services/policyengine_api/data/policyengine_api/utils/banditconfig or semgrep rule to CI that fails onexecute(f"..."and similar patterns. Example rule:CONTRIBUTING.md: "SQL must use parameterized queries; neverf"..."or.format()into a SQL string."Severity
Medium — no known active vulnerability, but the class of bug has now resurfaced twice in the same repo.
Relates to
Fixes #3445, #3451 (both closed).