soup/soup_cli/monitoring
Alpamys ff55e751ab fix(v0.33.0): review-wave findings (CRITICAL + HIGH + MEDIUM + LOW)
Addresses findings from 5-agent review wave (python-reviewer,
code-reviewer, security-reviewer, tdd-guide, smoke-verification).

CRITICAL:
- cans/run.py _deploy_target ollama path: rglob *.gguf result is now
  realpath+commonpath checked against extract_dir before forwarding to
  `soup deploy ollama --gguf`. Prevents a crafted symlink in the can
  from making rglob point at an arbitrary on-disk path.

HIGH:
- cans/publish.py: removed dead update_repo_settings + bare-except tag
  block (was a no-op network round-trip). Tag attachment via README
  front-matter is documented as a v0.33.x docs follow-up.
- registry/attach.py lookup_entry_by_output_dir: emits ResourceWarning
  when the 1000-row scan limit is hit (was a silent miss).
- data/collators.py CrossDocCollator: stops mutating input dicts via
  pop() — uses get + dict comprehension. HF Dataset rows are cached and
  reused; mutation broke subsequent batches silently. Bare-except now
  logs at DEBUG level so production degradation is inspectable.
- monitoring/callback.py _write_spike_recovery_hint: added is_under_cwd
  guard. args.output_dir came from raw HF TrainingArguments without
  separate path-containment check.
- trainer/rewards.py MACOS_SANDBOX_PROFILE: narrowed (allow mach-lookup)
  to a 3-name allowlist (SecurityServer, notification_center,
  opendirectoryd.libinfo). Broad mach-lookup permitted DNS / NSURLSession
  via launchd, defeating (deny network*).
- cans/run.py: PermissionError → ValueError so a caller wrapping in
  `except OSError` cannot silently swallow the consent gate.
  PermissionError is an OSError subclass.
- commands/can.py run_cmd: assigns result=None up front + explicit None
  guard so a future _fail bypass cannot trigger NameError on result.
- utils/v028_features.py: added type annotations on apply_v028_speed_memory
  (model: Any, tcfg: TrainingConfig via TYPE_CHECKING, console: Console)
  and warn_unsupported_features.
- cans/run.py: confirm_callback now annotated
  Callable[[Manifest], bool] for IDE introspection.
- tests/test_part_b.py reexec test: drops env-var contamination
  (RANK/WORLD_SIZE/LOCAL_RANK/ACCELERATE_*) before run, patches
  imported names on train module, and forces assertion that
  os.execvp was called — no more silent skip-on-bypass.
- tests/test_part_d.py: added TestGenerateResponseSignature
  source-level guard that catches the lenient logits_processor mock
  silently passing.

MEDIUM:
- cans/run.py _run_subprocess: catches subprocess.TimeoutExpired and
  returns rc=124 (coreutils convention) so callers see a clean
  CanRunResult instead of an unhandled traceback after the 24h cap.
- cans/run.py: temp dir created via mkdtemp is now cleaned up on
  extract_can failure (try/except + cleanup_extract_dir).
- cans/run.py cleanup_extract_dir: switched startswith path check to
  os.path.commonpath (project-standard idiom; Windows-safe).
- cans/schema.py DeployTarget._safe_relpath: normalises mixed
  separators before splitting on '/' so foo/..\bar can no longer
  bypass the .. check.
- utils/lr_finder.py run_lr_sweep: removed redundant local
  `import math as _math` (math already at module level).

LOW:
- eval/gate.py _parse_judge_url: removed bare http:// catchall after
  scheme allowlist. Defence-in-depth for callers that bypass the
  Pydantic GateTask validator.
- utils/auto_quant.py evaluate_candidate: latency mean now divides by
  *completed* prompts (excludes crashed). Crashed candidate no longer
  appears artificially fast.
- utils/auto_quant.py Candidate.__post_init__: explicitly rejects bool
  in score / latency_ms (bool is a subclass of int, was sneaking past).
- utils/mii.py: removed `noqa: F401` on Optional import (now actually
  used in type annotation since we restored it).

Tests added (+7, total 3811→3818):
- test_part_a_wave1: attach_artifact outside-cwd rejection.
- test_part_a_wave2: PermissionError→ValueError migration in 2 tests.
- test_part_c: CrossDocCollator mismatched doc_lengths fallback,
  does-not-mutate-input-dict regression guard.
- test_part_d: source-level _generate_response signature guard.
- test_part_e: should_recover at max_attempts, outside-cwd skip.

Lint: clean. Full suite: 3818 passed in 156s.

Findings deliberately not actioned (with rationale):
- code-review M1 (mii Pydantic at import-time): forward-ref resolution
  requires module-level definitions for FastAPI; documented in mii.py.
- code-review M4 (supports_v028_features vs validator divergence):
  the v0.33.0 schema validator was renamed to
  _validate_v028_speed_memory_supported_tasks and now imports
  supports_v028_features — they cannot drift.
- python-review LOW (_deploy_target vllm silent no-op): documented in
  the docstring as advisory; logging requires a console arg the
  helper does not currently take.
- security-review LOW 8/9 (TOCTOU window, CLONE_NEWPID): theoretical;
  documented in CLAUDE.md security section in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-27 19:57:57 +05:00
..
__init__.py Initial project setup: CLI skeleton + config + trainer + data pipeline 2026-02-20 16:14:56 +05:00
callback.py fix(v0.33.0): review-wave findings (CRITICAL + HIGH + MEDIUM + LOW) 2026-04-27 19:57:57 +05:00
display.py feat: v0.24.0 — Dataset Hub, Freeze Training, Loss Watchdog, Dataset Registry 2026-04-03 16:35:23 +05:00
hf_push.py feat(hf): v0.29.0 — HuggingFace Hub Deep Integration 2026-04-23 16:17:28 +05:00