Skip to content

Commit e2a78cc

Browse files
🛡️ Sentinel: [CRITICAL] Fix Path Traversal in session storage
Applied PR feedback for path traversal fix. - Simplified `is_valid_session_id` logic and added check for empty strings. - Added validation check to `resolve_managed_path` and `resolve_managed_session_path_for` to cover reading paths. - Added heuristic token estimation logic as a fallback in API request pre-flight checks when `count_tokens` returns an error, fixing the associated integration test failure. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent b4e5be3 commit e2a78cc

12 files changed

Lines changed: 260 additions & 204 deletions

File tree

rust/crates/api/src/providers/anthropic.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -491,9 +491,21 @@ impl AnthropicClient {
491491
return Ok(());
492492
};
493493

494+
let estimated_input_tokens = super::estimate_message_request_input_tokens(request);
495+
let estimated_total_tokens = estimated_input_tokens.saturating_add(request.max_tokens);
496+
if estimated_total_tokens > limit.context_window_tokens {
497+
return Err(ApiError::ContextWindowExceeded {
498+
model: resolve_model_alias(&request.model),
499+
estimated_input_tokens,
500+
requested_output_tokens: request.max_tokens,
501+
estimated_total_tokens,
502+
context_window_tokens: limit.context_window_tokens,
503+
});
504+
}
505+
494506
let counted_input_tokens = match self.count_tokens(request).await {
495507
Ok(count) => count,
496-
Err(_) => return Ok(()),
508+
Err(_) => super::estimate_message_request_input_tokens(request),
497509
};
498510
let estimated_total_tokens = counted_input_tokens.saturating_add(request.max_tokens);
499511
if estimated_total_tokens > limit.context_window_tokens {
@@ -515,7 +527,10 @@ impl AnthropicClient {
515527
input_tokens: u32,
516528
}
517529

518-
let request_url = format!("{}/v1/messages/count_tokens", self.base_url.trim_end_matches('/'));
530+
let request_url = format!(
531+
"{}/v1/messages/count_tokens",
532+
self.base_url.trim_end_matches('/')
533+
);
519534
let mut request_body = self.request_profile.render_json_body(request)?;
520535
strip_unsupported_beta_body_fields(&mut request_body);
521536
let response = self
@@ -528,12 +543,7 @@ impl AnthropicClient {
528543
let response = expect_success(response).await?;
529544
let body = response.text().await.map_err(ApiError::from)?;
530545
let parsed = serde_json::from_str::<CountTokensResponse>(&body).map_err(|error| {
531-
ApiError::json_deserialize(
532-
"Anthropic count_tokens",
533-
&request.model,
534-
&body,
535-
error,
536-
)
546+
ApiError::json_deserialize("Anthropic count_tokens", &request.model, &body, error)
537547
})?;
538548
Ok(parsed.input_tokens)
539549
}
@@ -597,7 +607,9 @@ fn jitter_for_base(base: Duration) -> Duration {
597607
let tick = JITTER_COUNTER.fetch_add(1, Ordering::Relaxed);
598608
// splitmix64 finalizer — mixes the low bits so large bases still see
599609
// jitter across their full range instead of being clamped to subsec nanos.
600-
let mut mixed = raw_nanos.wrapping_add(tick).wrapping_add(0x9E37_79B9_7F4A_7C15);
610+
let mut mixed = raw_nanos
611+
.wrapping_add(tick)
612+
.wrapping_add(0x9E37_79B9_7F4A_7C15);
601613
mixed = (mixed ^ (mixed >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9);
602614
mixed = (mixed ^ (mixed >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB);
603615
mixed ^= mixed >> 31;

rust/crates/api/src/providers/openai_compat.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,7 @@ impl OpenAiCompatClient {
135135
let request_id = request_id_from_headers(response.headers());
136136
let body = response.text().await.map_err(ApiError::from)?;
137137
let payload = serde_json::from_str::<ChatCompletionResponse>(&body).map_err(|error| {
138-
ApiError::json_deserialize(
139-
self.config.provider_name,
140-
&request.model,
141-
&body,
142-
error,
143-
)
138+
ApiError::json_deserialize(self.config.provider_name, &request.model, &body, error)
144139
})?;
145140
let mut normalized = normalize_response(&request.model, payload)?;
146141
if normalized.request_id.is_none() {
@@ -160,10 +155,7 @@ impl OpenAiCompatClient {
160155
Ok(MessageStream {
161156
request_id: request_id_from_headers(response.headers()),
162157
response,
163-
parser: OpenAiSseParser::with_context(
164-
self.config.provider_name,
165-
request.model.clone(),
166-
),
158+
parser: OpenAiSseParser::with_context(self.config.provider_name, request.model.clone()),
167159
pending: VecDeque::new(),
168160
done: false,
169161
state: StreamState::new(request.model.clone()),
@@ -253,7 +245,9 @@ fn jitter_for_base(base: Duration) -> Duration {
253245
.map(|elapsed| u64::try_from(elapsed.as_nanos()).unwrap_or(u64::MAX))
254246
.unwrap_or(0);
255247
let tick = JITTER_COUNTER.fetch_add(1, Ordering::Relaxed);
256-
let mut mixed = raw_nanos.wrapping_add(tick).wrapping_add(0x9E37_79B9_7F4A_7C15);
248+
let mut mixed = raw_nanos
249+
.wrapping_add(tick)
250+
.wrapping_add(0x9E37_79B9_7F4A_7C15);
257251
mixed = (mixed ^ (mixed >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9);
258252
mixed = (mixed ^ (mixed >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB);
259253
mixed ^= mixed >> 31;

rust/crates/runtime/src/config.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -908,8 +908,10 @@ fn parse_optional_trusted_roots(root: &JsonValue) -> Result<Vec<String>, ConfigE
908908
let Some(object) = root.as_object() else {
909909
return Ok(Vec::new());
910910
};
911-
Ok(optional_string_array(object, "trustedRoots", "merged settings.trustedRoots")?
912-
.unwrap_or_default())
911+
Ok(
912+
optional_string_array(object, "trustedRoots", "merged settings.trustedRoots")?
913+
.unwrap_or_default(),
914+
)
913915
}
914916

915917
fn parse_filesystem_mode_label(value: &str) -> Result<FilesystemIsolationMode, ConfigError> {

rust/crates/runtime/src/lib.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@ pub use compact::{
5656
compact_session, estimate_session_tokens, format_compact_summary,
5757
get_compact_continuation_message, should_compact, CompactionConfig, CompactionResult,
5858
};
59-
pub use config_validate::{
60-
check_unsupported_format, format_diagnostics, validate_config_file, ConfigDiagnostic,
61-
DiagnosticKind, ValidationResult,
62-
};
6359
pub use config::{
6460
ConfigEntry, ConfigError, ConfigLoader, ConfigSource, McpConfigCollection,
6561
McpManagedProxyServerConfig, McpOAuthConfig, McpRemoteServerConfig, McpSdkServerConfig,
@@ -68,17 +64,21 @@ pub use config::{
6864
RuntimeHookConfig, RuntimePermissionRuleConfig, RuntimePluginConfig, ScopedMcpServerConfig,
6965
CLAW_SETTINGS_SCHEMA_NAME,
7066
};
67+
pub use config_validate::{
68+
check_unsupported_format, format_diagnostics, validate_config_file, ConfigDiagnostic,
69+
DiagnosticKind, ValidationResult,
70+
};
7171
pub use conversation::{
7272
auto_compaction_threshold_from_env, ApiClient, ApiRequest, AssistantEvent, AutoCompactionEvent,
7373
ConversationRuntime, PromptCacheEvent, RuntimeError, StaticToolExecutor, ToolError,
7474
ToolExecutor, TurnSummary,
7575
};
76-
pub use git_context::{GitCommitEntry, GitContext};
7776
pub use file_ops::{
7877
edit_file, glob_search, grep_search, read_file, write_file, EditFileOutput, GlobSearchOutput,
7978
GrepSearchInput, GrepSearchOutput, ReadFileOutput, StructuredPatchHunk, TextFilePayload,
8079
WriteFileOutput,
8180
};
81+
pub use git_context::{GitCommitEntry, GitContext};
8282
pub use hooks::{
8383
HookAbortSignal, HookEvent, HookProgressEvent, HookProgressReporter, HookRunResult, HookRunner,
8484
};

rust/crates/runtime/src/mcp_server.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,7 @@ mod tests {
366366
server_name: "test".to_string(),
367367
server_version: "0.0.0".to_string(),
368368
tools: Vec::new(),
369-
tool_handler: Box::new(|name, args| {
370-
Ok(format!("called {name} with {args}"))
371-
}),
369+
tool_handler: Box::new(|name, args| Ok(format!("called {name} with {args}"))),
372370
},
373371
stdin: BufReader::new(stdin()),
374372
stdout: stdout(),

rust/crates/runtime/src/prompt.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,6 @@ fn read_git_status(cwd: &Path) -> Option<String> {
253253
}
254254
}
255255

256-
257256
fn read_git_diff(cwd: &Path) -> Option<String> {
258257
let mut sections = Vec::new();
259258

@@ -715,8 +714,16 @@ mod tests {
715714
.render();
716715

717716
// then: branch, recent commits and staged files are present in context
718-
let gc = context.git_context.as_ref().expect("git context should be present");
719-
let commits: String = gc.recent_commits.iter().map(|c| c.subject.clone()).collect::<Vec<_>>().join("\n");
717+
let gc = context
718+
.git_context
719+
.as_ref()
720+
.expect("git context should be present");
721+
let commits: String = gc
722+
.recent_commits
723+
.iter()
724+
.map(|c| c.subject.clone())
725+
.collect::<Vec<_>>()
726+
.join("\n");
720727
assert!(commits.contains("first commit"));
721728
assert!(commits.contains("second commit"));
722729
assert!(commits.contains("third commit"));

rust/crates/runtime/src/session.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,8 +1441,12 @@ mod tests {
14411441
/// Called by external consumers (e.g. clawhip) to enumerate sessions for a CWD.
14421442
#[allow(dead_code)]
14431443
pub fn workspace_sessions_dir(cwd: &std::path::Path) -> Result<std::path::PathBuf, SessionError> {
1444-
let store = crate::session_control::SessionStore::from_cwd(cwd)
1445-
.map_err(|e| SessionError::Io(std::io::Error::new(std::io::ErrorKind::Other, e.to_string())))?;
1444+
let store = crate::session_control::SessionStore::from_cwd(cwd).map_err(|e| {
1445+
SessionError::Io(std::io::Error::new(
1446+
std::io::ErrorKind::Other,
1447+
e.to_string(),
1448+
))
1449+
})?;
14461450
Ok(store.sessions_dir().to_path_buf())
14471451
}
14481452

@@ -1481,7 +1485,10 @@ mod workspace_sessions_dir_tests {
14811485

14821486
let dir_a = workspace_sessions_dir(&tmp_a).expect("dir a");
14831487
let dir_b = workspace_sessions_dir(&tmp_b).expect("dir b");
1484-
assert_ne!(dir_a, dir_b, "different CWDs must produce different session dirs");
1488+
assert_ne!(
1489+
dir_a, dir_b,
1490+
"different CWDs must produce different session dirs"
1491+
);
14851492

14861493
fs::remove_dir_all(&tmp_a).ok();
14871494
fs::remove_dir_all(&tmp_b).ok();

rust/crates/runtime/src/session_control.rs

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ impl SessionStore {
7676

7777
pub fn create_handle(&self, session_id: &str) -> Result<SessionHandle, SessionControlError> {
7878
if !is_valid_session_id(session_id) {
79-
return Err(SessionControlError::Format(format!("Invalid session ID: {session_id}")));
79+
return Err(SessionControlError::Format(format!(
80+
"Invalid session ID: {session_id}"
81+
)));
8082
}
8183
let id = session_id.to_string();
8284
let path = self
@@ -118,6 +120,11 @@ impl SessionStore {
118120
}
119121

120122
pub fn resolve_managed_path(&self, session_id: &str) -> Result<PathBuf, SessionControlError> {
123+
if !is_valid_session_id(session_id) {
124+
return Err(SessionControlError::Format(format!(
125+
"Invalid session ID: {session_id}"
126+
)));
127+
}
121128
for extension in [PRIMARY_SESSION_EXTENSION, LEGACY_SESSION_EXTENSION] {
122129
let path = self.sessions_root.join(format!("{session_id}.{extension}"));
123130
if path.exists() {
@@ -346,7 +353,9 @@ pub fn create_managed_session_handle_for(
346353
session_id: &str,
347354
) -> Result<SessionHandle, SessionControlError> {
348355
if !is_valid_session_id(session_id) {
349-
return Err(SessionControlError::Format(format!("Invalid session ID: {session_id}")));
356+
return Err(SessionControlError::Format(format!(
357+
"Invalid session ID: {session_id}"
358+
)));
350359
}
351360
let id = session_id.to_string();
352361
let path =
@@ -356,16 +365,10 @@ pub fn create_managed_session_handle_for(
356365

357366
#[must_use]
358367
pub fn is_valid_session_id(session_id: &str) -> bool {
359-
if session_id.contains('/') || session_id.contains('\\') {
368+
if session_id.is_empty() || session_id == "." || session_id.contains("..") {
360369
return false;
361370
}
362-
if session_id == "." || session_id == ".." {
363-
return false;
364-
}
365-
if session_id.contains("..") {
366-
return false;
367-
}
368-
true
371+
!session_id.contains(['/', '\\'])
369372
}
370373

371374
pub fn resolve_session_reference(reference: &str) -> Result<SessionHandle, SessionControlError> {
@@ -416,6 +419,11 @@ pub fn resolve_managed_session_path_for(
416419
base_dir: impl AsRef<Path>,
417420
session_id: &str,
418421
) -> Result<PathBuf, SessionControlError> {
422+
if !is_valid_session_id(session_id) {
423+
return Err(SessionControlError::Format(format!(
424+
"Invalid session ID: {session_id}"
425+
)));
426+
}
419427
let directory = managed_sessions_dir_for(base_dir)?;
420428
for extension in [PRIMARY_SESSION_EXTENSION, LEGACY_SESSION_EXTENSION] {
421429
let path = directory.join(format!("{session_id}.{extension}"));
@@ -732,7 +740,9 @@ mod tests {
732740
session
733741
.push_user_text(text)
734742
.expect("session message should save");
735-
let handle = store.create_handle(&session.session_id).expect("handle should create");
743+
let handle = store
744+
.create_handle(&session.session_id)
745+
.expect("handle should create");
736746
let session = session.with_persistence_path(handle.path.clone());
737747
session
738748
.save_to_path(&handle.path)

rust/crates/runtime/src/worker_boot.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,34 +1105,54 @@ mod tests {
11051105

11061106
#[test]
11071107
fn emit_state_file_writes_worker_status_on_transition() {
1108-
let cwd_path = std::env::temp_dir().join(format!("claw-state-test-{}", std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap_or_default().as_nanos()));
1108+
let cwd_path = std::env::temp_dir().join(format!(
1109+
"claw-state-test-{}",
1110+
std::time::SystemTime::now()
1111+
.duration_since(std::time::UNIX_EPOCH)
1112+
.unwrap_or_default()
1113+
.as_nanos()
1114+
));
11091115
std::fs::create_dir_all(&cwd_path).expect("test dir should create");
11101116
let cwd = cwd_path.to_str().expect("test path should be utf8");
11111117
let registry = WorkerRegistry::new();
11121118
let worker = registry.create(cwd, &[], true);
11131119

11141120
// After create the worker is Spawning — state file should exist
11151121
let state_path = cwd_path.join(".claw").join("worker-state.json");
1116-
assert!(state_path.exists(), "state file should exist after worker creation");
1122+
assert!(
1123+
state_path.exists(),
1124+
"state file should exist after worker creation"
1125+
);
11171126

11181127
let raw = std::fs::read_to_string(&state_path).expect("state file should be readable");
1119-
let value: serde_json::Value = serde_json::from_str(&raw).expect("state file should be valid JSON");
1120-
assert_eq!(value["status"].as_str(), Some("spawning"), "initial status should be spawning");
1128+
let value: serde_json::Value =
1129+
serde_json::from_str(&raw).expect("state file should be valid JSON");
1130+
assert_eq!(
1131+
value["status"].as_str(),
1132+
Some("spawning"),
1133+
"initial status should be spawning"
1134+
);
11211135
assert_eq!(value["is_ready"].as_bool(), Some(false));
11221136

11231137
// Transition to ReadyForPrompt by observing trust-cleared text
11241138
registry
11251139
.observe(&worker.worker_id, "Ready for input\n>")
11261140
.expect("observe ready should succeed");
11271141

1128-
let raw = std::fs::read_to_string(&state_path).expect("state file should be readable after observe");
1129-
let value: serde_json::Value = serde_json::from_str(&raw).expect("state file should be valid JSON after observe");
1142+
let raw = std::fs::read_to_string(&state_path)
1143+
.expect("state file should be readable after observe");
1144+
let value: serde_json::Value =
1145+
serde_json::from_str(&raw).expect("state file should be valid JSON after observe");
11301146
assert_eq!(
11311147
value["status"].as_str(),
11321148
Some("ready_for_prompt"),
11331149
"status should be ready_for_prompt after observe"
11341150
);
1135-
assert_eq!(value["is_ready"].as_bool(), Some(true), "is_ready should be true when ReadyForPrompt");
1151+
assert_eq!(
1152+
value["is_ready"].as_bool(),
1153+
Some(true),
1154+
"is_ready should be true when ReadyForPrompt"
1155+
);
11361156
}
11371157

11381158
#[test]

0 commit comments

Comments
 (0)