fix: address #179 and #199 with tests

This commit is contained in:
Jared Mahotiere 2026-02-15 07:30:09 -05:00
parent a7bbda147e
commit 1e966a0e9e
10 changed files with 293 additions and 39 deletions

View file

@ -138,7 +138,7 @@ picoclaw onboard
"agents": {
"defaults": {
"workspace": "~/.picoclaw/workspace",
"model": "glm-4.7",
"model": "openrouter/auto",
"max_tokens": 8192,
"temperature": 0.7,
"max_tool_iterations": 20

View file

@ -2,7 +2,7 @@
"agents": {
"defaults": {
"workspace": "~/.picoclaw/workspace",
"model": "glm-4.7",
"model": "openrouter/auto",
"max_tokens": 8192,
"temperature": 0.7,
"max_tool_iterations": 20

View file

@ -3,10 +3,19 @@ package auth
import (
"os"
"path/filepath"
"runtime"
"testing"
"time"
)
func setTestHome(t *testing.T, dir string) {
t.Helper()
t.Setenv("HOME", dir)
t.Setenv("USERPROFILE", dir)
t.Setenv("HOMEDRIVE", "")
t.Setenv("HOMEPATH", "")
}
func TestAuthCredentialIsExpired(t *testing.T) {
tests := []struct {
name string
@ -52,9 +61,7 @@ func TestAuthCredentialNeedsRefresh(t *testing.T) {
func TestStoreRoundtrip(t *testing.T) {
tmpDir := t.TempDir()
origHome := os.Getenv("HOME")
t.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", origHome)
setTestHome(t, tmpDir)
cred := &AuthCredential{
AccessToken: "test-access-token",
@ -88,10 +95,12 @@ func TestStoreRoundtrip(t *testing.T) {
}
func TestStoreFilePermissions(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("file permission bits are not reliable on windows")
}
tmpDir := t.TempDir()
origHome := os.Getenv("HOME")
t.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", origHome)
setTestHome(t, tmpDir)
cred := &AuthCredential{
AccessToken: "secret-token",
@ -115,9 +124,7 @@ func TestStoreFilePermissions(t *testing.T) {
func TestStoreMultiProvider(t *testing.T) {
tmpDir := t.TempDir()
origHome := os.Getenv("HOME")
t.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", origHome)
setTestHome(t, tmpDir)
openaiCred := &AuthCredential{AccessToken: "openai-token", Provider: "openai", AuthMethod: "oauth"}
anthropicCred := &AuthCredential{AccessToken: "anthropic-token", Provider: "anthropic", AuthMethod: "token"}
@ -148,9 +155,7 @@ func TestStoreMultiProvider(t *testing.T) {
func TestDeleteCredential(t *testing.T) {
tmpDir := t.TempDir()
origHome := os.Getenv("HOME")
t.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", origHome)
setTestHome(t, tmpDir)
cred := &AuthCredential{AccessToken: "to-delete", Provider: "openai", AuthMethod: "oauth"}
if err := SetCredential("openai", cred); err != nil {
@ -172,9 +177,7 @@ func TestDeleteCredential(t *testing.T) {
func TestLoadStoreEmpty(t *testing.T) {
tmpDir := t.TempDir()
origHome := os.Getenv("HOME")
t.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", origHome)
setTestHome(t, tmpDir)
store, err := LoadStore()
if err != nil {

View file

@ -282,9 +282,9 @@ func (c *SlackChannel) handleMessageEvent(ev *slackevents.MessageEvent) {
}
logger.DebugCF("slack", "Received message", map[string]interface{}{
"sender_id": senderID,
"chat_id": chatID,
"preview": utils.Truncate(content, 50),
"sender_id": senderID,
"chat_id": chatID,
"preview": utils.Truncate(content, 50),
"has_thread": threadTS != "",
})
@ -295,6 +295,12 @@ func (c *SlackChannel) handleAppMention(ev *slackevents.AppMentionEvent) {
if ev.User == c.botUserID {
return
}
if !c.IsAllowed(ev.User) {
logger.DebugCF("slack", "App mention rejected by allowlist", map[string]interface{}{
"user_id": ev.User,
})
return
}
senderID := ev.User
channelID := ev.Channel
@ -346,6 +352,14 @@ func (c *SlackChannel) handleSlashCommand(event socketmode.Event) {
}
senderID := cmd.UserID
if !c.IsAllowed(senderID) {
logger.DebugCF("slack", "Slash command rejected by allowlist", map[string]interface{}{
"user_id": senderID,
"command": cmd.Command,
})
return
}
channelID := cmd.ChannelID
chatID := channelID
content := cmd.Text

View file

@ -1,10 +1,15 @@
package channels
import (
"context"
"testing"
"time"
"github.com/sipeed/picoclaw/pkg/bus"
"github.com/sipeed/picoclaw/pkg/config"
"github.com/slack-go/slack"
"github.com/slack-go/slack/slackevents"
"github.com/slack-go/slack/socketmode"
)
func TestParseSlackChatID(t *testing.T) {
@ -172,3 +177,74 @@ func TestSlackChannelIsAllowed(t *testing.T) {
}
})
}
func TestSlackAppMentionRejectsUsersOutsideAllowlist(t *testing.T) {
msgBus := bus.NewMessageBus()
cfg := config.SlackConfig{
BotToken: "xoxb-test",
AppToken: "xapp-test",
AllowFrom: []string{"U_ALLOWED"},
}
ch, err := NewSlackChannel(cfg, msgBus)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
ch.botUserID = "U_BOT"
ch.api = nil
ev := &slackevents.AppMentionEvent{
User: "U_BLOCKED",
Channel: "C123456",
TimeStamp: "1700000000.000001",
Text: "<@U_BOT> hello",
}
ch.handleAppMention(ev)
chatID := "C123456/1700000000.000001"
if _, ok := ch.pendingAcks.Load(chatID); ok {
t.Fatalf("blocked user should not create pending ack for chat %s", chatID)
}
assertNoInboundMessage(t, msgBus)
}
func TestSlackSlashCommandRejectsUsersOutsideAllowlist(t *testing.T) {
msgBus := bus.NewMessageBus()
cfg := config.SlackConfig{
BotToken: "xoxb-test",
AppToken: "xapp-test",
AllowFrom: []string{"U_ALLOWED"},
}
ch, err := NewSlackChannel(cfg, msgBus)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
event := socketmode.Event{
Data: slack.SlashCommand{
UserID: "U_BLOCKED",
ChannelID: "C123456",
Command: "/picoclaw",
Text: "hello",
},
}
ch.handleSlashCommand(event)
assertNoInboundMessage(t, msgBus)
}
func assertNoInboundMessage(t *testing.T, msgBus *bus.MessageBus) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
if msg, ok := msgBus.ConsumeInbound(ctx); ok {
t.Fatalf("expected no inbound message, got %+v", msg)
}
}

View file

@ -83,16 +83,16 @@ type QQConfig struct {
}
type DingTalkConfig struct {
Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_DINGTALK_ENABLED"`
ClientID string `json:"client_id" env:"PICOCLAW_CHANNELS_DINGTALK_CLIENT_ID"`
ClientSecret string `json:"client_secret" env:"PICOCLAW_CHANNELS_DINGTALK_CLIENT_SECRET"`
AllowFrom []string `json:"allow_from" env:"PICOCLAW_CHANNELS_DINGTALK_ALLOW_FROM"`
Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_DINGTALK_ENABLED"`
ClientID string `json:"client_id" env:"PICOCLAW_CHANNELS_DINGTALK_CLIENT_ID"`
ClientSecret string `json:"client_secret" env:"PICOCLAW_CHANNELS_DINGTALK_CLIENT_SECRET"`
AllowFrom []string `json:"allow_from" env:"PICOCLAW_CHANNELS_DINGTALK_ALLOW_FROM"`
}
type SlackConfig struct {
Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_SLACK_ENABLED"`
BotToken string `json:"bot_token" env:"PICOCLAW_CHANNELS_SLACK_BOT_TOKEN"`
AppToken string `json:"app_token" env:"PICOCLAW_CHANNELS_SLACK_APP_TOKEN"`
Enabled bool `json:"enabled" env:"PICOCLAW_CHANNELS_SLACK_ENABLED"`
BotToken string `json:"bot_token" env:"PICOCLAW_CHANNELS_SLACK_BOT_TOKEN"`
AppToken string `json:"app_token" env:"PICOCLAW_CHANNELS_SLACK_APP_TOKEN"`
AllowFrom []string `json:"allow_from" env:"PICOCLAW_CHANNELS_SLACK_ALLOW_FROM"`
}
@ -135,7 +135,7 @@ func DefaultConfig() *Config {
Agents: AgentsConfig{
Defaults: AgentDefaults{
Workspace: "~/.picoclaw/workspace",
Model: "glm-4.7",
Model: "openrouter/auto",
MaxTokens: 8192,
Temperature: 0.7,
MaxToolIterations: 20,
@ -233,6 +233,8 @@ func LoadConfig(path string) (*Config, error) {
return nil, err
}
normalizeLegacyModelDefaults(cfg)
return cfg, nil
}
@ -250,7 +252,7 @@ func SaveConfig(path string, cfg *Config) error {
return err
}
return os.WriteFile(path, data, 0644)
return writePrivateFile(path, data)
}
func (c *Config) WorkspacePath() string {
@ -317,3 +319,25 @@ func expandHome(path string) string {
}
return path
}
func writePrivateFile(path string, data []byte) error {
if err := os.WriteFile(path, data, 0600); err != nil {
return err
}
return os.Chmod(path, 0600)
}
func normalizeLegacyModelDefaults(cfg *Config) {
if cfg == nil {
return
}
if cfg.Agents.Defaults.Model != "glm-4.7" {
return
}
if cfg.Providers.Zhipu.APIKey != "" {
return
}
if cfg.Providers.OpenRouter.APIKey != "" {
cfg.Agents.Defaults.Model = "openrouter/auto"
}
}

88
pkg/config/config_test.go Normal file
View file

@ -0,0 +1,88 @@
package config
import (
"os"
"path/filepath"
"runtime"
"testing"
)
func TestDefaultConfigUsesOpenRouterAutoModel(t *testing.T) {
cfg := DefaultConfig()
if cfg.Agents.Defaults.Model != "openrouter/auto" {
t.Fatalf("default model = %q, want %q", cfg.Agents.Defaults.Model, "openrouter/auto")
}
}
func TestSaveConfigWritesPrivatePermissions(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("file permission bits are not reliable on windows")
}
path := filepath.Join(t.TempDir(), "config.json")
if err := os.WriteFile(path, []byte("{}"), 0644); err != nil {
t.Fatalf("failed to create seed config: %v", err)
}
cfg := DefaultConfig()
cfg.Providers.OpenRouter.APIKey = "secret"
if err := SaveConfig(path, cfg); err != nil {
t.Fatalf("SaveConfig failed: %v", err)
}
info, err := os.Stat(path)
if err != nil {
t.Fatalf("stat config: %v", err)
}
if got := info.Mode().Perm(); got != 0600 {
t.Fatalf("config perms = %o, want 600", got)
}
}
func TestLoadConfigNormalizesLegacyGLMModelForOpenRouter(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
content := `{
"agents": { "defaults": { "model": "glm-4.7" } },
"providers": { "openrouter": { "api_key": "sk-or-test" } }
}`
if err := os.WriteFile(path, []byte(content), 0644); err != nil {
t.Fatalf("failed to write config fixture: %v", err)
}
cfg, err := LoadConfig(path)
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}
if cfg.Agents.Defaults.Model != "openrouter/auto" {
t.Fatalf("model = %q, want %q", cfg.Agents.Defaults.Model, "openrouter/auto")
}
}
func TestLoadConfigKeepsLegacyGLMModelWhenZhipuConfigured(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
content := `{
"agents": { "defaults": { "model": "glm-4.7" } },
"providers": {
"openrouter": { "api_key": "sk-or-test" },
"zhipu": { "api_key": "zhipu-key" }
}
}`
if err := os.WriteFile(path, []byte(content), 0644); err != nil {
t.Fatalf("failed to write config fixture: %v", err)
}
cfg, err := LoadConfig(path)
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}
if cfg.Agents.Defaults.Model != "glm-4.7" {
t.Fatalf("model = %q, want %q", cfg.Agents.Defaults.Model, "glm-4.7")
}
}

View file

@ -318,7 +318,7 @@ func (cs *CronService) saveStoreUnsafe() error {
return err
}
return os.WriteFile(cs.storePath, data, 0644)
return writePrivateFile(cs.storePath, data)
}
func (cs *CronService) AddJob(name string, schedule CronSchedule, message string, deliver bool, channel, to string) (*CronJob, error) {
@ -456,3 +456,10 @@ func generateID() string {
}
return hex.EncodeToString(b)
}
func writePrivateFile(path string, data []byte) error {
if err := os.WriteFile(path, data, 0600); err != nil {
return err
}
return os.Chmod(path, 0600)
}

42
pkg/cron/service_test.go Normal file
View file

@ -0,0 +1,42 @@
package cron
import (
"os"
"path/filepath"
"runtime"
"testing"
)
func TestAddJobWritesPrivatePermissions(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("file permission bits are not reliable on windows")
}
storePath := filepath.Join(t.TempDir(), "cron", "jobs.json")
if err := os.MkdirAll(filepath.Dir(storePath), 0755); err != nil {
t.Fatalf("failed to create store dir: %v", err)
}
if err := os.WriteFile(storePath, []byte(`{"version":1,"jobs":[]}`), 0644); err != nil {
t.Fatalf("failed to create seed cron store: %v", err)
}
cs := NewCronService(storePath, nil)
everyMS := int64(60000)
schedule := CronSchedule{
Kind: "every",
EveryMS: &everyMS,
}
if _, err := cs.AddJob("perm-test", schedule, "hello", true, "cli", "direct"); err != nil {
t.Fatalf("AddJob failed: %v", err)
}
info, err := os.Stat(storePath)
if err != nil {
t.Fatalf("stat cron store: %v", err)
}
if got := info.Mode().Perm(); got != 0600 {
t.Fatalf("cron store perms = %o, want 600", got)
}
}

View file

@ -44,8 +44,8 @@ func TestConvertKeysToSnake(t *testing.T) {
"apiKey": "test-key",
"apiBase": "https://example.com",
"nested": map[string]interface{}{
"maxTokens": float64(8192),
"allowFrom": []interface{}{"user1", "user2"},
"maxTokens": float64(8192),
"allowFrom": []interface{}{"user1", "user2"},
"deeperLevel": map[string]interface{}{
"clientId": "abc",
},
@ -256,11 +256,11 @@ func TestConvertConfig(t *testing.T) {
data := map[string]interface{}{
"agents": map[string]interface{}{
"defaults": map[string]interface{}{
"model": "claude-3-opus",
"max_tokens": float64(4096),
"temperature": 0.5,
"max_tool_iterations": float64(10),
"workspace": "~/.openclaw/workspace",
"model": "claude-3-opus",
"max_tokens": float64(4096),
"temperature": 0.5,
"max_tool_iterations": float64(10),
"workspace": "~/.openclaw/workspace",
},
},
}
@ -293,8 +293,8 @@ func TestConvertConfig(t *testing.T) {
if len(warnings) != 0 {
t.Errorf("expected no warnings, got %v", warnings)
}
if cfg.Agents.Defaults.Model != "glm-4.7" {
t.Errorf("default model should be glm-4.7, got %q", cfg.Agents.Defaults.Model)
if cfg.Agents.Defaults.Model != "openrouter/auto" {
t.Errorf("default model should be openrouter/auto, got %q", cfg.Agents.Defaults.Model)
}
})
}