Merge pull request #1 from maxidce/codex/apply-complete-code-hardening-for-go-repo

Codex-generated pull request
2026-02-15 21:49:49 -03:00 · 2026-02-15 21:49:49 -03:00 · 92aef5f3f0
commit 92aef5f3f0
parent 1d748fb742 16f094d8f8
27 changed files with 442 additions and 104 deletions
--- a/.github/workflows/secret-scan.yml
+++ b/.github/workflows/secret-scan.yml
@ -0,0 +1,23 @@
+name: secret-scan
+
+on:
+  pull_request:
+  push:
+    branches: [ main, master ]
+
+jobs:
+  gitleaks:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Gitleaks scan
+        uses: gitleaks/gitleaks-action@v2
+        env:
+          GITLEAKS_ENABLE_UPLOAD_ARTIFACT: false
+      - name: Block sk- like keys in tracked files
+        run: |
+          if rg -n "sk-[A-Za-z0-9_-]{10,}" --glob '!README.md' --glob '!README.*.md' --glob '!.git/**' . ; then
+            echo "::error::Potential secret key pattern detected (sk-...)"
+            exit 1
+          fi
--- a/.gitignore
+++ b/.gitignore
@ -24,6 +24,14 @@ build/

 # Secrets & Config (keep templates, ignore actual secrets)
 .env
+.env.*
+.secrets/
+.secrets/local.env
+*.pem
+*.key
+*.p12
+*.pfx
+*secrets*.json
 config/config.json

 # Test
--- a/.gitleaks.toml
+++ b/.gitleaks.toml
@ -0,0 +1,10 @@
+[extend]
+useDefault = true
+
+[[allowlists]]
+description = "Allow example docs"
+paths = [
+  '''README\.md''',
+  '''README\.zh\.md''',
+  '''README\.ja\.md'''
+]
--- a/Dockerfile.hardened
+++ b/Dockerfile.hardened
@ -0,0 +1,18 @@
+FROM golang:1.26.0-alpine AS builder
+RUN apk add --no-cache git make
+WORKDIR /src
+COPY go.mod go.sum ./
+RUN go mod download
+COPY . .
+RUN make build
+
+FROM alpine:3.23
+RUN apk add --no-cache ca-certificates tzdata curl \
+    && addgroup -S picoclaw && adduser -S -G picoclaw -h /home/picoclaw picoclaw \
+    && mkdir -p /home/picoclaw/.picoclaw/workspace /home/picoclaw/.picoclaw/config \
+    && chown -R picoclaw:picoclaw /home/picoclaw
+COPY --from=builder /src/build/picoclaw /usr/local/bin/picoclaw
+USER picoclaw
+WORKDIR /home/picoclaw
+ENTRYPOINT ["picoclaw"]
+CMD ["gateway"]
--- a/README.md
+++ b/README.md
@ -353,7 +353,7 @@ picoclaw gateway
 }
 ```

-> Set `allow_from` to empty to allow all users, or specify QQ numbers to restrict access.
+> `allow_from` is deny-by-default. You must explicitly list allowed users/channels.

 **3. Run**

@ -387,7 +387,7 @@ picoclaw gateway
 }
 ```

-> Set `allow_from` to empty to allow all users, or specify QQ numbers to restrict access.
+> `allow_from` is deny-by-default. You must explicitly list allowed users/channels.

 **3. Run**

@ -415,7 +415,7 @@ picoclaw gateway
      "enabled": true,
      "channel_secret": "YOUR_CHANNEL_SECRET",
      "channel_access_token": "YOUR_CHANNEL_ACCESS_TOKEN",
-      "webhook_host": "0.0.0.0",
+      "webhook_host": "127.0.0.1",
      "webhook_port": 18791,
      "webhook_path": "/webhook/line",
      "allow_from": []
@ -513,7 +513,7 @@ When `restrict_to_workspace: true`, the following tools are sandboxed:

 #### Additional Exec Protection

-Even with `restrict_to_workspace: false`, the `exec` tool blocks these dangerous commands:
+By default, `exec` blocks shell metacharacters and dangerous command patterns:

 * `rm -rf`, `del /f`, `rmdir /s` — Bulk deletion
 * `format`, `mkfs`, `diskpart` — Disk formatting
@ -522,6 +522,63 @@ Even with `restrict_to_workspace: false`, the `exec` tool blocks these dangerous
 * `shutdown`, `reboot`, `poweroff` — System shutdown
 * Fork bomb `:(){ :|:& };:`

