mirror of
https://github.com/instructkr/claude-code.git
synced 2026-04-05 18:58:48 +03:00
Prevent invalid hook configs from poisoning merged runtime settings
Validate hook arrays in each config file before deep-merging so malformed entries fail with source-path context instead of surfacing later as a merged hook parse error. Constraint: Runtime hook config currently supports only string command arrays Rejected: Add hook-specific schema logic inside deep_merge_objects | keeps generic merge helper decoupled from config semantics Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep hook validation source-aware before generic config merges so file-specific errors remain diagnosable Tested: cargo build --workspace; cargo test --workspace Not-tested: live claw --help against a malformed external user config
This commit is contained in:
@@ -242,6 +242,7 @@ impl ConfigLoader {
|
|||||||
let Some(value) = read_optional_json_object(&entry.path)? else {
|
let Some(value) = read_optional_json_object(&entry.path)? else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
|
validate_optional_hooks_config(&value, &entry.path)?;
|
||||||
merge_mcp_servers(&mut mcp_servers, entry.source, &value, &entry.path)?;
|
merge_mcp_servers(&mut mcp_servers, entry.source, &value, &entry.path)?;
|
||||||
deep_merge_objects(&mut merged, &value);
|
deep_merge_objects(&mut merged, &value);
|
||||||
loaded_entries.push(entry);
|
loaded_entries.push(entry);
|
||||||
@@ -617,24 +618,32 @@ fn parse_optional_hooks_config(root: &JsonValue) -> Result<RuntimeHookConfig, Co
|
|||||||
let Some(object) = root.as_object() else {
|
let Some(object) = root.as_object() else {
|
||||||
return Ok(RuntimeHookConfig::default());
|
return Ok(RuntimeHookConfig::default());
|
||||||
};
|
};
|
||||||
|
parse_optional_hooks_config_object(object, "merged settings.hooks")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn parse_optional_hooks_config_object(
|
||||||
|
object: &BTreeMap<String, JsonValue>,
|
||||||
|
context: &str,
|
||||||
|
) -> Result<RuntimeHookConfig, ConfigError> {
|
||||||
let Some(hooks_value) = object.get("hooks") else {
|
let Some(hooks_value) = object.get("hooks") else {
|
||||||
return Ok(RuntimeHookConfig::default());
|
return Ok(RuntimeHookConfig::default());
|
||||||
};
|
};
|
||||||
let hooks = expect_object(hooks_value, "merged settings.hooks")?;
|
let hooks = expect_object(hooks_value, context)?;
|
||||||
Ok(RuntimeHookConfig {
|
Ok(RuntimeHookConfig {
|
||||||
pre_tool_use: optional_string_array(hooks, "PreToolUse", "merged settings.hooks")?
|
pre_tool_use: optional_string_array(hooks, "PreToolUse", context)?.unwrap_or_default(),
|
||||||
|
post_tool_use: optional_string_array(hooks, "PostToolUse", context)?.unwrap_or_default(),
|
||||||
|
post_tool_use_failure: optional_string_array(hooks, "PostToolUseFailure", context)?
|
||||||
.unwrap_or_default(),
|
.unwrap_or_default(),
|
||||||
post_tool_use: optional_string_array(hooks, "PostToolUse", "merged settings.hooks")?
|
|
||||||
.unwrap_or_default(),
|
|
||||||
post_tool_use_failure: optional_string_array(
|
|
||||||
hooks,
|
|
||||||
"PostToolUseFailure",
|
|
||||||
"merged settings.hooks",
|
|
||||||
)?
|
|
||||||
.unwrap_or_default(),
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn validate_optional_hooks_config(
|
||||||
|
root: &BTreeMap<String, JsonValue>,
|
||||||
|
path: &Path,
|
||||||
|
) -> Result<(), ConfigError> {
|
||||||
|
parse_optional_hooks_config_object(root, &format!("{}: hooks", path.display())).map(|_| ())
|
||||||
|
}
|
||||||
|
|
||||||
fn parse_optional_permission_rules(
|
fn parse_optional_permission_rules(
|
||||||
root: &JsonValue,
|
root: &JsonValue,
|
||||||
) -> Result<RuntimePermissionRuleConfig, ConfigError> {
|
) -> Result<RuntimePermissionRuleConfig, ConfigError> {
|
||||||
@@ -1513,6 +1522,43 @@ mod tests {
|
|||||||
assert!(target.contains_key("sandbox"));
|
assert!(target.contains_key("sandbox"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejects_invalid_hook_entries_before_merge() {
|
||||||
|
// given
|
||||||
|
let root = temp_dir();
|
||||||
|
let cwd = root.join("project");
|
||||||
|
let home = root.join("home").join(".claw");
|
||||||
|
let project_settings = cwd.join(".claw").join("settings.json");
|
||||||
|
fs::create_dir_all(cwd.join(".claw")).expect("project config dir");
|
||||||
|
fs::create_dir_all(&home).expect("home config dir");
|
||||||
|
|
||||||
|
fs::write(
|
||||||
|
home.join("settings.json"),
|
||||||
|
r#"{"hooks":{"PreToolUse":["base"]}}"#,
|
||||||
|
)
|
||||||
|
.expect("write user settings");
|
||||||
|
fs::write(
|
||||||
|
&project_settings,
|
||||||
|
r#"{"hooks":{"PreToolUse":["project",42]}}"#,
|
||||||
|
)
|
||||||
|
.expect("write invalid project settings");
|
||||||
|
|
||||||
|
// when
|
||||||
|
let error = ConfigLoader::new(&cwd, &home)
|
||||||
|
.load()
|
||||||
|
.expect_err("config should fail");
|
||||||
|
|
||||||
|
// then
|
||||||
|
let rendered = error.to_string();
|
||||||
|
assert!(rendered.contains(&format!(
|
||||||
|
"{}: hooks: field PreToolUse must contain only strings",
|
||||||
|
project_settings.display()
|
||||||
|
)));
|
||||||
|
assert!(!rendered.contains("merged settings.hooks"));
|
||||||
|
|
||||||
|
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn permission_mode_aliases_resolve_to_expected_modes() {
|
fn permission_mode_aliases_resolve_to_expected_modes() {
|
||||||
// given / when / then
|
// given / when / then
|
||||||
|
|||||||
Reference in New Issue
Block a user