fix(python): avoid FileAlreadyExistsException in venv cleanup by ruromero · Pull Request #373 · guacsec/trustify-da-java-client

ruromero · 2026-03-26T11:25:30Z

Summary

Remove redundant Files.createFile() before Files.write() in PythonControllerVirtualEnv
Files.write() already creates the file if it doesn't exist
The createFile() call caused FileAlreadyExistsException when a previous analysis failed mid-way and left the requirements.txt behind in /tmp/trustify_da_env/

Test plan

All existing Python provider tests pass (35 tests, 0 failures)
Manual: trigger a Python analysis that fails (e.g. unresolvable dep), then trigger another — should no longer throw FileAlreadyExistsException

Implements TC-3894

🤖 Generated with Claude Code

Remove redundant Files.createFile() before Files.write() in PythonControllerVirtualEnv. Files.write() already creates the file if it doesn't exist. The createFile() call caused FileAlreadyExistsException when a previous analysis failed mid-way and left the requirements.txt behind in /tmp/trustify_da_env/. Implements TC-3894 Assisted-by: Claude Code

qodo-code-review · 2026-03-26T11:25:41Z

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Remove redundant file creation in Python venv cleanup

🐞 Bug fix

Walkthroughs

Description

• Remove redundant Files.createFile() call before Files.write()
• Files.write() already creates file if missing
• Fixes FileAlreadyExistsException when retrying failed analyses
• Prevents stale requirements.txt from causing race conditions

Diagram

flowchart LR
  A["Previous: createFile + write"] -->|"FileAlreadyExistsException"| B["Failed on retry"]
  C["Fixed: write only"] -->|"Auto-creates file"| D["Works on retry"]

File Changes

1. src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java 🐞 Bug fix +0/-1

Remove redundant file creation in venv cleanup

• Removed redundant Files.createFile(envRequirements) call
• Files.write() automatically creates the file if it doesn't exist
• Eliminates FileAlreadyExistsException when stale requirements.txt remains from failed analysis
• Simplifies code by removing unnecessary operation

src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java

qodo-code-review · 2026-03-26T11:25:43Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. TOCTOU write in /tmp 🐞 Bug ⛨ Security

Description

PythonControllerVirtualEnv.cleanEnvironment() deletes requirements.txt, runs a non-trivial external
command (pip freeze), then writes requirements.txt; after removing Files.createFile(), a file
recreated during that window will now be overwritten instead of failing fast. Because the venv
directory is a fixed /tmp path and prepareEnvironment() trusts any pre-existing path, this enables
symlink/race-style clobbering and can corrupt cleanup behavior.

Code

src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[R91-96]

        Files.deleteIfExists(envRequirements);
        String freezeOutput =
            Operations.runProcessGetOutput(pythonEnvironmentDir, pipBinaryLocation, "freeze");
-        Files.createFile(envRequirements);
        Files.write(envRequirements, freezeOutput.getBytes());
        Operations.runProcessGetOutput(
            pythonEnvironmentDir, pipBinaryLocation, "uninstall", "-y", "-r", "requirements.txt");

Evidence

The virtualenv directory is hard-coded to /tmp/trustify_da_env and is only created if it doesn’t
already exist (no validation of what already exists at that path). In cleanEnvironment(false), the
code deletes requirements.txt, runs pip freeze (time gap), then writes requirements.txt; removing
the explicit createFile() step eliminates the prior behavior where a recreated path would have
triggered FileAlreadyExistsException, leaving a TOCTOU window for another process/user to recreate
the path before the write.

src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[29-54]
src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[71-101]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`cleanEnvironment(false)` deletes `/tmp/trustify_da_env/requirements.txt`, runs `pip freeze`, then writes `requirements.txt`. With the removal of `Files.createFile()`, a path recreated between delete and write will be overwritten, creating a TOCTOU window on a `/tmp`-based directory.

### Issue Context
- The environment directory is hard-coded under `/tmp` and `prepareEnvironment()` trusts any existing path at that location.
- `pip freeze` introduces a time gap between delete and write.

### Fix Focus Areas
- src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[39-43]
- src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[89-96]

