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 crates/ironclaw_host_runtime/src/first_party.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ impl FirstPartyCapabilityRequest {
secret_store: None,
audit_sink: None,
unsafe_raw_diagnostics_allowed: false,
post_edit_check: None,
},
input,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ mod tests {
secret_store: None,
audit_sink: None,
unsafe_raw_diagnostics_allowed: false,
post_edit_check: None,
},
input,
}
Expand Down
68 changes: 62 additions & 6 deletions crates/ironclaw_host_runtime/src/first_party_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,51 @@ fn first_party_capability_manifest(
pub struct BuiltinFirstPartyTools {
coding_state: CodingCapabilityState,
memory_state: memory::MemoryCapabilityState,
post_edit_check_seen: crate::post_edit_check::PostEditCheckSeenLines,
}

impl BuiltinFirstPartyTools {
/// Run the operator-configured post-edit check after a SUCCESSFUL
/// `write_file` / `apply_patch` and append the advisory `post_edit_check`
/// value to the edit's model-visible output. Read-only coding tools never
/// trigger it, and it never fails the edit — the edit already succeeded
/// when this runs. The invocation-services resolver only supplies
/// `services.post_edit_check` when the process policy permits spawning
/// through `services.process`, so no placement decision happens here.
///
/// Returns `true` when a check process actually ran (completed or timed
/// out), so the caller can account for it like a `builtin.shell` spawn.
async fn append_post_edit_check(
&self,
kind: CodingCapabilityKind,
request: &FirstPartyCapabilityRequest,
output: &mut serde_json::Value,
) -> bool {
if !matches!(
kind,
CodingCapabilityKind::WriteFile | CodingCapabilityKind::ApplyPatch
) {
return false;
}
let Some(config) = &request.services.post_edit_check else {
return false;
};
let Some(check) = crate::post_edit_check::run_post_edit_check(
&self.post_edit_check_seen,
request.services.process.as_ref(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses the invocation's process service for edit tools that do not declare a process effect. For write_file/apply_patch, requires_process is false, so the resolver hands back the default local host port even under ProcessBackendKind::None or tenant-sandbox policies. A configured post-edit check can therefore spawn a host process outside the process policy/sandbox. Please gate this through the same process planning path as shell, or disable it when process execution is not allowed.

&request.scope,
request.mounts.as_ref(),
config,
)
.await
else {
return false;
};
if let Some(object) = output.as_object_mut() {
object.insert("post_edit_check".to_string(), check);
}
true
}
}

#[async_trait]
Expand All @@ -474,6 +519,7 @@ impl FirstPartyCapabilityHandler for BuiltinFirstPartyTools {
normalize_optional_null_sentinels(&mut request);
let start = Instant::now();
let mut network_egress_bytes = 0;
let mut process_count = 0u32;
let (output, display_preview) = match request.capability_id.as_str() {
ECHO_CAPABILITY_ID => (echo::dispatch(&request.input)?, None),
TIME_CAPABILITY_ID => (time::dispatch(&request.input)?, None),
Expand Down Expand Up @@ -552,7 +598,7 @@ impl FirstPartyCapabilityHandler for BuiltinFirstPartyTools {
RuntimeDispatchErrorKind::UndeclaredCapability,
));
};
let request = CodingCapabilityRequest::new(
let coding_request = CodingCapabilityRequest::new(
&request.capability_id,
metadata.kind,
&request.scope,
Expand All @@ -561,11 +607,19 @@ impl FirstPartyCapabilityHandler for BuiltinFirstPartyTools {
Arc::clone(&request.services.filesystem),
&request.input,
);
let result = self
let mut result = self
.coding_state
.dispatch(&request)
.dispatch(&coding_request)
.await
.map_err(coding_error)?;
if self
.append_post_edit_check(metadata.kind, &request, &mut result.output)
.await
{
// The advisory check spawned one process; account for it
// exactly like a `builtin.shell` invocation.
process_count = 1;
}
(result.output, result.display_preview)
}
};
Expand All @@ -575,11 +629,12 @@ impl FirstPartyCapabilityHandler for BuiltinFirstPartyTools {
_ => FIRST_PARTY_MAX_OUTPUT_BYTES,
};
let output_bytes = bounded_output_bytes(&output, output_limit_bytes).map_err(|error| {
if network_egress_bytes > 0 {
if network_egress_bytes > 0 || process_count > 0 {
error.with_usage(
ResourceUsage::default()
.set_wall_clock_ms(wall_clock_ms)
.set_network_egress_bytes(network_egress_bytes),
.set_network_egress_bytes(network_egress_bytes)
.set_process_count(process_count),
)
} else {
error
Expand All @@ -588,7 +643,8 @@ impl FirstPartyCapabilityHandler for BuiltinFirstPartyTools {
let usage = ResourceUsage::default()
.set_wall_clock_ms(wall_clock_ms)
.set_output_bytes(output_bytes)
.set_network_egress_bytes(network_egress_bytes);
.set_network_egress_bytes(network_egress_bytes)
.set_process_count(process_count);
Ok(FirstPartyCapabilityResult::new(output, usage).with_display_preview(display_preview))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ mod tests {
secret_store: None,
audit_sink: None,
unsafe_raw_diagnostics_allowed: false,
post_edit_check: None,
},
input,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// arch-exempt: large_file, trace-commons capability family shares one dispatch surface, plan #4539
//! First-party Trace Commons capabilities: onboard, status, credits, profile token, profile set,
//! and account login link.
//!
Expand Down Expand Up @@ -1373,6 +1374,7 @@ mod tests {
secret_store: None,
audit_sink: None,
unsafe_raw_diagnostics_allowed: false,
post_edit_check: None,
},
input,
}
Expand Down
47 changes: 43 additions & 4 deletions crates/ironclaw_host_runtime/src/invocation_services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use ironclaw_host_api::{
use ironclaw_secrets::SecretStore;
use thiserror::Error;

use crate::{ExecutionPlan, RuntimeProcessPort, RuntimeSecretMaterialStager};
use crate::{ExecutionPlan, PostEditCheckConfig, RuntimeProcessPort, RuntimeSecretMaterialStager};

/// Concrete host API bindings for an already-authorized invocation.
///
Expand All @@ -53,6 +53,15 @@ pub struct InvocationServices {
pub secret_store: Option<Arc<dyn SecretStore>>,
pub audit_sink: Option<Arc<dyn AuditSink>>,
pub unsafe_raw_diagnostics_allowed: bool,
/// Operator-configured post-edit check appended to successful
/// `builtin.write_file` / `builtin.apply_patch` output. Resolved once at
/// composition time (never from env in per-call handlers); `None` keeps
/// the feature off. Resolvers must only populate this when the plan's
/// process policy permits spawning through [`InvocationServices::process`]
/// — the edit plans themselves never declare a process effect, so this
/// field is the policy gate that keeps the check from bypassing
/// process-backend selection.
pub post_edit_check: Option<PostEditCheckConfig>,
}

impl fmt::Debug for InvocationServices {
Expand Down Expand Up @@ -88,6 +97,10 @@ impl fmt::Debug for InvocationServices {
"unsafe_raw_diagnostics_allowed",
&self.unsafe_raw_diagnostics_allowed,
)
.field(
"post_edit_check",
&self.post_edit_check.as_ref().map(|_| "[CONFIGURED]"),
)
.finish()
}
}
Expand Down Expand Up @@ -167,6 +180,7 @@ pub struct LocalInvocationServicesResolver {
tenant_sandbox_process: Option<Arc<dyn RuntimeProcessPort>>,
secret_store: Option<Arc<dyn SecretStore>>,
audit_sink: Option<Arc<dyn AuditSink>>,
post_edit_check: Option<PostEditCheckConfig>,
}

impl LocalInvocationServicesResolver {
Expand All @@ -185,6 +199,7 @@ impl LocalInvocationServicesResolver {
tenant_sandbox_process: None,
secret_store,
audit_sink: None,
post_edit_check: None,
}
}

Expand Down Expand Up @@ -221,6 +236,11 @@ impl LocalInvocationServicesResolver {
self.audit_sink = Some(audit_sink);
self
}

pub fn with_post_edit_check(mut self, post_edit_check: PostEditCheckConfig) -> Self {
self.post_edit_check = Some(post_edit_check);
self
}
}

impl InvocationServicesResolver for LocalInvocationServicesResolver {
Expand All @@ -232,9 +252,7 @@ impl InvocationServicesResolver for LocalInvocationServicesResolver {
let filesystem = self.filesystem_for_plan(plan, request.mounts)?;
let process = if plan.requires_process {
match plan.process_backend {
ProcessBackendKind::LocalHost
if matches!(plan.deployment, DeploymentMode::LocalSingleUser) =>
{
ProcessBackendKind::LocalHost if local_host_process_execution_permitted(plan) => {
Arc::clone(&self.process)
}
ProcessBackendKind::TenantSandbox => self.tenant_sandbox_process.clone().ok_or(
Expand Down Expand Up @@ -286,10 +304,31 @@ impl InvocationServicesResolver for LocalInvocationServicesResolver {
plan.deployment,
plan.resolved_profile,
),
// The post-edit check spawns an operator command through the
// default `process` port above, from edit plans that never
// declare a process effect. Withhold it unless the effective
// process policy is the one under which that default port is
// the policy-approved backend (the same LocalHost +
// LocalSingleUser arm that `requires_process` plans resolve
// to); under `ProcessBackendKind::None` or sandbox-backed
// policies the advisory check is disabled instead of escaping
// onto the provider host.
post_edit_check: self
.post_edit_check
.clone()
.filter(|_| local_host_process_execution_permitted(plan)),
})
}
}

/// Whether the plan's process policy resolves process execution to the local
/// host port for this resolver — the condition under which `resolve` hands
/// `self.process` to a process-requiring plan such as `builtin.shell`.
fn local_host_process_execution_permitted(plan: &ExecutionPlan) -> bool {
matches!(plan.process_backend, ProcessBackendKind::LocalHost)
&& matches!(plan.deployment, DeploymentMode::LocalSingleUser)
}

impl LocalInvocationServicesResolver {
fn filesystem_for_plan(
&self,
Expand Down
68 changes: 68 additions & 0 deletions crates/ironclaw_host_runtime/src/invocation_services/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,74 @@ fn local_resolver_rejects_hosted_local_host_process_backend() {
));
}

#[test]
fn local_resolver_provides_post_edit_check_only_under_local_host_process_policy() {
// The post-edit check spawns an operator command through the default
// process port from plans that never declare a process effect, so the
// resolver must withhold it whenever the effective process policy would
// not hand a local host port to a process-requiring plan.
let resolver = resolver_without_http()
.with_post_edit_check(PostEditCheckConfig::new(
"cargo check",
std::time::Duration::from_secs(30),
))
.with_tenant_sandbox_process_port(Arc::new(NamedProcessPort("tenant-sandbox")));

let resolve = |process_backend, deployment, resolved_profile| {
let mut plan = plan(process_backend, false, false, NetworkMode::Deny, false);
plan.deployment = deployment;
plan.resolved_profile = resolved_profile;
resolver
.resolve(InvocationServicesResolutionRequest {
plan: &plan,
scope: &ResourceScope::system(),
mounts: None,
})
.expect("non-process plans must still resolve")
};

let local = resolve(
ProcessBackendKind::LocalHost,
DeploymentMode::LocalSingleUser,
RuntimeProfile::LocalDev,
);
assert!(
local.post_edit_check.is_some(),
"local-dev LocalHost policy keeps the post-edit check available"
);

let no_process = resolve(
ProcessBackendKind::None,
DeploymentMode::LocalSingleUser,
RuntimeProfile::SecureDefault,
);
assert!(
no_process.post_edit_check.is_none(),
"ProcessBackendKind::None must withhold the post-edit check"
);

let tenant_sandbox = resolve(
ProcessBackendKind::TenantSandbox,
DeploymentMode::HostedMultiTenant,
RuntimeProfile::HostedDev,
);
assert!(
tenant_sandbox.post_edit_check.is_none(),
"tenant-sandbox process policy must withhold the post-edit check even \
when a sandbox port is configured"
);

let hosted_local_host = resolve(
ProcessBackendKind::LocalHost,
DeploymentMode::HostedMultiTenant,
RuntimeProfile::HostedDev,
);
assert!(
hosted_local_host.post_edit_check.is_none(),
"hosted deployments must never run the check on the provider host"
);
}

#[test]
fn local_resolver_rejects_sandbox_process_backend_without_local_fallback() {
let resolver = resolver_without_http();
Expand Down
4 changes: 4 additions & 0 deletions crates/ironclaw_host_runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ mod latency;
pub mod memory_context;
mod obligations;
mod planner;
mod post_edit_check;
mod process_aliases;
mod process_output;
mod process_port;
Expand Down Expand Up @@ -112,6 +113,9 @@ pub use obligations::{
RuntimeCredentialAccountRequest, RuntimeCredentialAccountResolver,
};
pub use planner::{ExecutionPlan, PlannerError, plan_capability};
pub use post_edit_check::{
POST_EDIT_CHECK_ENV, POST_EDIT_CHECK_TIMEOUT_ENV, PostEditCheckConfig, PostEditCheckConfigError,
};
pub use process_output::{SavedCommandOutput, SavedCommandOutputSanitization};
pub use process_port::{
CommandExecutionOutput, CommandExecutionRequest, LocalHostProcessPort, RuntimeProcessError,
Expand Down
Loading
Loading