mirror of
https://github.com/ultraworkers/claw-code.git
synced 2026-04-29 00:02:01 +08:00
Clarify allowed tool status handling
Reject empty --allowedTools inputs instead of treating them as an empty restriction, and surface status JSON metadata that distinguishes default unrestricted tools from flag-provided allow lists. Confidence: high Scope-risk: narrow Tested: cargo test -p rusty-claude-cli rejects_empty_allowed_tools_flag -- --nocapture Tested: cargo test -p tools allowed_tools_rejects_empty_token_lists -- --nocapture Tested: cargo check -p rusty-claude-cli -p tools Tested: cargo test -p rusty-claude-cli -p tools Not-tested: full workspace cargo fmt --check is blocked by pre-existing unrelated formatting drift
This commit is contained in:
parent
6db68a2baa
commit
77afde768c
@ -372,7 +372,14 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
|
|||||||
model_flag_raw,
|
model_flag_raw,
|
||||||
permission_mode,
|
permission_mode,
|
||||||
output_format,
|
output_format,
|
||||||
} => print_status_snapshot(&model, model_flag_raw.as_deref(), permission_mode, output_format)?,
|
allowed_tools,
|
||||||
|
} => print_status_snapshot(
|
||||||
|
&model,
|
||||||
|
model_flag_raw.as_deref(),
|
||||||
|
permission_mode,
|
||||||
|
output_format,
|
||||||
|
allowed_tools.as_ref(),
|
||||||
|
)?,
|
||||||
CliAction::Sandbox { output_format } => print_sandbox_status_snapshot(output_format)?,
|
CliAction::Sandbox { output_format } => print_sandbox_status_snapshot(output_format)?,
|
||||||
CliAction::Prompt {
|
CliAction::Prompt {
|
||||||
prompt,
|
prompt,
|
||||||
@ -510,6 +517,7 @@ enum CliAction {
|
|||||||
model_flag_raw: Option<String>,
|
model_flag_raw: Option<String>,
|
||||||
permission_mode: PermissionMode,
|
permission_mode: PermissionMode,
|
||||||
output_format: CliOutputFormat,
|
output_format: CliOutputFormat,
|
||||||
|
allowed_tools: Option<AllowedToolSet>,
|
||||||
},
|
},
|
||||||
Sandbox {
|
Sandbox {
|
||||||
output_format: CliOutputFormat,
|
output_format: CliOutputFormat,
|
||||||
@ -844,9 +852,14 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
|
|||||||
if let Some(action) = parse_local_help_action(&rest) {
|
if let Some(action) = parse_local_help_action(&rest) {
|
||||||
return action;
|
return action;
|
||||||
}
|
}
|
||||||
if let Some(action) =
|
if let Some(action) = parse_single_word_command_alias(
|
||||||
parse_single_word_command_alias(&rest, &model, model_flag_raw.as_deref(), permission_mode_override, output_format)
|
&rest,
|
||||||
{
|
&model,
|
||||||
|
model_flag_raw.as_deref(),
|
||||||
|
permission_mode_override,
|
||||||
|
output_format,
|
||||||
|
allowed_tools.clone(),
|
||||||
|
) {
|
||||||
return action;
|
return action;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1051,6 +1064,7 @@ fn parse_single_word_command_alias(
|
|||||||
model_flag_raw: Option<&str>,
|
model_flag_raw: Option<&str>,
|
||||||
permission_mode_override: Option<PermissionMode>,
|
permission_mode_override: Option<PermissionMode>,
|
||||||
output_format: CliOutputFormat,
|
output_format: CliOutputFormat,
|
||||||
|
allowed_tools: Option<AllowedToolSet>,
|
||||||
) -> Option<Result<CliAction, String>> {
|
) -> Option<Result<CliAction, String>> {
|
||||||
if rest.is_empty() {
|
if rest.is_empty() {
|
||||||
return None;
|
return None;
|
||||||
@ -1095,6 +1109,7 @@ fn parse_single_word_command_alias(
|
|||||||
model_flag_raw: model_flag_raw.map(str::to_string), // #148
|
model_flag_raw: model_flag_raw.map(str::to_string), // #148
|
||||||
permission_mode: permission_mode_override.unwrap_or_else(default_permission_mode),
|
permission_mode: permission_mode_override.unwrap_or_else(default_permission_mode),
|
||||||
output_format,
|
output_format,
|
||||||
|
allowed_tools,
|
||||||
})),
|
})),
|
||||||
"sandbox" => Some(Ok(CliAction::Sandbox { output_format })),
|
"sandbox" => Some(Ok(CliAction::Sandbox { output_format })),
|
||||||
"doctor" => Some(Ok(CliAction::Doctor { output_format })),
|
"doctor" => Some(Ok(CliAction::Doctor { output_format })),
|
||||||
@ -3226,6 +3241,7 @@ fn run_resume_command(
|
|||||||
default_permission_mode().as_str(),
|
default_permission_mode().as_str(),
|
||||||
&context,
|
&context,
|
||||||
None, // #148: resumed sessions don't have flag provenance
|
None, // #148: resumed sessions don't have flag provenance
|
||||||
|
None,
|
||||||
)),
|
)),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@ -5417,6 +5433,7 @@ fn print_status_snapshot(
|
|||||||
model_flag_raw: Option<&str>,
|
model_flag_raw: Option<&str>,
|
||||||
permission_mode: PermissionMode,
|
permission_mode: PermissionMode,
|
||||||
output_format: CliOutputFormat,
|
output_format: CliOutputFormat,
|
||||||
|
allowed_tools: Option<&AllowedToolSet>,
|
||||||
) -> Result<(), Box<dyn std::error::Error>> {
|
) -> Result<(), Box<dyn std::error::Error>> {
|
||||||
let usage = StatusUsage {
|
let usage = StatusUsage {
|
||||||
message_count: 0,
|
message_count: 0,
|
||||||
@ -5450,6 +5467,7 @@ fn print_status_snapshot(
|
|||||||
permission_mode.as_str(),
|
permission_mode.as_str(),
|
||||||
&context,
|
&context,
|
||||||
Some(&provenance),
|
Some(&provenance),
|
||||||
|
allowed_tools,
|
||||||
))?
|
))?
|
||||||
),
|
),
|
||||||
}
|
}
|
||||||
@ -5467,6 +5485,7 @@ fn status_json_value(
|
|||||||
// that don't have provenance (legacy resume paths) pass None, in which
|
// that don't have provenance (legacy resume paths) pass None, in which
|
||||||
// case both new fields are omitted.
|
// case both new fields are omitted.
|
||||||
provenance: Option<&ModelProvenance>,
|
provenance: Option<&ModelProvenance>,
|
||||||
|
allowed_tools: Option<&AllowedToolSet>,
|
||||||
) -> serde_json::Value {
|
) -> serde_json::Value {
|
||||||
// #143: top-level `status` marker so claws can distinguish
|
// #143: top-level `status` marker so claws can distinguish
|
||||||
// a clean run from a degraded run (config parse failed but other fields
|
// a clean run from a degraded run (config parse failed but other fields
|
||||||
@ -5476,6 +5495,7 @@ fn status_json_value(
|
|||||||
let degraded = context.config_load_error.is_some();
|
let degraded = context.config_load_error.is_some();
|
||||||
let model_source = provenance.map(|p| p.source.as_str());
|
let model_source = provenance.map(|p| p.source.as_str());
|
||||||
let model_raw = provenance.and_then(|p| p.raw.clone());
|
let model_raw = provenance.and_then(|p| p.raw.clone());
|
||||||
|
let allowed_tool_entries = allowed_tools.map(|tools| tools.iter().cloned().collect::<Vec<_>>());
|
||||||
json!({
|
json!({
|
||||||
"kind": "status",
|
"kind": "status",
|
||||||
"status": if degraded { "degraded" } else { "ok" },
|
"status": if degraded { "degraded" } else { "ok" },
|
||||||
@ -5484,6 +5504,11 @@ fn status_json_value(
|
|||||||
"model_source": model_source,
|
"model_source": model_source,
|
||||||
"model_raw": model_raw,
|
"model_raw": model_raw,
|
||||||
"permission_mode": permission_mode,
|
"permission_mode": permission_mode,
|
||||||
|
"allowed_tools": {
|
||||||
|
"source": if allowed_tools.is_some() { "flag" } else { "default" },
|
||||||
|
"restricted": allowed_tools.is_some(),
|
||||||
|
"entries": allowed_tool_entries,
|
||||||
|
},
|
||||||
"usage": {
|
"usage": {
|
||||||
"messages": usage.message_count,
|
"messages": usage.message_count,
|
||||||
"turns": usage.turns,
|
"turns": usage.turns,
|
||||||
@ -9807,6 +9832,18 @@ mod tests {
|
|||||||
assert!(error.contains("unsupported tool in --allowedTools: teleport"));
|
assert!(error.contains("unsupported tool in --allowedTools: teleport"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejects_empty_allowed_tools_flag() {
|
||||||
|
for raw in ["", ",,"] {
|
||||||
|
let error = parse_args(&["--allowedTools".to_string(), raw.to_string()])
|
||||||
|
.expect_err("empty allowedTools should be rejected");
|
||||||
|
assert!(
|
||||||
|
error.contains("--allowedTools was provided with no usable tool names"),
|
||||||
|
"unexpected error for {raw:?}: {error}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parses_system_prompt_options() {
|
fn parses_system_prompt_options() {
|
||||||
let args = vec![
|
let args = vec![
|
||||||
@ -10261,7 +10298,14 @@ mod tests {
|
|||||||
cumulative: runtime::TokenUsage::default(),
|
cumulative: runtime::TokenUsage::default(),
|
||||||
estimated_tokens: 0,
|
estimated_tokens: 0,
|
||||||
};
|
};
|
||||||
let json = super::status_json_value(Some("test-model"), usage, "workspace-write", &context, None);
|
let json = super::status_json_value(
|
||||||
|
Some("test-model"),
|
||||||
|
usage,
|
||||||
|
"workspace-write",
|
||||||
|
&context,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
json.get("status").and_then(|v| v.as_str()),
|
json.get("status").and_then(|v| v.as_str()),
|
||||||
Some("degraded"),
|
Some("degraded"),
|
||||||
@ -10280,6 +10324,44 @@ mod tests {
|
|||||||
);
|
);
|
||||||
assert!(json.get("workspace").is_some(), "workspace field still reported");
|
assert!(json.get("workspace").is_some(), "workspace field still reported");
|
||||||
assert!(json.get("sandbox").is_some(), "sandbox field still reported");
|
assert!(json.get("sandbox").is_some(), "sandbox field still reported");
|
||||||
|
assert_eq!(
|
||||||
|
json.pointer("/allowed_tools/source").and_then(|v| v.as_str()),
|
||||||
|
Some("default"),
|
||||||
|
"default status should expose unrestricted tool source: {json}"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
json.pointer("/allowed_tools/restricted").and_then(|v| v.as_bool()),
|
||||||
|
Some(false),
|
||||||
|
"default status should expose unrestricted tool state: {json}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let allowed: super::AllowedToolSet = ["read_file", "grep_search"]
|
||||||
|
.into_iter()
|
||||||
|
.map(str::to_string)
|
||||||
|
.collect();
|
||||||
|
let restricted_json = super::status_json_value(
|
||||||
|
Some("test-model"),
|
||||||
|
usage,
|
||||||
|
"workspace-write",
|
||||||
|
&context,
|
||||||
|
None,
|
||||||
|
Some(&allowed),
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
restricted_json
|
||||||
|
.pointer("/allowed_tools/source")
|
||||||
|
.and_then(|v| v.as_str()),
|
||||||
|
Some("flag"),
|
||||||
|
"flag status should expose allow-list source: {restricted_json}"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
restricted_json
|
||||||
|
.pointer("/allowed_tools/entries")
|
||||||
|
.and_then(|v| v.as_array())
|
||||||
|
.map(Vec::len),
|
||||||
|
Some(2),
|
||||||
|
"flag status should expose allow-list entries: {restricted_json}"
|
||||||
|
);
|
||||||
|
|
||||||
// Clean path: no config error → status: "ok", config_load_error: null.
|
// Clean path: no config error → status: "ok", config_load_error: null.
|
||||||
let clean_cwd = root.join("project-with-clean-config");
|
let clean_cwd = root.join("project-with-clean-config");
|
||||||
@ -10288,8 +10370,14 @@ mod tests {
|
|||||||
super::status_context(None).expect("clean status_context should succeed")
|
super::status_context(None).expect("clean status_context should succeed")
|
||||||
});
|
});
|
||||||
assert!(clean_context.config_load_error.is_none());
|
assert!(clean_context.config_load_error.is_none());
|
||||||
let clean_json =
|
let clean_json = super::status_json_value(
|
||||||
super::status_json_value(Some("test-model"), usage, "workspace-write", &clean_context, None);
|
Some("test-model"),
|
||||||
|
usage,
|
||||||
|
"workspace-write",
|
||||||
|
&clean_context,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
clean_json.get("status").and_then(|v| v.as_str()),
|
clean_json.get("status").and_then(|v| v.as_str()),
|
||||||
Some("ok"),
|
Some("ok"),
|
||||||
@ -10366,6 +10454,7 @@ mod tests {
|
|||||||
model_flag_raw: None, // #148: no --model flag passed
|
model_flag_raw: None, // #148: no --model flag passed
|
||||||
permission_mode: PermissionMode::DangerFullAccess,
|
permission_mode: PermissionMode::DangerFullAccess,
|
||||||
output_format: CliOutputFormat::Text,
|
output_format: CliOutputFormat::Text,
|
||||||
|
allowed_tools: None,
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@ -240,6 +240,13 @@ impl GlobalToolRegistry {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if allowed.is_empty() {
|
||||||
|
return Err(format!(
|
||||||
|
"--allowedTools was provided with no usable tool names (got `{}`). Omit the flag to allow all tools.",
|
||||||
|
values.join(" ")
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
Ok(Some(allowed))
|
Ok(Some(allowed))
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -6883,6 +6890,21 @@ mod tests {
|
|||||||
assert!(empty_permission.contains("unsupported plugin permission: "));
|
assert!(empty_permission.contains("unsupported plugin permission: "));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn allowed_tools_rejects_empty_token_lists() {
|
||||||
|
let registry = GlobalToolRegistry::builtin();
|
||||||
|
|
||||||
|
for raw in ["", ",,", " "] {
|
||||||
|
let err = registry
|
||||||
|
.normalize_allowed_tools(&[raw.to_string()])
|
||||||
|
.expect_err("empty allow-list input should be rejected");
|
||||||
|
assert!(
|
||||||
|
err.contains("--allowedTools was provided with no usable tool names"),
|
||||||
|
"unexpected error for {raw:?}: {err}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn runtime_tools_extend_registry_definitions_permissions_and_search() {
|
fn runtime_tools_extend_registry_definitions_permissions_and_search() {
|
||||||
let registry = GlobalToolRegistry::builtin()
|
let registry = GlobalToolRegistry::builtin()
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user