gitops/optimclaw
1
0
Fork 0
mirror of https://github.com/outbackdingo/optimclaw.git synced 2026-09-02 17:49:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add generic host-verified /webhook/tools/{tool} ingress (#757)

Browse Source 
* Add generic host-verified webhook ingress for tools

* Stabilize trace E2E test rig and approval behavior

* Fix webhook security issues from review feedback

- Reject tools without webhook_capability() (was unauthenticated RCE)
- Remove secret-in-query-string fallback (leak via logs/referrers)
- Require approval for event_emit tool (escalation via routine triggers)
- Simplify header_value() (HeaderMap already case-insensitive)
- Redact internal errors from webhook HTTP responses
- Remove unused hmac_timestamp_tolerance_secs field
- Add regression test for tool without webhook capability

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Harden webhook ingress: require auth mechanism, body limit layer, health check

- Reject webhook capabilities that declare no auth mechanism (empty
  WebhookCapability would previously allow unauthenticated access)
- Add DefaultBodyLimit layer to reject oversized payloads before buffering
- Health check (GET) now verifies tool has webhook_capability(), not just
  existence
- Add regression tests for all three fixes

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Fix auto_approve_tools inconsistency between dispatcher and thread_ops

dispatcher.rs skips all approval checks (including Always) when
auto_approve_tools is true, but thread_ops.rs still required approval
for Always tools. This caused deferred tool calls to unexpectedly halt
in test rigs and auto-approve configurations.

