Skip to content

fix: close the ten code-review findings (v0.2.1) - #11

Merged
Chirudeva-Reddy merged 1 commit into
mainfrom
fix/review-findings
Sep 24, 2026
Merged

Chirudeva-Reddy merged 1 commit into
mainfrom
fix/review-findings

Conversation

@Chirudeva-Reddy

Copy link
Copy Markdown
Owner

Every fix has a regression test in tests/regressions/test_issue_.py that failed first
(27 red + 1 collection error before this change).

Detection

  • 17/18: shell evasion. sh -c '...', bash -lc, eval scripts are re-parsed (3 levels), and
    wrapper programs are skipped together with their own flags (sudo -u root, env -i, nice -n 10,
    timeout 5, stdbuf, ionice, ...). Benign cases (git rm -r --cached, sudo apt list) stay clean.
  • 19: scheme-less SSRF. An argument that is only an address (169.254.169.254/latest,
    localhost:6379, //10.0.0.1/x, [::1]:8080) is treated as a network target. Prose mentioning an IP
    and small numbers (42/7) stay clean.
  • 20: tool output is scanned in overlapping 64 KB chunks up to 1 MB (1 MB takes about 127 ms);
    untrusted output beyond that fails closed; taint fingerprints cover the head and tail.
  • 21: base64 decoding has no candidate cap; the decoded text is joined and scanned once.

Integrity

  • 22: verify_integrity catches up with other writers and reads under the writers' lock; the
    reader never consumes a partially written line. Fixes the false "truncated" health alarm.
  • 23: keys are written to a temp file and link()ed into place (atomic, never overwritten); an
    empty or short key file is refused instead of silently used.

Coverage and structure

  • 24: POST /api/v1/results (agent key) runs tool output through the guard, so HTTP agents get
    spotlighting and taint tracking too.
  • 25: one fold_tool_name() used by normalize, the policy, approval digests, the gateway and
    the MCP proxy (the policy previously skipped NFKC).
  • 16: moved the duplicate-entry-point regression test to tests/regressions as AGENTS.md requires.

Corpus: +6 attacks (nested shell, eval, sudo flags, timeout, 2 scheme-less SSRF), +5 benign
near-misses. 48 attacks: 100% flagged, 90% stopped on detector evidence; 0/55 false positives.
No existing snapshot row changed.

Every fix has a regression test in tests/regressions/test_issue_<n>.py that failed first
(27 red + 1 collection error before this change).

Detection
- 17/18: shell evasion. `sh -c '...'`, `bash -lc`, `eval` scripts are re-parsed (3 levels), and
  wrapper programs are skipped together with their own flags (sudo -u root, env -i, nice -n 10,
  timeout 5, stdbuf, ionice, ...). Benign cases (git rm -r --cached, sudo apt list) stay clean.
- 19: scheme-less SSRF. An argument that is only an address (169.254.169.254/latest,
  localhost:6379, //10.0.0.1/x, [::1]:8080) is treated as a network target. Prose mentioning an IP
  and small numbers (42/7) stay clean.
- 20: tool output is scanned in overlapping 64 KB chunks up to 1 MB (1 MB takes about 127 ms);
  untrusted output beyond that fails closed; taint fingerprints cover the head and tail.
- 21: base64 decoding has no candidate cap; the decoded text is joined and scanned once.

Integrity
- 22: verify_integrity catches up with other writers and reads under the writers' lock; the
  reader never consumes a partially written line. Fixes the false "truncated" health alarm.
- 23: keys are written to a temp file and link()ed into place (atomic, never overwritten); an
  empty or short key file is refused instead of silently used.

Coverage and structure
- 24: POST /api/v1/results (agent key) runs tool output through the guard, so HTTP agents get
  spotlighting and taint tracking too.
- 25: one fold_tool_name() used by normalize, the policy, approval digests, the gateway and
  the MCP proxy (the policy previously skipped NFKC).
- 16: moved the duplicate-entry-point regression test to tests/regressions as AGENTS.md requires.

Corpus: +6 attacks (nested shell, eval, sudo flags, timeout, 2 scheme-less SSRF), +5 benign
near-misses. 48 attacks: 100% flagged, 90% stopped on detector evidence; 0/55 false positives.
No existing snapshot row changed.
@Chirudeva-Reddy
Chirudeva-Reddy merged commit 81b9aea into main Sep 24, 2026
8 checks passed
@Chirudeva-Reddy
Chirudeva-Reddy deleted the fix/review-findings branch September 24, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant