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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions kiln-component/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ criterion = { version = "0.8", features = ["html_reports"] }
# Build + decode synthetic components in unit tests (E5DC2 / #382).
wat = "1.244"
kiln-decoder = { workspace = true, features = ["std"] }
# Engine-driving tests contend on the global capability memory budget
# (see CLAUDE.md — such tests use serial_test).
serial_test = "3.4"

[features]
# By default, enable std to match kiln-runtime's default behavior
Expand Down
90 changes: 83 additions & 7 deletions kiln-component/src/components/component_instantiation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2449,6 +2449,43 @@ impl ComponentInstance {
tracing::info!(?main_handle, "Main instance selected (exports _start)");
}

// SR-53 / #443: validate each `canon lift`'s declared param arity
// against the backing core function in the LIVE instantiated
// module — the exact function `call_direct_export` would execute
// on `main_handle`. A spec-invalid component whose lift declares
// a different param arity than its core function is REJECTED at
// load, matching `wasm-tools validate` / wasmtime ("lowered
// parameter types do not match parameter types of core
// function"). On this scalar-only direct-hosting path every
// scalar param lowers to exactly one core param, so the counts
// must be equal; lifts with non-scalar params cannot be invoked
// on this path at all (argument lowering fails loud), so no
// mismatch can execute either way.
for target in instance.direct_export_targets.values() {
let all_params_scalar = target
.params
.iter()
.all(|vt| Self::format_val_type_to_scalar_component_type(vt).is_ok());
if !all_params_scalar {
continue;
}
let module = engine.get_instance(main_handle)?.module().clone();
if let Some(func_idx) = module.find_function_by_name(&target.core_export_name) {
let signature = module.get_function_signature(func_idx).ok_or_else(|| {
Error::validation_error(
"lifted core export has no resolvable function signature",
)
})?;
if signature.params.len() != target.params.len() {
return Err(Error::validation_error(
"canon lift parameter arity does not match the backing \
core function's parameter count (spec-invalid \
component; wasm-tools validate rejects this)",
));
}
}
}

// Store the engine AFTER main_handle search (which may have swapped
// the engine with a nested component's engine for P3 components)
instance.runtime_engine = Some(Box::new(engine));
Expand Down Expand Up @@ -2892,8 +2929,20 @@ impl ComponentInstance {
_ => None,
};

