Skip to content

fix(save): preserve destination extended attributes on macOS - #3042

Open
PerchAnton wants to merge 1 commit into
storytold:mainfrom
PerchAnton:fix/preserve-save-xattrs
Open

PerchAnton wants to merge 1 commit into
storytold:mainfrom
PerchAnton:fix/preserve-save-xattrs

Conversation

@PerchAnton

Copy link
Copy Markdown

What this changes

Addresses #2658: Finder tags and accessible extended attributes on macOS only.

Rotating a synthetic PDF and saving it successfully removed its Finder tags, binary custom attribute and empty custom attribute. The shared atomic writer now copies the existing regular destination's accessible xattrs to the open staging file after writing the PDF and before flush/rename. Both desktop Save and CLI/automation use this writer.

A metadata read/copy error refuses publication, removes the staging file and suggests Save As. The original file and unsaved edit remain available. Save As retains an existing destination's attributes; a new destination does not inherit the input PDF's tags.

How you verified it

On macOS, based directly on main 924c4da4, with two Cargo jobs and bounded command timeouts:

  • cargo fmt --check
  • cargo test -p pdfcraft-platform -p pdfcraft-automation -j 2: 251 passed, 0 failed, 0 ignored (17 platform tests; 234 automation unit/integration tests)
  • cargo clippy -p pdfcraft-platform -p pdfcraft-automation -j 2 --all-targets -- -D warnings
  • cargo check -p pdfcraft-platform -p pdfcraft-automation --target wasm32-unknown-unknown -j 2 (four existing renderer warnings)
  • Rebuilt CLI/xtask; layers, assets and parity pass
  • 21 CLI steps: incremental/full saves, close/reopen, exact binary-plist Finder tags and binary/empty attributes, existing/new Save As destinations, text extraction and page_render. Reopened renders inspected.

Red before green: both the platform attribute-preservation regression and the automation save regression were run before the fix and failed with Attribute not found. The unfixed CLI also reproduced successful incremental saving with all three test attributes lost.

The six new macOS tests cover exact metadata before flush and after repeated saves using both staging name forms; binary/empty values; flush refusal; oversized resource-fork refusal before publication; symlink referent isolation; incremental/full saves and Save As; and an unsaved edit surviving a metadata failure and saving to a new name.

Full workspace builds, desktop GUI/Finder interaction, Windows/Linux runtime and external PDF viewers were not tested. Hosted CI still needs to run.

Never-crash

  • No new panicking shortcuts, unsafe or input-derived indexing in production code
  • Metadata copying is bounded: 64 KiB of attribute names, 1024 entries, 16 MiB per value and 64 MiB total
  • Copy/limit failures return actionable errors before rename; regression coverage checks original bytes, metadata, staging cleanup and retained dirty state

Provenance

  • All fixtures are contributor-original data generated in test code; no PDF, plist, font or other asset files added
  • No Adobe-derived assets or copied GPL/AGPL code
  • Uses the existing safe rustix dependency; automation adds it only as a macOS test dependency
  • Behaviour follows the public Apple listxattr and getxattr APIs

Security

  • Existing --root checks and opt-in MCP/control behaviour retained; no network added
  • Source is opened with NOFOLLOW and NONBLOCK; metadata is copied through file descriptors to the newly created staging file
  • A symlink destination does not disclose its referent's attributes to the replacement file

Parity and reach

  • Existing doc_save tool reaches the fix; two new automation regressions
  • Incremental-save evidence/limitations, platform README and roadmaps updated; readiness estimates unchanged

Anything a reviewer should look at first

This preserves accessible xattrs on an existing regular macOS destination. The OS-managed com.apple.provenance is left to macOS, and kernel-hidden compression attributes are not requested. Filesystems reporting no xattr support continue to work. A regular target whose attributes cannot be read/copied, or exceed the bounds above, now refuses in-place save rather than dropping metadata; Save As to a fresh name remains available.

Permissions, ACLs, hard-link identity, creation dates and read-only policy are separate parts of #2658. In particular, #2544 preserves permission bits and #2699 refuses read-only targets; this branch does not include either PR and will need to retain their behaviour when merged. No claim is made to close all of #2658.

This branch has not been deployed

No deployments
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