fix: tighten file perms and enforce Slack ACL checks (#186)
- write config and cron store with 0600 instead of 0644 - check allow list in Slack slash commands and app mentions - pass workspace restrict flag to cron exec tool Closes #179
This commit is contained in:
@@ -562,7 +562,7 @@ func gatewayCmd() {
|
|||||||
})
|
})
|
||||||
|
|
||||||
// Setup cron tool and service
|
// Setup cron tool and service
|
||||||
cronService := setupCronTool(agentLoop, msgBus, cfg.WorkspacePath())
|
cronService := setupCronTool(agentLoop, msgBus, cfg.WorkspacePath(), cfg.Agents.Defaults.RestrictToWorkspace)
|
||||||
|
|
||||||
heartbeatService := heartbeat.NewHeartbeatService(
|
heartbeatService := heartbeat.NewHeartbeatService(
|
||||||
cfg.WorkspacePath(),
|
cfg.WorkspacePath(),
|
||||||
@@ -984,14 +984,14 @@ func getConfigPath() string {
|
|||||||
return filepath.Join(home, ".picoclaw", "config.json")
|
return filepath.Join(home, ".picoclaw", "config.json")
|
||||||
}
|
}
|
||||||
|
|
||||||
func setupCronTool(agentLoop *agent.AgentLoop, msgBus *bus.MessageBus, workspace string) *cron.CronService {
|
func setupCronTool(agentLoop *agent.AgentLoop, msgBus *bus.MessageBus, workspace string, restrict bool) *cron.CronService {
|
||||||
cronStorePath := filepath.Join(workspace, "cron", "jobs.json")
|
cronStorePath := filepath.Join(workspace, "cron", "jobs.json")
|
||||||
|
|
||||||
// Create cron service
|
// Create cron service
|
||||||
cronService := cron.NewCronService(cronStorePath, nil)
|
cronService := cron.NewCronService(cronStorePath, nil)
|
||||||
|
|
||||||
// Create and register CronTool
|
// Create and register CronTool
|
||||||
cronTool := tools.NewCronTool(cronService, agentLoop, msgBus, workspace)
|
cronTool := tools.NewCronTool(cronService, agentLoop, msgBus, workspace, restrict)
|
||||||
agentLoop.RegisterTool(cronTool)
|
agentLoop.RegisterTool(cronTool)
|
||||||
|
|
||||||
// Set the onJob handler
|
// Set the onJob handler
|
||||||
|
|||||||
@@ -296,6 +296,13 @@ func (c *SlackChannel) handleAppMention(ev *slackevents.AppMentionEvent) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !c.IsAllowed(ev.User) {
|
||||||
|
logger.DebugCF("slack", "Mention rejected by allowlist", map[string]interface{}{
|
||||||
|
"user_id": ev.User,
|
||||||
|
})
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
senderID := ev.User
|
senderID := ev.User
|
||||||
channelID := ev.Channel
|
channelID := ev.Channel
|
||||||
threadTS := ev.ThreadTimeStamp
|
threadTS := ev.ThreadTimeStamp
|
||||||
@@ -345,6 +352,13 @@ func (c *SlackChannel) handleSlashCommand(event socketmode.Event) {
|
|||||||
c.socketClient.Ack(*event.Request)
|
c.socketClient.Ack(*event.Request)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !c.IsAllowed(cmd.UserID) {
|
||||||
|
logger.DebugCF("slack", "Slash command rejected by allowlist", map[string]interface{}{
|
||||||
|
"user_id": cmd.UserID,
|
||||||
|
})
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
senderID := cmd.UserID
|
senderID := cmd.UserID
|
||||||
channelID := cmd.ChannelID
|
channelID := cmd.ChannelID
|
||||||
chatID := channelID
|
chatID := channelID
|
||||||
|
|||||||
@@ -370,7 +370,7 @@ func SaveConfig(path string, cfg *Config) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
return os.WriteFile(path, data, 0644)
|
return os.WriteFile(path, data, 0600)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c *Config) WorkspacePath() string {
|
func (c *Config) WorkspacePath() string {
|
||||||
|
|||||||
@@ -1,6 +1,9 @@
|
|||||||
package config
|
package config
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"runtime"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -147,6 +150,30 @@ func TestDefaultConfig_WebTools(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSaveConfig_FilePermissions(t *testing.T) {
|
||||||
|
if runtime.GOOS == "windows" {
|
||||||
|
t.Skip("file permission bits are not enforced on Windows")
|
||||||
|
}
|
||||||
|
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
path := filepath.Join(tmpDir, "config.json")
|
||||||
|
|
||||||
|
cfg := DefaultConfig()
|
||||||
|
if err := SaveConfig(path, cfg); err != nil {
|
||||||
|
t.Fatalf("SaveConfig failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
info, err := os.Stat(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Stat failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
perm := info.Mode().Perm()
|
||||||
|
if perm != 0600 {
|
||||||
|
t.Errorf("config file has permission %04o, want 0600", perm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestConfig_Complete verifies all config fields are set
|
// TestConfig_Complete verifies all config fields are set
|
||||||
func TestConfig_Complete(t *testing.T) {
|
func TestConfig_Complete(t *testing.T) {
|
||||||
cfg := DefaultConfig()
|
cfg := DefaultConfig()
|
||||||
|
|||||||
@@ -340,7 +340,7 @@ func (cs *CronService) saveStoreUnsafe() error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
return os.WriteFile(cs.storePath, data, 0644)
|
return os.WriteFile(cs.storePath, data, 0600)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (cs *CronService) AddJob(name string, schedule CronSchedule, message string, deliver bool, channel, to string) (*CronJob, error) {
|
func (cs *CronService) AddJob(name string, schedule CronSchedule, message string, deliver bool, channel, to string) (*CronJob, error) {
|
||||||
|
|||||||
38
pkg/cron/service_test.go
Normal file
38
pkg/cron/service_test.go
Normal file
@@ -0,0 +1,38 @@
|
|||||||
|
package cron
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"runtime"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestSaveStore_FilePermissions(t *testing.T) {
|
||||||
|
if runtime.GOOS == "windows" {
|
||||||
|
t.Skip("file permission bits are not enforced on Windows")
|
||||||
|
}
|
||||||
|
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
storePath := filepath.Join(tmpDir, "cron", "jobs.json")
|
||||||
|
|
||||||
|
cs := NewCronService(storePath, nil)
|
||||||
|
|
||||||
|
_, err := cs.AddJob("test", CronSchedule{Kind: "every", EveryMS: int64Ptr(60000)}, "hello", false, "cli", "direct")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("AddJob failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
info, err := os.Stat(storePath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Stat failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
perm := info.Mode().Perm()
|
||||||
|
if perm != 0600 {
|
||||||
|
t.Errorf("cron store has permission %04o, want 0600", perm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func int64Ptr(v int64) *int64 {
|
||||||
|
return &v
|
||||||
|
}
|
||||||
@@ -28,12 +28,12 @@ type CronTool struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// NewCronTool creates a new CronTool
|
// NewCronTool creates a new CronTool
|
||||||
func NewCronTool(cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string) *CronTool {
|
func NewCronTool(cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool) *CronTool {
|
||||||
return &CronTool{
|
return &CronTool{
|
||||||
cronService: cronService,
|
cronService: cronService,
|
||||||
executor: executor,
|
executor: executor,
|
||||||
msgBus: msgBus,
|
msgBus: msgBus,
|
||||||
execTool: NewExecTool(workspace, false),
|
execTool: NewExecTool(workspace, restrict),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user