[MEDIUM] review/core.py: _shell_runner silently discards non-zero subprocess exit #10

New issue

Closed

opened 2026-05-19 04:42:47 +00:00 by foravo_admin · 1 comment

foravo_admin commented

2026-05-19 04:42:47 +00:00

Owner

Imported from GitHub issue M00C1FER/mesh-review#3.

Source: https://github.com/M00C1FER/mesh-review/issues/3
Original author: @M00C1FER
Original state: closed

Finding 2 — MEDIUM

File: src/mesh_review/review/core.py
Function: _shell_runner()
Commit fixing this: 834f8d8441226ac383868a9cb8ac5c69b650581e (PR #1)

Description

After subprocess.run() returns, proc.returncode is never checked:

proc = subprocess.run(
    cmd + [full_prompt], capture_output=True, text=True, timeout=timeout
)
# returncode never checked — error output treated as LLM output
out = proc.stdout
findings = _parse_findings(out, ...)

If the CLI process exits non-zero (authentication failure, missing model, rate-limit, network error), proc.stderr contains the error and proc.stdout is empty or contains garbage, which is then fed to _parse_findings. This silently produces an empty or corrupted ReviewResult with no indication anything went wrong.

Impact

Silent failure: auth errors, model-not-found errors, and rate limits produce empty findings with no error field set.
Operators have no way to distinguish a clean empty review from a broken CLI invocation.

Fix Applied

if proc.returncode != 0:
    return ReviewResult(cli=cli, findings=[], raw_output=proc.stdout,
                        error=f"{cli} exited {proc.returncode}: {proc.stderr.strip()[:300]}")

Imported from GitHub issue `M00C1FER/mesh-review#3`. Source: https://github.com/M00C1FER/mesh-review/issues/3 Original author: @M00C1FER Original state: closed  --- ## Finding 2 — MEDIUM **File:** `src/mesh_review/review/core.py` **Function:** `_shell_runner()` **Commit fixing this:** 834f8d8441226ac383868a9cb8ac5c69b650581e (PR #1) ### Description After `subprocess.run()` returns, `proc.returncode` is never checked: ```python proc = subprocess.run( cmd + [full_prompt], capture_output=True, text=True, timeout=timeout ) # returncode never checked — error output treated as LLM output out = proc.stdout findings = _parse_findings(out, ...) ``` If the CLI process exits non-zero (authentication failure, missing model, rate-limit, network error), `proc.stderr` contains the error and `proc.stdout` is empty or contains garbage, which is then fed to `_parse_findings`. This silently produces an empty or corrupted `ReviewResult` with no indication anything went wrong. ### Impact - Silent failure: auth errors, model-not-found errors, and rate limits produce empty findings with no error field set. - Operators have no way to distinguish a clean empty review from a broken CLI invocation. ### Fix Applied ```python if proc.returncode != 0: return ReviewResult(cli=cli, findings=[], raw_output=proc.stdout, error=f"{cli} exited {proc.returncode}: {proc.stderr.strip()[:300]}") ```

foravo_admin

2026-05-19 04:42:47 +00:00

closed this issue
added the
code-review
label

foravo_admin commented

2026-05-19 04:42:47 +00:00

Author

Owner

Imported from GitHub issue comment M00C1FER/mesh-review#3:4362220369.

Source: https://github.com/M00C1FER/mesh-review/issues/3#issuecomment-4362220369
Original author: @M00C1FER

Fixed by merged PR #1.

Imported from GitHub issue comment `M00C1FER/mesh-review#3:4362220369`. Source: https://github.com/M00C1FER/mesh-review/issues/3#issuecomment-4362220369 Original author: @M00C1FER  --- Fixed by merged PR #1.