Found while adding test coverage for scan_safety.py in #30. Not introduced there — the pattern predates the module extraction, which only moved it out of report.py.
The gap
scan_safety.is_safe_to_scan(filepath, project_root) validates a path and returns (True, ""). The caller then opens that path again, by name:
safe, reason = is_safe_to_scan(filepath, root_resolved)
if not safe:
continue
content = fpath.read_text(...) # second resolution — may not be the file that was checkedBetween the check and the read, filepath can be replaced with a symlink pointing outside project_root. The guard validated one inode; the read gets another. That defeats the symlink-escape protection the guard exists to provide.
Affects both callers: scripts/report.py and scripts/sbom.py (4 walkers).
Severity: low
Requires local write access to the tree being scanned, plus timing the swap inside a narrow window. Realistic exposure is a shared CI runner scanning an untrusted repository where an attacker also controls a concurrent process. Worth fixing on a defence-in-depth basis, not urgently.
Suggested fix
Resolve the check-then-use split by opening once and validating the descriptor:
os.open(path, os.O_RDONLY | os.O_NOFOLLOW) to refuse a symlink at open timeos.fstat(fd) for the size cap, so the size checked is the size read- containment verified against the opened descriptor, then read from
fd
This makes the guard and the read operate on the same inode rather than the same name. O_NOFOLLOW is POSIX; Windows needs a different approach, so the guard should degrade to current behaviour there rather than fail closed on a platform where it cannot be enforced.
Not to be handled by suppression
CodeQL flags lines 41 and 51 of scan_safety.py (py/path-injection), which is a different, false-positive finding — resolve() and stat() read no file content. Those alerts stay open and undismissed, per project policy. This issue is the real weakness at that site, and wants a code fix rather than any change to scanner configuration.
Verification when fixed
A regression test should reproduce the race deterministically rather than by timing — e.g. patch the guard to swap the file between check and read, and assert the out-of-root content never reaches the findings or the BOM. tests/test_scan_safety.py already covers the non-racing symlink-escape cases and is the natural home.
Found while adding test coverage for
scan_safety.pyin #30. Not introduced there — the pattern predates the module extraction, which only moved it out ofreport.py.The gap
scan_safety.is_safe_to_scan(filepath, project_root)validates a path and returns(True, ""). The caller then opens that path again, by name:Between the check and the read,
filepathcan be replaced with a symlink pointing outsideproject_root. The guard validated one inode; the read gets another. That defeats the symlink-escape protection the guard exists to provide.Affects both callers:
scripts/report.pyandscripts/sbom.py(4 walkers).Severity: low
Requires local write access to the tree being scanned, plus timing the swap inside a narrow window. Realistic exposure is a shared CI runner scanning an untrusted repository where an attacker also controls a concurrent process. Worth fixing on a defence-in-depth basis, not urgently.
Suggested fix
Resolve the check-then-use split by opening once and validating the descriptor:
os.open(path, os.O_RDONLY | os.O_NOFOLLOW)to refuse a symlink at open timeos.fstat(fd)for the size cap, so the size checked is the size readfdThis makes the guard and the read operate on the same inode rather than the same name.
O_NOFOLLOWis POSIX; Windows needs a different approach, so the guard should degrade to current behaviour there rather than fail closed on a platform where it cannot be enforced.Not to be handled by suppression
CodeQL flags lines 41 and 51 of
scan_safety.py(py/path-injection), which is a different, false-positive finding —resolve()andstat()read no file content. Those alerts stay open and undismissed, per project policy. This issue is the real weakness at that site, and wants a code fix rather than any change to scanner configuration.Verification when fixed
A regression test should reproduce the race deterministically rather than by timing — e.g. patch the guard to swap the file between check and read, and assert the out-of-root content never reaches the findings or the BOM.
tests/test_scan_safety.pyalready covers the non-racing symlink-escape cases and is the natural home.