mirror of
https://github.com/guillaumemeyer/watermarks-remover.git
synced 2026-08-22 13:11:57 +02:00
fix: an unreadable text file is a failed scan, not a clean one (#169)
* fix: an unreadable text file is a failed scan, not a clean one scan_file's text branch caught the OSError and returned an item with an error key and no confidence key — is_actionable read that as clean, so a file the scanner could not open was indistinguishable from one it opened and found clean. All three audit callers inherited the lie: the pre-commit hook returned 0, and audit_dir's _scan_worker never produced the EXIT_PARTIAL signal its try/except exists for, because scan_file swallowed the error before it could raise (#158; the same probe-did-not-answer class as #155). Let the OSError propagate: the audit_dir/audit_website wrappers already convert a raised exception into a files_skipped entry with EXIT_PARTIAL, which is exactly the contract common.py documents for a partial scan. * chore: satisfy ruff on the unreadable-text scan fix - drop the unused OSError binding (F841) in scan_file's text branch - sort the test's import block (I001): stdlib first, then audit_lib --------- Co-authored-by: yzxcj797 <yzxcj797@users.noreply.github.com> Co-authored-by: Guillaume Meyer (The Opinionated Man) <1385518+guillaumemeyer@users.noreply.github.com> Co-authored-by: guillaumemeyer <guillaumemeyer@users.noreply.github.com>
This commit is contained in:
co-authored by
yzxcj797
Guillaume Meyer
guillaumemeyer
parent
3d2bac7720
commit
546a9f1576
@@ -47,8 +47,16 @@ def scan_file(
|
||||
if kind == "text":
|
||||
try:
|
||||
text = path.read_text(encoding="utf-8", errors="surrogateescape")
|
||||
except OSError as e:
|
||||
return {"path": name, "kind": "text", "error": str(e)}
|
||||
except OSError:
|
||||
# A file that could not be read is a FAILED SCAN, not a clean one:
|
||||
# the old swallowed-error item carried no confidence key, so
|
||||
# is_actionable answered False and every caller recorded the file
|
||||
# as scanned-and-clean (the pre-commit hook returned 0; audit_dir
|
||||
# never produced the EXIT_PARTIAL signal its wrapper exists for).
|
||||
# Let the OSError propagate: audit_dir's _scan_worker and
|
||||
# audit_website's wrapper already convert a raised exception into
|
||||
# a files_skipped entry with EXIT_PARTIAL (#158).
|
||||
raise
|
||||
report = inspect_text(text)
|
||||
findings, confidences, suspicious = text_findings(report)
|
||||
item: dict[str, Any] = {
|
||||
|
||||
@@ -463,3 +463,37 @@ def test_main_rejects_private_base_cleanly(monkeypatch, capsys):
|
||||
err = capsys.readouterr().err
|
||||
assert "invalid base URL" in err
|
||||
assert "non-public address" in err
|
||||
|
||||
|
||||
def test_unreadable_text_file_is_a_failed_scan_not_clean(tmp_path, monkeypatch):
|
||||
"""A text file the scanner cannot open must surface as skipped/partial,
|
||||
never as scanned-and-clean (#158)."""
|
||||
from pathlib import Path
|
||||
|
||||
import audit_lib
|
||||
|
||||
target = tmp_path / "notes.txt"
|
||||
target.write_text("hello", encoding="utf-8")
|
||||
|
||||
def _deny(self, *args, **kwargs):
|
||||
raise OSError(13, "Permission denied")
|
||||
|
||||
monkeypatch.setattr(Path, "read_text", _deny)
|
||||
|
||||
# scan_file raises instead of returning a confidence-less "clean" item.
|
||||
try:
|
||||
audit_lib.scan_file(target)
|
||||
except OSError:
|
||||
pass
|
||||
else:
|
||||
raise AssertionError("scan_file must propagate the read failure")
|
||||
|
||||
# The old swallowed-error item shape must never count as actionable.
|
||||
assert audit_lib.is_actionable({"path": "x.txt", "kind": "text", "error": "denied"}) is False
|
||||
|
||||
# audit_dir's worker converts the raise into a skipped entry.
|
||||
import audit_dir
|
||||
|
||||
item, skipped = audit_dir._scan_worker(target, False)
|
||||
assert item is None
|
||||
assert skipped is not None and "Permission denied" in skipped.get("reason", "")
|
||||
|
||||
Reference in New Issue
Block a user