fix: harden default exec remote access and session store permissions
Change exec.allow_remote default to false so fresh installations do not expose shell execution to remote channels (Telegram, Discord, etc.) unless explicitly opted-in. Tighten JSONL session store permissions from 0755/0644 to 0700/0600 so that chat histories containing tool outputs and user prompts are only readable by the process owner. Fixes #1525, fixes #1527
This commit is contained in:
parent
9ab1450ab5
commit
b320c34eb5
4 changed files with 133 additions and 5 deletions
|
|
@ -449,7 +449,7 @@ func DefaultConfig() *Config {
|
||||||
Enabled: true,
|
Enabled: true,
|
||||||
},
|
},
|
||||||
EnableDenyPatterns: true,
|
EnableDenyPatterns: true,
|
||||||
AllowRemote: true,
|
AllowRemote: false,
|
||||||
TimeoutSeconds: 60,
|
TimeoutSeconds: 60,
|
||||||
},
|
},
|
||||||
Skills: SkillsToolsConfig{
|
Skills: SkillsToolsConfig{
|
||||||
|
|
|
||||||
|
|
@ -60,7 +60,7 @@ type JSONLStore struct {
|
||||||
|
|
||||||
// NewJSONLStore creates a new JSONL-backed store rooted at dir.
|
// NewJSONLStore creates a new JSONL-backed store rooted at dir.
|
||||||
func NewJSONLStore(dir string) (*JSONLStore, error) {
|
func NewJSONLStore(dir string) (*JSONLStore, error) {
|
||||||
err := os.MkdirAll(dir, 0o755)
|
err := os.MkdirAll(dir, 0o700)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("memory: create directory: %w", err)
|
return nil, fmt.Errorf("memory: create directory: %w", err)
|
||||||
}
|
}
|
||||||
|
|
@ -121,7 +121,7 @@ func (s *JSONLStore) writeMeta(key string, meta sessionMeta) error {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("memory: encode meta: %w", err)
|
return fmt.Errorf("memory: encode meta: %w", err)
|
||||||
}
|
}
|
||||||
return fileutil.WriteFileAtomic(s.metaPath(key), data, 0o644)
|
return fileutil.WriteFileAtomic(s.metaPath(key), data, 0o600)
|
||||||
}
|
}
|
||||||
|
|
||||||
// readMessages reads valid JSON lines from a .jsonl file, skipping
|
// readMessages reads valid JSON lines from a .jsonl file, skipping
|
||||||
|
|
@ -230,7 +230,7 @@ func (s *JSONLStore) addMsg(sessionKey string, msg providers.Message) error {
|
||||||
f, err := os.OpenFile(
|
f, err := os.OpenFile(
|
||||||
s.jsonlPath(sessionKey),
|
s.jsonlPath(sessionKey),
|
||||||
os.O_CREATE|os.O_WRONLY|os.O_APPEND,
|
os.O_CREATE|os.O_WRONLY|os.O_APPEND,
|
||||||
0o644,
|
0o600,
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("memory: open jsonl for append: %w", err)
|
return fmt.Errorf("memory: open jsonl for append: %w", err)
|
||||||
|
|
@ -452,7 +452,7 @@ func (s *JSONLStore) rewriteJSONL(
|
||||||
buf.Write(line)
|
buf.Write(line)
|
||||||
buf.WriteByte('\n')
|
buf.WriteByte('\n')
|
||||||
}
|
}
|
||||||
return fileutil.WriteFileAtomic(s.jsonlPath(sessionKey), buf.Bytes(), 0o644)
|
return fileutil.WriteFileAtomic(s.jsonlPath(sessionKey), buf.Bytes(), 0o600)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *JSONLStore) Close() error {
|
func (s *JSONLStore) Close() error {
|
||||||
|
|
|
||||||
99
pkg/memory/jsonl_permissions_test.go
Normal file
99
pkg/memory/jsonl_permissions_test.go
Normal file
|
|
@ -0,0 +1,99 @@
|
||||||
|
//go:build !windows
|
||||||
|
|
||||||
|
package memory
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestJSONLStore_DirectoryPermissions verifies the session store directory
|
||||||
|
// is created with 0700 (owner-only) to protect private chat history.
|
||||||
|
func TestJSONLStore_DirectoryPermissions(t *testing.T) {
|
||||||
|
dir := filepath.Join(t.TempDir(), "sessions")
|
||||||
|
store, err := NewJSONLStore(dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewJSONLStore: %v", err)
|
||||||
|
}
|
||||||
|
defer store.Close()
|
||||||
|
|
||||||
|
info, err := os.Stat(dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Stat: %v", err)
|
||||||
|
}
|
||||||
|
if perm := info.Mode().Perm(); perm != 0o700 {
|
||||||
|
t.Errorf("directory permissions = %04o, want 0700", perm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestJSONLStore_FilePermissions verifies that session data files (.jsonl)
|
||||||
|
// and metadata files (.meta.json) are created with 0600 (owner-only).
|
||||||
|
func TestJSONLStore_FilePermissions(t *testing.T) {
|
||||||
|
store := newTestStore(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
err := store.AddMessage(ctx, "perms", "user", "secret conversation")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("AddMessage: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check .jsonl file permissions
|
||||||
|
jsonlPath := store.jsonlPath("perms")
|
||||||
|
info, err := os.Stat(jsonlPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Stat jsonl: %v", err)
|
||||||
|
}
|
||||||
|
if perm := info.Mode().Perm(); perm != 0o600 {
|
||||||
|
t.Errorf("jsonl permissions = %04o, want 0600", perm)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check .meta.json file permissions
|
||||||
|
metaPath := store.metaPath("perms")
|
||||||
|
info, err = os.Stat(metaPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Stat meta: %v", err)
|
||||||
|
}
|
||||||
|
if perm := info.Mode().Perm(); perm != 0o600 {
|
||||||
|
t.Errorf("meta permissions = %04o, want 0600", perm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestJSONLStore_RewritePreservesPermissions verifies that SetHistory
|
||||||
|
// (which rewrites the JSONL file) maintains restrictive permissions.
|
||||||
|
func TestJSONLStore_RewritePreservesPermissions(t *testing.T) {
|
||||||
|
store := newTestStore(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Write initial data
|
||||||
|
err := store.AddMessage(ctx, "rewrite", "user", "old message")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("AddMessage: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Rewrite via SetHistory
|
||||||
|
err = store.SetHistory(ctx, "rewrite", nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("SetHistory: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify the rewritten files still have restrictive permissions
|
||||||
|
jsonlPath := store.jsonlPath("rewrite")
|
||||||
|
info, err := os.Stat(jsonlPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Stat jsonl after rewrite: %v", err)
|
||||||
|
}
|
||||||
|
if perm := info.Mode().Perm(); perm != 0o600 {
|
||||||
|
t.Errorf("jsonl permissions after rewrite = %04o, want 0600", perm)
|
||||||
|
}
|
||||||
|
|
||||||
|
metaPath := store.metaPath("rewrite")
|
||||||
|
info, err = os.Stat(metaPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Stat meta after rewrite: %v", err)
|
||||||
|
}
|
||||||
|
if perm := info.Mode().Perm(); perm != 0o600 {
|
||||||
|
t.Errorf("meta permissions after rewrite = %04o, want 0600", perm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -380,6 +380,35 @@ func TestShellTool_AllowRemoteBypassesChannelCheck(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestShellTool_DefaultConfigBlocksRemote verifies that DefaultConfig() blocks remote exec
|
||||||
|
func TestShellTool_DefaultConfigBlocksRemote(t *testing.T) {
|
||||||
|
cfg := config.DefaultConfig()
|
||||||
|
|
||||||
|
tool, err := NewExecToolWithConfig("", false, cfg)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewExecToolWithConfig() error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Remote channel should be blocked with default config
|
||||||
|
ctx := WithToolContext(context.Background(), "telegram", "chat-1")
|
||||||
|
result := tool.Execute(ctx, map[string]any{"command": "echo hi"})
|
||||||
|
|
||||||
|
if !result.IsError {
|
||||||
|
t.Fatal("expected default config to block remote exec")
|
||||||
|
}
|
||||||
|
if !strings.Contains(result.ForLLM, "restricted to internal channels") {
|
||||||
|
t.Errorf("expected restriction message, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Internal channel should still work
|
||||||
|
ctx = WithToolContext(context.Background(), "cli", "direct")
|
||||||
|
result = tool.Execute(ctx, map[string]any{"command": "echo hi"})
|
||||||
|
|
||||||
|
if result.IsError {
|
||||||
|
t.Fatalf("expected default config to allow internal exec, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestShellTool_RestrictToWorkspace verifies workspace restriction
|
// TestShellTool_RestrictToWorkspace verifies workspace restriction
|
||||||
func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
||||||
tmpDir := t.TempDir()
|
tmpDir := t.TempDir()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue