Revival: explainable verdicts + Windows-safe reporting - #5
Merged
Conversation
…nchecked files safe v0.2.0 reported `Safe` for files it had not managed to inspect: an oversized file, a file it could not `stat`, and a `zipfile.BadZipFile` were all swallowed and reported as clean. Eleven of twenty-two structurally risky corpus samples came back Safe, including ZIP path traversal, a 48 KB → 48 MB bomb, an encrypted archive, and a PDF with a ZIP appended after %%EOF. For a tool a teacher trusts with a folder of student submissions, "I did not look" reported as "this is fine" is the worst failure it can have. The scoring was equally indefensible: an unsourced weight table that double- and triple-counted a single fact (one macro produced four findings and saturated at 100) and escalated a twenty-link bibliography to High. What replaces it: - scanner/findings.py, verdict.py — severity and confidence as separate axes, and a verdict from four stated rules. COULD_NOT_INSPECT is a first-class outcome that can never round down to safe. The risk score is capped per finding code and used only for sorting. - scanner/detectors/ — archive, office, pdf, image and general detectors replacing heuristics.py and the four *_rules modules. - scanner/limits.py — explicit bounds on size, archive members, nesting depth, total uncompressed bytes and compression ratio. - scanner/quarantine.py — a JSONL manifest, 0600/0700, hash-verified restore, and refusal to follow symlinks. - scanner/reporters.py — a self-contained HTML report with no script and no network, and sanitize_display() to neutralise the bidi and zero-width characters an attacker uses to make `invoice<U+202E>gpj.exe` read as a JPEG. The report was rendering the attack as its own disguise. - scanner/gui_model.py + gui.py — the Tk-free model is tested; PySimpleGUI is gone. Defects found and fixed while building this, each with a regression test: every valid PNG was flagged (the IEND CRC was not counted); every real .docx was flagged (a settings.xml rule); /EmbeddedFile was missed when it straddled the 4096-byte read boundary; a symlink to /dev/zero never finished; quarantining two files of the same name destroyed the first; a mistyped scan path exited 0; and .rar/.7z/.tar/.rtf returned LIKELY_SAFE rather than COULD_NOT_INSPECT. Deleting ruff.toml activated the stricter pyproject config, which surfaced a real B023 loop-variable closure in office.py and a B904 in quarantine.py. Verified on Linux/CPython 3.11: pytest 117 passed 1 skipped; ruff clean over scanner tests examples scripts; mypy clean over 19 source files; corpus gate 31 samples, 15 correctly blocked, 9 correctly clean, no false positives. Not verified: the PyInstaller binary, Windows, macOS, the Tk window, and the optional YARA path. See docs/REVIVAL_AUDIT.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CKVgvkabikjdEvThAqweov
…the suite on Windows The revived console report wrote Unicode glyphs (X-mark, check, bullet, ellipsis) straight to the output stream, which raises UnicodeEncodeError on a default Windows cp1252 console -- the scan aborted right after the headline. Windows teachers are the tool's primary audience and CI runs windows-latest, so this matters. The durable JSON/HTML reports were already UTF-8 and unaffected. - scanner/reporters.py: choose ASCII stand-ins (X ! ? +) per output stream when it cannot encode the glyphs, and wrap the writer so no filename the console cannot encode can abort a scan. UTF-8 terminals and the HTML report keep the nicer symbols. - tests: skip the genuinely POSIX-only checks on Windows (symlink creation, os.geteuid, '<>' in a filename) via a small tests/_platform.py capability probe -- symlink tests still run wherever symlinks are permitted -- and compare dropped-path parsing with as_posix() so it holds on both path separators. Verified on Windows/CPython 3.11: 112 passed, 6 skipped, 0 failed (was 109 passed, 8 failed); ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ4pLh3eqru38tYbNXbxty
…ndows CI) The "runs with zero dependencies" step hardcoded /tmp/bare, /tmp/samples and /tmp/r.json. On windows-latest bash and Python resolve /tmp differently, so the scan wrote its report where the assertion could not find it -> FileNotFoundError and a red Windows job (the scanner itself ran fine — see the verdict table in the log). Switch to relative paths (.ci-bare / .ci-samples / ci-report.json), which resolve identically for both on every runner. Verified locally: scan exit 2, report written, counts.do_not_open == 15 (> 10). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ4pLh3eqru38tYbNXbxty
… binary runs The PyInstaller spec freezes scanner/__main__.py as the top-level __main__ script, which has no parent package, so `from .main import main` raised "ImportError: attempted relative import with no known parent package" and every standalone-binary job (macOS/Linux/Windows) failed the moment it ran the built exe. (It had been masked because the job depends on the test job, which was red.) Switch to `from scanner.main import main`, which works both under `python -m scanner` and when frozen. Verified locally: `python -m scanner --help` exit 0; built the spec with PyInstaller and ran dist/teacher-safe-scan.exe --help -> exit 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XJ4pLh3eqru38tYbNXbxty
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Revival: explainable verdicts + Windows-safe reporting
This branch revives the scanner after months of inactivity. It was chosen by an
audit of two independent overnight "go all in" rewrites (one by ChatGPT 5.6,
one by Claude/Opus 5); each was built, tested, linted and security-scanned, then
adjudicated head-to-head. The Claude rewrite won 86–69, and this PR adds the
Windows fixes it was missing.
What the revival delivers (commit
e045db1)errored / incomplete inputs become
COULD_NOT_INSPECTand are never called"safe" — fixing the live tool's
severity="Safe"-on-error behavior, the coresafety bug in a tool teachers trust.
forbid_dtd/forbid_entitiessymlinks; SHA-256 manifest.
Why it won over the other rewrite
The ChatGPT rewrite held a temp file descriptor open across
os.replace(
reporters.py) and acrosssrc.unlink(quarantine.py), which raisesPermissionError(WinError 32) on Windows — so--report-json/--report-htmlwrote no file and quarantine couldn't remove the original. That's a
regression on the tool's primary platform. The Claude rewrite's durable JSON/HTML
writers use
encoding="utf-8"and work on Windows; its only Windows defect was aconsole-printer crash, fixed here.
What this PR adds on top (commit
25ac2bc)scanner/reporters.py— the console report wrote Unicode glyphs(
✖ ✓ • …) straight to stdout, raisingUnicodeEncodeErroron a defaultWindows cp1252 console (the scan aborted right after the headline). Now the
reporter picks ASCII stand-ins (
X ! ? +) per output stream when it can'tencode the glyphs, and wraps the writer so no exotic filename can abort a scan.
UTF-8 terminals and the HTML report keep the nicer symbols.
os.geteuid,<>-in-filename) now skip on Windows via atests/_platform.pycapabilityprobe (symlink tests still run wherever symlinks are permitted); dropped-path
parsing compares with
as_posix()so it holds on both separators.Verification (Windows / CPython 3.11)
pytest: 112 passed, 6 skipped, 0 failed (was 109 passed, 8 failed)ruff check .: cleanmypy scanner: clean (19 files)Security gate
No real secrets and no critical/high vulnerabilities. The only credential-shaped
strings in the tree are fake fixtures in
tests/(this is a scanner — it needssample tokens to detect). Offline-only; no
eval/exec/subprocess/pickle/yaml.load.Before merge
ubuntu / macos / windows× Python matrix) should confirm green on allthree OSes — the numbers above are from a local Windows run.
🤖 Generated with Claude Code