mirror of
https://github.com/openai/codex.git
synced 2026-05-22 20:14:17 +00:00
Compare commits
5 Commits
pr24108
...
rreichel3/
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a2a5acbd3c | ||
|
|
d189b55730 | ||
|
|
7b43801e2a | ||
|
|
4a4419b1d4 | ||
|
|
270ed782e5 |
@@ -6574,8 +6574,11 @@
|
||||
]
|
||||
},
|
||||
"forced_chatgpt_workspace_id": {
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": [
|
||||
"string",
|
||||
"array",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
|
||||
@@ -3185,8 +3185,11 @@
|
||||
]
|
||||
},
|
||||
"forced_chatgpt_workspace_id": {
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": [
|
||||
"string",
|
||||
"array",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
|
||||
@@ -234,8 +234,11 @@
|
||||
]
|
||||
},
|
||||
"forced_chatgpt_workspace_id": {
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": [
|
||||
"string",
|
||||
"array",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
|
||||
@@ -20,4 +20,4 @@ export type Config = {model: string | null, review_model: string | null, model_c
|
||||
* [UNSTABLE] Optional default for where approval requests are routed for
|
||||
* review.
|
||||
*/
|
||||
approvals_reviewer: ApprovalsReviewer | null, sandbox_mode: SandboxMode | null, sandbox_workspace_write: SandboxWorkspaceWrite | null, forced_chatgpt_workspace_id: string | null, forced_login_method: ForcedLoginMethod | null, web_search: WebSearchMode | null, tools: ToolsV2 | null, profile: string | null, profiles: { [key in string]?: ProfileV2 }, instructions: string | null, developer_instructions: string | null, compact_prompt: string | null, model_reasoning_effort: ReasoningEffort | null, model_reasoning_summary: ReasoningSummary | null, model_verbosity: Verbosity | null, service_tier: ServiceTier | null, analytics: AnalyticsConfig | null} & ({ [key in string]?: number | string | boolean | Array<JsonValue> | { [key in string]?: JsonValue } | null });
|
||||
approvals_reviewer: ApprovalsReviewer | null, sandbox_mode: SandboxMode | null, sandbox_workspace_write: SandboxWorkspaceWrite | null, forced_chatgpt_workspace_id: Array<string> | null, forced_login_method: ForcedLoginMethod | null, web_search: WebSearchMode | null, tools: ToolsV2 | null, profile: string | null, profiles: { [key in string]?: ProfileV2 }, instructions: string | null, developer_instructions: string | null, compact_prompt: string | null, model_reasoning_effort: ReasoningEffort | null, model_reasoning_summary: ReasoningSummary | null, model_verbosity: Verbosity | null, service_tier: ServiceTier | null, analytics: AnalyticsConfig | null} & ({ [key in string]?: number | string | boolean | Array<JsonValue> | { [key in string]?: JsonValue } | null });
|
||||
|
||||
@@ -198,7 +198,7 @@ pub struct UserSavedConfig {
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
pub sandbox_mode: Option<SandboxMode>,
|
||||
pub sandbox_settings: Option<SandboxSettings>,
|
||||
pub forced_chatgpt_workspace_id: Option<String>,
|
||||
pub forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
pub forced_login_method: Option<ForcedLoginMethod>,
|
||||
pub model: Option<String>,
|
||||
pub model_reasoning_effort: Option<ReasoningEffort>,
|
||||
|
||||
@@ -776,7 +776,7 @@ pub struct Config {
|
||||
pub approvals_reviewer: Option<ApprovalsReviewer>,
|
||||
pub sandbox_mode: Option<SandboxMode>,
|
||||
pub sandbox_workspace_write: Option<SandboxWorkspaceWrite>,
|
||||
pub forced_chatgpt_workspace_id: Option<String>,
|
||||
pub forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
pub forced_login_method: Option<ForcedLoginMethod>,
|
||||
pub web_search: Option<WebSearchMode>,
|
||||
pub tools: Option<ToolsV2>,
|
||||
|
||||
@@ -1677,13 +1677,13 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(expected_workspace) = self.config.forced_chatgpt_workspace_id.as_deref()
|
||||
&& chatgpt_account_id != expected_workspace
|
||||
if let Some(expected_workspaces) = self.config.forced_chatgpt_workspace_id.as_deref()
|
||||
&& !expected_workspaces.contains(&chatgpt_account_id)
|
||||
{
|
||||
let error = JSONRPCErrorError {
|
||||
code: INVALID_REQUEST_ERROR_CODE,
|
||||
message: format!(
|
||||
"External auth must use workspace {expected_workspace}, but received {chatgpt_account_id:?}."
|
||||
"External auth must use one of workspace(s) {expected_workspaces:?}, but received {chatgpt_account_id:?}.",
|
||||
),
|
||||
data: None,
|
||||
};
|
||||
|
||||
@@ -112,7 +112,7 @@ fn print_login_server_start(actual_port: u16, auth_url: &str) {
|
||||
|
||||
pub async fn login_with_chatgpt(
|
||||
codex_home: PathBuf,
|
||||
forced_chatgpt_workspace_id: Option<String>,
|
||||
forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode,
|
||||
) -> std::io::Result<()> {
|
||||
let opts = ServerOptions::new(
|
||||
|
||||
@@ -64,6 +64,23 @@ const RESERVED_MODEL_PROVIDER_IDS: [&str; 4] = [
|
||||
LMSTUDIO_OSS_PROVIDER_ID,
|
||||
];
|
||||
|
||||
/// Backward-compatible shape for workspace restrictions in config.toml.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)]
|
||||
#[serde(untagged)]
|
||||
pub enum ForcedChatgptWorkspaceIds {
|
||||
Single(String),
|
||||
Multiple(Vec<String>),
|
||||
}
|
||||
|
||||
impl ForcedChatgptWorkspaceIds {
|
||||
pub fn into_vec(self) -> Vec<String> {
|
||||
match self {
|
||||
Self::Single(value) => vec![value],
|
||||
Self::Multiple(values) => values,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Base config deserialized from ~/.codex/config.toml.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, JsonSchema)]
|
||||
#[schemars(deny_unknown_fields)]
|
||||
@@ -151,9 +168,9 @@ pub struct ConfigToml {
|
||||
/// Set to an empty string to disable automatic commit attribution.
|
||||
pub commit_attribution: Option<String>,
|
||||
|
||||
/// When set, restricts ChatGPT login to a specific workspace identifier.
|
||||
/// When set, restricts ChatGPT login to one or more workspace identifiers.
|
||||
#[serde(default)]
|
||||
pub forced_chatgpt_workspace_id: Option<String>,
|
||||
pub forced_chatgpt_workspace_id: Option<ForcedChatgptWorkspaceIds>,
|
||||
|
||||
/// When set, restricts the login mechanism users may use.
|
||||
#[serde(default)]
|
||||
@@ -413,7 +430,9 @@ impl From<ConfigToml> for UserSavedConfig {
|
||||
approval_policy: config_toml.approval_policy,
|
||||
sandbox_mode: config_toml.sandbox_mode,
|
||||
sandbox_settings: config_toml.sandbox_workspace_write.map(From::from),
|
||||
forced_chatgpt_workspace_id: config_toml.forced_chatgpt_workspace_id,
|
||||
forced_chatgpt_workspace_id: config_toml
|
||||
.forced_chatgpt_workspace_id
|
||||
.map(ForcedChatgptWorkspaceIds::into_vec),
|
||||
forced_login_method: config_toml.forced_login_method,
|
||||
model: config_toml.model,
|
||||
model_reasoning_effort: config_toml.model_reasoning_effort,
|
||||
|
||||
@@ -731,6 +731,20 @@
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"ForcedChatgptWorkspaceIds": {
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "string"
|
||||
},
|
||||
{
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
}
|
||||
],
|
||||
"description": "Backward-compatible shape for workspace restrictions in config.toml."
|
||||
},
|
||||
"ForcedLoginMethod": {
|
||||
"enum": [
|
||||
"chatgpt",
|
||||
@@ -2564,9 +2578,13 @@
|
||||
"description": "Optional URI-based file opener. If set, citations to files in the model output will be hyperlinked using the specified URI scheme."
|
||||
},
|
||||
"forced_chatgpt_workspace_id": {
|
||||
"allOf": [
|
||||
{
|
||||
"$ref": "#/definitions/ForcedChatgptWorkspaceIds"
|
||||
}
|
||||
],
|
||||
"default": null,
|
||||
"description": "When set, restricts ChatGPT login to a specific workspace identifier.",
|
||||
"type": "string"
|
||||
"description": "When set, restricts ChatGPT login to one or more workspace identifiers."
|
||||
},
|
||||
"forced_login_method": {
|
||||
"allOf": [
|
||||
|
||||
@@ -454,19 +454,24 @@ impl AgentIdentityBinding {
|
||||
}
|
||||
}
|
||||
|
||||
fn from_auth(auth: &CodexAuth, forced_workspace_id: Option<String>) -> Option<Self> {
|
||||
fn from_auth(auth: &CodexAuth, forced_workspace_id: Option<Vec<String>>) -> Option<Self> {
|
||||
if !auth.is_chatgpt_auth() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let token_data = auth.get_token_data().ok()?;
|
||||
let resolved_account_id =
|
||||
forced_workspace_id
|
||||
.filter(|value| !value.is_empty())
|
||||
.or(token_data
|
||||
.account_id
|
||||
.clone()
|
||||
.filter(|value| !value.is_empty()))?;
|
||||
let token_account_id = token_data
|
||||
.account_id
|
||||
.clone()
|
||||
.filter(|value| !value.is_empty());
|
||||
let forced_account_id = forced_workspace_id.and_then(|values| {
|
||||
let mut values = values.into_iter().filter(|value| !value.is_empty());
|
||||
match token_account_id.as_ref() {
|
||||
Some(account_id) => values.find(|value| value == account_id),
|
||||
None => values.next(),
|
||||
}
|
||||
});
|
||||
let resolved_account_id = forced_account_id.or(token_account_id)?;
|
||||
|
||||
Some(Self {
|
||||
binding_id: format!("chatgpt-account-{resolved_account_id}"),
|
||||
|
||||
@@ -50,6 +50,7 @@ pub(crate) async fn apply_patch(
|
||||
action,
|
||||
auto_approved: !user_explicitly_approved,
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -62,6 +63,7 @@ pub(crate) async fn apply_patch(
|
||||
action,
|
||||
auto_approved: false,
|
||||
exec_approval_requirement: ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
|
||||
@@ -123,6 +123,8 @@ pub use service::ConfigServiceError;
|
||||
|
||||
pub use codex_git_utils::GhostSnapshotConfig;
|
||||
|
||||
const DISABLE_MANAGED_CONFIG_ENV_VAR: &str = "CODEX_DISABLE_MANAGED_CONFIG";
|
||||
|
||||
/// Maximum number of bytes of the documentation that will be embedded. Larger
|
||||
/// files are *silently truncated* to this size so we do not take up too much of
|
||||
/// the context window.
|
||||
@@ -530,12 +532,11 @@ pub struct Config {
|
||||
/// instructions inserted into developer messages when realtime becomes
|
||||
/// active.
|
||||
pub experimental_realtime_start_instructions: Option<String>,
|
||||
|
||||
/// Experimental / do not use. When set, app-server uses a remote thread
|
||||
/// store at this endpoint instead of the local filesystem/SQLite store.
|
||||
pub experimental_thread_store_endpoint: Option<String>,
|
||||
/// When set, restricts ChatGPT login to a specific workspace identifier.
|
||||
pub forced_chatgpt_workspace_id: Option<String>,
|
||||
/// When set, restricts ChatGPT login to one or more workspace identifiers.
|
||||
pub forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
|
||||
/// When set, restricts the login mechanism users may use.
|
||||
pub forced_login_method: Option<ForcedLoginMethod>,
|
||||
@@ -634,7 +635,7 @@ impl AuthManagerConfig for Config {
|
||||
self.cli_auth_credentials_store_mode
|
||||
}
|
||||
|
||||
fn forced_chatgpt_workspace_id(&self) -> Option<String> {
|
||||
fn forced_chatgpt_workspace_id(&self) -> Option<Vec<String>> {
|
||||
self.forced_chatgpt_workspace_id.clone()
|
||||
}
|
||||
|
||||
@@ -737,7 +738,9 @@ impl ConfigBuilder {
|
||||
};
|
||||
let cli_overrides = cli_overrides.unwrap_or_default();
|
||||
let mut harness_overrides = harness_overrides.unwrap_or_default();
|
||||
let loader_overrides = loader_overrides.unwrap_or_default();
|
||||
let loader_overrides = loader_overrides
|
||||
.map(apply_debug_loader_overrides)
|
||||
.unwrap_or_else(|| apply_debug_loader_overrides(LoaderOverrides::default()));
|
||||
let cwd_override = harness_overrides.cwd.as_deref().or(fallback_cwd.as_deref());
|
||||
let cwd = match cwd_override {
|
||||
Some(path) => AbsolutePathBuf::relative_to_current_dir(path)?,
|
||||
@@ -794,6 +797,42 @@ impl ConfigBuilder {
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_debug_loader_overrides(mut loader_overrides: LoaderOverrides) -> LoaderOverrides {
|
||||
#[cfg(debug_assertions)]
|
||||
if disable_managed_config_from_debug_env() {
|
||||
if loader_overrides.managed_config_path.is_none() {
|
||||
loader_overrides.managed_config_path = Some(
|
||||
std::env::temp_dir()
|
||||
.join("codex-config-tests")
|
||||
.join("managed_config.toml"),
|
||||
);
|
||||
}
|
||||
#[cfg(target_os = "macos")]
|
||||
if loader_overrides.managed_preferences_base64.is_none() {
|
||||
loader_overrides.managed_preferences_base64 = Some(String::new());
|
||||
}
|
||||
if loader_overrides
|
||||
.macos_managed_config_requirements_base64
|
||||
.is_none()
|
||||
{
|
||||
loader_overrides.macos_managed_config_requirements_base64 = Some(String::new());
|
||||
}
|
||||
}
|
||||
|
||||
loader_overrides
|
||||
}
|
||||
|
||||
fn disable_managed_config_from_debug_env() -> bool {
|
||||
#[cfg(debug_assertions)]
|
||||
{
|
||||
if let Ok(value) = std::env::var(DISABLE_MANAGED_CONFIG_ENV_VAR) {
|
||||
return matches!(value.as_str(), "1" | "true" | "TRUE" | "yes" | "YES");
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
impl Config {
|
||||
pub fn to_models_manager_config(&self) -> ModelsManagerConfig {
|
||||
ModelsManagerConfig {
|
||||
@@ -1946,15 +1985,18 @@ impl Config {
|
||||
let include_apply_patch_tool_flag = features.enabled(Feature::ApplyPatchFreeform);
|
||||
let use_experimental_unified_exec_tool = features.enabled(Feature::UnifiedExec);
|
||||
|
||||
let forced_chatgpt_workspace_id =
|
||||
cfg.forced_chatgpt_workspace_id.as_ref().and_then(|value| {
|
||||
let trimmed = value.trim();
|
||||
if trimmed.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(trimmed.to_string())
|
||||
}
|
||||
});
|
||||
let forced_chatgpt_workspace_id = cfg
|
||||
.forced_chatgpt_workspace_id
|
||||
.clone()
|
||||
.map(codex_config::config_toml::ForcedChatgptWorkspaceIds::into_vec)
|
||||
.map(|values| {
|
||||
values
|
||||
.into_iter()
|
||||
.map(|value| value.trim().to_string())
|
||||
.filter(|value| !value.is_empty())
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.filter(|values| !values.is_empty());
|
||||
|
||||
let forced_login_method = cfg.forced_login_method;
|
||||
|
||||
|
||||
@@ -267,6 +267,7 @@ impl ExecPolicyManager {
|
||||
&exec_policy_fallback,
|
||||
&match_options,
|
||||
);
|
||||
let execpolicy_matched = evaluation.matched_rules.iter().any(is_policy_match);
|
||||
|
||||
let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule(
|
||||
prefix_rule.as_ref(),
|
||||
@@ -279,6 +280,7 @@ impl ExecPolicyManager {
|
||||
|
||||
match evaluation.decision {
|
||||
Decision::Forbidden => ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched,
|
||||
reason: derive_forbidden_reason(command, &evaluation),
|
||||
},
|
||||
Decision::Prompt => {
|
||||
@@ -287,9 +289,11 @@ impl ExecPolicyManager {
|
||||
});
|
||||
match prompt_is_rejected_by_policy(approval_policy, prompt_is_rule) {
|
||||
Some(reason) => ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched,
|
||||
reason: reason.to_string(),
|
||||
},
|
||||
None => ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched,
|
||||
reason: derive_prompt_reason(command, &evaluation),
|
||||
proposed_execpolicy_amendment: requested_amendment.or_else(|| {
|
||||
if auto_amendment_allowed {
|
||||
@@ -304,6 +308,7 @@ impl ExecPolicyManager {
|
||||
}
|
||||
}
|
||||
Decision::Allow => ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched,
|
||||
// Bypass sandbox only when every parsed command segment is
|
||||
// explicitly allowed by execpolicy.
|
||||
bypass_sandbox: commands.iter().all(|command| {
|
||||
|
||||
@@ -636,6 +636,7 @@ async fn evaluates_bash_lc_inner_commands() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: "`bash -lc 'rm -rf /some/important/folder'` rejected: policy forbids commands starting with `rm`".to_string(),
|
||||
},
|
||||
)
|
||||
@@ -718,6 +719,7 @@ async fn evaluates_heredoc_script_against_prefix_rules() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -742,6 +744,7 @@ async fn omits_auto_amendment_for_heredoc_fallback_prompts() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -770,6 +773,7 @@ async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_wont_mat
|
||||
]),
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -803,6 +807,7 @@ prefix_rule(
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: "`rm -rf /some/important/folder` rejected: destructive command".to_string(),
|
||||
},
|
||||
)
|
||||
@@ -822,6 +827,7 @@ async fn exec_approval_requirement_prefers_execpolicy_match() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: Some("`rm` requires approval by policy".to_string()),
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -850,6 +856,7 @@ prefix_rule(pattern=["git"], decision="allow")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -884,6 +891,7 @@ prefix_rule(pattern=["git"], decision="prompt")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
disallowed_git_path,
|
||||
@@ -911,6 +919,7 @@ async fn requested_prefix_rule_can_approve_absolute_path_commands() {
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"cargo".to_string(),
|
||||
@@ -934,6 +943,7 @@ async fn exec_approval_requirement_respects_approval_policy() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: PROMPT_CONFLICT_REASON.to_string(),
|
||||
},
|
||||
)
|
||||
@@ -998,6 +1008,7 @@ async fn exec_approval_requirement_prompts_for_inline_additional_permissions_und
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"touch".to_string(),
|
||||
@@ -1028,6 +1039,7 @@ async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_gra
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(),
|
||||
},
|
||||
)
|
||||
@@ -1105,6 +1117,7 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled(
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: REJECT_RULES_APPROVAL_REASON.to_string(),
|
||||
}
|
||||
);
|
||||
@@ -1129,6 +1142,7 @@ async fn exec_approval_requirement_falls_back_to_heuristics() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command))
|
||||
}
|
||||
@@ -1154,6 +1168,7 @@ async fn empty_bash_lc_script_falls_back_to_original_command() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
}
|
||||
@@ -1183,6 +1198,7 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
}
|
||||
@@ -1212,6 +1228,7 @@ async fn request_rule_uses_prefix_rule() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"cargo".to_string(),
|
||||
@@ -1244,6 +1261,7 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands(
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"rm".to_string(),
|
||||
@@ -1280,6 +1298,7 @@ async fn heuristics_apply_when_other_commands_match_policy() {
|
||||
})
|
||||
.await,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"orange".to_string()
|
||||
@@ -1354,6 +1373,7 @@ async fn proposed_execpolicy_amendment_is_present_for_single_command_without_pol
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
},
|
||||
@@ -1374,6 +1394,7 @@ async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: Some("`rm` requires approval by policy".to_string()),
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1398,6 +1419,7 @@ async fn proposed_execpolicy_amendment_is_present_for_multi_command_scripts() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"cargo".to_string(),
|
||||
@@ -1428,6 +1450,7 @@ async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scri
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: true,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
|
||||
"apple".to_string(),
|
||||
@@ -1452,6 +1475,7 @@ async fn proposed_execpolicy_amendment_is_present_when_heuristics_allow() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
},
|
||||
@@ -1472,6 +1496,7 @@ async fn proposed_execpolicy_amendment_is_suppressed_when_policy_matches_allow()
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1503,6 +1528,7 @@ prefix_rule(pattern=["cat"], decision="allow")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1535,6 +1561,7 @@ prefix_rule(pattern=["bash"], decision="allow")
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: true,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -1740,6 +1767,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
that no sandbox is present, so anything that is not "provably
|
||||
safe" should require approval."#,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: expected_amendment.clone(),
|
||||
},
|
||||
@@ -1748,6 +1776,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
(
|
||||
"On non-Windows, rely on the read-only sandbox to prevent harm.",
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: expected_amendment.clone(),
|
||||
},
|
||||
@@ -1772,6 +1801,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
let dangerous_command = vec_str(&["rm", "-rf", "/important/data"]);
|
||||
assert_eq!(
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec_str(&[
|
||||
"rm",
|
||||
@@ -1797,6 +1827,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
// AskForApproval::Never.
|
||||
assert_eq!(
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: "`rm -rf /important/data` rejected: blocked by policy".to_string(),
|
||||
},
|
||||
policy
|
||||
@@ -1830,6 +1861,7 @@ async fn dangerous_command_allowed_when_sandbox_is_explicitly_disabled() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment {
|
||||
command: vec_str(&["rm", "-rf", "/tmp/nonexistent"]),
|
||||
@@ -1855,6 +1887,7 @@ async fn dangerous_command_forbidden_in_external_sandbox_when_policy_matches() {
|
||||
prefix_rule: None,
|
||||
},
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: true,
|
||||
reason: "approval required by policy, but AskForApproval is set to Never".to_string(),
|
||||
},
|
||||
)
|
||||
|
||||
@@ -124,6 +124,7 @@ impl ToolOrchestrator {
|
||||
let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| {
|
||||
default_exec_approval_requirement(approval_policy, &turn_ctx.file_system_sandbox_policy)
|
||||
});
|
||||
let execpolicy_matched = requirement.execpolicy_matched();
|
||||
match requirement {
|
||||
ExecApprovalRequirement::Skip { .. } => {
|
||||
otel.tool_decision(
|
||||
@@ -131,9 +132,10 @@ impl ToolOrchestrator {
|
||||
otel_ci,
|
||||
&ReviewDecision::Approved,
|
||||
ToolDecisionSource::Config,
|
||||
Some(execpolicy_matched),
|
||||
);
|
||||
}
|
||||
ExecApprovalRequirement::Forbidden { reason } => {
|
||||
ExecApprovalRequirement::Forbidden { reason, .. } => {
|
||||
return Err(ToolError::Rejected(reason));
|
||||
}
|
||||
ExecApprovalRequirement::NeedsApproval { reason, .. } => {
|
||||
@@ -154,6 +156,7 @@ impl ToolOrchestrator {
|
||||
tool_ctx,
|
||||
use_guardian,
|
||||
&otel,
|
||||
Some(execpolicy_matched),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -387,6 +390,7 @@ impl ToolOrchestrator {
|
||||
tool_ctx: &ToolCtx,
|
||||
use_guardian: bool,
|
||||
otel: &codex_otel::SessionTelemetry,
|
||||
execpolicy_matched: Option<bool>,
|
||||
) -> Result<ReviewDecision, ToolError>
|
||||
where
|
||||
T: ToolRuntime<Rq, Out>,
|
||||
@@ -407,6 +411,7 @@ impl ToolOrchestrator {
|
||||
&tool_ctx.call_id,
|
||||
&decision,
|
||||
ToolDecisionSource::Config,
|
||||
execpolicy_matched,
|
||||
);
|
||||
return Ok(decision);
|
||||
}
|
||||
@@ -417,6 +422,7 @@ impl ToolOrchestrator {
|
||||
&tool_ctx.call_id,
|
||||
&decision,
|
||||
ToolDecisionSource::Config,
|
||||
execpolicy_matched,
|
||||
);
|
||||
return Err(ToolError::Rejected(message));
|
||||
}
|
||||
@@ -435,6 +441,7 @@ impl ToolOrchestrator {
|
||||
&tool_ctx.call_id,
|
||||
&decision,
|
||||
otel_source,
|
||||
execpolicy_matched,
|
||||
);
|
||||
Ok(decision)
|
||||
}
|
||||
|
||||
@@ -58,6 +58,7 @@ fn guardian_review_request_includes_patch_context() {
|
||||
},
|
||||
)]),
|
||||
exec_approval_requirement: ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -95,6 +96,7 @@ fn file_system_sandbox_context_uses_active_attempt() {
|
||||
file_paths: vec![path.clone()],
|
||||
changes: HashMap::new(),
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -151,6 +153,7 @@ fn file_system_sandbox_context_omits_legacy_equivalent_policy() {
|
||||
file_paths: vec![path.clone()],
|
||||
changes: HashMap::new(),
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -192,6 +195,7 @@ fn no_sandbox_attempt_has_no_file_system_context() {
|
||||
file_paths: vec![path.clone()],
|
||||
changes: HashMap::new(),
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
|
||||
@@ -143,6 +143,8 @@ pub(crate) struct PermissionRequestPayload {
|
||||
pub(crate) enum ExecApprovalRequirement {
|
||||
/// No approval required for this tool call.
|
||||
Skip {
|
||||
/// Whether at least one concrete execpolicy rule matched this command.
|
||||
execpolicy_matched: bool,
|
||||
/// The first attempt should skip sandboxing (e.g., when explicitly
|
||||
/// greenlit by policy).
|
||||
bypass_sandbox: bool,
|
||||
@@ -152,13 +154,19 @@ pub(crate) enum ExecApprovalRequirement {
|
||||
},
|
||||
/// Approval required for this tool call.
|
||||
NeedsApproval {
|
||||
/// Whether at least one concrete execpolicy rule matched this command.
|
||||
execpolicy_matched: bool,
|
||||
reason: Option<String>,
|
||||
/// Proposed execpolicy amendment to skip future approvals for similar commands
|
||||
/// See core/src/exec_policy.rs for more details on how proposed_execpolicy_amendment is determined.
|
||||
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
|
||||
},
|
||||
/// Execution forbidden for this tool call.
|
||||
Forbidden { reason: String },
|
||||
Forbidden {
|
||||
/// Whether at least one concrete execpolicy rule matched this command.
|
||||
execpolicy_matched: bool,
|
||||
reason: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl ExecApprovalRequirement {
|
||||
@@ -175,6 +183,20 @@ impl ExecApprovalRequirement {
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn execpolicy_matched(&self) -> bool {
|
||||
match self {
|
||||
Self::Skip {
|
||||
execpolicy_matched, ..
|
||||
}
|
||||
| Self::NeedsApproval {
|
||||
execpolicy_matched, ..
|
||||
}
|
||||
| Self::Forbidden {
|
||||
execpolicy_matched, ..
|
||||
} => *execpolicy_matched,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// - Never, OnFailure: do not ask
|
||||
@@ -205,15 +227,18 @@ pub(crate) fn default_exec_approval_requirement(
|
||||
)
|
||||
{
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: "approval policy disallowed sandbox approval prompt".to_string(),
|
||||
}
|
||||
} else if needs_approval {
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
} else {
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ fn external_sandbox_skips_exec_approval_on_request() {
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
||||
),
|
||||
ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
@@ -30,6 +31,7 @@ fn restricted_sandbox_requires_exec_approval_on_request() {
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy)
|
||||
),
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
@@ -53,6 +55,7 @@ fn default_exec_approval_requirement_rejects_sandbox_prompt_when_granular_disabl
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
execpolicy_matched: false,
|
||||
reason: "approval policy disallowed sandbox approval prompt".to_string(),
|
||||
}
|
||||
);
|
||||
@@ -75,6 +78,7 @@ fn default_exec_approval_requirement_keeps_prompt_when_granular_allows_sandbox_a
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
execpolicy_matched: false,
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
@@ -87,6 +91,7 @@ fn additional_permissions_allow_bypass_sandbox_first_attempt_when_execpolicy_ski
|
||||
sandbox_override_for_first_attempt(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
&ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
@@ -101,6 +106,7 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() {
|
||||
sandbox_override_for_first_attempt(
|
||||
SandboxPermissions::RequireEscalated,
|
||||
&ExecApprovalRequirement::Skip {
|
||||
execpolicy_matched: false,
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
|
||||
@@ -1040,10 +1040,12 @@ fn tool_decision_assertion<'a>(
|
||||
call_id: &'a str,
|
||||
expected_decision: &'a str,
|
||||
expected_source: &'a str,
|
||||
expected_execpolicy_matched: &'a str,
|
||||
) -> impl Fn(&[&str]) -> Result<(), String> + 'a {
|
||||
let call_id = call_id.to_string();
|
||||
let expected_decision = expected_decision.to_string();
|
||||
let expected_source = expected_source.to_string();
|
||||
let expected_execpolicy_matched = expected_execpolicy_matched.to_string();
|
||||
|
||||
move |lines: &[&str]| {
|
||||
let line = lines
|
||||
@@ -1063,6 +1065,11 @@ fn tool_decision_assertion<'a>(
|
||||
if !lower.contains(&format!("source={expected_source}")) {
|
||||
return Err(format!("unexpected source for {expected_source}"));
|
||||
}
|
||||
if !lower.contains(&format!("execpolicy_matched={expected_execpolicy_matched}")) {
|
||||
return Err(format!(
|
||||
"unexpected execpolicy_matched for {call_id}: expected {expected_execpolicy_matched}"
|
||||
));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -1122,6 +1129,7 @@ async fn handle_container_exec_autoapprove_from_config_records_tool_decision() {
|
||||
"auto_config_call",
|
||||
"approved",
|
||||
"config",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1189,6 +1197,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() {
|
||||
"user_approved_call",
|
||||
"approved",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1256,6 +1265,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision()
|
||||
"user_approved_session_call",
|
||||
"approvedforsession",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1323,6 +1333,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() {
|
||||
"sandbox_retry_call",
|
||||
"approved",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1390,6 +1401,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() {
|
||||
"user_denied_call",
|
||||
"denied",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1457,6 +1469,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision()
|
||||
"sandbox_session_call",
|
||||
"approvedforsession",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1525,5 +1538,6 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() {
|
||||
"sandbox_deny_call",
|
||||
"denied",
|
||||
"user",
|
||||
"false",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -605,7 +605,7 @@ async fn auth_manager_notifies_when_auth_state_changes() {
|
||||
.expect("auth reload notification should still arrive")
|
||||
.expect("auth state watch should remain open");
|
||||
|
||||
manager.set_forced_chatgpt_workspace_id(Some("workspace-123".to_string()));
|
||||
manager.set_forced_chatgpt_workspace_id(Some(vec!["workspace-123".to_string()]));
|
||||
timeout(Duration::from_secs(1), auth_state_rx.changed())
|
||||
.await
|
||||
.expect("workspace change notification should arrive")
|
||||
@@ -674,7 +674,7 @@ fn fake_jwt_for_auth_file_params(params: &AuthFileParams) -> std::io::Result<Str
|
||||
async fn build_config(
|
||||
codex_home: &Path,
|
||||
forced_login_method: Option<ForcedLoginMethod>,
|
||||
forced_chatgpt_workspace_id: Option<String>,
|
||||
forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
) -> AuthConfig {
|
||||
AuthConfig {
|
||||
codex_home: codex_home.to_path_buf(),
|
||||
@@ -754,13 +754,13 @@ async fn enforce_login_restrictions_logs_out_for_workspace_mismatch() {
|
||||
let config = build_config(
|
||||
codex_home.path(),
|
||||
/*forced_login_method*/ None,
|
||||
Some("org_mine".to_string()),
|
||||
Some(vec!["org_mine".to_string()]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let err = super::enforce_login_restrictions(&config)
|
||||
.expect_err("expected workspace mismatch to error");
|
||||
assert!(err.to_string().contains("workspace org_mine"));
|
||||
assert!(err.to_string().contains("workspace(s) org_mine"));
|
||||
assert!(
|
||||
!codex_home.path().join("auth.json").exists(),
|
||||
"auth.json should be removed on mismatch"
|
||||
@@ -784,7 +784,7 @@ async fn enforce_login_restrictions_allows_matching_workspace() {
|
||||
let config = build_config(
|
||||
codex_home.path(),
|
||||
/*forced_login_method*/ None,
|
||||
Some("org_mine".to_string()),
|
||||
Some(vec!["org_mine".to_string()]),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -795,6 +795,31 @@ async fn enforce_login_restrictions_allows_matching_workspace() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial(codex_api_key)]
|
||||
async fn enforce_login_restrictions_allows_any_matching_workspace_in_list() {
|
||||
let codex_home = tempdir().unwrap();
|
||||
let _jwt = write_auth_file(
|
||||
AuthFileParams {
|
||||
openai_api_key: None,
|
||||
chatgpt_plan_type: Some("pro".to_string()),
|
||||
chatgpt_account_id: Some("org_mine".to_string()),
|
||||
},
|
||||
codex_home.path(),
|
||||
)
|
||||
.expect("failed to write auth file");
|
||||
|
||||
let config = build_config(
|
||||
codex_home.path(),
|
||||
/*forced_login_method*/ None,
|
||||
Some(vec!["org_other".to_string(), "org_mine".to_string()]),
|
||||
)
|
||||
.await;
|
||||
|
||||
super::enforce_login_restrictions(&config)
|
||||
.expect("any matching workspace in the allowed list should succeed");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn enforce_login_restrictions_allows_api_key_if_login_method_not_set_but_forced_chatgpt_workspace_id_is_set()
|
||||
{
|
||||
@@ -805,7 +830,7 @@ async fn enforce_login_restrictions_allows_api_key_if_login_method_not_set_but_f
|
||||
let config = build_config(
|
||||
codex_home.path(),
|
||||
/*forced_login_method*/ None,
|
||||
Some("org_mine".to_string()),
|
||||
Some(vec!["org_mine".to_string()]),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -582,7 +582,7 @@ pub struct AuthConfig {
|
||||
pub codex_home: PathBuf,
|
||||
pub auth_credentials_store_mode: AuthCredentialsStoreMode,
|
||||
pub forced_login_method: Option<ForcedLoginMethod>,
|
||||
pub forced_chatgpt_workspace_id: Option<String>,
|
||||
pub forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
pub fn enforce_login_restrictions(config: &AuthConfig) -> std::io::Result<()> {
|
||||
@@ -620,7 +620,7 @@ pub fn enforce_login_restrictions(config: &AuthConfig) -> std::io::Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(expected_account_id) = config.forced_chatgpt_workspace_id.as_deref() {
|
||||
if let Some(expected_account_ids) = config.forced_chatgpt_workspace_id.as_deref() {
|
||||
if !auth.is_chatgpt_auth() {
|
||||
return Ok(());
|
||||
}
|
||||
@@ -640,13 +640,18 @@ pub fn enforce_login_restrictions(config: &AuthConfig) -> std::io::Result<()> {
|
||||
|
||||
// workspace is the external identifier for account id.
|
||||
let chatgpt_account_id = token_data.id_token.chatgpt_account_id.as_deref();
|
||||
if chatgpt_account_id != Some(expected_account_id) {
|
||||
if !chatgpt_account_id.is_some_and(|actual| {
|
||||
expected_account_ids
|
||||
.iter()
|
||||
.any(|expected| expected == actual)
|
||||
}) {
|
||||
let expected_workspaces = expected_account_ids.join(", ");
|
||||
let message = match chatgpt_account_id {
|
||||
Some(actual) => format!(
|
||||
"Login is restricted to workspace {expected_account_id}, but current credentials belong to {actual}. Logging out."
|
||||
"Login is restricted to workspace(s) {expected_workspaces}, but current credentials belong to {actual}. Logging out."
|
||||
),
|
||||
None => format!(
|
||||
"Login is restricted to workspace {expected_account_id}, but current credentials lack a workspace identifier. Logging out."
|
||||
"Login is restricted to workspace(s) {expected_workspaces}, but current credentials lack a workspace identifier. Logging out."
|
||||
),
|
||||
};
|
||||
return logout_with_message(
|
||||
@@ -1197,7 +1202,7 @@ pub struct AuthManager {
|
||||
inner: RwLock<CachedAuth>,
|
||||
enable_codex_api_key_env: bool,
|
||||
auth_credentials_store_mode: AuthCredentialsStoreMode,
|
||||
forced_chatgpt_workspace_id: RwLock<Option<String>>,
|
||||
forced_chatgpt_workspace_id: RwLock<Option<Vec<String>>>,
|
||||
chatgpt_base_url: RwLock<Option<String>>,
|
||||
background_agent_task_auth_mode: RwLock<BackgroundAgentTaskAuthMode>,
|
||||
refresh_lock: Semaphore,
|
||||
@@ -1218,8 +1223,8 @@ pub trait AuthManagerConfig {
|
||||
/// Returns the CLI auth credential storage mode for auth loading.
|
||||
fn cli_auth_credentials_store_mode(&self) -> AuthCredentialsStoreMode;
|
||||
|
||||
/// Returns the workspace ID that ChatGPT auth should be restricted to, if any.
|
||||
fn forced_chatgpt_workspace_id(&self) -> Option<String>;
|
||||
/// Returns the workspace IDs that ChatGPT auth should be restricted to, if any.
|
||||
fn forced_chatgpt_workspace_id(&self) -> Option<Vec<String>>;
|
||||
|
||||
/// Returns the ChatGPT backend base URL used for first-party backend authorization.
|
||||
fn chatgpt_base_url(&self) -> Option<String> {
|
||||
@@ -1512,7 +1517,7 @@ impl AuthManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn set_forced_chatgpt_workspace_id(&self, workspace_id: Option<String>) {
|
||||
pub fn set_forced_chatgpt_workspace_id(&self, workspace_id: Option<Vec<String>>) {
|
||||
if let Ok(mut guard) = self.forced_chatgpt_workspace_id.write()
|
||||
&& *guard != workspace_id
|
||||
{
|
||||
@@ -1521,7 +1526,7 @@ impl AuthManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn forced_chatgpt_workspace_id(&self) -> Option<String> {
|
||||
pub fn forced_chatgpt_workspace_id(&self) -> Option<Vec<String>> {
|
||||
self.forced_chatgpt_workspace_id
|
||||
.read()
|
||||
.ok()
|
||||
@@ -1913,13 +1918,13 @@ impl AuthManager {
|
||||
"external auth refresh did not return ChatGPT metadata",
|
||||
)));
|
||||
};
|
||||
if let Some(expected_workspace_id) = forced_chatgpt_workspace_id.as_deref()
|
||||
&& chatgpt_metadata.account_id != expected_workspace_id
|
||||
if let Some(expected_workspace_ids) = forced_chatgpt_workspace_id.as_deref()
|
||||
&& !expected_workspace_ids.contains(&chatgpt_metadata.account_id)
|
||||
{
|
||||
return Err(RefreshTokenError::Transient(std::io::Error::other(
|
||||
format!(
|
||||
"external auth refresh returned workspace {:?}, expected {expected_workspace_id:?}",
|
||||
chatgpt_metadata.account_id,
|
||||
"external auth refresh returned workspace {:?}, expected one of {:?}",
|
||||
chatgpt_metadata.account_id, expected_workspace_ids,
|
||||
),
|
||||
)));
|
||||
}
|
||||
|
||||
@@ -64,7 +64,7 @@ pub struct ServerOptions {
|
||||
pub port: u16,
|
||||
pub open_browser: bool,
|
||||
pub force_state: Option<String>,
|
||||
pub forced_chatgpt_workspace_id: Option<String>,
|
||||
pub forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
pub cli_auth_credentials_store_mode: AuthCredentialsStoreMode,
|
||||
}
|
||||
|
||||
@@ -73,7 +73,7 @@ impl ServerOptions {
|
||||
pub fn new(
|
||||
codex_home: PathBuf,
|
||||
client_id: String,
|
||||
forced_chatgpt_workspace_id: Option<String>,
|
||||
forced_chatgpt_workspace_id: Option<Vec<String>>,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode,
|
||||
) -> Self {
|
||||
Self {
|
||||
@@ -471,7 +471,7 @@ fn build_authorize_url(
|
||||
redirect_uri: &str,
|
||||
pkce: &PkceCodes,
|
||||
state: &str,
|
||||
forced_chatgpt_workspace_id: Option<&str>,
|
||||
forced_chatgpt_workspace_ids: Option<&[String]>,
|
||||
) -> String {
|
||||
let mut query = vec![
|
||||
("response_type".to_string(), "code".to_string()),
|
||||
@@ -492,8 +492,13 @@ fn build_authorize_url(
|
||||
("state".to_string(), state.to_string()),
|
||||
("originator".to_string(), originator().value),
|
||||
];
|
||||
if let Some(workspace_id) = forced_chatgpt_workspace_id {
|
||||
query.push(("allowed_workspace_id".to_string(), workspace_id.to_string()));
|
||||
if let Some(workspace_ids) = forced_chatgpt_workspace_ids {
|
||||
query.extend(
|
||||
workspace_ids
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|workspace_id| ("allowed_workspace_id".to_string(), workspace_id)),
|
||||
);
|
||||
}
|
||||
let qs = query
|
||||
.into_iter()
|
||||
@@ -870,7 +875,7 @@ fn jwt_auth_claims(jwt: &str) -> serde_json::Map<String, serde_json::Value> {
|
||||
|
||||
/// Validates the ID token against an optional workspace restriction.
|
||||
pub(crate) fn ensure_workspace_allowed(
|
||||
expected: Option<&str>,
|
||||
expected: Option<&[String]>,
|
||||
id_token: &str,
|
||||
) -> Result<(), String> {
|
||||
let Some(expected) = expected else {
|
||||
@@ -882,10 +887,13 @@ pub(crate) fn ensure_workspace_allowed(
|
||||
return Err("Login is restricted to a specific workspace, but the token did not include an chatgpt_account_id claim.".to_string());
|
||||
};
|
||||
|
||||
if actual == expected {
|
||||
if expected.iter().any(|workspace_id| workspace_id == actual) {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(format!("Login is restricted to workspace id {expected}."))
|
||||
Err(format!(
|
||||
"Login is restricted to workspace id(s) {}.",
|
||||
expected.join(", ")
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -183,7 +183,7 @@ async fn device_code_login_rejects_workspace_mismatch() -> anyhow::Result<()> {
|
||||
|
||||
let issuer = mock_server.uri();
|
||||
let mut opts = server_opts(&codex_home, issuer, AuthCredentialsStoreMode::File);
|
||||
opts.forced_chatgpt_workspace_id = Some("org-required".to_string());
|
||||
opts.forced_chatgpt_workspace_id = Some(vec!["org-required".to_string()]);
|
||||
|
||||
let err = run_device_code_login(opts)
|
||||
.await
|
||||
|
||||
@@ -117,7 +117,7 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> {
|
||||
port: 0,
|
||||
open_browser: false,
|
||||
force_state: Some(state),
|
||||
forced_chatgpt_workspace_id: Some(chatgpt_account_id.to_string()),
|
||||
forced_chatgpt_workspace_id: Some(vec![chatgpt_account_id.to_string()]),
|
||||
};
|
||||
let server = run_login_server(opts)?;
|
||||
assert!(
|
||||
@@ -198,6 +198,45 @@ async fn creates_missing_codex_home_dir() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn login_server_includes_all_forced_workspace_query_params() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let (issuer_addr, _issuer_handle) = start_mock_issuer("org-123");
|
||||
let issuer = format!("http://{}:{}", issuer_addr.ip(), issuer_addr.port());
|
||||
|
||||
let tmp = tempdir()?;
|
||||
let codex_home = tmp.path().to_path_buf();
|
||||
let state = "state-multi".to_string();
|
||||
|
||||
let opts = ServerOptions {
|
||||
codex_home,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
client_id: codex_login::CLIENT_ID.to_string(),
|
||||
issuer,
|
||||
port: 0,
|
||||
open_browser: false,
|
||||
force_state: Some(state),
|
||||
forced_chatgpt_workspace_id: Some(vec![
|
||||
"org-required-a".to_string(),
|
||||
"org-required-b".to_string(),
|
||||
]),
|
||||
};
|
||||
let server = run_login_server(opts)?;
|
||||
assert!(
|
||||
server
|
||||
.auth_url
|
||||
.contains("allowed_workspace_id=org-required-a")
|
||||
);
|
||||
assert!(
|
||||
server
|
||||
.auth_url
|
||||
.contains("allowed_workspace_id=org-required-b")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn forced_chatgpt_workspace_id_mismatch_blocks_login() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -217,7 +256,7 @@ async fn forced_chatgpt_workspace_id_mismatch_blocks_login() -> Result<()> {
|
||||
port: 0,
|
||||
open_browser: false,
|
||||
force_state: Some(state.clone()),
|
||||
forced_chatgpt_workspace_id: Some("org-required".to_string()),
|
||||
forced_chatgpt_workspace_id: Some(vec!["org-required".to_string()]),
|
||||
};
|
||||
let server = run_login_server(opts)?;
|
||||
assert!(
|
||||
@@ -234,7 +273,7 @@ async fn forced_chatgpt_workspace_id_mismatch_blocks_login() -> Result<()> {
|
||||
assert!(resp.status().is_success());
|
||||
let body = resp.text().await?;
|
||||
assert!(
|
||||
body.contains("Login is restricted to workspace id org-required"),
|
||||
body.contains("Login is restricted to workspace id(s) org-required"),
|
||||
"error body should mention workspace restriction"
|
||||
);
|
||||
|
||||
|
||||
@@ -867,6 +867,7 @@ impl SessionTelemetry {
|
||||
call_id: &str,
|
||||
decision: &ReviewDecision,
|
||||
source: ToolDecisionSource,
|
||||
execpolicy_matched: Option<bool>,
|
||||
) {
|
||||
log_event!(
|
||||
self,
|
||||
@@ -875,6 +876,9 @@ impl SessionTelemetry {
|
||||
call_id = %call_id,
|
||||
decision = %decision.clone().to_string().to_lowercase(),
|
||||
source = %source.to_string(),
|
||||
execpolicy_matched = %execpolicy_matched
|
||||
.map(|matched| matched.to_string())
|
||||
.unwrap_or_default(),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -16,7 +16,7 @@ pub(crate) struct LocalChatgptAuth {
|
||||
pub(crate) fn load_local_chatgpt_auth(
|
||||
codex_home: &Path,
|
||||
auth_credentials_store_mode: AuthCredentialsStoreMode,
|
||||
forced_chatgpt_workspace_id: Option<&str>,
|
||||
forced_chatgpt_workspace_id: Option<&[String]>,
|
||||
) -> Result<LocalChatgptAuth, String> {
|
||||
let auth = load_auth_dot_json(codex_home, auth_credentials_store_mode)
|
||||
.map_err(|err| format!("failed to load local auth: {err}"))?
|
||||
@@ -33,11 +33,11 @@ pub(crate) fn load_local_chatgpt_auth(
|
||||
.account_id
|
||||
.or(tokens.id_token.chatgpt_account_id.clone())
|
||||
.ok_or_else(|| "local ChatGPT auth is missing chatgpt account id".to_string())?;
|
||||
if let Some(expected_workspace) = forced_chatgpt_workspace_id
|
||||
&& chatgpt_account_id != expected_workspace
|
||||
if let Some(expected_workspaces) = forced_chatgpt_workspace_id
|
||||
&& !expected_workspaces.contains(&chatgpt_account_id)
|
||||
{
|
||||
return Err(format!(
|
||||
"local ChatGPT auth must use workspace {expected_workspace}, but found {chatgpt_account_id:?}"
|
||||
"local ChatGPT auth must use one of workspace(s) {expected_workspaces:?}, but found {chatgpt_account_id:?}",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -122,7 +122,7 @@ mod tests {
|
||||
let auth = load_local_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
AuthCredentialsStoreMode::File,
|
||||
Some("workspace-1"),
|
||||
Some(&["workspace-1".to_string()]),
|
||||
)
|
||||
.expect("chatgpt auth should load");
|
||||
|
||||
@@ -186,7 +186,7 @@ mod tests {
|
||||
let auth = load_local_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
AuthCredentialsStoreMode::File,
|
||||
Some("workspace-1"),
|
||||
Some(&["workspace-1".to_string(), "workspace-2".to_string()]),
|
||||
)
|
||||
.expect("managed auth should win");
|
||||
|
||||
@@ -202,7 +202,7 @@ mod tests {
|
||||
let auth = load_local_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
AuthCredentialsStoreMode::File,
|
||||
Some("workspace-1"),
|
||||
Some(&["workspace-1".to_string()]),
|
||||
)
|
||||
.expect("chatgpt auth should load");
|
||||
|
||||
|
||||
@@ -6281,7 +6281,7 @@ class Config(BaseModel):
|
||||
] = None
|
||||
compact_prompt: str | None = None
|
||||
developer_instructions: str | None = None
|
||||
forced_chatgpt_workspace_id: str | None = None
|
||||
forced_chatgpt_workspace_id: list[str] | None = None
|
||||
forced_login_method: ForcedLoginMethod | None = None
|
||||
instructions: str | None = None
|
||||
model: str | None = None
|
||||
|
||||
Reference in New Issue
Block a user