chore: rename unless-allow-listed to untrusted (#1378)
For the `approval_policy` config option, renames `unless-allow-listed` to `untrusted`. In general, when it comes to exec'ing commands, I think "trusted" is a more accurate term than "safe." Also drops the `AskForApproval::AutoEdit` variant, as we were not really making use of it, anyway. Fixes https://github.com/openai/codex/issues/1250. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1378). * #1379 * __->__ #1378
This commit is contained in:
@@ -8,16 +8,16 @@ use codex_core::protocol::AskForApproval;
|
|||||||
#[derive(Clone, Copy, Debug, ValueEnum)]
|
#[derive(Clone, Copy, Debug, ValueEnum)]
|
||||||
#[value(rename_all = "kebab-case")]
|
#[value(rename_all = "kebab-case")]
|
||||||
pub enum ApprovalModeCliArg {
|
pub enum ApprovalModeCliArg {
|
||||||
|
/// Only run "trusted" commands (e.g. ls, cat, sed) without asking for user
|
||||||
|
/// approval. Will escalate to the user if the model proposes a command that
|
||||||
|
/// is not in the "trusted" set.
|
||||||
|
Untrusted,
|
||||||
|
|
||||||
/// Run all commands without asking for user approval.
|
/// Run all commands without asking for user approval.
|
||||||
/// Only asks for approval if a command fails to execute, in which case it
|
/// Only asks for approval if a command fails to execute, in which case it
|
||||||
/// will escalate to the user to ask for un-sandboxed execution.
|
/// will escalate to the user to ask for un-sandboxed execution.
|
||||||
OnFailure,
|
OnFailure,
|
||||||
|
|
||||||
/// Only run "known safe" commands (e.g. ls, cat, sed) without
|
|
||||||
/// asking for user approval. Will escalate to the user if the model
|
|
||||||
/// proposes a command that is not allow-listed.
|
|
||||||
UnlessAllowListed,
|
|
||||||
|
|
||||||
/// Never ask for user approval
|
/// Never ask for user approval
|
||||||
/// Execution failures are immediately returned to the model.
|
/// Execution failures are immediately returned to the model.
|
||||||
Never,
|
Never,
|
||||||
@@ -26,8 +26,8 @@ pub enum ApprovalModeCliArg {
|
|||||||
impl From<ApprovalModeCliArg> for AskForApproval {
|
impl From<ApprovalModeCliArg> for AskForApproval {
|
||||||
fn from(value: ApprovalModeCliArg) -> Self {
|
fn from(value: ApprovalModeCliArg) -> Self {
|
||||||
match value {
|
match value {
|
||||||
|
ApprovalModeCliArg::Untrusted => AskForApproval::UnlessAllowListed,
|
||||||
ApprovalModeCliArg::OnFailure => AskForApproval::OnFailure,
|
ApprovalModeCliArg::OnFailure => AskForApproval::OnFailure,
|
||||||
ApprovalModeCliArg::UnlessAllowListed => AskForApproval::UnlessAllowListed,
|
|
||||||
ApprovalModeCliArg::Never => AskForApproval::Never,
|
ApprovalModeCliArg::Never => AskForApproval::Never,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -80,8 +80,13 @@ wire_api = "chat"
|
|||||||
Determines when the user should be prompted to approve whether Codex can execute a command:
|
Determines when the user should be prompted to approve whether Codex can execute a command:
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
# This is analogous to --suggest in the TypeScript Codex CLI
|
# Codex has hardcoded logic that defines a set of "trusted" commands.
|
||||||
approval_policy = "unless-allow-listed"
|
# Setting the approval_policy to `untrusted` means that Codex will prompt the
|
||||||
|
# user before running a command not in the "trusted" set.
|
||||||
|
#
|
||||||
|
# See https://github.com/openai/codex/issues/1260 for the plan to enable
|
||||||
|
# end-users to define their own trusted commands.
|
||||||
|
approval_policy = "untrusted"
|
||||||
```
|
```
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
|
|||||||
@@ -586,7 +586,7 @@ writable_roots = [
|
|||||||
fn create_test_fixture() -> std::io::Result<PrecedenceTestFixture> {
|
fn create_test_fixture() -> std::io::Result<PrecedenceTestFixture> {
|
||||||
let toml = r#"
|
let toml = r#"
|
||||||
model = "o3"
|
model = "o3"
|
||||||
approval_policy = "unless-allow-listed"
|
approval_policy = "untrusted"
|
||||||
disable_response_storage = false
|
disable_response_storage = false
|
||||||
|
|
||||||
# Can be used to determine which profile to use if not specified by
|
# Can be used to determine which profile to use if not specified by
|
||||||
|
|||||||
@@ -110,22 +110,18 @@ pub enum Op {
|
|||||||
GetHistoryEntryRequest { offset: usize, log_id: u64 },
|
GetHistoryEntryRequest { offset: usize, log_id: u64 },
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Determines how liberally commands are auto‑approved by the system.
|
/// Determines the conditions under which the user is consulted to approve
|
||||||
|
/// running the command proposed by Codex.
|
||||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)]
|
||||||
#[serde(rename_all = "kebab-case")]
|
#[serde(rename_all = "kebab-case")]
|
||||||
pub enum AskForApproval {
|
pub enum AskForApproval {
|
||||||
/// Under this policy, only “known safe” commands—as determined by
|
/// Under this policy, only "known safe" commands—as determined by
|
||||||
/// `is_safe_command()`—that **only read files** are auto‑approved.
|
/// `is_safe_command()`—that **only read files** are auto‑approved.
|
||||||
/// Everything else will ask the user to approve.
|
/// Everything else will ask the user to approve.
|
||||||
#[default]
|
#[default]
|
||||||
|
#[serde(rename = "untrusted")]
|
||||||
UnlessAllowListed,
|
UnlessAllowListed,
|
||||||
|
|
||||||
/// In addition to everything allowed by **`Suggest`**, commands that
|
|
||||||
/// *write* to files **within the user’s approved list of writable paths**
|
|
||||||
/// are also auto‑approved.
|
|
||||||
/// TODO(ragona): fix
|
|
||||||
AutoEdit,
|
|
||||||
|
|
||||||
/// *All* commands are auto‑approved, but they are expected to run inside a
|
/// *All* commands are auto‑approved, but they are expected to run inside a
|
||||||
/// sandbox where network access is disabled and writes are confined to a
|
/// sandbox where network access is disabled and writes are confined to a
|
||||||
/// specific set of paths. If the command fails, it will be escalated to
|
/// specific set of paths. If the command fails, it will be escalated to
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ pub fn assess_patch_safety(
|
|||||||
}
|
}
|
||||||
|
|
||||||
match policy {
|
match policy {
|
||||||
AskForApproval::OnFailure | AskForApproval::AutoEdit | AskForApproval::Never => {
|
AskForApproval::OnFailure | AskForApproval::Never => {
|
||||||
// Continue to see if this can be auto-approved.
|
// Continue to see if this can be auto-approved.
|
||||||
}
|
}
|
||||||
// TODO(ragona): I'm not sure this is actually correct? I believe in this case
|
// TODO(ragona): I'm not sure this is actually correct? I believe in this case
|
||||||
|
|||||||
@@ -47,8 +47,7 @@ pub(crate) struct CodexToolCallParam {
|
|||||||
#[derive(Debug, Clone, Deserialize, JsonSchema)]
|
#[derive(Debug, Clone, Deserialize, JsonSchema)]
|
||||||
#[serde(rename_all = "kebab-case")]
|
#[serde(rename_all = "kebab-case")]
|
||||||
pub(crate) enum CodexToolCallApprovalPolicy {
|
pub(crate) enum CodexToolCallApprovalPolicy {
|
||||||
AutoEdit,
|
Untrusted,
|
||||||
UnlessAllowListed,
|
|
||||||
OnFailure,
|
OnFailure,
|
||||||
Never,
|
Never,
|
||||||
}
|
}
|
||||||
@@ -56,8 +55,7 @@ pub(crate) enum CodexToolCallApprovalPolicy {
|
|||||||
impl From<CodexToolCallApprovalPolicy> for AskForApproval {
|
impl From<CodexToolCallApprovalPolicy> for AskForApproval {
|
||||||
fn from(value: CodexToolCallApprovalPolicy) -> Self {
|
fn from(value: CodexToolCallApprovalPolicy) -> Self {
|
||||||
match value {
|
match value {
|
||||||
CodexToolCallApprovalPolicy::AutoEdit => AskForApproval::AutoEdit,
|
CodexToolCallApprovalPolicy::Untrusted => AskForApproval::UnlessAllowListed,
|
||||||
CodexToolCallApprovalPolicy::UnlessAllowListed => AskForApproval::UnlessAllowListed,
|
|
||||||
CodexToolCallApprovalPolicy::OnFailure => AskForApproval::OnFailure,
|
CodexToolCallApprovalPolicy::OnFailure => AskForApproval::OnFailure,
|
||||||
CodexToolCallApprovalPolicy::Never => AskForApproval::Never,
|
CodexToolCallApprovalPolicy::Never => AskForApproval::Never,
|
||||||
}
|
}
|
||||||
@@ -164,8 +162,7 @@ mod tests {
|
|||||||
"approval-policy": {
|
"approval-policy": {
|
||||||
"description": "Execution approval policy expressed as the kebab-case variant name (`unless-allow-listed`, `auto-edit`, `on-failure`, `never`).",
|
"description": "Execution approval policy expressed as the kebab-case variant name (`unless-allow-listed`, `auto-edit`, `on-failure`, `never`).",
|
||||||
"enum": [
|
"enum": [
|
||||||
"auto-edit",
|
"untrusted",
|
||||||
"unless-allow-listed",
|
|
||||||
"on-failure",
|
"on-failure",
|
||||||
"never"
|
"never"
|
||||||
],
|
],
|
||||||
|
|||||||
Reference in New Issue
Block a user