From 15b3bab0d015378c46e4ce6098547acfae03ab2c Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Mon, 27 Apr 2026 23:28:12 +0300 Subject: [PATCH] fix(reborn): close foundation fail-closed gaps --- crates/ironclaw_host_api/src/approval.rs | 33 ++++++++++++++----- .../tests/host_api_contract.rs | 28 ++++++++++++++++ crates/ironclaw_resources/src/lib.rs | 20 +++++++++++ .../tests/resource_governor_contract.rs | 27 +++++++++++++++ 4 files changed, 100 insertions(+), 8 deletions(-) diff --git a/crates/ironclaw_host_api/src/approval.rs b/crates/ironclaw_host_api/src/approval.rs index 7f43213b603..67991cd1d44 100644 --- a/crates/ironclaw_host_api/src/approval.rs +++ b/crates/ironclaw_host_api/src/approval.rs @@ -47,7 +47,7 @@ impl InvocationFingerprint { input: &'a serde_json::Value, } - let canonical_input = canonical_json(input); + let canonical_input = canonical_json(input)?; let payload = Payload { version: 1, kind: "dispatch", @@ -67,21 +67,38 @@ impl InvocationFingerprint { } } -fn canonical_json(value: &serde_json::Value) -> serde_json::Value { +const MAX_CANONICAL_JSON_DEPTH: usize = 64; + +fn canonical_json(value: &serde_json::Value) -> Result { + canonical_json_at_depth(value, 0) +} + +fn canonical_json_at_depth( + value: &serde_json::Value, + depth: usize, +) -> Result { + if depth > MAX_CANONICAL_JSON_DEPTH { + return Err(HostApiError::invariant( + "canonical_json: max depth exceeded", + )); + } + match value { - serde_json::Value::Array(items) => { - serde_json::Value::Array(items.iter().map(canonical_json).collect()) - } + serde_json::Value::Array(items) => items + .iter() + .map(|item| canonical_json_at_depth(item, depth + 1)) + .collect::, _>>() + .map(serde_json::Value::Array), serde_json::Value::Object(map) => { let mut entries = map.iter().collect::>(); entries.sort_by_key(|(key, _)| *key); let mut canonical = serde_json::Map::new(); for (key, value) in entries { - canonical.insert(key.clone(), canonical_json(value)); + canonical.insert(key.clone(), canonical_json_at_depth(value, depth + 1)?); } - serde_json::Value::Object(canonical) + Ok(serde_json::Value::Object(canonical)) } - _ => value.clone(), + _ => Ok(value.clone()), } } diff --git a/crates/ironclaw_host_api/tests/host_api_contract.rs b/crates/ironclaw_host_api/tests/host_api_contract.rs index f819a7ae0bb..188764f260b 100644 --- a/crates/ironclaw_host_api/tests/host_api_contract.rs +++ b/crates/ironclaw_host_api/tests/host_api_contract.rs @@ -395,6 +395,34 @@ fn invocation_fingerprint_is_stable_and_input_hashed() { assert!(!first.as_str().contains("secret payload")); } +#[test] +fn invocation_fingerprint_rejects_deeply_nested_input() { + let ctx = sample_context(); + let capability = CapabilityId::new("echo.say").unwrap(); + let estimate = ResourceEstimate::default(); + let mut input = serde_json::Value::String("leaf".to_string()); + + for _ in 0..10_000 { + let mut object = serde_json::Map::new(); + object.insert("a".to_string(), input); + input = serde_json::Value::Object(object); + } + + // serde_json::Value drops nested objects recursively; leak this intentionally + // so the test exercises fingerprint rejection rather than Value teardown. + let input = Box::leak(Box::new(input)); + + let err = + InvocationFingerprint::for_dispatch(&ctx.resource_scope, &capability, &estimate, input) + .unwrap_err(); + + assert!(matches!( + err, + HostApiError::InvariantViolation { reason } + if reason == "canonical_json: max depth exceeded" + )); +} + #[test] fn invocation_fingerprint_changes_when_authorized_invocation_changes() { let ctx = sample_context(); diff --git a/crates/ironclaw_resources/src/lib.rs b/crates/ironclaw_resources/src/lib.rs index 4c5ec5407df..c699956e557 100644 --- a/crates/ironclaw_resources/src/lib.rs +++ b/crates/ironclaw_resources/src/lib.rs @@ -232,6 +232,11 @@ pub enum ResourceError { LimitExceeded(Box), #[error("resource reservation {id} already exists")] ReservationAlreadyExists { id: ResourceReservationId }, + #[error("invalid resource estimate for {dimension}: {reason}")] + InvalidEstimate { + dimension: ResourceDimension, + reason: &'static str, + }, #[error("resource reservation {id} does not match requested scope or estimate")] ReservationMismatch { id: ResourceReservationId }, #[error("unknown resource reservation {id}")] @@ -427,6 +432,8 @@ impl ResourceGovernor for InMemoryResourceGovernor { estimate: ResourceEstimate, reservation_id: ResourceReservationId, ) -> Result { + validate_estimate(&estimate)?; + let mut state = self.lock_state(); if state.reservations.contains_key(&reservation_id) { return Err(ResourceError::ReservationAlreadyExists { id: reservation_id }); @@ -566,6 +573,19 @@ impl ResourceGovernor for InMemoryResourceGovernor { } } +fn validate_estimate(estimate: &ResourceEstimate) -> Result<(), ResourceError> { + if let Some(usd) = estimate.usd + && usd < Decimal::ZERO + { + return Err(ResourceError::InvalidEstimate { + dimension: ResourceDimension::Usd, + reason: "must be non-negative", + }); + } + + Ok(()) +} + /// Returns the first denied dimension in canonical resource order. /// /// This intentionally reports one denial rather than aggregating all failed diff --git a/crates/ironclaw_resources/tests/resource_governor_contract.rs b/crates/ironclaw_resources/tests/resource_governor_contract.rs index 755be9e95e0..7f69362c466 100644 --- a/crates/ironclaw_resources/tests/resource_governor_contract.rs +++ b/crates/ironclaw_resources/tests/resource_governor_contract.rs @@ -60,6 +60,33 @@ fn reserve_with_id_uses_requested_identifier_and_rejects_duplicates() { )); } +#[test] +fn reserve_with_id_rejects_negative_usd_estimates() { + let governor = InMemoryResourceGovernor::new(); + let scope = sample_scope("tenant1", "user1", Some("project1")); + let account = ResourceAccount::tenant(scope.tenant_id.clone()); + + let err = governor + .reserve_with_id( + scope, + ResourceEstimate { + usd: Some(dec!(-100.00)), + ..ResourceEstimate::default() + }, + ResourceReservationId::new(), + ) + .unwrap_err(); + + assert!(matches!( + err, + ResourceError::InvalidEstimate { + dimension: ResourceDimension::Usd, + reason: "must be non-negative" + } + )); + assert_eq!(governor.reserved_for(&account).usd, dec!(0)); +} + #[test] fn usd_tally_saturates_instead_of_panicking_on_decimal_overflow() { let governor = InMemoryResourceGovernor::new();