+
+### Secrets Management (Hardening)
+
+- Never commit API keys in `config.json`, docs, or source code.
+- Primary source for OpenAI key: `OPENAI_API_KEY` environment variable.
+- Optional local-dev source: `.secrets/local.env` (auto-loaded if present, ignored by git).
+- Optional Docker secret file source: `OPENAI_API_KEY_FILE=/run/secrets/openai_api_key`.
+- Model override: `OPENAI_MODEL` (default from config, recommended `gpt-4o-mini`).
+
+```bash
+mkdir -p .secrets
+printf 'OPENAI_API_KEY=...\nOPENAI_MODEL=gpt-4o-mini\n' > .secrets/local.env
+chmod 600 .secrets/local.env
+```
+
+### GitHub Actions Secrets
+
+1. Go to repository **Settings → Secrets and variables → Actions**.
+2. Add `OPENAI_API_KEY`.
+3. In workflow jobs, expose it as env:
+
+```yaml
+env:
+  OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
+```
+
+### Secret Leak Prevention
+
+- CI now runs `gitleaks` on PRs/pushes.
+- CI also fails if tracked files contain `sk-...` key-like patterns (excluding README examples).
+- Recommended local pre-commit:
+
+```bash
+pipx install pre-commit
+pre-commit install
+# or run directly
+gitleaks detect --source . --no-git
+```
+
+### Hardened Docker Run
+
+Use `Dockerfile.hardened` and `docker-compose.hardened.yml`:
+
+```bash
+docker build -f Dockerfile.hardened -t picoclaw:hardened .
+docker run --rm \
+  --user 1000:1000 \
+  --read-only \
+  --cap-drop ALL \
+  --security-opt no-new-privileges:true \
+  --tmpfs /tmp:rw,noexec,nosuid,size=64m \
+  -e OPENAI_API_KEY="$OPENAI_API_KEY" \
+  -v "$PWD/config/config.json:/home/picoclaw/.picoclaw/config.json:ro" \
+  -v picoclaw-workspace:/home/picoclaw/.picoclaw/workspace \
+  picoclaw:hardened gateway
+```
+
 #### Error Examples

 ```
@ -711,7 +768,7 @@ picoclaw agent -m "Hello"
  },
  "providers": {
    "openrouter": {
-      "api_key": "sk-or-v1-xxx"
+      "api_key": "${OPENROUTER_API_KEY}"
    },
    "groq": {
      "api_key": "gsk_xxx"
--- a/cmd/picoclaw/main.go
+++ b/cmd/picoclaw/main.go
@ -239,8 +239,8 @@ func onboard() {

 	fmt.Printf("%s picoclaw is ready!\n", logo)
 	fmt.Println("\nNext steps:")
-	fmt.Println("  1. Add your API key to", configPath)
-	fmt.Println("     Get one at: https://openrouter.ai/keys")
+	fmt.Println("  1. Export OPENAI_API_KEY (or configure provider key via env/secrets)")
+	fmt.Println("     Example: export OPENAI_API_KEY=...;")
 	fmt.Println("  2. Chat: picoclaw agent -m \"Hello!\"")
 }

@ -1300,7 +1300,7 @@ func skillsInstallBuiltinCmd(workspace string) {
 			continue
 		}

-		if err := os.MkdirAll(workspacePath, 0755); err != nil {
+		if err := os.MkdirAll(workspacePath, 0700); err != nil {
 			fmt.Printf("✗ Failed to create directory for %s: %v\n", skillName, err)
 			continue
 		}
--- a/config/config.example.json
+++ b/config/config.example.json
@ -23,7 +23,7 @@
    },
    "maixcam": {
      "enabled": false,
-      "host": "0.0.0.0",
+      "host": "127.0.0.1",
      "port": 18790,
      "allow_from": []
    },
@ -56,7 +56,7 @@
      "enabled": false,
      "channel_secret": "YOUR_LINE_CHANNEL_SECRET",
      "channel_access_token": "YOUR_LINE_CHANNEL_ACCESS_TOKEN",
-      "webhook_host": "0.0.0.0",
+      "webhook_host": "127.0.0.1",
      "webhook_port": 18791,
      "webhook_path": "/webhook/line",
      "allow_from": []
@ -80,11 +80,11 @@
      "api_base": ""
    },
    "openrouter": {
-      "api_key": "sk-or-v1-xxx",
+      "api_key": "${OPENROUTER_API_KEY}",
      "api_base": ""
    },
    "groq": {
-      "api_key": "gsk_xxx",
+      "api_key": "${GROQ_API_KEY}",
      "api_base": ""
    },
    "zhipu": {
@ -100,12 +100,12 @@
      "api_base": ""
    },
    "nvidia": {
-      "api_key": "nvapi-xxx",
+      "api_key": "${NVIDIA_API_KEY}",
      "api_base": "",
      "proxy": "http://127.0.0.1:7890"
    },
    "moonshot": {
-      "api_key": "sk-xxx",
+      "api_key": "${MOONSHOT_API_KEY}",
      "api_base": ""
    }
  },
@ -126,7 +126,7 @@
    "monitor_usb": true
  },
  "gateway": {
-    "host": "0.0.0.0",
+    "host": "127.0.0.1",
    "port": 18790
  }
 }
--- a/docker-compose.hardened.yml
+++ b/docker-compose.hardened.yml
@ -0,0 +1,28 @@
+services:
+  picoclaw-gateway:
+    build:
+      context: .
+      dockerfile: Dockerfile.hardened
+    read_only: true
+    cap_drop:
+      - ALL
+    security_opt:
+      - no-new-privileges:true
+    tmpfs:
+      - /tmp:rw,noexec,nosuid,size=64m
+    environment:
+      - OPENAI_API_KEY_FILE=/run/secrets/openai_api_key
+      - OPENAI_MODEL=gpt-4o-mini
+    secrets:
+      - openai_api_key
+    volumes:
+      - picoclaw-workspace:/home/picoclaw/.picoclaw/workspace
+      - ./config/config.json:/home/picoclaw/.picoclaw/config.json:ro
+    command: ["gateway"]
+
+secrets:
+  openai_api_key:
+    file: .secrets/openai_api_key
+
+volumes:
+  picoclaw-workspace:
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@ -104,7 +104,7 @@ func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msg

 func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop {
 	workspace := cfg.WorkspacePath()
-	os.MkdirAll(workspace, 0755)
+	os.MkdirAll(workspace, 0700)

 	restrict := cfg.Agents.Defaults.RestrictToWorkspace

--- a/pkg/agent/memory.go
+++ b/pkg/agent/memory.go
@ -11,6 +11,8 @@ import (
 	"os"
 	"path/filepath"
 	"time"
+
+	"github.com/sipeed/picoclaw/pkg/utils"
 )

 // MemoryStore manages persistent memory for the agent.
@ -29,7 +31,7 @@ func NewMemoryStore(workspace string) *MemoryStore {
 	memoryFile := filepath.Join(memoryDir, "MEMORY.md")

 	// Ensure memory directory exists
-	os.MkdirAll(memoryDir, 0755)
+	os.MkdirAll(memoryDir, 0700)

 	return &MemoryStore{
 		workspace:  workspace,
@ -57,7 +59,7 @@ func (ms *MemoryStore) ReadLongTerm() string {

 // WriteLongTerm writes content to the long-term memory file (MEMORY.md).
 func (ms *MemoryStore) WriteLongTerm(content string) error {
-	return os.WriteFile(ms.memoryFile, []byte(content), 0644)
+	return utils.WriteFileAtomic(ms.memoryFile, []byte(content), 0600, 0700)
 }

 // ReadToday reads today's daily note.
@ -77,7 +79,7 @@ func (ms *MemoryStore) AppendToday(content string) error {

 	// Ensure month directory exists
 	monthDir := filepath.Dir(todayFile)
-	os.MkdirAll(monthDir, 0755)
+	os.MkdirAll(monthDir, 0700)

 	var existingContent string
 	if data, err := os.ReadFile(todayFile); err == nil {
@ -94,7 +96,7 @@ func (ms *MemoryStore) AppendToday(content string) error {
 		newContent = existingContent + "\n" + content
 	}

-	return os.WriteFile(todayFile, []byte(newContent), 0644)
+	return utils.WriteFileAtomic(todayFile, []byte(newContent), 0600, 0700)
 }

 // GetRecentDailyNotes returns daily notes from the last N days.
--- a/pkg/auth/store.go
+++ b/pkg/auth/store.go
@ -5,6 +5,8 @@ import (
 	"os"
 	"path/filepath"
 	"time"
+
+	"github.com/sipeed/picoclaw/pkg/utils"
 )

 type AuthCredential struct {
@ -62,7 +64,7 @@ func LoadStore() (*AuthStore, error) {
 func SaveStore(store *AuthStore) error {
 	path := authFilePath()
 	dir := filepath.Dir(path)
-	if err := os.MkdirAll(dir, 0755); err != nil {
+	if err := os.MkdirAll(dir, 0700); err != nil {
 		return err
 	}

@ -70,7 +72,7 @@ func SaveStore(store *AuthStore) error {
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(path, data, 0600)
+	return utils.WriteFileAtomic(path, data, 0600, 0700)
 }

 func GetCredential(provider string) (*AuthCredential, error) {
--- a/pkg/channels/base.go
+++ b/pkg/channels/base.go
@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"sync"
+	"time"

 	"github.com/sipeed/picoclaw/pkg/bus"
 )
@ -23,6 +25,8 @@ type BaseChannel struct {
 	running   bool
 	name      string
 	allowList []string
+	rateMu    sync.Mutex
+	rateMap   map[string]time.Time
 }

 func NewBaseChannel(name string, config interface{}, bus *bus.MessageBus, allowList []string) *BaseChannel {
@ -32,6 +36,7 @@ func NewBaseChannel(name string, config interface{}, bus *bus.MessageBus, allowL
 		name:      name,
 		allowList: allowList,
 		running:   false,
+		rateMap:   make(map[string]time.Time),
 	}
 }

@ -45,7 +50,7 @@ func (c *BaseChannel) IsRunning() bool {

 func (c *BaseChannel) IsAllowed(senderID string) bool {
 	if len(c.allowList) == 0 {
-		return true
+		return false
 	}

 	// Extract parts from compound senderID like "123456|username"
@ -86,6 +91,9 @@ func (c *BaseChannel) HandleMessage(senderID, chatID, content string, media []st
 	if !c.IsAllowed(senderID) {
 		return
 	}
+	if !c.allowByRate(senderID) {
+		return
+	}

 	// Build session key: channel:chatID
 	sessionKey := fmt.Sprintf("%s:%s", c.name, chatID)
@ -103,6 +111,18 @@ func (c *BaseChannel) HandleMessage(senderID, chatID, content string, media []st
 	c.bus.PublishInbound(msg)
 }

+func (c *BaseChannel) allowByRate(senderID string) bool {
+	c.rateMu.Lock()
+	defer c.rateMu.Unlock()
+	last, ok := c.rateMap[senderID]
+	now := time.Now()
+	if ok && now.Sub(last) < 250*time.Millisecond {
+		return false
+	}
+	c.rateMap[senderID] = now
+	return true
+}
+
 func (c *BaseChannel) setRunning(running bool) {
 	c.running = running
 }
--- a/pkg/channels/base_test.go
+++ b/pkg/channels/base_test.go
@ -10,10 +10,10 @@ func TestBaseChannelIsAllowed(t *testing.T) {
 		want      bool
 	}{
 		{
-			name:      "empty allowlist allows all",
+			name:      "empty allowlist denies by default",
 			allowList: nil,
 			senderID:  "anyone",
-			want:      true,
+			want:      false,
 		},
 		{
 			name:      "compound sender matches numeric allowlist",
--- a/pkg/channels/slack_test.go
+++ b/pkg/channels/slack_test.go
@ -145,15 +145,15 @@ func TestNewSlackChannel(t *testing.T) {
 func TestSlackChannelIsAllowed(t *testing.T) {
 	msgBus := bus.NewMessageBus()

-	t.Run("empty allowlist allows all", func(t *testing.T) {
+	t.Run("empty allowlist denies all", func(t *testing.T) {
 		cfg := config.SlackConfig{
 			BotToken:  "xoxb-test",
 			AppToken:  "xapp-test",
 			AllowFrom: []string{},
 		}
 		ch, _ := NewSlackChannel(cfg, msgBus)
-		if !ch.IsAllowed("U_ANYONE") {
-			t.Error("empty allowlist should allow all users")
+		if ch.IsAllowed("U_ANYONE") {
+			t.Error("empty allowlist should deny by default")
 		}
 	})

--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@ -5,9 +5,11 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"

 	"github.com/caarlos0/env/v11"
+	"github.com/sipeed/picoclaw/pkg/utils"
 )

 // FlexibleStringSlice is a []string that also accepts JSON numbers,
@ -253,7 +255,7 @@ func DefaultConfig() *Config {
 			},
 			MaixCam: MaixCamConfig{
 				Enabled:   false,
-				Host:      "0.0.0.0",
+				Host:      "127.0.0.1",
 				Port:      18790,
 				AllowFrom: FlexibleStringSlice{},
 			},
@ -279,7 +281,7 @@ func DefaultConfig() *Config {
 				Enabled:            false,
 				ChannelSecret:      "",
 				ChannelAccessToken: "",
-				WebhookHost:        "0.0.0.0",
+				WebhookHost:        "127.0.0.1",
 				WebhookPort:        18791,
 				WebhookPath:        "/webhook/line",
 				AllowFrom:          FlexibleStringSlice{},
@ -306,7 +308,7 @@ func DefaultConfig() *Config {
 			ShengSuanYun: ProviderConfig{},
 		},
 		Gateway: GatewayConfig{
-			Host: "0.0.0.0",
+			Host: "127.0.0.1",
 			Port: 18790,
 		},
 		Tools: ToolsConfig{
@ -335,6 +337,7 @@ func DefaultConfig() *Config {

 func LoadConfig(path string) (*Config, error) {
 	cfg := DefaultConfig()
+	_ = loadLocalEnvSecrets(filepath.Join(".secrets", "local.env"))

 	data, err := os.ReadFile(path)
 	if err != nil {
@ -352,6 +355,18 @@ func LoadConfig(path string) (*Config, error) {
 		return nil, err
 	}

+	if cfg.Providers.OpenAI.APIKey == "" {
+		if keyFile := strings.TrimSpace(os.Getenv("OPENAI_API_KEY_FILE")); keyFile != "" {
+			if data, err := os.ReadFile(keyFile); err == nil {
+				_ = os.Setenv("OPENAI_API_KEY", strings.TrimSpace(string(data)))
+			}
+		}
+		cfg.Providers.OpenAI.APIKey = strings.TrimSpace(os.Getenv("OPENAI_API_KEY"))
+	}
+	if cfg.Agents.Defaults.Provider == "openai" && os.Getenv("OPENAI_MODEL") != "" {
+		cfg.Agents.Defaults.Model = strings.TrimSpace(os.Getenv("OPENAI_MODEL"))
+	}
+
 	return cfg, nil
 }

@ -365,11 +380,41 @@ func SaveConfig(path string, cfg *Config) error {
 	}

 	dir := filepath.Dir(path)
-	if err := os.MkdirAll(dir, 0755); err != nil {
+	if err := os.MkdirAll(dir, 0700); err != nil {
 		return err
 	}

-	return os.WriteFile(path, data, 0644)
+	return utils.WriteFileAtomic(path, data, 0600, 0700)
+}
+
+func loadLocalEnvSecrets(path string) error {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return err
+	}
+
+	for _, line := range strings.Split(string(data), "\n") {
+		line = strings.TrimSpace(line)
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+		parts := strings.SplitN(line, "=", 2)
+		if len(parts) != 2 {
+			continue
+		}
+		k := strings.TrimSpace(parts[0])
+		v := strings.Trim(strings.TrimSpace(parts[1]), "\"'")
+		if k != "OPENAI_API_KEY" && k != "OPENAI_MODEL" {
+			continue
+		}
+		if os.Getenv(k) == "" {
+			_ = os.Setenv(k, v)
+		}
+	}
+	return nil
 }

 func (c *Config) WorkspacePath() string {
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@ -1,6 +1,8 @@
 package config

 import (
+	"os"
+	"path/filepath"
 	"testing"
 )

@ -64,7 +66,7 @@ func TestDefaultConfig_Temperature(t *testing.T) {
 func TestDefaultConfig_Gateway(t *testing.T) {
 	cfg := DefaultConfig()

-	if cfg.Gateway.Host != "0.0.0.0" {
+	if cfg.Gateway.Host != "127.0.0.1" {
 		t.Error("Gateway host should have default value")
 	}
 	if cfg.Gateway.Port == 0 {
@ -167,7 +169,7 @@ func TestConfig_Complete(t *testing.T) {
 	if cfg.Agents.Defaults.MaxToolIterations == 0 {
 		t.Error("MaxToolIterations should not be zero")
 	}
-	if cfg.Gateway.Host != "0.0.0.0" {
+	if cfg.Gateway.Host != "127.0.0.1" {
 		t.Error("Gateway host should have default value")
 	}
 	if cfg.Gateway.Port == 0 {
@ -177,3 +179,18 @@ func TestConfig_Complete(t *testing.T) {
 		t.Error("Heartbeat should be enabled by default")
 	}
 }
+
+func TestSaveConfigPermissions(t *testing.T) {
+	cfg := DefaultConfig()
+	path := filepath.Join(t.TempDir(), "config.json")
+	if err := SaveConfig(path, cfg); err != nil {
+		t.Fatalf("save config: %v", err)
+	}
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat config: %v", err)
+	}
+	if info.Mode().Perm() != 0600 {
+		t.Fatalf("expected 0600, got %o", info.Mode().Perm())
+	}
+}
--- 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 {
@ -331,7 +332,7 @@ func (cs *CronService) loadStore() error {

 func (cs *CronService) saveStoreUnsafe() error {
 	dir := filepath.Dir(cs.storePath)
-	if err := os.MkdirAll(dir, 0755); err != nil {
+	if err := os.MkdirAll(dir, 0700); err != nil {
 		return err
 	}

@ -340,7 +341,7 @@ func (cs *CronService) saveStoreUnsafe() error {
 		return err
 	}

-	return os.WriteFile(cs.storePath, data, 0644)
+	return utils.WriteFileAtomic(cs.storePath, data, 0600, 0700)
 }

 func (cs *CronService) AddJob(name string, schedule CronSchedule, message string, deliver bool, channel, to string) (*CronJob, error) {
--- a/pkg/cron/service_test.go
+++ b/pkg/cron/service_test.go
@ -0,0 +1,25 @@
+package cron
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestCronStorePermissions(t *testing.T) {
+	store := filepath.Join(t.TempDir(), "cron", "jobs.json")
+	cs := NewCronService(store, nil)
+	_, err := cs.AddJob("test", CronSchedule{Kind: "every", EveryMS: int64Ptr(1000)}, "hello", true, "cli", "direct")
+	if err != nil {
+		t.Fatalf("add job: %v", err)
+	}
+	info, err := os.Stat(store)
+	if err != nil {
+		t.Fatalf("stat: %v", err)
+	}
+	if info.Mode().Perm() != 0600 {
+		t.Fatalf("expected 0600, got %o", info.Mode().Perm())
+	}
+}
+
+func int64Ptr(v int64) *int64 { return &v }
--- a/pkg/providers/http_provider.go
+++ b/pkg/providers/http_provider.go
@ -12,6 +12,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"math"
 	"net/http"
 	"net/url"
 	"strings"
@ -25,6 +26,7 @@ type HTTPProvider struct {
 	apiKey     string
 	apiBase    string
 	httpClient *http.Client
+	maxRetries int
 }

 func NewHTTPProvider(apiKey, apiBase, proxy string) *HTTPProvider {
@ -45,6 +47,7 @@ func NewHTTPProvider(apiKey, apiBase, proxy string) *HTTPProvider {
 		apiKey:     apiKey,
 		apiBase:    strings.TrimRight(apiBase, "/"),
 		httpClient: client,
+		maxRetries: 3,
 	}
 }

@ -94,28 +97,41 @@ func (p *HTTPProvider) Chat(ctx context.Context, messages []Message, tools []Too
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal request: %w", err)
 	}
-
-	req, err := http.NewRequestWithContext(ctx, "POST", p.apiBase+"/chat/completions", bytes.NewReader(jsonData))
-	if err != nil {
-		return nil, fmt.Errorf("failed to create request: %w", err)
+	if len(jsonData) > 1_000_000 {
+		return nil, fmt.Errorf("request payload too large")
 	}

+	var resp *http.Response
+	for attempt := 0; attempt <= p.maxRetries; attempt++ {
+		req, reqErr := http.NewRequestWithContext(ctx, "POST", p.apiBase+"/chat/completions", bytes.NewReader(jsonData))
+		if reqErr != nil {
+			return nil, fmt.Errorf("failed to create request: %w", reqErr)
+		}
 		req.Header.Set("Content-Type", "application/json")
 		if p.apiKey != "" {
 			req.Header.Set("Authorization", "Bearer "+p.apiKey)
 		}

-	resp, err := p.httpClient.Do(req)
-	if err != nil {
+		resp, err = p.httpClient.Do(req)
+		if err == nil {
+			break
+		}
+		if attempt == p.maxRetries {
 			return nil, fmt.Errorf("failed to send request: %w", err)
 		}
+		time.Sleep(time.Duration(math.Pow(2, float64(attempt))) * 200 * time.Millisecond)
+	}
 	defer resp.Body.Close()

-	body, err := io.ReadAll(resp.Body)
+	body, err := io.ReadAll(io.LimitReader(resp.Body, 4_000_000))
 	if err != nil {
 		return nil, fmt.Errorf("failed to read response: %w", err)
 	}

+	if resp.StatusCode >= 500 && resp.StatusCode <= 599 {
+		return nil, fmt.Errorf("upstream temporary failure: status %d", resp.StatusCode)
+	}
+
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("API request failed:\n  Status: %d\n  Body:   %s", resp.StatusCode, string(body))
 	}
--- a/pkg/session/manager.go
+++ b/pkg/session/manager.go
@ -9,6 +9,7 @@ import (
 	"time"

 	"github.com/sipeed/picoclaw/pkg/providers"
+	"github.com/sipeed/picoclaw/pkg/utils"
 )

 type Session struct {
@ -32,7 +33,7 @@ func NewSessionManager(storage string) *SessionManager {
 	}

 	if storage != "" {
-		os.MkdirAll(storage, 0755)
+		os.MkdirAll(storage, 0700)
 		sm.loadSessions()
 	}

@ -197,40 +198,7 @@ func (sm *SessionManager) Save(key string) error {
 	}

 	sessionPath := filepath.Join(sm.storage, filename+".json")
-	tmpFile, err := os.CreateTemp(sm.storage, "session-*.tmp")
-	if err != nil {
-		return err
-	}
-
-	tmpPath := tmpFile.Name()
-	cleanup := true
-	defer func() {
-		if cleanup {
-			_ = os.Remove(tmpPath)
-		}
-	}()
-
-	if _, err := tmpFile.Write(data); err != nil {
-		_ = tmpFile.Close()
-		return err
-	}
-	if err := tmpFile.Chmod(0644); err != nil {
-		_ = tmpFile.Close()
-		return err
-	}
-	if err := tmpFile.Sync(); err != nil {
-		_ = tmpFile.Close()
-		return err
-	}
-	if err := tmpFile.Close(); err != nil {
-		return err
-	}
-
-	if err := os.Rename(tmpPath, sessionPath); err != nil {
-		return err
-	}
-	cleanup = false
-	return nil
+	return utils.WriteFileAtomic(sessionPath, data, 0600, 0700)
 }

 func (sm *SessionManager) loadSessions() error {
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"sync"
 	"time"
+
+	"github.com/sipeed/picoclaw/pkg/utils"
 )

 // State represents the persistent state for a workspace.
@ -38,7 +40,7 @@ func NewManager(workspace string) *Manager {
 	oldStateFile := filepath.Join(workspace, "state.json")

 	// Create state directory if it doesn't exist
-	os.MkdirAll(stateDir, 0755)
+	os.MkdirAll(stateDir, 0700)

 	sm := &Manager{
 		workspace: workspace,
@ -129,25 +131,14 @@ func (sm *Manager) GetTimestamp() time.Time {
 //
 // Must be called with the lock held.
 func (sm *Manager) saveAtomic() error {
-	// Create temp file in the same directory as the target
-	tempFile := sm.stateFile + ".tmp"
-
 	// Marshal state to JSON
 	data, err := json.MarshalIndent(sm.state, "", "  ")
 	if err != nil {
 		return fmt.Errorf("failed to marshal state: %w", err)
 	}

-	// Write to temp file
-	if err := os.WriteFile(tempFile, data, 0644); err != nil {
-		return fmt.Errorf("failed to write temp file: %w", err)
-	}
-
-	// Atomic rename from temp to target
-	if err := os.Rename(tempFile, sm.stateFile); err != nil {
-		// Cleanup temp file if rename fails
-		os.Remove(tempFile)
-		return fmt.Errorf("failed to rename temp file: %w", err)
+	if err := utils.WriteFileAtomic(sm.stateFile, data, 0600, 0700); err != nil {
+		return fmt.Errorf("failed to write state file atomically: %w", err)
 	}

 	return nil
--- a/pkg/state/state_test.go
+++ b/pkg/state/state_test.go
@ -40,6 +40,11 @@ func TestAtomicSave(t *testing.T) {
 	if _, err := os.Stat(stateFile); os.IsNotExist(err) {
 		t.Error("Expected state file to exist")
 	}
+	if info, err := os.Stat(stateFile); err == nil {
+		if info.Mode().Perm() != 0600 {
+			t.Fatalf("expected state file mode 0600, got %o", info.Mode().Perm())
+		}
+	}

 	// Create a new manager to verify persistence
 	sm2 := NewManager(tmpDir)
--- a/pkg/tools/cron.go
+++ b/pkg/tools/cron.go
@ -33,7 +33,7 @@ func NewCronTool(cronService *cron.CronService, executor JobExecutor, msgBus *bu
 		cronService: cronService,
 		executor:    executor,
 		msgBus:      msgBus,
-		execTool:    NewExecTool(workspace, false),
+		execTool:    NewExecTool(workspace, true),
 	}
 }

--- a/pkg/tools/cron_test.go
+++ b/pkg/tools/cron_test.go
@ -0,0 +1,30 @@
+package tools
+
+import (
+	"context"
+	"path/filepath"
+	"testing"
+
+	"github.com/sipeed/picoclaw/pkg/bus"
+	"github.com/sipeed/picoclaw/pkg/cron"
+)
+
+func TestCronTool_ExecuteJobCommandRespectsWorkspaceRestriction(t *testing.T) {
+	msgBus := bus.NewMessageBus()
+	service := cron.NewCronService(filepath.Join(t.TempDir(), "jobs.json"), nil)
+	tool := NewCronTool(service, nil, msgBus, t.TempDir())
+
+	job := &cron.CronJob{}
+	job.Payload.Command = "cat /etc/passwd"
+	job.Payload.Channel = "cli"
+	job.Payload.To = "direct"
+
+	tool.ExecuteJob(context.Background(), job)
+	outbound, ok := msgBus.SubscribeOutbound(context.Background())
+	if !ok {
+		t.Fatalf("expected outbound message")
+	}
+	if outbound.Content == "" || outbound.Content == "ok" {
+		t.Fatalf("expected error output from blocked command")
+	}
+}
--- a/pkg/tools/shell.go
+++ b/pkg/tools/shell.go
@ -31,6 +31,7 @@ func NewExecTool(workingDir string, restrict bool) *ExecTool {
 		regexp.MustCompile(`>\s*/dev/sd[a-z]\b`), // Block writes to disk devices (but allow /dev/null)
 		regexp.MustCompile(`\b(shutdown|reboot|poweroff)\b`),
 		regexp.MustCompile(`:\(\)\s*\{.*\};\s*:`),
+		regexp.MustCompile(`&&|\|\||;|` + "`" + `|\$\(|\||>|<`),
 	}

 	return &ExecTool{
--- a/pkg/tools/shell_test.go
+++ b/pkg/tools/shell_test.go
@ -133,6 +133,16 @@ func TestShellTool_DangerousCommand(t *testing.T) {
 	}
 }

