[AUDIT] Automated code-review summary — mesh-review #4

New issue

Closed

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

foravo_admin commented

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

Owner

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

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

Automated Code-Review Audit — mesh-review

Date: 2026-05-02
Tool: GitHub Copilot CLI (automated analysis + fix)
Fix commit: 834f8d8441226ac383868a9cb8ac5c69b650581e
Fix PR: #1

All 7 findings have been fixed in PR #1 on branch fix/code-review-audit.

Finding Summary

#	Issue	Severity	File	Status
1	#2	HIGH	`summary/core.py`	Fixed in PR #1
2	#3	MEDIUM	`review/core.py`	Fixed in PR #1
3	#4	MEDIUM	`summary/core.py`	Fixed in PR #1
4	#5	MEDIUM	`review/falsify.py`	Fixed in PR #1
5	#6	MEDIUM	`review/falsify.py`	Fixed in PR #1
6	#7	MEDIUM	`cli.py`	Fixed in PR #1
7	#8	LOW	`install.sh`	Fixed in PR #1

Findings Detail

Finding 1 — HIGH: Format-string crash on diffs with curly braces (summary/core.py)

prompt.format(diff=diff) raises KeyError or ValueError whenever the diff contains {word} (git conflict markers, JSON, f-strings). This crashes summarization for all CLIs on a large class of common diffs.

Fix: prompt.replace("{diff}", diff) — literal substitution with no format-string semantics.

Finding 2 — MEDIUM: Subprocess exit silently discarded (review/core.py)

_shell_runner never checks proc.returncode. Auth errors, missing models, and rate limits are silently treated as empty LLM output, producing empty findings with no error indication.

Fix: Return ReviewResult(error=...) immediately when proc.returncode != 0.

Finding 3 — MEDIUM: Subprocess exit silently discarded (summary/core.py)

Same pattern as Finding 2, mirrored in summary/core.py:_shell_runner.

Fix: Return SummaryDoc(error=...) immediately when proc.returncode != 0.

Finding 4 — MEDIUM: Subprocess exit silently discarded (falsify.py)

make_subprocess_falsifier never checks proc.returncode. Non-zero exits fall through to _parse_falsifier_output(None) with a misleading "unparseable output" error rather than the actual CLI error.

Fix: Return early with the actual stderr content when proc.returncode != 0.

Finding 5 — MEDIUM: LLM-controlled values in str.format() — KeyError risk (falsify.py)

sigma_gate passes LLM-generated finding titles and bodies as kwargs to _FALSIFY_PROMPT.format(). Any finding about JSON, templates, or Python f-strings can contain {...} and crash the sigma gate for all findings.

Fix: Added _esc() helper that doubles all curly braces; applied to all LLM-controlled kwargs.

Finding 6 — MEDIUM: Unhandled FileNotFoundError on --diff-file (cli.py)

open(args.diff_file) has no exception handling. Missing or unreadable files produce a Python traceback instead of a clean error message.

Fix: Wrapped in try/except FileNotFoundError, OSError with clean stderr output and exit code 1.

Finding 7 — LOW: YAML injection in build_yaml() (install.sh)

User-supplied CLI names and command tokens are written into YAML without quoting or escaping. Names with : break YAML structure; tokens with " or \ break YAML string quoting.

Fix: All values now double-quoted in YAML with \ and " escaped via sed.

What Was Not Found

No shell injection: All subprocess calls use list-form subprocess.run() without shell=True — correct.
No path traversal: File paths are user-supplied but only used locally; no server-side risk.
No hardcoded secrets or credentials.
Dependency risk is low: Single runtime dependency (pyyaml); no network-facing components.

