fix(auth): protect OAuth token caches (#10516)

This commit is contained in:
Jasper
2026-07-17 10:50:38 +02:00
committed by GitHub
parent 8168cb8378
commit 01ceb16b2b
8 changed files with 521 additions and 21 deletions
+1 -1
View File
@@ -229,7 +229,7 @@ subtle = { version = "2.5", default-features = false, features = ["std"] }
gethostname = "1.1.0"
[target.'cfg(target_os = "windows")'.dependencies]
winapi = { workspace = true }
winapi = { workspace = true, features = ["accctrl", "aclapi", "fileapi", "handleapi", "minwinbase", "sddl", "securitybaseapi", "winbase", "winerror"] }
keyring = { workspace = true, features = ["windows-native"], optional = true }
# Platform-specific GPU acceleration for Whisper and local inference
+29 -4
View File
@@ -3,6 +3,7 @@ use crate::conversation::message::{Message, MessageContent};
use crate::providers::api_client::{AuthProvider, RequestBuilderDecorator};
use crate::providers::base::{ConfigKey, MessageStream, Provider, ProviderDef, ProviderMetadata};
use crate::providers::openai_compatible::handle_status;
use crate::providers::private_file::write_private_file;
use crate::providers::retry::ProviderRetry;
use anyhow::{anyhow, Result};
use async_stream::try_stream;
@@ -347,11 +348,8 @@ impl TokenCache {
}
fn save(&self, token_data: &TokenData) -> Result<()> {
if let Some(parent) = self.cache_path.parent() {
std::fs::create_dir_all(parent)?;
}
let contents = serde_json::to_string(token_data)?;
std::fs::write(&self.cache_path, contents)?;
write_private_file(&self.cache_path, &contents)?;
Ok(())
}
@@ -1102,6 +1100,33 @@ mod tests {
assert!(TokenCache::new().has_token());
}
#[cfg(unix)]
#[test]
fn token_cache_replaces_loose_file_with_owner_only_permissions() {
use std::os::unix::fs::PermissionsExt;
let directory = tempfile::tempdir().unwrap();
let cache_path = directory.path().join("tokens.json");
std::fs::write(&cache_path, "{}").unwrap();
std::fs::set_permissions(&cache_path, std::fs::Permissions::from_mode(0o644)).unwrap();
let cache = TokenCache {
cache_path: cache_path.clone(),
};
cache
.save(&TokenData {
access_token: "access".to_string(),
refresh_token: "refresh".to_string(),
id_token: None,
expires_at: Utc::now() + chrono::Duration::hours(1),
account_id: None,
})
.unwrap();
let mode = std::fs::metadata(cache_path).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600);
}
#[test_case(
vec![
Message::user().with_text("user text"),
+30 -4
View File
@@ -7,6 +7,7 @@ use crate::providers::base::{
};
use crate::providers::formats::google::{create_request, response_to_streaming_message};
use crate::providers::google::GOOGLE_DOC_URL;
use crate::providers::private_file::write_private_file;
use goose_providers::errors::ProviderError;
use goose_providers::model::ModelConfig;
use goose_providers::request_log::{start_log, LoggerHandleExt};
@@ -172,11 +173,8 @@ impl TokenCache {
}
fn save(&self, data: &SetupData) -> Result<()> {
if let Some(parent) = self.cache_path.parent() {
std::fs::create_dir_all(parent)?;
}
let contents = serde_json::to_string(data)?;
std::fs::write(&self.cache_path, contents)?;
write_private_file(&self.cache_path, &contents)?;
Ok(())
}
@@ -1127,4 +1125,32 @@ mod tests {
assert!(cache.load().is_none());
assert!(!cache.has_token());
}
#[cfg(unix)]
#[test]
fn token_cache_replaces_loose_file_with_owner_only_permissions() {
use std::os::unix::fs::PermissionsExt;
let directory = tempfile::tempdir().unwrap();
let cache_path = directory.path().join("tokens.json");
std::fs::write(&cache_path, "{}").unwrap();
std::fs::set_permissions(&cache_path, std::fs::Permissions::from_mode(0o644)).unwrap();
let cache = TokenCache {
cache_path: cache_path.clone(),
};
cache
.save(&SetupData {
project_id: "project".to_string(),
token: TokenData {
access_token: "access".to_string(),
refresh_token: "refresh".to_string(),
expires_at: Utc::now() + chrono::Duration::hours(1),
},
})
.unwrap();
let mode = std::fs::metadata(cache_path).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600);
}
}
+39 -4
View File
@@ -4,6 +4,7 @@ use crate::providers::oauth_device_flow::{run_device_flow, DeviceFlowConfig, Req
use crate::providers::openai_compatible::{
handle_status, stream_openai_compat, stream_responses_compat,
};
use crate::providers::private_file::write_private_file;
use anyhow::{anyhow, Context, Result};
use async_trait::async_trait;
use axum::http;
@@ -170,11 +171,9 @@ impl DiskCache {
}
async fn save(&self, info: &CopilotState) -> Result<()> {
if let Some(parent) = self.cache_path.parent() {
tokio::fs::create_dir_all(parent).await?;
}
let contents = serde_json::to_string(info)?;
tokio::fs::write(&self.cache_path, contents).await?;
let cache_path = self.cache_path.clone();
tokio::task::spawn_blocking(move || write_private_file(&cache_path, &contents)).await??;
Ok(())
}
@@ -712,6 +711,42 @@ mod tests {
use super::*;
use serde_json::json;
#[cfg(unix)]
#[tokio::test]
async fn disk_cache_saves_owner_only_file() {
use std::os::unix::fs::{MetadataExt, PermissionsExt};
let directory = tempfile::tempdir().unwrap();
let cache_path = directory.path().join("info.json");
std::fs::write(&cache_path, "old-secret").unwrap();
std::fs::set_permissions(&cache_path, std::fs::Permissions::from_mode(0o644)).unwrap();
let cache = DiskCache {
cache_path: cache_path.clone(),
};
let state = CopilotState {
expires_at: Utc::now(),
info: CopilotTokenInfo {
token: "copilot-secret".to_string(),
expires_at: 1,
refresh_in: 1,
endpoints: CopilotTokenEndpoints {
api: "https://api.githubcopilot.com".to_string(),
_extra: HashMap::new(),
},
_extra: HashMap::new(),
},
};
cache.save(&state).await.unwrap();
let metadata = std::fs::metadata(&cache_path).unwrap();
assert_eq!(metadata.mode() & 0o777, 0o600);
let saved: CopilotState =
serde_json::from_str(&std::fs::read_to_string(cache_path).unwrap()).unwrap();
assert_eq!(saved.info.token, "copilot-secret");
}
#[test]
fn responses_models_routed_correctly() {
assert!(is_openai_responses_model("gpt-5.5"));
+1
View File
@@ -68,6 +68,7 @@ pub mod openai_compatible {
}
pub mod openrouter;
pub mod pi_acp;
mod private_file;
pub mod provider_registry;
pub mod provider_secrets;
pub mod provider_test;
+27 -4
View File
@@ -1,4 +1,5 @@
use crate::config::paths::Paths;
use crate::providers::private_file::write_private_file;
use crate::utils::bytes_to_hex;
use anyhow::Result;
use axum::{extract::Query, response::Html, routing::get, Router};
@@ -88,11 +89,8 @@ impl TokenCache {
}
fn save_token(&self, token_data: &TokenData) -> Result<()> {
if let Some(parent) = self.cache_path.parent() {
fs::create_dir_all(parent)?;
}
let contents = serde_json::to_string(token_data)?;
fs::write(&self.cache_path, contents)?;
write_private_file(&self.cache_path, &contents)?;
Ok(())
}
}
@@ -546,6 +544,31 @@ mod tests {
Ok(())
}
#[cfg(unix)]
#[test]
fn token_cache_replaces_loose_file_with_owner_only_permissions() {
use std::os::unix::fs::PermissionsExt;
let directory = tempfile::tempdir().unwrap();
let cache_path = directory.path().join("token.json");
fs::write(&cache_path, "{}").unwrap();
fs::set_permissions(&cache_path, fs::Permissions::from_mode(0o644)).unwrap();
let cache = TokenCache {
cache_path: cache_path.clone(),
};
cache
.save_token(&TokenData {
access_token: "access".to_string(),
refresh_token: Some("refresh".to_string()),
expires_at: None,
})
.unwrap();
let mode = fs::metadata(cache_path).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600);
}
#[test]
fn test_extract_token_data() -> Result<()> {
let endpoints = OidcEndpoints {
+366
View File
@@ -0,0 +1,366 @@
use std::io::{self, Write};
use std::path::Path;
#[cfg(windows)]
fn to_windows_api_path(path: &Path) -> io::Result<Vec<u16>> {
use std::os::windows::ffi::OsStrExt;
const LEGACY_MAX_PATH: usize = 248;
const SEP: u16 = b'\\' as u16;
const ALT_SEP: u16 = b'/' as u16;
const QUERY: u16 = b'?' as u16;
const COLON: u16 = b':' as u16;
const DOT: u16 = b'.' as u16;
const VERBATIM_PREFIX: &[u16] = &[SEP, SEP, QUERY, SEP];
const NT_PREFIX: &[u16] = &[SEP, QUERY, QUERY, SEP];
const UNC_PREFIX: &[u16] = &[
SEP,
SEP,
QUERY,
SEP,
b'U' as u16,
b'N' as u16,
b'C' as u16,
SEP,
];
let encode = |path: &Path| -> io::Result<Vec<u16>> {
let mut encoded: Vec<u16> = path.as_os_str().encode_wide().collect();
if encoded.contains(&0) {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"Windows paths cannot contain null characters",
));
}
encoded.push(0);
Ok(encoded)
};
let encoded = encode(path)?;
if encoded.starts_with(VERBATIM_PREFIX)
|| encoded.starts_with(NT_PREFIX)
|| encoded.as_slice() == [0]
{
return Ok(encoded);
}
if encoded.len() < LEGACY_MAX_PATH {
match encoded.as_slice() {
[drive, COLON, 0] | [drive, COLON, SEP | ALT_SEP, ..]
if *drive != SEP && *drive != ALT_SEP =>
{
return Ok(encoded);
}
[SEP | ALT_SEP, SEP | ALT_SEP, ..] => return Ok(encoded),
_ => {}
}
}
let absolute = std::path::absolute(path)?;
let encoded = encode(&absolute)?;
let (prefix, suffix) = match encoded.as_slice() {
[_, COLON, SEP, ..] => (VERBATIM_PREFIX, encoded.as_slice()),
[SEP, SEP, DOT, SEP, rest @ ..] => (VERBATIM_PREFIX, rest),
[SEP, SEP, QUERY, SEP, ..] | [SEP, QUERY, QUERY, SEP, ..] => (&[][..], encoded.as_slice()),
[SEP, SEP, rest @ ..] => (UNC_PREFIX, rest),
_ => (&[][..], encoded.as_slice()),
};
let mut normalized = Vec::with_capacity(prefix.len() + suffix.len());
normalized.extend_from_slice(prefix);
normalized.extend_from_slice(suffix);
Ok(normalized)
}
#[cfg(windows)]
fn create_owner_only_file(path: &Path) -> io::Result<std::fs::File> {
use std::os::windows::io::{FromRawHandle, RawHandle};
use std::ptr;
use winapi::shared::minwindef::HLOCAL;
use winapi::shared::sddl::{
ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1,
};
use winapi::um::fileapi::{CreateFileW, CREATE_NEW};
use winapi::um::handleapi::INVALID_HANDLE_VALUE;
use winapi::um::minwinbase::SECURITY_ATTRIBUTES;
use winapi::um::winbase::LocalFree;
use winapi::um::winnt::{
FILE_ATTRIBUTE_TEMPORARY, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE,
GENERIC_READ, GENERIC_WRITE, PSECURITY_DESCRIPTOR,
};
let sddl: Vec<u16> = "D:P(A;;FA;;;OW)\0".encode_utf16().collect();
let mut descriptor: PSECURITY_DESCRIPTOR = ptr::null_mut();
if unsafe {
ConvertStringSecurityDescriptorToSecurityDescriptorW(
sddl.as_ptr(),
SDDL_REVISION_1 as u32,
&mut descriptor,
ptr::null_mut(),
)
} == 0
{
return Err(io::Error::last_os_error());
}
let mut security_attributes = SECURITY_ATTRIBUTES {
nLength: std::mem::size_of::<SECURITY_ATTRIBUTES>() as u32,
lpSecurityDescriptor: descriptor,
bInheritHandle: 0,
};
let path = to_windows_api_path(path)?;
let handle = unsafe {
CreateFileW(
path.as_ptr(),
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
&mut security_attributes,
CREATE_NEW,
FILE_ATTRIBUTE_TEMPORARY,
ptr::null_mut(),
)
};
let error = (handle == INVALID_HANDLE_VALUE).then(io::Error::last_os_error);
unsafe {
LocalFree(descriptor as HLOCAL);
}
if let Some(error) = error {
Err(error)
} else {
Ok(unsafe { std::fs::File::from_raw_handle(handle as RawHandle) })
}
}
#[cfg(windows)]
fn create_private_temporary_file(parent: &Path) -> io::Result<tempfile::NamedTempFile> {
tempfile::Builder::new().make_in(parent, create_owner_only_file)
}
#[cfg(not(windows))]
fn create_private_temporary_file(parent: &Path) -> io::Result<tempfile::NamedTempFile> {
tempfile::NamedTempFile::new_in(parent)
}
#[cfg(windows)]
fn persist_private_temporary_file(
temporary: tempfile::NamedTempFile,
path: &Path,
) -> io::Result<()> {
use winapi::um::fileapi::SetFileAttributesW;
use winapi::um::winbase::{MoveFileExW, MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH};
use winapi::um::winnt::{FILE_ATTRIBUTE_NORMAL, FILE_ATTRIBUTE_TEMPORARY};
let temporary_path = to_windows_api_path(temporary.path())?;
let destination_path = to_windows_api_path(path)?;
if unsafe { SetFileAttributesW(temporary_path.as_ptr(), FILE_ATTRIBUTE_NORMAL) } == 0 {
return Err(io::Error::last_os_error());
}
if unsafe {
MoveFileExW(
temporary_path.as_ptr(),
destination_path.as_ptr(),
MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH,
)
} == 0
{
let error = io::Error::last_os_error();
unsafe {
SetFileAttributesW(temporary_path.as_ptr(), FILE_ATTRIBUTE_TEMPORARY);
}
return Err(error);
}
let (_file, mut temporary_path) = temporary.into_parts();
temporary_path.disable_cleanup(true);
Ok(())
}
#[cfg(not(windows))]
fn persist_private_temporary_file(
temporary: tempfile::NamedTempFile,
path: &Path,
) -> io::Result<()> {
temporary
.persist(path)
.map(|_| ())
.map_err(|error| error.error)
}
pub(crate) fn write_private_file(path: &Path, contents: &str) -> io::Result<()> {
let parent = path.parent().ok_or_else(|| {
io::Error::new(
io::ErrorKind::InvalidInput,
"private file path must have a parent directory",
)
})?;
std::fs::create_dir_all(parent)?;
let mut temporary = create_private_temporary_file(parent)?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
temporary
.as_file()
.set_permissions(std::fs::Permissions::from_mode(0o600))?;
}
temporary.write_all(contents.as_bytes())?;
temporary.as_file().sync_all()?;
persist_private_temporary_file(temporary, path)?;
Ok(())
}
#[cfg(all(test, unix))]
mod tests {
use super::*;
use std::fs::File;
use std::os::unix::fs::{MetadataExt, PermissionsExt};
#[test]
fn replaces_loose_existing_file_with_private_inode() {
let directory = tempfile::tempdir().unwrap();
let path = directory.path().join("token.json");
std::fs::write(&path, "old").unwrap();
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
let old_file = File::open(&path).unwrap();
let old_inode = old_file.metadata().unwrap().ino();
write_private_file(&path, "new-secret").unwrap();
let metadata = std::fs::metadata(&path).unwrap();
assert_ne!(metadata.ino(), old_inode);
assert_eq!(metadata.permissions().mode() & 0o777, 0o600);
assert_eq!(std::fs::read_to_string(path).unwrap(), "new-secret");
}
}
#[cfg(all(test, windows))]
mod windows_tests {
use super::*;
use std::ffi::c_void;
use std::fs::File;
use std::os::windows::io::AsRawHandle;
use std::ptr;
use winapi::shared::minwindef::HLOCAL;
use winapi::shared::winerror::ERROR_SUCCESS;
use winapi::um::accctrl::SE_FILE_OBJECT;
use winapi::um::aclapi::GetSecurityInfo;
use winapi::um::securitybaseapi::{
CreateWellKnownSid, EqualSid, GetAce, GetSecurityDescriptorControl,
};
use winapi::um::winbase::LocalFree;
use winapi::um::winnt::{
WinCreatorOwnerRightsSid, ACCESS_ALLOWED_ACE, ACCESS_ALLOWED_ACE_TYPE,
DACL_SECURITY_INFORMATION, FILE_ALL_ACCESS, OWNER_SECURITY_INFORMATION, PACL,
PSECURITY_DESCRIPTOR, PSID, SECURITY_MAX_SID_SIZE, SE_DACL_PROTECTED,
};
fn assert_owner_only_protected_dacl(file: &File) {
let mut owner: PSID = ptr::null_mut();
let mut dacl: PACL = ptr::null_mut();
let mut descriptor: PSECURITY_DESCRIPTOR = ptr::null_mut();
let status = unsafe {
GetSecurityInfo(
file.as_raw_handle(),
SE_FILE_OBJECT,
OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION,
&mut owner,
ptr::null_mut(),
&mut dacl,
ptr::null_mut(),
&mut descriptor,
)
};
assert_eq!(status, ERROR_SUCCESS);
let mut control = 0;
let mut revision = 0;
assert_ne!(
unsafe { GetSecurityDescriptorControl(descriptor, &mut control, &mut revision) },
0
);
assert_ne!(control & SE_DACL_PROTECTED, 0);
assert!(!dacl.is_null());
assert_eq!(unsafe { (*dacl).AceCount }, 1);
let mut ace: *mut c_void = ptr::null_mut();
assert_ne!(unsafe { GetAce(dacl, 0, &mut ace) }, 0);
let allowed = ace.cast::<ACCESS_ALLOWED_ACE>();
assert_eq!(
unsafe { (*allowed).Header.AceType },
ACCESS_ALLOWED_ACE_TYPE
);
assert_eq!(
unsafe { (*allowed).Mask } & FILE_ALL_ACCESS,
FILE_ALL_ACCESS
);
let mut expected_sid = [0u8; SECURITY_MAX_SID_SIZE];
let mut expected_sid_size = expected_sid.len() as u32;
assert_ne!(
unsafe {
CreateWellKnownSid(
WinCreatorOwnerRightsSid,
ptr::null_mut(),
expected_sid.as_mut_ptr().cast(),
&mut expected_sid_size,
)
},
0
);
let actual_sid = unsafe { &mut (*allowed).SidStart as *mut u32 as PSID };
assert_ne!(
unsafe { EqualSid(actual_sid, expected_sid.as_mut_ptr().cast()) },
0
);
unsafe {
LocalFree(descriptor as HLOCAL);
}
}
#[test]
fn creates_temporary_file_with_owner_only_protected_dacl() {
let directory = tempfile::tempdir().unwrap();
let temporary = create_private_temporary_file(directory.path()).unwrap();
assert_owner_only_protected_dacl(temporary.as_file());
}
#[test]
fn normalizes_long_windows_paths_to_verbatim_form() {
let path = std::path::PathBuf::from(format!(r"C:\{}", "a".repeat(250)));
let encoded = to_windows_api_path(&path).unwrap();
let prefix: Vec<u16> = r"\\?\C:\".encode_utf16().collect();
assert!(encoded.starts_with(&prefix));
assert_eq!(encoded.last(), Some(&0));
}
#[test]
fn writes_owner_only_protected_dacl() {
let directory = tempfile::tempdir().unwrap();
let path = directory.path().join("token.json");
write_private_file(&path, "new-secret").unwrap();
let file = File::open(&path).unwrap();
assert_owner_only_protected_dacl(&file);
assert_eq!(std::fs::read_to_string(path).unwrap(), "new-secret");
}
#[test]
fn writes_owner_only_file_beyond_legacy_path_limit() {
let directory = tempfile::tempdir().unwrap();
let path = directory
.path()
.join("a".repeat(120))
.join("b".repeat(120))
.join("token.json");
write_private_file(&path, "new-secret").unwrap();
let file = File::open(&path).unwrap();
assert_owner_only_protected_dacl(&file);
assert_eq!(std::fs::read_to_string(path).unwrap(), "new-secret");
}
}
+28 -4
View File
@@ -4,6 +4,7 @@ use super::openai_compatible::OpenAiCompatibleProvider;
use super::xai::{XAI_API_HOST, XAI_DEFAULT_MODEL, XAI_KNOWN_MODELS};
use crate::config::paths::Paths;
use crate::conversation::message::Message;
use crate::providers::private_file::write_private_file;
use anyhow::{anyhow, Result};
use async_trait::async_trait;
use axum::{extract::Query, response::Html, routing::get, Router};
@@ -123,11 +124,8 @@ impl TokenCache {
}
fn save(&self, token_data: &TokenData) -> Result<()> {
if let Some(parent) = self.cache_path.parent() {
std::fs::create_dir_all(parent)?;
}
let contents = serde_json::to_string(token_data)?;
std::fs::write(&self.cache_path, contents)?;
write_private_file(&self.cache_path, &contents)?;
Ok(())
}
@@ -873,4 +871,30 @@ mod tests {
);
assert!(s.ends_with("tokens.json"));
}
#[cfg(unix)]
#[test]
fn token_cache_replaces_loose_file_with_owner_only_permissions() {
use std::os::unix::fs::PermissionsExt;
let directory = tempfile::tempdir().unwrap();
let cache_path = directory.path().join("tokens.json");
std::fs::write(&cache_path, "{}").unwrap();
std::fs::set_permissions(&cache_path, std::fs::Permissions::from_mode(0o644)).unwrap();
let cache = TokenCache {
cache_path: cache_path.clone(),
};
cache
.save(&TokenData {
access_token: "access".to_string(),
refresh_token: "refresh".to_string(),
id_token: None,
expires_at: Utc::now() + chrono::Duration::hours(1),
})
.unwrap();
let mode = std::fs::metadata(cache_path).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600);
}
}