fix: correct redaction-cover placement and de-duplicate covers - #1
Merged
Merged
Conversation
The operator-list cover walk produced false-positive leaks in three ways: - It ignored Form XObject matrices, so covers drawn inside a form (such as a redaction mark's appearance stream) were placed in the parent's coordinate space, shifted off their real position and onto neighbouring text, which was then "recovered" as if hidden. Handle paintFormXObjectBegin/End by mirroring pdf.js's implicit save + form matrix so covers land where they actually paint. - Clip paths (constructPath -> clip -> endPath) lingered in the pending path list because endPath was unhandled, and the next unrelated fill flushed them as covers, fabricating dark boxes over visible text (e.g. footer social handles). Discard the path on endPath. - A single mark painted many times over produced dozens of identical findings; dedupe covers by kind + rounded bbox, keeping the earliest paint so overlay-label paint-order stays correct. Also tighten the word-size hint: require a genuine inline gap (text on both sides) and skip once the estimated length exceeds a plausible single word (MAX_WORD_SIZE_CHARS), so classification portion marks and whole redacted fields like email "From:" lines no longer read as length leaks.
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.
Description
Fixes three sources of false-positive leaks in the operator-list cover walk, found while running the shipping analyzer over a batch of real government redaction releases:
Form XObject matrices were ignored. Covers drawn inside a form (e.g. a redaction mark's appearance stream) were placed in the parent coordinate space, shifted off their real position and onto neighbouring text, which was then "recovered" as if hidden. We now handle
paintFormXObjectBegin/paintFormXObjectEnd, mirroring pdf.js's implicit save + form-matrix so covers land where they actually paint. (getTextContentalready places text correctly, so an unhandled form matrix desynced covers from text.)Clip paths leaked into covers.
constructPath → clip → endPathleft the rectangle in the pending path list becauseendPathwas unhandled, and the next unrelatedfillflushed it as a dark cover — fabricating a redaction over whatever visible text the clip region bounded (e.g. footer social-media handles). We now discard the path onendPath. A path that is both clipped and filled still reaches its fill op withpendingintact, so genuine filled shapes are unaffected.Repeated covers were double-counted. A single mark painted many times over produced dozens of identical findings (one box seen 67×). Covers are now de-duplicated by kind + rounded bbox, keeping the earliest paint so overlay-label paint-order stays correct.
Also tightens the word-size hint so it only fires for a plausibly guessable single word: it now requires a genuine inline gap (text on both sides, matching the check's documented intent) and bails once the estimated length exceeds a plausible word (
MAX_WORD_SIZE_CHARS). This drops classification portion-marks (text only to the right) and whole redacted fields like emailFrom:lines (text only to the left), neither of which is guessable from width.Net effect on the real-world corpus: genuine dark-box-over-text failures are unaffected, while offset/clip/duplicate artifacts no longer register as leaks.
Testing
npm run typecheck,npm run lint,npm run check-format— clean.npm test— unit + example suites pass. Genuine failures in the real-world fixtures still detect (Manafort → F, JPMorgan → F) and the clean control still passes (AstraZeneca → A), confirming the detector is intact and only the artifacts were removed.Note for reviewers
The
tsa-screening-sop-2008real-world fixture still expects gradeF, but the corrected analyzer now returnsA: that document's failure mode is text outlined/marked for redaction but never covered by a dark fill (visible text in a red-stroke box), which was only ever "detected" via the clip-path bug fixed here — not a dark-box-over-text leak. This fixture/README expectation needs a follow-up re-baseline (or a dedicated check for the outline-only failure mode). Not fetched in CI, so it skips there.