mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Merge 6d40797232 into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -78,6 +78,7 @@ jobs:
|
||||
30. automations - Issues involving scheduled automation tasks or heartbeats.
|
||||
31. pets - Issues involving pets avatars and animations.
|
||||
32. agent - Fallback only for core agent loop or agent-related issues that do not fit app-server, connectivity, subagent, session, config, plan, computer-use, browser, memory, imagen, remote, performance, automations, or pets.
|
||||
33. aws-bedrock - Issues involving the Amazon Bedrock model provider or Bedrock Mantle. Do not use for unrelated AWS services or generic AWS mentions.
|
||||
|
||||
Issue number: ${{ github.event.issue.number }}
|
||||
|
||||
|
||||
@@ -43,7 +43,6 @@ use tracing::debug;
|
||||
use tracing::error;
|
||||
use tracing::info;
|
||||
use tracing::instrument;
|
||||
use tracing::trace;
|
||||
use tungstenite::extensions::ExtensionsConfig;
|
||||
use tungstenite::extensions::compression::deflate::DeflateConfig;
|
||||
use tungstenite::protocol::WebSocketConfig;
|
||||
@@ -782,8 +781,6 @@ async fn send_websocket_request(
|
||||
telemetry: Option<&Arc<dyn WebsocketTelemetry>>,
|
||||
connection_reused: bool,
|
||||
) -> Result<(), ApiError> {
|
||||
trace!("websocket request: {request_text}");
|
||||
|
||||
let request_start = Instant::now();
|
||||
let result = tokio::time::timeout(
|
||||
idle_timeout,
|
||||
|
||||
@@ -7,6 +7,7 @@ use std::path::Path;
|
||||
use crate::HooksToml;
|
||||
use crate::permissions_toml::PermissionsToml;
|
||||
use crate::profile_toml::ConfigProfile;
|
||||
use crate::requirements_exec_policy::RequirementsExecPolicyToml;
|
||||
use crate::types::AnalyticsConfigToml;
|
||||
use crate::types::ApprovalsReviewer;
|
||||
use crate::types::AppsConfigToml;
|
||||
@@ -182,6 +183,10 @@ pub struct ConfigToml {
|
||||
#[serde(default)]
|
||||
pub auto_review: Option<AutoReviewToml>,
|
||||
|
||||
/// Optional command execution policy rules for this config layer.
|
||||
#[serde(default)]
|
||||
pub rules: Option<RequirementsExecPolicyToml>,
|
||||
|
||||
#[serde(default)]
|
||||
pub shell_environment_policy: ShellEnvironmentPolicyToml,
|
||||
|
||||
|
||||
@@ -5,7 +5,9 @@ use codex_execpolicy::rule::PatternToken;
|
||||
use codex_execpolicy::rule::PrefixPattern;
|
||||
use codex_execpolicy::rule::PrefixRule;
|
||||
use multimap::MultiMap;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::sync::Arc;
|
||||
use thiserror::Error;
|
||||
|
||||
@@ -46,7 +48,7 @@ fn policy_fingerprint(policy: &Policy) -> Vec<String> {
|
||||
}
|
||||
|
||||
/// TOML representation of `[rules]` within `requirements.toml`.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
|
||||
pub struct RequirementsExecPolicyToml {
|
||||
pub prefix_rules: Vec<RequirementsExecPolicyPrefixRuleToml>,
|
||||
}
|
||||
@@ -54,11 +56,12 @@ pub struct RequirementsExecPolicyToml {
|
||||
/// A TOML representation of the `prefix_rule(...)` Starlark builtin.
|
||||
///
|
||||
/// This mirrors the builtin defined in `execpolicy/src/parser.rs`.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
|
||||
pub struct RequirementsExecPolicyPrefixRuleToml {
|
||||
pub pattern: Vec<RequirementsExecPolicyPatternTokenToml>,
|
||||
pub decision: Option<RequirementsExecPolicyDecisionToml>,
|
||||
pub justification: Option<String>,
|
||||
pub permissions: Option<String>,
|
||||
}
|
||||
|
||||
/// TOML-friendly representation of a pattern token.
|
||||
@@ -66,13 +69,13 @@ pub struct RequirementsExecPolicyPrefixRuleToml {
|
||||
/// Starlark supports either a string token or a list of alternative tokens at
|
||||
/// each position, but TOML arrays cannot mix strings and arrays. Using an
|
||||
/// array of tables sidesteps that restriction.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
|
||||
pub struct RequirementsExecPolicyPatternTokenToml {
|
||||
pub token: Option<String>,
|
||||
pub any_of: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)]
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
|
||||
#[serde(rename_all = "kebab-case")]
|
||||
pub enum RequirementsExecPolicyDecisionToml {
|
||||
Allow,
|
||||
@@ -110,6 +113,9 @@ pub enum RequirementsExecPolicyParseError {
|
||||
#[error("rules prefix_rule at index {rule_index} has an empty justification")]
|
||||
EmptyJustification { rule_index: usize },
|
||||
|
||||
#[error("rules prefix_rule at index {rule_index} has an empty permissions value")]
|
||||
EmptyPermissions { rule_index: usize },
|
||||
|
||||
#[error("rules prefix_rule at index {rule_index} is missing a decision")]
|
||||
MissingDecision { rule_index: usize },
|
||||
|
||||
@@ -123,6 +129,22 @@ impl RequirementsExecPolicyToml {
|
||||
/// Convert requirements TOML rules into the internal `.rules`
|
||||
/// representation used by `codex-execpolicy`.
|
||||
pub fn to_policy(&self) -> Result<Policy, RequirementsExecPolicyParseError> {
|
||||
self.to_policy_with_allow_rules(/*allow_allow_decision*/ false)
|
||||
}
|
||||
|
||||
/// Convert config TOML rules into the internal `.rules` representation.
|
||||
///
|
||||
/// Unlike `requirements.toml`, regular config layers may contain allow
|
||||
/// rules because they are user/project policy rather than managed
|
||||
/// restrictions.
|
||||
pub fn to_config_policy(&self) -> Result<Policy, RequirementsExecPolicyParseError> {
|
||||
self.to_policy_with_allow_rules(/*allow_allow_decision*/ true)
|
||||
}
|
||||
|
||||
fn to_policy_with_allow_rules(
|
||||
&self,
|
||||
allow_allow_decision: bool,
|
||||
) -> Result<Policy, RequirementsExecPolicyParseError> {
|
||||
if self.prefix_rules.is_empty() {
|
||||
return Err(RequirementsExecPolicyParseError::EmptyPrefixRules);
|
||||
}
|
||||
@@ -135,6 +157,11 @@ impl RequirementsExecPolicyToml {
|
||||
{
|
||||
return Err(RequirementsExecPolicyParseError::EmptyJustification { rule_index });
|
||||
}
|
||||
if let Some(permissions) = &rule.permissions
|
||||
&& permissions.trim().is_empty()
|
||||
{
|
||||
return Err(RequirementsExecPolicyParseError::EmptyPermissions { rule_index });
|
||||
}
|
||||
|
||||
if rule.pattern.is_empty() {
|
||||
return Err(RequirementsExecPolicyParseError::EmptyPattern { rule_index });
|
||||
@@ -148,7 +175,7 @@ impl RequirementsExecPolicyToml {
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
|
||||
let decision = match rule.decision {
|
||||
Some(RequirementsExecPolicyDecisionToml::Allow) => {
|
||||
Some(RequirementsExecPolicyDecisionToml::Allow) if !allow_allow_decision => {
|
||||
return Err(RequirementsExecPolicyParseError::AllowDecisionNotAllowed {
|
||||
rule_index,
|
||||
});
|
||||
@@ -159,6 +186,7 @@ impl RequirementsExecPolicyToml {
|
||||
}
|
||||
};
|
||||
let justification = rule.justification.clone();
|
||||
let permissions = rule.permissions.clone();
|
||||
|
||||
let (first_token, remaining_tokens) = pattern_tokens
|
||||
.split_first()
|
||||
@@ -174,6 +202,7 @@ impl RequirementsExecPolicyToml {
|
||||
},
|
||||
decision,
|
||||
justification: justification.clone(),
|
||||
permissions: permissions.clone(),
|
||||
});
|
||||
rules_by_program.insert(head.clone(), rule);
|
||||
}
|
||||
|
||||
@@ -2711,6 +2711,68 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"RequirementsExecPolicyDecisionToml": {
|
||||
"enum": [
|
||||
"allow",
|
||||
"prompt",
|
||||
"forbidden"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
"RequirementsExecPolicyPatternTokenToml": {
|
||||
"description": "TOML-friendly representation of a pattern token.\n\nStarlark supports either a string token or a list of alternative tokens at each position, but TOML arrays cannot mix strings and arrays. Using an array of tables sidesteps that restriction.",
|
||||
"properties": {
|
||||
"any_of": {
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"token": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"RequirementsExecPolicyPrefixRuleToml": {
|
||||
"description": "A TOML representation of the `prefix_rule(...)` Starlark builtin.\n\nThis mirrors the builtin defined in `execpolicy/src/parser.rs`.",
|
||||
"properties": {
|
||||
"decision": {
|
||||
"$ref": "#/definitions/RequirementsExecPolicyDecisionToml"
|
||||
},
|
||||
"justification": {
|
||||
"type": "string"
|
||||
},
|
||||
"pattern": {
|
||||
"items": {
|
||||
"$ref": "#/definitions/RequirementsExecPolicyPatternTokenToml"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"permissions": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"pattern"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"RequirementsExecPolicyToml": {
|
||||
"description": "TOML representation of `[rules]` within `requirements.toml`.",
|
||||
"properties": {
|
||||
"prefix_rules": {
|
||||
"items": {
|
||||
"$ref": "#/definitions/RequirementsExecPolicyPrefixRuleToml"
|
||||
},
|
||||
"type": "array"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"prefix_rules"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
"RolloutBudgetConfigToml": {
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
@@ -5416,6 +5478,15 @@
|
||||
"description": "Review model override used by the `/review` feature.",
|
||||
"type": "string"
|
||||
},
|
||||
"rules": {
|
||||
"allOf": [
|
||||
{
|
||||
"$ref": "#/definitions/RequirementsExecPolicyToml"
|
||||
}
|
||||
],
|
||||
"default": null,
|
||||
"description": "Optional command execution policy rules for this config layer."
|
||||
},
|
||||
"sandbox_mode": {
|
||||
"allOf": [
|
||||
{
|
||||
|
||||
@@ -3515,6 +3515,7 @@ mod requirements_exec_policy_tests {
|
||||
use codex_config::RequirementsExecPolicyToml;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::MatchOptions;
|
||||
use codex_execpolicy::RuleMatch;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -3572,6 +3573,7 @@ prefix_rules = [
|
||||
}],
|
||||
decision: Some(RequirementsExecPolicyDecisionToml::Forbidden),
|
||||
justification: None,
|
||||
permissions: None,
|
||||
}],
|
||||
}
|
||||
);
|
||||
@@ -3601,6 +3603,7 @@ prefix_rules = [
|
||||
}],
|
||||
decision: Some(RequirementsExecPolicyDecisionToml::Forbidden),
|
||||
justification: None,
|
||||
permissions: None,
|
||||
},
|
||||
RequirementsExecPolicyPrefixRuleToml {
|
||||
pattern: vec![
|
||||
@@ -3615,6 +3618,7 @@ prefix_rules = [
|
||||
],
|
||||
decision: Some(RequirementsExecPolicyDecisionToml::Prompt),
|
||||
justification: Some("review changes before push or commit".to_string()),
|
||||
permissions: None,
|
||||
},
|
||||
],
|
||||
}
|
||||
@@ -3650,6 +3654,58 @@ prefix_rules = [
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn converts_rules_toml_permissions_scope() -> anyhow::Result<()> {
|
||||
let toml_str = r#"
|
||||
prefix_rules = [
|
||||
{ pattern = [{ token = "git" }, { token = "status" }], decision = "prompt", permissions = "strict" },
|
||||
]
|
||||
"#;
|
||||
|
||||
let parsed: RequirementsExecPolicyToml = from_str(toml_str)?;
|
||||
let policy = parsed.to_policy()?;
|
||||
let command = tokens(&["git", "status"]);
|
||||
|
||||
assert_eq!(
|
||||
policy.check_with_options(
|
||||
&command,
|
||||
&|_| Decision::Allow,
|
||||
&MatchOptions {
|
||||
active_permission_profile: Some("strict".to_string()),
|
||||
..MatchOptions::default()
|
||||
}
|
||||
),
|
||||
Evaluation {
|
||||
decision: Decision::Prompt,
|
||||
matched_rules: vec![RuleMatch::PrefixRuleMatch {
|
||||
matched_prefix: command.clone(),
|
||||
decision: Decision::Prompt,
|
||||
resolved_program: None,
|
||||
justification: None,
|
||||
}],
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
policy.check_with_options(
|
||||
&command,
|
||||
&|_| Decision::Allow,
|
||||
&MatchOptions {
|
||||
active_permission_profile: Some("relaxed".to_string()),
|
||||
..MatchOptions::default()
|
||||
}
|
||||
),
|
||||
Evaluation {
|
||||
decision: Decision::Allow,
|
||||
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
|
||||
command,
|
||||
decision: Decision::Allow,
|
||||
}],
|
||||
}
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn head_any_of_expands_into_multiple_program_rules() -> anyhow::Result<()> {
|
||||
let toml_str = r#"
|
||||
|
||||
@@ -5,9 +5,12 @@ use std::sync::Arc;
|
||||
|
||||
use arc_swap::ArcSwap;
|
||||
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::RequirementsExecPolicyParseError;
|
||||
use codex_execpolicy::AmendError;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Error as ExecPolicyRuleError;
|
||||
@@ -135,7 +138,9 @@ struct ExecPolicyCommands {
|
||||
}
|
||||
|
||||
pub(crate) fn child_uses_parent_exec_policy(parent_config: &Config, child_config: &Config) -> bool {
|
||||
fn exec_policy_config_folders(config: &Config) -> Vec<AbsolutePathBuf> {
|
||||
fn exec_policy_layer_inputs(
|
||||
config: &Config,
|
||||
) -> Vec<(Option<AbsolutePathBuf>, Option<toml::Value>)> {
|
||||
config
|
||||
.config_layer_stack
|
||||
.get_layers(
|
||||
@@ -143,11 +148,18 @@ pub(crate) fn child_uses_parent_exec_policy(parent_config: &Config, child_config
|
||||
/*include_disabled*/ false,
|
||||
)
|
||||
.into_iter()
|
||||
.filter_map(codex_config::ConfigLayerEntry::config_folder)
|
||||
.filter(|layer| {
|
||||
!should_ignore_layer_exec_policy_rules(&config.config_layer_stack, layer)
|
||||
})
|
||||
.filter_map(|layer| {
|
||||
let config_folder = layer.config_folder();
|
||||
let rules = layer.config.get("rules").cloned();
|
||||
(config_folder.is_some() || rules.is_some()).then_some((config_folder, rules))
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
exec_policy_config_folders(parent_config) == exec_policy_config_folders(child_config)
|
||||
exec_policy_layer_inputs(parent_config) == exec_policy_layer_inputs(child_config)
|
||||
&& parent_config
|
||||
.config_layer_stack
|
||||
.ignore_user_and_project_exec_policy_rules()
|
||||
@@ -214,6 +226,18 @@ pub enum ExecPolicyError {
|
||||
path: String,
|
||||
source: codex_execpolicy::Error,
|
||||
},
|
||||
|
||||
#[error("failed to parse rules in {path}: {source}")]
|
||||
ParseConfigRules {
|
||||
path: String,
|
||||
source: toml::de::Error,
|
||||
},
|
||||
|
||||
#[error("failed to load rules in {path}: {source}")]
|
||||
InvalidConfigRules {
|
||||
path: String,
|
||||
source: RequirementsExecPolicyParseError,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
@@ -240,6 +264,7 @@ pub(crate) struct ExecApprovalRequest<'a> {
|
||||
pub(crate) command: &'a [String],
|
||||
pub(crate) approval_policy: AskForApproval,
|
||||
pub(crate) permission_profile: PermissionProfile,
|
||||
pub(crate) active_permission_profile: Option<String>,
|
||||
pub(crate) windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub(crate) sandbox_permissions: SandboxPermissions,
|
||||
pub(crate) prefix_rule: Option<Vec<String>>,
|
||||
@@ -274,6 +299,7 @@ impl ExecPolicyManager {
|
||||
command,
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
windows_sandbox_level,
|
||||
sandbox_permissions,
|
||||
prefix_rule,
|
||||
@@ -303,6 +329,7 @@ impl ExecPolicyManager {
|
||||
};
|
||||
let match_options = MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
active_permission_profile,
|
||||
};
|
||||
let evaluation = exec_policy.check_multiple_with_options(
|
||||
commands.iter(),
|
||||
@@ -403,6 +430,7 @@ impl ExecPolicyManager {
|
||||
let current_policy = self.current();
|
||||
let match_options = MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
active_permission_profile: None,
|
||||
};
|
||||
let existing_evaluation = current_policy.check_multiple_with_options(
|
||||
[&amendment.command],
|
||||
@@ -550,6 +578,12 @@ pub fn format_exec_policy_error_with_source(error: &ExecPolicyError) -> String {
|
||||
None => format!("{path}: {message}"),
|
||||
}
|
||||
}
|
||||
ExecPolicyError::ParseConfigRules { path, source } => {
|
||||
format!("{path}: failed to parse [rules]: {source}")
|
||||
}
|
||||
ExecPolicyError::InvalidConfigRules { path, source } => {
|
||||
format!("{path}: failed to load [rules]: {source}")
|
||||
}
|
||||
_ => error.to_string(),
|
||||
}
|
||||
}
|
||||
@@ -559,34 +593,86 @@ async fn load_exec_policy_with_warning(
|
||||
) -> Result<(Policy, Option<ExecPolicyError>), ExecPolicyError> {
|
||||
match load_exec_policy(config_stack).await {
|
||||
Ok(policy) => Ok((policy, None)),
|
||||
Err(err @ ExecPolicyError::ParsePolicy { .. }) => Ok((Policy::empty(), Some(err))),
|
||||
Err(
|
||||
err @ (ExecPolicyError::ParsePolicy { .. }
|
||||
| ExecPolicyError::ParseConfigRules { .. }
|
||||
| ExecPolicyError::InvalidConfigRules { .. }),
|
||||
) => Ok((Policy::empty(), Some(err))),
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
fn should_ignore_layer_exec_policy_rules(
|
||||
config_stack: &ConfigLayerStack,
|
||||
layer: &ConfigLayerEntry,
|
||||
) -> bool {
|
||||
config_stack.ignore_user_and_project_exec_policy_rules()
|
||||
&& matches!(
|
||||
layer.name,
|
||||
ConfigLayerSource::User { .. } | ConfigLayerSource::Project { .. }
|
||||
)
|
||||
}
|
||||
|
||||
fn config_layer_rules_path(layer: &ConfigLayerEntry) -> String {
|
||||
codex_config::format_config_layer_source(&layer.name, CONFIG_TOML_FILE)
|
||||
}
|
||||
|
||||
fn config_layer_rules_policy(layer: &ConfigLayerEntry) -> Result<Option<Policy>, ExecPolicyError> {
|
||||
let Some(rules) = layer.config.get("rules") else {
|
||||
return Ok(None);
|
||||
};
|
||||
let path = config_layer_rules_path(layer);
|
||||
let rules: codex_config::RequirementsExecPolicyToml =
|
||||
rules
|
||||
.clone()
|
||||
.try_into()
|
||||
.map_err(|source| ExecPolicyError::ParseConfigRules {
|
||||
path: path.clone(),
|
||||
source,
|
||||
})?;
|
||||
rules
|
||||
.to_config_policy()
|
||||
.map(Some)
|
||||
.map_err(|source| ExecPolicyError::InvalidConfigRules { path, source })
|
||||
}
|
||||
|
||||
pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result<Policy, ExecPolicyError> {
|
||||
// Disabled project layers already represent the trust decision, so hooks
|
||||
// and exec-policy loading can reuse the normal trusted-layer view.
|
||||
// Iterate the layers in increasing order of precedence, adding the *.rules
|
||||
// from each layer, so that higher-precedence layers can override
|
||||
// rules defined in lower-precedence ones.
|
||||
let mut parser = PolicyParser::new();
|
||||
let mut policy_paths = Vec::new();
|
||||
for layer in config_stack.get_layers(
|
||||
ConfigLayerStackOrdering::LowestPrecedenceFirst,
|
||||
/*include_disabled*/ false,
|
||||
) {
|
||||
if config_stack.ignore_user_and_project_exec_policy_rules()
|
||||
&& matches!(
|
||||
layer.name,
|
||||
ConfigLayerSource::User { .. } | ConfigLayerSource::Project { .. }
|
||||
)
|
||||
{
|
||||
if should_ignore_layer_exec_policy_rules(config_stack, layer) {
|
||||
continue;
|
||||
}
|
||||
if let Some(config_folder) = layer.config_folder() {
|
||||
let policy_dir = config_folder.join(RULES_DIR_NAME);
|
||||
let layer_policy_paths = collect_policy_files(&policy_dir).await?;
|
||||
policy_paths.extend(layer_policy_paths);
|
||||
for policy_path in layer_policy_paths {
|
||||
let contents = fs::read_to_string(&policy_path).await.map_err(|source| {
|
||||
ExecPolicyError::ReadFile {
|
||||
path: policy_path.clone(),
|
||||
source,
|
||||
}
|
||||
})?;
|
||||
let identifier = policy_path.to_string_lossy().to_string();
|
||||
parser.parse(&identifier, &contents).map_err(|source| {
|
||||
ExecPolicyError::ParsePolicy {
|
||||
path: identifier,
|
||||
source,
|
||||
}
|
||||
})?;
|
||||
policy_paths.push(policy_path);
|
||||
}
|
||||
}
|
||||
if let Some(policy) = config_layer_rules_policy(layer)? {
|
||||
parser.merge_overlay(&policy);
|
||||
}
|
||||
}
|
||||
tracing::trace!(
|
||||
@@ -594,24 +680,6 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result<Policy,
|
||||
"loaded exec policies"
|
||||
);
|
||||
|
||||
let mut parser = PolicyParser::new();
|
||||
for policy_path in &policy_paths {
|
||||
let contents =
|
||||
fs::read_to_string(policy_path)
|
||||
.await
|
||||
.map_err(|source| ExecPolicyError::ReadFile {
|
||||
path: policy_path.clone(),
|
||||
source,
|
||||
})?;
|
||||
let identifier = policy_path.to_string_lossy().to_string();
|
||||
parser
|
||||
.parse(&identifier, &contents)
|
||||
.map_err(|source| ExecPolicyError::ParsePolicy {
|
||||
path: identifier,
|
||||
source,
|
||||
})?;
|
||||
}
|
||||
|
||||
let policy = parser.build();
|
||||
tracing::debug!("loaded rules from {} files", policy_paths.len());
|
||||
tracing::trace!(rules = ?policy, "exec policy rules loaded");
|
||||
|
||||
@@ -208,6 +208,44 @@ async fn child_does_not_use_parent_exec_policy_when_requirements_exec_policy_dif
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn child_does_not_use_parent_exec_policy_when_config_toml_rules_differ() -> anyhow::Result<()>
|
||||
{
|
||||
let (_home, parent_config) = test_config().await;
|
||||
let mut child_config = parent_config.clone();
|
||||
let mut layers: Vec<_> = child_config
|
||||
.config_layer_stack
|
||||
.get_layers(
|
||||
ConfigLayerStackOrdering::LowestPrecedenceFirst,
|
||||
/*include_disabled*/ true,
|
||||
)
|
||||
.into_iter()
|
||||
.cloned()
|
||||
.collect();
|
||||
layers.push(ConfigLayerEntry::new(
|
||||
ConfigLayerSource::SessionFlags,
|
||||
toml::from_str::<TomlValue>(
|
||||
r#"
|
||||
[rules]
|
||||
prefix_rules = [
|
||||
{ pattern = [{ token = "echo" }], decision = "allow" },
|
||||
]
|
||||
"#,
|
||||
)?,
|
||||
));
|
||||
child_config.config_layer_stack = ConfigLayerStack::new(
|
||||
layers,
|
||||
child_config.config_layer_stack.requirements().clone(),
|
||||
child_config.config_layer_stack.requirements_toml().clone(),
|
||||
)?;
|
||||
|
||||
assert!(!child_uses_parent_exec_policy(
|
||||
&parent_config,
|
||||
&child_config
|
||||
));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn returns_empty_policy_when_no_policy_files_exist() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
@@ -452,6 +490,87 @@ async fn ignores_policies_outside_policy_dir() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_config_toml_rules_with_starlark_rules() -> anyhow::Result<()> {
|
||||
let temp_dir = tempdir()?;
|
||||
let policy_dir = temp_dir.path().join(RULES_DIR_NAME);
|
||||
fs::create_dir_all(&policy_dir)?;
|
||||
fs::write(
|
||||
policy_dir.join("deny.rules"),
|
||||
r#"prefix_rule(pattern=["rm"], decision="forbidden")"#,
|
||||
)?;
|
||||
let dot_codex_folder =
|
||||
AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute dot_codex_folder");
|
||||
let layer = ConfigLayerEntry::new(
|
||||
ConfigLayerSource::Project { dot_codex_folder },
|
||||
toml::from_str::<TomlValue>(
|
||||
r#"
|
||||
[rules]
|
||||
prefix_rules = [
|
||||
{ pattern = [{ token = "echo" }], decision = "allow" },
|
||||
]
|
||||
"#,
|
||||
)?,
|
||||
);
|
||||
let config_stack = ConfigLayerStack::new(
|
||||
vec![layer],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)?;
|
||||
|
||||
let policy = load_exec_policy(&config_stack).await?;
|
||||
|
||||
assert_eq!(
|
||||
policy
|
||||
.check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow)
|
||||
.decision,
|
||||
Decision::Forbidden
|
||||
);
|
||||
assert_eq!(
|
||||
policy
|
||||
.check_multiple(
|
||||
[vec!["echo".to_string(), "hello".to_string()]].iter(),
|
||||
&|_| { Decision::Forbidden }
|
||||
)
|
||||
.decision,
|
||||
Decision::Allow
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_config_toml_rules_from_user_config_file() -> anyhow::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[rules]
|
||||
prefix_rules = [
|
||||
{ pattern = [{ token = "echo" }], decision = "allow" },
|
||||
]
|
||||
"#,
|
||||
)?;
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
let policy = load_exec_policy(&config.config_layer_stack).await?;
|
||||
|
||||
assert_eq!(
|
||||
policy
|
||||
.check_multiple(
|
||||
[vec!["echo".to_string(), "hello".to_string()]].iter(),
|
||||
&|_| { Decision::Forbidden }
|
||||
)
|
||||
.decision,
|
||||
Decision::Allow
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ignores_policy_files_when_config_stack_disables_exec_policy_rules() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
@@ -479,6 +598,42 @@ async fn ignores_policy_files_when_config_stack_disables_exec_policy_rules() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ignores_config_toml_rules_when_config_stack_disables_exec_policy_rules()
|
||||
-> anyhow::Result<()> {
|
||||
let temp_dir = tempdir()?;
|
||||
let dot_codex_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path())?;
|
||||
let layer = ConfigLayerEntry::new(
|
||||
ConfigLayerSource::Project { dot_codex_folder },
|
||||
toml::from_str::<TomlValue>(
|
||||
r#"
|
||||
[rules]
|
||||
prefix_rules = [
|
||||
{ pattern = [{ token = "curl" }], decision = "allow" },
|
||||
]
|
||||
"#,
|
||||
)?,
|
||||
);
|
||||
let config_stack = ConfigLayerStack::new(
|
||||
vec![layer],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)?
|
||||
.with_user_and_project_exec_policy_rules_ignored(
|
||||
/*ignore_user_and_project_exec_policy_rules*/ true,
|
||||
);
|
||||
|
||||
let policy = load_exec_policy(&config_stack).await?;
|
||||
|
||||
assert_eq!(
|
||||
policy
|
||||
.check_multiple([vec!["curl".to_string()]].iter(), &|_| Decision::Forbidden)
|
||||
.decision,
|
||||
Decision::Forbidden,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ignore_user_project_rules_keeps_system_policy_files() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
@@ -1335,6 +1490,7 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision()
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
@@ -1372,6 +1528,7 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled(
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
@@ -1396,6 +1553,7 @@ async fn exec_approval_requirement_falls_back_to_heuristics() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -1411,6 +1569,55 @@ async fn exec_approval_requirement_falls_back_to_heuristics() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_requirement_honors_permissions_scoped_rule() {
|
||||
let policy_src = r#"prefix_rule(pattern=["git"], decision="prompt", permissions="strict")"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.rules", policy_src)
|
||||
.expect("parse policy");
|
||||
let command = vec!["git".to_string(), "status".to_string()];
|
||||
let manager = ExecPolicyManager::new(Arc::new(parser.build()));
|
||||
|
||||
let scoped_requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: Some("strict".to_string()),
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
})
|
||||
.await;
|
||||
assert_eq!(
|
||||
scoped_requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: Some("`git status` requires approval by policy".to_string()),
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
);
|
||||
|
||||
let unscoped_requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
})
|
||||
.await;
|
||||
assert_eq!(
|
||||
unscoped_requirement,
|
||||
ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_bash_lc_script_falls_back_to_original_command() {
|
||||
let command = vec!["bash".to_string(), "-lc".to_string(), "".to_string()];
|
||||
@@ -1421,6 +1628,7 @@ async fn empty_bash_lc_script_falls_back_to_original_command() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -1450,6 +1658,7 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -1479,6 +1688,7 @@ async fn request_rule_uses_prefix_rule() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
@@ -1511,6 +1721,7 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands(
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
@@ -1550,6 +1761,7 @@ async fn heuristics_apply_when_other_commands_match_policy() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -2031,6 +2243,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
command: &sneaky_command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: permissions,
|
||||
prefix_rule: None,
|
||||
@@ -2055,6 +2268,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
command: &dangerous_command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: permissions,
|
||||
prefix_rule: None,
|
||||
@@ -2075,6 +2289,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
command: &dangerous_command,
|
||||
approval_policy: AskForApproval::Never,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: permissions,
|
||||
prefix_rule: None,
|
||||
@@ -2170,6 +2385,7 @@ async fn exec_approval_requirement_for_command(
|
||||
command: &command,
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::RestrictedToken,
|
||||
sandbox_permissions,
|
||||
prefix_rule,
|
||||
|
||||
@@ -10778,6 +10778,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
command: &command,
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
permission_profile: turn_context.permission_profile(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
|
||||
@@ -172,6 +172,11 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
permission_profile: turn.permission_profile(),
|
||||
active_permission_profile: turn
|
||||
.config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.map(|profile| profile.id),
|
||||
windows_sandbox_level: turn.windows_sandbox_level,
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
|
||||
codex_protocol::models::SandboxPermissions::UseDefault
|
||||
|
||||
@@ -241,6 +241,12 @@ pub(super) async fn try_run_zsh_fork(
|
||||
tool_name: GuardianCommandSource::Shell,
|
||||
approval_policy: ctx.turn.approval_policy.value(),
|
||||
permission_profile: command_executor.permission_profile.clone(),
|
||||
active_permission_profile: ctx
|
||||
.turn
|
||||
.config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.map(|profile| profile.id),
|
||||
file_system_sandbox_policy: command_executor.file_system_sandbox_policy.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
approval_sandbox_permissions,
|
||||
@@ -325,6 +331,12 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
|
||||
tool_name: GuardianCommandSource::UnifiedExec,
|
||||
approval_policy: ctx.turn.approval_policy.value(),
|
||||
permission_profile: exec_request.permission_profile.clone(),
|
||||
active_permission_profile: ctx
|
||||
.turn
|
||||
.config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.map(|profile| profile.id),
|
||||
file_system_sandbox_policy: exec_request.file_system_sandbox_policy.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
approval_sandbox_permissions: approval_sandbox_permissions(
|
||||
@@ -360,6 +372,7 @@ struct CoreShellActionProvider {
|
||||
tool_name: GuardianCommandSource,
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
active_permission_profile: Option<String>,
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
approval_sandbox_permissions: SandboxPermissions,
|
||||
@@ -651,6 +664,7 @@ impl CoreShellActionProvider {
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: self.approval_policy,
|
||||
permission_profile: self.permission_profile.clone(),
|
||||
active_permission_profile: self.active_permission_profile.clone(),
|
||||
windows_sandbox_level: self.turn.windows_sandbox_level,
|
||||
sandbox_permissions: self.approval_sandbox_permissions,
|
||||
enable_shell_wrapper_parsing:
|
||||
@@ -720,6 +734,7 @@ fn evaluate_intercepted_exec_policy(
|
||||
let InterceptedExecPolicyContext {
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
windows_sandbox_level,
|
||||
sandbox_permissions,
|
||||
enable_shell_wrapper_parsing,
|
||||
@@ -760,6 +775,7 @@ fn evaluate_intercepted_exec_policy(
|
||||
&fallback,
|
||||
&MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
active_permission_profile,
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -768,6 +784,7 @@ fn evaluate_intercepted_exec_policy(
|
||||
struct InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
active_permission_profile: Option<String>,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
enable_shell_wrapper_parsing: bool,
|
||||
|
||||
@@ -430,6 +430,7 @@ async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyho
|
||||
tool_name: GuardianCommandSource::Shell,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: permission_profile.clone(),
|
||||
active_permission_profile: None,
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
approval_sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
@@ -566,6 +567,7 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul
|
||||
tool_name: GuardianCommandSource::Shell,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
@@ -631,6 +633,7 @@ fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_pars
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing,
|
||||
@@ -682,6 +685,7 @@ fn evaluate_intercepted_exec_policy_matches_inner_shell_commands_when_enabled()
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing,
|
||||
@@ -724,6 +728,7 @@ host_executable(name = "git", paths = ["{git_path_literal}"])
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: false,
|
||||
@@ -777,6 +782,7 @@ prefix_rule(pattern = ["{cat_path_literal}"], decision = "allow")
|
||||
tool_name: GuardianCommandSource::Shell,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile,
|
||||
active_permission_profile: None,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
approval_sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
@@ -820,6 +826,7 @@ async fn denied_reads_keep_granular_sandbox_rejection_for_escalation() -> anyhow
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile,
|
||||
active_permission_profile: None,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
approval_sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
@@ -859,6 +866,7 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy,
|
||||
permission_profile: permission_profile.clone(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: super::approval_sandbox_permissions(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
@@ -874,6 +882,7 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
enable_shell_wrapper_parsing: false,
|
||||
@@ -907,6 +916,7 @@ host_executable(name = "git", paths = ["{allowed_git_literal}"])
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: false,
|
||||
|
||||
@@ -1138,6 +1138,12 @@ impl UnifiedExecProcessManager {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
permission_profile: context.turn.permission_profile(),
|
||||
active_permission_profile: context
|
||||
.turn
|
||||
.config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.map(|profile| profile.id),
|
||||
windows_sandbox_level: context.turn.windows_sandbox_level,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
|
||||
@@ -3235,6 +3235,151 @@ async fn approving_fallback_rule_for_compound_command_works() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn permissions_scoped_prefix_rule_uses_active_profile_in_shell_flow() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
fs::write(
|
||||
home.path().join("config.toml"),
|
||||
r#"default_permissions = "project_profile"
|
||||
|
||||
[permissions.project_profile.filesystem]
|
||||
":minimal" = "read"
|
||||
|
||||
[permissions.project_profile.filesystem.":workspace_roots"]
|
||||
"." = "write"
|
||||
|
||||
[permissions.other_profile.filesystem]
|
||||
":minimal" = "read"
|
||||
|
||||
[permissions.other_profile.filesystem.":workspace_roots"]
|
||||
"." = "write"
|
||||
"#,
|
||||
)?;
|
||||
let rules_dir = home.path().join("rules");
|
||||
fs::create_dir_all(&rules_dir)?;
|
||||
fs::write(
|
||||
rules_dir.join("default.rules"),
|
||||
r#"prefix_rule(pattern=["touch", "scoped-allow.txt"], decision="allow", permissions="project_profile")
|
||||
prefix_rule(pattern=["touch", "scoped-prompt.txt"], decision="allow", permissions="other_profile")
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let mut builder = test_codex().with_home(home).with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
assert_eq!(
|
||||
test.session_configured
|
||||
.active_permission_profile
|
||||
.as_ref()
|
||||
.map(|profile| profile.id.as_str()),
|
||||
Some("project_profile")
|
||||
);
|
||||
|
||||
let allow_call_id = "scoped-profile-allow";
|
||||
let allow_command = "touch scoped-allow.txt";
|
||||
let allow_event = shell_event(
|
||||
allow_call_id,
|
||||
allow_command,
|
||||
/*timeout_ms*/ 1_000,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
)?;
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-scoped-profile-allow-1"),
|
||||
allow_event,
|
||||
ev_completed("resp-scoped-profile-allow-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let allow_results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-scoped-profile-allow", "done"),
|
||||
ev_completed("resp-scoped-profile-allow-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn_preserving_active_permission_profile(
|
||||
&test,
|
||||
"run matching scoped rule",
|
||||
approval_policy,
|
||||
)
|
||||
.await?;
|
||||
wait_for_completion_without_approval(&test).await;
|
||||
|
||||
let allow_output = parse_result(
|
||||
&allow_results
|
||||
.single_request()
|
||||
.function_call_output(allow_call_id),
|
||||
);
|
||||
assert_eq!(
|
||||
allow_output.exit_code,
|
||||
Some(0),
|
||||
"expected matching scoped rule to run without approval: {}",
|
||||
allow_output.stdout
|
||||
);
|
||||
let allow_path = test.cwd.path().join("scoped-allow.txt");
|
||||
assert!(
|
||||
allow_path.exists(),
|
||||
"expected matching scoped rule to create {allow_path:?}"
|
||||
);
|
||||
fs::remove_file(allow_path)?;
|
||||
|
||||
let prompt_call_id = "scoped-profile-prompt";
|
||||
let prompt_command = "touch scoped-prompt.txt";
|
||||
let prompt_event = shell_event(
|
||||
prompt_call_id,
|
||||
prompt_command,
|
||||
/*timeout_ms*/ 1_000,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
)?;
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-scoped-profile-prompt-1"),
|
||||
prompt_event,
|
||||
ev_completed("resp-scoped-profile-prompt-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-scoped-profile-prompt", "done"),
|
||||
ev_completed("resp-scoped-profile-prompt-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn_preserving_active_permission_profile(
|
||||
&test,
|
||||
"run non-matching scoped rule",
|
||||
approval_policy,
|
||||
)
|
||||
.await?;
|
||||
let approval = expect_exec_approval(&test, prompt_command).await;
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Denied,
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion(&test).await;
|
||||
assert!(
|
||||
!test.cwd.path().join("scoped-prompt.txt").exists(),
|
||||
"non-matching scoped rule should not bypass approval"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn denying_network_policy_amendment_persists_policy_and_skips_future_network_prompt()
|
||||
-> Result<()> {
|
||||
|
||||
@@ -274,7 +274,9 @@ impl ExecServerClient {
|
||||
connect_async_with_config(
|
||||
request,
|
||||
Some(noise_relay_websocket_config()),
|
||||
/*disable_nagle*/ false,
|
||||
// Rendezvous sends small, latency-sensitive frames, so avoid Nagle's coalescing delay.
|
||||
/*disable_nagle*/
|
||||
true,
|
||||
),
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -561,7 +561,9 @@ async fn connect_rendezvous(
|
||||
connect_async_with_config(
|
||||
request,
|
||||
Some(noise_relay_websocket_config()),
|
||||
/*disable_nagle*/ false,
|
||||
// Rendezvous sends small, latency-sensitive frames, so avoid Nagle's coalescing delay.
|
||||
/*disable_nagle*/
|
||||
true,
|
||||
)
|
||||
.await
|
||||
.map(|(websocket, _)| websocket)
|
||||
|
||||
@@ -28,6 +28,10 @@ pub struct ExecPolicyCheckCommand {
|
||||
#[arg(long)]
|
||||
pub resolve_host_executables: bool,
|
||||
|
||||
/// Active permissions profile id to use for permissions-scoped rules.
|
||||
#[arg(long, value_name = "ID")]
|
||||
pub permissions: Option<String>,
|
||||
|
||||
/// Command tokens to check against the policy.
|
||||
#[arg(
|
||||
value_name = "COMMAND",
|
||||
@@ -47,6 +51,7 @@ impl ExecPolicyCheckCommand {
|
||||
/*heuristics_fallback*/ None,
|
||||
&MatchOptions {
|
||||
resolve_host_executables: self.resolve_host_executables,
|
||||
active_permission_profile: self.permissions.clone(),
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -81,6 +81,10 @@ impl PolicyParser {
|
||||
pub fn build(self) -> crate::policy::Policy {
|
||||
self.builder.into_inner().build()
|
||||
}
|
||||
|
||||
pub fn merge_overlay(&mut self, overlay: &crate::policy::Policy) {
|
||||
self.builder.borrow_mut().merge_overlay(overlay);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, ProvidesStaticType)]
|
||||
@@ -158,6 +162,22 @@ impl PolicyBuilder {
|
||||
self.host_executables_by_name,
|
||||
)
|
||||
}
|
||||
|
||||
fn merge_overlay(&mut self, overlay: &crate::policy::Policy) {
|
||||
for (program, rules) in overlay.rules().iter_all() {
|
||||
for rule in rules {
|
||||
self.rules_by_program.insert(program.clone(), rule.clone());
|
||||
}
|
||||
}
|
||||
self.network_rules
|
||||
.extend(overlay.network_rules().iter().cloned());
|
||||
self.host_executables_by_name.extend(
|
||||
overlay
|
||||
.host_executables()
|
||||
.iter()
|
||||
.map(|(name, paths)| (name.clone(), paths.clone())),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -257,6 +277,16 @@ fn parse_network_rule_decision(raw: &str) -> Result<Decision> {
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_permissions(permissions: Option<&str>) -> Result<Option<String>> {
|
||||
match permissions {
|
||||
Some(raw) if raw.trim().is_empty() => Err(Error::InvalidRule(
|
||||
"permissions cannot be empty".to_string(),
|
||||
)),
|
||||
Some(raw) => Ok(Some(raw.to_string())),
|
||||
None => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
fn error_location_from_file_span(span: FileSpan) -> ErrorLocation {
|
||||
let resolved = span.resolve_span();
|
||||
ErrorLocation {
|
||||
@@ -352,6 +382,7 @@ fn policy_builtins(builder: &mut GlobalsBuilder) {
|
||||
r#match: Option<UnpackList<Value<'v>>>,
|
||||
not_match: Option<UnpackList<Value<'v>>>,
|
||||
justification: Option<&'v str>,
|
||||
permissions: Option<&'v str>,
|
||||
eval: &mut Evaluator<'v, '_, '_>,
|
||||
) -> anyhow::Result<NoneType> {
|
||||
let decision = match decision {
|
||||
@@ -366,6 +397,7 @@ fn policy_builtins(builder: &mut GlobalsBuilder) {
|
||||
Some(raw) => Some(raw.to_string()),
|
||||
None => None,
|
||||
};
|
||||
let permissions = parse_permissions(permissions)?;
|
||||
|
||||
let pattern_tokens = parse_pattern(pattern)?;
|
||||
|
||||
@@ -398,6 +430,7 @@ fn policy_builtins(builder: &mut GlobalsBuilder) {
|
||||
},
|
||||
decision,
|
||||
justification: justification.clone(),
|
||||
permissions: permissions.clone(),
|
||||
}) as RuleRef
|
||||
})
|
||||
.collect();
|
||||
|
||||
@@ -22,6 +22,7 @@ type HeuristicsFallback<'a> = Option<&'a dyn Fn(&[String]) -> Decision>;
|
||||
#[derive(Clone, Debug, Default, Eq, PartialEq)]
|
||||
pub struct MatchOptions {
|
||||
pub resolve_host_executables: bool,
|
||||
pub active_permission_profile: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
@@ -75,6 +76,9 @@ impl Policy {
|
||||
if prefix_rule.decision != Decision::Allow {
|
||||
continue;
|
||||
}
|
||||
if prefix_rule.permissions.is_some() {
|
||||
continue;
|
||||
}
|
||||
|
||||
let mut prefix = Vec::with_capacity(prefix_rule.pattern.rest.len() + 1);
|
||||
prefix.push(prefix_rule.pattern.first.as_ref().to_string());
|
||||
@@ -104,6 +108,7 @@ impl Policy {
|
||||
},
|
||||
decision,
|
||||
justification: None,
|
||||
permissions: None,
|
||||
});
|
||||
|
||||
self.rules_by_program.insert(first_token.clone(), rule);
|
||||
@@ -272,12 +277,12 @@ impl Policy {
|
||||
options: &MatchOptions,
|
||||
) -> Vec<RuleMatch> {
|
||||
let matched_rules = self
|
||||
.match_exact_rules(cmd)
|
||||
.match_exact_rules(cmd, options)
|
||||
.filter(|matched_rules| !matched_rules.is_empty())
|
||||
.or_else(|| {
|
||||
options
|
||||
.resolve_host_executables
|
||||
.then(|| self.match_host_executable_rules(cmd))
|
||||
.then(|| self.match_host_executable_rules(cmd, options))
|
||||
.filter(|matched_rules| !matched_rules.is_empty())
|
||||
})
|
||||
.unwrap_or_default();
|
||||
@@ -294,17 +299,26 @@ impl Policy {
|
||||
}
|
||||
}
|
||||
|
||||
fn match_exact_rules(&self, cmd: &[String]) -> Option<Vec<RuleMatch>> {
|
||||
fn match_exact_rules(&self, cmd: &[String], options: &MatchOptions) -> Option<Vec<RuleMatch>> {
|
||||
let first = cmd.first()?;
|
||||
Some(
|
||||
self.rules_by_program
|
||||
.get_vec(first)
|
||||
.map(|rules| rules.iter().filter_map(|rule| rule.matches(cmd)).collect())
|
||||
.map(|rules| {
|
||||
rules
|
||||
.iter()
|
||||
.filter_map(|rule| rule.matches(cmd, options))
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default(),
|
||||
)
|
||||
}
|
||||
|
||||
fn match_host_executable_rules(&self, cmd: &[String]) -> Vec<RuleMatch> {
|
||||
fn match_host_executable_rules(
|
||||
&self,
|
||||
cmd: &[String],
|
||||
options: &MatchOptions,
|
||||
) -> Vec<RuleMatch> {
|
||||
let Some(first) = cmd.first() else {
|
||||
return Vec::new();
|
||||
};
|
||||
@@ -328,7 +342,7 @@ impl Policy {
|
||||
.collect::<Vec<_>>();
|
||||
rules
|
||||
.iter()
|
||||
.filter_map(|rule| rule.matches(&basename_command))
|
||||
.filter_map(|rule| rule.matches(&basename_command, options))
|
||||
.map(|rule_match| rule_match.with_resolved_program(&program))
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -112,6 +112,7 @@ pub struct PrefixRule {
|
||||
pub pattern: PrefixPattern,
|
||||
pub decision: Decision,
|
||||
pub justification: Option<String>,
|
||||
pub permissions: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
@@ -214,7 +215,7 @@ pub(crate) fn normalize_network_rule_host(raw: &str) -> Result<String> {
|
||||
pub trait Rule: Any + Debug + Send + Sync {
|
||||
fn program(&self) -> &str;
|
||||
|
||||
fn matches(&self, cmd: &[String]) -> Option<RuleMatch>;
|
||||
fn matches(&self, cmd: &[String], options: &MatchOptions) -> Option<RuleMatch>;
|
||||
|
||||
fn as_any(&self) -> &dyn Any;
|
||||
}
|
||||
@@ -226,7 +227,13 @@ impl Rule for PrefixRule {
|
||||
self.pattern.first.as_ref()
|
||||
}
|
||||
|
||||
fn matches(&self, cmd: &[String]) -> Option<RuleMatch> {
|
||||
fn matches(&self, cmd: &[String], options: &MatchOptions) -> Option<RuleMatch> {
|
||||
if let Some(permissions) = self.permissions.as_deref()
|
||||
&& options.active_permission_profile.as_deref() != Some(permissions)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
self.pattern
|
||||
.matches_prefix(cmd)
|
||||
.map(|matched_prefix| RuleMatch::PrefixRuleMatch {
|
||||
@@ -251,6 +258,7 @@ pub(crate) fn validate_match_examples(
|
||||
let mut unmatched_examples = Vec::new();
|
||||
let options = MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
active_permission_profile: validation_permissions(rules),
|
||||
};
|
||||
|
||||
for example in matches {
|
||||
@@ -281,11 +289,12 @@ pub(crate) fn validate_match_examples(
|
||||
/// Ensure that no rule matches any provided negative example.
|
||||
pub(crate) fn validate_not_match_examples(
|
||||
policy: &Policy,
|
||||
_rules: &[RuleRef],
|
||||
rules: &[RuleRef],
|
||||
not_matches: &[Vec<String>],
|
||||
) -> Result<()> {
|
||||
let options = MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
active_permission_profile: validation_permissions(rules),
|
||||
};
|
||||
|
||||
for example in not_matches {
|
||||
@@ -304,3 +313,21 @@ pub(crate) fn validate_not_match_examples(
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validation_permissions(rules: &[RuleRef]) -> Option<String> {
|
||||
let mut permissions = None;
|
||||
for rule in rules {
|
||||
let Some(prefix_rule) = rule.as_any().downcast_ref::<PrefixRule>() else {
|
||||
continue;
|
||||
};
|
||||
match (&permissions, &prefix_rule.permissions) {
|
||||
(None, Some(rule_permissions)) => {
|
||||
permissions = Some(rule_permissions.clone());
|
||||
}
|
||||
(Some(permissions), Some(rule_permissions)) if permissions == rule_permissions => {}
|
||||
(_, None) => {}
|
||||
(Some(_), Some(_)) => return None,
|
||||
}
|
||||
}
|
||||
permissions
|
||||
}
|
||||
|
||||
@@ -260,6 +260,7 @@ fn add_prefix_rule_extends_policy() -> Result<()> {
|
||||
},
|
||||
decision: Decision::Prompt,
|
||||
justification: None,
|
||||
permissions: None,
|
||||
})],
|
||||
rules
|
||||
);
|
||||
@@ -292,6 +293,88 @@ fn add_prefix_rule_rejects_empty_prefix() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prefix_rule_can_be_scoped_to_permissions() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(pattern = ["git", "status"], decision = "prompt", permissions = "strict")
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser.parse("test.rules", policy_src)?;
|
||||
let policy = parser.build();
|
||||
let command = tokens(&["git", "status"]);
|
||||
|
||||
let matching_eval = policy.check_with_options(
|
||||
&command,
|
||||
&allow_all,
|
||||
&MatchOptions {
|
||||
active_permission_profile: Some("strict".to_string()),
|
||||
..MatchOptions::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
matching_eval,
|
||||
Evaluation {
|
||||
decision: Decision::Prompt,
|
||||
matched_rules: vec![RuleMatch::PrefixRuleMatch {
|
||||
matched_prefix: command.clone(),
|
||||
decision: Decision::Prompt,
|
||||
resolved_program: None,
|
||||
justification: None,
|
||||
}],
|
||||
}
|
||||
);
|
||||
|
||||
let non_matching_eval = policy.check_with_options(
|
||||
&command,
|
||||
&allow_all,
|
||||
&MatchOptions {
|
||||
active_permission_profile: Some("relaxed".to_string()),
|
||||
..MatchOptions::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
non_matching_eval,
|
||||
Evaluation {
|
||||
decision: Decision::Allow,
|
||||
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
|
||||
command,
|
||||
decision: Decision::Allow,
|
||||
}],
|
||||
}
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_scoped_allow_rules_are_not_reported_as_global_prefixes() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(pattern = ["sl", "status"], decision = "allow")
|
||||
prefix_rule(pattern = ["cargo", "test"], decision = "allow", permissions = "workspace")
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser.parse("test.rules", policy_src)?;
|
||||
let policy = parser.build();
|
||||
|
||||
assert_eq!(
|
||||
policy.get_allowed_prefixes(),
|
||||
vec![tokens(&["sl", "status"])]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prefix_rule_rejects_empty_permissions() {
|
||||
let mut parser = PolicyParser::new();
|
||||
let err = parser
|
||||
.parse(
|
||||
"test.rules",
|
||||
r#"prefix_rule(pattern=["git"], permissions="")"#,
|
||||
)
|
||||
.expect_err("expected empty permissions to fail");
|
||||
assert!(err.to_string().contains("permissions cannot be empty"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_multiple_policy_files() -> Result<()> {
|
||||
let first_policy = r#"
|
||||
@@ -321,6 +404,7 @@ prefix_rule(
|
||||
},
|
||||
decision: Decision::Prompt,
|
||||
justification: None,
|
||||
permissions: None,
|
||||
}),
|
||||
RuleSnapshot::Prefix(PrefixRule {
|
||||
pattern: PrefixPattern {
|
||||
@@ -329,6 +413,7 @@ prefix_rule(
|
||||
},
|
||||
decision: Decision::Forbidden,
|
||||
justification: None,
|
||||
permissions: None,
|
||||
}),
|
||||
],
|
||||
git_rules
|
||||
@@ -398,6 +483,7 @@ prefix_rule(
|
||||
},
|
||||
decision: Decision::Allow,
|
||||
justification: None,
|
||||
permissions: None,
|
||||
})],
|
||||
bash_rules
|
||||
);
|
||||
@@ -409,6 +495,7 @@ prefix_rule(
|
||||
},
|
||||
decision: Decision::Allow,
|
||||
justification: None,
|
||||
permissions: None,
|
||||
})],
|
||||
sh_rules
|
||||
);
|
||||
@@ -470,6 +557,7 @@ prefix_rule(
|
||||
},
|
||||
decision: Decision::Allow,
|
||||
justification: None,
|
||||
permissions: None,
|
||||
})],
|
||||
rules
|
||||
);
|
||||
@@ -786,6 +874,7 @@ host_executable(name = "git", paths = ["{git_path_literal}"])
|
||||
&allow_all,
|
||||
&MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
..MatchOptions::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -843,6 +932,7 @@ host_executable(name = "git", paths = [])
|
||||
&allow_all,
|
||||
&MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
..MatchOptions::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -878,6 +968,7 @@ host_executable(name = "git", paths = ["{allowed_git_literal}"])
|
||||
&allow_all,
|
||||
&MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
..MatchOptions::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -908,6 +999,7 @@ prefix_rule(pattern = ["git"], decision = "prompt")
|
||||
&allow_all,
|
||||
&MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
..MatchOptions::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -945,6 +1037,7 @@ host_executable(name = "git", paths = ["{git_path_literal}"])
|
||||
&allow_all,
|
||||
&MatchOptions {
|
||||
resolve_host_executables: true,
|
||||
..MatchOptions::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(
|
||||
|
||||
@@ -4,6 +4,7 @@ use super::*;
|
||||
use codex_app_server_protocol::ModelSafetyBufferingUpdatedNotification;
|
||||
|
||||
const SAFETY_BUFFERING_PROMPT_VIEW_ID: &str = "safety-buffering-prompt";
|
||||
const SAFETY_BUFFERING_LEARN_MORE_URL: &str = "https://help.openai.com/en/articles/20001326";
|
||||
|
||||
const SAFETY_BUFFERING_MESSAGE_WITH_RETRY: &str = "This request requires additional safety checks, which can take extra time. Hang tight or retry with a faster model for a quicker response, though it may be less capable of handling complex requests.";
|
||||
const SAFETY_BUFFERING_MESSAGE_WITHOUT_RETRY: &str =
|
||||
@@ -12,7 +13,7 @@ const SAFETY_BUFFERING_MESSAGE_WITHOUT_RETRY: &str =
|
||||
#[derive(Debug)]
|
||||
struct ActiveSafetyBuffering {
|
||||
turn_id: String,
|
||||
retry_prompt_shown: bool,
|
||||
last_prompt_had_retry: bool,
|
||||
agent_message_started: bool,
|
||||
}
|
||||
|
||||
@@ -119,22 +120,18 @@ impl ChatWidget {
|
||||
.map(|(_, turn)| turn.clone());
|
||||
let thread_id = self.thread_id;
|
||||
let can_offer_retry = faster_model.is_some() && retry_turn.is_some() && thread_id.is_some();
|
||||
if !can_offer_retry {
|
||||
self.bottom_pane
|
||||
.dismiss_view_by_id(SAFETY_BUFFERING_PROMPT_VIEW_ID);
|
||||
}
|
||||
let previous_active = self
|
||||
.safety_buffering
|
||||
.active
|
||||
.as_ref()
|
||||
.filter(|active| active.turn_id == turn_id);
|
||||
let retry_prompt_shown = previous_active.is_some_and(|active| active.retry_prompt_shown);
|
||||
let should_show_retry_prompt = can_offer_retry && !retry_prompt_shown;
|
||||
let should_show_prompt =
|
||||
previous_active.is_none_or(|active| active.last_prompt_had_retry != can_offer_retry);
|
||||
let agent_message_started =
|
||||
previous_active.is_some_and(|active| active.agent_message_started);
|
||||
self.safety_buffering.active = Some(ActiveSafetyBuffering {
|
||||
turn_id: turn_id.clone(),
|
||||
retry_prompt_shown: retry_prompt_shown || should_show_retry_prompt,
|
||||
last_prompt_had_retry: can_offer_retry,
|
||||
agent_message_started,
|
||||
});
|
||||
|
||||
@@ -151,45 +148,54 @@ impl ChatWidget {
|
||||
/*details_max_lines*/ 6,
|
||||
);
|
||||
|
||||
let (Some(faster_model), Some(turn), Some(thread_id)) =
|
||||
(faster_model, retry_turn, thread_id)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
if !should_show_retry_prompt {
|
||||
if !should_show_prompt {
|
||||
return;
|
||||
}
|
||||
self.bottom_pane
|
||||
.dismiss_view_by_id(SAFETY_BUFFERING_PROMPT_VIEW_ID);
|
||||
|
||||
let header = ColumnRenderable::with(vec![
|
||||
Box::new(Line::from("Additional safety checks").bold()) as Box<dyn Renderable>,
|
||||
Box::new(
|
||||
Paragraph::new(Line::from(SAFETY_BUFFERING_MESSAGE_WITH_RETRY).dim())
|
||||
.wrap(Wrap { trim: false }),
|
||||
),
|
||||
Box::new(Paragraph::new(Line::from(message).dim()).wrap(Wrap { trim: false })),
|
||||
]);
|
||||
let mut items = Vec::new();
|
||||
if let (Some(faster_model), Some(turn), Some(thread_id)) =
|
||||
(faster_model, retry_turn, thread_id)
|
||||
{
|
||||
items.push(SelectionItem {
|
||||
name: "Retry with a faster model".to_string(),
|
||||
actions: vec![Box::new(move |tx| {
|
||||
tx.send(AppEvent::RetrySafetyBufferedTurn {
|
||||
thread_id,
|
||||
turn_id: turn_id.clone(),
|
||||
model: faster_model.clone(),
|
||||
turn: turn.clone(),
|
||||
});
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
});
|
||||
}
|
||||
items.extend([
|
||||
SelectionItem {
|
||||
name: "Keep waiting".to_string(),
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
SelectionItem {
|
||||
name: "Learn more".to_string(),
|
||||
actions: vec![Box::new(|tx| {
|
||||
tx.send(AppEvent::OpenUrlInBrowser {
|
||||
url: SAFETY_BUFFERING_LEARN_MORE_URL.to_string(),
|
||||
});
|
||||
})],
|
||||
..Default::default()
|
||||
},
|
||||
]);
|
||||
self.bottom_pane.show_selection_view(SelectionViewParams {
|
||||
view_id: Some(SAFETY_BUFFERING_PROMPT_VIEW_ID),
|
||||
header: Box::new(header),
|
||||
items: vec![
|
||||
SelectionItem {
|
||||
name: "Retry with a faster model".to_string(),
|
||||
actions: vec![Box::new(move |tx| {
|
||||
tx.send(AppEvent::RetrySafetyBufferedTurn {
|
||||
thread_id,
|
||||
turn_id: turn_id.clone(),
|
||||
model: faster_model.clone(),
|
||||
turn: turn.clone(),
|
||||
});
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
SelectionItem {
|
||||
name: "Keep waiting".to_string(),
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
],
|
||||
items,
|
||||
..Default::default()
|
||||
});
|
||||
}
|
||||
|
||||
+1
@@ -9,5 +9,6 @@ expression: popup
|
||||
|
||||
› 1. Retry with a faster model
|
||||
2. Keep waiting
|
||||
3. Learn more
|
||||
|
||||
Press enter to confirm or esc to go back
|
||||
|
||||
+6
-6
@@ -1,11 +1,11 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/app_server.rs
|
||||
expression: "render_bottom_popup(&chat, 80)"
|
||||
expression: popup
|
||||
---
|
||||
• Working (0s • esc to interrupt)
|
||||
└ This request requires additional safety checks, which can take extra time.
|
||||
Additional safety checks
|
||||
This request requires additional safety checks, which can take extra time.
|
||||
|
||||
› 1. Keep waiting
|
||||
2. Learn more
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-5.5 default · /tmp/project
|
||||
Press enter to confirm or esc to go back
|
||||
|
||||
@@ -125,6 +125,21 @@ async fn safety_buffering_offers_one_retry_with_app_wording() {
|
||||
let popup = render_bottom_popup(&chat, /*width*/ 80);
|
||||
assert_chatwidget_snapshot!("safety_buffering_retry_prompt", popup);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
let opened_url = loop {
|
||||
match rx.try_recv() {
|
||||
Ok(AppEvent::OpenUrlInBrowser { url }) => break url,
|
||||
Ok(_) => continue,
|
||||
Err(err) => panic!("expected learn-more URL event: {err}"),
|
||||
}
|
||||
};
|
||||
assert_eq!(opened_url, "https://help.openai.com/en/articles/20001326");
|
||||
assert!(render_bottom_popup(&chat, /*width*/ 80).contains("Additional safety checks"));
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
let (event_thread_id, event_turn_id, model, turn) = loop {
|
||||
match rx.try_recv() {
|
||||
@@ -201,6 +216,9 @@ async fn safety_buffering_without_retry_shows_short_app_message() {
|
||||
Some(ReplayKind::ThreadSnapshot),
|
||||
);
|
||||
assert_eq!(render_popup(&chat), popup);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
assert!(!render_bottom_popup(&chat, /*width*/ 80).contains("Additional safety checks"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -96,8 +96,8 @@ const SAFETY_ACCESS_BLOCK_LEARN_MORE_URL: &str = "https://help.openai.com/en/art
|
||||
|
||||
pub(crate) fn new_safety_access_block_event() -> SafetyAccessBlockCell {
|
||||
SafetyAccessBlockCell {
|
||||
body: "We take extra caution with requests involving biological research and applications that could pose safety risks. If you’re a researcher at an approved organization, you may be able to apply for Trusted Access.",
|
||||
trusted_access_url: "https://openai.com/form/trusted-access-for-life-sciences",
|
||||
body: "We take extra caution with requests involving biological research and applications that could pose safety risks. Eligible researchers can apply for Trusted Access.",
|
||||
trusted_access_url: "https://www.openai.com/form/trusted-access-for-biology-research/",
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+4
-3
@@ -4,7 +4,8 @@ expression: rendered
|
||||
---
|
||||
ⓘ This content can't be shown
|
||||
We take extra caution with requests involving biological research and
|
||||
applications that could pose safety risks. If you’re a researcher at an
|
||||
approved organization, you may be able to apply for Trusted Access.
|
||||
Trusted Access: https://openai.com/form/trusted-access-for-life-sciences
|
||||
applications that could pose safety risks. Eligible researchers can apply
|
||||
for Trusted Access.
|
||||
Trusted Access: https://www.openai.com/form/trusted-access-for-biology-
|
||||
research/
|
||||
Learn more: https://help.openai.com/en/articles/20001326
|
||||
|
||||
Reference in New Issue
Block a user