+func TestShellTool_BlocksMetaCharacters(t *testing.T) {
+	tool := NewExecTool("", false)
+	ctx := context.Background()
+
+	result := tool.Execute(ctx, map[string]interface{}{"command": "echo ok && whoami"})
+	if !result.IsError {
+		t.Fatalf("expected meta-character command to be blocked")
+	}
+}
+
 // TestShellTool_MissingCommand verifies error handling for missing command
 func TestShellTool_MissingCommand(t *testing.T) {
 	tool := NewExecTool("", false)
@ -154,16 +164,13 @@ func TestShellTool_StderrCapture(t *testing.T) {

 	ctx := context.Background()
 	args := map[string]interface{}{
-		"command": "sh -c 'echo stdout; echo stderr >&2'",
+		"command": "ls /nonexistent_directory_for_stderr_test",
 	}

 	result := tool.Execute(ctx, args)

 	// Both stdout and stderr should be in output
-	if !strings.Contains(result.ForLLM, "stdout") {
-		t.Errorf("Expected stdout in output, got: %s", result.ForLLM)
-	}
-	if !strings.Contains(result.ForLLM, "stderr") {
+	if !strings.Contains(result.ForLLM, "STDERR") {
 		t.Errorf("Expected stderr in output, got: %s", result.ForLLM)
 	}
 }
@ -208,3 +215,18 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) {
 		t.Errorf("Expected 'blocked' message for path traversal, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser)
 	}
 }
+
+func TestShellTool_RestrictToWorkspaceAbsolutePath(t *testing.T) {
+	tmpDir := t.TempDir()
+	tool := NewExecTool(tmpDir, true)
+
+	ctx := context.Background()
+	args := map[string]interface{}{
+		"command": "cat /etc/hosts",
+	}
+
+	result := tool.Execute(ctx, args)
+	if !result.IsError {
+		t.Fatalf("expected absolute path outside workspace to be blocked")
+	}
+}
--- a/pkg/utils/securefile.go
+++ b/pkg/utils/securefile.go
@ -0,0 +1,49 @@
+package utils
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+// WriteFileAtomic writes data using temp file + rename and applies strict permissions.
+func WriteFileAtomic(path string, data []byte, filePerm, dirPerm os.FileMode) error {
+	dir := filepath.Dir(path)
+	if err := os.MkdirAll(dir, dirPerm); err != nil {
+		return err
+	}
+	_ = os.Chmod(dir, dirPerm)
+
+	tmp, err := os.CreateTemp(dir, filepath.Base(path)+".*.tmp")
+	if err != nil {
+		return err
+	}
+	tmpPath := tmp.Name()
+	defer os.Remove(tmpPath)
+
+	if err := tmp.Chmod(filePerm); err != nil {
+		_ = tmp.Close()
+		return err
+	}
+
+	if _, err := tmp.Write(data); err != nil {
+		_ = tmp.Close()
+		return err
+	}
+
+	if err := tmp.Sync(); err != nil {
+		_ = tmp.Close()
+		return err
+	}
+
+	if err := tmp.Close(); err != nil {
+		return err
+	}
+
+	if err := os.Rename(tmpPath, path); err != nil {
+		return fmt.Errorf("rename temp file: %w", err)
+	}
+
+	_ = os.Chmod(path, filePerm)
+	return nil
+}