### Proposed fix
1. Write freeze output to a temp file in `pythonEnvironmentDir` (e.g., `Files.createTempFile(pythonEnvironmentDir, "requirements", ".txt")`).
2. Atomically replace the final `requirements.txt` using `Files.move(temp, envRequirements, ATOMIC_MOVE, REPLACE_EXISTING)` (fallback to non-atomic move if ATOMIC_MOVE unsupported).
3. (Optional hardening) Validate `pythonEnvironmentDir` is a real directory (not a symlink) before using it, and consider creating a per-run temp venv directory instead of a fixed `/tmp/trustify_da_env` path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Global venv path shared 🐞 Bug ⛯ Reliability

Description

PythonControllerVirtualEnv uses a single fixed directory (/tmp/trustify_da_env) for all virtualenv
operations, so overlapping runs can interfere by reading/writing the same requirements.txt and
uninstalling packages from the same environment. The changed write behavior makes such overlap more
likely to proceed (rather than erroring) and can lead to cross-run contamination.

Code

src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[R91-96]

        Files.deleteIfExists(envRequirements);
        String freezeOutput =
            Operations.runProcessGetOutput(pythonEnvironmentDir, pipBinaryLocation, "freeze");
-        Files.createFile(envRequirements);
        Files.write(envRequirements, freezeOutput.getBytes());
        Operations.runProcessGetOutput(
            pythonEnvironmentDir, pipBinaryLocation, "uninstall", "-y", "-r", "requirements.txt");

Evidence
The controller hard-codes pythonEnvironmentDir to /tmp/trustify_da_env, and the cleanup flow
writes a shared requirements.txt then executes pip uninstall -r requirements.txt inside that
same directory. Separately, PythonProvider creates a new controller instance when not injected,
meaning multiple call sites can end up operating on the same global directory (even if via distinct
controller objects).
src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[29-34]
src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[89-96]
src/main/java/io/github/guacsec/trustifyda/providers/PythonProvider.java[228-257]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A single fixed venv directory (`/tmp/trustify_da_env`) and shared `requirements.txt` are used for all virtualenv runs. Any overlapping runs can step on each other’s environment state and cleanup (freeze/uninstall).

### Issue Context
This PR reduces one failure mode (`FileAlreadyExistsException`), but the underlying shared-state design remains and can manifest as non-deterministic cleanup/analysis outcomes when runs overlap.

### Fix Focus Areas
- src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[29-34]
- src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java[71-101]
- src/main/java/io/github/guacsec/trustifyda/providers/PythonProvider.java[228-257]

### Proposed fix options
- Prefer per-analysis isolation: create a unique temp directory for each venv (e.g., `Files.createTempDirectory("trustify_da_env_")`) and clean it up afterwards.
- Or serialize access: add a lock (e.g., a filesystem lock file in the env dir) around prepare/install/cleanup so only one run mutates the shared venv at a time.
- Additionally, consider caching a single controller per provider instance if shared venv semantics are intended (and still guard with locking).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

github-actions · 2026-03-26T11:28:57Z

Test Results

379 tests 379 ✅ 1m 36s ⏱️
25 suites 0 💤
25 files 0 ❌

Results for commit 1d6f4c5.

♻️ This comment has been updated with latest results.

src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java

qodo-code-review · 2026-03-27T07:48:09Z

CI Feedback 🧐

