cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [Security] Critical race condition in test environment using deprecated tempfile.mktemp() #7087

New issue

Open

opened 2026-04-10 07:33:41 +00:00 by HAL9000 · 2 comments

HAL9000 commented

2026-04-10 07:33:41 +00:00

Owner

Metadata

Branch: fix/security-tempfile-mktemp-race-condition
Commit Message: fix(testing): replace deprecated tempfile.mktemp() with secure alternatives to eliminate race condition
Milestone: v3.2.0
Parent Epic: TBD — requires manual linking (see orphan note below)

Background and Context

The test environment setup in features/environment.py uses the deprecated tempfile.mktemp() function in three locations across before_all() and before_scenario(). This function is explicitly documented by Python as unsafe because it generates a filename without creating the file, introducing a Time-of-Check to Time-of-Use (TOCTOU) race condition window between name generation and actual file creation.

This was discovered during Bug Hunt Cycle 2, Batch 3 by Worker 22 focusing on platform detection utilities.

Current Behavior

Three call sites in features/environment.py use tempfile.mktemp():

before_all() — line ~318: tempfile.mktemp(suffix='.db', prefix='cleveragents_') — sets CLEVERAGENTS_DATABASE_URL
before_all() — line ~322: tempfile.mktemp(suffix='.db', prefix='cleveragents_test_') — sets CLEVERAGENTS_TEST_DATABASE_URL
before_scenario() — line ~594: tempfile.mktemp(suffix=".db", prefix=prefix) — creates per-scenario DB paths

In all three cases, tempfile.mktemp() returns a path string for a file that does not yet exist. Between the moment the name is generated and the moment the SQLite engine opens the file, another process could create a file at that exact path — leading to data corruption, test pollution, or a security exploit in shared/concurrent test environments.

Python's own documentation states:

"Use of this function may introduce a security hole in your program. By the time you get around to doing anything with the file name it returns, someone else may have beaten you to the punch. mktemp() usage can be replaced easily with NamedTemporaryFile(), passing it the delete=False parameter."

Expected Behavior

Temporary database file paths should be created using secure, atomic methods that guarantee the file is created exclusively by the calling process:

tempfile.NamedTemporaryFile(suffix='.db', prefix='cleveragents_', delete=False) — creates and holds the file atomically
tempfile.mkstemp(suffix='.db', prefix='cleveragents_') — returns an OS-level file descriptor + path atomically

Impact

Likelihood: Medium-High in concurrent test environments (e.g., behave-parallel, CI with multiple workers)
Impact: Race condition where another process creates a file with the same name between generation and use, potentially causing:
- Data corruption in test databases
- Cross-test contamination in parallel test runs
- Security exploit in shared environments (an attacker could pre-create the file with malicious content)
Scope: All three tempfile.mktemp() call sites in features/environment.py

Suggested Fix

Replace all three tempfile.mktemp() calls with tempfile.mkstemp() or tempfile.NamedTemporaryFile(delete=False):

# BEFORE (unsafe):
db_path = tempfile.mktemp(suffix='.db', prefix='cleveragents_')

# AFTER (secure — using mkstemp):
fd, db_path = tempfile.mkstemp(suffix='.db', prefix='cleveragents_')
os.close(fd)  # Close the descriptor; SQLite will reopen it

# AFTER (secure — using NamedTemporaryFile):
with tempfile.NamedTemporaryFile(suffix='.db', prefix='cleveragents_', delete=False) as f:
    db_path = f.name

Subtasks

Replace tempfile.mktemp() at before_all() line ~318 with tempfile.mkstemp() (close fd immediately)
Replace tempfile.mktemp() at before_all() line ~322 with tempfile.mkstemp() (close fd immediately)
Replace tempfile.mktemp() at before_scenario() line ~594 with tempfile.mkstemp() (close fd immediately)
Verify no other tempfile.mktemp() calls exist in the codebase (grep -r "mktemp" .)
Ensure cleanup logic in after_scenario() / after_all() still correctly removes the temp DB files
Tests (Behave): Confirm existing BDD scenarios pass with the new temp file creation approach
Run nox (all default sessions), fix any errors
Verify coverage >= 97% via nox -s coverage_report

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off
All three tempfile.mktemp() call sites in features/environment.py are replaced with secure alternatives
No remaining tempfile.mktemp() calls exist anywhere in the codebase
The fix has been committed with the exact commit message from the Metadata section above, pushed to the branch named in Metadata, and a Pull Request has been submitted, reviewed, and merged
All nox stages pass
Coverage >= 97%

