cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [logic-error] ValidationAttachmentRepository.attach() silently swaps arguments when resource_id contains a slash #7749

New issue

Open

opened 2026-04-12 03:24:03 +00:00 by HAL9000 · 3 comments

HAL9000 commented

2026-04-12 03:24:03 +00:00

Owner

Bug Report: Logic Error — Silent Argument Swap in `ValidationAttachmentRepository.attach()`

Severity Assessment

Impact: Validation attachments can be stored with validation_name and resource_id swapped silently, corrupting data. Queries by resource or by validation name will return wrong results, and duplicate detection may fail
Likelihood: Medium — triggered whenever a resource_id contains a slash (which is the norm for namespaced resources like local/myresource) and validation_name does not
Priority: High

Location

File: src/cleveragents/infrastructure/database/repositories.py
Function/Class: ValidationAttachmentRepository.attach
Lines: 3838–3839

Description

The method contains a "convenience" argument swap that silently reorders validation_name and resource_id based on which one contains a slash:

if "/" in resource_id and "/" not in validation_name:
    validation_name, resource_id = resource_id, validation_name

This is intended to handle callers who pass arguments in the wrong order, but the heuristic is incorrect and fragile:

Namespaced resources have slashes by design: resource_id values like local/myresource or builtin/database contain slashes. If validation_name happens to NOT contain a slash (e.g., validate_schema), the arguments are swapped even when the caller passed them correctly.
No warning is logged: The swap happens silently, making debugging extremely difficult.
Both can have slashes: If both contain slashes, or neither does, the swap doesn't trigger — so the heuristic is inconsistent.

Evidence

# repositories.py lines 3838-3839
def attach(self, validation_name, resource_id, ...):
    ...
    if "/" in resource_id and "/" not in validation_name:
        validation_name, resource_id = resource_id, validation_name

Scenario 1 - Correct call, silently swapped:

repo.attach(
    validation_name="validate_schema",  # no slash (correct)
    resource_id="local/my-db",          # has slash (correct)
)
# RESULT: stored as validation_name="local/my-db", resource_id="validate_schema" -- WRONG

Scenario 2 - Both have slashes, no swap:

repo.attach(
    validation_name="local/schema-check",
    resource_id="local/my-db",
)
# RESULT: stored correctly (no swap triggered, both have slashes)

Expected Behavior

The function should store validation_name and resource_id exactly as provided by the caller. Argument ordering should be enforced via type signatures or documented conventions, not silent runtime swaps.

Actual Behavior

The function silently swaps validation_name and resource_id based on a fragile slash-presence heuristic, corrupting data whenever a namespaced resource_id is paired with an unnamespaced validation_name.

Suggested Fix

Remove the silent argument swap entirely:

# Remove lines 3838-3839:
# if "/" in resource_id and "/" not in validation_name:
#     validation_name, resource_id = resource_id, validation_name

If backwards compatibility with incorrect call sites is needed, add a deprecation warning instead of a silent swap, or fix the call sites.

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: Logic Error — Silent Argument Swap in `ValidationAttachmentRepository.attach()` ### Severity Assessment - **Impact**: Validation attachments can be stored with `validation_name` and `resource_id` swapped silently, corrupting data. Queries by resource or by validation name will return wrong results, and duplicate detection may fail - **Likelihood**: Medium — triggered whenever a `resource_id` contains a slash (which is the norm for namespaced resources like `local/myresource`) and `validation_name` does not - **Priority**: High ### Location - **File**: `src/cleveragents/infrastructure/database/repositories.py` - **Function/Class**: `ValidationAttachmentRepository.attach` - **Lines**: 3838–3839 ### Description The method contains a "convenience" argument swap that silently reorders `validation_name` and `resource_id` based on which one contains a slash: ```python if "/" in resource_id and "/" not in validation_name: validation_name, resource_id = resource_id, validation_name ``` This is intended to handle callers who pass arguments in the wrong order, but the heuristic is incorrect and fragile: 1. **Namespaced resources have slashes by design**: `resource_id` values like `local/myresource` or `builtin/database` contain slashes. If `validation_name` happens to NOT contain a slash (e.g., `validate_schema`), the arguments are swapped even when the caller passed them correctly. 2. **No warning is logged**: The swap happens silently, making debugging extremely difficult. 3. **Both can have slashes**: If both contain slashes, or neither does, the swap doesn't trigger — so the heuristic is inconsistent. ### Evidence ```python # repositories.py lines 3838-3839 def attach(self, validation_name, resource_id, ...): ... if "/" in resource_id and "/" not in validation_name: validation_name, resource_id = resource_id, validation_name ``` **Scenario 1 - Correct call, silently swapped**: ```python repo.attach( validation_name="validate_schema", # no slash (correct) resource_id="local/my-db", # has slash (correct) ) # RESULT: stored as validation_name="local/my-db", resource_id="validate_schema" -- WRONG ``` **Scenario 2 - Both have slashes, no swap**: ```python repo.attach( validation_name="local/schema-check", resource_id="local/my-db", ) # RESULT: stored correctly (no swap triggered, both have slashes) ``` ### Expected Behavior The function should store `validation_name` and `resource_id` exactly as provided by the caller. Argument ordering should be enforced via type signatures or documented conventions, not silent runtime swaps. ### Actual Behavior The function silently swaps `validation_name` and `resource_id` based on a fragile slash-presence heuristic, corrupting data whenever a namespaced `resource_id` is paired with an unnamespaced `validation_name`. ### Suggested Fix Remove the silent argument swap entirely: ```python # Remove lines 3838-3839: # if "/" in resource_id and "/" not in validation_name: # validation_name, resource_id = resource_id, validation_name ``` If backwards compatibility with incorrect call sites is needed, add a deprecation warning instead of a silent swap, or fix the call sites. ### Category logic-error ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added this to the v3.2.0 milestone

2026-04-12 03:41:38 +00:00

HAL9000 referenced this issue

2026-04-12 03:47:56 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 1) #7724

HAL9000 added the

labels

2026-04-12 07:15:48 +00:00

freemo added the

Type

Bug

label

2026-04-12 18:33:46 +00:00

HAL9000 added

and removed

labels

2026-04-14 05:58:33 +00:00

HAL9000 commented

2026-04-14 05:58:33 +00:00

Author

Owner

✅ Verified — Logic error: ValidationAttachmentRepository.attach() silently swaps arguments when resource_id contains slash. MoSCoW: Must-have. Priority: High — data integrity issue.

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

✅ **Verified** — Logic error: ValidationAttachmentRepository.attach() silently swaps arguments when resource_id contains slash. MoSCoW: Must-have. Priority: High — data integrity issue. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 commented

2026-04-14 06:11:50 +00:00

Author

Owner

✅ Verified — Logic error: ValidationAttachmentRepository.attach() silently swaps arguments when resource_id contains slash. MoSCoW: Must-have. Priority: High — data integrity issue.

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

HAL9000 commented

2026-04-14 06:15:16 +00:00

Author

Owner

✅ Verified — Logic error: ValidationAttachmentRepository.attach() silently swaps arguments when resource_id contains slash. MoSCoW: Must-have. Priority: High — data integrity issue.

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