fix(automation): close ambient filesystem authorization gaps (#763)

* fix(automation): block ambient filesystem bypasses

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e77cd34a-ce3e-4981-996f-4d37d49df90d

* test(automation): cover ambient path denial

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e77cd34a-ce3e-4981-996f-4d37d49df90d

* fix(automation): guard filesystem path authorization

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e85c024b-ace5-4415-9208-b8d4e4684505

* automation: judge the first non-empty prefs segment; refuse any path without data

prefs.set skips empty path segments, so `.scriptEvents.enabled` reached
script events past the section check. Preset import / plugin.install
with a whitespace path also read the filesystem.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Copilot-Session: e77cd34a-ce3e-4981-996f-4d37d49df90d
Copilot-Session: e85c024b-ace5-4415-9208-b8d4e4684505

SECURITY.md

@@ -49,4 +49,4 @@ General bugs without a security impact should use the project's normal issue and
 
 ## Current limitations
 
-TCP control connections require a 256-bit bearer token before method dispatch and enforce request-line, active-connection, and batch-step limits. These protections do not provide a capability model: an authenticated client still receives the complete exposed control surface. Automation file operations are not restricted to configured workspace roots, and JSON-depth, render, document-memory, command-duration, client-identity, encryption, and audit controls are not yet implemented. See the [security documentation](book/src/security/overview.md) for the source-backed status and hardening roadmap.
+TCP control connections require a 256-bit bearer token before method dispatch and enforce request-line, active-connection, and batch-step limits. Authentication does not provide per-client or per-tool capabilities, so an authenticated client still receives the exposed control surface. Remote filesystem methods use separately configured read and write workspace roots, and engine commands that still use ambient filesystem paths are denied at the automation boundary. Automation can change individual safe preferences, but whole-preferences updates and filesystem-bearing preference sections are refused; applying the Preferences dialog through automation is also refused. JSON-depth, render, document-memory, command-duration, client-identity, encryption, and audit controls are not yet implemented. See the [security documentation](book/src/security/overview.md) for the source-backed status and hardening roadmap.

book/src/security/automation-security.md

@@ -34,11 +34,17 @@ Automation is a privilege boundary because requests can cause filesystem access,
 - Engine commands that still use ambient paths fail closed at the automation boundary. Synthetic
   UI input reaches the same command policy, and automation-triggered file hooks do not run
   user-configured script-event paths.
+- `prefs.set` applies the engine's dotted-path normalization before checking for filesystem-bearing
+  preference sections, including paths with leading or repeated dots. It refuses whole-preferences
+  updates and those sections; individual safe preference keys remain available.
+- Applying the Preferences dialog with `ui.dialog.apply` is deliberately refused over automation.
 
 ## Known limitations
 
 - no per-client or per-tool capabilities;
 - no general per-client or per-tool capability model beyond filesystem read/write authority;
+- no automated bulk apply of the Preferences dialog; agents can change individual safe keys with
+  `prefs.set`;
 - no explicit JSON-depth policy, aggregate document/session-memory accounting, compositor
   scratch-space accounting, or command-duration/cancellation budget;
 - one thread per accepted TCP connection;

book/src/security/threat-model.md

@@ -39,15 +39,15 @@ A compact file declares oversized dimensions, excessive channels, nested descrip
 
 ### Filesystem authority abuse
 
-An automation client supplies an absolute path, traversal sequence, symlink, junction, or replaced path to read or overwrite data outside the intended project. Current automation uses caller-supplied OS paths and has no workspace policy. This scenario is a known limitation.
+An automation client supplies an absolute path, traversal sequence, symlink, junction, or replaced path to read or overwrite data outside the intended project. Remote filesystem methods use separately configured directory capabilities, and engine commands that still require ambient paths fail closed at the automation boundary. This is not yet a general per-command capability model, and trusted desktop operations can still use ambient paths.
 
 ### Local control takeover
 
-Another process on the same host connects to the loopback control server, discovers methods, drives UI input, executes commands, or opens/saves files. Loopback reduces network exposure but does not authenticate the client.
+Another process on the same host connects to the loopback control server, discovers methods, drives UI input, executes commands, or opens/saves files. A 256-bit bearer token is required before method dispatch, but authentication does not grant per-client or per-tool capabilities.
 
 ### Resource exhaustion
 
-A client opens many TCP connections, sends an extremely long JSON line, creates a huge batch, requests full-size rendering, or repeatedly invokes expensive commands. Current automation transports lack explicit request, batch, and connection budgets.
+A client opens many TCP connections, sends an extremely long JSON line, creates a huge batch, requests full-size rendering, or repeatedly invokes expensive commands. Request-line, connection, batch, and preview limits mitigate some cases; explicit JSON-depth, document-memory, and command-duration budgets remain absent.
 
 ### Supply-chain compromise
 

crates/automation/src/workspace.rs

@@ -180,6 +180,7 @@ pub fn authorize_engine_command(id: &str, params: &Value) -> Result<(), Automati
                 | "layer.videoLayers.reloadFrame"
                 | "image.applyDataSet"
         )
