BUG-HUNT: [error-handling] Missing Argument Validation in Changeset Repositories #3009

New issue

Open

opened 2026-04-05 03:36:49 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 03:36:49 +00:00

Owner

Metadata

Branch: bugfix/m3-changeset-repo-arg-validation
Commit Message: fix(error-handling): validate session_factory is callable in ChangeSetEntryRepository and ToolInvocationRepository
Milestone: v3.3.0
Parent Epic: #362

Bug Report: [error-handling] — Missing Argument Validation in Changeset Repositories

Severity Assessment

Impact: A TypeError will be raised when a repository method is called, which is less clear than a ValueError in the constructor. This violates the fail-fast principle.
Likelihood: Low. This would only occur if the dependency injection container is misconfigured.
Priority: Low

Location

File: src/cleveragents/infrastructure/database/changeset_repository.py
Function/Class: ChangeSetEntryRepository.__init__, ToolInvocationRepository.__init__
Lines: 72-80, 224-230

Description

The __init__ methods of ChangeSetEntryRepository and ToolInvocationRepository check if session_factory is None, but they do not validate that the provided argument is a callable. This can lead to a TypeError when a repository method is called, which is less informative than a ValueError during object construction.

Evidence

# ChangeSetEntryRepository
    def __init__(
        self,
        session_factory: Callable[[], Session],
    ) -> None:
        """Initialise with a callable returning a SQLAlchemy Session."""
        if session_factory is None:
            raise ValueError("session_factory must not be None")
        self._sf = session_factory

# ToolInvocationRepository
    def __init__(
        self,
        session_factory: Callable[[], Session],
    ) -> None:
        if session_factory is None:
            raise ValueError("session_factory must not be None")
        self._sf = session_factory

Expected Behavior

The constructors should validate that session_factory is a callable and raise a TypeError or ValueError if it is not.

Actual Behavior

The constructors accept non-callable arguments, leading to a TypeError later when the session factory is invoked.

Suggested Fix

Add a callable() check to both constructors:

        if not callable(session_factory):
            raise TypeError("session_factory must be a callable")

Subtasks

Add callable() check to ChangeSetEntryRepository.__init__ in changeset_repository.py
Add callable() check to ToolInvocationRepository.__init__ in changeset_repository.py
Add/update unit tests to cover non-callable argument rejection for both constructors
Verify coverage >= 97% via nox -s coverage_report

Definition of Done

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.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
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 Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `bugfix/m3-changeset-repo-arg-validation` - **Commit Message**: `fix(error-handling): validate session_factory is callable in ChangeSetEntryRepository and ToolInvocationRepository` - **Milestone**: v3.3.0 - **Parent Epic**: #362 ## Bug Report: [error-handling] — Missing Argument Validation in Changeset Repositories ### Severity Assessment - **Impact**: A `TypeError` will be raised when a repository method is called, which is less clear than a `ValueError` in the constructor. This violates the fail-fast principle. - **Likelihood**: Low. This would only occur if the dependency injection container is misconfigured. - **Priority**: Low ### Location - **File**: `src/cleveragents/infrastructure/database/changeset_repository.py` - **Function/Class**: `ChangeSetEntryRepository.__init__`, `ToolInvocationRepository.__init__` - **Lines**: 72-80, 224-230 ### Description The `__init__` methods of `ChangeSetEntryRepository` and `ToolInvocationRepository` check if `session_factory` is `None`, but they do not validate that the provided argument is a callable. This can lead to a `TypeError` when a repository method is called, which is less informative than a `ValueError` during object construction. ### Evidence ```python # ChangeSetEntryRepository def __init__( self, session_factory: Callable[[], Session], ) -> None: """Initialise with a callable returning a SQLAlchemy Session.""" if session_factory is None: raise ValueError("session_factory must not be None") self._sf = session_factory ``` ```python # ToolInvocationRepository def __init__( self, session_factory: Callable[[], Session], ) -> None: if session_factory is None: raise ValueError("session_factory must not be None") self._sf = session_factory ``` ### Expected Behavior The constructors should validate that `session_factory` is a callable and raise a `TypeError` or `ValueError` if it is not. ### Actual Behavior The constructors accept non-callable arguments, leading to a `TypeError` later when the session factory is invoked. ### Suggested Fix Add a `callable()` check to both constructors: ```python if not callable(session_factory): raise TypeError("session_factory must be a callable") ``` ### Category error-handling ## Subtasks - [ ] Add `callable()` check to `ChangeSetEntryRepository.__init__` in `changeset_repository.py` - [ ] Add `callable()` check to `ToolInvocationRepository.__init__` in `changeset_repository.py` - [ ] Add/update unit tests to cover non-callable argument rejection for both constructors - [ ] Verify coverage >= 97% via `nox -s coverage_report` ## Definition of Done - [ ] 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. - [ ] The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - [ ] 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 Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.3.0 milestone

2026-04-05 03:38:48 +00:00

freemo added a new dependency

2026-04-05 03:44:59 +00:00

#362 Epic: Security & Safety Hardening

freemo commented

2026-04-05 03:59:00 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Confirmed
MoSCoW: Should Have

Valid finding verified during batch triage.

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

Issue triaged by project owner: - **State**: Verified - **Priority**: Confirmed - **MoSCoW**: Should Have Valid finding verified during batch triage. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-05 06:15:22 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094