Skip to content

MCP: confined exports refuse native files that name a linked JavaScript file outside the read roots - #7600

Open
dotCooCoo wants to merge 8 commits into
storytold:mainfrom
dotCooCoo:fix/confined-exports-script-paths
Open

dotCooCoo wants to merge 8 commits into
storytold:mainfrom
dotCooCoo:fix/confined-exports-script-paths

Conversation

@dotCooCoo

@dotCooCoo dotCooCoo commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Follows #7264.

What and why

#7264 refuses an SVG or SVGZ export by a caller confined with --automation-read-root when the document links a JavaScript file outside the read roots. The native document stores the same linked file paths, so every other output that carries the native document now gets the same control, with the same BadParams error, "a linked JavaScript file is outside the read roots", which names no file or folder. A JavaScript Files entry can be a file picked from the computer or a typed URL (the help page). The read roots and this refusal are VectorCraft's own rules.

  • check_linked_scripts (crates/engine/src/cmd/fileio/access.rs) now takes the effective options and asks names_linked_scripts whether the output carries the native document: native files and templates and EPS files always do; a .ai or .ait file does unless it is a legacy version 7 .ai file, has only some of the artboards, or is a .ai file of the art's bounds (useArtboards: false); a PDF does when Preserve Editing is on, as its preset, standard and options leave it, and it holds every artboard. A PDF saved with Preserve Editing holds the full editable document (the help page).
  • document.serialize, document.export, document.exportSelection and document.exportEps check after presets are expanded. document.exportPdf gains the check. document.save, file.saveAs, file.saveCopy and file.saveAsTemplate check with the save's final options; a .ai save always has every artboard, so useArtboards does not exempt it.
  • document.exportForScreens and assets.export check each file with the artboards and options it is written with, before the folder is made: a PDF per artboard leaves the editing data out and is written, while one PDF of every artboard or Full Document's PDF with Preserve Editing is refused.
  • file.exportAs in the app (crates/ui-egui/src/io/export.rs) checks with a saved PDF preset expanded.
  • file.package (crates/engine/src/cmd/package.rs) checks the document before anything is collected or written, and checks each linked native document, and the native document a linked PDF, .ai or EPS file carries, before copying it. A package gathers the document with its linked graphics and the fonts it uses, and adds a report (the help page).
  • The params text of document.serialize, document.export, document.exportSelection, document.exportForScreens, document.exportPdf, document.save (which also covers file.saveAs and file.saveCopy), file.saveAsTemplate, file.package and file.exportAs, docs/mcp.md (Confining file access, Preserve Editing, Package) and one line in docs/target-app-parity.md describe the refusal. No user-visible string changed.

