Skip to content

MCP: SVG output under read roots refuses to name a linked image file outside them - #7653

Open
dotCooCoo wants to merge 1 commit into
storytold:mainfrom
dotCooCoo:fix/svg-export-confined-image-links
Open

dotCooCoo wants to merge 1 commit into
storytold:mainfrom
dotCooCoo:fix/svg-export-confined-image-links

Conversation

@dotCooCoo

Copy link
Copy Markdown
Contributor

What and why

This adds a control for automation read roots (--automation-read-root): an SVG or SVGZ made for a confined caller no longer carries the stored path of a linked image file outside the caller's read roots. It applies the policy #7264 set for linked JavaScript files to linked images. Until now such a caller got the path in the image's href.

SVG export keeps both image modes. The SVG options on the help page (https://helpx.adobe.com/illustrator/using/saving-artwork.html) either put raster images inside the SVG file or link the SVG to JPEG or PNG images exported from the original file. Another help page (https://helpx.adobe.com/illustrator/desktop/add-and-import-files/import-other-file-types/place-multiple-files-in-a-document.html) describes a linked file as one that keeps its connection to the source file, and an embedded file as one copied into the document. The pages say nothing about read roots; the refusal below is VectorCraft's own rule.

  • With read roots in force, an SVG or SVGZ that would name a linked image's file outside them is refused before any file, folder or reply is written. The error is "a linked image file is outside the read roots" and names no file or folder. The document keeps its link.
  • An <image> names the file with images: "link", and with embed when the document holds no pixels for the image. An image whose pixels the document holds still exports embedded. An explicit link export is refused rather than switched to embed.
  • With preserveEditing, the native document the SVG carries names every linked image and placed file, the relative paths kept of them and each data set's image, so all of them are checked. A stored brush entry that does not read as a brush is refused, because its links cannot be checked.
  • The check runs where the MCP: SVG export by a confined caller refuses script links outside its read roots #7264 check runs: document.serialize, document.export, document.exportSelection, document.exportForScreens and assets.export, every SVG and SVGZ save, file.exportAs and the SVG Options dialog's Show Code. It also covers clipboard.exportSvg, and a Copy that an automation request asks for puts no SVG on the system clipboard when it would name such a file.
  • Only the art each file writes is checked: the selected objects with selectedOnly and Export Selection, the art over each chosen artboard, hidden layers and objects only when hiddenLayers is on (a save keeps them by default), never template layers or the symbol or opacity-mask editing layer, and the symbols, patterns and brushes that art uses. An image colorized with a solid fill whose pixels are gray or missing names no file, so it is checked only with preserveEditing.
  • A file inside the roots, a relative path that leads inside them and a URL are written as before. File › Export by the person at the keyboard writes every link as stored.

The check is fileio::check_linked_images (crates/engine/src/cmd/fileio/access.rs), next to check_linked_scripts, with its predicates in crates/engine/src/cmd/links.rs. The params text of document.serialize, document.export, document.exportSelection, document.exportForScreens (which assets.export points to), document.save (shared by file.saveAs and file.saveCopy), clipboard.exportSvg and file.exportAs, docs/mcp.md and a line in docs/target-app-parity.md describe it. The de and nl catalogs, which have a row for the JavaScript message, get a row for the new one.

How it was tested

New tests, each failing without the change:

  • crates/engine/src/tests/links.rs:
    • confined_svg_output_refuses_a_linked_image_file_outside_the_read_roots: images: "link", preserveEditing, and embed without pixels; SVG and SVGZ; serialize, export with and without a path, selectedOnly, Export Selection, document.save, file.saveAs, file.saveCopy, Export for Screens with a PNG and an SVG row and as a ZIP, and assets.export. No file or folder is written, the error names no file or folder, the link stays, and embedded pixels still export.
    • confined_svg_output_checks_only_the_linked_images_it_writes: hidden layers and objects (with and without hiddenLayers, and in saves), template layers, and an unused symbol in editing mode.
    • confined_svg_output_writes_a_colorized_gray_image_without_its_file
    • confined_clipboard_svg_refuses_a_linked_image_file_outside_the_read_roots
    • confined_preserve_editing_svg_refuses_a_brush_library_entry_it_cannot_read
  • crates/ui-egui/src/tests/automation.rs: control_svg_export_refuses_a_linked_image_file_outside_the_read_root (app.export with and without a path, SVGZ, preserveEditing, file.exportAs; the export at the keyboard writes the link).
  • crates/ui-egui/src/dialogs/svg_options.rs: show_code_leaves_out_a_linked_image_file_outside_the_read_roots.

svg_output_names_linked_image_files_a_caller_may_read (crates/engine/src/tests/links.rs) covers the outputs that must keep working: a link inside the roots, a relative link that leads inside them, a URL, an outside image in an unused symbol, outside images left out by a selection or by a chosen artboard, and an export without roots. It also passes without the change.

Focused checks on the branch merged with main at 0adbdce6cb:

  • cargo fmt --all -- --check and cargo check --workspace --tests
  • cargo clippy -p vectorcraft-engine --all-targets -- -D warnings
  • cargo test -p vectorcraft-engine --lib tests::links: 72 passed; clipboard: 69 passed
  • cargo xtask brands, layers (30 crates) and assets (337 files)

On the same branch merged with main at 67f6d3a50f, before the last engine-only fix:

  • cargo clippy -p vectorcraft-ui-egui --all-targets -- -D warnings and cargo clippy -p vectorcraft-mcp --all-targets -- -D warnings
  • cargo test -p vectorcraft-engine --lib tests_svg: 27 passed; linked_javascript: 2 passed
  • cargo test -p vectorcraft-ui-egui --lib linked_image: 2 passed; show_code: 2 passed; i18n: 122 passed

The branch is one commit on main at bada8e1710, checked again before posting. On that commit cargo fmt -p vectorcraft-engine -p vectorcraft-ui-egui -p vectorcraft-mcp -- --check passes. main at bada8e1710 does not build on its own: crates/render/src/paint.rs and the tests in crates/trace/src/opaque.rs do not compile against the pinned craft-image-codecs, and cargo fmt --all -- --check reports files in crates/pdf, crates/render and crates/trace. This branch changes none of those files or Cargo.lock.

  • cargo xtask ci runs in this pull request's CI.

Checklist

  • cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm).
  • No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
    unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
    messages. Errors are returned as Result and shown to the user
    (why and how).
  • Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
  • Every new asset has a row in ASSETS.md.

