Skip to content

fix(redact): cover pivot caches, threaded comments and metadata - #242

Open
eliahilse wants to merge 1 commit into
mainfrom
fix/redact-scrub-coverage
Open

fix(redact): cover pivot caches, threaded comments and metadata#242
eliahilse wants to merge 1 commit into
mainfrom
fix/redact-scrub-coverage

Conversation

@eliahilse

Copy link
Copy Markdown
Member

TL;DR:

Summary:

  • scrub pivot-cache and pivot-table metadata end to end: shared items (type-valid placeholders per xsd type), cache fields, calculated items, page/range sets, hierarchy captions, worksheet sources, refresh metadata
  • cover threaded comments + legacy comments consistently: person ids as schema-valid braced GUIDs with cross-part referential equality, tc={uid} author linkage, mention identifiers, extension checksums neutralized
  • cover connections (dbPr/webPr/olapPr/parameters), slicer/drawing name chains, DOCX mail-merge children, modern password verifiers, rsids (schema-valid 8-hex), revision dates across all tracked-change elements
  • placeholders are type-valid (hex/dateTime/GUID/double), reference-consistent via a per-run bijective salted mapping, and collision-free in O(1)
  • parts are classified by content type as well as path, so relocated or unusually named XML parts still get scrubbed

Known follow-ups (long tail, intentionally deferred):

  • a few deep pivot-cache attributes (calculatedItem@formula attr form, sharedItems min/max bounds, rangePr) and custom-property typed variants (vt:cy/clsid/i1 overflow) still pass through; tracked for a follow-up PR
  • pseudonym digest is salted FNV — adequate against casual dictionary attacks, not a keyed PRF; revisit if redacted docs face targeted adversaries

Test plan:

  • cargo test -p betteroffice-redact green (33 tests incl. package-level integration)
  • redact real-world xlsx with pivot table + slicers + threaded comments; open in Excel without repair prompts
  • redact docx with tracked changes + people store; verify authors/dates gone and file opens clean

@openooxml-bot

openooxml-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA — thank you! ✍️ ✅

Posted by the CLA bot.

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR substantially expands type-valid, reference-consistent redaction across pivot metadata, threaded and legacy comments, connections, DOCX revision metadata, and unusually named XML parts.

  • Adds per-run salted mappings for GUIDs, hexadecimal IDs, and cross-part reference names.
  • Classifies XML parts using package content types in addition to conventional paths.
  • Extends typed XML replacements and adds broad package-level regression coverage.

Confidence Score: 4/5

The PR appears safe to merge after optional cleanup of redundant private implementation comments.

The expanded redaction and content-type classification paths have no accepted functional or security failure; the sole finding is non-blocking repository-style cleanup.

Files Needing Attention: crates/ooxml-redact/src/lib.rs

Important Files Changed

Filename Overview
crates/ooxml-redact/src/lib.rs Adds content-type-driven XML routing and per-run identity mappings; only redundant private-item comments require cleanup.
crates/ooxml-redact/src/xml.rs Broadens format-aware, type-valid XML redaction and preserves cross-part identity references without an accepted blocking defect.
crates/ooxml-redact/src/tests.rs Adds extensive focused and package-level coverage for the new redaction paths and schema-valid placeholders.
.changeset/redact-scrub-coverage.md Records the expanded redaction coverage as a patch release.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[OOXML package] --> B[Unpack and detect format]
    B --> C[Parse content-type declarations]
    C --> D[Classify XML by path or content type]
    D --> E[Rewrite sensitive text and attributes]
    E --> F[Apply per-run reference mappings]
    F --> G[Repack redacted package]
Loading

Reviews (1): Last reviewed commit: "fix(redact): cover pivot caches, threade..." | Re-trigger Greptile

Comment on lines +133 to +134
/// Per-run entropy so FNV-derived pseudonyms cannot be dictionary-tested
/// across documents produced by separate redaction runs.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant private implementation comments

These comments restate behavior already clear from the private helpers, adding maintenance overhead and a risk of drifting from the implementation; the same pattern appears above PackageContentTypes.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant