Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 43 additions & 6 deletions crates/registry-coordinator/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{CoordinatorError, Result};
/// Changing this identity requires a new Workflow revision.
pub const PURE_PROFILE: &str = "coordinator/bounded-pure/v1";

pub const INTERPRETER_ABI: &str = "coordinator/pure-rhai-1.27.0/v2;named-input-object;raw-arithmetic-logic-string-more-string-deterministic-math-array-map-iterator-instant;ops=100000;calls=32;expr=64;string=16384;array=256;map=256;source=65536;json=131072;depth=32";
pub const INTERPRETER_ABI: &str = "coordinator/pure-rhai-1.27.0/v3;named-input-object;raw-arithmetic-logic-string-more-string-deterministic-math-array-map-iterator-instant;ops=100000;calls=32;expr=64;string=16384;array=256;map=256;source=65536;json=131072;depth=32";

pub(crate) const MAXIMUM_FUNCTION_OPERATIONS: u32 = 100_000;
pub(crate) const MAXIMUM_FUNCTION_CALL_LEVELS: u32 = 32;
Expand Down Expand Up @@ -49,7 +49,7 @@ const PROFILE: RhaiProfile = RhaiProfile {
map_entries: MAXIMUM_FUNCTION_MAP_ENTRIES as usize,
},
base: RhaiBaseEngine::Raw,
disabled_symbols: &["import", "export", "eval", "print", "debug"],
disabled_symbols: &["import", "export", "eval", "print", "debug", "curry"],
allow_anonymous_fn: false,
maximum_source_bytes: MAXIMUM_FUNCTION_SOURCE_BYTES as usize,
};
Expand Down Expand Up @@ -91,6 +91,7 @@ impl Functions {
let mut entries = BTreeMap::new();
let mut findings = Vec::new();
let mut reported = BTreeSet::new();
let mut refusals = BTreeSet::new();
for (name, function) in definitions {
let result = (|| {
let source = sources
Expand Down Expand Up @@ -124,6 +125,9 @@ impl Functions {
let ast = platform::compile_entrypoint(&profile(function), source, &function.entrypoint, &[1])
.map_err(|error| match error {
platform::RhaiCompileError::SourceBound => failure("function.source_limit").at(&function.source, "/"),
platform::RhaiCompileError::Parse(position) if refused_only_for_curry(source) => CoordinatorError::new("function.definition", "function source uses `curry`, which Coordinator Functions do not offer")
.at(&function.source, format!("line {} column {}", position.line().unwrap_or(0), position.position().unwrap_or(0)))
.suggest("Pass the value as an argument where the function is called, for example add(1, x) instead of Fn(\"add\").curry(1).call(x)."),
platform::RhaiCompileError::Parse(position) => CoordinatorError::new("function.definition", syntax_refusal(source))
.at(&function.source, format!("line {} column {}", position.line().unwrap_or(0), position.position().unwrap_or(0)))
.suggest("Correct the named Function source syntax and rerun check."),
Expand Down Expand Up @@ -180,7 +184,12 @@ impl Functions {
}
}
}
Err(error) => findings.push(error),
// A refusal of a shared source is reported once too.
Err(error) => {
if refusals.insert(error.to_string()) {
findings.push(error);
}
}
}
}
if findings.is_empty() {
Expand Down Expand Up @@ -319,6 +328,19 @@ fn located(name: &str, error: &EvalAltResult) -> String {
}
}

/// Whether `source` parses once `curry` is allowed, so `curry` is the only
/// reason it was refused. The profile disables `curry` because Rhai does not
/// count data curried into a function pointer against the size limits.
fn refused_only_for_curry(source: &str) -> bool {
const WITH_CURRY: RhaiProfile = RhaiProfile {
disabled_symbols: &["import", "export", "eval", "print", "debug"],
..PROFILE
};
platform::build_engine(&WITH_CURRY, None, |_| {})
.compile(source)
.is_ok()
}

/// The parser's own reason for refusing authored source, which names source
/// text and never a run value.
fn syntax_refusal(source: &str) -> String {
Expand Down Expand Up @@ -612,10 +634,9 @@ fn pad_text(

/// Calls the interpreter answers itself, so its function table does not list
/// them.
const INTERPRETER_CALLS: [&str; 7] = [
const INTERPRETER_CALLS: [&str; 6] = [
"Fn",
"call",
"curry",
"is_def_fn",
"is_def_var",
"is_shared",
Expand Down Expand Up @@ -969,13 +990,29 @@ mod tests {
);
}

#[test]
fn curry_is_refused_in_every_call_form() {
// Rhai does not count data curried into a function pointer against
// the size limits, so the profile leaves `curry` out.
assert!(!helpers().contains("curry"));
for script in [
r#"fn g(x) { x } curry(Fn("g"), 1)"#,
r#"fn g(x) { x } Fn("g").curry(1)"#,
r#"fn g(x) { x } Fn("curry")"#,
] {
assert!(
engine().eval::<rhai::Dynamic>(script).is_err(),
"curry accepted in: {script}"
);
}
}

#[test]
fn the_interpreters_own_calls_are_listed_and_run() {
for (name, script) in [
("type_of", r#"type_of(1)"#),
("Fn", r#"fn g(x) { x } Fn("g")"#),
("call", r#"fn g(x) { x } call(Fn("g"), 1)"#),
("curry", r#"fn g(x) { x } curry(Fn("g"), 1)"#),
("is_def_fn", r#"is_def_fn("g", 1)"#),
("is_def_var", r#"is_def_var("x")"#),
("is_shared", r#"let x = 1; is_shared(x)"#),
Expand Down
15 changes: 14 additions & 1 deletion crates/registry-coordinator/tests/authoring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ fn snapshot_restore_refuses_the_v1_interpreter_that_lacked_the_string_and_number
assert!(value["interpreterAbi"]
.as_str()
.unwrap()
.starts_with("coordinator/pure-rhai-1.27.0/v2;"));
.starts_with("coordinator/pure-rhai-1.27.0/v3;"));
value["interpreterAbi"] = json!(
"coordinator/pure-rhai-1.27.0/v1;named-input-object;raw-arithmetic-logic-string-array-map-iterator-instant;ops=100000;calls=32;expr=64;string=16384;array=256;map=256;source=65536;json=131072;depth=32"
);
Expand All @@ -488,6 +488,19 @@ fn snapshot_restore_refuses_the_v1_interpreter_that_lacked_the_string_and_number
);
}

#[test]
fn snapshot_restore_refuses_the_v2_interpreter_that_allowed_curry() {
let snapshot = fixture().snapshot().unwrap();
let mut value: Value = serde_json::from_str(&snapshot).unwrap();
value["interpreterAbi"] = json!(
"coordinator/pure-rhai-1.27.0/v2;named-input-object;raw-arithmetic-logic-string-more-string-deterministic-math-array-map-iterator-instant;ops=100000;calls=32;expr=64;string=16384;array=256;map=256;source=65536;json=131072;depth=32"
);
assert_eq!(
refused(Definition::from_snapshot(&value.to_string())).code,
"definition.abi"
);
}

fn canonical(value: &Value) -> String {
String::from_utf8(registry_platform_canonical_json::canonicalize_json(value).unwrap()).unwrap()
}
Expand Down
51 changes: 49 additions & 2 deletions crates/registry-coordinator/tests/helper_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ fn explain_functions_lists_the_helpers_of_the_profile() {
"timestamp",
"print",
"eval",
"curry",
"rand",
"sin",
"exp",
Expand All @@ -263,7 +264,7 @@ fn explain_functions_lists_the_helpers_of_the_profile() {
sorted.sort_unstable();
sorted.dedup();
assert_eq!(helpers, sorted, "sorted and without repeats");
assert!(report["interpreterAbi"].as_str().unwrap().contains("/v2;"));
assert!(report["interpreterAbi"].as_str().unwrap().contains("/v3;"));
assert_eq!(report["limits"]["stringBytes"], 16384);

let human = cli(&["explain", "functions"]);
Expand Down Expand Up @@ -500,6 +501,52 @@ fn a_result_holding_nan_or_infinity_is_refused_not_turned_into_null() {
);
}

#[test]
fn a_function_that_curries_is_refused_at_check_with_a_way_forward() {
let root = tempfile::tempdir().unwrap();
write_project(
root.path(),
"fn add(a, b) { a + b }\nfn probe(inputs) { let f = Fn(\"add\").curry(1); f.call(2) }\n",
);
let Err(error) = Definition::load(root.path()) else {
panic!("a Function that curries was accepted");
};
assert_eq!(error.code, "coordinator.function.definition");
assert!(error.message.contains("curry"), "{}", error.message);
let hint = error.suggested_action.as_deref().unwrap_or_default();
assert!(hint.contains("curry"), "{hint}");
}

#[test]
fn a_refusal_in_a_shared_source_is_reported_once() {
// Two Functions read one source; its syntax refusal is one finding.
let root = tempfile::tempdir().unwrap();
write_project(
root.path(),
"fn add(a, b) { a + b }\nfn probe(inputs) { Fn(\"add\").curry(1) }\nfn other(inputs) { 1 }\n",
);
let path = root.path().join("workflow.yaml");
let mut workflow: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap();
workflow["functions"]["other"] = workflow["functions"]["probe"].clone();
workflow["functions"]["other"]["entrypoint"] = json!("other");
fs::write(&path, workflow.to_string()).unwrap();
let output = cli(&[
"--format",
"json",
"check",
"--project",
root.path().to_str().unwrap(),
]);
let report: Value = serde_json::from_slice(&output.stdout).unwrap();
let refusals = report["diagnostics"]
.as_array()
.unwrap()
.iter()
.filter(|d| d["code"] == "coordinator.function.definition")
.count();
assert_eq!(refusals, 1, "{report}");
}

fn unknown_call_names(source: &str) -> Vec<String> {
let root = tempfile::tempdir().unwrap();
write_project(root.path(), source);
Expand Down Expand Up @@ -551,7 +598,7 @@ fn a_function_name_given_as_a_string_literal_is_checked() {
fn probe(inputs) {
let a = call(Fn("nope_a"));
let b = Fn("nope_b").call();
let c = curry(Fn("nope_c"), 1);
let c = Fn("nope_c").call(1);
let d = [1].map(Fn("nope_d"));
let e = [1].map("nope_e");
let f = is_def_fn("nope_f", 1);
Expand Down
2 changes: 1 addition & 1 deletion crates/registry-coordinator/tests/nested_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ fn a_flat_mapping_is_unchanged() {
);
assert_eq!(
definition.digest,
"sha256:f50dbad620b01d86d3f6214c4acfe82f92e8db4c81442e3beb8c807a2a0c0801"
"sha256:862523b816685b1cd9b1ae8ee0c439b3a8db7519ca13b869e9fc0980fde3f5e2"
);
}

Expand Down
6 changes: 6 additions & 0 deletions products/coordinator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@
both answer 403 `operation.not-authorized`. Add `subjects` to every
`deployment.authentication.policies` entry: the client's own service-account
subject for a producer, and each operator's subject for an operator policy.
- BREAKING: Functions can no longer call `curry`, and the pure Function
interpreter ABI is now `v3`. Check refuses a source that uses `curry` and
suggests passing the value as an argument instead. Every definition frozen
by 0.45.0 records `v2` and is refused with `definition.abi`, whether or not
it uses `curry`: settle runs with the 0.45.0 binaries, then check and
promote the project again.

## 0.45.0 - 2026-11-03

Expand Down
7 changes: 6 additions & 1 deletion products/coordinator/SECURITY-MODEL.md
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,13 @@ attempt recomputes, and a changed engine could reinterpret a frozen definition.
expression levels, 65,536 source bytes, 16,384-byte strings, 256 array items,
256 map members, 131,072 JSON bytes and depth 32. The profile has no clock,
sleep or I/O.
- `curry` is disabled, because Rhai does not count data curried into a
function pointer against these limits and every copy of the pointer copies
that data. Check refuses a source that uses it and names `curry`
(`curry_is_refused_in_every_call_form`,
`a_function_that_curries_is_refused_at_check_with_a_way_forward`).
- The interpreter identity in each frozen definition (`INTERPRETER_ABI` in
`src/functions.rs`) names Rhai 1.27.0 and ABI `v2`, because the callable set
`src/functions.rs`) names Rhai 1.27.0 and ABI `v3`, because the callable set
is part of what a definition means. A definition frozen under another identity
is refused with `definition.abi`, and a live run under it is held
(COORDINATOR-INV-12); Coordinator never reinterprets an old snapshot under
Expand Down
Loading