…outside them

With automation read roots in force, an SVG or SVGZ that would name a
linked image's file outside them is refused before any file, folder or
reply is written, with "a linked image file is outside the read roots",
which names no file or folder. An <image> names the file with
images: link, and with embed when the document holds no pixels for the
image. With preserveEditing, the native document the SVG carries names
every linked image and placed file, the relative paths kept of them and
each data set's image, so those are checked too, and a stored brush
entry that does not read as a brush is refused because its links cannot
be checked.

- The check runs where the linked JavaScript check runs:
  document.serialize, document.export, document.exportSelection,
  document.exportForScreens and assets.export, every SVG and SVGZ save,
  file.exportAs and the SVG Options dialog's Show Code. It also covers
  clipboard.exportSvg, and a Copy that an automation request asks for
  puts no SVG on the system clipboard when it would name such a file.
- Only the art each file writes is checked: the selected objects with
  selectedOnly and Export Selection, the art over each chosen artboard,
  hidden layers and objects only when hiddenLayers is on (a save keeps
  them by default), never template layers or the symbol or opacity-mask
  editing layer, and the symbols, patterns and brushes that art uses.
  An image colorized with a solid fill whose pixels are gray or missing
  names no file, so it is not checked outside Preserve Editing.
- Embedded pixels, a file inside the roots, a relative path that leads
  inside them and a URL are written as before, and the person at the
  keyboard exports every link as stored.

Docs: docs/mcp.md, docs/target-app-parity.md and the params text; de
and nl rows for the new message, next to the JavaScript message's rows.
@coderabbitai

coderabbitai Bot commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 37 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between bada8e1 and 56cbdbb.

Files ignored due to path filters: 2

  • crates/ui-egui/src/i18n/de.tsv is excluded by !**/*.tsv
  • crates/ui-egui/src/i18n/nl.tsv is excluded by !**/*.tsv
Files selected for processing: 18

  • crates/engine/src/cmd/clipboard.rs
  • crates/engine/src/cmd/fileio.rs
  • crates/engine/src/cmd/fileio/access.rs
  • crates/engine/src/cmd/fileio/commands.rs
  • crates/engine/src/cmd/fileio/export.rs
  • crates/engine/src/cmd/fileio/save.rs
  • crates/engine/src/cmd/fileio/screens.rs
  • crates/engine/src/cmd/fileio/svg.rs
  • crates/engine/src/cmd/links.rs
  • crates/engine/src/tests/links.rs
  • crates/mcp/src/ui_catalog.rs
  • crates/ui-egui/src/dialogs/svg_options.rs
  • crates/ui-egui/src/io/export.rs
  • crates/ui-egui/src/menus/ui_commands.rs
  • crates/ui-egui/src/sysclip.rs
  • crates/ui-egui/src/tests/automation.rs
  • docs/mcp.md
  • docs/target-app-parity.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

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