From 7249a8ee6a9f5a69887c5108218025072b8abe4c Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 11 Apr 2026 19:28:59 +0900 Subject: [PATCH] =?UTF-8?q?Pod=E3=81=AB=E3=82=AD=E3=83=BC=E3=82=92?= =?UTF-8?q?=E6=B8=A1=E3=81=99=E5=AE=9F=E8=A3=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.lock | 43 +++++++ TODO.md | 2 +- crates/manifest/src/lib.rs | 29 ++++- crates/pod/examples/pod_cli.rs | 3 +- crates/pod/examples/pod_protocol.rs | 3 +- crates/pod/src/main.rs | 7 +- crates/pod/src/pod.rs | 4 +- crates/provider/Cargo.toml | 4 + crates/provider/src/lib.rs | 190 ++++++++++++++++++++++++++-- tickets/api-key-file.md | 44 ------- 10 files changed, 256 insertions(+), 73 deletions(-) delete mode 100644 tickets/api-key-file.md diff --git a/Cargo.lock b/Cargo.lock index 8bc7e633..24ea6f6d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1279,6 +1279,8 @@ version = "0.1.0" dependencies = [ "llm-worker", "manifest", + "serial_test", + "tempfile", "thiserror", ] @@ -1489,6 +1491,15 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" +[[package]] +name = "scc" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46e6f046b7fef48e2660c57ed794263155d713de679057f2d0c169bfc6e756cc" +dependencies = [ + "sdd", +] + [[package]] name = "schannel" version = "0.1.29" @@ -1529,6 +1540,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "sdd" +version = "3.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "490dcfcbfef26be6800d11870ff2df8774fa6e86d047e3e8c8a76b25655e41ca" + [[package]] name = "security-framework" version = "3.7.0" @@ -1621,6 +1638,32 @@ dependencies = [ "serde_core", ] +[[package]] +name = "serial_test" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "911bd979bf1070a3f3aa7b691a3b3e9968f339ceeec89e08c280a8a22207a32f" +dependencies = [ + "futures-executor", + "futures-util", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a7d91949b85b0d2fb687445e448b40d322b6b3e4af6b44a29b21d9a5f33e6d9" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "sha2" version = "0.11.0" diff --git a/TODO.md b/TODO.md index 41f251a7..2e56d6f3 100644 --- a/TODO.md +++ b/TODO.md @@ -11,6 +11,6 @@ - [x] Subscriber → クロージャ API 移行 - [x] JSONL ストリーム変換ユーティリティ (protocol::stream) - [x] Hook モジュールの llm-worker からの除去 → [tickets/remove-hook-module.md](tickets/remove-hook-module.md) -- [ ] api_key_file: ファイルパスによるAPIキー解決 → [tickets/api-key-file.md](tickets/api-key-file.md) +- [x] api_key_file: ファイルパスによるAPIキー解決 → [tickets/api-key-file.md](tickets/api-key-file.md) - [ ] コンテキスト圧縮 (Prune + Compact) → [tickets/context-compaction.md](tickets/context-compaction.md) - [ ] パーミッション: パターンベースのツール実行制御 → [tickets/permission-extension-point.md](tickets/permission-extension-point.md) diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index fc625f3d..717597d8 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -31,9 +31,9 @@ pub struct PodMeta { pub struct ProviderConfig { pub kind: ProviderKind, pub model: String, - /// Environment variable name holding the API key. + /// Path to a file containing the API key (read and trimmed at startup). #[serde(default)] - pub api_key_env: Option, + pub api_key_file: Option, /// Custom base URL for the provider API. #[serde(default)] pub base_url: Option, @@ -49,6 +49,21 @@ pub enum ProviderKind { Ollama, } +impl ProviderKind { + /// Conventional environment variable name for the API key. + /// + /// Returns `INSOMNIA_API_KEY_{KIND}` (e.g. `INSOMNIA_API_KEY_ANTHROPIC`). + pub fn env_var_name(self) -> String { + let kind = match self { + Self::Anthropic => "ANTHROPIC", + Self::Openai => "OPENAI", + Self::Gemini => "GEMINI", + Self::Ollama => "OLLAMA", + }; + format!("INSOMNIA_API_KEY_{kind}") + } +} + /// Worker-level configuration embedded in the manifest. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct WorkerManifest { @@ -95,7 +110,7 @@ model = "claude-sonnet-4-20250514" assert_eq!(manifest.pod.name, "test-agent"); assert_eq!(manifest.provider.kind, ProviderKind::Anthropic); assert_eq!(manifest.provider.model, "claude-sonnet-4-20250514"); - assert!(manifest.provider.api_key_env.is_none()); + assert!(manifest.provider.api_key_file.is_none()); assert!(manifest.scope.is_none()); assert!(manifest.worker.system_prompt.is_none()); } @@ -109,7 +124,7 @@ name = "code-reviewer" [provider] kind = "anthropic" model = "claude-sonnet-4-20250514" -api_key_env = "ANTHROPIC_API_KEY" +api_key_file = "~/.config/insomnia/keys/anthropic" [worker] system_prompt = "You are a code reviewer." @@ -122,8 +137,8 @@ root = "./src" let manifest = PodManifest::from_toml(toml).unwrap(); assert_eq!(manifest.pod.name, "code-reviewer"); assert_eq!( - manifest.provider.api_key_env.as_deref(), - Some("ANTHROPIC_API_KEY") + manifest.provider.api_key_file.as_deref(), + Some(std::path::Path::new("~/.config/insomnia/keys/anthropic")) ); assert_eq!( manifest.worker.system_prompt.as_deref(), @@ -151,7 +166,7 @@ model = "llama3" "#; let manifest = PodManifest::from_toml(toml).unwrap(); assert_eq!(manifest.provider.kind, ProviderKind::Ollama); - assert!(manifest.provider.api_key_env.is_none()); + assert!(manifest.provider.api_key_file.is_none()); } #[test] diff --git a/crates/pod/examples/pod_cli.rs b/crates/pod/examples/pod_cli.rs index 8b4e4b82..e6376385 100644 --- a/crates/pod/examples/pod_cli.rs +++ b/crates/pod/examples/pod_cli.rs @@ -21,7 +21,6 @@ name = "hello-pod" [provider] kind = "anthropic" model = "claude-sonnet-4-20250514" -api_key_env = "ANTHROPIC_API_KEY" [worker] system_prompt = "You are a concise assistant. Reply in one or two sentences." @@ -41,7 +40,7 @@ async fn main() -> Result<(), Box> { let store = FsStore::new(tmp.path()).await?; // 3. Build the Pod from manifest - let mut pod = Pod::from_manifest(manifest, store, None).await?; + let mut pod = Pod::from_manifest(manifest, store, None, None).await?; println!("Session: {}", pod.session_id()); // 4. Run a prompt diff --git a/crates/pod/examples/pod_protocol.rs b/crates/pod/examples/pod_protocol.rs index 516eb6c8..991ba884 100644 --- a/crates/pod/examples/pod_protocol.rs +++ b/crates/pod/examples/pod_protocol.rs @@ -15,7 +15,6 @@ name = "protocol-demo" [provider] kind = "anthropic" model = "claude-sonnet-4-20250514" -api_key_env = "ANTHROPIC_API_KEY" [worker] system_prompt = "You are a concise assistant. Reply in one or two sentences." @@ -29,7 +28,7 @@ async fn main() -> Result<(), Box> { let manifest = PodManifest::from_toml(MANIFEST_TOML)?; let tmp = tempfile::tempdir()?; let store = FsStore::new(tmp.path()).await?; - let pod = pod::Pod::from_manifest(manifest, store, None).await?; + let pod = pod::Pod::from_manifest(manifest, store, None, None).await?; let runtime_tmp = tempfile::tempdir()?; let handle = PodController::spawn(pod, runtime_tmp.path()).await?; diff --git a/crates/pod/src/main.rs b/crates/pod/src/main.rs index 45da5376..beda01fb 100644 --- a/crates/pod/src/main.rs +++ b/crates/pod/src/main.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::ExitCode; use clap::Parser; @@ -84,7 +84,10 @@ async fn main() -> ExitCode { }; // Build the Pod - let pod = match Pod::from_manifest(manifest, store, scope).await { + let manifest_dir = std::fs::canonicalize(&cli.manifest) + .ok() + .and_then(|p| p.parent().map(Path::to_path_buf)); + let pod = match Pod::from_manifest(manifest, store, scope, manifest_dir).await { Ok(p) => p, Err(e) => { eprintln!("error: failed to create pod: {e}"); diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index 508e38f4..dcf263f7 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -1,3 +1,4 @@ +use std::path::PathBuf; use std::sync::Arc; use llm_worker::llm_client::client::LlmClient; @@ -173,8 +174,9 @@ impl Pod, St> { manifest: PodManifest, store: St, scope: Option, + manifest_dir: Option, ) -> Result { - let client = provider::build_client(&manifest.provider)?; + let client = provider::build_client(&manifest.provider, manifest_dir.as_deref())?; let mut worker = Worker::new(client); apply_worker_manifest(&mut worker, &manifest.worker); let session = Session::new(worker, store, SessionConfig::default()).await?; diff --git a/crates/provider/Cargo.toml b/crates/provider/Cargo.toml index 3a365b78..a1930ccd 100644 --- a/crates/provider/Cargo.toml +++ b/crates/provider/Cargo.toml @@ -8,3 +8,7 @@ license.workspace = true llm-worker = { version = "0.2.1", path = "../llm-worker" } manifest = { version = "0.1.0", path = "../manifest" } thiserror = "2.0" + +[dev-dependencies] +serial_test = "3.4.0" +tempfile = "3.27.0" diff --git a/crates/provider/src/lib.rs b/crates/provider/src/lib.rs index c68de706..48534a8a 100644 --- a/crates/provider/src/lib.rs +++ b/crates/provider/src/lib.rs @@ -1,3 +1,5 @@ +use std::path::{Path, PathBuf}; + use llm_worker::llm_client::client::LlmClient; use llm_worker::llm_client::providers::anthropic::AnthropicClient; use llm_worker::llm_client::providers::gemini::GeminiClient; @@ -11,23 +13,79 @@ use manifest::{ProviderConfig, ProviderKind}; pub enum ProviderError { #[error("provider configuration error: {0}")] Config(String), + + #[error("API key not provided for {provider}")] + ApiKeyMissing { provider: String }, +} + +/// Resolve the API key for the given provider configuration. +/// +/// Resolution order: +/// 1. Environment variable `INSOMNIA_API_KEY_{KIND}` +/// 2. File specified by `api_key_file` (trimmed) +/// 3. `None` +fn resolve_api_key( + config: &ProviderConfig, + manifest_dir: Option<&Path>, +) -> Result, ProviderError> { + // 1. Convention-based environment variable + let env_name = config.kind.env_var_name(); + if let Ok(val) = std::env::var(&env_name) { + return Ok(Some(val)); + } + + // 2. File + if let Some(ref raw_path) = config.api_key_file { + let path = expand_key_path(raw_path, manifest_dir)?; + let contents = std::fs::read_to_string(&path).map_err(|e| { + ProviderError::Config(format!("failed to read api_key_file {}: {e}", path.display())) + })?; + return Ok(Some(contents.trim().to_owned())); + } + + Ok(None) +} + +/// Expand `~` and resolve relative paths against `manifest_dir`. +fn expand_key_path( + raw: &Path, + manifest_dir: Option<&Path>, +) -> Result { + let path = if raw.starts_with("~") { + let home = std::env::var("HOME") + .map_err(|_| ProviderError::Config("HOME is not set for ~ expansion".into()))?; + PathBuf::from(home).join(raw.strip_prefix("~").unwrap()) + } else { + raw.to_path_buf() + }; + + if path.is_relative() { + match manifest_dir { + Some(dir) => Ok(dir.join(&path)), + None => Err(ProviderError::Config(format!( + "relative api_key_file '{}' requires a manifest directory", + path.display() + ))), + } + } else { + Ok(path) + } } /// Build an [`LlmClient`] from a [`ProviderConfig`]. /// -/// Resolves the API key from the environment variable specified in the config. -pub fn build_client(config: &ProviderConfig) -> Result, ProviderError> { - let api_key = config - .api_key_env - .as_deref() - .map(std::env::var) - .transpose() - .map_err(|e| ProviderError::Config(format!("env var: {e}")))?; +/// Resolves the API key from `INSOMNIA_API_KEY_{KIND}` env var or `api_key_file`. +/// `manifest_dir` is used to resolve relative `api_key_file` paths. +pub fn build_client( + config: &ProviderConfig, + manifest_dir: Option<&Path>, +) -> Result, ProviderError> { + let api_key = resolve_api_key(config, manifest_dir)?; match config.kind { ProviderKind::Anthropic => { - let key = api_key.ok_or_else(|| { - ProviderError::Config("anthropic requires api_key_env".into()) + let key = api_key.ok_or_else(|| ProviderError::ApiKeyMissing { + provider: "anthropic".into(), })?; let mut client = AnthropicClient::new(key, &config.model); if let Some(ref url) = config.base_url { @@ -36,8 +94,8 @@ pub fn build_client(config: &ProviderConfig) -> Result, Provi Ok(Box::new(client)) } ProviderKind::Openai => { - let key = api_key.ok_or_else(|| { - ProviderError::Config("openai requires api_key_env".into()) + let key = api_key.ok_or_else(|| ProviderError::ApiKeyMissing { + provider: "openai".into(), })?; let mut client = OpenAIClient::new(key, &config.model); if let Some(ref url) = config.base_url { @@ -46,8 +104,8 @@ pub fn build_client(config: &ProviderConfig) -> Result, Provi Ok(Box::new(client)) } ProviderKind::Gemini => { - let key = api_key.ok_or_else(|| { - ProviderError::Config("gemini requires api_key_env".into()) + let key = api_key.ok_or_else(|| ProviderError::ApiKeyMissing { + provider: "gemini".into(), })?; let mut client = GeminiClient::new(key, &config.model); if let Some(ref url) = config.base_url { @@ -64,3 +122,107 @@ pub fn build_client(config: &ProviderConfig) -> Result, Provi } } } + +#[cfg(test)] +mod tests { + use super::*; + use serial_test::serial; + use std::io::Write; + + fn anthropic_config() -> ProviderConfig { + ProviderConfig { + kind: ProviderKind::Anthropic, + model: "test-model".into(), + api_key_file: None, + base_url: None, + } + } + + #[test] + #[serial] + fn resolve_from_env() { + let env_name = ProviderKind::Anthropic.env_var_name(); + unsafe { std::env::set_var(&env_name, "sk-from-env") }; + let key = resolve_api_key(&anthropic_config(), None).unwrap(); + unsafe { std::env::remove_var(&env_name) }; + assert_eq!(key.as_deref(), Some("sk-from-env")); + } + + #[test] + fn resolve_from_file() { + let dir = tempfile::tempdir().unwrap(); + let key_path = dir.path().join("key.txt"); + { + let mut f = std::fs::File::create(&key_path).unwrap(); + write!(f, " sk-from-file\n").unwrap(); + } + let config = ProviderConfig { + api_key_file: Some(key_path), + ..anthropic_config() + }; + let key = resolve_api_key(&config, None).unwrap(); + assert_eq!(key.as_deref(), Some("sk-from-file")); + } + + #[test] + #[serial] + fn env_takes_precedence_over_file() { + let dir = tempfile::tempdir().unwrap(); + let key_path = dir.path().join("key.txt"); + std::fs::write(&key_path, "sk-from-file").unwrap(); + + let env_name = ProviderKind::Anthropic.env_var_name(); + unsafe { std::env::set_var(&env_name, "sk-from-env") }; + + let config = ProviderConfig { + api_key_file: Some(key_path), + ..anthropic_config() + }; + let key = resolve_api_key(&config, None).unwrap(); + unsafe { std::env::remove_var(&env_name) }; + assert_eq!(key.as_deref(), Some("sk-from-env")); + } + + #[test] + fn relative_path_resolved_against_manifest_dir() { + let dir = tempfile::tempdir().unwrap(); + let key_path = dir.path().join("keys").join("anthropic"); + std::fs::create_dir_all(key_path.parent().unwrap()).unwrap(); + std::fs::write(&key_path, "sk-relative").unwrap(); + + let config = ProviderConfig { + api_key_file: Some(PathBuf::from("keys/anthropic")), + ..anthropic_config() + }; + let key = resolve_api_key(&config, Some(dir.path())).unwrap(); + assert_eq!(key.as_deref(), Some("sk-relative")); + } + + #[test] + fn relative_path_without_manifest_dir_errors() { + let config = ProviderConfig { + api_key_file: Some(PathBuf::from("keys/anthropic")), + ..anthropic_config() + }; + let err = resolve_api_key(&config, None).unwrap_err(); + assert!(matches!(err, ProviderError::Config(_))); + } + + #[test] + fn missing_key_returns_api_key_missing() { + let config = anthropic_config(); + let result = build_client(&config, None); + assert!(matches!(result, Err(ProviderError::ApiKeyMissing { .. }))); + } + + #[test] + fn ollama_succeeds_without_key() { + let config = ProviderConfig { + kind: ProviderKind::Ollama, + model: "llama3".into(), + api_key_file: None, + base_url: None, + }; + assert!(build_client(&config, None).is_ok()); + } +} diff --git a/tickets/api-key-file.md b/tickets/api-key-file.md deleted file mode 100644 index 1f20db4d..00000000 --- a/tickets/api-key-file.md +++ /dev/null @@ -1,44 +0,0 @@ -# api_key_file: ファイルパスによるAPIキー解決 - -## 背景 - -現状、APIキーの取得手段は `api_key_env`（環境変数名の指定）のみ。 -永続化やインタラクティブ入力の仕組みがなく、キー管理をユーザーのシェル環境に完全依存している。 - -## やること - -マニフェストの `ProviderConfig` に `api_key_file: Option` を追加し、ファイルからAPIキーを読み取れるようにする。 - -### マニフェスト - -```toml -[provider] -kind = "anthropic" -model = "claude-sonnet-4-20250514" -api_key_file = "~/.config/insomnia/keys/anthropic" -``` - -- ファイルにはキーのみを記載（読み込み時に trim） -- `~` 展開が必要 -- 相対パスはマニフェストファイルの位置基準 - -### api_key_env との関係 - -- 排他。両方指定されたらエラー -- Ollama は両方不要のまま - -### 変更箇所 - -1. **manifest**: `ProviderConfig` に `api_key_file: Option` を追加 -2. **provider**: `build_client()` でファイル読み取りロジックを追加。排他バリデーション -3. **provider**: `ProviderError` にキー不在を明示するバリアント追加（将来の TUI フォールバック用） - -### 暗号化について - -現段階では扱わない。ファイルパーミッション（0600）で十分。 -将来エンドユーザー向けに暗号化が必要になった場合、provider の手前に復号レイヤーを挟む形で対応できる。`api_key_file` の設計自体は変更不要。 - -### 将来の拡張 - -- TUI サブコマンド（`insomnia key set anthropic` 等）がこのファイルに書き込むラッパーになる -- `api_key_cmd`（コマンド実行によるキー取得）は `api_key_file` で不足が生じた時点で検討