From 63086c7e73bded3b1e367bfa3d2e0ece4eb3b416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 20 Feb 2026 10:24:51 +0100 Subject: [PATCH 1/6] feat(auth): encrypt Copilot token cache at rest --- .../agent-runtime/src/providers/copilot.rs | 138 +++++++++++------- clients/web/apps/dashboard/postcss.config.js | 5 - 2 files changed, 85 insertions(+), 58 deletions(-) delete mode 100644 clients/web/apps/dashboard/postcss.config.js diff --git a/clients/agent-runtime/src/providers/copilot.rs b/clients/agent-runtime/src/providers/copilot.rs index 74da5fdd4..0d2382baf 100755 --- a/clients/agent-runtime/src/providers/copilot.rs +++ b/clients/agent-runtime/src/providers/copilot.rs @@ -15,6 +15,7 @@ use crate::providers::traits::{ ChatMessage, ChatRequest as ProviderChatRequest, ChatResponse as ProviderChatResponse, Provider, ToolCall as ProviderToolCall, }; +use crate::security::SecretStore; use crate::tools::ToolSpec; use async_trait::async_trait; use reqwest::Client; @@ -169,13 +170,14 @@ pub struct CopilotProvider { /// preventing duplicate device flow prompts or redundant API calls. refresh_lock: Arc>>, http: Client, + secret_store: SecretStore, token_dir: PathBuf, } impl CopilotProvider { pub fn new(github_token: Option<&str>) -> Self { - let token_dir = directories::ProjectDirs::from("", "", "corvus") - .map(|dir| dir.config_dir().join("copilot")) + let corvus_dir = directories::ProjectDirs::from("", "", "corvus") + .map(|dir| dir.config_dir().to_path_buf()) .unwrap_or_else(|| { // Fall back to a user-specific temp directory to avoid // shared-directory symlink attacks. @@ -184,6 +186,7 @@ impl CopilotProvider { .unwrap_or_else(|_| "unknown".to_string()); std::env::temp_dir().join(format!("corvus-copilot-{user}")) }); + let token_dir = corvus_dir.join("copilot"); if let Err(err) = std::fs::create_dir_all(&token_dir) { warn!( @@ -216,10 +219,57 @@ impl CopilotProvider { .connect_timeout(Duration::from_secs(10)) .build() .unwrap_or_else(|_| Client::new()), + secret_store: SecretStore::new(&corvus_dir, true), token_dir, } } + async fn read_token_file_secure(&self, path: &Path) -> Option { + let data = tokio::fs::read_to_string(path).await.ok()?; + let value = data.trim(); + if value.is_empty() { + return None; + } + + match self.secret_store.decrypt(value) { + Ok(decrypted) => { + let token = decrypted.trim(); + if token.is_empty() { + None + } else { + Some(token.to_string()) + } + } + Err(err) => { + warn!("Failed to decrypt Copilot token file {:?}: {err}", path); + None + } + } + } + + async fn write_token_file_secure(&self, path: &Path, content: &str) { + let path = path.to_path_buf(); + let write_path = path.clone(); + let content = content.to_string(); + let secret_store = self.secret_store.clone(); + + let result = tokio::task::spawn_blocking(move || -> anyhow::Result<()> { + let encrypted = secret_store.encrypt(&content)?; + write_file_secure_blocking(&write_path, &encrypted)?; + Ok(()) + }) + .await; + + match result { + Ok(Ok(())) => {} + Ok(Err(err)) => warn!( + "Failed to write secure Copilot token file {:?}: {err}", + path + ), + Err(err) => warn!("Failed to spawn token write task for {:?}: {err}", path), + } + } + /// Required headers for Copilot API requests (editor identification). const COPILOT_HEADERS: [(&str, &str); 4] = [ ("Editor-Version", "vscode/1.85.1"), @@ -430,15 +480,13 @@ impl CopilotProvider { } let access_token_path = self.token_dir.join("access-token"); - if let Ok(cached) = tokio::fs::read_to_string(&access_token_path).await { - let token = cached.trim(); - if !token.is_empty() { - return Ok(token.to_string()); - } + if let Some(token) = self.read_token_file_secure(&access_token_path).await { + return Ok(token); } let token = self.device_code_login().await?; - write_file_secure(&access_token_path, &token).await; + self.write_token_file_secure(&access_token_path, &token) + .await; Ok(token) } @@ -546,53 +594,41 @@ impl CopilotProvider { async fn load_api_key_from_disk(&self) -> Option { let path = self.token_dir.join("api-key.json"); - let data = tokio::fs::read_to_string(&path).await.ok()?; + let data = self.read_token_file_secure(&path).await?; serde_json::from_str(&data).ok() } async fn save_api_key_to_disk(&self, info: &ApiKeyInfo) { let path = self.token_dir.join("api-key.json"); if let Ok(json) = serde_json::to_string_pretty(info) { - write_file_secure(&path, &json).await; + self.write_token_file_secure(&path, &json).await; } } } /// Write a file with 0600 permissions (owner read/write only). /// Uses `spawn_blocking` to avoid blocking the async runtime. -async fn write_file_secure(path: &Path, content: &str) { - let path = path.to_path_buf(); - let content = content.to_string(); - - let result = tokio::task::spawn_blocking(move || { - #[cfg(unix)] - { - use std::io::Write; - use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; - - let mut file = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .mode(0o600) - .open(&path)?; - file.write_all(content.as_bytes())?; - - std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))?; - Ok::<(), std::io::Error>(()) - } - #[cfg(not(unix))] - { - std::fs::write(&path, &content)?; - Ok::<(), std::io::Error>(()) - } - }) - .await; - - match result { - Ok(Ok(())) => {} - Ok(Err(err)) => warn!("Failed to write secure file: {err}"), - Err(err) => warn!("Failed to spawn blocking write: {err}"), +fn write_file_secure_blocking(path: &Path, content: &str) -> std::io::Result<()> { + #[cfg(unix)] + { + use std::io::Write; + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + + let mut file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(path)?; + file.write_all(content.as_bytes())?; + + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; + Ok(()) + } + #[cfg(not(unix))] + { + std::fs::write(path, content)?; + Ok(()) } } @@ -696,16 +732,12 @@ mod tests { #[test] fn copilot_headers_include_required_fields() { let headers = CopilotProvider::COPILOT_HEADERS; - assert!( - headers - .iter() - .any(|(header, _)| *header == "Editor-Version") - ); - assert!( - headers - .iter() - .any(|(header, _)| *header == "Editor-Plugin-Version") - ); + assert!(headers + .iter() + .any(|(header, _)| *header == "Editor-Version")); + assert!(headers + .iter() + .any(|(header, _)| *header == "Editor-Plugin-Version")); assert!(headers.iter().any(|(header, _)| *header == "User-Agent")); } diff --git a/clients/web/apps/dashboard/postcss.config.js b/clients/web/apps/dashboard/postcss.config.js deleted file mode 100644 index c2ddf7482..000000000 --- a/clients/web/apps/dashboard/postcss.config.js +++ /dev/null @@ -1,5 +0,0 @@ -export default { - plugins: { - "@tailwindcss/postcss": {}, - }, -}; From ef459af8e7174cbffa5a3525d4458555b634e0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 20 Feb 2026 10:35:39 +0100 Subject: [PATCH 2/6] refactor: clean up code formatting and improve readability --- clients/agent-runtime/src/channels/irc.rs | 6 +----- clients/agent-runtime/src/channels/mod.rs | 5 +---- clients/agent-runtime/src/config/schema.rs | 6 +++++- .../src/observability/prometheus.rs | 8 +++----- clients/agent-runtime/src/onboard/wizard.rs | 9 ++++----- .../src/providers/openai_codex.rs | 3 +-- .../agent-runtime/src/providers/openrouter.rs | 20 ++++--------------- clients/agent-runtime/src/service/mod.rs | 9 +++------ clients/agent-runtime/tests/agent_e2e.rs | 4 ++-- clients/web/apps/dashboard/package.json | 10 +++++----- clients/web/package.json | 2 +- clients/web/pnpm-lock.yaml | 14 ++++++------- 12 files changed, 37 insertions(+), 59 deletions(-) diff --git a/clients/agent-runtime/src/channels/irc.rs b/clients/agent-runtime/src/channels/irc.rs index 3a90fa738..15b822ccb 100755 --- a/clients/agent-runtime/src/channels/irc.rs +++ b/clients/agent-runtime/src/channels/irc.rs @@ -388,11 +388,7 @@ impl Channel for IrcChannel { // --- Nick/User registration --- Self::send_raw(&mut writer, &format!("NICK {current_nick}")).await?; - Self::send_raw( - &mut writer, - &format!("USER {} 0 * :Corvus", self.username), - ) - .await?; + Self::send_raw(&mut writer, &format!("USER {} 0 * :Corvus", self.username)).await?; // Store writer for send() { diff --git a/clients/agent-runtime/src/channels/mod.rs b/clients/agent-runtime/src/channels/mod.rs index 4c3057cbe..877ff818e 100755 --- a/clients/agent-runtime/src/channels/mod.rs +++ b/clients/agent-runtime/src/channels/mod.rs @@ -1997,10 +1997,7 @@ mod tests { assert!(prompt.contains("### SOUL.md"), "missing SOUL.md header"); assert!(prompt.contains("Be helpful"), "missing SOUL content"); assert!(prompt.contains("### IDENTITY.md"), "missing IDENTITY.md"); - assert!( - prompt.contains("Name: Corvus"), - "missing IDENTITY content" - ); + assert!(prompt.contains("Name: Corvus"), "missing IDENTITY content"); assert!(prompt.contains("### USER.md"), "missing USER.md"); assert!(prompt.contains("### AGENTS.md"), "missing AGENTS.md"); assert!(prompt.contains("### TOOLS.md"), "missing TOOLS.md"); diff --git a/clients/agent-runtime/src/config/schema.rs b/clients/agent-runtime/src/config/schema.rs index dec7a3e15..6d165131f 100755 --- a/clients/agent-runtime/src/config/schema.rs +++ b/clients/agent-runtime/src/config/schema.rs @@ -4393,7 +4393,11 @@ default_model = "legacy-model" std::fs::set_permissions(&config_path, std::fs::Permissions::from_mode(0o644)).unwrap(); config.save().unwrap(); - let mode = std::fs::metadata(&config_path).unwrap().permissions().mode() & 0o777; + let mode = std::fs::metadata(&config_path) + .unwrap() + .permissions() + .mode() + & 0o777; assert_eq!( mode, 0o600, "Save should enforce owner-only config permissions, got {mode:o}" diff --git a/clients/agent-runtime/src/observability/prometheus.rs b/clients/agent-runtime/src/observability/prometheus.rs index b7c535ff2..f7cb8415a 100755 --- a/clients/agent-runtime/src/observability/prometheus.rs +++ b/clients/agent-runtime/src/observability/prometheus.rs @@ -86,11 +86,9 @@ impl PrometheusObserver { ) .expect("valid metric"); - let tokens_used = prometheus::IntGauge::new( - "corvus_tokens_used_last", - "Tokens used in the last request", - ) - .expect("valid metric"); + let tokens_used = + prometheus::IntGauge::new("corvus_tokens_used_last", "Tokens used in the last request") + .expect("valid metric"); let active_sessions = GaugeVec::new( prometheus::Opts::new("corvus_active_sessions", "Number of active sessions"), diff --git a/clients/agent-runtime/src/onboard/wizard.rs b/clients/agent-runtime/src/onboard/wizard.rs index 17917e5f0..89f700f47 100755 --- a/clients/agent-runtime/src/onboard/wizard.rs +++ b/clients/agent-runtime/src/onboard/wizard.rs @@ -12,7 +12,7 @@ use crate::providers::{ canonical_china_provider_name, is_glm_alias, is_glm_cn_alias, is_minimax_alias, is_moonshot_alias, is_qianfan_alias, is_qwen_alias, is_zai_alias, is_zai_cn_alias, }; -use anyhow::{Context, Result, bail}; +use anyhow::{bail, Context, Result}; use console::style; use dialoguer::{Confirm, Input, Password, Select}; use directories::UserDirs; @@ -5079,10 +5079,9 @@ mod tests { }; let err = run_models_refresh(&config, None, true).unwrap_err(); - assert!( - err.to_string() - .contains("does not support live model discovery") - ); + assert!(err + .to_string() + .contains("does not support live model discovery")); } // ── provider_env_var ──────────────────────────────────────── diff --git a/clients/agent-runtime/src/providers/openai_codex.rs b/clients/agent-runtime/src/providers/openai_codex.rs index 30e4eda45..6a30436fc 100755 --- a/clients/agent-runtime/src/providers/openai_codex.rs +++ b/clients/agent-runtime/src/providers/openai_codex.rs @@ -9,8 +9,7 @@ use serde_json::Value; use std::path::PathBuf; const CODEX_RESPONSES_URL: &str = "https://chatgpt.com/backend-api/codex/responses"; -const DEFAULT_CODEX_INSTRUCTIONS: &str = - "You are Corvus, a concise and helpful coding assistant."; +const DEFAULT_CODEX_INSTRUCTIONS: &str = "You are Corvus, a concise and helpful coding assistant."; pub struct OpenAiCodexProvider { auth: AuthService, diff --git a/clients/agent-runtime/src/providers/openrouter.rs b/clients/agent-runtime/src/providers/openrouter.rs index dca75e1cf..fef06967d 100755 --- a/clients/agent-runtime/src/providers/openrouter.rs +++ b/clients/agent-runtime/src/providers/openrouter.rs @@ -277,10 +277,7 @@ impl Provider for OpenRouterProvider { .client .post("https://openrouter.ai/api/v1/chat/completions") .header("Authorization", format!("Bearer {credential}")) - .header( - "HTTP-Referer", - "https://github.com/theonlyhennygod/corvus", - ) + .header("HTTP-Referer", "https://github.com/theonlyhennygod/corvus") .header("X-Title", "Corvus") .json(&request) .send() @@ -327,10 +324,7 @@ impl Provider for OpenRouterProvider { .client .post("https://openrouter.ai/api/v1/chat/completions") .header("Authorization", format!("Bearer {credential}")) - .header( - "HTTP-Referer", - "https://github.com/theonlyhennygod/corvus", - ) + .header("HTTP-Referer", "https://github.com/theonlyhennygod/corvus") .header("X-Title", "Corvus") .json(&request) .send() @@ -375,10 +369,7 @@ impl Provider for OpenRouterProvider { .client .post("https://openrouter.ai/api/v1/chat/completions") .header("Authorization", format!("Bearer {credential}")) - .header( - "HTTP-Referer", - "https://github.com/theonlyhennygod/corvus", - ) + .header("HTTP-Referer", "https://github.com/theonlyhennygod/corvus") .header("X-Title", "Corvus") .json(&native_request) .send() @@ -463,10 +454,7 @@ impl Provider for OpenRouterProvider { .client .post("https://openrouter.ai/api/v1/chat/completions") .header("Authorization", format!("Bearer {credential}")) - .header( - "HTTP-Referer", - "https://github.com/theonlyhennygod/corvus", - ) + .header("HTTP-Referer", "https://github.com/theonlyhennygod/corvus") .header("X-Title", "Corvus") .json(&native_request) .send() diff --git a/clients/agent-runtime/src/service/mod.rs b/clients/agent-runtime/src/service/mod.rs index 3932ec580..2e6c27209 100755 --- a/clients/agent-runtime/src/service/mod.rs +++ b/clients/agent-runtime/src/service/mod.rs @@ -101,12 +101,9 @@ fn status(config: &Config) -> Result<()> { } if cfg!(target_os = "linux") { - let out = run_capture(Command::new("systemctl").args([ - "--user", - "is-active", - "corvus.service", - ])) - .unwrap_or_else(|_| "unknown".into()); + let out = + run_capture(Command::new("systemctl").args(["--user", "is-active", "corvus.service"])) + .unwrap_or_else(|_| "unknown".into()); println!("Service state: {}", out.trim()); println!("Unit: {}", linux_service_file(config)?.display()); return Ok(()); diff --git a/clients/agent-runtime/tests/agent_e2e.rs b/clients/agent-runtime/tests/agent_e2e.rs index a4395ef1d..3de4d1729 100755 --- a/clients/agent-runtime/tests/agent_e2e.rs +++ b/clients/agent-runtime/tests/agent_e2e.rs @@ -9,8 +9,6 @@ use anyhow::Result; use async_trait::async_trait; -use serde_json::json; -use std::sync::{Arc, Mutex}; use corvus::agent::agent::Agent; use corvus::agent::dispatcher::{NativeToolDispatcher, XmlToolDispatcher}; use corvus::config::MemoryConfig; @@ -19,6 +17,8 @@ use corvus::memory::Memory; use corvus::observability::{NoopObserver, Observer}; use corvus::providers::{ChatRequest, ChatResponse, Provider, ToolCall}; use corvus::tools::{Tool, ToolResult}; +use serde_json::json; +use std::sync::{Arc, Mutex}; // ───────────────────────────────────────────────────────────────────────────── // Mock infrastructure diff --git a/clients/web/apps/dashboard/package.json b/clients/web/apps/dashboard/package.json index 8bdb9af49..b10a1255e 100644 --- a/clients/web/apps/dashboard/package.json +++ b/clients/web/apps/dashboard/package.json @@ -11,16 +11,16 @@ "dev": "vite --port 4323", "build": "vue-tsc -b && vite build", "preview": "vite preview --port 4323", - "format": "biome format --write src package.json components.json tsconfig*.json vite.config.ts index.html postcss.config.js", - "check": "biome check src package.json components.json tsconfig*.json vite.config.ts index.html postcss.config.js", + "format": "biome format --write src package.json components.json tsconfig*.json vite.config.ts index.html", + "check": "biome check src package.json components.json tsconfig*.json vite.config.ts index.html", "test": "vitest --run" }, "dependencies": { "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", - "tailwind-merge": "^3.3.1", - "vue": "^3.5.22", - "vue-i18n": "^11.2.1" + "tailwind-merge": "^3.5.0", + "vue": "^3.5.28", + "vue-i18n": "^11.2.8" }, "devDependencies": { "@biomejs/biome": "2.3.15", diff --git a/clients/web/package.json b/clients/web/package.json index 7a9daf80b..003e1d621 100644 --- a/clients/web/package.json +++ b/clients/web/package.json @@ -24,7 +24,7 @@ "@biomejs/biome": "2.4.2", "typescript": "5.9.3" }, - "packageManager": "pnpm@10.30.0", + "packageManager": "pnpm@10.30.1", "engines": { "node": ">=22.0.0", "pnpm": ">=10.28.0" diff --git a/clients/web/pnpm-lock.yaml b/clients/web/pnpm-lock.yaml index b3938f9d4..701c547af 100644 --- a/clients/web/pnpm-lock.yaml +++ b/clients/web/pnpm-lock.yaml @@ -24,13 +24,13 @@ importers: specifier: ^2.1.1 version: 2.1.1 tailwind-merge: - specifier: ^3.3.1 - version: 3.4.1 + specifier: ^3.5.0 + version: 3.5.0 vue: - specifier: ^3.5.22 + specifier: ^3.5.28 version: 3.5.28(typescript@5.9.3) vue-i18n: - specifier: ^11.2.1 + specifier: ^11.2.8 version: 11.2.8(vue@3.5.28(typescript@5.9.3)) devDependencies: '@biomejs/biome': @@ -2512,8 +2512,8 @@ packages: engines: {node: '>=16'} hasBin: true - tailwind-merge@3.4.1: - resolution: {integrity: sha512-2OA0rFqWOkITEAOFWSBSApYkDeH9t2B3XSJuI4YztKBzK3mX0737A2qtxDZ7xkw9Zfh0bWl+r34sF3HXV+Ig7Q==} + tailwind-merge@3.5.0: + resolution: {integrity: sha512-I8K9wewnVDkL1NTGoqWmVEIlUcB9gFriAEkXkfCjX5ib8ezGxtR3xD7iZIxrfArjEsH7F1CHD4RFUtxefdqV/A==} tailwindcss@4.1.18: resolution: {integrity: sha512-4+Z+0yiYyEtUVCScyfHCxOYP06L5Ne+JiHhY2IjR2KWMIWhJOYZKLSGZaP5HkZ8+bY0cxfzwDE5uOmzFXyIwxw==} @@ -5749,7 +5749,7 @@ snapshots: picocolors: 1.1.1 sax: 1.4.4 - tailwind-merge@3.4.1: {} + tailwind-merge@3.5.0: {} tailwindcss@4.1.18: {} From 06f75ba0354fca83bf664218de86a6db61326885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 20 Feb 2026 11:01:11 +0100 Subject: [PATCH 3/6] feat(security): enhance token file security and permissions handling --- clients/agent-runtime/src/channels/irc.rs | 94 +++++++++++++------ .../agent-runtime/src/providers/copilot.rs | 93 ++++++++++++++---- 2 files changed, 141 insertions(+), 46 deletions(-) diff --git a/clients/agent-runtime/src/channels/irc.rs b/clients/agent-runtime/src/channels/irc.rs index 15b822ccb..bbb8fa23e 100755 --- a/clients/agent-runtime/src/channels/irc.rs +++ b/clients/agent-runtime/src/channels/irc.rs @@ -1,5 +1,6 @@ use crate::channels::traits::{Channel, ChannelMessage, SendMessage}; use async_trait::async_trait; +use base64::Engine; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; @@ -115,37 +116,30 @@ impl IrcMessage { /// Encode SASL PLAIN credentials: base64(\0nick\0password). fn encode_sasl_plain(nick: &str, password: &str) -> String { - // Simple base64 encoder — avoids adding a base64 crate dependency. - // The project's Discord channel uses a similar inline approach. - const CHARS: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; - let input = format!("\0{nick}\0{password}"); - let bytes = input.as_bytes(); - let mut out = String::with_capacity(bytes.len().div_ceil(3) * 4); - - for chunk in bytes.chunks(3) { - let b0 = u32::from(chunk[0]); - let b1 = u32::from(chunk.get(1).copied().unwrap_or(0)); - let b2 = u32::from(chunk.get(2).copied().unwrap_or(0)); - let triple = (b0 << 16) | (b1 << 8) | b2; + base64::engine::general_purpose::STANDARD.encode(input) +} - out.push(CHARS[(triple >> 18 & 0x3F) as usize] as char); - out.push(CHARS[(triple >> 12 & 0x3F) as usize] as char); +/// Split a base64 SASL payload into RFC-compliant IRCv3 AUTHENTICATE chunks. +/// +/// Each chunk must be <= 400 bytes. If payload length is an exact multiple of +/// 400, send a final `+` marker to terminate the exchange. +fn split_sasl_authenticate_payload(encoded: &str) -> Vec { + const SASL_CHUNK_MAX: usize = 400; - if chunk.len() > 1 { - out.push(CHARS[(triple >> 6 & 0x3F) as usize] as char); - } else { - out.push('='); - } + let mut chunks = Vec::new(); + let mut start = 0; + while start < encoded.len() { + let end = (start + SASL_CHUNK_MAX).min(encoded.len()); + chunks.push(encoded[start..end].to_string()); + start = end; + } - if chunk.len() > 2 { - out.push(CHARS[(triple & 0x3F) as usize] as char); - } else { - out.push('='); - } + if encoded.is_empty() || encoded.len() % SASL_CHUNK_MAX == 0 { + chunks.push("+".to_string()); } - out + chunks } /// Split a message into lines safe for IRC transmission. @@ -266,7 +260,11 @@ impl IrcChannel { &self, ) -> anyhow::Result> { let addr = format!("{}:{}", self.server, self.port); - let tcp = tokio::net::TcpStream::connect(&addr).await?; + let tcp = tokio::time::timeout(READ_TIMEOUT, tokio::net::TcpStream::connect(&addr)) + .await + .map_err(|_| { + anyhow::anyhow!("IRC TCP connect timed out after {READ_TIMEOUT:?} to {addr}") + })??; let tls_config = if self.verify_tls { let root_store: rustls::RootCertStore = @@ -283,7 +281,14 @@ impl IrcChannel { let connector = tokio_rustls::TlsConnector::from(Arc::new(tls_config)); let domain = rustls::pki_types::ServerName::try_from(self.server.clone())?; - let tls = connector.connect(domain, tcp).await?; + let tls = tokio::time::timeout(READ_TIMEOUT, connector.connect(domain, tcp)) + .await + .map_err(|_| { + anyhow::anyhow!( + "IRC TLS handshake timed out after {READ_TIMEOUT:?} with {}", + self.server + ) + })??; Ok(tls) } @@ -428,13 +433,21 @@ impl Channel for IrcChannel { // CAP responses for SASL "CAP" => { if sasl_pending && msg.params.iter().any(|p| p.contains("sasl")) { - if msg.params.iter().any(|p| p.contains("ACK")) { + if msg + .params + .iter() + .any(|p| p.trim_start_matches(':') == "ACK") + { // CAP * ACK :sasl — server accepted, start SASL auth let mut guard = self.writer.lock().await; if let Some(ref mut w) = *guard { Self::send_raw(w, "AUTHENTICATE PLAIN").await?; } - } else if msg.params.iter().any(|p| p.contains("NAK")) { + } else if msg + .params + .iter() + .any(|p| p.trim_start_matches(':') == "NAK") + { // CAP * NAK :sasl — server rejected SASL, proceed without it tracing::warn!( "IRC server does not support SASL, continuing without it" @@ -455,7 +468,9 @@ impl Channel for IrcChannel { let encoded = encode_sasl_plain(¤t_nick, password); let mut guard = self.writer.lock().await; if let Some(ref mut w) = *guard { - Self::send_raw(w, &format!("AUTHENTICATE {encoded}")).await?; + for chunk in split_sasl_authenticate_payload(&encoded) { + Self::send_raw(w, &format!("AUTHENTICATE {chunk}")).await?; + } } } else { // SASL was requested but no password is configured; abort SASL @@ -710,6 +725,25 @@ mod tests { assert_eq!(encoded, "AG5pY2sA"); } + #[test] + fn sasl_payload_chunks_include_terminator_for_exact_boundary() { + let payload = "a".repeat(800); + let chunks = split_sasl_authenticate_payload(&payload); + assert_eq!(chunks.len(), 3); + assert_eq!(chunks[0].len(), 400); + assert_eq!(chunks[1].len(), 400); + assert_eq!(chunks[2], "+"); + } + + #[test] + fn sasl_payload_chunks_without_terminator_for_partial_tail() { + let payload = "a".repeat(401); + let chunks = split_sasl_authenticate_payload(&payload); + assert_eq!(chunks.len(), 2); + assert_eq!(chunks[0].len(), 400); + assert_eq!(chunks[1].len(), 1); + } + // ── Message splitting ─────────────────────────────────── #[test] diff --git a/clients/agent-runtime/src/providers/copilot.rs b/clients/agent-runtime/src/providers/copilot.rs index 0d2382baf..5762b50f9 100755 --- a/clients/agent-runtime/src/providers/copilot.rs +++ b/clients/agent-runtime/src/providers/copilot.rs @@ -198,6 +198,15 @@ impl CopilotProvider { { use std::os::unix::fs::PermissionsExt; + if let Err(err) = + std::fs::set_permissions(&corvus_dir, std::fs::Permissions::from_mode(0o700)) + { + warn!( + "Failed to set Corvus config directory permissions on {:?}: {err}", + corvus_dir + ); + } + if let Err(err) = std::fs::set_permissions(&token_dir, std::fs::Permissions::from_mode(0o700)) { @@ -249,24 +258,29 @@ impl CopilotProvider { async fn write_token_file_secure(&self, path: &Path, content: &str) { let path = path.to_path_buf(); - let write_path = path.clone(); + let path_display = path.display().to_string(); let content = content.to_string(); let secret_store = self.secret_store.clone(); let result = tokio::task::spawn_blocking(move || -> anyhow::Result<()> { let encrypted = secret_store.encrypt(&content)?; - write_file_secure_blocking(&write_path, &encrypted)?; + write_file_secure_blocking(&path, &encrypted)?; Ok(()) }) .await; match result { Ok(Ok(())) => {} - Ok(Err(err)) => warn!( - "Failed to write secure Copilot token file {:?}: {err}", - path + Ok(Err(err)) => { + warn!( + "Failed to write secure Copilot token file {}: {err}", + path_display + ) + } + Err(err) => warn!( + "Failed to spawn token write task for {}: {err}", + path_display ), - Err(err) => warn!("Failed to spawn token write task for {:?}: {err}", path), } } @@ -612,7 +626,7 @@ fn write_file_secure_blocking(path: &Path, content: &str) -> std::io::Result<()> #[cfg(unix)] { use std::io::Write; - use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + use std::os::unix::fs::OpenOptionsExt; let mut file = std::fs::OpenOptions::new() .write(true) @@ -621,15 +635,53 @@ fn write_file_secure_blocking(path: &Path, content: &str) -> std::io::Result<()> .mode(0o600) .open(path)?; file.write_all(content.as_bytes())?; - - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; Ok(()) } - #[cfg(not(unix))] + #[cfg(windows)] { - std::fs::write(path, content)?; + use std::io::Write; + use std::os::windows::fs::OpenOptionsExt; + + const FILE_ATTRIBUTE_NORMAL: u32 = 0x80; + let mut file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .attributes(FILE_ATTRIBUTE_NORMAL) + .open(path)?; + file.write_all(content.as_bytes())?; + + let username = std::env::var("USERNAME").map_err(|_| { + std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "USERNAME is unset; cannot secure Copilot token ACL", + ) + })?; + let grant_arg = format!("{username}:(R,W)"); + let output = std::process::Command::new("icacls") + .arg(path) + .args(["/inheritance:r", "/grant:r"]) + .arg(grant_arg) + .output()?; + if !output.status.success() { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + format!( + "failed to secure Copilot token ACL via icacls (exit code {:?})", + output.status.code() + ), + )); + } Ok(()) } + #[cfg(all(not(unix), not(windows)))] + { + let _ = (path, content); + Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "secure token file permissions are not implemented for this platform", + )) + } } #[async_trait] @@ -732,13 +784,22 @@ mod tests { #[test] fn copilot_headers_include_required_fields() { let headers = CopilotProvider::COPILOT_HEADERS; - assert!(headers + let editor_version = headers .iter() - .any(|(header, _)| *header == "Editor-Version")); - assert!(headers + .find(|(header, _)| *header == "Editor-Version") + .map(|(_, value)| *value); + let plugin_version = headers .iter() - .any(|(header, _)| *header == "Editor-Plugin-Version")); - assert!(headers.iter().any(|(header, _)| *header == "User-Agent")); + .find(|(header, _)| *header == "Editor-Plugin-Version") + .map(|(_, value)| *value); + let user_agent = headers + .iter() + .find(|(header, _)| *header == "User-Agent") + .map(|(_, value)| *value); + + assert_eq!(editor_version, Some("vscode/1.85.1")); + assert_eq!(plugin_version, Some("copilot/1.155.0")); + assert_eq!(user_agent, Some("GithubCopilot/1.155.0")); } #[test] From 38f14f76197c67904934c6371f168868cfa6b771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 20 Feb 2026 12:00:27 +0100 Subject: [PATCH 4/6] feat(security): implement secure file handling and ACL hardening for Windows --- clients/agent-runtime/Cargo.lock | 2 + clients/agent-runtime/Cargo.toml | 11 ++ clients/agent-runtime/src/channels/irc.rs | 24 ++- .../agent-runtime/src/providers/copilot.rs | 168 ++++++++++++++---- 4 files changed, 167 insertions(+), 38 deletions(-) diff --git a/clients/agent-runtime/Cargo.lock b/clients/agent-runtime/Cargo.lock index 2d02983cf..0da13c0d5 100755 --- a/clients/agent-runtime/Cargo.lock +++ b/clients/agent-runtime/Cargo.lock @@ -1189,6 +1189,8 @@ dependencies = [ "urlencoding", "uuid", "webpki-roots 1.0.6", + "windows-sys 0.59.0", + "zeroize", ] [[package]] diff --git a/clients/agent-runtime/Cargo.toml b/clients/agent-runtime/Cargo.toml index 6d40070a3..3e03782b6 100755 --- a/clients/agent-runtime/Cargo.toml +++ b/clients/agent-runtime/Cargo.toml @@ -44,6 +44,9 @@ prometheus = { version = "0.14", default-features = false } # Base64 encoding (screenshots, image data) base64 = "0.22" +# Securely zero sensitive plaintext buffers on drop +zeroize = "1.8" + # URL encoding for web search urlencoding = "2.1" @@ -135,6 +138,14 @@ pdf-extract = { version = "0.10", optional = true } rppal = { version = "0.22", optional = true } landlock = { version = "0.4", optional = true } +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.59", features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_Security_Authorization", + "Win32_Storage_FileSystem", +] } + [features] default = ["hardware"] hardware = ["nusb", "tokio-serial"] diff --git a/clients/agent-runtime/src/channels/irc.rs b/clients/agent-runtime/src/channels/irc.rs index bbb8fa23e..0128b5fd0 100755 --- a/clients/agent-runtime/src/channels/irc.rs +++ b/clients/agent-runtime/src/channels/irc.rs @@ -13,6 +13,9 @@ use tokio_rustls::rustls; /// connection is considered dead. IRC servers typically PING every 60-120s. const READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300); +/// Setup timeout for TCP connect + TLS handshake. +const CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); + /// Monotonic counter to ensure unique message IDs under burst traffic. static MSG_SEQ: AtomicU64 = AtomicU64::new(0); @@ -260,10 +263,10 @@ impl IrcChannel { &self, ) -> anyhow::Result> { let addr = format!("{}:{}", self.server, self.port); - let tcp = tokio::time::timeout(READ_TIMEOUT, tokio::net::TcpStream::connect(&addr)) + let tcp = tokio::time::timeout(CONNECT_TIMEOUT, tokio::net::TcpStream::connect(&addr)) .await .map_err(|_| { - anyhow::anyhow!("IRC TCP connect timed out after {READ_TIMEOUT:?} to {addr}") + anyhow::anyhow!("IRC TCP connect timed out after {CONNECT_TIMEOUT:?} to {addr}") })??; let tls_config = if self.verify_tls { @@ -281,11 +284,11 @@ impl IrcChannel { let connector = tokio_rustls::TlsConnector::from(Arc::new(tls_config)); let domain = rustls::pki_types::ServerName::try_from(self.server.clone())?; - let tls = tokio::time::timeout(READ_TIMEOUT, connector.connect(domain, tcp)) + let tls = tokio::time::timeout(CONNECT_TIMEOUT, connector.connect(domain, tcp)) .await .map_err(|_| { anyhow::anyhow!( - "IRC TLS handshake timed out after {READ_TIMEOUT:?} with {}", + "IRC TLS handshake timed out after {CONNECT_TIMEOUT:?} with {}", self.server ) })??; @@ -744,6 +747,19 @@ mod tests { assert_eq!(chunks[1].len(), 1); } + #[test] + fn sasl_payload_chunks_empty_payload_returns_plus() { + let chunks = split_sasl_authenticate_payload(""); + assert_eq!(chunks, vec!["+"]); + } + + #[test] + fn sasl_payload_chunks_exact_single_chunk_has_terminator() { + let payload = "a".repeat(400); + let chunks = split_sasl_authenticate_payload(&payload); + assert_eq!(chunks, vec![payload, "+".to_string()]); + } + // ── Message splitting ─────────────────────────────────── #[test] diff --git a/clients/agent-runtime/src/providers/copilot.rs b/clients/agent-runtime/src/providers/copilot.rs index 5762b50f9..ada9c8a74 100755 --- a/clients/agent-runtime/src/providers/copilot.rs +++ b/clients/agent-runtime/src/providers/copilot.rs @@ -25,6 +25,7 @@ use std::sync::Arc; use std::time::Duration; use tokio::sync::Mutex; use tracing::warn; +use zeroize::Zeroizing; /// GitHub OAuth client ID for Copilot (VS Code extension). const GITHUB_CLIENT_ID: &str = "Iv1.b507a08c87ecfe98"; @@ -33,6 +34,7 @@ const GITHUB_ACCESS_TOKEN_URL: &str = "https://github.com/login/oauth/access_tok const GITHUB_API_KEY_URL: &str = "https://api.github.com/copilot_internal/v2/token"; const DEFAULT_API: &str = "https://api.githubcopilot.com"; const OAUTH_POLLING_SAFETY_MARGIN_MS: u64 = 3000; +const TOKEN_STORE_ENCRYPTION_ENABLED: bool = true; // ── Token types ────────────────────────────────────────────────── @@ -216,6 +218,22 @@ impl CopilotProvider { ); } } + + #[cfg(windows)] + { + if let Err(err) = harden_windows_acl(&corvus_dir, true) { + warn!( + "Failed to harden Corvus config directory ACL on {:?}: {err}", + corvus_dir + ); + } + if let Err(err) = harden_windows_acl(&token_dir, true) { + warn!( + "Failed to harden Copilot token directory ACL on {:?}: {err}", + token_dir + ); + } + } } Self { @@ -228,7 +246,7 @@ impl CopilotProvider { .connect_timeout(Duration::from_secs(10)) .build() .unwrap_or_else(|_| Client::new()), - secret_store: SecretStore::new(&corvus_dir, true), + secret_store: SecretStore::new(&corvus_dir, TOKEN_STORE_ENCRYPTION_ENABLED), token_dir, } } @@ -259,11 +277,11 @@ impl CopilotProvider { async fn write_token_file_secure(&self, path: &Path, content: &str) { let path = path.to_path_buf(); let path_display = path.display().to_string(); - let content = content.to_string(); + let content = Zeroizing::new(content.to_string()); let secret_store = self.secret_store.clone(); let result = tokio::task::spawn_blocking(move || -> anyhow::Result<()> { - let encrypted = secret_store.encrypt(&content)?; + let encrypted = secret_store.encrypt(content.as_str())?; write_file_secure_blocking(&path, &encrypted)?; Ok(()) }) @@ -620,7 +638,118 @@ impl CopilotProvider { } } -/// Write a file with 0600 permissions (owner read/write only). +#[cfg(windows)] +fn windows_icacls_path() -> PathBuf { + let system_root = std::env::var("SystemRoot").unwrap_or_else(|_| "C:\\Windows".to_string()); + PathBuf::from(system_root) + .join("System32") + .join("icacls.exe") +} + +#[cfg(windows)] +fn harden_windows_acl(path: &Path, is_directory: bool) -> std::io::Result<()> { + let username = std::env::var("USERNAME").map_err(|_| { + std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "USERNAME is unset; cannot harden ACL", + ) + })?; + let grant = if is_directory { + format!("{username}:(OI)(CI)F") + } else { + format!("{username}:(R,W)") + }; + + let output = std::process::Command::new(windows_icacls_path()) + .arg(path) + .args(["/inheritance:r", "/grant:r"]) + .arg(grant) + .output()?; + + if !output.status.success() { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + format!( + "failed to harden ACL via icacls (exit code {:?})", + output.status.code() + ), + )); + } + + Ok(()) +} + +#[cfg(windows)] +fn create_secure_file_windows(path: &Path) -> std::io::Result { + use std::ffi::OsStr; + use std::iter; + use std::os::windows::ffi::OsStrExt; + use std::os::windows::io::FromRawHandle; + use std::ptr::null_mut; + use windows_sys::Win32::Foundation::{LocalFree, INVALID_HANDLE_VALUE}; + use windows_sys::Win32::Security::Authorization::{ + ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1, + }; + use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; + use windows_sys::Win32::Storage::FileSystem::{ + CreateFileW, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, + }; + + const GENERIC_WRITE: u32 = 0x40000000; + let path_wide: Vec = path + .as_os_str() + .encode_wide() + .chain(iter::once(0)) + .collect(); + let sddl: Vec = OsStr::new("D:P(A;;GA;;;OW)") + .encode_wide() + .chain(iter::once(0)) + .collect(); + + let mut security_descriptor = null_mut(); + let ok = unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl.as_ptr(), + SDDL_REVISION_1 as u32, + &mut security_descriptor, + null_mut(), + ) + }; + if ok == 0 { + return Err(std::io::Error::last_os_error()); + } + + let mut security_attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: security_descriptor, + bInheritHandle: 0, + }; + + let handle = unsafe { + CreateFileW( + path_wide.as_ptr(), + GENERIC_WRITE, + 0, + &mut security_attributes, + CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, + 0, + ) + }; + + unsafe { + LocalFree(security_descriptor as isize); + } + + if handle == INVALID_HANDLE_VALUE { + return Err(std::io::Error::last_os_error()); + } + + let file = unsafe { std::fs::File::from_raw_handle(handle as *mut _) }; + Ok(file) +} + +/// Write a file with restrictive owner-only permissions. /// Uses `spawn_blocking` to avoid blocking the async runtime. fn write_file_secure_blocking(path: &Path, content: &str) -> std::io::Result<()> { #[cfg(unix)] @@ -640,38 +769,9 @@ fn write_file_secure_blocking(path: &Path, content: &str) -> std::io::Result<()> #[cfg(windows)] { use std::io::Write; - use std::os::windows::fs::OpenOptionsExt; - const FILE_ATTRIBUTE_NORMAL: u32 = 0x80; - let mut file = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .attributes(FILE_ATTRIBUTE_NORMAL) - .open(path)?; + let mut file = create_secure_file_windows(path)?; file.write_all(content.as_bytes())?; - - let username = std::env::var("USERNAME").map_err(|_| { - std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - "USERNAME is unset; cannot secure Copilot token ACL", - ) - })?; - let grant_arg = format!("{username}:(R,W)"); - let output = std::process::Command::new("icacls") - .arg(path) - .args(["/inheritance:r", "/grant:r"]) - .arg(grant_arg) - .output()?; - if !output.status.success() { - return Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - format!( - "failed to secure Copilot token ACL via icacls (exit code {:?})", - output.status.code() - ), - )); - } Ok(()) } #[cfg(all(not(unix), not(windows)))] From 8e43d7582245430e303571eed9a0268f99788518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 20 Feb 2026 12:15:53 +0100 Subject: [PATCH 5/6] feat(security): use Zeroizing for secure token handling and improve API key management --- clients/agent-runtime/src/channels/irc.rs | 64 ++++++++++++++----- .../agent-runtime/src/providers/copilot.rs | 45 +++++++------ 2 files changed, 75 insertions(+), 34 deletions(-) diff --git a/clients/agent-runtime/src/channels/irc.rs b/clients/agent-runtime/src/channels/irc.rs index 0128b5fd0..ba8196436 100755 --- a/clients/agent-runtime/src/channels/irc.rs +++ b/clients/agent-runtime/src/channels/irc.rs @@ -1,6 +1,6 @@ use crate::channels::traits::{Channel, ChannelMessage, SendMessage}; use async_trait::async_trait; -use base64::Engine; +use base64::{engine::general_purpose, Engine as _}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; @@ -13,7 +13,8 @@ use tokio_rustls::rustls; /// connection is considered dead. IRC servers typically PING every 60-120s. const READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300); -/// Setup timeout for TCP connect + TLS handshake. +/// Per-phase timeout applied independently to TCP connect and TLS handshake. +/// Total setup wait can be up to 2x this value. const CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); /// Monotonic counter to ensure unique message IDs under burst traffic. @@ -120,7 +121,27 @@ impl IrcMessage { /// Encode SASL PLAIN credentials: base64(\0nick\0password). fn encode_sasl_plain(nick: &str, password: &str) -> String { let input = format!("\0{nick}\0{password}"); - base64::engine::general_purpose::STANDARD.encode(input) + general_purpose::STANDARD.encode(input) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SaslCapAction { + StartAuthenticatePlain, + EndCapAndDisable, +} + +fn sasl_cap_action(msg: &IrcMessage, sasl_pending: bool) -> Option { + if !sasl_pending || msg.command != "CAP" || !msg.params.iter().any(|p| p == "sasl") { + return None; + } + + if msg.params.iter().any(|p| p == "ACK") { + Some(SaslCapAction::StartAuthenticatePlain) + } else if msg.params.iter().any(|p| p == "NAK") { + Some(SaslCapAction::EndCapAndDisable) + } else { + None + } } /// Split a base64 SASL payload into RFC-compliant IRCv3 AUTHENTICATE chunks. @@ -129,6 +150,8 @@ fn encode_sasl_plain(nick: &str, password: &str) -> String { /// 400, send a final `+` marker to terminate the exchange. fn split_sasl_authenticate_payload(encoded: &str) -> Vec { const SASL_CHUNK_MAX: usize = 400; + // `encode_sasl_plain` produces base64 ASCII, so byte slicing is safe here. + debug_assert!(encoded.is_ascii()); let mut chunks = Vec::new(); let mut start = 0; @@ -435,22 +458,15 @@ impl Channel for IrcChannel { // CAP responses for SASL "CAP" => { - if sasl_pending && msg.params.iter().any(|p| p.contains("sasl")) { - if msg - .params - .iter() - .any(|p| p.trim_start_matches(':') == "ACK") - { - // CAP * ACK :sasl — server accepted, start SASL auth + match sasl_cap_action(&msg, sasl_pending) { + Some(SaslCapAction::StartAuthenticatePlain) => { + // CAP * ACK :sasl — server accepted, start SASL auth. let mut guard = self.writer.lock().await; if let Some(ref mut w) = *guard { Self::send_raw(w, "AUTHENTICATE PLAIN").await?; } - } else if msg - .params - .iter() - .any(|p| p.trim_start_matches(':') == "NAK") - { + } + Some(SaslCapAction::EndCapAndDisable) => { // CAP * NAK :sasl — server rejected SASL, proceed without it tracing::warn!( "IRC server does not support SASL, continuing without it" @@ -461,6 +477,7 @@ impl Channel for IrcChannel { Self::send_raw(w, "CAP END").await?; } } + None => {} } } @@ -712,6 +729,23 @@ mod tests { assert_eq!(msg.params, vec!["+"]); } + #[test] + fn parse_cap_nak_with_sasl() { + let msg = IrcMessage::parse(":server CAP * NAK :sasl").unwrap(); + assert_eq!(msg.command, "CAP"); + assert!(msg.params.iter().any(|p| p == "NAK")); + assert!(msg.params.iter().any(|p| p == "sasl")); + } + + #[test] + fn sasl_cap_action_nak_ends_sasl_negotiation() { + let msg = IrcMessage::parse(":server CAP * NAK :sasl").unwrap(); + assert_eq!( + sasl_cap_action(&msg, true), + Some(SaslCapAction::EndCapAndDisable) + ); + } + // ── SASL PLAIN encoding ───────────────────────────────── #[test] diff --git a/clients/agent-runtime/src/providers/copilot.rs b/clients/agent-runtime/src/providers/copilot.rs index ada9c8a74..89217a31c 100755 --- a/clients/agent-runtime/src/providers/copilot.rs +++ b/clients/agent-runtime/src/providers/copilot.rs @@ -83,7 +83,7 @@ struct ApiEndpoints { } struct CachedApiKey { - token: String, + token: Zeroizing, api_endpoint: String, expires_at: i64, } @@ -251,7 +251,7 @@ impl CopilotProvider { } } - async fn read_token_file_secure(&self, path: &Path) -> Option { + async fn read_token_file_secure(&self, path: &Path) -> Option> { let data = tokio::fs::read_to_string(path).await.ok()?; let value = data.trim(); if value.is_empty() { @@ -260,11 +260,12 @@ impl CopilotProvider { match self.secret_store.decrypt(value) { Ok(decrypted) => { + let decrypted = Zeroizing::new(decrypted); let token = decrypted.trim(); if token.is_empty() { None } else { - Some(token.to_string()) + Some(Zeroizing::new(token.to_string())) } } Err(err) => { @@ -464,7 +465,10 @@ impl CopilotProvider { if let Some(cached_key) = cached.as_ref() { if chrono::Utc::now().timestamp() + 120 < cached_key.expires_at { - return Ok((cached_key.token.clone(), cached_key.api_endpoint.clone())); + return Ok(( + cached_key.token.as_str().to_string(), + cached_key.api_endpoint.clone(), + )); } } @@ -475,19 +479,20 @@ impl CopilotProvider { .as_ref() .and_then(|e| e.api.clone()) .unwrap_or_else(|| DEFAULT_API.to_string()); - let token = info.token; + let token = Zeroizing::new(info.token); + let token_out = token.as_str().to_string(); *cached = Some(CachedApiKey { - token: token.clone(), + token, api_endpoint: endpoint.clone(), expires_at: info.expires_at, }); - return Ok((token, endpoint)); + return Ok((token_out, endpoint)); } } - let access_token = self.get_github_access_token().await?; - let api_key_info = self.exchange_for_api_key(&access_token).await?; + let access_token = Zeroizing::new(self.get_github_access_token().await?); + let api_key_info = self.exchange_for_api_key(access_token.as_ref()).await?; self.save_api_key_to_disk(&api_key_info).await; let endpoint = api_key_info @@ -496,13 +501,15 @@ impl CopilotProvider { .and_then(|e| e.api.clone()) .unwrap_or_else(|| DEFAULT_API.to_string()); + let api_token = Zeroizing::new(api_key_info.token); + let api_token_out = api_token.as_str().to_string(); *cached = Some(CachedApiKey { - token: api_key_info.token.clone(), + token: api_token, api_endpoint: endpoint.clone(), expires_at: api_key_info.expires_at, }); - Ok((api_key_info.token, endpoint)) + Ok((api_token_out, endpoint)) } /// Get a GitHub access token from config, cache, or device flow. @@ -513,13 +520,13 @@ impl CopilotProvider { let access_token_path = self.token_dir.join("access-token"); if let Some(token) = self.read_token_file_secure(&access_token_path).await { - return Ok(token); + return Ok(token.as_str().to_string()); } - let token = self.device_code_login().await?; - self.write_token_file_secure(&access_token_path, &token) + let token = Zeroizing::new(self.device_code_login().await?); + self.write_token_file_secure(&access_token_path, token.as_ref()) .await; - Ok(token) + Ok(token.as_str().to_string()) } /// Run GitHub OAuth device code flow. @@ -627,13 +634,13 @@ impl CopilotProvider { async fn load_api_key_from_disk(&self) -> Option { let path = self.token_dir.join("api-key.json"); let data = self.read_token_file_secure(&path).await?; - serde_json::from_str(&data).ok() + serde_json::from_str(data.as_ref()).ok() } async fn save_api_key_to_disk(&self, info: &ApiKeyInfo) { let path = self.token_dir.join("api-key.json"); - if let Ok(json) = serde_json::to_string_pretty(info) { - self.write_token_file_secure(&path, &json).await; + if let Ok(json) = serde_json::to_string_pretty(info).map(Zeroizing::new) { + self.write_token_file_secure(&path, json.as_ref()).await; } } } @@ -750,7 +757,7 @@ fn create_secure_file_windows(path: &Path) -> std::io::Result { } /// Write a file with restrictive owner-only permissions. -/// Uses `spawn_blocking` to avoid blocking the async runtime. +/// This is synchronous and intended to run inside `spawn_blocking`. fn write_file_secure_blocking(path: &Path, content: &str) -> std::io::Result<()> { #[cfg(unix)] { From 71f166b8fbd8b6d304ae3b901cfb258c4a021ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 20 Feb 2026 12:38:10 +0100 Subject: [PATCH 6/6] feat(security): implement secure directory creation and improve token decryption handling --- clients/agent-runtime/src/channels/irc.rs | 5 +- .../agent-runtime/src/providers/copilot.rs | 111 ++++++++++++++++-- 2 files changed, 101 insertions(+), 15 deletions(-) diff --git a/clients/agent-runtime/src/channels/irc.rs b/clients/agent-runtime/src/channels/irc.rs index ba8196436..2579ec2c5 100755 --- a/clients/agent-runtime/src/channels/irc.rs +++ b/clients/agent-runtime/src/channels/irc.rs @@ -5,6 +5,7 @@ use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::sync::{mpsc, Mutex}; +use zeroize::Zeroizing; // Use tokio_rustls's re-export of rustls types use tokio_rustls::rustls; @@ -120,8 +121,8 @@ impl IrcMessage { /// Encode SASL PLAIN credentials: base64(\0nick\0password). fn encode_sasl_plain(nick: &str, password: &str) -> String { - let input = format!("\0{nick}\0{password}"); - general_purpose::STANDARD.encode(input) + let input = Zeroizing::new(format!("\0{nick}\0{password}")); + general_purpose::STANDARD.encode(input.as_bytes()) } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/clients/agent-runtime/src/providers/copilot.rs b/clients/agent-runtime/src/providers/copilot.rs index 89217a31c..aeb7cfa33 100755 --- a/clients/agent-runtime/src/providers/copilot.rs +++ b/clients/agent-runtime/src/providers/copilot.rs @@ -190,7 +190,12 @@ impl CopilotProvider { }); let token_dir = corvus_dir.join("copilot"); - if let Err(err) = std::fs::create_dir_all(&token_dir) { + #[cfg(windows)] + let create_result = create_secure_dir_windows(&token_dir); + #[cfg(not(windows))] + let create_result = std::fs::create_dir_all(&token_dir); + + if let Err(err) = create_result { warn!( "Failed to create Copilot token directory {:?}: {err}. Token caching is disabled.", token_dir @@ -258,21 +263,34 @@ impl CopilotProvider { return None; } - match self.secret_store.decrypt(value) { - Ok(decrypted) => { - let decrypted = Zeroizing::new(decrypted); - let token = decrypted.trim(); - if token.is_empty() { + let secret_store = self.secret_store.clone(); + let encrypted_value = value.to_string(); + let path_display = path.display().to_string(); + let decrypt_result = + tokio::task::spawn_blocking(move || match secret_store.decrypt(&encrypted_value) { + Ok(decrypted) => Some(decrypted), + Err(err) => { + warn!( + "Failed to decrypt Copilot token file {}: {err}", + path_display + ); None - } else { - Some(Zeroizing::new(token.to_string())) } - } - Err(err) => { - warn!("Failed to decrypt Copilot token file {:?}: {err}", path); - None - } + }) + .await; + + if let Err(err) = &decrypt_result { + warn!("Failed to spawn token decrypt task for {:?}: {err}", path); + } + + let decrypted = decrypt_result.ok().flatten()?; + let decrypted = Zeroizing::new(decrypted); + let token = decrypted.trim(); + if token.is_empty() { + return None; } + + Some(Zeroizing::new(token.to_string())) } async fn write_token_file_secure(&self, path: &Path, content: &str) { @@ -653,6 +671,73 @@ fn windows_icacls_path() -> PathBuf { .join("icacls.exe") } +#[cfg(windows)] +fn create_secure_dir_windows(path: &Path) -> std::io::Result<()> { + use std::iter; + use std::os::windows::ffi::OsStrExt; + use std::ptr::null_mut; + use windows_sys::Win32::Foundation::{LocalFree, ERROR_ALREADY_EXISTS}; + use windows_sys::Win32::Security::Authorization::{ + ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1, + }; + use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; + use windows_sys::Win32::Storage::FileSystem::CreateDirectoryW; + + if path.exists() { + return Ok(()); + } + + if let Some(parent) = path.parent() { + if !parent.exists() { + create_secure_dir_windows(parent)?; + } + } + + let path_wide: Vec = path + .as_os_str() + .encode_wide() + .chain(iter::once(0)) + .collect(); + let sddl: Vec = "D:P(A;;GA;;;OW)" + .encode_utf16() + .chain(iter::once(0)) + .collect(); + + let mut security_descriptor = null_mut(); + let ok = unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl.as_ptr(), + SDDL_REVISION_1 as u32, + &mut security_descriptor, + null_mut(), + ) + }; + if ok == 0 { + return Err(std::io::Error::last_os_error()); + } + + let mut security_attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: security_descriptor, + bInheritHandle: 0, + }; + + let created = unsafe { CreateDirectoryW(path_wide.as_ptr(), &mut security_attributes) }; + + unsafe { + LocalFree(security_descriptor as isize); + } + + if created == 0 { + let err = std::io::Error::last_os_error(); + if err.raw_os_error() != Some(ERROR_ALREADY_EXISTS as i32) { + return Err(err); + } + } + + Ok(()) +} + #[cfg(windows)] fn harden_windows_acl(path: &Path, is_directory: bool) -> std::io::Result<()> { let username = std::env::var("USERNAME").map_err(|_| {