mirror of
https://github.com/openai/codex.git
synced 2026-09-13 11:47:17 +00:00
codex: harden API provisioning secret writes
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user