cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [SECURITY] SQL injection risk in project configuration updates #7032

New issue

Open

opened 2026-04-10 07:16:51 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-10 07:16:51 +00:00

Owner

Background and Context

A potential SQL injection vulnerability has been identified in the _store_project_extras helper function in the project CLI command module. The function constructs a raw SQL UPDATE statement using f-string interpolation with dynamically assembled field names from the updates list.

While the current code appears safe because the field name strings ("invariants_json = :inv_json", "invariant_actor = :inv_actor") are hardcoded in the function body and values are passed via SQLAlchemy bound parameters (:inv_json, :inv_actor), the pattern itself is dangerous. Any future modification that introduces user-controlled input into the updates list — or any refactor that generalises the field-name construction — could introduce a real SQL injection vector.

File: src/cleveragents/cli/commands/project.py
Function: _store_project_extras
Lines: 127–130
Severity: Medium (current code appears safe, but the pattern is high-risk)
Category: Security

Vulnerable Code Pattern

# Lines 127–130 — f-string SQL construction with dynamic field names
if updates:
    sql = text(
        f"UPDATE ns_projects SET {', '.join(updates)} "
        f"WHERE namespaced_name = :ns_name"
    )
    session.execute(sql, params)

The updates list is assembled earlier in the function:

updates: list[str] = []
params: dict[str, str] = {"ns_name": namespaced_name}
if invariant_texts:
    updates.append("invariants_json = :inv_json")   # hardcoded — currently safe
    params["inv_json"] = _json.dumps(inv_list)
if inv_actor:
    updates.append("invariant_actor = :inv_actor")  # hardcoded — currently safe
    params["inv_actor"] = inv_actor

The field names in updates are currently literals, but the f-string pattern means any future contributor could inadvertently pass user-controlled data into the SQL fragment.

Current Behavior

The function executes a raw SQL UPDATE via SQLAlchemy text() with the SET clause constructed by joining the updates list using an f-string. Bound parameters are used for values, but the column names are interpolated directly into the SQL string.

Expected Behavior

SQL construction should not rely on f-string interpolation of any dynamic content. The preferred approaches are:

Use SQLAlchemy ORM — update the NamespacedProject domain model to expose invariants_json and invariant_actor, then use the repository API (eliminates raw SQL entirely).
Explicit allowlist — if raw SQL must be retained, validate each field name in updates against a strict allowlist before interpolation, and raise a ValueError for any unrecognised field name.

Acceptance Criteria

The f-string SQL construction pattern at lines 127–130 is eliminated or hardened
Either the ORM approach is adopted (preferred) or an explicit allowlist guard is added
No user-controlled string can reach the SQL fragment without validation
Existing behaviour of _store_project_extras is preserved (invariants and actor stored correctly)
A regression test is added that verifies the function rejects unexpected field names (if allowlist approach is used)
All nox stages pass
Coverage ≥ 97%

Supporting Information

The root cause is an architectural gap: invariants_json and invariant_actor are JSON columns on ns_projects that are not exposed on the NamespacedProject Pydantic domain model, forcing this raw-SQL workaround.
The cleanest fix is to expose these fields on the domain model and use the repository API — this also removes the need for a separate SQLAlchemy engine/session inside a CLI helper.
OWASP reference: SQL Injection
Related architectural context: docs/specification.md — Project abstraction, NamespacedProject domain model

Backlog note: This issue was discovered during autonomous operation
on milestone Cycle 2 Bug Hunt. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Metadata

Branch: fix/security-sql-injection-project-extras
Commit Message: fix(cli): eliminate f-string SQL construction in _store_project_extras
Milestone: (none — backlog pending human review)
Parent Epic: #7023

Subtasks

Audit _store_project_extras and confirm no user-controlled data currently reaches updates
Evaluate ORM approach: expose invariants_json and invariant_actor on NamespacedProject domain model
If ORM approach adopted: refactor _store_project_extras to use repository API; remove raw SQL
If allowlist approach retained: add strict field-name allowlist guard before f-string interpolation
Add/update unit tests covering the fix (including rejection of unexpected field names if allowlist used)
Run nox -s coverage_report — verify coverage ≥ 97%
Run nox (all default sessions) — fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly: fix(cli): eliminate f-string SQL construction in _store_project_extras
The commit is pushed to the remote on the branch matching the Branch in Metadata: fix/security-sql-injection-project-extras
The commit is submitted as a pull request to master, reviewed, and merged.
All nox stages pass.
Coverage ≥ 97%.

Automated by CleverAgents Bot
Supervisor: Bug Hunt Cycle 2 | Agent: new-issue-creator