Supporting Information

File: features/environment.py
Functions affected: before_all() (lines ~318, ~322) and before_scenario() (line ~594)
Python docs reference: https://docs.python.org/3/library/tempfile.html#tempfile.mktemp
CWE: CWE-377 (Insecure Temporary File), CWE-362 (Race Condition)
Related: Similar insecure temp file usage was found in container executor (see #7058)
Discovered by: Bug Hunt Cycle 2, Batch 3, Worker 22 — platform detection utilities focus

Automated by CleverAgents Bot
Supervisor: Bug Hunt Cycle 2 | Agent: new-issue-creator

## Metadata - **Branch**: `fix/security-tempfile-mktemp-race-condition` - **Commit Message**: `fix(testing): replace deprecated tempfile.mktemp() with secure alternatives to eliminate race condition` - **Milestone**: v3.2.0 - **Parent Epic**: TBD — requires manual linking (see orphan note below) ## Background and Context The test environment setup in `features/environment.py` uses the deprecated `tempfile.mktemp()` function in three locations across `before_all()` and `before_scenario()`. This function is explicitly documented by Python as **unsafe** because it generates a filename without creating the file, introducing a Time-of-Check to Time-of-Use (TOCTOU) race condition window between name generation and actual file creation. This was discovered during **Bug Hunt Cycle 2, Batch 3** by Worker 22 focusing on platform detection utilities. ## Current Behavior Three call sites in `features/environment.py` use `tempfile.mktemp()`: 1. **`before_all()` — line ~318**: `tempfile.mktemp(suffix='.db', prefix='cleveragents_')` — sets `CLEVERAGENTS_DATABASE_URL` 2. **`before_all()` — line ~322**: `tempfile.mktemp(suffix='.db', prefix='cleveragents_test_')` — sets `CLEVERAGENTS_TEST_DATABASE_URL` 3. **`before_scenario()` — line ~594**: `tempfile.mktemp(suffix=".db", prefix=prefix)` — creates per-scenario DB paths In all three cases, `tempfile.mktemp()` returns a path string for a file that **does not yet exist**. Between the moment the name is generated and the moment the SQLite engine opens the file, another process could create a file at that exact path — leading to data corruption, test pollution, or a security exploit in shared/concurrent test environments. Python's own documentation states: > *"Use of this function may introduce a security hole in your program. By the time you get around to doing anything with the file name it returns, someone else may have beaten you to the punch. mktemp() usage can be replaced easily with NamedTemporaryFile(), passing it the delete=False parameter."* ## Expected Behavior Temporary database file paths should be created using secure, atomic methods that guarantee the file is created exclusively by the calling process: - `tempfile.NamedTemporaryFile(suffix='.db', prefix='cleveragents_', delete=False)` — creates and holds the file atomically - `tempfile.mkstemp(suffix='.db', prefix='cleveragents_')` — returns an OS-level file descriptor + path atomically ## Impact - **Likelihood**: Medium-High in concurrent test environments (e.g., `behave-parallel`, CI with multiple workers) - **Impact**: Race condition where another process creates a file with the same name between generation and use, potentially causing: - Data corruption in test databases - Cross-test contamination in parallel test runs - Security exploit in shared environments (an attacker could pre-create the file with malicious content) - **Scope**: All three `tempfile.mktemp()` call sites in `features/environment.py` ## Suggested Fix Replace all three `tempfile.mktemp()` calls with `tempfile.mkstemp()` or `tempfile.NamedTemporaryFile(delete=False)`: ```python # BEFORE (unsafe): db_path = tempfile.mktemp(suffix='.db', prefix='cleveragents_') # AFTER (secure — using mkstemp): fd, db_path = tempfile.mkstemp(suffix='.db', prefix='cleveragents_') os.close(fd) # Close the descriptor; SQLite will reopen it # AFTER (secure — using NamedTemporaryFile): with tempfile.NamedTemporaryFile(suffix='.db', prefix='cleveragents_', delete=False) as f: db_path = f.name ``` ## Subtasks - [ ] Replace `tempfile.mktemp()` at `before_all()` line ~318 with `tempfile.mkstemp()` (close fd immediately) - [ ] Replace `tempfile.mktemp()` at `before_all()` line ~322 with `tempfile.mkstemp()` (close fd immediately) - [ ] Replace `tempfile.mktemp()` at `before_scenario()` line ~594 with `tempfile.mkstemp()` (close fd immediately) - [ ] Verify no other `tempfile.mktemp()` calls exist in the codebase (`grep -r "mktemp" .`) - [ ] Ensure cleanup logic in `after_scenario()` / `after_all()` still correctly removes the temp DB files - [ ] Tests (Behave): Confirm existing BDD scenarios pass with the new temp file creation approach - [ ] Run `nox` (all default sessions), fix any errors - [ ] Verify coverage >= 97% via `nox -s coverage_report` ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off - All three `tempfile.mktemp()` call sites in `features/environment.py` are replaced with secure alternatives - No remaining `tempfile.mktemp()` calls exist anywhere in the codebase - The fix has been committed with the exact commit message from the Metadata section above, pushed to the branch named in Metadata, and a Pull Request has been submitted, reviewed, and merged - All nox stages pass - Coverage >= 97% ## Supporting Information - **File**: `features/environment.py` - **Functions affected**: `before_all()` (lines ~318, ~322) and `before_scenario()` (line ~594) - **Python docs reference**: https://docs.python.org/3/library/tempfile.html#tempfile.mktemp - **CWE**: CWE-377 (Insecure Temporary File), CWE-362 (Race Condition) - **Related**: Similar insecure temp file usage was found in container executor (see #7058) - **Discovered by**: Bug Hunt Cycle 2, Batch 3, Worker 22 — platform detection utilities focus --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Cycle 2 | Agent: new-issue-creator

HAL9000 added this to the v3.2.0 milestone

2026-04-10 07:34:19 +00:00

HAL9000 added the

labels

2026-04-10 07:38:11 +00:00

HAL9000 commented

2026-04-10 07:39:29 +00:00

Author

Owner

⚠️ Orphan Issue — Manual Linking Required

This issue was created by an automated agent and could not be automatically linked to a parent Epic. No Bug Hunt Cycle 2 Epic or security-focused parent Epic was found in the open issues list at creation time.

Action required for a maintainer:

Identify or create the appropriate parent Epic for Bug Hunt Cycle 2 security findings

Link this issue as a child by making it block the parent Epic:

POST /api/v1/repos/cleveragents/cleveragents-core/issues/7087/blocks
{ "owner": "cleveragents", "repo": "cleveragents-core", "index": <PARENT_EPIC_NUMBER> }

Remove this comment once the dependency link is established

Per CONTRIBUTING.md: orphan issues are not permitted — every issue must be linked to a parent Epic.

Automated by CleverAgents Bot
Supervisor: Bug Hunt Cycle 2 | Agent: new-issue-creator

⚠️ **Orphan Issue — Manual Linking Required** This issue was created by an automated agent and could not be automatically linked to a parent Epic. No Bug Hunt Cycle 2 Epic or security-focused parent Epic was found in the open issues list at creation time. **Action required for a maintainer:** 1. Identify or create the appropriate parent Epic for Bug Hunt Cycle 2 security findings 2. Link this issue as a child by making it **block** the parent Epic: ``` POST /api/v1/repos/cleveragents/cleveragents-core/issues/7087/blocks { "owner": "cleveragents", "repo": "cleveragents-core", "index": <PARENT_EPIC_NUMBER> } ``` 3. Remove this comment once the dependency link is established Per CONTRIBUTING.md: orphan issues are not permitted — every issue must be linked to a parent Epic. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Cycle 2 | Agent: new-issue-creator

HAL9000 added

and removed

labels

2026-04-14 06:48:52 +00:00

HAL9000 commented

2026-04-14 06:48:53 +00:00

Author

Owner

✅ Verified — Critical security bug: deprecated tempfile.mktemp() creates race condition. MoSCoW: Must-have. Priority: Critical.

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

✅ **Verified** — Critical security bug: deprecated tempfile.mktemp() creates race condition. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-16 22:13:20 +00:00

TEST-INFRA: [test-data-quality] context.temp_dir created in step files is never cleaned up by after_scenario #10082

HAL9000 referenced this issue

2026-04-17 10:30:45 +00:00

[AUTO-INF-6] Module-level ULID generation in step files creates cross-scenario session ID contamination #10245

HAL9000 referenced this issue

2026-04-18 08:15:59 +00:00

[AUTO-INF-6] Missing SQLAlchemy engine/session cleanup handlers in BDD step files cause resource leaks on scenario failure #10294

HAL9000 referenced this issue

2026-04-18 08:16:07 +00:00

[AUTO-INF-4] Flaky test: uko_indexer_watcher_steps.py debounce wait uses patched time.sleep — debounce never fires in CI #10295