(Feedback updated until commit `1d6f4c5`)

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: call-shared / integration-tests (ubuntu-latest, gradle-groovy)
Failed stage: Run Integration Tests [❌]
Failed test name: stack_analysis validation (provider rhtpa, source osv-github transitive mismatch)
Failure summary: The action failed during integration test validation for stack analysis results: - The stack analysis validation reported a dependency count mismatch for `provider rhtpa`, `source osv-github`, `transitive`: expected `26`, got `27` (log line `15292`). - Because of this mismatch, the runner reported `Stack analysis validation failed` and exited with code `1` (log lines `15293-15294`).
Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 287: env: 288: TRUSTIFY_DA_DEV_MODE: true 289: TRUSTIFY_DA_BACKEND_URL: https://exhort.stage.devshift.net 290: PYTHONIOENCODING: utf-8 291: PYTHONUNBUFFERED: 1 292: pythonLocation: /opt/hostedtoolcache/Python/3.11.15/x64 293: PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib/pkgconfig 294: Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 295: Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 296: Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64 297: LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib 298: ##[endgroup] 299: + python -u shared-scripts/run_tests_no_runtime.py java artifact gradle-groovy 300: --- 301: Scenario: No runtime available 302: Description: It fails when no runtime is available 303: Manifest: /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/scenarios/gradle-groovy/simple/build.gradle 304: Expecting failure (no runtime available) 305: Executing: java -jar /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/artifact/cli.jar component /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/scenarios/gradle-groovy/simple/build.gradle 306: ✅ Command failed as expected (no runtime available) 307: Executing: java -jar /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/artifact/cli.jar stack /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/scenarios/gradle-groovy/simple/build.gradle 308: ✅ Command failed as expected (no runtime available) 309: Executing: java -jar /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/artifact/cli.jar stack /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/scenarios/gradle-groovy/simple/build.gradle --summary 310: ✅ Command failed as expected (no runtime available) 311: Executing: java -jar /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/artifact/cli.jar stack /home/runner/work/trustify-da-java-client/trustify-da-java-client/integration-tests/scenarios/gradle-groovy/simple/build.gradle --html 312: ✅ Command failed as expected (no runtime available) 313: --- ... 12556: "name": "Apache License 2.0", 12557: "isDeprecated": false, 12558: "isOsiApproved": true, 12559: "isFsfLibre": true, 12560: "category": "PERMISSIVE" 12561: } 12562: ], 12563: "expression": "Apache-2.0", 12564: "name": "Apache License 2.0", 12565: "category": "PERMISSIVE", 12566: "source": "deps.dev", 12567: "sourceUrl": "https://api.deps.dev" 12568: } 12569: ] 12570: }, 12571: "pkg:maven/com.google.errorprone/error_prone_annotations@2.10.0?scope=compile": { 12572: "concluded": { ... 15278: "direct": 4, 15279: "transitive": 9, 15280: "total": 13, 15281: "dependencies": 10, 15282: "critical": 1, 15283: "high": 4, 15284: "medium": 8, 15285: "low": 0, 15286: "remediations": 6, 15287: "recommendations": 119, 15288: "unscanned": 0 15289: } 15290: } 15291: } 15292: ❌ stack_analysis provider rhtpa source osv-github transitive mismatch: expected 26, got 27 15293: ❌ Stack analysis validation failed 15294: ##[error]Process completed with exit code 1. 15295: Post job cleanup.

qodo-code-review bot reviewed Mar 26, 2026

View reviewed changes

src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerVirtualEnv.java Show resolved Hide resolved

ruromero requested a review from soul2zimate March 26, 2026 14:45

soul2zimate approved these changes Mar 27, 2026

View reviewed changes

Merge branch 'main' into fix/venv-cleanup-race

1d6f4c5

soul2zimate enabled auto-merge (squash) March 27, 2026 07:46

soul2zimate merged commit 7d61ac1 into main Mar 27, 2026
31 of 40 checks passed

ruromero deleted the fix/venv-cleanup-race branch March 27, 2026 08:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(python): avoid FileAlreadyExistsException in venv cleanup#373

fix(python): avoid FileAlreadyExistsException in venv cleanup#373
soul2zimate merged 2 commits intomainfrom
fix/venv-cleanup-race

ruromero commented Mar 26, 2026 •

edited

Loading

Uh oh!

qodo-code-review bot commented Mar 26, 2026

Uh oh!

qodo-code-review bot commented Mar 26, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

qodo-code-review bot commented Mar 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ruromero commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

qodo-code-review bot commented Mar 26, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

github-actions bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

qodo-code-review bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Feedback 🧐

(Feedback updated until commit 1d6f4c5)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ruromero commented Mar 26, 2026 •

edited

Loading

qodo-code-review bot commented Mar 26, 2026 •

edited

Loading

github-actions bot commented Mar 26, 2026 •

edited

Loading

qodo-code-review bot commented Mar 27, 2026 •

edited

Loading

(Feedback updated until commit `1d6f4c5`)