From b320c34eb5e2540ae11a45dbe054543febbc85bb Mon Sep 17 00:00:00 2001 From: Subash Date: Mon, 16 Mar 2026 07:41:49 +0530 Subject: [PATCH] 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 --- pkg/config/defaults.go | 2 +- pkg/memory/jsonl.go | 8 +-- pkg/memory/jsonl_permissions_test.go | 99 ++++++++++++++++++++++++++++ pkg/tools/shell_test.go | 29 ++++++++ 4 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 pkg/memory/jsonl_permissions_test.go diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 189af0a84..b01042b29 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -449,7 +449,7 @@ func DefaultConfig() *Config { Enabled: true, }, EnableDenyPatterns: true, - AllowRemote: true, + AllowRemote: false, TimeoutSeconds: 60, }, Skills: SkillsToolsConfig{ diff --git a/pkg/memory/jsonl.go b/pkg/memory/jsonl.go index afe374166..eec70dcc3 100644 --- a/pkg/memory/jsonl.go +++ b/pkg/memory/jsonl.go @@ -60,7 +60,7 @@ type JSONLStore struct { // NewJSONLStore creates a new JSONL-backed store rooted at dir. func NewJSONLStore(dir string) (*JSONLStore, error) { - err := os.MkdirAll(dir, 0o755) + err := os.MkdirAll(dir, 0o700) if err != nil { 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 { 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 @@ -230,7 +230,7 @@ func (s *JSONLStore) addMsg(sessionKey string, msg providers.Message) error { f, err := os.OpenFile( s.jsonlPath(sessionKey), os.O_CREATE|os.O_WRONLY|os.O_APPEND, - 0o644, + 0o600, ) if err != nil { return fmt.Errorf("memory: open jsonl for append: %w", err) @@ -452,7 +452,7 @@ func (s *JSONLStore) rewriteJSONL( buf.Write(line) 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 { diff --git a/pkg/memory/jsonl_permissions_test.go b/pkg/memory/jsonl_permissions_test.go new file mode 100644 index 000000000..24c95c4b2 --- /dev/null +++ b/pkg/memory/jsonl_permissions_test.go @@ -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) + } +} diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index c4553020f..4a1e3816d 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -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 func TestShellTool_RestrictToWorkspace(t *testing.T) { tmpDir := t.TempDir()