UAT: `_store_project_extras()` builds SQL UPDATE with f-string column interpolation — SQL injection risk pattern #3628

New issue

Open

opened 2026-04-05 20:57:41 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 20:57:41 +00:00

Owner

Metadata

Branch: fix/backlog/store-project-extras-sql-injection-pattern
Commit Message: fix(cli): replace raw SQL f-string in _store_project_extras() with ORM repository call
Milestone: Backlog (no milestone — see backlog note below)
Parent Epic: #362

Background

The _store_project_extras() function in src/cleveragents/cli/commands/project.py (lines 128–131) constructs a raw SQL UPDATE statement using Python f-string interpolation to build the column list:

sql = text(
    f"UPDATE ns_projects SET {', '.join(updates)} "
    f"WHERE namespaced_name = :ns_name"
)
session.execute(sql, params)

While the column names in updates are currently hardcoded strings ("invariants_json = :inv_json", "invariant_actor = :inv_actor"), the pattern of using f-string interpolation to build SQL statements is a dangerous anti-pattern that:

Violates the project's secure coding standards
Creates a maintenance hazard — future contributors may add user-controlled values to updates without realising the injection risk
Is flagged by static analysis tools (bandit, semgrep) as a SQL injection risk pattern

Additionally, _store_project_extras() creates its own engine and session outside the DI container, bypassing the established session management pattern required by the Repository / Unit of Work architecture defined in docs/specification.md.

What was tested:

Code review of src/cleveragents/cli/commands/project.py lines 110–145
Verified that updates list currently only contains hardcoded column assignment strings
Verified that params dict uses SQLAlchemy bound parameters (:ns_name, :inv_json, :inv_actor) correctly for values

Expected behaviour (from spec / CONTRIBUTING.md):
All database operations must use parameterized queries exclusively. The ORM layer (SQLAlchemy) must be used for all mutations. Raw SQL with f-string interpolation must not appear in the codebase. Session management must flow through the DI container.

Actual behaviour:
_store_project_extras() bypasses the ORM entirely and uses raw SQL with f-string column interpolation. It also creates its own engine and session outside the DI container.

Recommended fix:
Expose invariants_json and invariant_actor fields on the NamespacedProject domain model and persist them through the NamespacedProjectRepository API, eliminating the need for the raw SQL workaround entirely.

Subtasks

Add invariants_json and invariant_actor fields to the NamespacedProject domain model
Add corresponding columns / migration to the ns_projects table schema (if not already present)
Implement persistence of invariants_json and invariant_actor via NamespacedProjectRepository.save() / update()
Remove _store_project_extras() from src/cleveragents/cli/commands/project.py
Update the agents project create --invariant command handler to use the repository instead of the removed helper
Audit the rest of src/cleveragents/cli/commands/project.py for any other raw SQL f-string patterns
Write / update Behave unit test scenarios covering invariants_json and invariant_actor persistence
Verify bandit and semgrep security scans report no SQL injection findings in project.py
Confirm all existing agents project create --invariant integration tests continue to pass

Definition of Done

_store_project_extras() is removed from the codebase
invariants_json and invariant_actor are persisted exclusively via NamespacedProjectRepository
No raw SQL with f-string interpolation remains anywhere in src/cleveragents/cli/commands/project.py
No out-of-DI-container engine/session creation remains in the CLI commands layer
Behave unit tests cover the new repository persistence path
bandit and semgrep security scans pass with no SQL injection findings
Existing agents project create --invariant tests continue to pass
All nox stages pass
Coverage >= 97%

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

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

