fix(input): expand 8.3 short DOS names before the opened-handle check - #484
fix(input): expand 8.3 short DOS names before the opened-handle check#484kevin9327 wants to merge 1 commit into
Conversation
Signed-off-by: kevin9327 <kevin9327@users.noreply.github.com>
|
The The failing assertion is This diff only touches On Windows, where it does execute, that whole file passes: Happy to rebase or add anything that would help; a rerun is the only thing I cannot do myself. |
Fixes #481.
Problem
On Windows, every file in a scan is rejected when the scanned path is spelled with an 8.3 short
component, so
execution_successfulisfalse, the ledger fills withmissing_file_cache, and therisk score means nothing regardless of what the skill contains.
_open_regular_file_from_windows_handleguards against a reparse-point swap by comparing thecanonical path of the handle it opened against the path it was asked to open.
GetFinalPathNameByHandleWalways answers with long components, while the requested path can carryshort ones, and
_windows_normalized_pathonly stripped the\\?\prefix and normcased. The twospellings never matched, so the guard rejected the file it had just safely opened.
This is not an unusual setup. Windows keeps an 8.3 alias for any directory whose name holds a space,
so a profile directory like
C:\Users\Hoang Phamreaches the scanner asC:\Users\HOANGP~1through%TEMP%. Becauseskillspector scan <url>clones into a temp directory, URL scans are broken bydefault on any machine with a spaced account name.
Fix
_windows_long_path_nameexpands short components throughGetLongPathNameW, and_windows_normalized_pathruns the path through it before comparing.The guard keeps its meaning. An 8.3 name is an alias the filesystem holds for one directory entry,
so expanding it names that same entry;
GetLongPathNameWdoes not resolve symlinks or junctions, soa reparse point that appeared mid-open still shows up as a different final path and is still
rejected. A path that no longer resolves comes back unchanged, which leaves the comparison
fail-closed.
Tests
Two tests in
tests/unit/test_input_handler.py, using the existing_mock_windows_secure_openharness so they run on any platform. The mock gains a
GetLongPathNameWstub driven by along_namesmapping.test_windows_no_follow_open_accepts_a_short_dos_name— a requested short spelling and a longfinal path open the same entry. Before the fix:
test_windows_no_follow_open_rejects_an_unresolvable_short_name— a short name that does notexpand is still refused, so the fail-closed half is pinned rather than assumed.
How I tested
Windows 11, Python 3.12.10, 8.3 name creation enabled on the volume. Built a two-file skill under
C:\Users\swsz9\claudeworkspace\Skill Space Test, whose short name isSKILLS~3, and scanned bothspellings with
--no-llm --format json:execution_successfulSKILLS~3before the fixfalsefailedSKILLS~3after the fixtruecompleteSkill Space Test(control)truecompleteThe first ledger exception in the failing run is the one from the report:
{"outcome": "failed", "phase": "static", "reason_code": "missing_file_cache", "message": "Applicable analyzer could not obtain file content.", "path": "SKILL.md", "fatal": true, "analyzers": ["static_yara"]}ruff check src/ tests/andruff format --check src/ tests/are clean.pytest -m "not integration and not provider" tests/adds no failures: 23 failed / 3943 passedbefore, 23 failed / 3945 passed after, the two extra passes being the tests above.
That suite is not green on Windows to begin with, and none of the 23 involve this code path:
12 in
tests/nodes/test_build_context.py(symlink fixtures, which need elevation or developer modeon Windows), 5 in
test_create_github_release.py, 4 intest_compare_scan_accuracy.py, 1 intest_static_yara.py, andtest_input_handler.py::test_resolve_file_open_failure_does_not_create_temp_dir.I ran
tests/nodes/test_build_context.pyon its own with and without this change to be sure: 12failed either way. Happy to file those separately if they are not already known.