This changes existing behavior for confined callers only: these outputs were written before and are now refused. Outputs without the native document (images, a PDF without Preserve Editing, a PDF or .ai file of some artboards or of the art's bounds, a legacy .ai file), linked files inside the read roots, URLs and the person at the keyboard are written as before.

How it was tested

New tests, each failing without the change:

  • confined_native_output_refuses_a_linked_javascript_file_outside_the_read_roots (crates/engine/src/tests/interactivity.rs): native, template, PDF, .ai and EPS output from document.serialize, export (also selectedOnly), exportSelection, exportPdf, exportEps, the saves, exportForScreens, assets.export and file.package are refused with BadParams and the exact message, which names neither folder nor the file; no file is written and the document keeps the link.
  • confined_output_without_the_document_or_with_scripts_the_caller_may_read_is_written (same file): a PDF without Preserve Editing, a PDF or .ai file of the art's bounds or of one artboard, a legacy .ai file, a PNG and per-artboard Export for Screens PDFs are written; one PDF of both artboards and Full Document's PDF are refused; a JavaScript file inside the roots, a relative path and a URL stay in the native document and the PDF editing data, and the package is written.
  • a_confined_package_refuses_a_linked_javascript_file_outside_the_read_root (crates/engine/src/tests/package.rs): a package of the document, or of a document that links such a native file or PDF with Preserve Editing, is refused for a folder and a ZIP, and nothing is written; the unconfined package copies it, and a JavaScript file inside the roots is packaged.
  • control_native_export_refuses_a_linked_javascript_file_outside_the_read_root (crates/ui-egui/src/tests/automation.rs): app.export to PDF, .ai and EPS and file.exportAs to a native file over the control connection are refused and write nothing; a PDF without Preserve Editing is written, and the person's file.exportAs keeps the link.

Focused checks on the final tree, one commit on main at a47d7eec912, which was still the head of main when this was posted:

  • cargo fmt --all -- --check
  • cargo check --workspace --tests
  • cargo clippy -p vectorcraft-engine -p vectorcraft-ui-egui -p vectorcraft-mcp --all-targets -- -D warnings
  • cargo test -p vectorcraft-engine --lib, filtered to tests::interactivity, tests::package and the Export for Screens tests: 98 passed
  • cargo test -p vectorcraft-ui-egui --lib, filtered to control_native_export_refuses, script and export_selection: 95 passed
  • cargo xtask brands, cargo xtask layers (30 crates), cargo xtask assets (337 files) and cargo xtask ui-commands --check
  • 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.

Summary by CodeRabbit

  • Bug Fixes
    • Automation requests now refuse to save, export, or package files that would include a native document containing links to JavaScript outside the permitted read locations. Refusals occur before files are written and avoid revealing file or folder paths.
    • Checks now account for the selected output format and content, including artboards and export settings. Outputs that do not include the restricted scripts remain available.
  • Documentation
    • Updated command descriptions and automation guidance to explain these restrictions and relevant exceptions.

…pt file outside the read roots

Under --automation-read-root, a document that links a JavaScript file
outside the read roots could still be written by a confined caller to
any output that carries the native document, and that document holds
the stored path. SVG and SVGZ output was already refused. This change
applies the same control to every output that carries the native
document, and they now return the same error, "a linked JavaScript file
is outside the read roots", without the path:

- document.serialize, document.export, document.exportSelection and
  document.exportEps for vectorcraft, template, EPS, .ai and .ait files
  (not a legacy version 7 .ai file, a .ai file of some of the
  artboards, or one of the art's bounds)
- PDF with Preserve Editing on, as its preset, standard and options
  leave it, when it holds every artboard, including document.exportPdf
- document.save, file.saveAs and file.saveAsTemplate
- document.exportForScreens and assets.export, checked with the
  artboards each file is written with
- file.exportAs in the app
- file.package, before the folder is made, including when a linked
  native document, or the native document a linked PDF, .ai or EPS
  file carries, links such a file

Outputs that do not carry the native document, unconfined callers and
linked files inside the read roots are written as before.
@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

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 3 minutes.

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 d4c9ef8 and 7a90fe5.

Files selected for processing: 17

  • 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/pdf.rs
  • crates/engine/src/cmd/fileio/save.rs
  • crates/engine/src/cmd/fileio/screens.rs
  • crates/engine/src/cmd/package.rs
  • crates/engine/src/tests/interactivity.rs
  • crates/engine/src/tests/package.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/tests/automation.rs
  • docs/mcp.md
  • docs/target-app-parity.md

No actionable comments were generated in the recent review.

Walkthrough

Automation read-root checks now determine whether output formats and selected content carry linked JavaScript. Export, save, screen-export, PDF, and package paths apply those checks before writing. Command descriptions, documentation, UI export handling, and tests reflect these rules. A separate parity note records Pencil snapping behavior.

Changes: Automation read-root linked-script checks

Layer / File(s) Summary
Determine output script inclusion
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/pdf.rs, crates/engine/src/cmd/fileio/save.rs, crates/engine/src/tests/interactivity.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/tests/automation.rs, docs/mcp.md
The shared check determines whether an output names linked scripts based on format, options, and artboards. Export, save, PDF, and UI paths pass prepared options to the check. Tests cover refusal, allowed outputs, and script-link preservation.
Check scripts for screen-export contents
crates/engine/src/cmd/fileio/commands.rs, crates/engine/src/cmd/fileio/screens.rs, crates/engine/src/tests/interactivity.rs
Screen export checks the documents and artboards each output file will contain before folder creation or file writes.
Check active and carried documents during packaging
crates/engine/src/cmd/package.rs, crates/engine/src/tests/package.rs, docs/mcp.md
Packaging checks linked scripts in the active document and supported native documents carried by linked files. Tests cover confined and unconfined package requests.

Pencil behavior parity note

Layer / File(s) Summary
Record Pencil snapping behavior
docs/target-app-parity.md
The revision history records that Pencil ignores Smart Guides and Snap to Point, while Snap to Grid and Snap to Pixel remain applicable.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AutomationRequest
  participant check_linked_scripts
  participant check_scripts_of
  participant names_linked_scripts
  participant FileWriter
  AutomationRequest->>check_linked_scripts: command, format, prepared options
  check_linked_scripts->>check_scripts_of: linked scripts and read-root status
  check_scripts_of->>names_linked_scripts: format options and artboard count
  names_linked_scripts-->>check_scripts_of: whether output names scripts
  check_scripts_of-->>AutomationRequest: allow or SCRIPT_OUTSIDE error
  AutomationRequest->>FileWriter: write output when allowed
Loading

Suggested reviewers: bflatastic, aiongg, dylanmtaylor

Merge Risk: Minimal · previous review d4c9e

This change makes confined automation callers unable to write outputs that carry the native document when it links a JavaScript file outside the read roots. Outputs without the native document, unconfined callers, and links inside the read roots keep their prior behavior. No concrete defect was found, so it looks ready to merge once the full CI run passes.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: confined exports refuse native-document outputs that name a linked JavaScript file outside the automation read roots. It matches the changeset, which extends the exist…
Description check Passed The description follows the template. It has a 'What and why' section that states the change, the reason, and the linked issue (#7264). It has a 'How it was tested' section that lists specific new tes…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: confined exports refuse native-document outputs that name a linked JavaScript file outside the automation read roots. It matches the changeset, which extends the existing SVG/SVGZ check to native-document outputs, and it is specific enough for a teammate scanning history. It is 99 characters, slightly long, but still readable.

Full details: Description check

Explanation

The description follows the template. It has a 'What and why' section that states the change, the reason, and the linked issue (#7264). It has a 'How it was tested' section that lists specific new tests, focused check commands, and results. The checklist is present, and the three boxes that were checked are backed by the description. The first checklist item is unchecked and the description says CI has not yet run, which is consistent with the template. The description is detailed and specific, though it is long for a PR description.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • 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

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 4830ea0 and d4c9ef8.

Files selected for processing: 17

  • 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/pdf.rs
  • crates/engine/src/cmd/fileio/save.rs
  • crates/engine/src/cmd/fileio/screens.rs
  • crates/engine/src/cmd/package.rs
  • crates/engine/src/tests/interactivity.rs
  • crates/engine/src/tests/package.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/tests/automation.rs
  • docs/mcp.md
  • docs/target-app-parity.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

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