+        || command_uses_ambient_path(id, params)
         || profile_command_may_read_ambient(id, params)
         || preferences_may_grant_ambient_paths(id, params)
         || params_contain_ambient_path(id, params)
@@ -217,6 +218,7 @@ fn params_contain_ambient_path(id: &str, params: &Value) -> bool {
         "image.adjustments.colorLookup" | "layer.newAdjustmentLayer.colorLookup" | "layer.setAdjustment" => &["file"],
         "filter.distort.displace" => &["mapPath"],
         "layer.quickExportAsPng" | "layer.exportAs" => &["path"],
+        "image.mode.rgb" | "image.mode.grayscale" | "image.mode.cmyk" | "image.mode.lab" => &["profile"],
         "edit.assignProfile" | "edit.convertToProfile" | "edit.profileInfo" | "view.proofSetup" | "view.gamutWarning" => &["profile"],
         "edit.colorSettings" => &["workingRgb", "workingCmyk", "workingGray"],
         _ => &[],
@@ -243,16 +245,29 @@ fn preferences_may_grant_ambient_paths(id: &str, params: &Value) -> bool {
     if id != "prefs.set" {
         return false;
     }
-    let direct = params
-        .get("path")
-        .and_then(Value::as_str)
-        .is_some_and(|path| path == "colorSettings" || path.starts_with("colorSettings.") || path == "scriptEvents" || path.starts_with("scriptEvents."));
-    let batch = params.get("values").and_then(Value::as_object).is_some_and(|values| {
-        values.keys().any(|path| path == "colorSettings" || path.starts_with("colorSettings.") || path == "scriptEvents" || path.starts_with("scriptEvents."))
-    });
+    let direct = params.get("path").and_then(Value::as_str).is_some_and(preference_uses_ambient_filesystem);
+    let batch = params.get("values").and_then(Value::as_object).is_some_and(|values| values.keys().any(|path| preference_uses_ambient_filesystem(path)));
     direct || batch
 }
 
+fn preference_uses_ambient_filesystem(path: &str) -> bool {
+    // The engine skips empty segments (`.scriptEvents.enabled` sets `scriptEvents.enabled`), so
+    // judge the first non-empty one; no segment at all is the whole-preferences update.
+    let Some(section) = path.split('.').find(|segment| !segment.is_empty()) else { return true };
+    matches!(section, "colorSettings" | "scriptEvents" | "historyLog" | "plugIns" | "scratchDisks")
+}
+
+fn command_uses_ambient_path(id: &str, params: &Value) -> bool {
+    match id {
+        "brush.presets.importAbr" | "gradient.presets.importGrd" | "plugin.install" => {
+            // `data` wins over `path` in these commands; any `path` without it reads the filesystem.
+            params.get("data").is_none() && params.get("path").is_some()
+        }
+        "plugin.reload" => true,
+        _ => false,
+    }
+}
+
 fn looks_like_path(value: &str) -> bool {
     value.contains('/')
         || value.contains('\\')
@@ -440,6 +455,8 @@ mod tests {
         }
         assert!(authorize_engine_command("image.mode.cmyk", &serde_json::json!({})).is_ok());
         assert!(authorize_desktop_engine_command("image.mode.cmyk", &serde_json::json!({})).is_err());
+        assert!(authorize_engine_command("image.mode.rgb", &serde_json::json!({"profile": "/outside/profile.icc"})).is_err());
+        assert!(authorize_engine_command("image.mode.rgb", &serde_json::json!({"profile": "srgb"})).is_ok());
         assert!(authorize_engine_command("filter.distort.displace", &serde_json::json!({"mapPath": "outside.png"})).is_err());
         assert!(authorize_engine_command("layer.setAdjustment", &serde_json::json!({"file": "outside.cube"})).is_err());
         assert!(authorize_engine_command("prefs.set", &serde_json::json!({"path": "colorSettings.workingRgb", "value": "outside.icc"})).is_err());
@@ -447,4 +464,144 @@ mod tests {
         assert!(authorize_engine_step("actions.play", &serde_json::json!({})).is_ok());
         assert!(authorize_desktop_engine_step("file.saveACopy", &serde_json::json!({})).is_err());
     }
+
+    #[test]
+    fn automation_rejects_ambient_path_commands_and_preferences() {
+        for (id, params) in [
+            ("brush.presets.importAbr", serde_json::json!({"path": "/outside/set.abr"})),
+            ("gradient.presets.importGrd", serde_json::json!({"path": "/outside/set.grd"})),
+            ("plugin.install", serde_json::json!({"path": "/outside/plugin.wasm"})),
+            ("plugin.reload", serde_json::json!({"path": "/outside/plugins"})),
+            ("plugin.reload", serde_json::json!({})),
+            ("plugin.install", serde_json::json!({"path": " "})),
+        ] {
+            assert!(authorize_engine_command(id, &params).is_err(), "{id}: {params}");
+        }
+        for (id, params) in [
+            ("brush.presets.importAbr", serde_json::json!({"data": "QUJD"})),
+            ("gradient.presets.importGrd", serde_json::json!({"data": "QUJD"})),
+            ("plugin.install", serde_json::json!({"data": "QUJD"})),
+        ] {
+            assert!(authorize_engine_command(id, &params).is_ok(), "{id}: {params}");
+        }
+        for path in [
+            "",
+            "colorSettings",
+            "colorSettings.workingRgb",
+            "scriptEvents",
+            "scriptEvents.enabled",
+            ".scriptEvents",
+            ".scriptEvents.enabled",
+            "historyLog.filePath",
+            ".historyLog.filePath",
+            "plugIns.additionalPluginsFolder",
+            "scratchDisks.disks",
+            ".",
+            "..historyLog.filePath",
+        ] {
+            assert!(authorize_engine_command("prefs.set", &serde_json::json!({"path": path, "value": {}})).is_err(), "{path}");
+        }
+        assert!(
+            authorize_engine_command("prefs.set", &serde_json::json!({"values": {"interface.language": "fr", "historyLog.filePath": "/outside/log"}})).is_err()
+        );
+        assert!(authorize_engine_command("prefs.set", &serde_json::json!({"path": "interface.language", "value": "fr"})).is_ok());
+    }
+
+    #[test]
+    fn registry_filesystem_path_params_are_classified() {
+        let probe = serde_json::json!({
+            "path": "/outside/photocraft-probe",
+            "file": "/outside/photocraft-probe.icc",
+            "mapPath": "/outside/photocraft-probe.png",
+            "profile": "/outside/photocraft-probe.icc",
+            "workingRgb": "/outside/photocraft-probe.icc",
+            "workingCmyk": "/outside/photocraft-probe.icc",
+            "workingGray": "/outside/photocraft-probe.icc",
+            "input": "/outside/photocraft-input",
+            "output": "/outside/photocraft-output",
+            "paths": ["/outside/photocraft-probe.psd"],
+        });
+        let preference_probe = serde_json::json!({"path": ".scriptEvents", "value": {}});
+        let unclassified: Vec<_> = photocraft_engine::command_specs()
+            .iter()
+            .filter(|spec| documents_filesystem_path_params(spec.id, spec.params))
+            .filter(|spec| {
+                let params = if spec.id == "prefs.set" { &preference_probe } else { &probe };
+                authorize_engine_command(spec.id, params).is_ok()
+            })
+            .map(|spec| spec.id)
+            .collect();
+        assert!(unclassified.is_empty(), "filesystem path parameters in the command registry need an automation policy: {unclassified:?}");
+    }
+
+    fn documents_filesystem_path_params(id: &str, params: &str) -> bool {
+        if matches!(id, "prefs.get" | "prefs.reset") {
+            return false;
+        }
+        // These registry descriptions refer to document vector paths, not host files.
+        if matches!(
+            id,
+            "filter.blurGallery.pathBlur"
+                | "filter.render.flame"
+                | "shape.create"
+                | "shape.edit"
+                | "shape.info"
+                | "shape.presets.list"
+                | "shape.presets.new"
+                | "path.list"
+                | "path.info"
+                | "path.set"
+                | "path.transform"
+                | "path.clippingPath.set"
+                | "path.rename"
+                | "select.toWorkPath"
+                | "layer.vectorMask.add"
+                | "layer.vectorMask.edit"
+                | "layer.vectorMask.info"
+                | "paint.symmetryFromPath"
+                | "edit.defineCustomShape"
+                | "layer.combineShapes.unite"
+                | "layer.combineShapes.subtractFrontShape"
+                | "layer.combineShapes.intersectShapeAreas"
+                | "layer.combineShapes.excludeOverlappingShapes"
+                | "layer.combineShapes.mergeShapeComponents"
+        ) {
+            return false;
+        }
+        params.to_ascii_lowercase().contains("path")
+    }
+
+    #[test]
+    fn whole_preferences_update_cannot_enable_automation_script_events() {
+        let mut headless = crate::headless::Headless::new();
+        let result = headless.command_run(
+            "prefs.set",
+            serde_json::json!({
+                "path": "",
+                "value": {
+                    "scriptEvents": {
+                        "enabled": true,
+                        "bindings": [{
+                            "event": "newDocument",
+                            "steps": [["file.saveACopy", {"path": "/outside/canary.psd"}]]
+                        }]
+                    }
+                }
+            }),
+        );
+        assert!(result.is_err());
+
+        for (id, params) in [
+            ("brush.presets.importAbr", serde_json::json!({"path": "/outside/set.abr"})),
+            ("gradient.presets.importGrd", serde_json::json!({"path": "/outside/set.grd"})),
+            ("plugin.install", serde_json::json!({"path": "/outside/plugin.wasm"})),
+            ("plugin.reload", serde_json::json!({"path": "/outside/plugins"})),
+            ("prefs.set", serde_json::json!({"path": "historyLog.filePath", "value": "/outside/log"})),
+            ("prefs.set", serde_json::json!({"values": {"interface.language": "fr", "historyLog.filePath": "/outside/log"}})),
+        ] {
+            assert!(headless.command_run(id, params).is_err(), "{id}");
+        }
+        headless.command_run("file.new", serde_json::json!({"width": 5, "height": 5})).unwrap();
+        assert!(headless.session.file_menu.event_log.is_empty());
+    }
 }

docs/control-protocol.md

@@ -89,7 +89,7 @@ store expose and persist the same setting; scripts keep using canonical command
 
 UI-level commands (`view.zoomIn`, `window.theme.pro`, `edit.search`, …) are also accepted by `engine.execute` and `ui.menu.invoke`.
 
-Commands that read or write files by path, instead of through the automation roots, are refused with "automation command `…` uses ambient filesystem paths and is disabled; use capability-scoped document methods". That covers every `file.*` command except `file.new`, the `file.close*` commands and a few path-free ones such as `file.fileInfo`, so `file.open`, `file.save`, `file.saveAs` and `file.saveACopy` always fail here: open and save with `app.open` / `app.save`. Path parameters of other commands (`layer.exportAs {path}`, `filter.distort.displace {mapPath}`, …) are refused the same way, and the desktop app also refuses `image.mode.*`, which can load the colour profiles set in its preferences. The rules are in `crates/automation/src/workspace.rs`.
+Commands that read or write files by path, instead of through the automation roots, are refused with "automation command `…` uses ambient filesystem paths and is disabled; use capability-scoped document methods". That covers every `file.*` command except `file.new`, the `file.close*` commands and a few path-free ones such as `file.fileInfo`, so `file.open`, `file.save`, `file.saveAs` and `file.saveACopy` always fail here: open and save with `app.open` / `app.save`. Path parameters of other commands (`layer.exportAs {path}`, `filter.distort.displace {mapPath}`, preset imports, and plug-in install/reload) are refused the same way. `prefs.set` rejects whole-preference updates and file-backed sections (`colorSettings`, `scriptEvents`, `historyLog`, `plugIns` and `scratchDisks`) so automation cannot configure ambient file access indirectly. The desktop app also refuses `image.mode.*`, which can load the colour profiles set in its preferences. The rules are in `crates/automation/src/workspace.rs`.
 
 `type.editText` starts inline editing of the active type layer and selects all its text, like
 double-clicking its thumbnail in Layers. Text input, Commit and Cancel use the existing Type tool