Skip to content

Commit 9108e1e

Browse files
authored
fix(tools): propagate claim_source in gate audit and detect relative paths (#2539)
Fixes #2535: AdversarialPolicyGateExecutor.write_audit() now accepts an explicit claim_source parameter. After successful inner execution, claim_source is copied from ToolOutput. Blocked/denied calls pass None. Fixes #2536: extract_paths() now detects relative path tokens without a ./ prefix (e.g. src/main.rs, .local/foo/bar). Added is_relative_path_token() helper that excludes URLs and env assignments.
1 parent 1b202bb commit 9108e1e

File tree

4 files changed

+172
-8
lines changed

4 files changed

+172
-8
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
1717

1818
### Fixed
1919

20+
- fix(tools): propagate `claim_source` from `ToolOutput` into the post-execution audit entry in `AdversarialPolicyGateExecutor`; `write_audit` now accepts an explicit `claim_source` parameter so the field is no longer hardcoded to `None` for successful executions (closes #2535)
21+
- fix(tools): `extract_paths` now detects relative path tokens that contain `/` but do not start with `/` or `./` (e.g. `src/main.rs`, `.local/foo/bar`); URL schemes (`://`) and shell variable assignments (`KEY=value`) are excluded from matching (closes #2536)
22+
2023
- fix(mcp): replace unbounded elicitation mpsc channel with a bounded channel (default capacity 16) to prevent memory exhaustion from misbehaving MCP servers; requests that arrive when the queue is full are auto-declined with a warning log instead of accumulating indefinitely; capacity is configurable via `[mcp] elicitation_queue_capacity` (closes #2524)
2124
- fix(mcp): pre-existing `clippy::non_exhaustive_omitted_patterns`, `match_single_binding`, and `uninlined_format_args` warnings in elicitation CLI prompt builder and test code (caught while adding bounded-channel support)
2225

crates/zeph-tools/src/adversarial_gate.rs

Lines changed: 102 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use std::sync::Arc;
1818

1919
use crate::adversarial_policy::{PolicyDecision, PolicyLlmClient, PolicyValidator};
2020
use crate::audit::{AuditEntry, AuditLogger, AuditResult, chrono_now};
21-
use crate::executor::{ToolCall, ToolError, ToolExecutor, ToolOutput};
21+
use crate::executor::{ClaimSource, ToolCall, ToolError, ToolExecutor, ToolOutput};
2222
use crate::registry::ToolDef;
2323

2424
/// Wraps an inner `ToolExecutor`, running an LLM-based adversarial policy check
@@ -75,7 +75,8 @@ impl<T: ToolExecutor> AdversarialPolicyGateExecutor<T> {
7575
match decision {
7676
PolicyDecision::Allow => {
7777
tracing::debug!(tool = %call.tool_id, "adversarial policy: allow");
78-
self.write_audit(call, "allow", AuditResult::Success).await;
78+
self.write_audit(call, "allow", AuditResult::Success, None)
79+
.await;
7980
Ok(())
8081
}
8182
PolicyDecision::Deny { reason } => {
@@ -90,6 +91,7 @@ impl<T: ToolExecutor> AdversarialPolicyGateExecutor<T> {
9091
AuditResult::Blocked {
9192
reason: reason.clone(),
9293
},
94+
None,
9395
)
9496
.await;
9597
// MED-03: do NOT surface the LLM reason to the main LLM.
@@ -105,8 +107,13 @@ impl<T: ToolExecutor> AdversarialPolicyGateExecutor<T> {
105107
"adversarial policy: LLM error"
106108
);
107109
if self.validator.fail_open() {
108-
self.write_audit(call, &format!("error:{message}"), AuditResult::Success)
109-
.await;
110+
self.write_audit(
111+
call,
112+
&format!("error:{message}"),
113+
AuditResult::Success,
114+
None,
115+
)
116+
.await;
110117
Ok(())
111118
} else {
112119
self.write_audit(
@@ -115,6 +122,7 @@ impl<T: ToolExecutor> AdversarialPolicyGateExecutor<T> {
115122
AuditResult::Blocked {
116123
reason: "adversarial policy LLM error (fail-closed)".to_owned(),
117124
},
125+
None,
118126
)
119127
.await;
120128
Err(ToolError::Blocked {
@@ -125,7 +133,13 @@ impl<T: ToolExecutor> AdversarialPolicyGateExecutor<T> {
125133
}
126134
}
127135

128-
async fn write_audit(&self, call: &ToolCall, decision: &str, result: AuditResult) {
136+
async fn write_audit(
137+
&self,
138+
call: &ToolCall,
139+
decision: &str,
140+
result: AuditResult,
141+
claim_source: Option<ClaimSource>,
142+
) {
129143
let Some(audit) = &self.audit else { return };
130144
let entry = AuditEntry {
131145
timestamp: chrono_now(),
@@ -136,7 +150,7 @@ impl<T: ToolExecutor> AdversarialPolicyGateExecutor<T> {
136150
error_category: None,
137151
error_domain: None,
138152
error_phase: None,
139-
claim_source: None,
153+
claim_source,
140154
mcp_server_id: None,
141155
injection_flagged: false,
142156
embedding_anomalous: false,
@@ -166,7 +180,17 @@ impl<T: ToolExecutor> ToolExecutor for AdversarialPolicyGateExecutor<T> {
166180

167181
async fn execute_tool_call(&self, call: &ToolCall) -> Result<Option<ToolOutput>, ToolError> {
168182
self.check_policy(call).await?;
169-
self.inner.execute_tool_call(call).await
183+
let output = self.inner.execute_tool_call(call).await?;
184+
if let Some(ref out) = output {
185+
self.write_audit(
186+
call,
187+
"allow:executed",
188+
AuditResult::Success,
189+
out.claim_source,
190+
)
191+
.await;
192+
}
193+
Ok(output)
170194
}
171195

172196
// MED-04: policy also enforced on confirmed calls.
@@ -175,7 +199,17 @@ impl<T: ToolExecutor> ToolExecutor for AdversarialPolicyGateExecutor<T> {
175199
call: &ToolCall,
176200
) -> Result<Option<ToolOutput>, ToolError> {
177201
self.check_policy(call).await?;
178-
self.inner.execute_tool_call_confirmed(call).await
202+
let output = self.inner.execute_tool_call_confirmed(call).await?;
203+
if let Some(ref out) = output {
204+
self.write_audit(
205+
call,
206+
"allow:executed",
207+
AuditResult::Success,
208+
out.claim_source,
209+
)
210+
.await;
211+
}
212+
Ok(output)
179213
}
180214

181215
fn set_skill_env(&self, env: Option<std::collections::HashMap<String, String>>) {
@@ -530,4 +564,64 @@ mod tests {
530564
"deny decision must be recorded in audit"
531565
);
532566
}
567+
568+
#[tokio::test]
569+
async fn audit_entry_propagates_claim_source() {
570+
use tempfile::TempDir;
571+
572+
#[derive(Debug)]
573+
struct InnerWithClaimSource;
574+
575+
impl ToolExecutor for InnerWithClaimSource {
576+
async fn execute(&self, _: &str) -> Result<Option<ToolOutput>, ToolError> {
577+
Ok(None)
578+
}
579+
580+
async fn execute_tool_call(
581+
&self,
582+
call: &ToolCall,
583+
) -> Result<Option<ToolOutput>, ToolError> {
584+
Ok(Some(ToolOutput {
585+
tool_name: call.tool_id.clone(),
586+
summary: "ok".into(),
587+
blocks_executed: 1,
588+
filter_stats: None,
589+
diff: None,
590+
streamed: false,
591+
terminal_id: None,
592+
locations: None,
593+
raw_response: None,
594+
claim_source: Some(crate::executor::ClaimSource::Shell),
595+
}))
596+
}
597+
}
598+
599+
let dir = TempDir::new().unwrap();
600+
let log_path = dir.path().join("audit.log");
601+
let audit_config = crate::config::AuditConfig {
602+
enabled: true,
603+
destination: log_path.display().to_string(),
604+
};
605+
let audit_logger = Arc::new(
606+
crate::audit::AuditLogger::from_config(&audit_config)
607+
.await
608+
.unwrap(),
609+
);
610+
611+
let (_, llm) = MockLlm::new("ALLOW");
612+
let gate = AdversarialPolicyGateExecutor::new(
613+
InnerWithClaimSource,
614+
make_validator(false),
615+
Arc::new(llm),
616+
)
617+
.with_audit(Arc::clone(&audit_logger));
618+
619+
gate.execute_tool_call(&make_call("shell")).await.unwrap();
620+
621+
let content = tokio::fs::read_to_string(&log_path).await.unwrap();
622+
assert!(
623+
content.contains("\"shell\""),
624+
"claim_source must be propagated into the post-execution audit entry"
625+
);
626+
}
533627
}

crates/zeph-tools/src/shell/mod.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,13 +1008,44 @@ fn extract_paths(code: &str) -> Vec<String> {
10081008
|| trimmed.starts_with("./")
10091009
|| trimmed.starts_with("../")
10101010
|| trimmed == ".."
1011+
|| (trimmed.starts_with('.') && trimmed.contains('/'))
1012+
|| is_relative_path_token(&trimmed)
10111013
{
10121014
result.push(trimmed);
10131015
}
10141016
}
10151017
result
10161018
}
10171019

1020+
/// Returns `true` if `token` looks like a relative path of the form `word/more`
1021+
/// (contains `/` but does not start with `/` or `.`).
1022+
///
1023+
/// Excluded:
1024+
/// - URL schemes (`scheme://`)
1025+
/// - Shell variable assignments (`KEY=value`)
1026+
fn is_relative_path_token(token: &str) -> bool {
1027+
// Must contain a slash but not start with `/` (absolute) or `.` (handled above).
1028+
if !token.contains('/') || token.starts_with('/') || token.starts_with('.') {
1029+
return false;
1030+
}
1031+
// Reject URLs: anything with `://`
1032+
if token.contains("://") {
1033+
return false;
1034+
}
1035+
// Reject shell variable assignments: `IDENTIFIER=...`
1036+
if let Some(eq_pos) = token.find('=') {
1037+
let key = &token[..eq_pos];
1038+
if key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
1039+
return false;
1040+
}
1041+
}
1042+
// First character must be an identifier-start (letter, digit, or `_`).
1043+
token
1044+
.chars()
1045+
.next()
1046+
.is_some_and(|c| c.is_ascii_alphanumeric() || c == '_')
1047+
}
1048+
10181049
/// Classify shell exit codes and stderr patterns into `ToolErrorCategory`.
10191050
///
10201051
/// Returns `Some(category)` only for well-known failure modes that benefit from

crates/zeph-tools/src/shell/tests.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,42 @@ fn extract_paths_empty() {
979979
assert!(extract_paths("").is_empty());
980980
}
981981

982+
#[test]
983+
fn extract_paths_relative_without_prefix() {
984+
let paths = extract_paths("cargo build src/main.rs");
985+
assert!(
986+
paths.contains(&"src/main.rs".to_owned()),
987+
"src/main.rs must be detected"
988+
);
989+
}
990+
991+
#[test]
992+
fn extract_paths_relative_nested() {
993+
let paths = extract_paths("cat .local/foo/bar");
994+
assert!(
995+
paths.contains(&".local/foo/bar".to_owned()),
996+
".local/foo/bar must be detected"
997+
);
998+
}
999+
1000+
#[test]
1001+
fn extract_paths_does_not_match_urls() {
1002+
let paths = extract_paths("curl https://example.com/file");
1003+
assert!(
1004+
!paths.contains(&"https://example.com/file".to_owned()),
1005+
"URLs must not be matched as paths"
1006+
);
1007+
}
1008+
1009+
#[test]
1010+
fn extract_paths_does_not_match_env_assignments() {
1011+
let paths = extract_paths("KEY=some/value cargo build");
1012+
assert!(
1013+
!paths.contains(&"KEY=some/value".to_owned()),
1014+
"env assignments must not be matched as paths"
1015+
);
1016+
}
1017+
9821018
#[tokio::test]
9831019
async fn policy_deny_blocks_command() {
9841020
let policy = PermissionPolicy::from_legacy(&["forbidden".to_owned()], &[]);

0 commit comments

Comments
 (0)