// Convert the resolved component-level result types into the canonical
// ABI `ComponentType` used by the callable function signature.
// Convert the resolved component-level param/result types into the
// canonical ABI `ComponentType` used by the callable function
// signature. Params were previously NOT resolved (always empty),
// which made `validate_function_args`'s arity check pass trivially
// and let a param-taking export run with zero-filled core
// parameters (SR-53, #443).
let resolved_params: Vec<ComponentType> = match &direct_target {
Some(t) => t
.params
.iter()
.map(Self::format_val_type_to_scalar_component_type)
.collect::<Result<Vec<_>>>()?,
None => Vec::new(),
};
let resolved_returns: Vec<ComponentType> = match &direct_target {
Some(t) => t
.returns
Expand Down Expand Up @@ -2968,12 +3017,21 @@ impl ComponentInstance {
handle: export.idx as FunctionHandle,
signature: FunctionSignature {
name: export_name.clone(),
// Real resolved param types so `validate_function_args`
// enforces argument arity instead of passing trivially
// against an always-empty param list (SR-53, #443).
#[cfg(feature = "std")]
params: Vec::new(),
params: resolved_params.clone(),
#[cfg(not(feature = "std"))]
params: {
use kiln_foundation::bounded::BoundedVec;
BoundedVec::new()
let mut pv = BoundedVec::new();
for param in &resolved_params {
pv.push(param.clone()).map_err(|_| {
Error::validation_error("Too many function parameters")
})?;
}
pv
},
// Real resolved result types so invocation knows how to
// lift the core result (#344).
Expand Down Expand Up @@ -3121,13 +3179,17 @@ impl ComponentInstance {
if component_func_idx == export_func_idx {
// Found the lift backing this export. Resolve its core export
// name via the `core func` alias for `core_func_idx`, and its
// result types via the component function type at `type_idx`.
// param/result types via the component function type at
// `type_idx`.
let core_export_name =
Self::resolve_core_func_export_name(parsed, *core_func_idx)?;

let returns = match parsed.types.get(*type_idx as usize) {
let (params, returns) = match parsed.types.get(*type_idx as usize) {
Some(ty) => match &ty.definition {
ComponentTypeDefinition::Function { results, .. } => results.clone(),
ComponentTypeDefinition::Function { params, results } => (
params.iter().map(|(_, vt)| vt.clone()).collect::<Vec<_>>(),
results.clone(),
),
_ => {
return Err(Error::component_resource_lifecycle_error(
"canon lift type index does not refer to a function type",
Expand All @@ -3143,6 +3205,7 @@ impl ComponentInstance {

return Ok(Some(crate::types::DirectExportTarget {
core_export_name,
params,
returns,
}));
}
Expand Down Expand Up @@ -4514,6 +4577,19 @@ impl ComponentInstance {
})?
.clone();

// SR-53 / #443: the supplied argument count must match the lift's
// declared parameter arity. This mirrors the result-arity check below;
// without it a param-taking export invoked with no args ran with
// zero-filled core parameters and its wrong result was reported as
// success. (`validate_function_args` performs the same check on the
// `call_function` path; this guards direct calls too.)
if args.len() != target.params.len() {
return Err(Error::runtime_type_mismatch(
"direct-hosting argument count does not match the lifted \
component function parameter count",
));
}

// Lower scalar component arguments to core WASM values. FAIL LOUD on
// non-scalar arguments — only scalars are supported on this path.
let wasm_args: Vec<Value> = args
Expand Down
3 changes: 3 additions & 0 deletions kiln-component/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ pub struct CommandEntry {
pub struct DirectExportTarget {
/// Export name of the backing core function (callable on the engine).
pub core_export_name: String,
/// Component-level parameter types declared by the `canon lift`'s function
/// type. Supplied arguments must match this arity (SR-53, #443).
pub params: Vec<kiln_format::component::FormatValType>,
/// Component-level result types to lift the core result into.
pub returns: Vec<kiln_format::component::FormatValType>,
}
Expand Down
112 changes: 112 additions & 0 deletions kiln-component/tests/direct_export_arity_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
//! Direct-hosting argument-arity enforcement tests (SR-53, #443).
//!
//! Two manifestations of the same missing check:
//!
//! 1. Invocation: `call_direct_export` validated only RESULT arity, and the
//! export's registered `FunctionSignature.params` was always empty — so
//! `validate_function_args` passed trivially and a param-taking lifted
//! export invoked with no args ran with ZERO-FILLED core params, its wrong
//! result reported as success.
//! 2. Load: a spec-invalid component whose `canon lift` declares a different
//! param arity than the backing core function was ACCEPTED and run, though
//! `wasm-tools validate` and wasmtime reject it.

#![cfg(all(feature = "std", feature = "kiln-execution"))]

use kiln_component::canonical_abi::ComponentValue;
use kiln_component::components::component_instantiation::ComponentInstance;

/// A valid component: `add: func(a: u32, b: u32) -> u32` lifted from a core
/// `add (param i32 i32) (result i32)`.
fn valid_add_component() -> Vec<u8> {
wat::parse_str(
r#"
(component
(core module $m
(func (export "add") (param i32 i32) (result i32)
(i32.add (local.get 0) (local.get 1))))
(core instance $i (instantiate $m))
(func $add (param "a" u32) (param "b" u32) (result u32)
(canon lift (core func $i "add")))
(export "add" (func $add)))
"#,
)
.expect("valid fixture must assemble as a component")
}

/// The #443 second manifestation: the lift declares ONE param but the backing
/// core function takes TWO. `wasm-tools validate` rejects this ("lowered
/// parameter types [I32] do not match parameter types [I32, I32] of core
/// function 0"); kilnd accepted and ran it.
fn param_arity_mismatch_component() -> Vec<u8> {
wat::parse_str(
r#"
(component
(core module $m
(func (export "add") (param i32 i32) (result i32)
(i32.add (local.get 0) (local.get 1))))
(core instance $i (instantiate $m))
(func $add (param "a" u32) (result u32)
(canon lift (core func $i "add")))
(export "add" (func $add)))
"#,
)
.expect("the invalid fixture is still syntactically well-formed WAT and must assemble")
}

fn instantiate(bytes: &[u8]) -> kiln_error::Result<ComponentInstance> {
let mut parsed =
Box::new(kiln_decoder::component::decode_component(bytes).expect("fixture must decode"));
ComponentInstance::from_parsed_with_handler(0, &mut parsed, None, None)
}

/// SR-53 / #443: invoking a 2-param lifted export with NO arguments must be
/// an ERROR via the existing `validate_function_args` arity check — not a
/// zero-filled core invocation reported as success. Before the fix the
/// export's registered signature had empty params, so `&[]` passed
/// validation and the core `add` ran as add(0, 0).
#[test]
#[serial_test::serial]
fn direct_export_invoked_with_missing_args_errors() {
let mut instance =
instantiate(&valid_add_component()).expect("valid component must instantiate");
assert!(
instance.call_function("add", &[], None).is_err(),
"invoking 'add' (2 params) with 0 arguments must fail loud, not run \
with zero-filled core parameters"
);
}

/// Matching arity must still work AND compute the correct answer:
/// add(5, 3) = 8. Before the fix this direction was ALSO broken — the empty
/// registered param signature made `validate_function_args` reject the
/// correctly-supplied arguments (2 args vs 0 declared params).
#[test]
#[serial_test::serial]
fn direct_export_with_matching_args_computes_correct_answer() {
let mut instance =
instantiate(&valid_add_component()).expect("valid component must instantiate");
let results = instance
.call_function(
"add",
&[ComponentValue::U32(5), ComponentValue::U32(3)],
None,
)
.expect("matching-arity invocation must succeed");
assert_eq!(results, vec![ComponentValue::U32(8)], "add(5, 3) must be 8");
}

/// SR-53 / #443 (load-time): a `canon lift` whose declared param arity
/// differs from the backing core function must be REJECTED at
/// decode/instantiation — matching wasm-tools and wasmtime — not accepted
/// and run. Before the fix `from_parsed_with_handler` succeeded and
/// `--invoke add` reported `✓ Execution completed successfully`.
#[test]
#[serial_test::serial]
fn canon_lift_param_arity_mismatch_rejected_at_load() {
assert!(
instantiate(&param_arity_mismatch_component()).is_err(),
"a canon lift declaring 1 param over a 2-param core function must be \
rejected at load, as wasm-tools validate and wasmtime do"
);
}
46 changes: 14 additions & 32 deletions kiln-runtime/src/stackless/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1794,40 +1794,22 @@ impl StacklessEngine {
"[EXEC] Function parameter info"
);

// Add provided arguments
for (i, arg) in args.iter().enumerate() {
if i < expected_param_count {
locals.push(arg.clone());
}
// SR-53 / #443: the argument count MUST match the function's
// declared parameter arity. The previous code zero-filled
// missing parameters (and silently truncated extras) — a
// banned masking fallback that turned wrong answers into
// reported successes (the #412 fabricated-reporting family).
// FAIL LOUD instead.
if args.len() != expected_param_count {
return Err(kiln_error::Error::runtime_type_mismatch(
"function invoked with wrong number of arguments: the \
argument count must match the function's declared \
parameter count (missing parameters are not zero-filled)",
));
}

// Pad with default values for missing parameters
if args.len() < expected_param_count {
for i in args.len()..expected_param_count {
let param_type = func_type.params.get(i)
.ok_or_else(|| kiln_error::Error::runtime_error(
"Parameter index out of bounds - type corrupted"
))?;
let default_value = match param_type {
kiln_foundation::ValueType::I32 => Value::I32(0),
kiln_foundation::ValueType::I64 => Value::I64(0),
kiln_foundation::ValueType::F32 => Value::F32(FloatBits32(0)),
kiln_foundation::ValueType::F64 => Value::F64(FloatBits64(0)),
kiln_foundation::ValueType::V128 => Value::V128(V128 { bytes: [0u8; 16] }),
kiln_foundation::ValueType::FuncRef => Value::FuncRef(None),
kiln_foundation::ValueType::NullFuncRef => Value::FuncRef(None),
kiln_foundation::ValueType::TypedFuncRef(_, _) => Value::FuncRef(None),
kiln_foundation::ValueType::ExternRef => Value::ExternRef(None),
kiln_foundation::ValueType::ExnRef => Value::ExnRef(None),
kiln_foundation::ValueType::AnyRef => Value::ExternRef(None),
kiln_foundation::ValueType::EqRef => Value::I31Ref(None),
kiln_foundation::ValueType::I31Ref => Value::I31Ref(None),
kiln_foundation::ValueType::StructRef(_) => Value::StructRef(None),
kiln_foundation::ValueType::ArrayRef(_) => Value::ArrayRef(None),
_ => Value::I32(0),
};
locals.push(default_value);
}
for arg in &args {
locals.push(arg.clone());
}

#[cfg(feature = "tracing")]
Expand Down
38 changes: 38 additions & 0 deletions kilnd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,44 @@ impl KilndEngine {
let has_main = engine.has_function(instance, function_name).unwrap_or(false);

if has_main {
// SR-53 / #443: reject an argument-arity mismatch BEFORE
// invoking. kilnd has no CLI mechanism to pass wasm function
// parameters (--wasi-arg is WASI argv, not wasm params), so a
// function with >= 1 declared parameter can only ever be
// called with missing arguments. The engine used to zero-fill
// them and the wrong result was reported as success (the #412
// fabricated-reporting family). FAIL LOUD with an actionable
// message instead.
let declared_params = {
let inst = engine.get_instance(instance)?;
let func_idx =
inst.module().find_function_by_name(function_name).ok_or_else(|| {
Error::runtime_function_not_found("Function not found in exports")
})?;
inst.module()
.get_function_signature(func_idx)
.ok_or_else(|| {
Error::runtime_function_not_found(
"Function signature not found for export",
)
})?
.params
.len()
};
if declared_params != 0 {
// User-facing diagnostic on stderr: says WHY the call is
// rejected and that kilnd cannot supply wasm params yet.
eprintln!(
"Error: function '{}' expects {} argument(s) but none were \
supplied; kilnd cannot yet pass wasm function parameters",
function_name, declared_params
);
return Err(Error::runtime_type_mismatch(
"exported function expects arguments but none were supplied; \
kilnd cannot yet pass wasm function parameters",
));
}

// Propagate the engine's error verbatim so the actual cause
// (e.g. "fuel exhausted", a trap message) reaches the user
// instead of a generic "Function execution failed".
Expand Down
Loading
Loading