Skip to content

fix(security): eliminate SQL string formatting in execute() calls#2061

Merged
teknium1 merged 1 commit intoNousResearch:mainfrom
dusterbloom:fix/sql-injection-parameterized-queries
Mar 19, 2026
Merged

fix(security): eliminate SQL string formatting in execute() calls#2061
teknium1 merged 1 commit intoNousResearch:mainfrom
dusterbloom:fix/sql-injection-parameterized-queries

Conversation

@dusterbloom
Copy link
Copy Markdown

Summary

Closes #1911

Eliminates SQL string formatting anti-patterns in execute() calls across the codebase. While current inputs are hardcoded constants (not directly exploitable), the f-string interpolation pattern is dangerous and violates parameterized query best practices.

Changes

File Change
agent/insights.py Pre-compute SELECT queries as class constants (_GET_SESSIONS_WITH_SOURCE, _GET_SESSIONS_ALL). F-string interpolation of _SESSION_COLS now runs once at class definition time, never at runtime.
hermes_state.py Add double-quote identifier escaping for ALTER TABLE column names in schema migrations. Defense-in-depth since SQLite DDL cannot be parameterized.
tests/test_sql_injection.py 4 new tests verifying no injection vectors in query construction.

How to test

python3 -m pytest tests/test_sql_injection.py tests/test_insights.py -v

All 64 tests pass (4 new + 60 existing insights tests).

Security note

This PR affects SQL query construction. The fix ensures:

  • No runtime string interpolation in execute() calls
  • Column identifiers are properly quoted
  • _SESSION_COLS contains only safe [a-zA-Z_][a-zA-Z0-9_]* identifiers (verified by test)

Platform

Tested on Linux (WSL2, Python 3.11). Changes are pure Python/SQLite — no platform-specific impact.

Full test suite: 4,449 passed, 214 failed (all pre-existing), 174 skipped, 0 new regressions.

Closes NousResearch#1911

- insights.py: Pre-compute SELECT queries as class constants instead of
  f-string interpolation at runtime. _SESSION_COLS is now evaluated once
  at class definition time.
- hermes_state.py: Add identifier quoting and whitelist validation for
  ALTER TABLE column names in schema migrations.
- Add 4 tests verifying no injection vectors in SQL query construction.
@teknium1 teknium1 merged commit 35558da into NousResearch:main Mar 19, 2026
@dusterbloom dusterbloom deleted the fix/sql-injection-parameterized-queries branch March 30, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants