fix(scan): read the hunks and the encodings the scan claims to cover - #32
Merged
Conversation
Two ways a commit-time scan reported full coverage of content it never read. A unified diff renders an added line as `+` followed by its content, so source starting `++ ` arrives as `+++ ...`, the same shape as a new-file header. The hunk parser read it as one, filed every later hunk of that file under a path nothing scans, and still reported the scan complete. Hunk bodies now run against the line budget each `@@` header declares, so a header is only recognised where one can appear. A `diff --git` line resets the current path, since content cannot forge that prefix. UTF-16 text was classified as binary on its NUL padding and skipped. A byte-order mark says the bytes are text, so a marked blob is decoded and handed to the content rules. PowerShell 5.1 writes exactly that from `>`, `Out-File` and `Tee-Object`. Unmarked NUL data still skips as binary, and an odd-length blob falls through instead of crashing the byte swap.
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
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.
Two ways a commit-time scan reported full coverage of content it never read.
Forged diff headers
A unified diff renders an added line as
+followed by its content. A sourceline starting
++therefore arrives as+++ ..., which is the same shape as anew-file header.
collectLineRangesread it as one: it reassigned the currentpath to a file that does not exist, filed every later hunk of the real file
under that path, and returned line ranges nothing would query. The rest of the
file was never handed to the content rules, and the scan still reported
complete: true, so no incomplete reason was recorded and the fail-closedcarve-outs never engaged.
Hunk bodies now run against the line budget each
@@header declares — bothsides, since Git emits every
-line before every+line — so a header isonly recognised where one can appear. A
diff --gitline resets the currentpath; content cannot forge that prefix, which makes it the one reliable resync
point if a body ever ends short.
++ /dev/nullis the same defect through a different branch: it set the currentpath to null and dropped the remaining hunks rather than misfiling them. The
budget covers both.
UTF-16 text classified as binary
isBinaryreturns true on the first NUL byte, so UTF-16 text skipped whole andthe scan stayed complete. PowerShell 5.1 writes UTF-16LE from
>,Out-Fileand
Tee-Object, so a Windows developer could commit a marker file no contentrule ever read.
A byte-order mark says the bytes are text, so a marked blob is now decoded and
scanned. Only the mark is trusted. Unmarked NUL data still skips as binary:
guessing there would decode images into garbage and feed it to the rules, and on
cleana spurious block unstages the user's asset.FF FE 00 00opens UTF-32LEand is left alone. An odd-length blob falls through to the binary skip rather
than crashing
swap16, and the big-endian path byte-swaps a copy so the readbatch is not mutated.
The oversize probe is deliberately untouched. Decoding there would flip an
oversized UTF-16 file from a silent
binaryskip to a fatalsize-limit, andstrictreturns 31 unconditionally on that — a new hard refusal for any repoholding a large PowerShell transcript, bought for no detection the in-budget
path does not already give.
Tests
Five added. Two cover a commit whose added line forges each header shape, with
the marker in a later hunk. Three cover UTF-16: little-endian and big-endian
marked text must scan, and an odd-length blob claiming a mark must not crash.
The existing binary-skip test keeps its assertions and gets an accurate comment:
it pins unmarked NUL data, not the PowerShell case.
Full suite green.