Skip to content

fix: review-pass findings + reviewer standards doc - #15

Merged
SuperElectron merged 1 commit into
stagingfrom
fix/review-findings
Aug 27, 2026
Merged

fix: review-pass findings + reviewer standards doc#15
SuperElectron merged 1 commit into
stagingfrom
fix/review-findings

Conversation

@SuperElectron

Copy link
Copy Markdown
Owner

Full review pass over staging (ruff/mypy/pytest already green; bandit + manual on top). 0 CRITICAL, 3 HIGH, 4 MEDIUM — all fixed here.

Sev Finding Fix
HIGH safe_eval computed True + 1 (bool subclasses int) and crashed with RecursionError on hostile nesting bool excluded explicitly; MAX_DEPTH guard → clean ValueError; both tested
HIGH sandbox used assert as a runtime file check (stripped under -O) real pre-flight FileNotFoundError; timeout path now keeps stderr
HIGH pickle memento shipped with no trust-boundary warning (bandit B301 / CWE-502) module docstring warning + README caveat + inline note
MED MCP server did disk I/O at import time — violating our own global_object unit lru_cache lazy accessors
MED facade rollback silent about its post-charge boundary; singleton/flyweight silent about thread-safety honest inline notes

Also adds docs/code-review.md — the reviewer's contract: machine-enforced layer (ruff/mypy/pytest/bandit), written standards (PEP 8/257/484, Google guide), severity checklist, and MCP reference sources (this repo's own server + Context7) for a review team.

Gates: ruff ✅ mypy --strict ✅ pytest 238 ✅ README table current ✅

🤖 Generated with Claude Code

…server

From a full review pass (bandit + manual, report in PR):

HIGH
- interpreter/real_world.py: safe_eval now rejects bool constants
  (bool subclasses int; True + 1 no longer computes) and depth-limits
  nesting so hostile input gets ValueError, not RecursionError
- sandbox.py: assert path.is_file() replaced with a real pre-flight
  FileNotFoundError check (asserts vanish under python -O); timeout
  path now preserves captured stderr
- memento/real_world.py: explicit CWE-502 warning — pickle.loads only
  on snapshots this process produced; README caveat added

MEDIUM
- server.py: catalog/index now lazy via lru_cache accessors — imports
  do no disk I/O, practicing what patterns/python/global_object teaches
- facade/singleton/flyweight pythonic variants: honest inline notes on
  the saga boundary, unguarded lazy init, and unbounded pool

docs/code-review.md: reviewer standards — tool layer, written
standards, severity checklist, MCP reference sources.

238 tests (2 new hostile-input tests); ruff/mypy --strict clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SuperElectron
SuperElectron merged commit fd800cd into staging Aug 27, 2026
3 checks passed
@SuperElectron
SuperElectron deleted the fix/review-findings branch August 27, 2026 21:16
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