mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Support addDirectories in PermissionRequest hooks
Add hook schema and runtime support for decision.addDirectories, persist writable roots by destination, and reuse granted roots in later shell approvals. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -3490,6 +3490,11 @@ impl Session {
|
||||
state.granted_permissions()
|
||||
}
|
||||
|
||||
pub(crate) async fn record_granted_session_permissions(&self, permissions: PermissionProfile) {
|
||||
let mut state = self.state.lock().await;
|
||||
state.record_granted_permissions(permissions);
|
||||
}
|
||||
|
||||
pub async fn notify_dynamic_tool_response(&self, call_id: &str, response: DynamicToolResponse) {
|
||||
let entry = {
|
||||
let mut active = self.active_turn.lock().await;
|
||||
|
||||
@@ -51,6 +51,8 @@ pub enum ConfigEdit {
|
||||
SetSkillConfig { path: PathBuf, enabled: bool },
|
||||
/// Set or clear a skill config entry under `[[skills.config]]` by name.
|
||||
SetSkillConfigByName { name: String, enabled: bool },
|
||||
/// Append normalized writable roots under `[sandbox_workspace_write]`.
|
||||
AppendSandboxWorkspaceWriteRoots { roots: Vec<PathBuf> },
|
||||
/// Set trust_level under `[projects."<path>"]`,
|
||||
/// migrating inline tables to explicit tables.
|
||||
SetProjectTrustLevel { path: PathBuf, level: TrustLevel },
|
||||
@@ -425,6 +427,9 @@ impl ConfigDocument {
|
||||
ConfigEdit::SetSkillConfigByName { name, enabled } => {
|
||||
Ok(self.set_skill_config(SkillConfigSelector::Name(name.clone()), *enabled))
|
||||
}
|
||||
ConfigEdit::AppendSandboxWorkspaceWriteRoots { roots } => {
|
||||
Ok(self.append_sandbox_workspace_write_roots(roots))
|
||||
}
|
||||
ConfigEdit::SetPath { segments, value } => Ok(self.insert(segments, value.clone())),
|
||||
ConfigEdit::ClearPath { segments } => Ok(self.clear_owned(segments)),
|
||||
ConfigEdit::SetProjectTrustLevel { path, level } => {
|
||||
@@ -625,6 +630,58 @@ impl ConfigDocument {
|
||||
mutated
|
||||
}
|
||||
|
||||
fn append_sandbox_workspace_write_roots(&mut self, roots: &[PathBuf]) -> bool {
|
||||
if roots.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
let Some(table) = self.descend(
|
||||
&["sandbox_workspace_write".to_string()],
|
||||
TraversalMode::Create,
|
||||
) else {
|
||||
return false;
|
||||
};
|
||||
|
||||
let mut created_writable_roots = false;
|
||||
let writable_roots = table.entry("writable_roots").or_insert_with(|| {
|
||||
created_writable_roots = true;
|
||||
let array = roots
|
||||
.iter()
|
||||
.map(|root| root.to_string_lossy().to_string())
|
||||
.collect::<toml_edit::Array>();
|
||||
TomlItem::Value(array.into())
|
||||
});
|
||||
|
||||
let Some(array) = writable_roots
|
||||
.as_value_mut()
|
||||
.and_then(toml_edit::Value::as_array_mut)
|
||||
else {
|
||||
let replacement = roots
|
||||
.iter()
|
||||
.map(|root| root.to_string_lossy().to_string())
|
||||
.collect::<toml_edit::Array>();
|
||||
let mut replacement = TomlItem::Value(replacement.into());
|
||||
Self::preserve_decor(writable_roots, &mut replacement);
|
||||
*writable_roots = replacement;
|
||||
return true;
|
||||
};
|
||||
|
||||
let mut mutated = created_writable_roots;
|
||||
for root in roots {
|
||||
let root = root.to_string_lossy().to_string();
|
||||
if !array
|
||||
.iter()
|
||||
.filter_map(toml_edit::Value::as_str)
|
||||
.any(|entry| entry == root)
|
||||
{
|
||||
array.push(root);
|
||||
mutated = true;
|
||||
}
|
||||
}
|
||||
|
||||
mutated
|
||||
}
|
||||
|
||||
fn scoped_segments(&self, scope: Scope, segments: &[&str]) -> Vec<String> {
|
||||
let resolved: Vec<String> = segments
|
||||
.iter()
|
||||
|
||||
@@ -340,6 +340,55 @@ model_reasoning_effort = "high"
|
||||
assert_eq!(contents, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_sandbox_workspace_write_roots_preserves_existing_keys() {
|
||||
let tmp = tempdir().expect("tmpdir");
|
||||
let codex_home = tmp.path();
|
||||
let original = r#"[sandbox_workspace_write]
|
||||
network_access = false
|
||||
writable_roots = ["/tmp/existing"]
|
||||
"#;
|
||||
std::fs::write(codex_home.join(CONFIG_TOML_FILE), original).expect("seed config");
|
||||
|
||||
apply_blocking(
|
||||
codex_home,
|
||||
/*profile*/ None,
|
||||
&[ConfigEdit::AppendSandboxWorkspaceWriteRoots {
|
||||
roots: vec![
|
||||
PathBuf::from("/tmp/existing"),
|
||||
PathBuf::from("/tmp/new-root"),
|
||||
],
|
||||
}],
|
||||
)
|
||||
.expect("persist");
|
||||
|
||||
let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
|
||||
let expected = r#"[sandbox_workspace_write]
|
||||
network_access = false
|
||||
writable_roots = ["/tmp/existing", "/tmp/new-root"]"#;
|
||||
assert_eq!(contents.trim_end(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_sandbox_workspace_write_roots_creates_config_file() {
|
||||
let tmp = tempdir().expect("tmpdir");
|
||||
let codex_home = tmp.path();
|
||||
|
||||
apply_blocking(
|
||||
codex_home,
|
||||
/*profile*/ None,
|
||||
&[ConfigEdit::AppendSandboxWorkspaceWriteRoots {
|
||||
roots: vec![PathBuf::from("/tmp/new-root")],
|
||||
}],
|
||||
)
|
||||
.expect("persist");
|
||||
|
||||
let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
|
||||
let expected = r#"[sandbox_workspace_write]
|
||||
writable_roots = ["/tmp/new-root"]"#;
|
||||
assert_eq!(contents.trim_end(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn blocking_set_model_scopes_to_active_profile() {
|
||||
let tmp = tempdir().expect("tmpdir");
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use std::future::Future;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_hooks::PermissionDirectoryUpdate;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_hooks::PermissionRequestOutcome;
|
||||
use codex_hooks::PermissionRequestRequest;
|
||||
@@ -18,6 +20,8 @@ use codex_hooks::UserPromptSubmitRequest;
|
||||
use codex_protocol::approvals::ExecPolicyAmendment;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::DeveloperInstructions;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -33,9 +37,13 @@ use tracing::warn;
|
||||
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::config::edit::ConfigEdit;
|
||||
use crate::config::edit::ConfigEditsBuilder;
|
||||
use crate::config_loader::resolve_project_root;
|
||||
use crate::event_mapping::parse_turn_item;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
pub(crate) struct HookRuntimeOutcome {
|
||||
pub should_stop: bool,
|
||||
@@ -191,9 +199,16 @@ pub(crate) async fn run_permission_request_hooks(
|
||||
|
||||
if let Some(PermissionRequestDecision::Allow {
|
||||
updated_permissions,
|
||||
add_directories,
|
||||
}) = &decision
|
||||
{
|
||||
apply_permission_updates_from_hook(sess, turn_context, updated_permissions).await;
|
||||
apply_permission_updates_from_hook(
|
||||
sess,
|
||||
turn_context,
|
||||
updated_permissions,
|
||||
add_directories,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
decision
|
||||
@@ -203,6 +218,7 @@ async fn apply_permission_updates_from_hook(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
updated_permissions: &[PermissionSuggestion],
|
||||
add_directories: &[PermissionDirectoryUpdate],
|
||||
) {
|
||||
for permission in updated_permissions {
|
||||
if let Err(err) = apply_permission_update_from_hook(sess, turn_context, permission).await {
|
||||
@@ -216,6 +232,21 @@ async fn apply_permission_updates_from_hook(
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
for directory_update in add_directories {
|
||||
if let Err(err) =
|
||||
apply_directory_update_from_hook(sess, turn_context, directory_update).await
|
||||
{
|
||||
let message =
|
||||
format!("PermissionRequest hook failed to apply updated writable roots: {err}");
|
||||
warn!("{message}");
|
||||
sess.send_event_raw(Event {
|
||||
id: turn_context.sub_id.clone(),
|
||||
msg: EventMsg::Warning(WarningEvent { message }),
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn apply_permission_update_from_hook(
|
||||
@@ -284,6 +315,95 @@ async fn apply_execpolicy_amendment_destination(
|
||||
}
|
||||
}
|
||||
|
||||
async fn apply_directory_update_from_hook(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
directory_update: &PermissionDirectoryUpdate,
|
||||
) -> Result<(), String> {
|
||||
let writable_roots = resolve_directory_update_paths(&turn_context.cwd, directory_update)?;
|
||||
|
||||
match directory_update.destination {
|
||||
PermissionSuggestionDestination::Session => {}
|
||||
PermissionSuggestionDestination::UserSettings => {
|
||||
let codex_home = sess.codex_home().await;
|
||||
append_writable_roots_to_config(codex_home.as_path(), &writable_roots).await?;
|
||||
}
|
||||
PermissionSuggestionDestination::ProjectSettings => {
|
||||
let config = turn_context.config.as_ref();
|
||||
let project_codex_home = resolve_project_root(&config.config_layer_stack, &config.cwd)
|
||||
.await
|
||||
.map_err(|err| format!("failed to resolve project root for writable roots: {err}"))?
|
||||
.join(".codex")
|
||||
.to_path_buf();
|
||||
tokio::fs::create_dir_all(&project_codex_home)
|
||||
.await
|
||||
.map_err(|err| format!("failed to create project config directory: {err}"))?;
|
||||
append_writable_roots_to_config(&project_codex_home, &writable_roots).await?;
|
||||
}
|
||||
}
|
||||
|
||||
sess.record_granted_session_permissions(permission_profile_for_writable_roots(&writable_roots))
|
||||
.await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn resolve_directory_update_paths(
|
||||
cwd: &AbsolutePathBuf,
|
||||
directory_update: &PermissionDirectoryUpdate,
|
||||
) -> Result<Vec<AbsolutePathBuf>, String> {
|
||||
let writable_roots = directory_update
|
||||
.directories
|
||||
.iter()
|
||||
.map(|directory| {
|
||||
let trimmed = directory.trim();
|
||||
if trimmed.is_empty() {
|
||||
return Err("writable root update contained an empty directory".to_string());
|
||||
}
|
||||
|
||||
let resolved = if Path::new(trimmed).is_absolute() {
|
||||
Path::new(trimmed).to_path_buf()
|
||||
} else {
|
||||
cwd.join(trimmed).to_path_buf()
|
||||
};
|
||||
|
||||
AbsolutePathBuf::from_absolute_path(resolved.as_path())
|
||||
.map_err(|err| format!("invalid writable root `{trimmed}`: {err}"))
|
||||
})
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
|
||||
normalize_additional_permissions(permission_profile_for_writable_roots(&writable_roots))
|
||||
.map_err(|err| format!("failed to normalize writable roots: {err}"))?
|
||||
.file_system
|
||||
.and_then(|file_system| file_system.write)
|
||||
.filter(|write_paths| !write_paths.is_empty())
|
||||
.ok_or_else(|| "writable root update did not include any directories".to_string())
|
||||
}
|
||||
|
||||
fn permission_profile_for_writable_roots(writable_roots: &[AbsolutePathBuf]) -> PermissionProfile {
|
||||
PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: None,
|
||||
write: Some(writable_roots.to_vec()),
|
||||
}),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
async fn append_writable_roots_to_config(
|
||||
codex_home: &Path,
|
||||
writable_roots: &[AbsolutePathBuf],
|
||||
) -> Result<(), String> {
|
||||
ConfigEditsBuilder::new(codex_home)
|
||||
.with_edits([ConfigEdit::AppendSandboxWorkspaceWriteRoots {
|
||||
roots: writable_roots
|
||||
.iter()
|
||||
.map(codex_utils_absolute_path::AbsolutePathBuf::to_path_buf)
|
||||
.collect(),
|
||||
}])
|
||||
.apply()
|
||||
.await
|
||||
.map_err(|err| format!("failed to persist writable roots: {err}"))
|
||||
}
|
||||
pub(crate) async fn run_post_tool_use_hooks(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
|
||||
@@ -37,6 +37,7 @@ use crate::tools::sandboxing::ToolCtx;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_shell_command::is_safe_command::is_known_safe_command;
|
||||
use codex_tools::ShellCommandBackendConfig;
|
||||
|
||||
@@ -501,6 +502,15 @@ impl ShellHandler {
|
||||
);
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let exec_approval_file_system_sandbox_policy =
|
||||
if effective_additional_permissions.permissions_preapproved {
|
||||
effective_file_system_sandbox_policy(
|
||||
&turn.file_system_sandbox_policy,
|
||||
normalized_additional_permissions.as_ref(),
|
||||
)
|
||||
} else {
|
||||
turn.file_system_sandbox_policy.clone()
|
||||
};
|
||||
let exec_approval_requirement = session
|
||||
.services
|
||||
.exec_policy
|
||||
@@ -508,7 +518,7 @@ impl ShellHandler {
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
sandbox_policy: turn.sandbox_policy.get(),
|
||||
file_system_sandbox_policy: &turn.file_system_sandbox_policy,
|
||||
file_system_sandbox_policy: &exec_approval_file_system_sandbox_policy,
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
|
||||
codex_protocol::models::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
|
||||
@@ -48,6 +48,7 @@ use crate::unified_exec::process::OutputHandles;
|
||||
use crate::unified_exec::process::SpawnLifecycleHandle;
|
||||
use crate::unified_exec::process::UnifiedExecProcess;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_output_truncation::approx_token_count;
|
||||
|
||||
@@ -658,6 +659,15 @@ impl UnifiedExecProcessManager {
|
||||
self,
|
||||
context.turn.tools_config.unified_exec_shell_mode.clone(),
|
||||
);
|
||||
let exec_approval_file_system_sandbox_policy = if request.additional_permissions_preapproved
|
||||
{
|
||||
effective_file_system_sandbox_policy(
|
||||
&context.turn.file_system_sandbox_policy,
|
||||
request.additional_permissions.as_ref(),
|
||||
)
|
||||
} else {
|
||||
context.turn.file_system_sandbox_policy.clone()
|
||||
};
|
||||
let exec_approval_requirement = context
|
||||
.session
|
||||
.services
|
||||
@@ -666,7 +676,7 @@ impl UnifiedExecProcessManager {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
sandbox_policy: context.turn.sandbox_policy.get(),
|
||||
file_system_sandbox_policy: &context.turn.file_system_sandbox_policy,
|
||||
file_system_sandbox_policy: &exec_approval_file_system_sandbox_policy,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
|
||||
@@ -326,6 +326,45 @@ elif mode == "allow_selected_user":
|
||||
}}
|
||||
}}
|
||||
}}))
|
||||
elif mode == "allow_add_directories_session":
|
||||
print(json.dumps({{
|
||||
"hookSpecificOutput": {{
|
||||
"hookEventName": "PermissionRequest",
|
||||
"decision": {{
|
||||
"behavior": "allow",
|
||||
"addDirectories": [{{
|
||||
"directories": [reason],
|
||||
"destination": "session"
|
||||
}}]
|
||||
}}
|
||||
}}
|
||||
}}))
|
||||
elif mode == "allow_add_directories_project":
|
||||
print(json.dumps({{
|
||||
"hookSpecificOutput": {{
|
||||
"hookEventName": "PermissionRequest",
|
||||
"decision": {{
|
||||
"behavior": "allow",
|
||||
"addDirectories": [{{
|
||||
"directories": [reason],
|
||||
"destination": "projectSettings"
|
||||
}}]
|
||||
}}
|
||||
}}
|
||||
}}))
|
||||
elif mode == "allow_add_directories_user":
|
||||
print(json.dumps({{
|
||||
"hookSpecificOutput": {{
|
||||
"hookEventName": "PermissionRequest",
|
||||
"decision": {{
|
||||
"behavior": "allow",
|
||||
"addDirectories": [{{
|
||||
"directories": [reason],
|
||||
"destination": "userSettings"
|
||||
}}]
|
||||
}}
|
||||
}}
|
||||
}}))
|
||||
elif mode == "allow_unoffered_permission":
|
||||
print(json.dumps({{
|
||||
"hookSpecificOutput": {{
|
||||
@@ -1530,6 +1569,125 @@ async fn permission_request_hook_persists_selected_user_exec_rule() -> Result<()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn permission_request_hook_persists_user_writable_root_and_reuses_it() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let call_id_first = "permissionrequest-user-dir-first";
|
||||
let call_id_second = "permissionrequest-user-dir-second";
|
||||
let requested_dir = std::env::temp_dir().join("permissionrequest-user-dir");
|
||||
fs::create_dir_all(&requested_dir)?;
|
||||
let first_file = requested_dir.join("first.txt");
|
||||
let second_file = requested_dir.join("second.txt");
|
||||
let first_command = format!("printf first > {}", first_file.display());
|
||||
let second_command = format!("printf second > {}", second_file.display());
|
||||
let first_args = serde_json::json!({
|
||||
"command": first_command,
|
||||
"sandbox_permissions": "require_escalated",
|
||||
});
|
||||
let second_args = serde_json::json!({
|
||||
"command": second_command,
|
||||
});
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(
|
||||
call_id_first,
|
||||
"shell_command",
|
||||
&serde_json::to_string(&first_args)?,
|
||||
),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "persisted writable root"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-3"),
|
||||
ev_function_call(
|
||||
call_id_second,
|
||||
"shell_command",
|
||||
&serde_json::to_string(&second_args)?,
|
||||
),
|
||||
ev_completed("resp-3"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-4"),
|
||||
ev_assistant_message("msg-2", "reused writable root"),
|
||||
ev_completed("resp-4"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let requested_dir_string = requested_dir.display().to_string();
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(move |home| {
|
||||
if let Err(error) = write_permission_request_hook(
|
||||
home,
|
||||
Some(PERMISSION_REQUEST_HOOK_MATCHER),
|
||||
"allow_add_directories_user",
|
||||
&requested_dir_string,
|
||||
) {
|
||||
panic!("failed to write permission request hook test fixture: {error}");
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodexHooks)
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let _ = fs::remove_file(&first_file);
|
||||
let _ = fs::remove_file(&second_file);
|
||||
|
||||
test.submit_turn_with_policies(
|
||||
"run the first shell command after hook approval",
|
||||
AskForApproval::OnRequest,
|
||||
SandboxPolicy::new_read_only_policy(),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(fs::read_to_string(&first_file)?, "first");
|
||||
|
||||
test.submit_turn_with_policies(
|
||||
"run the second shell command with the persisted writable root",
|
||||
AskForApproval::OnRequest,
|
||||
SandboxPolicy::new_read_only_policy(),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(fs::read_to_string(&second_file)?, "second");
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 4);
|
||||
requests[1].function_call_output(call_id_first);
|
||||
requests[3].function_call_output(call_id_second);
|
||||
|
||||
let hook_inputs = read_permission_request_hook_inputs(test.codex_home_path())?;
|
||||
assert_eq!(
|
||||
hook_inputs.len(),
|
||||
1,
|
||||
"persisted writable root should bypass the second permission hook"
|
||||
);
|
||||
|
||||
let config_path = test.codex_home_path().join("config.toml");
|
||||
let config_contents = fs::read_to_string(&config_path)
|
||||
.with_context(|| format!("read {}", config_path.display()))?;
|
||||
assert!(
|
||||
config_contents.contains("writable_roots")
|
||||
&& config_contents.contains(&requested_dir.display().to_string()),
|
||||
"expected {config_path:?} to contain writable root for {}, got:\n{config_contents}",
|
||||
requested_dir.display(),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn permission_request_hook_allows_network_approval_without_prompt() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -13,6 +13,24 @@
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
"PermissionDirectoryUpdate": {
|
||||
"properties": {
|
||||
"destination": {
|
||||
"$ref": "#/definitions/PermissionSuggestionDestination"
|
||||
},
|
||||
"directories": {
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"destination",
|
||||
"directories"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"PermissionRequestBehaviorWire": {
|
||||
"enum": [
|
||||
"allow",
|
||||
@@ -23,6 +41,13 @@
|
||||
"PermissionRequestDecisionWire": {
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
"addDirectories": {
|
||||
"default": null,
|
||||
"items": {
|
||||
"$ref": "#/definitions/PermissionDirectoryUpdate"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"behavior": {
|
||||
"$ref": "#/definitions/PermissionRequestBehaviorWire"
|
||||
},
|
||||
|
||||
@@ -23,6 +23,7 @@ pub(crate) struct PreToolUseOutput {
|
||||
pub(crate) enum PermissionRequestDecision {
|
||||
Allow {
|
||||
updated_permissions: Vec<crate::events::permission_request::PermissionSuggestion>,
|
||||
add_directories: Vec<crate::events::permission_request::PermissionDirectoryUpdate>,
|
||||
},
|
||||
Deny {
|
||||
message: String,
|
||||
@@ -305,9 +306,12 @@ fn unsupported_permission_request_hook_specific_output(
|
||||
if decision.updated_input.is_some() {
|
||||
Some("PermissionRequest hook returned unsupported updatedInput".to_string())
|
||||
} else if matches!(decision.behavior, PermissionRequestBehaviorWire::Deny)
|
||||
&& decision.updated_permissions.is_some()
|
||||
&& (decision.updated_permissions.is_some() || decision.add_directories.is_some())
|
||||
{
|
||||
Some("PermissionRequest hook returned updatedPermissions for deny decision".to_string())
|
||||
Some(
|
||||
"PermissionRequest hook returned updatedPermissions/addDirectories for deny decision"
|
||||
.to_string(),
|
||||
)
|
||||
} else if decision.interrupt {
|
||||
Some("PermissionRequest hook returned unsupported interrupt:true".to_string())
|
||||
} else {
|
||||
@@ -321,6 +325,7 @@ fn permission_request_decision(
|
||||
match decision.behavior {
|
||||
PermissionRequestBehaviorWire::Allow => PermissionRequestDecision::Allow {
|
||||
updated_permissions: decision.updated_permissions.clone().unwrap_or_default(),
|
||||
add_directories: decision.add_directories.clone().unwrap_or_default(),
|
||||
},
|
||||
PermissionRequestBehaviorWire::Deny => PermissionRequestDecision::Deny {
|
||||
message: decision
|
||||
@@ -495,6 +500,42 @@ mod tests {
|
||||
crate::events::permission_request::PermissionSuggestionBehavior::Allow,
|
||||
destination: crate::events::permission_request::PermissionSuggestionDestination::UserSettings,
|
||||
}],
|
||||
add_directories: vec![],
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_request_accepts_add_directories_for_allow_decision() {
|
||||
let parsed = parse_permission_request(
|
||||
&json!({
|
||||
"continue": true,
|
||||
"hookSpecificOutput": {
|
||||
"hookEventName": "PermissionRequest",
|
||||
"decision": {
|
||||
"behavior": "allow",
|
||||
"addDirectories": [{
|
||||
"directories": ["./logs", "/tmp/output"],
|
||||
"destination": "session"
|
||||
}]
|
||||
}
|
||||
}
|
||||
})
|
||||
.to_string(),
|
||||
)
|
||||
.expect("permission request hook output should parse");
|
||||
|
||||
assert_eq!(
|
||||
parsed.decision,
|
||||
Some(PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![
|
||||
crate::events::permission_request::PermissionDirectoryUpdate {
|
||||
directories: vec!["./logs".to_string(), "/tmp/output".to_string()],
|
||||
destination:
|
||||
crate::events::permission_request::PermissionSuggestionDestination::Session,
|
||||
},
|
||||
],
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -527,7 +568,37 @@ mod tests {
|
||||
assert_eq!(
|
||||
parsed.invalid_reason,
|
||||
Some(
|
||||
"PermissionRequest hook returned updatedPermissions for deny decision".to_string()
|
||||
"PermissionRequest hook returned updatedPermissions/addDirectories for deny decision"
|
||||
.to_string()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_request_rejects_add_directories_for_deny_decision() {
|
||||
let parsed = parse_permission_request(
|
||||
&json!({
|
||||
"continue": true,
|
||||
"hookSpecificOutput": {
|
||||
"hookEventName": "PermissionRequest",
|
||||
"decision": {
|
||||
"behavior": "deny",
|
||||
"addDirectories": [{
|
||||
"directories": ["./logs"],
|
||||
"destination": "session"
|
||||
}]
|
||||
}
|
||||
}
|
||||
})
|
||||
.to_string(),
|
||||
)
|
||||
.expect("permission request hook output should parse");
|
||||
|
||||
assert_eq!(
|
||||
parsed.invalid_reason,
|
||||
Some(
|
||||
"PermissionRequest hook returned updatedPermissions/addDirectories for deny decision"
|
||||
.to_string()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -45,6 +45,13 @@ pub struct PermissionSuggestion {
|
||||
pub destination: PermissionSuggestionDestination,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct PermissionDirectoryUpdate {
|
||||
pub directories: Vec<String>,
|
||||
pub destination: PermissionSuggestionDestination,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub enum PermissionSuggestionType {
|
||||
@@ -94,6 +101,7 @@ pub struct PermissionRequestRequest {
|
||||
pub enum PermissionRequestDecision {
|
||||
Allow {
|
||||
updated_permissions: Vec<PermissionSuggestion>,
|
||||
add_directories: Vec<PermissionDirectoryUpdate>,
|
||||
},
|
||||
Deny {
|
||||
message: String,
|
||||
@@ -214,10 +222,12 @@ fn resolve_permission_request_decision<'a>(
|
||||
) -> Option<PermissionRequestDecision> {
|
||||
let mut saw_allow = false;
|
||||
let mut updated_permissions = Vec::new();
|
||||
let mut add_directories = Vec::new();
|
||||
for decision in decisions {
|
||||
match decision {
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: selected_permissions,
|
||||
add_directories: selected_directories,
|
||||
} => {
|
||||
saw_allow = true;
|
||||
for permission in selected_permissions {
|
||||
@@ -225,6 +235,11 @@ fn resolve_permission_request_decision<'a>(
|
||||
updated_permissions.push(permission.clone());
|
||||
}
|
||||
}
|
||||
for directory_update in selected_directories {
|
||||
if !add_directories.contains(directory_update) {
|
||||
add_directories.push(directory_update.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
PermissionRequestDecision::Deny { message } => {
|
||||
return Some(PermissionRequestDecision::Deny {
|
||||
@@ -235,6 +250,7 @@ fn resolve_permission_request_decision<'a>(
|
||||
}
|
||||
saw_allow.then_some(PermissionRequestDecision::Allow {
|
||||
updated_permissions,
|
||||
add_directories,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -244,6 +260,7 @@ fn invalid_permission_updates(
|
||||
) -> Option<String> {
|
||||
let PermissionRequestDecision::Allow {
|
||||
updated_permissions,
|
||||
add_directories: _,
|
||||
} = decision?
|
||||
else {
|
||||
return None;
|
||||
@@ -317,9 +334,11 @@ fn parse_completed(
|
||||
match parsed_decision {
|
||||
output_parser::PermissionRequestDecision::Allow {
|
||||
updated_permissions,
|
||||
add_directories,
|
||||
} => {
|
||||
decision = Some(PermissionRequestDecision::Allow {
|
||||
updated_permissions,
|
||||
add_directories,
|
||||
});
|
||||
}
|
||||
output_parser::PermissionRequestDecision::Deny { message } => {
|
||||
@@ -388,6 +407,7 @@ fn parse_completed(
|
||||
mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::PermissionDirectoryUpdate;
|
||||
use super::PermissionRequestDecision;
|
||||
use super::PermissionSuggestion;
|
||||
use super::PermissionSuggestionBehavior;
|
||||
@@ -402,6 +422,7 @@ mod tests {
|
||||
let decisions = [
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![],
|
||||
},
|
||||
PermissionRequestDecision::Deny {
|
||||
message: "repo deny".to_string(),
|
||||
@@ -421,9 +442,11 @@ mod tests {
|
||||
let decisions = [
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![],
|
||||
},
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![],
|
||||
},
|
||||
];
|
||||
|
||||
@@ -431,6 +454,7 @@ mod tests {
|
||||
resolve_permission_request_decision(decisions.iter()),
|
||||
Some(PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![],
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -455,9 +479,11 @@ mod tests {
|
||||
let decisions = [
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![permission.clone()],
|
||||
add_directories: vec![],
|
||||
},
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![permission.clone()],
|
||||
add_directories: vec![],
|
||||
},
|
||||
];
|
||||
|
||||
@@ -465,6 +491,33 @@ mod tests {
|
||||
resolve_permission_request_decision(decisions.iter()),
|
||||
Some(PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![permission],
|
||||
add_directories: vec![],
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_request_accumulates_distinct_directory_updates() {
|
||||
let directory_update = PermissionDirectoryUpdate {
|
||||
directories: vec!["./logs".to_string(), "/tmp/output".to_string()],
|
||||
destination: PermissionSuggestionDestination::Session,
|
||||
};
|
||||
let decisions = [
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![directory_update.clone()],
|
||||
},
|
||||
PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![directory_update.clone()],
|
||||
},
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
resolve_permission_request_decision(decisions.iter()),
|
||||
Some(PermissionRequestDecision::Allow {
|
||||
updated_permissions: vec![],
|
||||
add_directories: vec![directory_update],
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -488,6 +541,7 @@ mod tests {
|
||||
behavior: PermissionSuggestionBehavior::Allow,
|
||||
destination: PermissionSuggestionDestination::UserSettings,
|
||||
}],
|
||||
add_directories: vec![],
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
|
||||
@@ -5,6 +5,7 @@ mod registry;
|
||||
mod schema;
|
||||
mod types;
|
||||
|
||||
pub use events::permission_request::PermissionDirectoryUpdate;
|
||||
pub use events::permission_request::PermissionRequestDecision;
|
||||
pub use events::permission_request::PermissionRequestOutcome;
|
||||
pub use events::permission_request::PermissionRequestRequest;
|
||||
|
||||
@@ -12,6 +12,7 @@ use serde_json::Value;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::events::permission_request::PermissionDirectoryUpdate;
|
||||
use crate::events::permission_request::PermissionSuggestion;
|
||||
|
||||
const GENERATED_DIR: &str = "generated";
|
||||
@@ -148,6 +149,8 @@ pub(crate) struct PermissionRequestDecisionWire {
|
||||
#[serde(default)]
|
||||
pub updated_permissions: Option<Vec<PermissionSuggestion>>,
|
||||
#[serde(default)]
|
||||
pub add_directories: Option<Vec<PermissionDirectoryUpdate>>,
|
||||
#[serde(default)]
|
||||
pub message: Option<String>,
|
||||
/// Reserved for future short-circuiting semantics.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user