Imported from GitHub issue `M00C1FER/mesh-review#9`. Source: https://github.com/M00C1FER/mesh-review/issues/9 Original author: @M00C1FER Original state: closed  --- ## Automated Code-Review Audit — mesh-review **Date:** 2026-05-02 **Tool:** GitHub Copilot CLI (automated analysis + fix) **Fix commit:** `834f8d8441226ac383868a9cb8ac5c69b650581e` **Fix PR:** #1 All 7 findings have been **fixed** in PR #1 on branch `fix/code-review-audit`. --- ## Finding Summary | # | Issue | Severity | File | Status | |---|-------|----------|------|--------| | 1 | #2 | **HIGH** | `summary/core.py` | Fixed in PR #1 | | 2 | #3 | MEDIUM | `review/core.py` | Fixed in PR #1 | | 3 | #4 | MEDIUM | `summary/core.py` | Fixed in PR #1 | | 4 | #5 | MEDIUM | `review/falsify.py` | Fixed in PR #1 | | 5 | #6 | MEDIUM | `review/falsify.py` | Fixed in PR #1 | | 6 | #7 | MEDIUM | `cli.py` | Fixed in PR #1 | | 7 | #8 | LOW | `install.sh` | Fixed in PR #1 | --- ## Findings Detail ### Finding 1 — HIGH: Format-string crash on diffs with curly braces (summary/core.py) `prompt.format(diff=diff)` raises `KeyError` or `ValueError` whenever the diff contains `{word}` (git conflict markers, JSON, f-strings). This crashes summarization for all CLIs on a large class of common diffs. **Fix:** `prompt.replace("{diff}", diff)` — literal substitution with no format-string semantics. --- ### Finding 2 — MEDIUM: Subprocess exit silently discarded (review/core.py) `_shell_runner` never checks `proc.returncode`. Auth errors, missing models, and rate limits are silently treated as empty LLM output, producing empty findings with no error indication. **Fix:** Return `ReviewResult(error=...)` immediately when `proc.returncode != 0`. --- ### Finding 3 — MEDIUM: Subprocess exit silently discarded (summary/core.py) Same pattern as Finding 2, mirrored in `summary/core.py:_shell_runner`. **Fix:** Return `SummaryDoc(error=...)` immediately when `proc.returncode != 0`. --- ### Finding 4 — MEDIUM: Subprocess exit silently discarded (falsify.py) `make_subprocess_falsifier` never checks `proc.returncode`. Non-zero exits fall through to `_parse_falsifier_output(None)` with a misleading "unparseable output" error rather than the actual CLI error. **Fix:** Return early with the actual stderr content when `proc.returncode != 0`. --- ### Finding 5 — MEDIUM: LLM-controlled values in str.format() — KeyError risk (falsify.py) `sigma_gate` passes LLM-generated finding titles and bodies as kwargs to `_FALSIFY_PROMPT.format()`. Any finding about JSON, templates, or Python f-strings can contain `{...}` and crash the sigma gate for all findings. **Fix:** Added `_esc()` helper that doubles all curly braces; applied to all LLM-controlled kwargs. --- ### Finding 6 — MEDIUM: Unhandled FileNotFoundError on --diff-file (cli.py) `open(args.diff_file)` has no exception handling. Missing or unreadable files produce a Python traceback instead of a clean error message. **Fix:** Wrapped in `try/except FileNotFoundError, OSError` with clean stderr output and exit code 1. --- ### Finding 7 — LOW: YAML injection in build_yaml() (install.sh) User-supplied CLI names and command tokens are written into YAML without quoting or escaping. Names with `:` break YAML structure; tokens with `"` or `\` break YAML string quoting. **Fix:** All values now double-quoted in YAML with `\` and `"` escaped via `sed`. --- ## What Was Not Found - **No shell injection:** All subprocess calls use list-form `subprocess.run()` without `shell=True` — correct. - **No path traversal:** File paths are user-supplied but only used locally; no server-side risk. - **No hardcoded secrets or credentials.** - **Dependency risk is low:** Single runtime dependency (`pyyaml`); no network-facing components.

foravo_admin

2026-05-19 04:42:46 +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#9:4362220547.

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

Fixed by merged PR #1.

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