diff --git a/codex-rs/login/src/api_provision.rs b/codex-rs/login/src/api_provision.rs index f018dc343d..50a3c775db 100644 --- a/codex-rs/login/src/api_provision.rs +++ b/codex-rs/login/src/api_provision.rs @@ -217,14 +217,19 @@ async fn exchange_authorization_code_for_tokens( client .request(Method::POST, &url) .header(reqwest::header::ACCEPT, "application/json") + .header( + reqwest::header::CONTENT_TYPE, + "application/x-www-form-urlencoded", + ) .header(reqwest::header::USER_AGENT, USER_AGENT) - .json(&serde_json::json!({ - "client_id": client_id, - "code_verifier": code_verifier, - "code": code, - "grant_type": "authorization_code", - "redirect_uri": redirect_uri, - })), + .body(format!( + "client_id={}&code_verifier={}&code={}&grant_type={}&redirect_uri={}", + urlencoding::encode(client_id), + urlencoding::encode(code_verifier), + urlencoding::encode(code), + urlencoding::encode("authorization_code"), + urlencoding::encode(redirect_uri) + )), "POST", &url, ) diff --git a/codex-rs/login/src/api_provision_tests.rs b/codex-rs/login/src/api_provision_tests.rs index 84f9ee3055..9d42d83065 100644 --- a/codex-rs/login/src/api_provision_tests.rs +++ b/codex-rs/login/src/api_provision_tests.rs @@ -4,7 +4,8 @@ use serde_json::json; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; -use wiremock::matchers::body_json; +use wiremock::matchers::body_string_contains; +use wiremock::matchers::header; use wiremock::matchers::method; use wiremock::matchers::path; use wiremock::matchers::query_param; @@ -62,13 +63,14 @@ async fn provision_from_authorization_code_provisions_api_key() { let server = MockServer::start().await; Mock::given(method("POST")) .and(path("/oauth/token")) - .and(body_json(json!({ - "client_id": "client-123", - "code_verifier": "verifier-123", - "code": "auth-code-123", - "grant_type": "authorization_code", - "redirect_uri": "http://localhost:5000/auth/callback", - }))) + .and(header("content-type", "application/x-www-form-urlencoded")) + .and(body_string_contains("client_id=client-123")) + .and(body_string_contains("code_verifier=verifier-123")) + .and(body_string_contains("code=auth-code-123")) + .and(body_string_contains("grant_type=authorization_code")) + .and(body_string_contains( + "redirect_uri=http%3A%2F%2Flocalhost%3A5000%2Fauth%2Fcallback", + )) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "id_token": "id-token-123", "access_token": "oauth-access-123", diff --git a/codex-rs/tui/src/chatwidget/dotenv_api_key.rs b/codex-rs/tui/src/chatwidget/dotenv_api_key.rs index 7717cf43b8..0d9ba67af2 100644 --- a/codex-rs/tui/src/chatwidget/dotenv_api_key.rs +++ b/codex-rs/tui/src/chatwidget/dotenv_api_key.rs @@ -12,9 +12,16 @@ use codex_login::OPENAI_API_KEY_ENV_VAR; pub(super) fn validate_dotenv_target(path: &Path) -> io::Result<()> { ensure_parent_dir(path)?; + reject_symlink(path)?; if path.exists() { - OpenOptions::new().append(true).open(path)?; + let mut options = OpenOptions::new(); + options.append(true); + #[cfg(unix)] + { + options.custom_flags(libc::O_NOFOLLOW); + } + options.open(path)?; return Ok(()); } @@ -22,6 +29,7 @@ pub(super) fn validate_dotenv_target(path: &Path) -> io::Result<()> { options.write(true).create_new(true); #[cfg(unix)] { + options.custom_flags(libc::O_NOFOLLOW); options.mode(0o600); } options.open(path)?; @@ -37,6 +45,7 @@ pub(super) fn upsert_dotenv_api_key(path: &Path, api_key: &str) -> io::Result<() } ensure_parent_dir(path)?; + reject_symlink(path)?; let existing = match std::fs::read_to_string(path) { Ok(contents) => contents, @@ -70,10 +79,13 @@ pub(super) fn upsert_dotenv_api_key(path: &Path, api_key: &str) -> io::Result<() } fn write_dotenv_file(path: &Path, contents: &str) -> io::Result<()> { + reject_symlink(path)?; + let mut options = OpenOptions::new(); options.write(true).create(true).truncate(true); #[cfg(unix)] { + options.custom_flags(libc::O_NOFOLLOW); options.mode(0o600); } @@ -89,6 +101,23 @@ fn write_dotenv_file(path: &Path, contents: &str) -> io::Result<()> { Ok(()) } +fn reject_symlink(path: &Path) -> io::Result<()> { + let metadata = match std::fs::symlink_metadata(path) { + Ok(metadata) => metadata, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(()), + Err(err) => return Err(err), + }; + + if metadata.file_type().is_symlink() { + return Err(io::Error::new( + ErrorKind::InvalidInput, + ".env.local must not be a symlink", + )); + } + + Ok(()) +} + fn ensure_parent_dir(path: &Path) -> io::Result<()> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; @@ -154,6 +183,22 @@ mod tests { assert_eq!(mode, 0o600); } + #[cfg(unix)] + #[test] + fn upsert_rejects_symlink_target() { + let temp_dir = tempdir().expect("tempdir"); + let dotenv_path = temp_dir.path().join(".env"); + let target_path = temp_dir.path().join("target.env"); + std::fs::write(&target_path, "OTHER=value\n").expect("seed target"); + std::os::unix::fs::symlink(&target_path, &dotenv_path).expect("symlink"); + + let err = upsert_dotenv_api_key(&dotenv_path, "sk-test-key").expect_err("reject symlink"); + + assert_eq!(err.kind(), ErrorKind::InvalidInput); + let target = std::fs::read_to_string(&target_path).expect("read target"); + assert_eq!(target, "OTHER=value\n"); + } + #[test] fn upsert_replaces_existing_api_key_and_collapses_duplicates() { let temp_dir = tempdir().expect("tempdir");