From 71fbf6db9de31f704c04dcc42404e3c26af2e9e1 Mon Sep 17 00:00:00 2001 From: sky5454 Date: Mon, 16 Mar 2026 11:28:34 +0800 Subject: [PATCH] lint: fix credential keygen lint, fix test keygen --- cmd/picoclaw/internal/onboard/helpers.go | 5 +++-- pkg/config/config.go | 4 +++- pkg/config/config_test.go | 24 ++++++++++++++++++------ pkg/credential/credential.go | 15 ++++++++++----- pkg/credential/keygen.go | 2 +- pkg/credential/keygen_test.go | 10 ++++++++-- 6 files changed, 43 insertions(+), 17 deletions(-) diff --git a/cmd/picoclaw/internal/onboard/helpers.go b/cmd/picoclaw/internal/onboard/helpers.go index 365dcffa5..cd637d89b 100644 --- a/cmd/picoclaw/internal/onboard/helpers.go +++ b/cmd/picoclaw/internal/onboard/helpers.go @@ -6,10 +6,11 @@ import ( "os" "path/filepath" + "golang.org/x/term" + "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() { @@ -49,7 +50,7 @@ func onboard() { // the current process and disappears when it exits. os.Setenv(credential.PassphraseEnvVar, passphrase) - if err := setupSSHKey(); err != nil { + if err = setupSSHKey(); err != nil { fmt.Printf("Error generating SSH key: %v\n", err) os.Exit(1) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 01b2fe4d0..2c03f7910 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -834,7 +834,9 @@ func LoadConfig(path string) (*Config, error) { 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) + fmt.Fprintf(os.Stderr, + "picoclaw: warning: model %q has a plaintext api_key; call SaveConfig to encrypt it\n", + m.ModelName) } } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 0ce77ab08..426113e86 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -11,6 +11,18 @@ import ( "github.com/sipeed/picoclaw/pkg/credential" ) +// mustSetupSSHKey generates a temporary Ed25519 SSH key in t.TempDir() and sets +// PICOCLAW_SSH_KEY_PATH to its path for the duration of the test. This is required +// whenever a test exercises encryption/decryption via credential.Encrypt or SaveConfig. +func mustSetupSSHKey(t *testing.T) { + t.Helper() + keyPath := filepath.Join(t.TempDir(), "picoclaw_ed25519.key") + if err := credential.GenerateSSHKey(keyPath); err != nil { + t.Fatalf("mustSetupSSHKey: %v", err) + } + t.Setenv("PICOCLAW_SSH_KEY_PATH", keyPath) +} + func TestAgentModelConfig_UnmarshalString(t *testing.T) { var m AgentModelConfig if err := json.Unmarshal([]byte(`"gpt-4"`), &m); err != nil { @@ -666,7 +678,7 @@ func TestSaveConfig_EncryptsPlaintextAPIKey(t *testing.T) { cfgPath := filepath.Join(dir, "config.json") t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") - t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + mustSetupSSHKey(t) cfg := DefaultConfig() cfg.ModelList = []ModelConfig{ @@ -755,7 +767,7 @@ func TestSaveConfig_MixedKeys(t *testing.T) { cfgPath := filepath.Join(dir, "config.json") t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") - t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + mustSetupSSHKey(t) // Pre-encrypt one key so we have a genuine enc:// value to put in the config. if err := SaveConfig(cfgPath, &Config{ @@ -844,7 +856,7 @@ func TestLoadConfig_MixedKeys_NoPassphrase(t *testing.T) { // First encrypt a key so we have a real enc:// value. t.Setenv("PICOCLAW_KEY_PASSPHRASE", "test-passphrase") - t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + mustSetupSSHKey(t) if err := SaveConfig(cfgPath, &Config{ ModelList: []ModelConfig{ {ModelName: "m", Model: "openai/gpt-4", APIKey: "sk-secret"}, @@ -901,7 +913,7 @@ func TestSaveConfig_UsesPassphraseProvider(t *testing.T) { // Ensure the env var is empty — passphrase must come from PassphraseProvider only. t.Setenv("PICOCLAW_KEY_PASSPHRASE", "") - t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + mustSetupSSHKey(t) // Replace PassphraseProvider with an in-memory function (simulating SecureStore). const testPassphrase = "provider-passphrase" @@ -931,7 +943,7 @@ func TestLoadConfig_UsesPassphraseProvider(t *testing.T) { // Ensure the env var is empty throughout. t.Setenv("PICOCLAW_KEY_PASSPHRASE", "") - t.Setenv("PICOCLAW_SSH_KEY_PATH", "") + mustSetupSSHKey(t) const testPassphrase = "provider-passphrase" const plainKey = "sk-secret" @@ -947,7 +959,7 @@ func TestLoadConfig_UsesPassphraseProvider(t *testing.T) { {"model_name": "test", "model": "openai/gpt-4", "api_key": encrypted}, }, }) - if err := os.WriteFile(cfgPath, raw, 0o600); err != nil { + if err = os.WriteFile(cfgPath, raw, 0o600); err != nil { t.Fatalf("setup: %v", err) } diff --git a/pkg/credential/credential.go b/pkg/credential/credential.go index 98bfe4c6a..83af3fc9f 100644 --- a/pkg/credential/credential.go +++ b/pkg/credential/credential.go @@ -1,6 +1,6 @@ // Package credential resolves API credential values for model_list entries. // -// An API key is a form of authorization credential. This package centralises +// An API key is a form of authorization credential. This package centralizes // how raw credential strings—plaintext or file references—are resolved into // their actual values, keeping that logic out of the config loader. // @@ -88,8 +88,8 @@ type Resolver struct { func NewResolver(configDir string) *Resolver { resolved := configDir if configDir != "" { - if real, err := filepath.EvalSymlinks(configDir); err == nil { - resolved = real + if linkedPath, err := filepath.EvalSymlinks(configDir); err == nil { + resolved = linkedPath } } return &Resolver{configDir: configDir, resolvedConfigDir: resolved} @@ -278,10 +278,15 @@ func allowedSSHKeyPath(path string) bool { // sshKeyPath must be non-empty; returns an error otherwise. func deriveKey(passphrase, sshKeyPath string, salt []byte) ([]byte, error) { if sshKeyPath == "" { - return nil, fmt.Errorf("credential: SSH private key is required but not found (set PICOCLAW_SSH_KEY_PATH or place key at ~/.ssh/picoclaw_ed25519.key)") + return nil, fmt.Errorf( + "credential: SSH private key is required but not found" + + " (set PICOCLAW_SSH_KEY_PATH or place key at ~/.ssh/picoclaw_ed25519.key)") } 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) + 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 { diff --git a/pkg/credential/keygen.go b/pkg/credential/keygen.go index a97bd3f27..c57564a76 100644 --- a/pkg/credential/keygen.go +++ b/pkg/credential/keygen.go @@ -42,7 +42,7 @@ func GenerateSSHKey(path string) error { } privPEM := pem.EncodeToMemory(block) - if err := os.WriteFile(path, privPEM, 0o600); err != nil { + if err = os.WriteFile(path, privPEM, 0o600); err != nil { return fmt.Errorf("credential: keygen: write private key %q: %w", path, err) } diff --git a/pkg/credential/keygen_test.go b/pkg/credential/keygen_test.go index 736b68f35..1e21ea0b9 100644 --- a/pkg/credential/keygen_test.go +++ b/pkg/credential/keygen_test.go @@ -61,10 +61,16 @@ func TestGenerateSSHKey_CreatesFiles(t *testing.T) { if err != nil { t.Fatalf("read public key: %v", err) } - _, _, _, _, err = ssh.ParseAuthorizedKey(pubBytes) + pubKey, _, _, rest, err := ssh.ParseAuthorizedKey(pubBytes) if err != nil { t.Fatalf("parse public key: %v", err) } + if pubKey == nil { + t.Fatal("expected non-nil public key") + } + if len(rest) > 0 { + t.Errorf("unexpected trailing bytes after public key: %d bytes", len(rest)) + } } func TestGenerateSSHKey_OverwritesExisting(t *testing.T) { @@ -80,7 +86,7 @@ func TestGenerateSSHKey_OverwritesExisting(t *testing.T) { t.Fatalf("read first key: %v", err) } - if err := GenerateSSHKey(keyPath); err != nil { + if err = GenerateSSHKey(keyPath); err != nil { t.Fatalf("second GenerateSSHKey() error = %v", err) } second, err := os.ReadFile(keyPath)