Support model-specific writable-root permission wording (#56309)

## What changed

Add optional `writable_root` and `writable_roots` templates to model-catalog permission messages. Select the template by the number of displayed roots and substitute exactly one `{{ roots }}` placeholder with the existing formatted path list.

Fall back to the bundled sentence for missing, empty, invalid, or oversized templates. Preserve path contents, permission-path limits, and denied-read instructions. Update the permission wording when the selected model changes.

## Testing

Add coverage for template validation and size limits, singular/plural selection after path truncation, path preservation, sparse serialization, and model switches that append changed permissions once while preserving request history.

GitOrigin-RevId: 4dcfb7ff70b91db3dba3dda684b355b920e07974
This commit is contained in:
goodman-oai
2026-10-23 00:40:04 +00:00
committed by copyberry
parent 334527705c
commit 878070b5a6
14 changed files with 644 additions and 58 deletions
@@ -269,6 +269,7 @@ fn permissions_state(
workspace_write: Some("Workspace write.".to_string()),
read_only: Some("Read only.".to_string()),
approved_prefixes: None,
..PermissionMessages::default()
};
let mut model = model_info_from_slug("test-model");
model.model_messages = Some(ModelMessages {
+3 -1
View File
@@ -236,10 +236,12 @@ impl Session {
let cwd = environment
.and_then(|environment| environment.cwd().to_abs_path().ok())
.unwrap_or_else(|| turn_context.cwd.clone());
let approval_context =
ApprovalPromptContext::new(settings.approvals_reviewer(), model_messages);
world_state.add_section(PermissionsState::new(
&permission_profile,
settings.approval_policy(),
ApprovalPromptContext::new(settings.approvals_reviewer(), model_messages),
approval_context,
exec_policy.as_ref(),
&cwd,
paths.as_ref(),
@@ -57,6 +57,7 @@ async fn catalog_permission_message_loaded_from_remote_models_is_sent(
workspace_write: None,
read_only: Some("remote catalog permissions: {{ network_access }}".to_string()),
approved_prefixes: Some(prefix_template.to_string()),
..PermissionMessages::default()
}),
multi_agent: None,
token_budget: None,
+257 -39
View File
@@ -9,6 +9,8 @@ use codex_protocol::openai_models::ApprovalMessages;
use codex_protocol::openai_models::ModelMessages;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::openai_models::PermissionMessages;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
@@ -25,14 +27,17 @@ use core_test_support::skip_if_no_network;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::collections::HashSet;
use tempfile::TempDir;
use test_case::test_case;
fn permissions_texts(request: &ResponsesRequest) -> Vec<String> {
request
.message_input_texts("developer")
.into_iter()
.filter(|text| text.contains("<permissions instructions>"))
.map(|text| text.replace("\r\n", "\n"))
.collect()
}
@@ -254,6 +259,8 @@ async fn catalog_permission_message_is_sent_initially_and_after_model_change() -
workspace_write: None,
read_only: Some("model A permissions: {{ network_access }}".to_string()),
approved_prefixes: None,
writable_root: None,
writable_roots: None,
},
);
let second = model_with_permission_messages(
@@ -263,6 +270,8 @@ async fn catalog_permission_message_is_sent_initially_and_after_model_change() -
workspace_write: None,
read_only: Some("model B permissions: {{ network_access }}".to_string()),
approved_prefixes: None,
writable_root: None,
writable_roots: None,
},
);
let mut builder = test_codex()
@@ -329,6 +338,8 @@ async fn empty_catalog_permission_message_preserves_approval_instructions() -> R
workspace_write: None,
read_only: Some(String::new()),
approved_prefixes: None,
writable_root: None,
writable_roots: None,
},
);
let mut builder = test_codex()
@@ -738,8 +749,50 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> {
Ok(())
}
const WORKSPACE_PERMISSIONS_PREFIX: &str = concat!(
"<permissions instructions>\n",
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval. Network access is restricted.\n",
"Approval policy is currently never. Do not provide the `sandbox_permissions` for any reason, commands will be rejected.\n",
);
#[test_case(
1,
PermissionMessages::default(),
" The writable root is {roots}.";
"default singular"
)]
#[test_case(
2,
PermissionMessages::default(),
" The writable roots are {roots}.";
"default plural"
)]
#[test_case(
1,
PermissionMessages {
writable_root: Some("Catalog single: {{ roots }}!".to_string()),
writable_roots: Some("Catalog plural: {{ roots }}!".to_string()),
..Default::default()
},
"Catalog single: {roots}!";
"override singular"
)]
#[test_case(
2,
PermissionMessages {
writable_root: Some("Catalog single: {{ roots }}!".to_string()),
writable_roots: Some("Catalog plural: {{ roots }}!".to_string()),
..Default::default()
},
"Catalog plural: {roots}!";
"override plural"
)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn permissions_message_includes_writable_roots() -> Result<()> {
async fn permissions_message_includes_writable_roots(
root_count: usize,
permissions: PermissionMessages,
expected_root_sentence: &str,
) -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -750,53 +803,218 @@ async fn permissions_message_includes_writable_roots() -> Result<()> {
.await;
let writable = TempDir::new()?;
let writable_root = AbsolutePathBuf::try_from(writable.path().canonicalize()?)?;
let writable_root_for_config = writable_root.clone();
let permission_profile = PermissionProfile::workspace_write_with(
std::slice::from_ref(&writable_root),
let extra_roots = if root_count == 2 {
vec![writable_root.clone()]
} else {
vec![]
};
let denied_root = writable_root.join("blocked");
let mut file_system = PermissionProfile::workspace_write_with(
&extra_roots,
NetworkSandboxPolicy::Restricted,
/*exclude_tmpdir_env_var*/ true,
/*exclude_slash_tmp*/ true,
);
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config
.permissions
.set_permission_profile(permission_profile)
.expect("test permission profile should be allowed");
let workspace_roots = vec![config.cwd.clone(), writable_root_for_config];
config.workspace_roots = workspace_roots.clone();
config.permissions.set_workspace_roots(workspace_roots);
config.config_layer_stack = ConfigLayerStack::default();
});
)
.file_system_sandbox_policy();
// Windows restricted-token sandboxes cannot enforce deny-read policies.
// Keep the writable-root cases enabled there without the extra denial.
let denied_reads = if cfg!(windows) {
String::new()
} else {
file_system.entries.push(FileSystemSandboxEntry::new(
denied_root.clone().into(),
FileSystemAccessMode::Deny,
));
let denied_root = denied_root.to_string_lossy();
format!(
"\n## Denied filesystem reads\nThe active permission profile denies reading these paths/globs. Do not request escalation or additional permissions to read them; these denials are policy restrictions.\n- path `{denied_root}`"
)
};
let permission_profile =
PermissionProfile::from_runtime_permissions(&file_system, NetworkSandboxPolicy::Restricted);
let model_slug = "catalog-writable-roots-model";
let mut model = model_with_permission_messages(model_slug, permissions);
model.use_responses_lite = true;
let mut builder = test_codex()
.with_model(model_slug)
.with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never);
config
.permissions
.set_permission_profile(permission_profile)
.expect("test permission profile should be allowed");
let mut workspace_roots = vec![config.cwd.clone()];
workspace_roots.extend(extra_roots);
config.workspace_roots = workspace_roots.clone();
config.permissions.set_workspace_roots(workspace_roots);
config.config_layer_stack = ConfigLayerStack::default();
config.model_catalog = Some(ModelsResponse {
models: vec![model],
});
});
// This case intentionally exercises host-local workspace and extra roots.
let test = builder.build(&server).await?;
submit_text_turn(&test, "hello").await?;
test.codex
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
text: "hello".into(),
text_elements: Vec::new(),
}]))
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let permissions = permissions_texts(&req.single_request());
assert_eq!(permissions.len(), 1);
let mut writable_roots = [
AbsolutePathBuf::try_from(test.config.cwd.as_path().canonicalize()?)?,
writable_root,
];
writable_roots.sort();
let writable_roots = writable_roots
let mut roots = vec![AbsolutePathBuf::try_from(
test.config.cwd.as_path().canonicalize()?,
)?];
if root_count == 2 {
roots.push(writable_root);
}
roots.sort();
let roots = roots
.iter()
.map(|root| format!("`{}`", root.to_string_lossy()))
.collect::<Vec<_>>()
.join(", ");
let expected = format!(" The writable roots are {writable_roots}.");
let actual = permissions[0]
.lines()
.filter(|line| line.starts_with(" The writable root"))
let root_sentence = expected_root_sentence.replace("{roots}", &roots);
let request = req.single_request();
let expected = format!(
"{WORKSPACE_PERMISSIONS_PREFIX}{root_sentence}{denied_reads}\n</permissions instructions>"
);
assert_eq!(permissions_texts(&request), vec![expected.clone()]);
let annotated_permissions = request
.input()
.into_iter()
.flat_map(|item| {
item["content"]
.as_array()
.into_iter()
.flatten()
.enumerate()
.filter(|(_, content)| {
content["text"]
.as_str()
.is_some_and(|text| text.replace("\r\n", "\n") == expected)
})
.map(|(index, _)| {
(
item["role"].clone(),
item["internal_chat_message_metadata_passthrough"]["content_item_kinds"]
[index]
.clone(),
item["internal_chat_message_metadata_passthrough"]["content_item_metadata"]
[index]
.clone(),
)
})
.collect::<Vec<_>>()
})
.collect::<Vec<_>>();
assert_eq!(actual, vec![expected.as_str()]);
assert_eq!(
annotated_permissions,
vec![(
json!("developer"),
json!("permissions.instructions"),
json!({"provenance": {"type": "harness", "hook": false}}),
)]
);
Ok(())
}
pub(super) async fn catalog_writable_roots_model_change_requests() -> Result<Vec<ResponsesRequest>>
{
let server = start_mock_server().await;
let req1 = mount_sse_once(
&server,
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
)
.await;
let first_slug = "catalog-writable-roots-a";
let second_slug = "catalog-writable-roots-b";
let models = [
(
first_slug,
"The writable root for model A is {{ roots }}.",
"Wrong A plural: {{ roots }}.",
),
(
second_slug,
"The writable root for model B is {{ roots }}.",
"Wrong B plural: {{ roots }}.",
),
]
.into_iter()
.map(|(slug, single, plural)| {
model_with_permission_messages(
slug,
serde_json::from_value(json!({"writable_root": single, "writable_roots": plural}))
.expect("valid catalog permission messages"),
)
})
.collect();
let mut builder = test_codex()
.with_model(first_slug)
.with_config(move |config| {
config.base_instructions = Some("Help with the workspace.".to_string());
// Match the canonical writable root so snapshot normalization recognizes the cwd.
if !core_test_support::is_remote_test_environment() {
config.cwd = dunce::canonicalize(config.cwd.as_path())
.expect("test workspace should be canonicalizable")
.try_into()
.expect("canonical workspace path should be absolute");
}
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never);
config
.permissions
.set_permission_profile(PermissionProfile::workspace_write_with(
&[],
NetworkSandboxPolicy::Restricted,
/*exclude_tmpdir_env_var*/ true,
/*exclude_slash_tmp*/ true,
))
.expect("workspace-write profile should be allowed");
config.workspace_roots = vec![config.cwd.clone()];
config
.permissions
.set_workspace_roots(config.workspace_roots.clone());
config.config_layer_stack = ConfigLayerStack::default();
config.model_catalog = Some(ModelsResponse { models });
});
let test = builder.build_with_auto_env(&server).await?;
submit_text_turn(&test, "Inspect the writable workspace.").await?;
core_test_support::submit_thread_settings(
&test.codex,
ThreadSettingsOverrides {
model: Some(second_slug.to_string()),
..Default::default()
},
)
.await?;
let req2 = mount_sse_once(
&server,
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
)
.await;
submit_text_turn(&test, "Continue with the selected model.").await?;
let req3 = mount_sse_once(
&server,
sse(vec![ev_response_created("resp-3"), ev_completed("resp-3")]),
)
.await;
submit_text_turn(&test, "Continue without changing permissions.").await?;
let writable_root = test
.fs()
.canonicalize(&test.workspace_path_uri("")?, /*sandbox*/ None)
.await?
.inferred_native_path_string();
let requests = vec![
req1.single_request(),
req2.single_request(),
req3.single_request(),
];
let first = format!(
"{WORKSPACE_PERMISSIONS_PREFIX}The writable root for model A is `{writable_root}`.\n</permissions instructions>"
);
let second = format!(
"{WORKSPACE_PERMISSIONS_PREFIX}The writable root for model B is `{writable_root}`.\n</permissions instructions>"
);
assert_eq!(permissions_texts(&requests[0]), vec![first.clone()]);
assert_eq!(permissions_texts(&requests[1]), vec![first, second]);
assert_eq!(
permissions_texts(&requests[2]),
permissions_texts(&requests[1])
);
Ok(requests)
}
+14
View File
@@ -2674,6 +2674,20 @@ async fn model_catalog_refresh_preserves_tools_and_history() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn catalog_writable_roots_follow_selected_model() -> Result<()> {
skip_if_no_network!(Ok(()));
let requests =
super::permissions_messages::catalog_writable_roots_model_change_requests().await?;
let snapshot = context_snapshot::format_request_history_snapshot(
"Catalog writable-root wording follows the selected model, appending changed permissions once while preserving prior input.",
&requests,
&ContextSnapshotOptions::default().include_request_settings(),
);
insta::assert_snapshot!(snapshot);
Ok(())
}
fn compaction_chunk(
gate: Option<oneshot::Receiver<()>>,
events: Vec<serde_json::Value>,
@@ -143,6 +143,7 @@ async fn protected_model_settings_use_the_proposed_permissions(
workspace_write: None,
read_only: Some(PERMISSION_INSTRUCTIONS.to_string()),
approved_prefixes: None,
..PermissionMessages::default()
});
let test = test_codex()
.with_model(INITIAL_MODEL)
@@ -0,0 +1,87 @@
---
source: core/tests/suite/scenarios.rs
expression: snapshot
---
Scenario: Catalog writable-root wording follows the selected model, appending changed permissions once while preserving prior input.
## Window 1
Settings:
include: ["reasoning.encrypted_content"]
model: "catalog-writable-roots-a"
parallel_tool_calls: true
prompt_cache_key: "<PROMPT_CACHE_KEY 1>"
reasoning: {"summary":"auto"}
store: false
stream: true
tool_choice: "auto"
tools (6; hash=3201044466fc72a5):
- function/exec_command: Runs a command in a PTY, returning output or a session ID for ongoing interaction.; args=[cmd, justification, login, max_output_tokens, prefix_rule, sandbox_permissions, shell, tty, workdir, yield_time_ms]; hash=c04108e51a8548c5
- function/write_stdin: Writes characters to an existing unified exec session and returns recent output.; args=[chars, max_output_tokens, session_id, yield_time_ms]; hash=7fb62d8a3e2df4e2
- function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6
- function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[path]; hash=1908de9c0be7118a
- namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=c826e5b07fd2820f
- function/close_agent
- function/resume_agent
- function/send_input
- function/spawn_agent
- function/wait_agent
- web_search; hash=b24567c220fd75ba
-- request 1 (turn) --
00:message/developer:
Help with the workspace.
01:message/developer:
<permissions instructions>
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: T...ries requires approval. Network access is restricted. [hash=99963de5837f6638]
Approval policy is currently never. Do not provide the `sandbox_permissions` for any reason, commands will be rejected.
The writable root for model A is `<CWD>`.
</permissions instructions>
02:message/user:
<environment_context>
<cwd><CWD></cwd>
<shell><HOST_SHELL></shell>
<current_date><CURRENT_DATE></current_date>
<timezone><HOST_TIMEZONE></timezone>
<filesystem><workspace_roots><root><CWD></root></workspace_roots><permission_profile type="managed"><f...ntry></file_system></permission_profile></filesystem> [hash=8d38f7686fcb5f7d]
</environment_context>
03:message/user:
Inspect the writable workspace.
## Window 2 (after request 1)
Settings: relative to window 1
model: "catalog-writable-roots-b"
-- request 2 (turn) --
00:message/developer:
Help with the workspace.
01:message/developer:
<permissions instructions>
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: T...ries requires approval. Network access is restricted. [hash=99963de5837f6638]
Approval policy is currently never. Do not provide the `sandbox_permissions` for any reason, commands will be rejected.
The writable root for model A is `<CWD>`.
</permissions instructions>
02:message/user:
<environment_context>
<cwd><CWD></cwd>
<shell><HOST_SHELL></shell>
<current_date><CURRENT_DATE></current_date>
<timezone><HOST_TIMEZONE></timezone>
<filesystem><workspace_roots><root><CWD></root></workspace_roots><permission_profile type="managed"><f...ntry></file_system></permission_profile></filesystem> [hash=8d38f7686fcb5f7d]
</environment_context>
03:message/user:
Inspect the writable workspace.
04:message/developer:
<model_switch>
The user was previously using a different model. Please continue the conversation according to the following instructions:
Help with the workspace.
</model_switch>
05:message/developer:
<permissions instructions>
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: T...ries requires approval. Network access is restricted. [hash=99963de5837f6638]
Approval policy is currently never. Do not provide the `sandbox_permissions` for any reason, commands will be rejected.
The writable root for model B is `<CWD>`.
</permissions instructions>
06:message/user:
Continue with the selected model.
-- request 3 (turn) --
07:message/user:
Continue without changing permissions.
@@ -52,6 +52,7 @@ fn base_instruction_override_is_literal_and_preserves_catalog_messages() {
workspace_write: Some(String::new()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
};
let multi_agent = MultiAgentMessages {
role: Some(MultiAgentRoleMessages {
@@ -19,6 +19,8 @@ pub(crate) const WORKSPACE_WRITE_TEMPLATE: &str =
include_str!("../../templates/permissions/sandbox_mode/workspace_write.md");
pub(crate) const READ_ONLY_TEMPLATE: &str =
include_str!("../../templates/permissions/sandbox_mode/read_only.md");
pub(crate) const WRITABLE_ROOT_TEMPLATE: &str = " The writable root is {{ roots }}.";
pub(crate) const WRITABLE_ROOTS_TEMPLATE: &str = " The writable roots are {{ roots }}.";
/// Resolved approval-policy text; policy and tool selection remain with the consumer.
#[derive(Debug, Clone, Copy)]
@@ -59,6 +61,8 @@ pub(crate) struct ResolvedPermissionMessages<'a> {
pub(crate) danger_full_access: ResolvedMessage<'a>,
pub(crate) workspace_write: ResolvedMessage<'a>,
pub(crate) read_only: ResolvedMessage<'a>,
pub(crate) writable_root: ResolvedMessage<'a>,
pub(crate) writable_roots: ResolvedMessage<'a>,
}
impl<'a> ResolvedPermissionMessages<'a> {
@@ -80,6 +84,14 @@ impl<'a> ResolvedPermissionMessages<'a> {
messages.and_then(|m| m.read_only.as_deref()),
READ_ONLY_TEMPLATE,
),
writable_root: ResolvedMessage::new(
messages.and_then(|m| m.writable_root.as_deref()),
WRITABLE_ROOT_TEMPLATE,
),
writable_roots: ResolvedMessage::new(
messages.and_then(|m| m.writable_roots.as_deref()),
WRITABLE_ROOTS_TEMPLATE,
),
}
}
}
@@ -27,6 +27,9 @@ use codex_utils_template::Template;
use std::collections::HashSet;
use std::path::Path;
use std::sync::LazyLock;
use writable_roots::writable_roots_text;
mod writable_roots;
const REQUEST_PERMISSION_RULE: &str =
include_str!("../templates/permissions/approval_policy/on_request_rule_request_permission.md");
@@ -139,7 +142,9 @@ impl PermissionsInstructions {
context.request_permissions_tool_enabled,
),
);
if let Some(writable_roots) = writable_roots_text(&writable_roots) {
if let Some(writable_roots) =
writable_roots_text(&writable_roots, approval_context.permission_messages)
{
append_section(&mut text, &writable_roots);
}
if let Some(denied_reads) = denied_reads_text(&denied_read_paths, &denied_read_globs) {
@@ -377,22 +382,6 @@ fn sandbox_text(
}
}
fn writable_roots_text(writable_roots: &[String]) -> Option<String> {
if writable_roots.is_empty() {
return None;
}
let roots_list: Vec<String> = writable_roots
.iter()
.map(|root| format!("`{root}`"))
.collect();
Some(if roots_list.len() == 1 {
format!(" The writable root is {}.", roots_list[0])
} else {
format!(" The writable roots are {}.", roots_list.join(", "))
})
}
fn denied_reads_text(paths: &[String], globs: &[String]) -> Option<String> {
let mut entries = paths
.iter()
@@ -0,0 +1,72 @@
//! Renders bounded catalog wording around an already bounded list of writable roots.
//! Validate template text before inserting paths once; unusable overrides retain legacy bytes.
use super::MAX_PERMISSION_PATH_BYTES;
use super::PermissionsInstructions;
use crate::model_messages::permissions::ResolvedPermissionMessages;
use crate::model_messages::permissions::WRITABLE_ROOT_TEMPLATE;
use crate::model_messages::permissions::WRITABLE_ROOTS_TEMPLATE;
use codex_context_fragments::ContextualUserFragment;
const ROOTS_PLACEHOLDER: &str = "{{ roots }}";
// Replacing the 11-byte marker leaves at most 989 bytes of catalog wording,
// bounding it below 1K tokens even with one byte-fallback token per byte.
const MAX_ROOTS_TEMPLATE_BYTES: usize = 1_000;
const MAX_ROOTS_SENTENCE_BYTES: usize = MAX_PERMISSION_PATH_BYTES + MAX_ROOTS_TEMPLATE_BYTES;
// The caller bounds paths at whole-entry boundaries before reaching this renderer.
pub(super) fn writable_roots_text(
roots: &[String],
messages: ResolvedPermissionMessages<'_>,
) -> Option<String> {
let (selected, fallback) = match roots.len() {
0 => return None,
1 => (messages.writable_root, WRITABLE_ROOT_TEMPLATE),
_ => (messages.writable_roots, WRITABLE_ROOTS_TEMPLATE),
};
let roots_list = roots
.iter()
.map(|root| format!("`{root}`"))
.collect::<Vec<_>>()
.join(", ");
Some(
selected
.catalog_override()
.and_then(|template| render_roots_override(template, &roots_list))
.unwrap_or_else(|| fallback.replace(ROOTS_PLACEHOLDER, &roots_list)),
)
}
fn render_roots_override(template: &str, roots: &str) -> Option<String> {
if template.len() > MAX_ROOTS_TEMPLATE_BYTES
|| template
.chars()
.any(|c| c.is_control() || matches!(c, '\u{2028}' | '\u{2029}'))
{
return None;
}
let (before, after) = template.split_once(ROOTS_PLACEHOLDER)?;
// Reject duplicate, unknown and malformed placeholders, without parsing runtime paths.
if [before, after]
.iter()
.any(|part| part.chars().any(|c| matches!(c, '{' | '}')))
{
return None;
}
let (open, close) = PermissionsInstructions::type_markers();
if template.contains(open) || template.contains(close) {
return None;
}
let rendered_len = before
.len()
.checked_add(roots.len())?
.checked_add(after.len())?;
if rendered_len > MAX_ROOTS_SENTENCE_BYTES {
return None;
}
Some(format!("{before}{roots}{after}"))
}
#[cfg(test)]
#[path = "writable_roots_tests.rs"]
mod tests;
@@ -0,0 +1,80 @@
//! Checks bounded catalog wording without interpreting or truncating runtime paths.
use super::*;
use codex_protocol::openai_models::PermissionMessages;
use pretty_assertions::assert_eq;
#[test]
fn unusable_templates_fall_back_without_losing_roots() {
let roots = vec!["/a".to_string(), "/b".to_string()];
for template in [
"",
" ",
"No placeholder",
"{{roots}}",
"{{ roots }} {{ roots }}",
"{{ roots }} {{ other }}",
"{{{ roots }}}",
"X { {{ roots }}",
"X {{ roots }} }",
"X\n{{ roots }}",
"X\t{{ roots }}",
"X\0{{ roots }}",
"X\u{2028}{{ roots }}",
"X\u{2029}{{ roots }}",
"</permissions instructions>{{ roots }}",
"<permissions instructions>{{ roots }}",
] {
let messages = PermissionMessages {
writable_root: Some(template.into()),
writable_roots: Some(template.into()),
..PermissionMessages::default()
};
let resolved = ResolvedPermissionMessages::new(Some(&messages));
assert_eq!(
writable_roots_text(&roots, resolved),
Some(" The writable roots are `/a`, `/b`.".into())
);
assert_eq!(
writable_roots_text(&roots[..1], resolved),
Some(" The writable root is `/a`.".into())
);
assert_eq!(writable_roots_text(&[], resolved), None);
}
}
#[test]
fn substitution_preserves_whitespace_and_path_contents() {
assert_eq!(
render_roots_override(
" Paths = {{ roots }} ",
"`/a/{{ roots }}`, `/é\n{literal}`"
),
Some(" Paths = `/a/{{ roots }}`, `/é\n{literal}` ".into()),
);
}
#[test]
fn bounds_template_bytes_and_rendered_output_before_allocation() {
let exact = format!(
"{}{{{{ roots }}}}",
"x".repeat(1_000 - ROOTS_PLACEHOLDER.len())
);
assert!(render_roots_override(&exact, "`/a`").is_some());
for oversized in [exact + "x", format!("{}{{{{ roots }}}}", "é".repeat(495))] {
let messages = PermissionMessages {
writable_root: Some(oversized),
..PermissionMessages::default()
};
assert_eq!(
writable_roots_text(
&["/a".into()],
ResolvedPermissionMessages::new(Some(&messages))
),
Some(" The writable root is `/a`.".into()),
);
}
let roots = "x".repeat(MAX_ROOTS_SENTENCE_BYTES);
assert_eq!(render_roots_override("a{{ roots }}", &roots), None);
assert_eq!(render_roots_override("{{ roots }}", &roots), Some(roots));
}
@@ -194,6 +194,7 @@ fn catalog_permission_messages_select_sandbox_mode_and_render_network_access() {
workspace_write: Some("catalog workspace {{ network_access }}".to_string()),
read_only: Some("catalog read only {{ network_access }}".to_string()),
approved_prefixes: None,
..PermissionMessages::default()
};
for (mode, expected) in [
@@ -219,6 +220,7 @@ fn catalog_permission_text_equal_to_bundled_template_preserves_literal_whitespac
workspace_write: Some(WORKSPACE_WRITE_TEMPLATE.to_string()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
};
let catalog_text = sandbox_text(
SandboxMode::WorkspaceWrite,
@@ -251,6 +253,7 @@ fn missing_catalog_permission_message_uses_legacy_sandbox_text() {
workspace_write: None,
read_only: Some("unused".to_string()),
approved_prefixes: None,
..PermissionMessages::default()
};
assert_eq!(
@@ -271,6 +274,7 @@ fn invalid_catalog_permission_message_is_preserved_verbatim() {
workspace_write: Some(workspace_write.to_string()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
};
assert_eq!(
sandbox_text(
@@ -291,6 +295,7 @@ fn catalog_permission_message_renders_network_access_and_preserves_other_placeho
workspace_write: Some(source.to_string()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
};
assert_eq!(
@@ -310,6 +315,7 @@ fn empty_catalog_permission_message_preserves_non_sandbox_sections() {
workspace_write: Some(String::new()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
};
let writable_root = "/tmp/repo".to_string();
let text = PermissionsInstructions::from_resolved(
@@ -734,6 +740,7 @@ fn preserves_supplied_path_spellings_and_order() {
workspace_write: Some(String::new()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
};
let instructions = PermissionsInstructions::from_resolved(
@@ -798,3 +805,72 @@ fn bounds_permission_paths_at_whole_entry_boundaries() {
let expected = render(&[writable], &[denied], &["/short/**".into()]);
assert_eq!(text, format!("{expected}{OMITTED_PERMISSION_PATHS}\n"));
}
#[test]
fn catalog_writable_roots_preserve_fragment_facts() {
let messages = PermissionMessages {
writable_root: Some("Catalog root: {{ roots }}.".into()),
writable_roots: Some("Catalog roots: {{ roots }}.".into()),
..PermissionMessages::default()
};
let catalog_context = ApprovalPromptContext {
permission_messages: ResolvedPermissionMessages::new(Some(&messages)),
..user_approval_context()
};
let denied = vec!["/private".to_string()];
for roots in [
vec![],
vec!["/a/{{ roots }}".to_string()],
vec![r"C:\work\z".to_string(), "/a/é\n{literal}".to_string()],
] {
let context = PermissionsRenderContext {
writable_roots: &roots,
denied_read_paths: &denied,
..WORKSPACE_CONTEXT
};
let original = PermissionsInstructions::from_resolved(context, user_approval_context());
let expected = PermissionsInstructions {
text: original
.text
.replace(" The writable root is ", "Catalog root: ")
.replace(" The writable roots are ", "Catalog roots: "),
};
assert_eq!(
PermissionsInstructions::from_resolved(context, catalog_context,).render_fragment(),
expected.render_fragment(),
);
}
}
#[test]
fn catalog_writable_roots_select_displayed_count_after_truncation() {
let messages = PermissionMessages {
writable_root: Some("Catalog root: {{ roots }}.".into()),
writable_roots: Some("Catalog roots: {{ roots }}.".into()),
..PermissionMessages::default()
};
let roots = vec!["x".repeat(MAX_PERMISSION_PATH_BYTES), "/short".to_string()];
let context = PermissionsRenderContext {
writable_roots: &roots,
..WORKSPACE_CONTEXT
};
let original = PermissionsInstructions::from_resolved(context, user_approval_context());
let expected = PermissionsInstructions {
text: original
.text
.replace(" The writable root is ", "Catalog root: "),
};
let actual = PermissionsInstructions::from_resolved(
context,
ApprovalPromptContext {
permission_messages: ResolvedPermissionMessages::new(Some(&messages)),
..user_approval_context()
},
);
assert_eq!(actual.render_fragment(), expected.render_fragment());
assert!(
actual
.text
.ends_with(&format!("{OMITTED_PERMISSION_PATHS}\n"))
);
}
+33 -1
View File
@@ -779,7 +779,7 @@ pub struct AutoReviewMessages {
pub timeout_instructions: Option<String>,
}
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
#[derive(Debug, Default, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
pub struct PermissionMessages {
pub danger_full_access: Option<String>,
pub workspace_write: Option<String>,
@@ -788,6 +788,14 @@ pub struct PermissionMessages {
/// an empty string suppresses the approved-prefix section.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub approved_prefixes: Option<String>,
/// Single writable-root sentence with exactly one `{{ roots }}` placeholder.
/// Missing, null, empty, invalid, or oversized templates use the bundled sentence.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub writable_root: Option<String>,
/// Multiple writable-roots sentence with exactly one `{{ roots }}` placeholder.
/// Missing, null, empty, invalid, or oversized templates use the bundled sentence.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub writable_roots: Option<String>,
}
#[derive(Debug, Default, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
@@ -1315,10 +1323,33 @@ mod tests {
workspace_write: Some(String::new()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
})
);
}
#[test]
fn writable_root_messages_preserve_sparse_wire_values() {
for input in [
serde_json::json!({}),
serde_json::json!({"writable_root": null, "writable_roots": null}),
serde_json::json!({"writable_root": "", "writable_roots": "Roots: {{ roots }}."}),
] {
let messages: PermissionMessages = serde_json::from_value(input.clone()).unwrap();
let mut expected = serde_json::json!({
"danger_full_access": null,
"workspace_write": null,
"read_only": null,
});
for key in ["writable_root", "writable_roots"] {
if let Some(value) = input.get(key).filter(|value| !value.is_null()) {
expected[key] = value.clone();
}
}
assert_eq!(serde_json::to_value(messages).unwrap(), expected);
}
}
#[test]
fn multi_agent_messages_preserve_missing_and_empty_values() {
let messages: ModelMessages = from_str(
@@ -1560,6 +1591,7 @@ mod tests {
workspace_write: Some("workspace".to_string()),
read_only: None,
approved_prefixes: None,
..PermissionMessages::default()
}),
multi_agent: None,
token_budget: None,