From 4921690b5ecb1775c779930b4f91c19d9504e2ea Mon Sep 17 00:00:00 2001 From: sky5454 Date: Sat, 14 Mar 2026 04:52:39 +0800 Subject: [PATCH 1/6] feat(credential): add AES-GCM encryption, SecureStore, and onboard keygen - pkg/credential: new package with AES-256-GCM enc:// credential format, HKDF-SHA256 key derivation (passphrase + optional SSH key binding), ErrPassphraseRequired / ErrDecryptionFailed sentinel errors, and PassphraseProvider hook for runtime passphrase injection - pkg/credential/store: lock-free SecureStore via atomic.Pointer[string]; passphrase never written to disk or os.Environ - pkg/credential/keygen: ed25519 SSH key generation helper used by onboard - pkg/config: replace os.Getenv(PassphraseEnvVar) with credential.PassphraseProvider() at all three call sites so that LoadConfig and SaveConfig use whatever passphrase source is active - cmd/picoclaw/onboard: prompt for passphrase with echo-off, generate picoclaw-specific SSH key, re-encrypt existing config on re-onboard - docs/credential_encryption.md: design doc for the enc:// format --- cmd/picoclaw/internal/onboard/helpers.go | 121 +++++++- docs/credential_encryption.md | 165 +++++++++++ pkg/config/config.go | 62 +++- pkg/config/config_test.go | 353 ++++++++++++++++++++++- pkg/credential/credential.go | 319 ++++++++++++++++++++ pkg/credential/credential_test.go | 268 +++++++++++++++++ pkg/credential/keygen.go | 62 ++++ pkg/credential/keygen_test.go | 109 +++++++ pkg/credential/store.go | 44 +++ pkg/credential/store_test.go | 65 +++++ 10 files changed, 1551 insertions(+), 17 deletions(-) create mode 100644 docs/credential_encryption.md create mode 100644 pkg/credential/credential.go create mode 100644 pkg/credential/credential_test.go create mode 100644 pkg/credential/keygen.go create mode 100644 pkg/credential/keygen_test.go create mode 100644 pkg/credential/store.go create mode 100644 pkg/credential/store_test.go diff --git a/cmd/picoclaw/internal/onboard/helpers.go b/cmd/picoclaw/internal/onboard/helpers.go index 4db8bdc8ba..365dcffa5c 100644 --- a/cmd/picoclaw/internal/onboard/helpers.go +++ b/cmd/picoclaw/internal/onboard/helpers.go @@ -8,23 +8,63 @@ import ( "github.com/sipeed/picoclaw/cmd/picoclaw/internal" "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/credential" + "golang.org/x/term" ) func onboard() { configPath := internal.GetConfigPath() + configExists := false if _, err := os.Stat(configPath); err == nil { - fmt.Printf("Config already exists at %s\n", configPath) - fmt.Print("Overwrite? (y/n): ") - var response string - fmt.Scanln(&response) - if response != "y" { - fmt.Println("Aborted.") - return + configExists = true + // Only ask for confirmation when *both* config and SSH key already exist, + // indicating a full re-onboard that would reset the config to defaults. + sshKeyPath, _ := credential.DefaultSSHKeyPath() + if _, err := os.Stat(sshKeyPath); err == nil { + // Both exist — confirm a full reset. + fmt.Printf("Config already exists at %s\n", configPath) + fmt.Print("Overwrite config with defaults? (y/n): ") + var response string + fmt.Scanln(&response) + if response != "y" { + fmt.Println("Aborted.") + return + } + configExists = false // user agreed to reset; treat as fresh } + // Config exists but SSH key is missing — keep existing config, only add SSH key. + } + + fmt.Println("\nSet up credential encryption") + fmt.Println("-----------------------------") + passphrase, err := promptPassphrase() + if err != nil { + fmt.Printf("Error: %v\n", err) + os.Exit(1) + } + // Expose the passphrase to credential.PassphraseProvider (which calls + // os.Getenv by default) so that SaveConfig can encrypt api_keys. + // This process is a one-shot CLI tool; the env var is never exposed outside + // the current process and disappears when it exits. + os.Setenv(credential.PassphraseEnvVar, passphrase) + + if err := setupSSHKey(); err != nil { + fmt.Printf("Error generating SSH key: %v\n", err) + os.Exit(1) } - cfg := config.DefaultConfig() + var cfg *config.Config + if configExists { + // Preserve the existing config; SaveConfig will re-encrypt api_keys with the new passphrase. + cfg, err = config.LoadConfig(configPath) + if err != nil { + fmt.Printf("Error loading existing config: %v\n", err) + os.Exit(1) + } + } else { + cfg = config.DefaultConfig() + } if err := config.SaveConfig(configPath, cfg); err != nil { fmt.Printf("Error saving config: %v\n", err) os.Exit(1) @@ -33,9 +73,13 @@ func onboard() { workspace := cfg.WorkspacePath() createWorkspaceTemplates(workspace) - fmt.Printf("%s picoclaw is ready!\n", internal.Logo) + fmt.Printf("\n%s picoclaw is ready!\n", internal.Logo) fmt.Println("\nNext steps:") - fmt.Println(" 1. Add your API key to", configPath) + fmt.Println(" 1. Set your encryption passphrase before starting picoclaw:") + fmt.Println(" export PICOCLAW_KEY_PASSPHRASE= # Linux/macOS") + fmt.Println(" set PICOCLAW_KEY_PASSPHRASE= # Windows cmd") + fmt.Println("") + fmt.Println(" 2. Add your API key to", configPath) fmt.Println("") fmt.Println(" Recommended:") fmt.Println(" - OpenRouter: https://openrouter.ai/keys (access 100+ models)") @@ -43,7 +87,62 @@ func onboard() { fmt.Println("") fmt.Println(" See README.md for 17+ supported providers.") fmt.Println("") - fmt.Println(" 2. Chat: picoclaw agent -m \"Hello!\"") + fmt.Println(" 3. Chat: picoclaw agent -m \"Hello!\"") +} + +// promptPassphrase reads the encryption passphrase twice from the terminal +// (with echo disabled) and returns it. Returns an error if the passphrase is +// empty or if the two inputs do not match. +func promptPassphrase() (string, error) { + fmt.Print("Enter passphrase for credential encryption: ") + p1, err := term.ReadPassword(int(os.Stdin.Fd())) + fmt.Println() + if err != nil { + return "", fmt.Errorf("reading passphrase: %w", err) + } + if len(p1) == 0 { + return "", fmt.Errorf("passphrase must not be empty") + } + + fmt.Print("Confirm passphrase: ") + p2, err := term.ReadPassword(int(os.Stdin.Fd())) + fmt.Println() + if err != nil { + return "", fmt.Errorf("reading passphrase confirmation: %w", err) + } + + if string(p1) != string(p2) { + return "", fmt.Errorf("passphrases do not match") + } + return string(p1), nil +} + +// setupSSHKey generates the picoclaw-specific SSH key at ~/.ssh/picoclaw_ed25519.key. +// If the key already exists the user is warned and asked to confirm overwrite. +// Answering anything other than "y" keeps the existing key (not an error). +func setupSSHKey() error { + keyPath, err := credential.DefaultSSHKeyPath() + if err != nil { + return fmt.Errorf("cannot determine SSH key path: %w", err) + } + + if _, err := os.Stat(keyPath); err == nil { + fmt.Printf("\n⚠️ WARNING: %s already exists.\n", keyPath) + fmt.Println(" Overwriting will invalidate any credentials previously encrypted with this key.") + fmt.Print(" Overwrite? (y/n): ") + var response string + fmt.Scanln(&response) + if response != "y" { + fmt.Println("Keeping existing SSH key.") + return nil + } + } + + if err := credential.GenerateSSHKey(keyPath); err != nil { + return err + } + fmt.Printf("SSH key generated: %s\n", keyPath) + return nil } func createWorkspaceTemplates(workspace string) { diff --git a/docs/credential_encryption.md b/docs/credential_encryption.md new file mode 100644 index 0000000000..4a8f12457f --- /dev/null +++ b/docs/credential_encryption.md @@ -0,0 +1,165 @@ +# Credential Encryption + +PicoClaw supports encrypting `api_key` values in `model_list` configuration entries. +Encrypted keys are stored as `enc://` strings and decrypted automatically at startup. + +--- + +## Quick Start + +**1. Set your passphrase** + +```bash +export PICOCLAW_KEY_PASSPHRASE="your-passphrase" +``` + +**2. Encrypt an API key** (using the built-in CLI subcommand, or Go API) + +```bash +picoclaw encrypt sk-your-openai-key +# outputs: enc://AAAA...base64... +``` + +**3. Paste the output into your config** + +```json +{ + "model_list": [ + { + "model_name": "gpt-4o", + "api_key": "enc://AAAA...base64...", + "base_url": "https://api.openai.com/v1" + } + ] +} +``` + +--- + +## Supported `api_key` Formats + +| Format | Example | Behaviour | +|--------|---------|-----------| +| Plaintext | `sk-abc123` | Used as-is | +| File reference | `file://openai.key` | Content read from the same directory as the config file | +| Encrypted | `enc://` | Decrypted at startup using `PICOCLAW_KEY_PASSPHRASE` | +| Empty | `""` | Passed through unchanged (used with `auth_method: oauth`) | + +--- + +## Cryptographic Design + +### Key Derivation + +Encryption uses **HKDF-SHA256** with an optional SSH private key as a second factor. + +``` +Without SSH key (passphrase only): + + ikm = SHA256(passphrase) + aes_key = HKDF-SHA256(ikm, salt, info="picoclaw-credential-v1", 32 bytes) + + +With SSH key (recommended): + + sshHash = SHA256(ssh_private_key_file_bytes) + ikm = HMAC-SHA256(key=sshHash, message=passphrase) + aes_key = HKDF-SHA256(ikm, salt, info="picoclaw-credential-v1", 32 bytes) +``` + +### Encryption + +``` +AES-256-GCM(key=aes_key, nonce=random[12], plaintext=api_key) +``` + +### Wire Format + +``` +enc:// +``` + +| Field | Size | Description | +|-------|------|-------------| +| `salt` | 16 bytes | Random per encryption; fed into HKDF | +| `nonce` | 12 bytes | Random per encryption; AES-GCM IV | +| `ciphertext` | variable | AES-256-GCM ciphertext + 16-byte authentication tag | + +The GCM authentication tag is appended to the ciphertext automatically. Any tampering causes decryption to fail with an error rather than returning corrupt plaintext. + +### Performance + +| Operation | Time (ARM Cortex-A) | +|-----------|---------------------| +| Key derivation (HKDF) | < 1 ms | +| AES-256-GCM decrypt | < 1 ms | +| **Total startup overhead** | **< 2 ms per key** | + +--- + +## Two-Factor Security with SSH Key + +When a SSH private key is provided, breaking the encryption requires **both**: + +1. The **passphrase** (`PICOCLAW_KEY_PASSPHRASE`) +2. The **SSH private key file** + +This means a leaked config file alone is not sufficient to recover the API key, even if the passphrase is weak. The SSH key contributes 256 bits of entropy (Ed25519) regardless of passphrase strength. + +### Threat Model + +| Attacker Has | Can Decrypt? | +|---|---| +| Config file only | No — needs passphrase + SSH key | +| SSH key only | No — needs passphrase | +| Passphrase only | No — needs SSH key | +| Config file + SSH key + passphrase | Yes — full compromise | + +--- + +## Environment Variables + +| Variable | Required | Description | +|----------|----------|-------------| +| `PICOCLAW_KEY_PASSPHRASE` | Yes (for `enc://`) | Passphrase used for key derivation | +| `PICOCLAW_SSH_KEY_PATH` | No | Path to SSH private key. Set to `""` to disable auto-detection and use passphrase-only mode | + +### SSH Key Auto-Detection + +If `PICOCLAW_SSH_KEY_PATH` is not set, PicoClaw looks for the picoclaw-specific key: + +``` +~/.ssh/picoclaw_ed25519.key +``` + +This dedicated file avoids conflicts with the user's existing SSH keys. +Run `picoclaw onboard` to generate it automatically. + +`os.UserHomeDir()` is used for cross-platform home directory resolution (reads `USERPROFILE` on Windows, `HOME` on Unix/macOS). + +To explicitly disable SSH key usage and use passphrase-only mode: + +```bash +export PICOCLAW_SSH_KEY_PATH="" +``` + +--- + +## Migration + +Because the only secret material is `PICOCLAW_KEY_PASSPHRASE` and the SSH private key file, migration is straightforward: + +1. Copy the config file to the new machine. +2. Set `PICOCLAW_KEY_PASSPHRASE` to the same value. +3. Copy the SSH private key file to the same path (or set `PICOCLAW_SSH_KEY_PATH` to its new location). + +No re-encryption is needed. + +--- + +## Security Considerations + +- **Passphrase strength matters in passphrase-only mode.** Without an SSH key, a weak passphrase can be brute-forced offline. Use `PICOCLAW_SSH_KEY_PATH=""` only in environments where no SSH key is available and the passphrase is sufficiently strong (≥ 32 random characters). +- **The SSH key is read-only at runtime.** PicoClaw never writes to or modifies the SSH key file. +- **Plaintext keys remain supported.** Existing configs without `enc://` are unaffected. +- **The `enc://` format is versioned** via the HKDF `info` field (`picoclaw-credential-v1`), allowing future algorithm upgrades without breaking existing encrypted values. diff --git a/pkg/config/config.go b/pkg/config/config.go index 7a7edb4894..638b84a1c7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,14 +1,17 @@ package config import ( + "bytes" "encoding/json" "fmt" "os" + "path/filepath" "strings" "sync/atomic" "github.com/caarlos0/env/v11" + "github.com/sipeed/picoclaw/pkg/credential" "github.com/sipeed/picoclaw/pkg/fileutil" ) @@ -829,10 +832,22 @@ func LoadConfig(path string) (*Config, error) { return nil, err } + if passphrase := credential.PassphraseProvider(); passphrase != "" { + for _, m := range cfg.ModelList { + if m.APIKey != "" && !strings.HasPrefix(m.APIKey, "enc://") && !strings.HasPrefix(m.APIKey, "file://") { + fmt.Fprintf(os.Stderr, "picoclaw: warning: model %q has a plaintext api_key; call SaveConfig to encrypt it\n", m.ModelName) + } + } + } + if err := env.Parse(cfg); err != nil { return nil, err } + if err := resolveAPIKeys(cfg.ModelList, filepath.Dir(path)); err != nil { + return nil, err + } + // Migrate legacy channel config fields to new unified structures cfg.migrateChannelConfigs() @@ -849,6 +864,46 @@ func LoadConfig(path string) (*Config, error) { return cfg, nil } +// encryptPlaintextAPIKeys rewrites plaintext api_key values in raw JSON using +// the entries already parsed into models. Returns the rewritten bytes when at +// least one key was sealed, or nil when nothing changed. +func encryptPlaintextAPIKeys(models []ModelConfig, passphrase string, raw []byte) []byte { + result := raw + changed := false + for _, m := range models { + if m.APIKey == "" || strings.HasPrefix(m.APIKey, "enc://") || strings.HasPrefix(m.APIKey, "file://") { + continue + } + encrypted, err := credential.Encrypt(passphrase, "", m.APIKey) + if err != nil { + fmt.Fprintf(os.Stderr, "picoclaw: warning: cannot seal api_key in config: %v\n", err) + continue + } + oldJSON, _ := json.Marshal(m.APIKey) + newJSON, _ := json.Marshal(encrypted) + result = bytes.Replace(result, oldJSON, newJSON, 1) + changed = true + } + if !changed { + return nil + } + return result +} + +// resolveAPIKeys decrypts or dereferences each api_key in models in-place. +// Supports plaintext (no-op), file:// (read from configDir), and enc:// (AES-GCM decrypt). +func resolveAPIKeys(models []ModelConfig, configDir string) error { + cr := credential.NewResolver(configDir) + for i := range models { + resolved, err := cr.Resolve(models[i].APIKey) + if err != nil { + return fmt.Errorf("model_list[%d] (%s): %w", i, models[i].ModelName, err) + } + models[i].APIKey = resolved + } + return nil +} + func (c *Config) migrateChannelConfigs() { // Discord: mention_only -> group_trigger.mention_only if c.Channels.Discord.MentionOnly && !c.Channels.Discord.GroupTrigger.MentionOnly { @@ -868,7 +923,12 @@ func SaveConfig(path string, cfg *Config) error { return err } - // Use unified atomic write utility with explicit sync for flash storage reliability. + if passphrase := credential.PassphraseProvider(); passphrase != "" { + if sealed := encryptPlaintextAPIKeys(cfg.ModelList, passphrase, data); sealed != nil { + data = sealed + } + } + return fileutil.WriteFileAtomic(path, data, 0o600) } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 1c93028c7c..20dfed8919 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -7,6 +7,8 @@ import ( "runtime" "strings" "testing" + + "github.com/sipeed/picoclaw/pkg/credential" ) func TestAgentModelConfig_UnmarshalString(t *testing.T) { @@ -482,13 +484,19 @@ func TestDefaultConfig_DMScope(t *testing.T) { } func TestDefaultConfig_WorkspacePath_Default(t *testing.T) { - // Unset to ensure we test the default t.Setenv("PICOCLAW_HOME", "") - // Set a known home for consistent test results - t.Setenv("HOME", "/tmp/home") + + var fakeHome string + if runtime.GOOS == "windows" { + fakeHome = `C:\tmp\home` + t.Setenv("USERPROFILE", fakeHome) + } else { + fakeHome = "/tmp/home" + t.Setenv("HOME", fakeHome) + } cfg := DefaultConfig() - want := filepath.Join("/tmp/home", ".picoclaw", "workspace") + want := filepath.Join(fakeHome, ".picoclaw", "workspace") if cfg.Agents.Defaults.Workspace != want { t.Errorf("Default workspace path = %q, want %q", cfg.Agents.Defaults.Workspace, want) @@ -499,7 +507,7 @@ func TestDefaultConfig_WorkspacePath_WithPicoclawHome(t *testing.T) { t.Setenv("PICOCLAW_HOME", "/custom/picoclaw/home") cfg := DefaultConfig() - want := "/custom/picoclaw/home/workspace" + want := filepath.Join("/custom/picoclaw/home", "workspace") if cfg.Agents.Defaults.Workspace != want { t.Errorf("Workspace path with PICOCLAW_HOME = %q, want %q", cfg.Agents.Defaults.Workspace, want) @@ -621,3 +629,338 @@ func TestFlexibleStringSlice_UnmarshalText_EmptySliceConsistency(t *testing.T) { } }) } + +// TestLoadConfig_WarnsForPlaintextAPIKey verifies that LoadConfig resolves a plaintext +// api_key into memory but does NOT rewrite the config file. File writes are the sole +// responsibility of SaveConfig. +func TestLoadConfig_WarnsForPlaintextAPIKey(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + const original = `{"model_list":[{"model_name":"test","model":"openai/gpt-4","api_key":"sk-plaintext"}]}` + if err := os.WriteFile(cfgPath, []byte(original), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + cfg, err := LoadConfig(cfgPath) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + // In-memory value must be the resolved plaintext. + if cfg.ModelList[0].APIKey != "sk-plaintext" { + t.Errorf("in-memory api_key = %q, want %q", cfg.ModelList[0].APIKey, "sk-plaintext") + } + // The file on disk must remain unchanged — LoadConfig must not write anything. + raw, _ := os.ReadFile(cfgPath) + if string(raw) != original { + t.Errorf("LoadConfig must not modify the config file; got:\n%s", string(raw)) + } +} + +// TestSaveConfig_EncryptsPlaintextAPIKey verifies that SaveConfig writes enc:// ciphertext +// to disk and that a subsequent LoadConfig decrypts it back to the original plaintext. +func TestSaveConfig_EncryptsPlaintextAPIKey(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + cfg := DefaultConfig() + cfg.ModelList = []ModelConfig{ + {ModelName: "test", Model: "openai/gpt-4", APIKey: "sk-plaintext"}, + } + if err := SaveConfig(cfgPath, cfg); err != nil { + t.Fatalf("SaveConfig: %v", err) + } + + // Disk must contain enc://, not the raw key. + raw, _ := os.ReadFile(cfgPath) + if !strings.Contains(string(raw), "enc://") { + t.Errorf("saved file should contain enc://, got:\n%s", string(raw)) + } + if strings.Contains(string(raw), "sk-plaintext") { + t.Errorf("saved file must not contain the plaintext key") + } + + // A fresh load must decrypt back to the original plaintext. + cfg2, err := LoadConfig(cfgPath) + if err != nil { + t.Fatalf("LoadConfig after SaveConfig: %v", err) + } + if cfg2.ModelList[0].APIKey != "sk-plaintext" { + t.Errorf("loaded api_key = %q, want %q", cfg2.ModelList[0].APIKey, "sk-plaintext") + } +} + +// TestLoadConfig_NoSealWithoutPassphrase verifies that api_key values are left +// unchanged when PICOCLAW_KEY_PASSPHRASE is not set. +func TestLoadConfig_NoSealWithoutPassphrase(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + data := `{"model_list":[{"model_name":"test","model":"openai/gpt-4","api_key":"sk-plaintext"}]}` + if err := os.WriteFile(cfgPath, []byte(data), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + if _, err := LoadConfig(cfgPath); err != nil { + t.Fatalf("LoadConfig: %v", err) + } + + raw, _ := os.ReadFile(cfgPath) + if strings.Contains(string(raw), "enc://") { + t.Error("config file must not be modified when no passphrase is set") + } +} + +// TestLoadConfig_FileRefNotSealed verifies that file:// api_key references are not +// converted to enc:// values (they are resolved at runtime by the Resolver). +func TestLoadConfig_FileRefNotSealed(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + keyFile := filepath.Join(dir, "openai.key") + if err := os.WriteFile(keyFile, []byte("sk-from-file"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + data := `{"model_list":[{"model_name":"test","model":"openai/gpt-4","api_key":"file://openai.key"}]}` + if err := os.WriteFile(cfgPath, []byte(data), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + if _, err := LoadConfig(cfgPath); err != nil { + t.Fatalf("LoadConfig: %v", err) + } + + raw, _ := os.ReadFile(cfgPath) + if !strings.Contains(string(raw), "file://openai.key") { + t.Error("file:// reference should be preserved unchanged in the config file") + } + if strings.Contains(string(raw), "enc://") { + t.Error("file:// reference must not be converted to enc://") + } +} + +// TestSaveConfig_MixedKeys verifies that SaveConfig encrypts only plaintext api_keys +// and leaves already-encrypted (enc://) and file:// entries unchanged. +func TestSaveConfig_MixedKeys(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + // Pre-encrypt one key so we have a genuine enc:// value to put in the config. + if err := SaveConfig(cfgPath, &Config{ + ModelList: []ModelConfig{ + {ModelName: "pre", Model: "openai/gpt-4", APIKey: "sk-already-plain"}, + }, + }); err != nil { + t.Fatalf("setup SaveConfig: %v", err) + } + raw, _ := os.ReadFile(cfgPath) + // Extract the enc:// value from the saved file. + var tmp struct { + ModelList []struct { + APIKey string `json:"api_key"` + } `json:"model_list"` + } + if err := json.Unmarshal(raw, &tmp); err != nil || len(tmp.ModelList) == 0 { + t.Fatalf("setup: could not parse saved config: %v", err) + } + alreadyEncrypted := tmp.ModelList[0].APIKey + if !strings.HasPrefix(alreadyEncrypted, "enc://") { + t.Fatalf("setup: expected enc:// key, got %q", alreadyEncrypted) + } + + // Build a config with three models: + // 1. plaintext → must be encrypted by SaveConfig + // 2. enc:// → must be left unchanged (already encrypted) + // 3. file:// → must be left unchanged (file reference) + keyFile := filepath.Join(dir, "api.key") + if err := os.WriteFile(keyFile, []byte("sk-from-file"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + cfg := &Config{ + ModelList: []ModelConfig{ + {ModelName: "plain", Model: "openai/gpt-4", APIKey: "sk-new-plaintext"}, + {ModelName: "enc", Model: "openai/gpt-4", APIKey: alreadyEncrypted}, + {ModelName: "file", Model: "openai/gpt-4", APIKey: "file://api.key"}, + }, + } + if err := SaveConfig(cfgPath, cfg); err != nil { + t.Fatalf("SaveConfig: %v", err) + } + + raw, _ = os.ReadFile(cfgPath) + s := string(raw) + + // 1. Plaintext must be encrypted. + if strings.Contains(s, "sk-new-plaintext") { + t.Error("plaintext key must not appear in saved file") + } + // 2. The pre-existing enc:// value must still be present (byte-for-byte unchanged). + if !strings.Contains(s, alreadyEncrypted) { + t.Error("pre-existing enc:// entry must be preserved unchanged") + } + // 3. file:// must be preserved. + if !strings.Contains(s, "file://api.key") { + t.Error("file:// reference must be preserved unchanged") + } + + // Now load and verify all three decrypt/resolve correctly. + cfg2, err := LoadConfig(cfgPath) + if err != nil { + t.Fatalf("LoadConfig after SaveConfig: %v", err) + } + byName := make(map[string]string) + for _, m := range cfg2.ModelList { + byName[m.ModelName] = m.APIKey + } + if byName["plain"] != "sk-new-plaintext" { + t.Errorf("plain model api_key = %q, want %q", byName["plain"], "sk-new-plaintext") + } + if byName["enc"] != "sk-already-plain" { + t.Errorf("enc model api_key = %q, want %q", byName["enc"], "sk-already-plain") + } + if byName["file"] != "sk-from-file" { + t.Errorf("file model api_key = %q, want %q", byName["file"], "sk-from-file") + } +} + +// TestLoadConfig_MixedKeys_NoPassphrase verifies that when PICOCLAW_KEY_PASSPHRASE +// is not set, enc:// entries cause LoadConfig to return an error, while plaintext +// and file:// entries in the same config are not affected. +func TestLoadConfig_MixedKeys_NoPassphrase(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + + // First encrypt a key so we have a real enc:// value. + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + if err := SaveConfig(cfgPath, &Config{ + ModelList: []ModelConfig{ + {ModelName: "m", Model: "openai/gpt-4", APIKey: "sk-secret"}, + }, + }); err != nil { + t.Fatalf("setup SaveConfig: %v", err) + } + raw, _ := os.ReadFile(cfgPath) + var tmp struct { + ModelList []struct { + APIKey string `json:"api_key"` + } `json:"model_list"` + } + if err := json.Unmarshal(raw, &tmp); err != nil { + t.Fatalf("setup parse: %v", err) + } + encValue := tmp.ModelList[0].APIKey + + // Write a mixed config: enc:// + plaintext + file:// + keyFile := filepath.Join(dir, "api.key") + if err := os.WriteFile(keyFile, []byte("sk-from-file"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + mixed, _ := json.Marshal(map[string]any{ + "model_list": []map[string]any{ + {"model_name": "enc", "model": "openai/gpt-4", "api_key": encValue}, + {"model_name": "plain", "model": "openai/gpt-4", "api_key": "sk-plain"}, + {"model_name": "file", "model": "openai/gpt-4", "api_key": "file://api.key"}, + }, + }) + if err := os.WriteFile(cfgPath, mixed, 0o600); err != nil { + t.Fatalf("setup write: %v", err) + } + + // Now clear the passphrase — LoadConfig must fail because enc:// cannot be decrypted. + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "") + + _, err := LoadConfig(cfgPath) + if err == nil { + t.Fatal("LoadConfig should fail when enc:// key is present and no passphrase is set") + } + if !strings.Contains(err.Error(), "PICOCLAW_KEY_PASSPHRASE") { + t.Errorf("error should mention PICOCLAW_KEY_PASSPHRASE, got: %v", err) + } +} + +// TestSaveConfig_UsesPassphraseProvider verifies that SaveConfig encrypts plaintext +// api_keys using credential.PassphraseProvider() rather than os.Getenv directly. +// This matters for the launcher, which clears the environment variable and redirects +// PassphraseProvider to an in-memory SecureStore. +func TestSaveConfig_UsesPassphraseProvider(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + + // Ensure the env var is empty — passphrase must come from PassphraseProvider only. + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + // Replace PassphraseProvider with an in-memory function (simulating SecureStore). + const testPassphrase = "provider-passphrase" + orig := credential.PassphraseProvider + credential.PassphraseProvider = func() string { return testPassphrase } + t.Cleanup(func() { credential.PassphraseProvider = orig }) + + cfg := DefaultConfig() + cfg.ModelList = []ModelConfig{ + {ModelName: "test", Model: "openai/gpt-4", APIKey: "sk-plaintext"}, + } + if err := SaveConfig(cfgPath, cfg); err != nil { + t.Fatalf("SaveConfig: %v", err) + } + + raw, _ := os.ReadFile(cfgPath) + if !strings.Contains(string(raw), "enc://") { + t.Errorf("SaveConfig should have encrypted plaintext key via PassphraseProvider; got:\n%s", raw) + } +} + +// TestLoadConfig_UsesPassphraseProvider verifies that LoadConfig decrypts enc:// keys +// using credential.PassphraseProvider() rather than os.Getenv directly. +func TestLoadConfig_UsesPassphraseProvider(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + + // Ensure the env var is empty throughout. + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + const testPassphrase = "provider-passphrase" + const plainKey = "sk-secret" + + // First, encrypt the key using the same passphrase. + encrypted, err := credential.Encrypt(testPassphrase, "", plainKey) + if err != nil { + t.Fatalf("Encrypt: %v", err) + } + + raw, _ := json.Marshal(map[string]any{ + "model_list": []map[string]any{ + {"model_name": "test", "model": "openai/gpt-4", "api_key": encrypted}, + }, + }) + if err := os.WriteFile(cfgPath, raw, 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + // Redirect PassphraseProvider — env var is empty, so without this the load would fail. + orig := credential.PassphraseProvider + credential.PassphraseProvider = func() string { return testPassphrase } + t.Cleanup(func() { credential.PassphraseProvider = orig }) + + cfg, err := LoadConfig(cfgPath) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + if cfg.ModelList[0].APIKey != plainKey { + t.Errorf("api_key = %q, want %q", cfg.ModelList[0].APIKey, plainKey) + } +} diff --git a/pkg/credential/credential.go b/pkg/credential/credential.go new file mode 100644 index 0000000000..8372a59adf --- /dev/null +++ b/pkg/credential/credential.go @@ -0,0 +1,319 @@ +// Package credential resolves API credential values for model_list entries. +// +// An API key is a form of authorization credential. This package centralises +// how raw credential strings—plaintext or file references—are resolved into +// their actual values, keeping that logic out of the config loader. +// +// Supported formats for the api_key field: +// +// - Plaintext: "sk-abc123" → returned as-is +// - File ref: "file://filename.key" → content read from configDir/filename.key +// - Encrypted: "enc://" → AES-256-GCM decrypt via PICOCLAW_KEY_PASSPHRASE +// - Empty: "" → returned as-is (auth_method=oauth etc.) +// +// Encryption uses AES-256-GCM with HKDF-SHA256 key derivation (< 1ms, safe for embedded Linux). +// Key derivation: +// +// With SSH key: HKDF-SHA256(ikm=HMAC-SHA256(SHA256(sshKeyBytes), passphrase), salt, info) +// Without: HKDF-SHA256(ikm=SHA256(passphrase), salt, info) +// +// SSH key path resolution priority: +// +// 1. sshKeyPath argument to Encrypt (explicit) +// 2. PICOCLAW_SSH_KEY_PATH env var (set to "" to disable auto-detection) +// 3. ~/.ssh/picoclaw_ed25519.key (os.UserHomeDir is cross-platform) +package credential + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/hkdf" + "crypto/hmac" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" +) + +// PassphraseEnvVar is the environment variable that holds the encryption passphrase. +// Other packages (e.g. config) reference this constant to avoid duplicating the string. +const PassphraseEnvVar = "PICOCLAW_KEY_PASSPHRASE" + +// PassphraseProvider is the function used to retrieve the passphrase for enc:// +// credential decryption. It defaults to reading PICOCLAW_KEY_PASSPHRASE from the +// process environment. Replace it at startup to use a different source, such as +// an in-memory SecureStore, so that all LoadConfig() calls everywhere share the +// same passphrase source without needing os.Environ. +// +// Example (launcher main.go): +// +// credential.PassphraseProvider = apiHandler.passphraseStore.Get +var PassphraseProvider func() string = func() string { + return os.Getenv(PassphraseEnvVar) +} + +// ErrPassphraseRequired is returned when an enc:// credential is encountered but +// PICOCLAW_KEY_PASSPHRASE is not set. Callers can detect this with errors.Is to +// distinguish a missing-passphrase condition from other credential errors. +var ErrPassphraseRequired = errors.New("credential: enc:// key requires " + PassphraseEnvVar + " env var") + +// ErrDecryptionFailed is returned when an enc:// credential cannot be decrypted, +// indicating a wrong passphrase or SSH key. Callers can detect this with errors.Is. +var ErrDecryptionFailed = errors.New("credential: enc:// decryption failed (wrong passphrase or SSH key?)") + +const ( + fileScheme = "file://" + encScheme = "enc://" + hkdfInfo = "picoclaw-credential-v1" + saltLen = 16 + nonceLen = 12 + keyLen = 32 + sshKeyEnv = "PICOCLAW_SSH_KEY_PATH" +) + +// Resolver resolves raw credential strings for model_list api_key fields. +// File references are resolved relative to the directory of the config file. +type Resolver struct { + configDir string +} + +// NewResolver returns a Resolver that resolves file:// references relative to +// configDir (typically filepath.Dir of the config file path). +func NewResolver(configDir string) *Resolver { + return &Resolver{configDir: configDir} +} + +// Resolve returns the actual credential value for raw: +// +// - "" → "" (no error; auth_method=oauth needs no key) +// - "file://name.key" → trimmed content of configDir/name.key +// - anything else → raw unchanged (plaintext credential) +func (r *Resolver) Resolve(raw string) (string, error) { + if raw == "" { + return "", nil + } + + if strings.HasPrefix(raw, fileScheme) { + fileName := strings.TrimSpace(strings.TrimPrefix(raw, fileScheme)) + if fileName == "" { + return "", fmt.Errorf("credential: file:// reference has no filename") + } + + keyPath := filepath.Join(r.configDir, fileName) + // Prevent path traversal: "../../etc/passwd" or "/abs/path" must not escape configDir. + if !isWithinDir(keyPath, r.configDir) { + return "", fmt.Errorf("credential: file:// path escapes config directory") + } + data, err := os.ReadFile(keyPath) + if err != nil { + return "", fmt.Errorf("credential: failed to read credential file %q: %w", keyPath, err) + } + + value := strings.TrimSpace(string(data)) + if value == "" { + return "", fmt.Errorf("credential: credential file %q is empty", keyPath) + } + + return value, nil + } + + if strings.HasPrefix(raw, encScheme) { + return resolveEncrypted(raw) + } + + // Plaintext credential — return unchanged. + return raw, nil +} + +// resolveEncrypted decrypts an enc:// credential using PassphraseProvider. +func resolveEncrypted(raw string) (string, error) { + passphrase := PassphraseProvider() + if passphrase == "" { + return "", ErrPassphraseRequired + } + + sshKeyPath := pickSSHKeyPath("") // override="": consult env then auto-detect + + b64 := strings.TrimPrefix(raw, encScheme) + blob, err := base64.StdEncoding.DecodeString(b64) + if err != nil { + return "", fmt.Errorf("credential: enc:// invalid base64: %w", err) + } + if len(blob) < saltLen+nonceLen+1 { + return "", fmt.Errorf("credential: enc:// payload too short") + } + + salt := blob[:saltLen] + nonce := blob[saltLen : saltLen+nonceLen] + ciphertext := blob[saltLen+nonceLen:] + + key, err := deriveKey(passphrase, sshKeyPath, salt) + if err != nil { + return "", err + } + block, err := aes.NewCipher(key) + if err != nil { + return "", fmt.Errorf("credential: enc:// cipher init: %w", err) + } + gcm, err := cipher.NewGCM(block) + if err != nil { + return "", fmt.Errorf("credential: enc:// gcm init: %w", err) + } + + plaintext, err := gcm.Open(nil, nonce, ciphertext, nil) + if err != nil { + return "", fmt.Errorf("%w: %w", ErrDecryptionFailed, err) + } + return string(plaintext), nil +} + +// Encrypt encrypts plaintext and returns an enc:// credential string. +// +// passphrase is required (PICOCLAW_KEY_PASSPHRASE value). +// sshKeyPath is the SSH private key file to incorporate; pass "" to use +// PICOCLAW_SSH_KEY_PATH env var or ~/.ssh/ auto-detection, or set +// PICOCLAW_SSH_KEY_PATH="" before calling to force passphrase-only mode. +func Encrypt(passphrase, sshKeyPath, plaintext string) (string, error) { + if passphrase == "" { + return "", fmt.Errorf("credential: passphrase must not be empty") + } + sshKeyPath = pickSSHKeyPath(sshKeyPath) + + salt := make([]byte, saltLen) + if _, err := io.ReadFull(rand.Reader, salt); err != nil { + return "", fmt.Errorf("credential: failed to generate salt: %w", err) + } + + key, err := deriveKey(passphrase, sshKeyPath, salt) + if err != nil { + return "", err + } + block, err := aes.NewCipher(key) + if err != nil { + return "", fmt.Errorf("credential: cipher init: %w", err) + } + gcm, err := cipher.NewGCM(block) + if err != nil { + return "", fmt.Errorf("credential: gcm init: %w", err) + } + + nonce := make([]byte, nonceLen) + if _, err := io.ReadFull(rand.Reader, nonce); err != nil { + return "", fmt.Errorf("credential: failed to generate nonce: %w", err) + } + + ciphertext := gcm.Seal(nil, nonce, []byte(plaintext), nil) + blob := make([]byte, 0, saltLen+nonceLen+len(ciphertext)) + blob = append(blob, salt...) + blob = append(blob, nonce...) + blob = append(blob, ciphertext...) + return encScheme + base64.StdEncoding.EncodeToString(blob), nil +} + +// isWithinDir reports whether path is contained within (or equal to) dir. +// Uses filepath.IsLocal on the relative path for robust cross-platform traversal detection. +func isWithinDir(path, dir string) bool { + rel, err := filepath.Rel(filepath.Clean(dir), filepath.Clean(path)) + return err == nil && filepath.IsLocal(rel) +} + +// allowedSSHKeyPath reports whether path is in a permitted location for SSH key files: +// - exact match with PICOCLAW_SSH_KEY_PATH env var +// - within the PICOCLAW_HOME env var directory +// - within ~/.ssh/ +func allowedSSHKeyPath(path string) bool { + if path == "" { + return true // passphrase-only mode; no file will be read + } + clean := filepath.Clean(path) + + // Exact match with PICOCLAW_SSH_KEY_PATH. + if envPath, ok := os.LookupEnv(sshKeyEnv); ok && envPath != "" { + if clean == filepath.Clean(envPath) { + return true + } + } + + // Within PICOCLAW_HOME. + if picoHome := os.Getenv("PICOCLAW_HOME"); picoHome != "" { + if isWithinDir(clean, picoHome) { + return true + } + } + + // Within ~/.ssh/. + if userHome, err := os.UserHomeDir(); err == nil { + if isWithinDir(clean, filepath.Join(userHome, ".ssh")) { + return true + } + } + + return false +} + +// deriveKey derives a 32-byte AES-256 key from passphrase and optional SSH key. +// +// With SSH key: ikm = HMAC-SHA256(key=SHA256(sshKeyBytes), msg=passphrase) +// Without: ikm = SHA256(passphrase) +// Final key: HKDF-SHA256(ikm, salt, info="picoclaw-credential-v1", 32 bytes) +func deriveKey(passphrase, sshKeyPath string, salt []byte) ([]byte, error) { + var ikm []byte + if sshKeyPath != "" { + if !allowedSSHKeyPath(sshKeyPath) { + return nil, fmt.Errorf("credential: SSH key path %q is not in an allowed location (PICOCLAW_SSH_KEY_PATH, PICOCLAW_HOME, or ~/.ssh/)", sshKeyPath) + } + sshBytes, err := os.ReadFile(sshKeyPath) + if err != nil { + return nil, fmt.Errorf("credential: cannot read SSH key %q: %w", sshKeyPath, err) + } + sshHash := sha256.Sum256(sshBytes) + mac := hmac.New(sha256.New, sshHash[:]) + mac.Write([]byte(passphrase)) + ikm = mac.Sum(nil) + } else { + h := sha256.Sum256([]byte(passphrase)) + ikm = h[:] + } + + key, err := hkdf.Key(sha256.New, ikm, salt, hkdfInfo, keyLen) + if err != nil { + return nil, fmt.Errorf("credential: HKDF expand failed: %w", err) + } + return key, nil +} + +// pickSSHKeyPath returns the SSH private key path to use for encryption/decryption. +// +// Priority: +// 1. override (non-empty explicit argument) +// 2. PICOCLAW_SSH_KEY_PATH env var — if the variable is set (even to ""), auto-detection +// is skipped; set it to "" to force passphrase-only mode +// 3. ~/.ssh/picoclaw_ed25519.key (auto-detection) +// +// Returns "" when no key is found (passphrase-only mode). +func pickSSHKeyPath(override string) string { + if override != "" { + return override + } + if p, ok := os.LookupEnv(sshKeyEnv); ok { + return p // respect explicit setting, even if "" + } + return findDefaultSSHKey() +} + +// findDefaultSSHKey returns the picoclaw-specific SSH key path if it exists. +func findDefaultSSHKey() string { + p, err := DefaultSSHKeyPath() + if err != nil { + return "" + } + if _, err := os.Stat(p); err == nil { + return p + } + return "" +} diff --git a/pkg/credential/credential_test.go b/pkg/credential/credential_test.go new file mode 100644 index 0000000000..53e4480f40 --- /dev/null +++ b/pkg/credential/credential_test.go @@ -0,0 +1,268 @@ +package credential_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/sipeed/picoclaw/pkg/credential" +) + +func TestResolve_PlainKey(t *testing.T) { + r := credential.NewResolver(t.TempDir()) + got, err := r.Resolve("sk-plaintext-key") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "sk-plaintext-key" { + t.Fatalf("got %q, want %q", got, "sk-plaintext-key") + } +} + +func TestResolve_FileKey_Success(t *testing.T) { + dir := t.TempDir() + keyFile := "openai_plain.key" + if err := os.WriteFile(filepath.Join(dir, keyFile), []byte("sk-from-file\n"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + r := credential.NewResolver(dir) + got, err := r.Resolve("file://" + keyFile) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "sk-from-file" { + t.Fatalf("got %q, want %q", got, "sk-from-file") + } +} + +func TestResolve_FileKey_NotFound(t *testing.T) { + r := credential.NewResolver(t.TempDir()) + _, err := r.Resolve("file://missing.key") + if err == nil { + t.Fatal("expected error for missing file, got nil") + } +} + +func TestResolve_FileKey_Empty(t *testing.T) { + dir := t.TempDir() + keyFile := "empty.key" + if err := os.WriteFile(filepath.Join(dir, keyFile), []byte(" \n"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + r := credential.NewResolver(dir) + _, err := r.Resolve("file://" + keyFile) + if err == nil { + t.Fatal("expected error for empty credential file, got nil") + } +} + +// TestResolve_EncKey_PassphraseOnly tests encryption/decryption with passphrase alone. +// PICOCLAW_SSH_KEY_PATH is set to "" to disable auto-detection and force passphrase-only mode. +func TestResolve_EncKey_PassphraseOnly(t *testing.T) { + const passphrase = "test-passphrase-32bytes-long-ok!" + const plaintext = "sk-encrypted-secret" + + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") // disable SSH key auto-detection + + enc, err := credential.Encrypt(passphrase, "", plaintext) + if err != nil { + t.Fatalf("Encrypt: %v", err) + } + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", passphrase) + + r := credential.NewResolver(t.TempDir()) + got, err := r.Resolve(enc) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got != plaintext { + t.Fatalf("got %q, want %q", got, plaintext) + } +} + +// TestResolve_EncKey_WithSSHKey tests that the SSH key file is incorporated into key derivation. +func TestResolve_EncKey_WithSSHKey(t *testing.T) { + dir := t.TempDir() + sshKeyPath := filepath.Join(dir, "picoclaw_ed25519.key") + if err := os.WriteFile(sshKeyPath, []byte("fake-ssh-private-key-material\n"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + const passphrase = "test-passphrase" + const plaintext = "sk-ssh-protected-secret" + + // Set PICOCLAW_SSH_KEY_PATH before Encrypt so the path passes allowedSSHKeyPath validation. + t.Setenv("PICOCLAW_KEY_PASSPHRASE", passphrase) + t.Setenv("PICOCLAW_SSH_KEY_PATH", sshKeyPath) + + enc, err := credential.Encrypt(passphrase, sshKeyPath, plaintext) + if err != nil { + t.Fatalf("Encrypt: %v", err) + } + + r := credential.NewResolver(t.TempDir()) + got, err := r.Resolve(enc) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got != plaintext { + t.Fatalf("got %q, want %q", got, plaintext) + } +} + +func TestResolve_EncKey_NoPassphrase(t *testing.T) { + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + enc, err := credential.Encrypt("some-passphrase", "", "sk-secret") + if err != nil { + t.Fatalf("Encrypt: %v", err) + } + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "") + + r := credential.NewResolver(t.TempDir()) + _, err = r.Resolve(enc) + if err == nil { + t.Fatal("expected error when PICOCLAW_KEY_PASSPHRASE is unset, got nil") + } +} + +func TestResolve_EncKey_BadCiphertext(t *testing.T) { + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "some-passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + r := credential.NewResolver(t.TempDir()) + _, err := r.Resolve("enc://!!not-valid-base64!!") + if err == nil { + t.Fatal("expected error for invalid enc:// payload, got nil") + } +} + +func TestResolve_EncKey_PayloadTooShort(t *testing.T) { + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "some-passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + // Valid base64 but fewer bytes than salt(16)+nonce(12)+1 minimum. + import64 := "dG9vc2hvcnQ=" // "tooshort" = 8 bytes + r := credential.NewResolver(t.TempDir()) + _, err := r.Resolve("enc://" + import64) + if err == nil { + t.Fatal("expected error for too-short enc:// payload, got nil") + } +} + +func TestResolve_EncKey_WrongPassphrase(t *testing.T) { + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + + enc, err := credential.Encrypt("correct-passphrase", "", "sk-secret") + if err != nil { + t.Fatalf("Encrypt: %v", err) + } + + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "wrong-passphrase") + + r := credential.NewResolver(t.TempDir()) + _, err = r.Resolve(enc) + if err == nil { + t.Fatal("expected decryption error for wrong passphrase, got nil") + } +} + +func TestEncrypt_EmptyPassphrase(t *testing.T) { + _, err := credential.Encrypt("", "", "sk-secret") + if err == nil { + t.Fatal("expected error for empty passphrase, got nil") + } +} + +func TestDeriveKey_SSHKeyNotFound(t *testing.T) { + // Encrypt with a real SSH key path, then try to decrypt with a missing path. + dir := t.TempDir() + sshKeyPath := filepath.Join(dir, "picoclaw_ed25519.key") + if err := os.WriteFile(sshKeyPath, []byte("fake-key\n"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + // Register the real key path so allowedSSHKeyPath validation passes for Encrypt. + t.Setenv("PICOCLAW_SSH_KEY_PATH", sshKeyPath) + + enc, err := credential.Encrypt("passphrase", sshKeyPath, "sk-secret") + if err != nil { + t.Fatalf("Encrypt: %v", err) + } + + // Point to a non-existent SSH key so deriveKey's ReadFile fails. + // The path is still under the same dir, so allowedSSHKeyPath passes (exact env match). + t.Setenv("PICOCLAW_KEY_PASSPHRASE", "passphrase") + t.Setenv("PICOCLAW_SSH_KEY_PATH", filepath.Join(dir, "nonexistent_key")) + + r := credential.NewResolver(t.TempDir()) + _, err = r.Resolve(enc) + if err == nil { + t.Fatal("expected error when SSH key file is missing, got nil") + } +} + +// TestResolve_FileRef_PathTraversal verifies that file:// references cannot escape configDir +// via relative traversal ("../../etc/passwd") or absolute paths ("/abs/path"). +func TestResolve_FileRef_PathTraversal(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json") + // Create a file outside configDir that the traversal would point to. + outsideFile := filepath.Join(t.TempDir(), "secret.key") + if err := os.WriteFile(outsideFile, []byte("stolen"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + r := credential.NewResolver(filepath.Dir(cfgPath)) + + cases := []string{ + "file://../../secret.key", + "file://../secret.key", + "file://" + outsideFile, // absolute path + } + for _, raw := range cases { + _, err := r.Resolve(raw) + if err == nil { + t.Errorf("Resolve(%q): expected path traversal error, got nil", raw) + } + } +} + +// TestResolve_FileRef_withinConfigDir verifies that a legitimate relative file:// ref works. +func TestResolve_FileRef_withinConfigDir(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "my.key"), []byte("sk-valid\n"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + r := credential.NewResolver(dir) + got, err := r.Resolve("file://my.key") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "sk-valid" { + t.Fatalf("got %q, want %q", got, "sk-valid") + } +} + +// TestEncrypt_SSHKeyOutsideAllowedDirs verifies that Encrypt rejects SSH key paths +// that are not under PICOCLAW_SSH_KEY_PATH, PICOCLAW_HOME, or ~/.ssh/. +func TestEncrypt_SSHKeyOutsideAllowedDirs(t *testing.T) { + dir := t.TempDir() + sshKeyPath := filepath.Join(dir, "picoclaw_ed25519.key") + if err := os.WriteFile(sshKeyPath, []byte("fake-key\n"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + // Make sure none of the allowed env vars point here. + t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + t.Setenv("PICOCLAW_HOME", "") + + _, err := credential.Encrypt("passphrase", sshKeyPath, "sk-secret") + if err == nil { + t.Fatal("expected error for SSH key outside allowed directories, got nil") + } +} diff --git a/pkg/credential/keygen.go b/pkg/credential/keygen.go new file mode 100644 index 0000000000..a97bd3f277 --- /dev/null +++ b/pkg/credential/keygen.go @@ -0,0 +1,62 @@ +package credential + +import ( + "crypto/ed25519" + "crypto/rand" + "encoding/pem" + "fmt" + "os" + "path/filepath" + + "golang.org/x/crypto/ssh" +) + +// DefaultSSHKeyPath returns the canonical path for the picoclaw-specific SSH key. +// The path is always ~/.ssh/picoclaw_ed25519.key (os.UserHomeDir is cross-platform). +func DefaultSSHKeyPath() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("credential: cannot determine home directory: %w", err) + } + return filepath.Join(home, ".ssh", "picoclaw_ed25519.key"), nil +} + +// GenerateSSHKey generates an Ed25519 SSH key pair and writes the private key +// to path (permissions 0600) and the public key to path+".pub" (permissions 0644). +// The ~/.ssh/ directory is created with 0700 if it does not exist. +// If the files already exist they are overwritten. +func GenerateSSHKey(path string) error { + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return fmt.Errorf("credential: keygen: cannot create directory %q: %w", filepath.Dir(path), err) + } + + pubRaw, privRaw, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return fmt.Errorf("credential: keygen: ed25519 key generation failed: %w", err) + } + + // Marshal private key as OpenSSH PEM. + block, err := ssh.MarshalPrivateKey(privRaw, "") + if err != nil { + return fmt.Errorf("credential: keygen: marshal private key: %w", err) + } + privPEM := pem.EncodeToMemory(block) + + if err := os.WriteFile(path, privPEM, 0o600); err != nil { + return fmt.Errorf("credential: keygen: write private key %q: %w", path, err) + } + + // Marshal public key as authorized_keys line. + sshPub, err := ssh.NewPublicKey(pubRaw) + if err != nil { + return fmt.Errorf("credential: keygen: marshal public key: %w", err) + } + pubLine := ssh.MarshalAuthorizedKey(sshPub) + + pubPath := path + ".pub" + if err := os.WriteFile(pubPath, pubLine, 0o644); err != nil { + return fmt.Errorf("credential: keygen: write public key %q: %w", pubPath, err) + } + + return nil +} diff --git a/pkg/credential/keygen_test.go b/pkg/credential/keygen_test.go new file mode 100644 index 0000000000..736b68f35d --- /dev/null +++ b/pkg/credential/keygen_test.go @@ -0,0 +1,109 @@ +package credential + +import ( + "crypto/ed25519" + "os" + "path/filepath" + "runtime" + "testing" + + "golang.org/x/crypto/ssh" +) + +func TestGenerateSSHKey_CreatesFiles(t *testing.T) { + dir := t.TempDir() + keyPath := filepath.Join(dir, "test_ed25519.key") + + if err := GenerateSSHKey(keyPath); err != nil { + t.Fatalf("GenerateSSHKey() error = %v", err) + } + + // Private key must exist. + privInfo, err := os.Stat(keyPath) + if err != nil { + t.Fatalf("private key file missing: %v", err) + } + + // Check permissions on non-Windows (Windows does not support Unix permission bits). + if runtime.GOOS != "windows" { + if got := privInfo.Mode().Perm(); got != 0o600 { + t.Errorf("private key permissions = %04o, want 0600", got) + } + } + + // Public key must exist. + pubPath := keyPath + ".pub" + pubInfo, err := os.Stat(pubPath) + if err != nil { + t.Fatalf("public key file missing: %v", err) + } + if runtime.GOOS != "windows" { + if got := pubInfo.Mode().Perm(); got != 0o644 { + t.Errorf("public key permissions = %04o, want 0644", got) + } + } + + // Private key must be parseable as an OpenSSH ed25519 key. + privPEM, err := os.ReadFile(keyPath) + if err != nil { + t.Fatalf("read private key: %v", err) + } + privKey, err := ssh.ParseRawPrivateKey(privPEM) + if err != nil { + t.Fatalf("parse private key: %v", err) + } + if _, ok := privKey.(*ed25519.PrivateKey); !ok { + t.Errorf("private key type = %T, want *ed25519.PrivateKey", privKey) + } + + // Public key must be parseable as authorized_keys line. + pubBytes, err := os.ReadFile(pubPath) + if err != nil { + t.Fatalf("read public key: %v", err) + } + _, _, _, _, err = ssh.ParseAuthorizedKey(pubBytes) + if err != nil { + t.Fatalf("parse public key: %v", err) + } +} + +func TestGenerateSSHKey_OverwritesExisting(t *testing.T) { + dir := t.TempDir() + keyPath := filepath.Join(dir, "test_ed25519.key") + + // Generate twice; second call must not error and must produce a different key. + if err := GenerateSSHKey(keyPath); err != nil { + t.Fatalf("first GenerateSSHKey() error = %v", err) + } + first, err := os.ReadFile(keyPath) + if err != nil { + t.Fatalf("read first key: %v", err) + } + + if err := GenerateSSHKey(keyPath); err != nil { + t.Fatalf("second GenerateSSHKey() error = %v", err) + } + second, err := os.ReadFile(keyPath) + if err != nil { + t.Fatalf("read second key: %v", err) + } + + // Two independently generated Ed25519 keys must differ. + if string(first) == string(second) { + t.Error("expected overwritten key to differ from original") + } +} + +func TestGenerateSSHKey_CreatesDirectory(t *testing.T) { + dir := t.TempDir() + // Nested directory that does not yet exist. + keyPath := filepath.Join(dir, "subdir", ".ssh", "picoclaw_ed25519.key") + + if err := GenerateSSHKey(keyPath); err != nil { + t.Fatalf("GenerateSSHKey() error = %v", err) + } + + if _, err := os.Stat(keyPath); err != nil { + t.Fatalf("private key not created: %v", err) + } +} diff --git a/pkg/credential/store.go b/pkg/credential/store.go new file mode 100644 index 0000000000..a9a5f988b7 --- /dev/null +++ b/pkg/credential/store.go @@ -0,0 +1,44 @@ +package credential + +import "sync/atomic" + +// SecureStore holds a passphrase in memory. +// +// Uses atomic.Pointer so reads and writes are lock-free. +// The passphrase is never written to disk or placed in os.Environ; +// it is injected only into the gateway child-process environment via cmd.Env. +type SecureStore struct { + val atomic.Pointer[string] +} + +// NewSecureStore creates an empty SecureStore. +func NewSecureStore() *SecureStore { + return &SecureStore{} +} + +// SetString stores the passphrase. An empty string clears the store. +func (s *SecureStore) SetString(passphrase string) { + if passphrase == "" { + s.val.Store(nil) + return + } + s.val.Store(&passphrase) +} + +// Get returns the stored passphrase, or "" if not set. +func (s *SecureStore) Get() string { + if p := s.val.Load(); p != nil { + return *p + } + return "" +} + +// IsSet reports whether a passphrase is currently stored. +func (s *SecureStore) IsSet() bool { + return s.val.Load() != nil +} + +// Clear removes the stored passphrase. +func (s *SecureStore) Clear() { + s.val.Store(nil) +} diff --git a/pkg/credential/store_test.go b/pkg/credential/store_test.go new file mode 100644 index 0000000000..4b0d9988b3 --- /dev/null +++ b/pkg/credential/store_test.go @@ -0,0 +1,65 @@ +package credential + +import ( + "testing" +) + +func TestSecureStore_SetGet(t *testing.T) { + s := NewSecureStore() + if s.IsSet() { + t.Error("expected empty store") + } + + s.SetString("hunter2") + if !s.IsSet() { + t.Error("expected store to be set") + } + if got := s.Get(); got != "hunter2" { + t.Errorf("Get() = %q, want %q", got, "hunter2") + } +} + +func TestSecureStore_Clear(t *testing.T) { + s := NewSecureStore() + s.SetString("secret") + s.Clear() + + if s.IsSet() { + t.Error("expected store to be empty after Clear()") + } + if got := s.Get(); got != "" { + t.Errorf("Get() after Clear() = %q, want empty", got) + } +} + +func TestSecureStore_SetOverwrites(t *testing.T) { + s := NewSecureStore() + s.SetString("first") + s.SetString("second") + + if got := s.Get(); got != "second" { + t.Errorf("Get() = %q, want %q", got, "second") + } +} + +func TestSecureStore_EmptyPassphrase(t *testing.T) { + s := NewSecureStore() + s.SetString("") // empty → should not mark as set + + if s.IsSet() { + t.Error("empty passphrase should not mark store as set") + } +} + +func TestSecureStore_GetReturnsCopy(t *testing.T) { + s := NewSecureStore() + s.SetString("abc") + + // Mutating the returned string bytes (not possible in Go, but we verify + // that a second Get() is not affected by the first). + got1 := s.Get() + got2 := s.Get() + if got1 != got2 { + t.Errorf("successive Get() calls returned different values: %q vs %q", got1, got2) + } +} From adc073ee8d6661190b07bbacb00b4fab602822ce Mon Sep 17 00:00:00 2001 From: sky5454 Date: Sat, 14 Mar 2026 04:54:28 +0800 Subject: [PATCH 2/6] feat(launcher): in-memory passphrase store with web UI unlock Replace env-var passphrase forwarding with a SecureStore-backed flow: - web/backend/main.go: seed passphrase from env var into SecureStore at startup, then clear the env var and redirect credential.PassphraseProvider to apiHandler.GetPassphrase so all LoadConfig calls share one source - web/backend/api/passphrase.go: POST /api/credential/passphrase stores passphrase and auto-starts gateway; GET status endpoint - web/backend/api/gateway.go: build child env via filterEnv() (strips PassphraseEnvVar from parent env) then inject only from SecureStore; passphraseState machine (pending/failed/none) tracks gateway start outcome; expose passphrase_state in /api/gateway/status response - web/backend/api/router.go: add passphraseStore, passphraseMu, passphraseLastState fields; SeedPassphrase / GetPassphrase methods - frontend: PassphraseCard component on credentials page; chat-empty-state shows passphrase input when start_reason contains 'passphrase' or passphraseState is failed; i18n keys for en + zh - Makefile: add -buildvcs=false to GOFLAGS --- Makefile | 2 +- web/backend/api/gateway.go | 72 ++++++++++- web/backend/api/passphrase.go | 80 ++++++++++++ web/backend/api/router.go | 43 ++++++- web/backend/dist/.gitkeep | 1 - web/backend/main.go | 16 +++ web/frontend/src/api/gateway.ts | 1 + web/frontend/src/api/passphrase.ts | 30 +++++ .../src/components/chat/chat-empty-state.tsx | 93 ++++++++++++++ .../src/components/chat/chat-page.tsx | 4 +- .../credentials/credentials-page.tsx | 6 + .../credentials/passphrase-card.tsx | 114 ++++++++++++++++++ web/frontend/src/hooks/use-gateway.ts | 8 +- web/frontend/src/i18n/locales/en.json | 13 ++ web/frontend/src/i18n/locales/zh.json | 13 ++ web/frontend/src/store/gateway.ts | 6 + 16 files changed, 489 insertions(+), 13 deletions(-) create mode 100644 web/backend/api/passphrase.go delete mode 100644 web/backend/dist/.gitkeep create mode 100644 web/frontend/src/api/passphrase.ts create mode 100644 web/frontend/src/components/credentials/passphrase-card.tsx diff --git a/Makefile b/Makefile index 98642703f4..fb2e21a850 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ LDFLAGS=-ldflags "-X $(CONFIG_PKG).Version=$(VERSION) -X $(CONFIG_PKG).GitCommit # Go variables GO?=CGO_ENABLED=0 go -GOFLAGS?=-v -tags stdjson +GOFLAGS?=-v -tags stdjson -buildvcs=false # Patch MIPS LE ELF e_flags (offset 36) for NaN2008-only kernels (e.g. Ingenic X2600). # diff --git a/web/backend/api/gateway.go b/web/backend/api/gateway.go index 41f702e326..01a27d5ec3 100644 --- a/web/backend/api/gateway.go +++ b/web/backend/api/gateway.go @@ -3,6 +3,7 @@ package api import ( "bufio" "encoding/json" + "errors" "fmt" "io" "log" @@ -18,6 +19,7 @@ import ( "time" "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/credential" "github.com/sipeed/picoclaw/web/backend/utils" ) @@ -57,7 +59,15 @@ func (h *Handler) TryAutoStartGateway() { ready, reason, err := h.gatewayStartReady() if err != nil { - log.Printf("Skip auto-starting gateway: %v", err) + if errors.Is(err, credential.ErrPassphraseRequired) { + log.Printf("Skip auto-starting gateway: encrypted credentials require a passphrase. "+ + "Enter it on the Credentials page to unlock.", ) + } else if errors.Is(err, credential.ErrDecryptionFailed) { + log.Printf("Skip auto-starting gateway: failed to decrypt credentials. "+ + "Check the passphrase and SSH key on the Credentials page.") + } else { + log.Printf("Skip auto-starting gateway: %v", err) + } return } if !ready { @@ -74,6 +84,8 @@ func (h *Handler) TryAutoStartGateway() { } // gatewayStartReady validates whether current config can start the gateway. +// LoadConfig uses credential.PassphraseProvider (set to SecureStore.Get at +// startup) so enc:// credentials are resolved correctly without os.Environ. func (h *Handler) gatewayStartReady() (bool, string, error) { cfg, err := config.LoadConfig(h.configPath) if err != nil { @@ -136,7 +148,18 @@ func (h *Handler) startGatewayLocked() (int, error) { execPath := utils.FindPicoclawBinary() cmd := exec.Command(execPath, "gateway") - cmd.Env = os.Environ() + + // Build a clean environment for the child process. + // Start from the launcher's current environment, but explicitly strip + // PICOCLAW_KEY_PASSPHRASE so it cannot leak from the parent env. + // The passphrase is then injected directly from the in-memory SecureStore + // (child-only; never stored in the launcher's own os.Environ). + childEnv := filterEnv(os.Environ(), credential.PassphraseEnvVar) + if passphrase := h.passphraseStore.Get(); passphrase != "" { + childEnv = append(childEnv, credential.PassphraseEnvVar+"="+passphrase) + } + cmd.Env = childEnv + // Forward the launcher's config path via the environment variable that // GetConfigPath() already reads, so the gateway sub-process uses the same // config file without requiring a --config flag on the gateway subcommand. @@ -183,8 +206,9 @@ func (h *Handler) startGatewayLocked() (int, error) { // Wait for exit in background and clean up go func() { - if err := cmd.Wait(); err != nil { - log.Printf("Gateway process exited: %v", err) + exitErr := cmd.Wait() + if exitErr != nil { + log.Printf("Gateway process exited: %v", exitErr) } else { log.Printf("Gateway process exited normally") } @@ -195,6 +219,25 @@ func (h *Handler) startGatewayLocked() (int, error) { } gateway.mu.Unlock() + // If we had an active passphrase attempt and the gateway crashed, + // mark passphrase as failed so the frontend can show an error. + if exitErr != nil { + h.passphraseMu.Lock() + if h.passphraseLastState == passphraseStatePending { + h.passphraseLastState = passphraseStateFailed + // Clear the bad passphrase so user must re-enter + h.passphraseStore.Clear() + } + h.passphraseMu.Unlock() + } else { + // Clean normal exit + h.passphraseMu.Lock() + if h.passphraseLastState == passphraseStatePending { + h.passphraseLastState = passphraseStateNone + } + h.passphraseMu.Unlock() + } + // Broadcast stopped event gateway.events.Broadcast(GatewayEvent{Status: "stopped"}) }() @@ -434,6 +477,13 @@ func (h *Handler) handleGatewayStatus(w http.ResponseWriter, r *http.Request) { } } + // Expose passphrase state so the frontend can distinguish + // "never entered" vs "wrong passphrase" vs "pending start". + h.passphraseMu.Lock() + ps := h.passphraseLastState + h.passphraseMu.Unlock() + data["passphrase_state"] = string(ps) + // Append incremental log data appendGatewayLogs(r, data) @@ -558,3 +608,17 @@ func scanPipe(r io.Reader, buf *LogBuffer) { buf.Append(scanner.Text()) } } + +// filterEnv returns a copy of environ with all entries whose key matches +// the supplied key removed. Used to strip the passphrase from the +// inherited environment before assembling the child-process environ. +func filterEnv(environ []string, key string) []string { + prefix := key + "=" + result := make([]string, 0, len(environ)) + for _, e := range environ { + if !strings.HasPrefix(e, prefix) { + result = append(result, e) + } + } + return result +} diff --git a/web/backend/api/passphrase.go b/web/backend/api/passphrase.go new file mode 100644 index 0000000000..ae7eae6492 --- /dev/null +++ b/web/backend/api/passphrase.go @@ -0,0 +1,80 @@ +package api + +import ( + "encoding/json" + "log" + "net/http" +) + +// registerPassphraseRoutes binds the passphrase management endpoints. +func (h *Handler) registerPassphraseRoutes(mux *http.ServeMux) { + mux.HandleFunc("POST /api/credential/passphrase", h.handleSetPassphrase) + mux.HandleFunc("GET /api/credential/passphrase/status", h.handlePassphraseStatus) +} + +// handleSetPassphrase stores the supplied passphrase in the in-memory +// SecureStore, then attempts to auto-start the gateway if it is not running. +// +// POST /api/credential/passphrase +// Body: {"passphrase": "..."} +func (h *Handler) handleSetPassphrase(w http.ResponseWriter, r *http.Request) { + var body struct { + Passphrase string `json:"passphrase"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + http.Error(w, "invalid JSON body", http.StatusBadRequest) + return + } + if body.Passphrase == "" { + http.Error(w, "passphrase must not be empty", http.StatusBadRequest) + return + } + + h.passphraseStore.SetString(body.Passphrase) + + // Mark state as pending before launching gateway + h.passphraseMu.Lock() + h.passphraseLastState = passphraseStatePending + h.passphraseMu.Unlock() + + // Try to start the gateway now that the passphrase is available. + // credential.PassphraseProvider points to passphraseStore.Get, so + // gatewayStartReady() (and all LoadConfig calls) will resolve enc:// + // credentials correctly using the newly stored passphrase. + go func() { + gateway.mu.Lock() + defer gateway.mu.Unlock() + if isGatewayProcessAliveLocked() { + return + } + pid, err := h.startGatewayLocked() + if err != nil { + log.Printf("Failed to start gateway after passphrase unlock: %v", err) + // startGatewayLocked failed before spawning the process, so the exit + // goroutine will never run. Transition pending → failed manually. + h.passphraseMu.Lock() + if h.passphraseLastState == passphraseStatePending { + h.passphraseLastState = passphraseStateFailed + h.passphraseStore.Clear() + } + h.passphraseMu.Unlock() + return + } + log.Printf("Gateway started after passphrase unlock (PID: %d)", pid) + }() + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "status": "ok", + }) +} + +// handlePassphraseStatus reports whether a passphrase is currently stored. +// +// GET /api/credential/passphrase/status +func (h *Handler) handlePassphraseStatus(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "passphrase_set": h.passphraseStore.IsSet(), + }) +} diff --git a/web/backend/api/router.go b/web/backend/api/router.go index 5f081dee9d..02dc52116e 100644 --- a/web/backend/api/router.go +++ b/web/backend/api/router.go @@ -4,9 +4,22 @@ import ( "net/http" "sync" + "github.com/sipeed/picoclaw/pkg/credential" "github.com/sipeed/picoclaw/web/backend/launcherconfig" ) +// passphraseState tracks what happened with the last passphrase attempt. +// "" → no passphrase submitted yet (or just cleared) +// "pending" → passphrase set, gateway starting +// "failed" → gateway exited; passphrase likely wrong +type passphraseState string + +const ( + passphraseStateNone passphraseState = "" + passphraseStatePending passphraseState = "pending" + passphraseStateFailed passphraseState = "failed" +) + // Handler serves HTTP API requests. type Handler struct { configPath string @@ -17,15 +30,19 @@ type Handler struct { oauthMu sync.Mutex oauthFlows map[string]*oauthFlow oauthState map[string]string + passphraseStore *credential.SecureStore + passphraseMu sync.Mutex + passphraseLastState passphraseState } // NewHandler creates an instance of the API handler. func NewHandler(configPath string) *Handler { return &Handler{ - configPath: configPath, - serverPort: launcherconfig.DefaultPort, - oauthFlows: make(map[string]*oauthFlow), - oauthState: make(map[string]string), + configPath: configPath, + serverPort: launcherconfig.DefaultPort, + oauthFlows: make(map[string]*oauthFlow), + oauthState: make(map[string]string), + passphraseStore: credential.NewSecureStore(), } } @@ -37,6 +54,21 @@ func (h *Handler) SetServerOptions(port int, public bool, publicExplicit bool, a h.serverCIDRs = append([]string(nil), allowedCIDRs...) } +// SeedPassphrase pre-loads the passphrase into the in-memory SecureStore. +// Call this at startup when the passphrase was supplied via an environment +// variable; after seeding, the caller should clear the env var so it is no +// longer visible in the process environment. +func (h *Handler) SeedPassphrase(passphrase string) { + h.passphraseStore.SetString(passphrase) +} + +// GetPassphrase returns the currently stored passphrase, or "" if not set. +// This satisfies the credential.PassphraseProvider signature so all LoadConfig +// calls in the launcher automatically use the in-memory store. +func (h *Handler) GetPassphrase() string { + return h.passphraseStore.Get() +} + // RegisterRoutes binds all API endpoint handlers to the ServeMux. func (h *Handler) RegisterRoutes(mux *http.ServeMux) { // Config CRUD @@ -54,6 +86,9 @@ func (h *Handler) RegisterRoutes(mux *http.ServeMux) { // OAuth login and credential management h.registerOAuthRoutes(mux) + // Passphrase management (in-memory store for encrypted credentials) + h.registerPassphraseRoutes(mux) + // Model list management h.registerModelRoutes(mux) diff --git a/web/backend/dist/.gitkeep b/web/backend/dist/.gitkeep deleted file mode 100644 index 4b533f03aa..0000000000 --- a/web/backend/dist/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -# Keep the embedded web backend dist directory in version control. diff --git a/web/backend/main.go b/web/backend/main.go index 650540ea88..11fc1aefc3 100644 --- a/web/backend/main.go +++ b/web/backend/main.go @@ -22,6 +22,7 @@ import ( "strconv" "time" + "github.com/sipeed/picoclaw/pkg/credential" "github.com/sipeed/picoclaw/web/backend/api" "github.com/sipeed/picoclaw/web/backend/launcherconfig" "github.com/sipeed/picoclaw/web/backend/middleware" @@ -115,6 +116,21 @@ func main() { // API Routes (e.g. /api/status) apiHandler := api.NewHandler(absPath) apiHandler.SetServerOptions(portNum, effectivePublic, explicitPublic, launcherCfg.AllowedCIDRs) + + // If PICOCLAW_KEY_PASSPHRASE is set in the environment at startup, seed it + // into the in-memory SecureStore and then remove it from the process + // environment so it is no longer visible via /proc//environ or similar. + if envPassphrase := os.Getenv(credential.PassphraseEnvVar); envPassphrase != "" { + apiHandler.SeedPassphrase(envPassphrase) + os.Unsetenv(credential.PassphraseEnvVar) + log.Printf("Seeded passphrase from %s environment variable (env var cleared)", credential.PassphraseEnvVar) + } + + // Point the credential package at the in-memory store so that all + // LoadConfig() calls in the launcher (config API, models API, gateway + // readiness check, etc.) use the same passphrase source. + credential.PassphraseProvider = apiHandler.GetPassphrase + apiHandler.RegisterRoutes(mux) // Frontend Embedded Assets diff --git a/web/frontend/src/api/gateway.ts b/web/frontend/src/api/gateway.ts index 020e92e3a1..cb9ed80581 100644 --- a/web/frontend/src/api/gateway.ts +++ b/web/frontend/src/api/gateway.ts @@ -4,6 +4,7 @@ interface GatewayStatusResponse { gateway_status: "running" | "starting" | "stopped" | "error" gateway_start_allowed?: boolean gateway_start_reason?: string + passphrase_state?: "" | "pending" | "failed" pid?: number logs?: string[] log_total?: number diff --git a/web/frontend/src/api/passphrase.ts b/web/frontend/src/api/passphrase.ts new file mode 100644 index 0000000000..47115ff780 --- /dev/null +++ b/web/frontend/src/api/passphrase.ts @@ -0,0 +1,30 @@ +// API client for in-memory passphrase management. + +const BASE_URL = "" + +async function request(path: string, options?: RequestInit): Promise { + const res = await fetch(`${BASE_URL}${path}`, options) + if (!res.ok) { + const text = await res.text().catch(() => res.statusText) + throw new Error(text || `API error: ${res.status}`) + } + return res.json() as Promise +} + +export interface PassphraseStatusResponse { + passphrase_set: boolean +} + +/** Returns whether a passphrase is currently held in the launcher. */ +export async function getPassphraseStatus(): Promise { + return request("/api/credential/passphrase/status") +} + +/** Stores the passphrase in the launcher's in-memory SecureStore. */ +export async function setPassphrase(passphrase: string): Promise<{ status: string }> { + return request<{ status: string }>("/api/credential/passphrase", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ passphrase }), + }) +} diff --git a/web/frontend/src/components/chat/chat-empty-state.tsx b/web/frontend/src/components/chat/chat-empty-state.tsx index 624ff9c590..3387b7cc9a 100644 --- a/web/frontend/src/components/chat/chat-empty-state.tsx +++ b/web/frontend/src/components/chat/chat-empty-state.tsx @@ -1,26 +1,118 @@ import { + IconKey, + IconLoader2, + IconLock, IconPlugConnectedX, IconRobot, IconRobotOff, IconStar, } from "@tabler/icons-react" import { Link } from "@tanstack/react-router" +import { useEffect, useState } from "react" import { useTranslation } from "react-i18next" +import { setPassphrase } from "@/api/passphrase" import { Button } from "@/components/ui/button" +import { Input } from "@/components/ui/input" interface ChatEmptyStateProps { hasConfiguredModels: boolean defaultModelName: string isConnected: boolean + gatewayStartReason?: string + passphraseState?: "" | "pending" | "failed" } export function ChatEmptyState({ hasConfiguredModels, defaultModelName, isConnected, + gatewayStartReason = "", + passphraseState = "", }: ChatEmptyStateProps) { const { t } = useTranslation() + const needsPassphrase = gatewayStartReason.toLowerCase().includes("passphrase") + || passphraseState === "failed" + || passphraseState === "pending" + + const [passphrase, setPassphraseValue] = useState("") + const [saving, setSaving] = useState(false) + const [error, setError] = useState("") + const [saved, setSaved] = useState(false) + + // When backend signals failure, reset the saved flag and show error. + useEffect(() => { + if (passphraseState === "failed") { + setSaved(false) + setError(t("credentials.passphrase.errorWrongPassphrase")) + } + }, [passphraseState, t]) + + async function handleUnlock() { + if (!passphrase.trim()) return + setSaving(true) + setError("") + try { + await setPassphrase(passphrase.trim()) + setSaved(true) + setPassphraseValue("") + } catch { + setError(t("credentials.passphrase.errorSave")) + } finally { + setSaving(false) + } + } + + // Passphrase unlock takes priority — models/config can't load without it + if (!isConnected && needsPassphrase) { + return ( +
+
+ +
+

