mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(skillscan): gate the directory scan file size before reading (#6359)
* fix(skillscan): gate the directory scan file size before reading scan_skill_dir() read every candidate file with read_bytes() before any size check; MAX_FILE_BYTES was only recorded as a finding afterwards. Measured: one 300MB file (4.7x the 64MB limit) drives a 600MB peak allocation and a 20s scan, linearly extrapolating to an OOM for larger planted files - reachable via review/analyzer.py on content that never passed the archive preflight gates. Gate with stat() first and bound oversized reads to MAX_FILE_BYTES + 1, mirroring _read_archive_member so both entry points share the same discipline. Regression test plants an oversized file and fails if read_bytes is used on it. Closes #6358 * fix(skillscan): collapse to a single bounded read per file Per review: Path.open + read(MAX_FILE_BYTES + 1) returns the whole file when it is smaller than the limit, identical to read_bytes for regular files - removing the size-conditional branch and closing the stat-to-read race where a file growing past the limit between calls would still be read unbounded. file_size from stat stays the source of truth for the package-oversized-file evidence.
This commit is contained in:
parent
a9b1675923
commit
3725ba1bfb
@ -270,12 +270,22 @@ def scan_skill_dir(skill_dir: Path) -> ScanResult:
|
||||
for path in sorted(candidate for candidate in root.rglob("*") if candidate.is_file()):
|
||||
rel_path = _relative_file(path, root)
|
||||
try:
|
||||
file_bytes = path.read_bytes()
|
||||
file_size = path.stat().st_size
|
||||
except OSError as e:
|
||||
scanner_errors.append(f"{rel_path}: failed to stat file: {e}")
|
||||
continue
|
||||
try:
|
||||
# One bounded read for every file: files smaller than MAX_FILE_BYTES
|
||||
# come back whole, oversized ones are truncated and recorded below —
|
||||
# no unbounded read regardless of growth between stat() and here
|
||||
# (mirrors _read_archive_member).
|
||||
with path.open("rb") as handle:
|
||||
file_bytes = handle.read(MAX_FILE_BYTES + 1)
|
||||
except OSError as e:
|
||||
scanner_errors.append(f"{rel_path}: failed to read file: {e}")
|
||||
continue
|
||||
|
||||
findings.extend(_scan_file_package_properties(rel_path, file_bytes, path.stat().st_size))
|
||||
findings.extend(_scan_file_package_properties(rel_path, file_bytes, file_size))
|
||||
text = _decode_text_for_analysis(file_bytes)
|
||||
if text is None:
|
||||
text = _decode_script_lossily(rel_path, file_bytes)
|
||||
|
||||
@ -13,7 +13,7 @@ import pytest
|
||||
from deerflow.skills.package_files import is_executable_binary_prefix
|
||||
from deerflow.skills.security_scanner import scan_skill_content
|
||||
from deerflow.skills.skillscan import StaticScanBlockedError, enforce_static_scan, scan_archive_preflight, scan_skill_dir
|
||||
from deerflow.skills.skillscan.orchestrator import _PYTHON_CLIENT_SINK_METHODS
|
||||
from deerflow.skills.skillscan.orchestrator import _PYTHON_CLIENT_SINK_METHODS, MAX_FILE_BYTES
|
||||
|
||||
_FINDING_FIELDS = {"rule_id", "severity", "file", "line", "message", "remediation", "evidence"}
|
||||
|
||||
@ -1812,6 +1812,30 @@ def test_secret_assignment_survives_nul_byte_in_python(tmp_path: Path) -> None:
|
||||
assert finding["file"] == "scripts/sample.py"
|
||||
|
||||
|
||||
def test_scan_dir_bounds_oversized_file_read(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""`scan_skill_dir` must gate the size BEFORE reading: an oversized file is
|
||||
recorded as a finding and scanned only through the bounded read, never via
|
||||
`read_bytes` (mirroring `_read_archive_member`'s gate-then-bounded-read)."""
|
||||
skill_dir = tmp_path / "skill"
|
||||
_write_skill(skill_dir)
|
||||
blob = skill_dir / "blob.bin"
|
||||
with blob.open("wb") as handle:
|
||||
handle.seek(300 * 1024 * 1024 - 1)
|
||||
handle.write(b"\0")
|
||||
|
||||
real_read_bytes = Path.read_bytes
|
||||
|
||||
def guarded_read_bytes(self: Path) -> bytes:
|
||||
if self.stat().st_size > MAX_FILE_BYTES:
|
||||
raise AssertionError("read_bytes used on an oversized file")
|
||||
return real_read_bytes(self)
|
||||
|
||||
monkeypatch.setattr(Path, "read_bytes", guarded_read_bytes)
|
||||
|
||||
findings = scan_skill_dir(skill_dir)["findings"]
|
||||
assert _finding_by_rule(findings, "package-oversized-file")
|
||||
|
||||
|
||||
def test_bundled_public_skill_scripts_report_no_secret_assignment() -> None:
|
||||
"""Bundled skill scripts must not fail the review gate on an unchanged checkout (#4996).
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user