## Metadata - **Branch**: `fix/backlog/store-project-extras-sql-injection-pattern` - **Commit Message**: `fix(cli): replace raw SQL f-string in _store_project_extras() with ORM repository call` - **Milestone**: Backlog (no milestone — see backlog note below) - **Parent Epic**: #362 ## Background The `_store_project_extras()` function in `src/cleveragents/cli/commands/project.py` (lines 128–131) constructs a raw SQL UPDATE statement using Python f-string interpolation to build the column list: ```python sql = text( f"UPDATE ns_projects SET {', '.join(updates)} " f"WHERE namespaced_name = :ns_name" ) session.execute(sql, params) ``` While the column names in `updates` are currently hardcoded strings (`"invariants_json = :inv_json"`, `"invariant_actor = :inv_actor"`), the pattern of using f-string interpolation to build SQL statements is a dangerous anti-pattern that: 1. Violates the project's secure coding standards 2. Creates a maintenance hazard — future contributors may add user-controlled values to `updates` without realising the injection risk 3. Is flagged by static analysis tools (bandit, semgrep) as a SQL injection risk pattern Additionally, `_store_project_extras()` creates its own engine and session outside the DI container, bypassing the established session management pattern required by the Repository / Unit of Work architecture defined in `docs/specification.md`. **What was tested:** - Code review of `src/cleveragents/cli/commands/project.py` lines 110–145 - Verified that `updates` list currently only contains hardcoded column assignment strings - Verified that `params` dict uses SQLAlchemy bound parameters (`:ns_name`, `:inv_json`, `:inv_actor`) correctly for values **Expected behaviour (from spec / CONTRIBUTING.md):** All database operations must use parameterized queries exclusively. The ORM layer (SQLAlchemy) must be used for all mutations. Raw SQL with f-string interpolation must not appear in the codebase. Session management must flow through the DI container. **Actual behaviour:** `_store_project_extras()` bypasses the ORM entirely and uses raw SQL with f-string column interpolation. It also creates its own engine and session outside the DI container. **Recommended fix:** Expose `invariants_json` and `invariant_actor` fields on the `NamespacedProject` domain model and persist them through the `NamespacedProjectRepository` API, eliminating the need for the raw SQL workaround entirely. ## Subtasks - [ ] Add `invariants_json` and `invariant_actor` fields to the `NamespacedProject` domain model - [ ] Add corresponding columns / migration to the `ns_projects` table schema (if not already present) - [ ] Implement persistence of `invariants_json` and `invariant_actor` via `NamespacedProjectRepository.save()` / `update()` - [ ] Remove `_store_project_extras()` from `src/cleveragents/cli/commands/project.py` - [ ] Update the `agents project create --invariant` command handler to use the repository instead of the removed helper - [ ] Audit the rest of `src/cleveragents/cli/commands/project.py` for any other raw SQL f-string patterns - [ ] Write / update Behave unit test scenarios covering `invariants_json` and `invariant_actor` persistence - [ ] Verify `bandit` and `semgrep` security scans report no SQL injection findings in `project.py` - [ ] Confirm all existing `agents project create --invariant` integration tests continue to pass ## Definition of Done - [ ] `_store_project_extras()` is removed from the codebase - [ ] `invariants_json` and `invariant_actor` are persisted exclusively via `NamespacedProjectRepository` - [ ] No raw SQL with f-string interpolation remains anywhere in `src/cleveragents/cli/commands/project.py` - [ ] No out-of-DI-container engine/session creation remains in the CLI commands layer - [ ] Behave unit tests cover the new repository persistence path - [ ] `bandit` and `semgrep` security scans pass with no SQL injection findings - [ ] Existing `agents project create --invariant` tests continue to pass - [ ] All nox stages pass - [ ] Coverage >= 97% > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.3.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester

freemo added a new dependency

2026-04-05 20:57:49 +00:00

#362 Epic: Security & Safety Hardening

freemo added this to the v3.7.0 milestone

2026-04-05 21:01:00 +00:00

freemo commented

2026-04-05 21:01:20 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: High — SQL injection risk pattern in _store_project_extras(). While currently using hardcoded column names (not user-controlled), the f-string SQL pattern is a maintenance hazard and violates secure coding standards.
Milestone: v3.7.0
Story Points: 3 — M — Replace raw SQL with ORM repository calls. Requires adding fields to domain model and repository.
MoSCoW: Should Have — Security anti-patterns must be eliminated. The spec requires all database operations to use parameterized queries via the ORM layer.

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

Issue triaged by project owner: - **State**: Verified - **Priority**: High — SQL injection risk pattern in `_store_project_extras()`. While currently using hardcoded column names (not user-controlled), the f-string SQL pattern is a maintenance hazard and violates secure coding standards. - **Milestone**: v3.7.0 - **Story Points**: 3 — M — Replace raw SQL with ORM repository calls. Requires adding fields to domain model and repository. - **MoSCoW**: Should Have — Security anti-patterns must be eliminated. The spec requires all database operations to use parameterized queries via the ORM layer. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-05 21:01:33 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3572

freemo referenced this issue

2026-04-05 21:07:35 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3572

freemo referenced this issue

2026-04-05 21:09:05 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this issue

2026-04-05 21:24:08 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377