+ {t("credentials.passphrase.title")} +

+

+ {t("credentials.passphrase.description")} +

+ {saved ? ( +

+ {t("credentials.passphrase.successMessage")} +

+ ) : ( +
+
+ setPassphraseValue(e.target.value)} + onKeyDown={(e) => { if (e.key === "Enter") void handleUnlock() }} + disabled={saving} + autoFocus + /> + +
+ {error && ( +

{error}

+ )} +
+ )} +
+ ) + } if (!hasConfiguredModels) { return ( @@ -85,3 +177,4 @@ export function ChatEmptyState({ ) } + diff --git a/web/frontend/src/components/chat/chat-page.tsx b/web/frontend/src/components/chat/chat-page.tsx index a3ab843b4d..dab4aabaab 100644 --- a/web/frontend/src/components/chat/chat-page.tsx +++ b/web/frontend/src/components/chat/chat-page.tsx @@ -31,7 +31,7 @@ export function ChatPage() { newChat, } = usePicoChat() - const { state: gwState } = useGateway() + const { state: gwState, startReason, passphraseState } = useGateway() const isConnected = gwState === "running" const { @@ -127,6 +127,8 @@ export function ChatPage() { hasConfiguredModels={hasConfiguredModels} defaultModelName={defaultModelName} isConnected={isConnected} + gatewayStartReason={startReason} + passphraseState={passphraseState} /> )} diff --git a/web/frontend/src/components/credentials/credentials-page.tsx b/web/frontend/src/components/credentials/credentials-page.tsx index 04aceb002e..e37d3f9d95 100644 --- a/web/frontend/src/components/credentials/credentials-page.tsx +++ b/web/frontend/src/components/credentials/credentials-page.tsx @@ -9,6 +9,7 @@ import { AntigravityCredentialCard } from "./antigravity-credential-card" import { DeviceCodeSheet } from "./device-code-sheet" import { LogoutConfirmDialog } from "./logout-confirm-dialog" import { OpenAICredentialCard } from "./openai-credential-card" +import { PassphraseCard } from "./passphrase-card" export function CredentialsPage() { const { t } = useTranslation() @@ -51,6 +52,11 @@ export function CredentialsPage() {

+ {/* Passphrase card is always visible — independent of OAuth loading state */} +
+ +
+ {error && (
{error} diff --git a/web/frontend/src/components/credentials/passphrase-card.tsx b/web/frontend/src/components/credentials/passphrase-card.tsx new file mode 100644 index 0000000000..2657fd56a7 --- /dev/null +++ b/web/frontend/src/components/credentials/passphrase-card.tsx @@ -0,0 +1,114 @@ +import { IconKey, IconLock, IconLockOpen, IconLoader2 } from "@tabler/icons-react" +import { useEffect, useState } from "react" +import { useTranslation } from "react-i18next" + +import { getPassphraseStatus, setPassphrase } from "@/api/passphrase" +import { Button } from "@/components/ui/button" +import { Input } from "@/components/ui/input" + +export function PassphraseCard() { + const { t } = useTranslation() + const [value, setValue] = useState("") + const [isSet, setIsSet] = useState(null) + const [saving, setSaving] = useState(false) + const [message, setMessage] = useState<{ text: string; error: boolean } | null>(null) + + useEffect(() => { + getPassphraseStatus() + .then((res) => setIsSet(res.passphrase_set)) + .catch(() => setIsSet(false)) + }, []) + + async function handleSave() { + if (!value.trim()) { + setMessage({ text: t("credentials.passphrase.errorEmpty"), error: true }) + return + } + setSaving(true) + setMessage(null) + try { + await setPassphrase(value.trim()) + setIsSet(true) + setValue("") + setMessage({ text: t("credentials.passphrase.successMessage"), error: false }) + } catch { + setMessage({ text: t("credentials.passphrase.errorSave"), error: true }) + } finally { + setSaving(false) + } + } + + return ( +
+
+

+ + + + {t("credentials.passphrase.title")} +

+

+ {t("credentials.passphrase.description")} +

+
+ +
+ {isSet === null ? ( + + ) : isSet ? ( + <> + + + {t("credentials.passphrase.statusSet")} + + + ) : ( + <> + + + {t("credentials.passphrase.statusNotSet")} + + + )} +
+ +
+
+
+
+ setValue(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") void handleSave() + }} + type="password" + placeholder={t("credentials.passphrase.placeholder")} + disabled={saving} + /> + +
+ {message && ( +

+ {message.text} +

+ )} +
+
+
+
+
+ ) +} diff --git a/web/frontend/src/hooks/use-gateway.ts b/web/frontend/src/hooks/use-gateway.ts index 097dc3598b..44d6e44838 100644 --- a/web/frontend/src/hooks/use-gateway.ts +++ b/web/frontend/src/hooks/use-gateway.ts @@ -13,7 +13,7 @@ import { gatewayAtom } from "@/store" let sseInitialized = false export function useGateway() { - const [{ status: state, canStart }, setGateway] = useAtom(gatewayAtom) + const [{ status: state, canStart, startReason, passphraseState }, setGateway] = useAtom(gatewayAtom) const [loading, setLoading] = useState(false) const applyGatewayStatus = useCallback( @@ -22,6 +22,8 @@ export function useGateway() { ...prev, status: data.gateway_status ?? "unknown", canStart: data.gateway_start_allowed ?? true, + startReason: data.gateway_start_reason ?? "", + passphraseState: data.passphrase_state ?? "", })) }, [setGateway], @@ -38,6 +40,8 @@ export function useGateway() { setGateway({ status: "unknown", canStart: true, + startReason: "", + passphraseState: "", }) }) @@ -117,5 +121,5 @@ export function useGateway() { } }, []) - return { state, loading, canStart, start, stop } + return { state, loading, canStart, startReason, passphraseState, start, stop } } diff --git a/web/frontend/src/i18n/locales/en.json b/web/frontend/src/i18n/locales/en.json index 453c5905f4..810d9976ec 100644 --- a/web/frontend/src/i18n/locales/en.json +++ b/web/frontend/src/i18n/locales/en.json @@ -78,6 +78,19 @@ "credentials": { "description": "Manage OAuth and token-based credentials for supported providers.", "loading": "Loading credentials...", + "passphrase": { + "title": "Encryption Passphrase", + "description": "Required to decrypt enc:// API keys in your config. Stored only in memory — never written to disk.", + "statusSet": "Passphrase loaded", + "statusNotSet": "No passphrase set", + "placeholder": "Enter passphrase", + "save": "Unlock", + "saving": "Unlocking...", + "successMessage": "Passphrase stored. Gateway is starting...", + "errorEmpty": "Passphrase must not be empty.", + "errorSave": "Failed to store passphrase.", + "errorWrongPassphrase": "Wrong passphrase — gateway failed to start. Please try again." + }, "providers": { "openai": { "description": "Supports browser OAuth, device code, and token login." diff --git a/web/frontend/src/i18n/locales/zh.json b/web/frontend/src/i18n/locales/zh.json index b6bdedbfa4..75441b2178 100644 --- a/web/frontend/src/i18n/locales/zh.json +++ b/web/frontend/src/i18n/locales/zh.json @@ -78,6 +78,19 @@ "credentials": { "description": "管理已支持服务商的 OAuth 与 Token 凭据。", "loading": "正在加载凭据...", + "passphrase": { + "title": "加密密码", + "description": "用于解密配置中的 enc:// API Key，仅存储在内存中，不会写入磁盘。", + "statusSet": "密码已加载", + "statusNotSet": "未设置密码", + "placeholder": "输入密码", + "save": "解锁", + "saving": "解锁中...", + "successMessage": "密码已存储，网关正在启动...", + "errorEmpty": "密码不能为空。", + "errorSave": "存储密码失败。", + "errorWrongPassphrase": "密码错误，网关启动失败，请重试。" + }, "providers": { "openai": { "description": "支持浏览器 OAuth、设备码和 Token 登录。" diff --git a/web/frontend/src/store/gateway.ts b/web/frontend/src/store/gateway.ts index 89da9d7fd3..11585a7a84 100644 --- a/web/frontend/src/store/gateway.ts +++ b/web/frontend/src/store/gateway.ts @@ -12,12 +12,16 @@ export type GatewayState = export interface GatewayStoreState { status: GatewayState canStart: boolean + startReason: string + passphraseState: "" | "pending" | "failed" } // Global atom for gateway state export const gatewayAtom = atom({ status: "unknown", canStart: true, + startReason: "", + passphraseState: "", }) function applyGatewayStatusToStore(data: GatewayStatusResponse) { @@ -25,6 +29,8 @@ function applyGatewayStatusToStore(data: GatewayStatusResponse) { ...prev, status: data.gateway_status ?? "unknown", canStart: data.gateway_start_allowed ?? true, + startReason: data.gateway_start_reason ?? "", + passphraseState: data.passphrase_state ?? "", })) } From 23c0ca1c7738155f59bc2b64cced6e8381f253a8 Mon Sep 17 00:00:00 2001 From: sky5454 Date: Sat, 14 Mar 2026 05:37:11 +0800 Subject: [PATCH 3/6] fix(credential): address Copilot review comments on PR #1521 - credential.go: decouple ErrPassphraseRequired from env var name; message is now 'enc:// passphrase required' since PassphraseProvider may come from any source, not just os.Environ - credential.go: Resolver resolves symlinks via EvalSymlinks before the isWithinDir containment check, preventing symlink-based path traversal for file:// credential references - store.go: tighten comment to describe only what SecureStore guarantees (in-memory only); remove claims about how callers transport the value - store_test.go: replace the meaningless GetReturnsCopy test (Go strings are immutable, equality across two calls proves nothing) with TestSecureStore_ConcurrentSetGet that exercises atomic.Pointer under 10-goroutine concurrent Set/Get load - config_test.go: update error-message assertion to match new sentinel text - docs/credential_encryption.md: remove reference to non-existent 'picoclaw encrypt' subcommand; describe the onboard flow instead --- docs/credential_encryption.md | 11 +++++++---- pkg/config/config_test.go | 4 ++-- pkg/credential/credential.go | 37 ++++++++++++++++++++++++----------- pkg/credential/store.go | 4 ++-- pkg/credential/store_test.go | 32 ++++++++++++++++++++++-------- 5 files changed, 61 insertions(+), 27 deletions(-) diff --git a/docs/credential_encryption.md b/docs/credential_encryption.md index 4a8f12457f..448eaaa102 100644 --- a/docs/credential_encryption.md +++ b/docs/credential_encryption.md @@ -13,11 +13,14 @@ Encrypted keys are stored as `enc://` strings and decrypted automaticall export PICOCLAW_KEY_PASSPHRASE="your-passphrase" ``` -**2. Encrypt an API key** (using the built-in CLI subcommand, or Go API) +**2. Encrypt an API key** -```bash -picoclaw encrypt sk-your-openai-key -# outputs: enc://AAAA...base64... +Run `picoclaw onboard` — it prompts for your passphrase and generates the SSH key, +then automatically re-encrypts any plaintext `api_key` entries in your config on +the next `SaveConfig` call. The resulting `enc://` value will look like: + +``` +enc://AAAA...base64... ``` **3. Paste the output into your config** diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 20dfed8919..0ce77ab087 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -886,8 +886,8 @@ func TestLoadConfig_MixedKeys_NoPassphrase(t *testing.T) { if err == nil { t.Fatal("LoadConfig should fail when enc:// key is present and no passphrase is set") } - if !strings.Contains(err.Error(), "PICOCLAW_KEY_PASSPHRASE") { - t.Errorf("error should mention PICOCLAW_KEY_PASSPHRASE, got: %v", err) + if !strings.Contains(err.Error(), "passphrase required") { + t.Errorf("error should mention passphrase required, got: %v", err) } } diff --git a/pkg/credential/credential.go b/pkg/credential/credential.go index 8372a59adf..e584d24159 100644 --- a/pkg/credential/credential.go +++ b/pkg/credential/credential.go @@ -58,9 +58,9 @@ var PassphraseProvider func() string = func() string { } // ErrPassphraseRequired is returned when an enc:// credential is encountered but -// PICOCLAW_KEY_PASSPHRASE is not set. Callers can detect this with errors.Is to -// distinguish a missing-passphrase condition from other credential errors. -var ErrPassphraseRequired = errors.New("credential: enc:// key requires " + PassphraseEnvVar + " env var") +// no passphrase is available from PassphraseProvider. Callers can detect this +// with errors.Is to distinguish a missing-passphrase condition from other errors. +var ErrPassphraseRequired = errors.New("credential: enc:// passphrase required") // ErrDecryptionFailed is returned when an enc:// credential cannot be decrypted, // indicating a wrong passphrase or SSH key. Callers can detect this with errors.Is. @@ -79,13 +79,20 @@ const ( // Resolver resolves raw credential strings for model_list api_key fields. // File references are resolved relative to the directory of the config file. type Resolver struct { - configDir string + configDir string + resolvedConfigDir string // symlink-resolved form of configDir } // NewResolver returns a Resolver that resolves file:// references relative to // configDir (typically filepath.Dir of the config file path). func NewResolver(configDir string) *Resolver { - return &Resolver{configDir: configDir} + resolved := configDir + if configDir != "" { + if real, err := filepath.EvalSymlinks(configDir); err == nil { + resolved = real + } + } + return &Resolver{configDir: configDir, resolvedConfigDir: resolved} } // Resolve returns the actual credential value for raw: @@ -104,19 +111,27 @@ func (r *Resolver) Resolve(raw string) (string, error) { return "", fmt.Errorf("credential: file:// reference has no filename") } - keyPath := filepath.Join(r.configDir, fileName) - // Prevent path traversal: "../../etc/passwd" or "/abs/path" must not escape configDir. - if !isWithinDir(keyPath, r.configDir) { + baseDir := r.resolvedConfigDir + if baseDir == "" { + baseDir = r.configDir + } + keyPath := filepath.Join(baseDir, fileName) + // Resolve symlinks before enforcing containment to prevent escaping via symlinks. + realKeyPath, err := filepath.EvalSymlinks(keyPath) + if err != nil { + return "", fmt.Errorf("credential: failed to resolve credential file path %q: %w", keyPath, err) + } + if !isWithinDir(realKeyPath, baseDir) { return "", fmt.Errorf("credential: file:// path escapes config directory") } - data, err := os.ReadFile(keyPath) + data, err := os.ReadFile(realKeyPath) if err != nil { - return "", fmt.Errorf("credential: failed to read credential file %q: %w", keyPath, err) + return "", fmt.Errorf("credential: failed to read credential file %q: %w", realKeyPath, err) } value := strings.TrimSpace(string(data)) if value == "" { - return "", fmt.Errorf("credential: credential file %q is empty", keyPath) + return "", fmt.Errorf("credential: credential file %q is empty", realKeyPath) } return value, nil diff --git a/pkg/credential/store.go b/pkg/credential/store.go index a9a5f988b7..9c72974b0c 100644 --- a/pkg/credential/store.go +++ b/pkg/credential/store.go @@ -5,8 +5,8 @@ import "sync/atomic" // SecureStore holds a passphrase in memory. // // Uses atomic.Pointer so reads and writes are lock-free. -// The passphrase is never written to disk or placed in os.Environ; -// it is injected only into the gateway child-process environment via cmd.Env. +// The passphrase is never written to disk; callers decide how to +// transport it outside this store (e.g., via cmd.Env or os.Environ). type SecureStore struct { val atomic.Pointer[string] } diff --git a/pkg/credential/store_test.go b/pkg/credential/store_test.go index 4b0d9988b3..63299743a6 100644 --- a/pkg/credential/store_test.go +++ b/pkg/credential/store_test.go @@ -1,6 +1,7 @@ package credential import ( + "sync" "testing" ) @@ -51,15 +52,30 @@ func TestSecureStore_EmptyPassphrase(t *testing.T) { } } -func TestSecureStore_GetReturnsCopy(t *testing.T) { +func TestSecureStore_ConcurrentSetGet(t *testing.T) { s := NewSecureStore() - s.SetString("abc") + const goroutines = 10 + const iterations = 1000 - // Mutating the returned string bytes (not possible in Go, but we verify - // that a second Get() is not affected by the first). - got1 := s.Get() - got2 := s.Get() - if got1 != got2 { - t.Errorf("successive Get() calls returned different values: %q vs %q", got1, got2) + var wg sync.WaitGroup + wg.Add(goroutines) + for i := 0; i < goroutines; i++ { + go func(id int) { + defer wg.Done() + for j := 0; j < iterations; j++ { + if id%2 == 0 { + s.SetString("even") + } else { + s.SetString("odd") + } + _ = s.Get() + } + }(i) + } + wg.Wait() + + final := s.Get() + if final != "" && final != "even" && final != "odd" { + t.Errorf("Get() returned unexpected value %q after concurrent Set/Get", final) } } From 6eedc4266fbace7543716b245ead14febaa984a5 Mon Sep 17 00:00:00 2001 From: duomi Date: Sat, 14 Mar 2026 05:49:23 +0800 Subject: [PATCH 4/6] docs(skills): add python script skill example --- README.md | 2 ++ pkg/skills/repo_skill_examples_test.go | 24 +++++++++++++ workspace/skills/python-script/SKILL.md | 48 +++++++++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 pkg/skills/repo_skill_examples_test.go create mode 100644 workspace/skills/python-script/SKILL.md diff --git a/README.md b/README.md index 58cdfe323a..c085c9730c 100644 --- a/README.md +++ b/README.md @@ -807,6 +807,8 @@ For advanced/test setups, you can override the builtin skills root with: export PICOCLAW_BUILTIN_SKILLS=/path/to/skills ``` +For a concrete example of a skill that executes a local script, see [workspace/skills/python-script/SKILL.md](workspace/skills/python-script/SKILL.md). It shows the recommended pattern for calling an existing `python3` script through the `exec` tool and returning structured output. + ### Unified Command Execution Policy - Generic slash commands are executed through a single path in `pkg/agent/loop.go` via `commands.Executor`. diff --git a/pkg/skills/repo_skill_examples_test.go b/pkg/skills/repo_skill_examples_test.go new file mode 100644 index 0000000000..c2e487c331 --- /dev/null +++ b/pkg/skills/repo_skill_examples_test.go @@ -0,0 +1,24 @@ +package skills + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRepoPythonScriptSkillMetadataIsValid(t *testing.T) { + sl := &SkillsLoader{} + skillPath := filepath.Join("..", "..", "workspace", "skills", "python-script", "SKILL.md") + + meta := sl.getSkillMetadata(skillPath) + require.NotNil(t, meta) + require.Equal(t, "python-script", meta.Name) + require.NotEmpty(t, meta.Description) + + info := SkillInfo{ + Name: meta.Name, + Description: meta.Description, + } + require.NoError(t, info.validate()) +} diff --git a/workspace/skills/python-script/SKILL.md b/workspace/skills/python-script/SKILL.md new file mode 100644 index 0000000000..0823101e7c --- /dev/null +++ b/workspace/skills/python-script/SKILL.md @@ -0,0 +1,48 @@ +--- +name: python-script +description: Run an existing local Python script through the exec tool and return the result clearly. +metadata: {"nanobot":{"emoji":"🐍","requires":{"bins":["python3"]}}} +--- + +# Python Script + +Use this skill when the user wants PicoClaw to run an existing Python script in the workspace or another explicitly allowed path. + +## Rules + +- Prefer `python3`, not `python`. +- Treat the script path and arguments as required inputs. If they are missing, ask for them. +- If the script is unfamiliar, inspect it with `read_file` before executing it. +- Prefer machine-readable output. If the script supports `--json`, use it. +- Do not invent results. Report stdout, stderr, exit code, and any missing dependency errors exactly. +- If the script writes files or updates state, tell the user what changed and where. + +## Command Patterns + +Run a script: +```bash +python3 path/to/script.py +``` + +Run with arguments: +```bash +python3 path/to/script.py --input data.json --limit 10 +``` + +Request JSON output when supported: +```bash +python3 path/to/script.py --json +``` + +Pass temporary environment variables inline: +```bash +FOO=bar python3 path/to/script.py --json +``` + +## Response Pattern + +1. Restate the script path and arguments you will run. +2. Use the `exec` tool to execute the command. +3. Summarize stdout briefly. +4. If stderr is non-empty or the exit code is non-zero, surface that clearly. +5. If stdout is JSON, extract the important fields instead of dumping raw output unless the user asks for the full payload. From 6bf92a59f816a9f6f1f0166972c01ede710df820 Mon Sep 17 00:00:00 2001 From: duomi Date: Sat, 14 Mar 2026 06:47:27 +0800 Subject: [PATCH 5/6] fix(skills): show scanned roots when no skills found --- README.md | 6 ++ cmd/picoclaw/internal/skills/helpers.go | 14 ++++ cmd/picoclaw/internal/skills/helpers_test.go | 73 ++++++++++++++++++++ docker/docker-compose.yml | 9 +++ 4 files changed, 102 insertions(+) create mode 100644 cmd/picoclaw/internal/skills/helpers_test.go diff --git a/README.md b/README.md index 58cdfe323a..2eaa3f77f9 100644 --- a/README.md +++ b/README.md @@ -179,10 +179,16 @@ docker compose -f docker/docker-compose.yml --profile gateway up # 3. Set your API keys vim docker/data/config.json # Set provider API keys, bot tokens, etc. +# Optional: add custom skills for this compose setup +mkdir -p docker/data/workspace/skills/my-skill +# put your SKILL.md at docker/data/workspace/skills/my-skill/SKILL.md + # 4. Start docker compose -f docker/docker-compose.yml --profile gateway up -d ``` +In this compose setup, PicoClaw reads the workspace from `docker/data/workspace` on the host, so local skills must live under `docker/data/workspace/skills`. The repo checkout's `workspace/skills` directory is not mounted into the container. + > [!TIP] > **Docker Users**: By default, the Gateway listens on `127.0.0.1` which is not accessible from the host. If you need to access the health endpoints or expose ports, set `PICOCLAW_GATEWAY_HOST=0.0.0.0` in your environment or update `config.json`. diff --git a/cmd/picoclaw/internal/skills/helpers.go b/cmd/picoclaw/internal/skills/helpers.go index a59a2013a2..012606bdb6 100644 --- a/cmd/picoclaw/internal/skills/helpers.go +++ b/cmd/picoclaw/internal/skills/helpers.go @@ -22,6 +22,7 @@ func skillsListCmd(loader *skills.SkillsLoader) { if len(allSkills) == 0 { fmt.Println("No skills installed.") + printSkillSearchRoots(loader) return } @@ -35,6 +36,19 @@ func skillsListCmd(loader *skills.SkillsLoader) { } } +func printSkillSearchRoots(loader *skills.SkillsLoader) { + roots := loader.SkillRoots() + if len(roots) == 0 { + return + } + + fmt.Println("Scanned skill roots:") + for i, root := range roots { + fmt.Printf(" %d. %s\n", i+1, root) + } + fmt.Printf("Install local skills under %s//SKILL.md\n", roots[0]) +} + func skillsInstallCmd(installer *skills.SkillInstaller, repo string) error { fmt.Printf("Installing skill from %s...\n", repo) diff --git a/cmd/picoclaw/internal/skills/helpers_test.go b/cmd/picoclaw/internal/skills/helpers_test.go new file mode 100644 index 0000000000..5147623ddf --- /dev/null +++ b/cmd/picoclaw/internal/skills/helpers_test.go @@ -0,0 +1,73 @@ +package skills + +import ( + "bytes" + "io" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + skillspkg "github.com/sipeed/picoclaw/pkg/skills" +) + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + oldStdout := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + + fn() + + require.NoError(t, w.Close()) + os.Stdout = oldStdout + + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + require.NoError(t, err) + require.NoError(t, r.Close()) + return buf.String() +} + +func TestSkillsListCmd_EmptyOutputIncludesSearchRoots(t *testing.T) { + tmp := t.TempDir() + workspace := filepath.Join(tmp, "workspace") + global := filepath.Join(tmp, "global") + builtin := filepath.Join(tmp, "builtin") + loader := skillspkg.NewSkillsLoader(workspace, global, builtin) + + output := captureStdout(t, func() { + skillsListCmd(loader) + }) + + assert.Contains(t, output, "No skills installed.") + assert.Contains(t, output, "Scanned skill roots:") + assert.Contains(t, output, filepath.Join(workspace, "skills")) + assert.Contains(t, output, global) + assert.Contains(t, output, builtin) + assert.Contains(t, output, filepath.Join(workspace, "skills", "", "SKILL.md")) +} + +func TestSkillsListCmd_ShowsInstalledSkills(t *testing.T) { + tmp := t.TempDir() + workspace := filepath.Join(tmp, "workspace") + skillDir := filepath.Join(workspace, "skills", "weather") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + + content := "---\nname: weather\ndescription: Weather lookup\n---\n\n# Weather\n" + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) + + loader := skillspkg.NewSkillsLoader(workspace, filepath.Join(tmp, "global"), filepath.Join(tmp, "builtin")) + output := captureStdout(t, func() { + skillsListCmd(loader) + }) + + assert.Contains(t, output, "Installed Skills:") + assert.Contains(t, output, "weather (workspace)") + assert.Contains(t, output, "Weather lookup") + assert.NotContains(t, output, "Scanned skill roots:") +} diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index b26cf4199b..b4a48b9c11 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -12,6 +12,9 @@ services: #extra_hosts: # - "host.docker.internal:host-gateway" volumes: + # Persistent PicoClaw home. Custom local skills should be placed under + # ./data/workspace/skills on the host so they appear at + # /root/.picoclaw/workspace/skills inside the container. - ./data:/root/.picoclaw entrypoint: ["picoclaw", "agent"] stdin_open: true @@ -31,6 +34,9 @@ services: #extra_hosts: # - "host.docker.internal:host-gateway" volumes: + # Persistent PicoClaw home. Custom local skills should be placed under + # ./data/workspace/skills on the host so they appear at + # /root/.picoclaw/workspace/skills inside the container. - ./data:/root/.picoclaw # ───────────────────────────────────────────── @@ -49,4 +55,7 @@ services: - "127.0.0.1:18800:18800" - "127.0.0.1:18790:18790" volumes: + # Persistent PicoClaw home. Custom local skills should be placed under + # ./data/workspace/skills on the host so they appear at + # /root/.picoclaw/workspace/skills inside the container. - ./data:/root/.picoclaw From e60b456f18a9d241718fb58e1204baf8738899d7 Mon Sep 17 00:00:00 2001 From: SHINE-six Date: Sat, 14 Mar 2026 16:45:15 +0800 Subject: [PATCH 6/6] feat(tools): add SpawnStatusTool for reporting subagent statuses --- pkg/agent/loop.go | 1 + pkg/tools/spawn_status.go | 127 ++++++++++++++++++ pkg/tools/spawn_status_test.go | 237 +++++++++++++++++++++++++++++++++ 3 files changed, 365 insertions(+) create mode 100644 pkg/tools/spawn_status.go create mode 100644 pkg/tools/spawn_status_test.go diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index f20a56b9c4..5bc7a328e3 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -233,6 +233,7 @@ func registerSharedTools( return registry.CanSpawnSubagent(currentAgentID, targetAgentID) }) agent.Tools.Register(spawnTool) + agent.Tools.Register(tools.NewSpawnStatusTool(subagentManager)) } else { logger.WarnCF("agent", "spawn tool requires subagent to be enabled", nil) } diff --git a/pkg/tools/spawn_status.go b/pkg/tools/spawn_status.go new file mode 100644 index 0000000000..7a1872eda5 --- /dev/null +++ b/pkg/tools/spawn_status.go @@ -0,0 +1,127 @@ +package tools + +import ( + "context" + "fmt" + "sort" + "strings" + "time" +) + +// SpawnStatusTool reports the status of subagents that were spawned via the +// spawn tool. It can query a specific task by ID, or list every known task with +// a summary count broken-down by status. +type SpawnStatusTool struct { + manager *SubagentManager +} + +// NewSpawnStatusTool creates a SpawnStatusTool backed by the given manager. +func NewSpawnStatusTool(manager *SubagentManager) *SpawnStatusTool { + return &SpawnStatusTool{manager: manager} +} + +func (t *SpawnStatusTool) Name() string { + return "spawn_status" +} + +func (t *SpawnStatusTool) Description() string { + return "Get the status of spawned subagents. " + + "Returns a list of all subagents and their current state " + + "(running, completed, failed, or canceled), or retrieves details " + + "for a specific subagent task when task_id is provided." +} + +func (t *SpawnStatusTool) Parameters() map[string]any { + return map[string]any{ + "type": "object", + "properties": map[string]any{ + "task_id": map[string]any{ + "type": "string", + "description": "Optional task ID (e.g. \"subagent-1\") to inspect a specific " + + "subagent. When omitted, all known subagents are listed.", + }, + }, + "required": []string{}, + } +} + +func (t *SpawnStatusTool) Execute(ctx context.Context, args map[string]any) *ToolResult { + if t.manager == nil { + return ErrorResult("Subagent manager not configured") + } + + taskID, _ := args["task_id"].(string) + taskID = strings.TrimSpace(taskID) + + if taskID != "" { + task, ok := t.manager.GetTask(taskID) + if !ok { + return ErrorResult(fmt.Sprintf("No subagent found with task ID: %s", taskID)) + } + return NewToolResult(spawnStatusFormatTask(task)) + } + + tasks := t.manager.ListTasks() + if len(tasks) == 0 { + return NewToolResult("No subagents have been spawned yet.") + } + + // Deterministic ordering: sort by ID string (e.g. "subagent-1" < "subagent-2"). + sort.Slice(tasks, func(i, j int) bool { + return tasks[i].ID < tasks[j].ID + }) + + counts := map[string]int{} + for _, task := range tasks { + counts[task.Status]++ + } + + var sb strings.Builder + sb.WriteString(fmt.Sprintf("Subagent status report (%d total):\n", len(tasks))) + for _, status := range []string{"running", "completed", "failed", "canceled"} { + if n := counts[status]; n > 0 { + label := strings.ToUpper(status[:1]) + status[1:] + ":" + sb.WriteString(fmt.Sprintf(" %-10s %d\n", label, n)) + } + } + sb.WriteString("\n") + + for _, task := range tasks { + sb.WriteString(spawnStatusFormatTask(task)) + sb.WriteString("\n\n") + } + + return NewToolResult(strings.TrimRight(sb.String(), "\n")) +} + +// spawnStatusFormatTask renders a single SubagentTask as a human-readable block. +func spawnStatusFormatTask(task *SubagentTask) string { + var sb strings.Builder + + header := fmt.Sprintf("[%s] status=%s", task.ID, task.Status) + if task.Label != "" { + header += fmt.Sprintf(" label=%q", task.Label) + } + if task.AgentID != "" { + header += fmt.Sprintf(" agent=%s", task.AgentID) + } + if task.Created > 0 { + created := time.UnixMilli(task.Created).UTC().Format("2006-01-02 15:04:05 UTC") + header += fmt.Sprintf(" created=%s", created) + } + sb.WriteString(header) + + if task.Task != "" { + sb.WriteString(fmt.Sprintf("\n task: %s", task.Task)) + } + if task.Result != "" { + result := task.Result + const maxResultLen = 300 + if len(result) > maxResultLen { + result = result[:maxResultLen] + "…" + } + sb.WriteString(fmt.Sprintf("\n result: %s", result)) + } + + return sb.String() +} diff --git a/pkg/tools/spawn_status_test.go b/pkg/tools/spawn_status_test.go new file mode 100644 index 0000000000..491fd39c85 --- /dev/null +++ b/pkg/tools/spawn_status_test.go @@ -0,0 +1,237 @@ +package tools + +import ( + "context" + "fmt" + "strings" + "testing" + "time" +) + +func TestSpawnStatusTool_Name(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + tool := NewSpawnStatusTool(manager) + + if tool.Name() != "spawn_status" { + t.Errorf("Expected name 'spawn_status', got '%s'", tool.Name()) + } +} + +func TestSpawnStatusTool_Description(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + tool := NewSpawnStatusTool(manager) + + desc := tool.Description() + if desc == "" { + t.Error("Description should not be empty") + } + if !strings.Contains(strings.ToLower(desc), "subagent") { + t.Errorf("Description should mention 'subagent', got: %s", desc) + } +} + +func TestSpawnStatusTool_Parameters(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + tool := NewSpawnStatusTool(manager) + + params := tool.Parameters() + if params["type"] != "object" { + t.Errorf("Expected type 'object', got: %v", params["type"]) + } + props, ok := params["properties"].(map[string]any) + if !ok { + t.Fatal("Expected 'properties' to be a map") + } + if _, hasTaskID := props["task_id"]; !hasTaskID { + t.Error("Expected 'task_id' parameter in properties") + } +} + +func TestSpawnStatusTool_NilManager(t *testing.T) { + tool := &SpawnStatusTool{manager: nil} + result := tool.Execute(context.Background(), map[string]any{}) + if !result.IsError { + t.Error("Expected error result when manager is nil") + } +} + +func TestSpawnStatusTool_Empty(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + tool := NewSpawnStatusTool(manager) + + result := tool.Execute(context.Background(), map[string]any{}) + if result.IsError { + t.Fatalf("Expected success, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "No subagents") { + t.Errorf("Expected 'No subagents' message, got: %s", result.ForLLM) + } +} + +func TestSpawnStatusTool_ListAll(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + + now := time.Now().UnixMilli() + manager.mu.Lock() + manager.tasks["subagent-1"] = &SubagentTask{ + ID: "subagent-1", + Task: "Do task A", + Label: "task-a", + Status: "running", + Created: now, + } + manager.tasks["subagent-2"] = &SubagentTask{ + ID: "subagent-2", + Task: "Do task B", + Label: "task-b", + Status: "completed", + Result: "Done successfully", + Created: now, + } + manager.tasks["subagent-3"] = &SubagentTask{ + ID: "subagent-3", + Task: "Do task C", + Status: "failed", + Result: "Error: something went wrong", + } + manager.mu.Unlock() + + tool := NewSpawnStatusTool(manager) + result := tool.Execute(context.Background(), map[string]any{}) + + if result.IsError { + t.Fatalf("Expected success, got error: %s", result.ForLLM) + } + + // Summary header + if !strings.Contains(result.ForLLM, "3 total") { + t.Errorf("Expected total count in header, got: %s", result.ForLLM) + } + + // Individual task IDs + for _, id := range []string{"subagent-1", "subagent-2", "subagent-3"} { + if !strings.Contains(result.ForLLM, id) { + t.Errorf("Expected task %s in output, got:\n%s", id, result.ForLLM) + } + } + + // Status values + for _, status := range []string{"running", "completed", "failed"} { + if !strings.Contains(result.ForLLM, status) { + t.Errorf("Expected status '%s' in output, got:\n%s", status, result.ForLLM) + } + } + + // Result content + if !strings.Contains(result.ForLLM, "Done successfully") { + t.Errorf("Expected result text in output, got:\n%s", result.ForLLM) + } +} + +func TestSpawnStatusTool_GetByID(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + + manager.mu.Lock() + manager.tasks["subagent-42"] = &SubagentTask{ + ID: "subagent-42", + Task: "Specific task", + Label: "my-task", + Status: "failed", + Result: "Something went wrong", + Created: time.Now().UnixMilli(), + } + manager.mu.Unlock() + + tool := NewSpawnStatusTool(manager) + result := tool.Execute(context.Background(), map[string]any{"task_id": "subagent-42"}) + + if result.IsError { + t.Fatalf("Expected success, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "subagent-42") { + t.Errorf("Expected task ID in output, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "failed") { + t.Errorf("Expected status 'failed' in output, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "Something went wrong") { + t.Errorf("Expected result text in output, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "my-task") { + t.Errorf("Expected label in output, got: %s", result.ForLLM) + } +} + +func TestSpawnStatusTool_GetByID_NotFound(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + tool := NewSpawnStatusTool(manager) + + result := tool.Execute(context.Background(), map[string]any{"task_id": "nonexistent-999"}) + if !result.IsError { + t.Errorf("Expected error for nonexistent task, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "nonexistent-999") { + t.Errorf("Expected task ID in error message, got: %s", result.ForLLM) + } +} + +func TestSpawnStatusTool_ResultTruncation(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + + longResult := strings.Repeat("X", 500) + manager.mu.Lock() + manager.tasks["subagent-1"] = &SubagentTask{ + ID: "subagent-1", + Task: "Long task", + Status: "completed", + Result: longResult, + } + manager.mu.Unlock() + + tool := NewSpawnStatusTool(manager) + result := tool.Execute(context.Background(), map[string]any{"task_id": "subagent-1"}) + + if result.IsError { + t.Fatalf("Unexpected error: %s", result.ForLLM) + } + // Output should be shorter than the raw result due to truncation + if len(result.ForLLM) >= len(longResult) { + t.Errorf("Expected result to be truncated, but ForLLM is %d chars", len(result.ForLLM)) + } + if !strings.Contains(result.ForLLM, "…") { + t.Errorf("Expected truncation indicator '…' in output, got: %s", result.ForLLM) + } +} + +func TestSpawnStatusTool_StatusCounts(t *testing.T) { + provider := &MockLLMProvider{} + manager := NewSubagentManager(provider, "test-model", "/tmp/test") + + manager.mu.Lock() + for i, status := range []string{"running", "running", "completed", "failed", "canceled"} { + id := fmt.Sprintf("subagent-%d", i+1) + manager.tasks[id] = &SubagentTask{ID: id, Task: "t", Status: status} + } + manager.mu.Unlock() + + tool := NewSpawnStatusTool(manager) + result := tool.Execute(context.Background(), map[string]any{}) + + if result.IsError { + t.Fatalf("Unexpected error: %s", result.ForLLM) + } + // The summary line should mention all statuses that have counts + for _, want := range []string{"Running:", "Completed:", "Failed:", "Canceled:"} { + if !strings.Contains(result.ForLLM, want) { + t.Errorf("Expected %q in summary, got:\n%s", want, result.ForLLM) + } + } +}