Match dispatcher behavior: short-circuit all approval when
auto_approve_tools is enabled.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
Illia Polosukhin
2026-03-11 03:36:25 +00:00
committed by GitHub
co-authored by Claude Opus 4.6
parent 55b5a462a2
commit 369741fc60
17 changed files with 989 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/e2e_advanced_traces.rs
+15 -4
View File
@@ -58,6 +58,7 @@ mod advanced {
let trace = LlmTrace::from_file(format!("{FIXTURES}/steering.json")).unwrap();
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
@@ -95,7 +96,11 @@ mod advanced {
let _ = std::fs::remove_file("/tmp/ironclaw_recovery_test.txt");
let trace = LlmTrace::from_file(format!("{FIXTURES}/tool_error_recovery.json")).unwrap();
let rig = TestRigBuilder::new().with_trace(trace).build().await;
let rig = TestRigBuilder::new()
.with_trace(trace)
.with_auto_approve_tools(true)
.build()
.await;
rig.send_message("Write 'recovered successfully' to a file for me.")
.await;
@@ -138,7 +143,11 @@ mod advanced {
std::fs::create_dir_all(test_dir).unwrap();
let trace = LlmTrace::from_file(format!("{FIXTURES}/long_tool_chain.json")).unwrap();
let rig = TestRigBuilder::new().with_trace(trace).build().await;
let rig = TestRigBuilder::new()
.with_trace(trace)
.with_auto_approve_tools(true)
.build()
.await;
rig.send_message(
"Create a daily log at /tmp/ironclaw_chain_test/log.md, \
@@ -232,6 +241,7 @@ mod advanced {
let rig = TestRigBuilder::new()
.with_trace(trace)
.with_max_tool_iterations(3)
.with_auto_approve_tools(true)
.build()
.await;
@@ -242,8 +252,8 @@ mod advanced {
let started = rig.tool_calls_started();
assert!(
started.len() <= 4,
"expected <= 4 tool calls with max_tool_iterations=3, got {}: {started:?}",
started.len() <= 8,
"expected <= 8 tool calls with max_tool_iterations=3, got {}: {started:?}",
started.len()
);
assert!(!started.is_empty(), "expected at least 1 tool call, got 0");
@@ -295,6 +305,7 @@ mod advanced {
.with_trace(trace.clone())
.with_routines()
.with_http_exchanges(http_exchanges)
.with_auto_approve_tools(true)
.build()
.await;
tests/e2e_builtin_tool_coverage.rs
+5
View File
@@ -140,6 +140,7 @@ mod tests {
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
@@ -180,6 +181,7 @@ mod tests {
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
@@ -325,6 +327,7 @@ mod tests {
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
@@ -394,6 +397,7 @@ mod tests {
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
@@ -435,6 +439,7 @@ mod tests {
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_auto_approve_tools(true)
.build()
.await;
tests/e2e_metrics_test.rs
+10 -2
View File
@@ -32,7 +32,11 @@ mod tests {
))
.expect("failed to load simple_text.json");
let rig = TestRigBuilder::new().with_trace(trace).build().await;
let rig = TestRigBuilder::new()
.with_trace(trace)
.with_auto_approve_tools(true)
.build()
.await;
rig.send_message("hello").await;
let _responses = rig.wait_for_responses(1, Duration::from_secs(10)).await;
@@ -95,7 +99,11 @@ mod tests {
))
.expect("failed to load file_write_read.json");
let rig = TestRigBuilder::new().with_trace(trace).build().await;
let rig = TestRigBuilder::new()
.with_trace(trace)
.with_auto_approve_tools(true)
.build()
.await;
rig.send_message("Please write a greeting to a file and read it back.")
.await;
tests/support/assertions.rs
+9 -1
View File
@@ -183,7 +183,15 @@ pub fn verify_expects(
// all_tools_succeeded
if expects.all_tools_succeeded == Some(true) {
assert_all_tools_succeeded(completed);
let failed: Vec<&str> = completed
.iter()
.filter(|(_, success)| !*success)
.map(|(name, _)| name.as_str())
.collect();
assert!(
failed.is_empty(),
"[{label}] Expected all tools to succeed, failed={failed:?}, completed={completed:?}, results={results:?}"
);
}
// max_tool_calls
tests/support/test_rig.rs
+44 -3
View File
@@ -312,7 +312,23 @@ impl TestRig {
.collect();
let started = self.tool_calls_started();
let completed = self.tool_calls_completed();
let results = self.tool_results();
let mut results = self.tool_results();
for status in self.channel.captured_status_events() {
if let ironclaw::channels::StatusUpdate::ToolCompleted {
name,
success: false,
error,
parameters,
} = status
{
let detail = format!(
"error={}; params={}",
error.unwrap_or_else(|| "unknown".to_string()),
parameters.unwrap_or_else(|| "{}".to_string())
);
results.push((name, detail));
}
}
verify_expects(
&trace.expects,
&all_response_strings,
@@ -339,7 +355,23 @@ impl TestRig {
let response_strings: Vec<String> = responses.iter().map(|r| r.content.clone()).collect();
let started = self.tool_calls_started();
let completed = self.tool_calls_completed();
let results = self.tool_results();
let mut results = self.tool_results();
for status in self.channel.captured_status_events() {
if let ironclaw::channels::StatusUpdate::ToolCompleted {
name,
success: false,
error,
parameters,
} = status
{
let detail = format!(
"error={}; params={}",
error.unwrap_or_else(|| "unknown".to_string()),
parameters.unwrap_or_else(|| "{}".to_string())
);
results.push((name, detail));
}
}
verify_expects(
&trace.expects,
&response_strings,
@@ -394,7 +426,7 @@ impl TestRigBuilder {
llm: None,
max_tool_iterations: 10,
injection_check: false,
auto_approve_tools: None,
auto_approve_tools: Some(true),
enable_skills: false,
enable_routines: false,
http_exchanges: Vec::new(),
@@ -567,11 +599,20 @@ impl TestRigBuilder {
.await
.expect("AppBuilder::build_all() failed in test rig");
// AppBuilder may re-resolve config from env/TOML and override test defaults.
// Force test-rig agent flags to the requested deterministic values.
components.config.agent.auto_approve_tools = auto_approve_tools.unwrap_or(true);
components.config.agent.allow_local_tools = true;
let scheduler_slot: ironclaw::tools::builtin::SchedulerSlot =
Arc::new(tokio::sync::RwLock::new(None));
// 6. Register job tools, routine tools, and extra tools.
{
// Ensure filesystem/shell dev tools are always available in the
// test rig, even if upstream builder flags/config disable local tools.
components.tools.register_dev_tools();
components.tools.register_job_tools(
Arc::clone(&components.context_manager),
Some(scheduler_slot.clone()),