From 1e966a0e9ea87a0efbc6c65ca79e25060c42c8aa Mon Sep 17 00:00:00 2001 From: Jared Mahotiere Date: Sun, 15 Feb 2026 07:30:09 -0500 Subject: [PATCH 1/7] fix: address #179 and #199 with tests --- README.md | 2 +- config.example.json | 2 +- pkg/auth/store_test.go | 33 +++++++------- pkg/channels/slack.go | 20 +++++++-- pkg/channels/slack_test.go | 76 ++++++++++++++++++++++++++++++++ pkg/config/config.go | 42 ++++++++++++++---- pkg/config/config_test.go | 88 +++++++++++++++++++++++++++++++++++++ pkg/cron/service.go | 9 +++- pkg/cron/service_test.go | 42 ++++++++++++++++++ pkg/migrate/migrate_test.go | 18 ++++---- 10 files changed, 293 insertions(+), 39 deletions(-) create mode 100644 pkg/config/config_test.go create mode 100644 pkg/cron/service_test.go diff --git a/README.md b/README.md index 6c9c4bdc2..bc8543eaa 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/config.example.json b/config.example.json index 12dc47316..938230f0b 100644 --- a/config.example.json +++ b/config.example.json @@ -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 diff --git a/pkg/auth/store_test.go b/pkg/auth/store_test.go index d96b460a1..814af5dcc 100644 --- a/pkg/auth/store_test.go +++ b/pkg/auth/store_test.go @@ -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 { diff --git a/pkg/channels/slack.go b/pkg/channels/slack.go index b3ac12e01..6c02ec09c 100644 --- a/pkg/channels/slack.go +++ b/pkg/channels/slack.go @@ -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 diff --git a/pkg/channels/slack_test.go b/pkg/channels/slack_test.go index 3707c2703..5775f1544 100644 --- a/pkg/channels/slack_test.go +++ b/pkg/channels/slack_test.go @@ -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) + } +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 175511108..fc7a68d2d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -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" + } +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go new file mode 100644 index 000000000..7ed7862d3 --- /dev/null +++ b/pkg/config/config_test.go @@ -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") + } +} diff --git a/pkg/cron/service.go b/pkg/cron/service.go index 9434ed875..fbd9bdad0 100644 --- a/pkg/cron/service.go +++ b/pkg/cron/service.go @@ -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) +} diff --git a/pkg/cron/service_test.go b/pkg/cron/service_test.go new file mode 100644 index 000000000..846fca21e --- /dev/null +++ b/pkg/cron/service_test.go @@ -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) + } +} diff --git a/pkg/migrate/migrate_test.go b/pkg/migrate/migrate_test.go index d93ea28fc..ec5088846 100644 --- a/pkg/migrate/migrate_test.go +++ b/pkg/migrate/migrate_test.go @@ -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) } }) } From 3f66ccbbfa47be0bb63caa19942d220771e33ca3 Mon Sep 17 00:00:00 2001 From: Jared Mahotiere Date: Sun, 15 Feb 2026 07:30:19 -0500 Subject: [PATCH 2/7] ci: add gofmt and go test workflow --- .github/workflows/ci.yml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..eb92f0daf --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,33 @@ +name: CI + +on: + push: + branches: + - main + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + + - name: Check formatting + run: | + unformatted="$(gofmt -l .)" + if [ -n "$unformatted" ]; then + echo "The following files need gofmt:" + echo "$unformatted" + exit 1 + fi + + - name: Run tests + run: go test ./... From e2f3b43b81a04696127a99ac4b1f1bd98196f0ac Mon Sep 17 00:00:00 2001 From: Jared Mahotiere Date: Wed, 18 Feb 2026 14:35:43 -0500 Subject: [PATCH 3/7] refactor: centralize private file writes and tighten config migration coverage Move duplicated writePrivateFile logic into pkg/utils.WritePrivateFile, reuse it from config and cron, add the glm-4.7 no-provider-keys test, and extend CI with go vet plus race-enabled tests. --- .github/workflows/ci.yml | 7 +++++-- pkg/config/config.go | 10 ++-------- pkg/config/config_test.go | 20 ++++++++++++++++++++ pkg/cron/service.go | 10 ++-------- pkg/utils/file.go | 11 +++++++++++ 5 files changed, 40 insertions(+), 18 deletions(-) create mode 100644 pkg/utils/file.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eb92f0daf..366e3deae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,5 +29,8 @@ jobs: exit 1 fi - - name: Run tests - run: go test ./... + - name: Run go vet + run: go vet ./... + + - name: Run tests (race) + run: go test -race ./... diff --git a/pkg/config/config.go b/pkg/config/config.go index fc7a68d2d..45f05b84b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/caarlos0/env/v11" + "github.com/sipeed/picoclaw/pkg/utils" ) type Config struct { @@ -252,7 +253,7 @@ func SaveConfig(path string, cfg *Config) error { return err } - return writePrivateFile(path, data) + return utils.WritePrivateFile(path, data) } func (c *Config) WorkspacePath() string { @@ -320,13 +321,6 @@ 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 diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 7ed7862d3..e6e17cbb4 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -86,3 +86,23 @@ func TestLoadConfigKeepsLegacyGLMModelWhenZhipuConfigured(t *testing.T) { t.Fatalf("model = %q, want %q", cfg.Agents.Defaults.Model, "glm-4.7") } } + +func TestLoadConfigKeepsLegacyGLMModelWhenNoProviderKeysConfigured(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.json") + content := `{ + "agents": { "defaults": { "model": "glm-4.7" } } +}` + + 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") + } +} diff --git a/pkg/cron/service.go b/pkg/cron/service.go index fbd9bdad0..b41f11826 100644 --- a/pkg/cron/service.go +++ b/pkg/cron/service.go @@ -12,6 +12,7 @@ import ( "time" "github.com/adhocore/gronx" + "github.com/sipeed/picoclaw/pkg/utils" ) type CronSchedule struct { @@ -318,7 +319,7 @@ func (cs *CronService) saveStoreUnsafe() error { return err } - return writePrivateFile(cs.storePath, data) + return utils.WritePrivateFile(cs.storePath, data) } func (cs *CronService) AddJob(name string, schedule CronSchedule, message string, deliver bool, channel, to string) (*CronJob, error) { @@ -456,10 +457,3 @@ 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) -} diff --git a/pkg/utils/file.go b/pkg/utils/file.go new file mode 100644 index 000000000..b43e891b3 --- /dev/null +++ b/pkg/utils/file.go @@ -0,0 +1,11 @@ +package utils + +import "os" + +// WritePrivateFile writes data and enforces 0600 permissions for both new and existing files. +func WritePrivateFile(path string, data []byte) error { + if err := os.WriteFile(path, data, 0600); err != nil { + return err + } + return os.Chmod(path, 0600) +} From ee798bd0340b21959e426f0acab43ee19fecfcae Mon Sep 17 00:00:00 2001 From: Jared Mahotiere Date: Wed, 18 Feb 2026 14:48:21 -0500 Subject: [PATCH 4/7] ci: generate embedded workspace before vet and tests Run go generate ./cmd/picoclaw on CI so //go:embed workspace assets exist before go vet and race tests. --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 366e3deae..aa8fbcb4a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,6 +29,9 @@ jobs: exit 1 fi + - name: Generate embedded workspace files + run: go generate ./cmd/picoclaw + - name: Run go vet run: go vet ./... From efb68501b9e1e7316a8103eb7e64578305faaf77 Mon Sep 17 00:00:00 2001 From: Jared Mahotiere Date: Mon, 23 Feb 2026 13:18:35 -0500 Subject: [PATCH 5/7] chore: fix lint formatting after conflict resolution --- pkg/channels/slack.go | 16 +--------------- pkg/channels/slack_test.go | 5 +++-- pkg/config/config.go | 1 + pkg/cron/service.go | 1 + pkg/cron/service_test.go | 9 ++++++++- pkg/utils/file.go | 4 ++-- 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/pkg/channels/slack.go b/pkg/channels/slack.go index 7e34d3eb5..5df62896e 100644 --- a/pkg/channels/slack.go +++ b/pkg/channels/slack.go @@ -307,13 +307,6 @@ 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 - } - if !c.IsAllowed(ev.User) { logger.DebugCF("slack", "Mention rejected by allowlist", map[string]any{ "user_id": ev.User, @@ -380,16 +373,9 @@ func (c *SlackChannel) handleSlashCommand(event socketmode.Event) { c.socketClient.Ack(*event.Request) } - if !c.IsAllowed(cmd.UserID) { - logger.DebugCF("slack", "Slash command rejected by allowlist", map[string]any{ - "user_id": cmd.UserID, - }) - return - } - senderID := cmd.UserID if !c.IsAllowed(senderID) { - logger.DebugCF("slack", "Slash command rejected by allowlist", map[string]interface{}{ + logger.DebugCF("slack", "Slash command rejected by allowlist", map[string]any{ "user_id": senderID, "command": cmd.Command, }) diff --git a/pkg/channels/slack_test.go b/pkg/channels/slack_test.go index 5775f1544..36da6adfe 100644 --- a/pkg/channels/slack_test.go +++ b/pkg/channels/slack_test.go @@ -5,11 +5,12 @@ import ( "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" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/config" ) func TestParseSlackChatID(t *testing.T) { diff --git a/pkg/config/config.go b/pkg/config/config.go index f4becfa71..ff8f3a1b8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -8,6 +8,7 @@ import ( "sync/atomic" "github.com/caarlos0/env/v11" + "github.com/sipeed/picoclaw/pkg/utils" ) diff --git a/pkg/cron/service.go b/pkg/cron/service.go index 1787a15c4..98a39efd4 100644 --- a/pkg/cron/service.go +++ b/pkg/cron/service.go @@ -12,6 +12,7 @@ import ( "time" "github.com/adhocore/gronx" + "github.com/sipeed/picoclaw/pkg/utils" ) diff --git a/pkg/cron/service_test.go b/pkg/cron/service_test.go index e02ce42f5..be1997b29 100644 --- a/pkg/cron/service_test.go +++ b/pkg/cron/service_test.go @@ -47,7 +47,14 @@ func TestSaveStoreFixesExistingFilePermissions(t *testing.T) { } cs := NewCronService(storePath, nil) - if _, err := cs.AddJob("perm-test", CronSchedule{Kind: "every", EveryMS: int64Ptr(60000)}, "hello", true, "cli", "direct"); err != nil { + if _, err := cs.AddJob( + "perm-test", + CronSchedule{Kind: "every", EveryMS: int64Ptr(60000)}, + "hello", + true, + "cli", + "direct", + ); err != nil { t.Fatalf("AddJob failed: %v", err) } diff --git a/pkg/utils/file.go b/pkg/utils/file.go index b43e891b3..8849c7f64 100644 --- a/pkg/utils/file.go +++ b/pkg/utils/file.go @@ -4,8 +4,8 @@ import "os" // WritePrivateFile writes data and enforces 0600 permissions for both new and existing files. func WritePrivateFile(path string, data []byte) error { - if err := os.WriteFile(path, data, 0600); err != nil { + if err := os.WriteFile(path, data, 0o600); err != nil { return err } - return os.Chmod(path, 0600) + return os.Chmod(path, 0o600) } From a37b3ba3ff763606b01131f34f38b4693c75dda7 Mon Sep 17 00:00:00 2001 From: Jared Mahotiere Date: Mon, 23 Feb 2026 13:21:39 -0500 Subject: [PATCH 6/7] fix(wecom): guard dedup map cleanup under mutex --- pkg/channels/wecom.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/channels/wecom.go b/pkg/channels/wecom.go index f8daf89de..863efd79a 100644 --- a/pkg/channels/wecom.go +++ b/pkg/channels/wecom.go @@ -339,14 +339,15 @@ func (c *WeComBotChannel) processMessage(ctx context.Context, msg WeComBotMessag return } c.processedMsgs[msgID] = true - c.msgMu.Unlock() - // Clean up old messages periodically (keep last 1000) + // Clean up old messages periodically (keep last 1000). + // Keep the current message id in the new map to preserve dedup behavior. if len(c.processedMsgs) > 1000 { - c.msgMu.Lock() - c.processedMsgs = make(map[string]bool) - c.msgMu.Unlock() + c.processedMsgs = map[string]bool{ + msgID: true, + } } + c.msgMu.Unlock() senderID := msg.From.UserID From 8da7f08a905f92033ed72acda55d4fcbdb351d4c Mon Sep 17 00:00:00 2001 From: Jared Mahotiere Date: Mon, 23 Feb 2026 13:23:49 -0500 Subject: [PATCH 7/7] fix: avoid wecom message dedupe race in async callback --- pkg/channels/wecom.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/channels/wecom.go b/pkg/channels/wecom.go index 863efd79a..b3edbcf44 100644 --- a/pkg/channels/wecom.go +++ b/pkg/channels/wecom.go @@ -37,7 +37,7 @@ type WeComBotChannel struct { ctx context.Context cancel context.CancelFunc processedMsgs map[string]bool // Message deduplication: msg_id -> processed - msgMu sync.RWMutex + msgMu *sync.RWMutex } // WeComBotMessage represents the JSON message structure from WeCom Bot (AIBOT) @@ -102,6 +102,7 @@ func NewWeComBotChannel(cfg config.WeComConfig, messageBus *bus.MessageBus) (*We BaseChannel: base, config: cfg, processedMsgs: make(map[string]bool), + msgMu: &sync.RWMutex{}, }, nil } @@ -330,6 +331,12 @@ func (c *WeComBotChannel) processMessage(ctx context.Context, msg WeComBotMessag // Message deduplication: Use msg_id to prevent duplicate processing msgID := msg.MsgID + if c.msgMu == nil { + c.msgMu = &sync.RWMutex{} + } + if c.processedMsgs == nil { + c.processedMsgs = make(map[string]bool) + } c.msgMu.Lock() if c.processedMsgs[msgID] { c.msgMu.Unlock()