Skip to content

fix(tests): hardened-YAML guard skips a tree-root virtualenv by path segment (#2890) - #2895

Merged
vybe merged 1 commit into
devfrom
fix/2890-yaml-guard-venv-exclusion
Sep 20, 2026
Merged

vybe merged 1 commit into
devfrom
fix/2890-yaml-guard-venv-exclusion

Conversation

@dolho

@dolho dolho commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • test_no_service_parses_yaml_without_the_shared_loader excluded virtualenvs with "/venv/" in rel on a path relative to the scanned root, so a venv at the root of the tree (src/backend/venv/ — where one actually lives) yielded venv/lib/..., never matched, and vendored starlette/uvicorn safe_load calls were reported as unguarded parses on a clean checkout.
  • The scan is now a pure helper _bare_safe_load_offenders(roots) whose exclusion matches path.relative_to(root).parts against _VENDORED_PARTS = {venv, .venv, site-packages, node_modules, __pycache__} — position-independent; a directory merely named like one (my-venv-tool/) is still scanned.
  • git ls-files (the issue's alternative) rejected: it would hide an untracked first-party file from the pre-commit scan.

Acceptance criteria

  • A virtualenv at the root of a scanned tree is excluded — fixture test (root venv/, .venv/) + proven on the real tree with a planted src/backend/venv/.../starlette/mod.py
  • The guard still fails on a genuine unguarded yaml.safe_load in first-party code — planted services/_tmp_bare.py → red; fixture test asserts svc.py is the only offender beside a vendored twin
  • Segment-based, not substring — test_2890_exclusion_is_segment_based_not_substring_based
  • .venv, site-packages, node_modules (and __pycache__) covered, root and nested

Sibling audit

The other 34 rglob("*.py") guards under tests/unit/ were run with the same planted root venv (bare safe_load, a SETNX lock, a 404 raise): 683 passed, only this guard false-fired. test_1920 already excludes venv/ via startswith.

Test Plan

  • cd tests && pytest unit/test_ent314_hardened_yaml.py unit/test_1965_agent_server_safe_yaml.py -v — 43 passed
  • Both directions proven on the real tree (see above); fixtures removed
  • lint_sys_modules.py — no new violations

Fixes #2890

🤖 Generated with Claude Code

…segment (#2890)

`test_no_service_parses_yaml_without_the_shared_loader` excluded virtualenvs
with `"/venv/" in rel`, where `rel` is RELATIVE to the scanned root — so a
venv at the root of that tree (`src/backend/venv/`, i.e. where one actually
lives) yields `venv/lib/...` with no leading slash, never matches, and every
vendored `safe_load` in `site-packages` is reported as an unguarded parse on
a clean checkout. Only a venv nested a directory deep was ever excluded.

The scan is now `_bare_safe_load_offenders(roots)`, pure over the filesystem,
with the exclusion matched on `path.relative_to(root).parts` against
`_VENDORED_PARTS` ({venv, .venv, site-packages, node_modules, __pycache__})
— position-independent, and a directory merely NAMED like one
(`my-venv-tool/`) is still scanned. `git ls-files` was rejected: it would
hide an untracked first-party file from the pre-commit scan.

Proven both ways on the real tree: a planted
`src/backend/venv/lib/python3.12/site-packages/starlette/mod.py` with a bare
parse no longer fails the guard, and a planted `services/_tmp_bare.py` still
does. The new fixture tests pin both directions across root/nested/.venv/
node_modules/__pycache__ placements. The other 34 `rglob("*.py")` guards
under tests/unit were audited with the same planted venv: none false-fires.

Fixes #2890

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: batch validated on train/20260920-1811 (#2913)

@vybe
vybe merged commit 693fb70 into dev Sep 20, 2026
24 checks passed
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.

2 participants