From c9bf520b35dcd26e0312571ea483abbc75e5e196 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 7 Nov 2026 23:18:37 +0100 Subject: [PATCH] fix(redact): see the annotations this document has added, not only the file's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The annotation check read the SOURCE page's /Annots. A mark and a widget are held on the Doc until the write — each names the page object it sits on, which has no number until then — so neither is in the source at all, and a redaction under one was accepted. The workbench found it within a minute of being wired up, in the flow a reader walks into: highlight a line, then decide to redact it. The text went and the annotation stayed, with the marked-up words still in the file as that annotation's own note. The app's test for "the library's refusal reaches the status line" is what failed, and it was right. Doc.Redact now refuses a region that a pending Markup's quads or a pending field's widget rectangle reaches, telling the caller to drop the mark or the field first. A field's places are its WIDGETS, each with its own page: one field can show on several pages, so the field is not the thing with a position. Seven refusals now, each its own subtest, and the page-number guards are tested with a mark and a widget sitting on page 2 exactly over the region's coordinates on page 1 — a page number is not a position. 393 functions at 100.0%. Co-Authored-By: Claude Opus 5 --- README.md | 1 + redact.go | 36 +++++++++++++++++++++ redact_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fca9ce9..8db29e8 100644 --- a/README.md +++ b/README.md @@ -931,6 +931,7 @@ not told believes the document safe to send. It refuses a region holding: | text inside a Form XObject | the bytes are not the page's, and an XObject is **shared** — cleaning one page would change every page that draws it. `RecolourText` stops at the same wall | | an image | the spec is explicit that a mask or a clip will not do and the pixels must be destroyed; re-encoding an image with them overwritten is not in this release | | an annotation | its own `/Contents` can hold the very words being redacted | +| an annotation **this document has added** and not yet written | a mark or a widget is held until the write, because each names the page *object* it sits on, so neither is in the source file — and reading only the source missed them. The workbench walked into it: highlight a line, then decide to redact it, and the text went while the note quoting it stayed. Drop the mark or the field first | | an operator only **part** of whose text is inside the region | blanking it would remove text nobody asked about, and the surviving words cannot simply be re-emitted: the bytes in the stream are font codes, and Unicode does not round-trip back to them through a CID font's map. Keeping the survivors' original bytes and replacing the removed ones with an equivalent kern is the way, and it is next | | text that **reaches outside** the region | a run is the unit that can be removed, so a producer drawing a whole line with one `Tj` makes that line the run — and redacting one word of it would take the line. That is not a leak, which is why it is tempting to allow; it is the other way a redaction betrays its caller, by removing what they did not ask about and not saying so. A region is allowed to miss a run's own box by up to two points, because a caller mapping a lit rectangle back into page space arrives through screen pixels | diff --git a/redact.go b/redact.go index d271a75..62e8ff9 100644 --- a/redact.go +++ b/redact.go @@ -128,6 +128,42 @@ func (d *Doc) checkRedactable(p *Page, r Redaction) error { r.Page) } } + // ⛔⛔ THE ANNOTATIONS THIS DOCUMENT HAS ADDED COUNT TOO, and reading only + // the source page missed them. Held until the write, because a widget and a + // markup annotation both name the page OBJECT they sit on, they are not in + // p.src at all — so a caller that marked a passage and then redacted it got + // the text removed and the annotation kept, with the marked-up words still + // in the file as that annotation's own note. The workbench walked straight + // into it: highlight a line, then decide to redact it. + for _, m := range d.marks { + if m.Page != p.number { + continue + } + for _, q := range m.Quads { + x0, y0, x1, y1 := q.bounds() + if overlaps(x0, y0, x1, y1, r.Quads) { + return fmt.Errorf("ops: page %d carries a mark this document has added across "+ + "the region, and its own note can hold the words being redacted: drop "+ + "the mark first", r.Page) + } + } + } + // ⛔ A field's places are its WIDGETS, each with its own page: one field can + // show on several pages, so the field is not the thing with a position. + for _, f := range d.addFields { + for _, wdg := range f.Widgets { + if wdg.Page != p.number { + continue + } + x0, y0 := min(wdg.Rect[0], wdg.Rect[2]), min(wdg.Rect[1], wdg.Rect[3]) + x1, y1 := max(wdg.Rect[0], wdg.Rect[2]), max(wdg.Rect[1], wdg.Rect[3]) + if overlaps(x0, y0, x1, y1, r.Quads) { + return fmt.Errorf("ops: page %d carries a form field this document has added "+ + "across the region, and a field holds a value of its own: drop the "+ + "field first", r.Page) + } + } + } page, _ := p.src.Page(p.number) if annots, ok := resolveArray(p.src, page.Get("Annots")); ok { for _, a := range annots { diff --git a/redact_test.go b/redact_test.go index 7e9856b..f7ba792 100644 --- a/redact_test.go +++ b/redact_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/go-pdfkit/extract" + "github.com/go-pdfkit/forms" "github.com/go-pdfkit/reader" ) @@ -233,7 +234,8 @@ func TestRedactingRefusesWhatItCannotRemove(t *testing.T) { assertRefused(t, pdf, overSecret(), "image") }) - // An annotation: its own contents can hold the words being redacted. + // An annotation the file already carries: its own contents can hold the + // words being redacted. t.Run("an annotation across the region", func(t *testing.T) { d, err := Open(secretPage(t, twoLines, nil)) if err != nil { @@ -250,6 +252,87 @@ func TestRedactingRefusesWhatItCannotRemove(t *testing.T) { assertRefused(t, out, overSecret(), "annotation") }) + // ⛔⛔ AND ONE THIS DOCUMENT HAS ADDED BUT NOT YET WRITTEN. Marks and + // widgets are held until the write, because each names the page OBJECT it + // sits on — so they are not in the source at all, and reading only the + // source missed them. The workbench walked straight into it: highlight a + // line, then decide to redact it, and the text went while the note that + // quoted it stayed. + t.Run("a mark this document has added", func(t *testing.T) { + d, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + if err := d.Mark(Markup{Page: 1, Kind: MarkHighlight, + Quads: []Quad{QuadOf(20, 98, 60, 112)}, Note: "the secret, again"}); err != nil { + t.Fatal(err) + } + err = d.Redact(overSecret()) + if err == nil { + t.Fatal("a redaction under a mark this document had added was accepted") + } + if !strings.Contains(err.Error(), "drop the mark first") { + t.Errorf("the refusal says %q", err) + } + // A mark somewhere else on the page is nobody's business. + d2, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + if err := d2.Mark(Markup{Page: 1, Kind: MarkHighlight, + Quads: []Quad{QuadOf(20, 148, 60, 162)}}); err != nil { + t.Fatal(err) + } + if err := d2.Redact(overSecret()); err != nil { + t.Errorf("a mark on another line stopped the redaction: %v", err) + } + // And a mark on another PAGE is not even looked at, however exactly it + // sits over the region's coordinates — a page number is not a position. + d3, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + d3.Blank(100, 200) + if err := d3.Mark(Markup{Page: 2, Kind: MarkHighlight, + Quads: []Quad{QuadOf(15, 95, 85, 115)}}); err != nil { + t.Fatal(err) + } + if err := d3.Redact(overSecret()); err != nil { + t.Errorf("a mark on page 2 stopped a redaction on page 1: %v", err) + } + }) + + t.Run("a field this document has added", func(t *testing.T) { + d, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + if err := d.AddFields(forms.Field{Name: "note", Kind: forms.Text, + Widgets: []forms.Widget{{Page: 1, Rect: [4]float64{20, 98, 60, 112}}}}); err != nil { + t.Fatal(err) + } + err = d.Redact(overSecret()) + if err == nil { + t.Fatal("a redaction under a field this document had added was accepted") + } + if !strings.Contains(err.Error(), "drop the field first") { + t.Errorf("the refusal says %q", err) + } + // A widget on another page is not in the way, wherever it sits. + d2, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + d2.Blank(100, 200) + if err := d2.AddFields(forms.Field{Name: "note", Kind: forms.Text, + Widgets: []forms.Widget{{Page: 2, Rect: [4]float64{20, 98, 60, 112}}}}); err != nil { + t.Fatal(err) + } + if err := d2.Redact(overSecret()); err != nil { + t.Errorf("a field on page 2 stopped a redaction on page 1: %v", err) + } + }) + // One operator drawing both what goes and what stays. t.Run("an operator it would half-erase", func(t *testing.T) { const body = "BT /F1 10 Tf 20 100 Td [(keep) -2000 (drop)] TJ ET"