BUG-HUNT: [consistency] ActorLoader.discover() re-reads files from disk to compute content hash, creating TOCTOU race and stale config cache #6414

New issue

Open

opened 2026-04-09 21:01:33 +00:00 by HAL9000 · 0 comments

HAL9000 commented

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

Owner

Bug Report: [consistency] Double file read in discover() creates a TOCTOU window where cached hash can diverge from cached config

Severity Assessment

Impact: Under a file-change race (e.g., actor YAML updated on disk during the discover() loop), the cached ActorConfigSchema is built from the old file content but the cached content_hash is computed from the new file content. On the next discover() call, the hash comparison succeeds (hashes match), so the actor is not reloaded — but the served config is stale. The stale config could persist indefinitely in a long-running process.
Likelihood: Low in interactive CLI use, but real in any scenario with automated YAML editing, file-watching, or reload on save (e.g., IDE integration, file-watcher tests, concurrent CLI invocations).
Priority: Medium

Location

File: src/cleveragents/actor/loader.py
Function: ActorLoader.discover
Lines: 113–116 (first read) and 200–201 (second read)

Description

Inside discover(), each actor YAML file is read twice:

Line 115: content = resolved.read_bytes() — used to compute the preliminary hash check (lines 116–121) and for YAML parsing (line 128).
Line 201: content_hash = _compute_hash(resolved_path.read_bytes()) — a second independent read used to compute the hash stored in the new _CacheEntry.

Between read #1 and read #2, the file on disk may have changed. When this happens:

The config stored in the cache is parsed from read #1 (old content).
The content_hash stored in the cache comes from read #2 (new content).

On the next call to discover(), the hash comparison at line 121 (entry.content_hash == content_hash) passes because the new content's hash was already stored. The cached entry is reused without re-parsing — but the cached config was built from the old content. The stale config is served until a third file change occurs.

Evidence

# loader.py – first read, lines 113-116
for path in found_files:
    resolved = path.resolve()
    content = resolved.read_bytes()          # ← READ #1
    content_hash = _compute_hash(content)    # hash from READ #1

    existing_name = self._path_to_name.get(resolved)
    if existing_name and existing_name in self._actors:
        entry = self._actors[existing_name]
        if entry.content_hash == content_hash:   # compare against stored hash
            ...
            continue
    # ... YAML parsing uses 'content' (READ #1) ...

# loader.py – second read, lines 199-201
for name, entries in pending.items():
    resolved_path, config = entries[0]
    content_hash = _compute_hash(resolved_path.read_bytes())  # ← READ #2 (race window!)
    # If file changed between READ #1 and READ #2:
    # - config was parsed from READ #1 (old)
    # - content_hash computed from READ #2 (new)
    # → stale config cached with new hash → change is permanently missed

Timeline of the race:

T1: discover() reads file (READ #1) → parses config from OLD content
T2: File is written on disk with NEW content
T3: discover() reads file (READ #2) → hash of NEW content stored in cache
T4: Next discover() → hash matches NEW content → cache reused → OLD config returned

Expected Behavior

The content hash used in the cache entry should be computed from the same bytes used to parse the config — i.e., content from READ #1. There should be no second read.

Actual Behavior

A second read_bytes() call at line 201 uses potentially different file content to compute the stored hash. This creates a TOCTOU window where the cached config and the cached hash refer to different versions of the file.

Suggested Fix

Re-use the content read at line 115 by passing it through the pending structure:

# In the first loop — store content alongside config:
pending.setdefault(name, []).append((resolved, config, content_hash))

# In the second loop — use stored hash, no second read:
for name, entries in pending.items():
    resolved_path, config, content_hash = entries[0]
    # content_hash already computed from the same bytes as config
    old_entry = self._actors.get(name)
    ...

This eliminates both the double read and the TOCTOU window.

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it.

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

## Bug Report: [consistency] Double file read in discover() creates a TOCTOU window where cached hash can diverge from cached config ### Severity Assessment - **Impact**: Under a file-change race (e.g., actor YAML updated on disk during the `discover()` loop), the cached `ActorConfigSchema` is built from the **old** file content but the cached `content_hash` is computed from the **new** file content. On the next `discover()` call, the hash comparison succeeds (hashes match), so the actor is not reloaded — but the served config is stale. The stale config could persist indefinitely in a long-running process. - **Likelihood**: Low in interactive CLI use, but real in any scenario with automated YAML editing, file-watching, or reload on save (e.g., IDE integration, file-watcher tests, concurrent CLI invocations). - **Priority**: Medium ### Location - **File**: `src/cleveragents/actor/loader.py` - **Function**: `ActorLoader.discover` - **Lines**: 113–116 (first read) and 200–201 (second read) ### Description Inside `discover()`, each actor YAML file is read **twice**: 1. **Line 115**: `content = resolved.read_bytes()` — used to compute the preliminary hash check (lines 116–121) and for YAML parsing (line 128). 2. **Line 201**: `content_hash = _compute_hash(resolved_path.read_bytes())` — a **second independent read** used to compute the hash stored in the new `_CacheEntry`. Between read #1 and read #2, the file on disk may have changed. When this happens: - The `config` stored in the cache is parsed from read #1 (old content). - The `content_hash` stored in the cache comes from read #2 (new content). On the **next** call to `discover()`, the hash comparison at line 121 (`entry.content_hash == content_hash`) passes because the new content's hash was already stored. The cached entry is reused without re-parsing — but the cached `config` was built from the old content. The stale config is served until a **third** file change occurs. ### Evidence ```python # loader.py – first read, lines 113-116 for path in found_files: resolved = path.resolve() content = resolved.read_bytes() # ← READ #1 content_hash = _compute_hash(content) # hash from READ #1 existing_name = self._path_to_name.get(resolved) if existing_name and existing_name in self._actors: entry = self._actors[existing_name] if entry.content_hash == content_hash: # compare against stored hash ... continue # ... YAML parsing uses 'content' (READ #1) ... # loader.py – second read, lines 199-201 for name, entries in pending.items(): resolved_path, config = entries[0] content_hash = _compute_hash(resolved_path.read_bytes()) # ← READ #2 (race window!) # If file changed between READ #1 and READ #2: # - config was parsed from READ #1 (old) # - content_hash computed from READ #2 (new) # → stale config cached with new hash → change is permanently missed ``` **Timeline of the race**: ``` T1: discover() reads file (READ #1) → parses config from OLD content T2: File is written on disk with NEW content T3: discover() reads file (READ #2) → hash of NEW content stored in cache T4: Next discover() → hash matches NEW content → cache reused → OLD config returned ``` ### Expected Behavior The content hash used in the cache entry should be computed from the **same bytes** used to parse the config — i.e., `content` from READ #1. There should be no second read. ### Actual Behavior A second `read_bytes()` call at line 201 uses potentially different file content to compute the stored hash. This creates a TOCTOU window where the cached config and the cached hash refer to different versions of the file. ### Suggested Fix Re-use the content read at line 115 by passing it through the pending structure: ```python # In the first loop — store content alongside config: pending.setdefault(name, []).append((resolved, config, content_hash)) # In the second loop — use stored hash, no second read: for name, entries in pending.items(): resolved_path, config, content_hash = entries[0] # content_hash already computed from the same bytes as config old_entry = self._actors.get(name) ... ``` This eliminates both the double read and the TOCTOU window. ### Category consistency ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added the

labels

2026-04-09 21:07:18 +00:00

HAL9000 added this to the v3.2.0 milestone

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

HAL9000 added