`ActorLoader.discover()` reads each YAML file twice causing TOCTOU race and incorrect cache hashes #10239

New issue

Open

opened 2026-04-17 09:46:49 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-17 09:46:49 +00:00

Owner

Metadata

Commit Message: fix(actor): eliminate double file read in ActorLoader.discover() to fix TOCTOU and hash mismatch
Branch: fix/actor-loader-toctou-double-read

Background and Context

src/cleveragents/actor/loader.py ActorLoader.discover() reads each YAML file twice during a single discovery run. The first read (line 115) is used to compute a content hash for cache-invalidation purposes. The second read (line 201) is used to compute the hash stored in the new _CacheEntry. Because the two reads are not atomic, a file that is modified between them will produce two different hashes, causing the cache entry to record a hash that does not match the content that was actually parsed. This is a classic Time-of-Check/Time-of-Use (TOCTOU) defect.

Current Behavior

# --- First read (line 115) ---
content = resolved.read_bytes()
content_hash = _compute_hash(content)

# ... file is parsed, config is built ...

# --- Second read (line 201) ---
for name, entries in pending.items():
    resolved_path, config = entries[0]
    content_hash = _compute_hash(resolved_path.read_bytes())  # re-reads the file!
    ...
    entry = _CacheEntry(
        config=config,
        content_hash=content_hash,   # hash of second read, not first
        source_path=resolved_path,
    )

Consequences:

Incorrect cache invalidation: On the next discover() call the stored hash (from the second read) is compared against the current file content. If the file changed between the two reads, the stored hash reflects the modified content, so the cache will incorrectly believe the file is unchanged and skip re-parsing.
Unnecessary I/O: Every file is read from disk twice per discovery run, doubling I/O cost.
Race condition: In environments where actor YAML files are updated frequently (e.g., hot-reload scenarios), the mismatch can cause stale actor configs to be served indefinitely.

Expected Behavior

The content read during the first pass (line 115) should be reused for the cache entry hash. No second read_bytes() call should be made for the same file within a single discover() invocation.

# First read — reuse content and hash throughout
content = resolved.read_bytes()
content_hash = _compute_hash(content)

# ... parse content ...

# When building cache entry, pass the already-computed hash
entry = _CacheEntry(
    config=config,
    content_hash=content_hash,  # from first read, no second read needed
    source_path=resolved_path,
)

Acceptance Criteria

Each YAML file is read from disk exactly once per discover() call.
The hash stored in _CacheEntry is computed from the same bytes that were parsed.
Cache invalidation on subsequent discover() calls correctly detects files that changed after the previous run.
No regression in existing cache-hit / cache-miss behavior.

Supporting Information

File: src/cleveragents/actor/loader.py
Lines: 113–116 (first read) and 197–214 (second read)
Pattern: TOCTOU (Time-of-Check/Time-of-Use) race condition

Subtasks

Refactor discover() to carry the (content, content_hash) tuple from the first read through to the cache-entry construction loop
Remove the second resolved_path.read_bytes() call at line 201
Tests (Behave): Add scenario — file modified between two discover() calls → cache is invalidated and actor is re-parsed
Tests (Behave): Add scenario — file unchanged between two discover() calls → cache hit, no re-parse
Verify coverage ≥97% via nox -s coverage_report
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, followed by a blank line, then additional lines providing relevant details about the implementation.
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 before this issue is marked done.

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit Message**: `fix(actor): eliminate double file read in ActorLoader.discover() to fix TOCTOU and hash mismatch` - **Branch**: `fix/actor-loader-toctou-double-read` ## Background and Context `src/cleveragents/actor/loader.py` `ActorLoader.discover()` reads each YAML file **twice** during a single discovery run. The first read (line 115) is used to compute a content hash for cache-invalidation purposes. The second read (line 201) is used to compute the hash stored in the new `_CacheEntry`. Because the two reads are not atomic, a file that is modified between them will produce two different hashes, causing the cache entry to record a hash that does not match the content that was actually parsed. This is a classic Time-of-Check/Time-of-Use (TOCTOU) defect. ## Current Behavior ```python # --- First read (line 115) --- content = resolved.read_bytes() content_hash = _compute_hash(content) # ... file is parsed, config is built ... # --- Second read (line 201) --- for name, entries in pending.items(): resolved_path, config = entries[0] content_hash = _compute_hash(resolved_path.read_bytes()) # re-reads the file! ... entry = _CacheEntry( config=config, content_hash=content_hash, # hash of second read, not first source_path=resolved_path, ) ``` Consequences: 1. **Incorrect cache invalidation:** On the next `discover()` call the stored hash (from the second read) is compared against the current file content. If the file changed between the two reads, the stored hash reflects the *modified* content, so the cache will incorrectly believe the file is unchanged and skip re-parsing. 2. **Unnecessary I/O:** Every file is read from disk twice per discovery run, doubling I/O cost. 3. **Race condition:** In environments where actor YAML files are updated frequently (e.g., hot-reload scenarios), the mismatch can cause stale actor configs to be served indefinitely. ## Expected Behavior The content read during the first pass (line 115) should be reused for the cache entry hash. No second `read_bytes()` call should be made for the same file within a single `discover()` invocation. ```python # First read — reuse content and hash throughout content = resolved.read_bytes() content_hash = _compute_hash(content) # ... parse content ... # When building cache entry, pass the already-computed hash entry = _CacheEntry( config=config, content_hash=content_hash, # from first read, no second read needed source_path=resolved_path, ) ``` ## Acceptance Criteria - [ ] Each YAML file is read from disk exactly once per `discover()` call. - [ ] The hash stored in `_CacheEntry` is computed from the same bytes that were parsed. - [ ] Cache invalidation on subsequent `discover()` calls correctly detects files that changed after the previous run. - [ ] No regression in existing cache-hit / cache-miss behavior. ## Supporting Information - **File:** `src/cleveragents/actor/loader.py` - **Lines:** 113–116 (first read) and 197–214 (second read) - **Pattern:** TOCTOU (Time-of-Check/Time-of-Use) race condition ## Subtasks - [ ] Refactor `discover()` to carry the `(content, content_hash)` tuple from the first read through to the cache-entry construction loop - [ ] Remove the second `resolved_path.read_bytes()` call at line 201 - [ ] Tests (Behave): Add scenario — file modified between two `discover()` calls → cache is invalidated and actor is re-parsed - [ ] Tests (Behave): Add scenario — file unchanged between two `discover()` calls → cache hit, no re-parse - [ ] Verify coverage ≥97% via `nox -s coverage_report` - [ ] 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, followed by a blank line, then additional lines providing relevant details about the implementation. - 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** before this issue is marked done. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added the

labels

2026-04-17 10:02:31 +00:00