automation: the Scripting settings have two more spellings (#2131)

`Prefs::set` makes a JSON pointer of the key (`key.replace('.', "/")`), so
`scripting/allowScriptsWriteFiles` resolves as well as the dotted spelling, and
the page on its own (`{key: "scripting", value: {…}}`) replaces every setting on
it — `coerce` takes an object for an object, and the page deserializes with
`serde(default)`. A prefix check on `scripting.` let both through. The page is
now read from the key's first segment, split on either separator.

Found by CodeRabbit on the pull request; reproduced before fixing, with
`prefs.set {key: "scripting/allowScriptsWriteFiles", value: true}` answering
`{"scripting/allowScriptsWriteFiles": true}` under confinement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Anubhav Rai
2026-10-27 23:37:32 +05:30
co-authored by Claude Opus 5
parent 2d6e790fde
commit 471ef40e25
2 changed files with 13 additions and 1 deletions
+8 -1
View File
@@ -153,7 +153,7 @@ impl Confinement {
// (`effectcraft_engine::prefs::Scripting::same_permissions`), and neither may a confined
// agent: `prefs.set` saves the settings file, so a script run long after this session
// would still have it. (`prefs.reset` only puts the defaults back, which grant nothing.)
if id == "prefs.set" && prefs_keys(params).iter().any(|k| k.to_ascii_lowercase().starts_with("scripting.")) {
if id == "prefs.set" && prefs_keys(params).iter().any(|k| is_scripting_pref(k)) {
return Err(Error::BadArgs(
"`prefs.set`: the Scripting settings decide what a script may do, so they can't be changed while file access is confined".into(),
));
@@ -238,6 +238,13 @@ impl Confinement {
}
}
/// Whether a `prefs.set` key reaches the Scripting page. `Prefs::set` turns the key into a JSON
/// pointer (`key.replace('.', "/")`), so `scripting/allowScriptsWriteFiles` resolves as well as
/// the dotted spelling, and the page on its own replaces every setting on it.
fn is_scripting_pref(key: &str) -> bool {
key.split(['.', '/']).next().is_some_and(|page| page.eq_ignore_ascii_case("scripting"))
}
/// The settings `prefs.set` is given: `key`, and the keys of `values`.
fn prefs_keys(params: &Value) -> Vec<&str> {
let mut keys: Vec<&str> = params.get("key").and_then(Value::as_str).into_iter().collect();
+5
View File
@@ -1421,6 +1421,11 @@ fn automation_roots_refuse_running_script_code() {
json!({"key": "scripting.allow_scripts_write_files", "value": true}),
json!({"values": {"scripting.warnExecutingFiles": false}}),
json!({"values": {"general.showToolTips": false, "scripting.enableJsDebugger": true}}),
// `Prefs::set` makes a JSON pointer of the key, so this spelling resolves too…
json!({"key": "scripting/allowScriptsWriteFiles", "value": true}),
// …and the page on its own replaces every setting on it.
json!({"key": "scripting", "value": {"allowScriptsWriteFiles": true}}),
json!({"values": {"scripting": {"warnExecutingFiles": false}}}),
] {
let msg = refused(&mut s, "execute_command", json!({"command": "prefs.set", "params": params}));
assert!(msg.contains("what a script may do"), "{msg}");