## Background and Context A potential SQL injection vulnerability has been identified in the `_store_project_extras` helper function in the project CLI command module. The function constructs a raw SQL `UPDATE` statement using f-string interpolation with dynamically assembled field names from the `updates` list. While the current code appears safe because the field name strings (`"invariants_json = :inv_json"`, `"invariant_actor = :inv_actor"`) are hardcoded in the function body and values are passed via SQLAlchemy bound parameters (`:inv_json`, `:inv_actor`), the **pattern itself is dangerous**. Any future modification that introduces user-controlled input into the `updates` list — or any refactor that generalises the field-name construction — could introduce a real SQL injection vector. - **File**: `src/cleveragents/cli/commands/project.py` - **Function**: `_store_project_extras` - **Lines**: 127–130 - **Severity**: Medium (current code appears safe, but the pattern is high-risk) - **Category**: Security ### Vulnerable Code Pattern ```python # Lines 127–130 — f-string SQL construction with dynamic field names if updates: sql = text( f"UPDATE ns_projects SET {', '.join(updates)} " f"WHERE namespaced_name = :ns_name" ) session.execute(sql, params) ``` The `updates` list is assembled earlier in the function: ```python updates: list[str] = [] params: dict[str, str] = {"ns_name": namespaced_name} if invariant_texts: updates.append("invariants_json = :inv_json") # hardcoded — currently safe params["inv_json"] = _json.dumps(inv_list) if inv_actor: updates.append("invariant_actor = :inv_actor") # hardcoded — currently safe params["inv_actor"] = inv_actor ``` The field names in `updates` are currently literals, but the f-string pattern means any future contributor could inadvertently pass user-controlled data into the SQL fragment. ## Current Behavior The function executes a raw SQL `UPDATE` via SQLAlchemy `text()` with the SET clause constructed by joining the `updates` list using an f-string. Bound parameters are used for **values**, but the **column names** are interpolated directly into the SQL string. ## Expected Behavior SQL construction should not rely on f-string interpolation of any dynamic content. The preferred approaches are: 1. **Use SQLAlchemy ORM** — update the `NamespacedProject` domain model to expose `invariants_json` and `invariant_actor`, then use the repository API (eliminates raw SQL entirely). 2. **Explicit allowlist** — if raw SQL must be retained, validate each field name in `updates` against a strict allowlist before interpolation, and raise a `ValueError` for any unrecognised field name. ## Acceptance Criteria - [ ] The f-string SQL construction pattern at lines 127–130 is eliminated or hardened - [ ] Either the ORM approach is adopted (preferred) or an explicit allowlist guard is added - [ ] No user-controlled string can reach the SQL fragment without validation - [ ] Existing behaviour of `_store_project_extras` is preserved (invariants and actor stored correctly) - [ ] A regression test is added that verifies the function rejects unexpected field names (if allowlist approach is used) - [ ] All nox stages pass - [ ] Coverage ≥ 97% ## Supporting Information - The root cause is an architectural gap: `invariants_json` and `invariant_actor` are JSON columns on `ns_projects` that are not exposed on the `NamespacedProject` Pydantic domain model, forcing this raw-SQL workaround. - The cleanest fix is to expose these fields on the domain model and use the repository API — this also removes the need for a separate SQLAlchemy engine/session inside a CLI helper. - OWASP reference: [SQL Injection](https://owasp.org/www-community/attacks/SQL_Injection) - Related architectural context: `docs/specification.md` — Project abstraction, `NamespacedProject` domain model > **Backlog note:** This issue was discovered during autonomous operation > on milestone Cycle 2 Bug Hunt. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- ## Metadata - **Branch**: `fix/security-sql-injection-project-extras` - **Commit Message**: `fix(cli): eliminate f-string SQL construction in _store_project_extras` - **Milestone**: *(none — backlog pending human review)* - **Parent Epic**: #7023 ## Subtasks - [ ] Audit `_store_project_extras` and confirm no user-controlled data currently reaches `updates` - [ ] Evaluate ORM approach: expose `invariants_json` and `invariant_actor` on `NamespacedProject` domain model - [ ] If ORM approach adopted: refactor `_store_project_extras` to use repository API; remove raw SQL - [ ] If allowlist approach retained: add strict field-name allowlist guard before f-string interpolation - [ ] Add/update unit tests covering the fix (including rejection of unexpected field names if allowlist used) - [ ] Run `nox -s coverage_report` — verify coverage ≥ 97% - [ ] Run `nox` (all default sessions) — fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly: `fix(cli): eliminate f-string SQL construction in _store_project_extras` - The commit is pushed to the remote on the branch matching the **Branch** in Metadata: `fix/security-sql-injection-project-extras` - The commit is submitted as a **pull request** to `master`, reviewed, and **merged**. - All nox stages pass. - Coverage ≥ 97%. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Cycle 2 | Agent: new-issue-creator

HAL9000 added a new dependency

2026-04-10 07:17:00 +00:00

#7023 [AUTO-BUG-POOL] Status: Bug Detection Report (Cycle 2)

HAL9000 added the

labels

2026-04-10 07:18:02 +00:00

HAL9000 referenced this issue

2026-04-10 07:45:09 +00:00

[AUTO-BUG-POOL] Status: Bug Detection Report (Cycle 2) #7023

freemo added the

Priority

Critical

label

2026-04-12 18:37:47 +00:00

freemo removed a dependency

2026-04-13 16:34:29 +00:00

#7023 [AUTO-BUG-POOL] Status: Bug Detection Report (Cycle 2)

HAL9000 added

and removed

labels

2026-04-14 06:59:45 +00:00

HAL9000 commented

2026-04-14 06:59:45 +00:00

Author

Owner

✅ Verified — Critical security bug: SQL injection risk in project configuration updates. MoSCoW: Must-have. Priority: Critical.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

✅ **Verified** — Critical security bug: SQL injection risk in project configuration updates. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-19 14:13:51 +00:00

BUG-HUNT: [security] Bandit B608 -- f-string SQL construction in plan phases migration risks future injection #10777