From 75cb3772d7f38d5939cdd501afe9dddaee13e26a Mon Sep 17 00:00:00 2001 From: mikrub <8495730+mikrub@users.noreply.github.com> Date: Tue, 21 Jul 2026 10:06:10 +0700 Subject: [PATCH] fix(store): don't silently create a second store; restrict store perms to 0700 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related problems with how the store directory is created. 1. --store defaults to a CWD-relative "./store" and the constructors create it unconditionally, so running any command from the wrong directory silently creates an empty store instead of reporting the mistake. Every later command run from there reads that empty store and reports zero messages, which is indistinguishable from data loss; an 'auth' against it leaves real session keys in a directory nobody is tracking. Only 'auth' may now bring a store into existence — everything else errors with the resolved path. 2. The store directories were created 0755, and SQLite creates the DBs 0644 under the usual umask. whatsapp.db holds the linked-device session keys (read access = account impersonation) and messages.db holds the plaintext message archive. Both are now 0700, along with the media output directory — media files themselves were already 0600. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/client/client.go | 6 ++- internal/commands/commands.go | 6 ++- internal/store/store.go | 5 ++- internal/store/store_test.go | 18 +++++++++ main.go | 28 ++++++++++++++ store_presence_test.go | 72 +++++++++++++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 store_presence_test.go diff --git a/internal/client/client.go b/internal/client/client.go index b60c54d..0c5af4a 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -56,8 +56,10 @@ type MessageDetails struct { func NewWAClient(storeDir string) (*WAClient, error) { - // Create store directory - if err := os.MkdirAll(storeDir, 0755); err != nil { + // 0700: this directory holds whatsapp.db, which contains the linked-device + // session keys. Anyone who can read it can impersonate the account on + // WhatsApp, so it must not be world-readable. + if err := os.MkdirAll(storeDir, 0700); err != nil { return nil, fmt.Errorf("failed to create store directory: %v", err) } diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 5b3c4d0..d35d516 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -420,7 +420,11 @@ func (a *App) downloadMediaAndPersist(ctx context.Context, info store.MessageDow if err != nil { return "", 0, time.Time{}, err } - if err := os.MkdirAll(filepath.Dir(finalPath), 0755); err != nil { + // 0700 to match the store: by default this resolves under store/media, and + // downloaded media is as private as the messages that carried it. The files + // themselves are already created 0600 by CreateTemp+Rename; without this the + // directory was the only part left world-readable. + if err := os.MkdirAll(filepath.Dir(finalPath), 0700); err != nil { return "", 0, time.Time{}, fmt.Errorf("failed to create destination directory: %w", err) } diff --git a/internal/store/store.go b/internal/store/store.go index bedee4a..575c47f 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -131,9 +131,10 @@ type ListChatsParams struct { } func NewMessageStore(dbPath string) (*MessageStore, error) { - // Create directory if it doesn't exist + // 0700: messages.db holds the full plaintext message archive — contents, + // contact names and phone numbers. Not world-readable. dir := filepath.Dir(dbPath) - if err := os.MkdirAll(dir, 0755); err != nil { + if err := os.MkdirAll(dir, 0700); err != nil { return nil, fmt.Errorf("failed to create directory: %v", err) } diff --git a/internal/store/store_test.go b/internal/store/store_test.go index e9ea68d..44d7f91 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -34,6 +34,24 @@ func TestNewMessageStore(t *testing.T) { assert.NoError(t, err) } +// messages.db holds the full plaintext archive — message bodies, contact names +// and phone numbers. The directory holding it must not be readable by other +// local users. +func TestNewMessageStoreCreatesPrivateDirectory(t *testing.T) { + tmpDir := t.TempDir() + storeDir := filepath.Join(tmpDir, "store") + dbPath := filepath.Join(storeDir, "messages.db") + + store, err := NewMessageStore(dbPath) + require.NoError(t, err) + defer store.Close() + + info, err := os.Stat(storeDir) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0700), info.Mode().Perm(), + "store directory must be owner-only; it holds the plaintext message archive") +} + func TestStoreChat(t *testing.T) { store := setupTestDB(t) diff --git a/main.go b/main.go index 718fd1d..1c53a49 100644 --- a/main.go +++ b/main.go @@ -82,6 +82,29 @@ func extractGlobalFlags(args []string) (string, []string) { return storeDir, remaining } +// checkStorePresence rejects commands that would otherwise silently create a +// second, empty store. +// +// --store defaults to "./store", resolved against the CURRENT WORKING +// DIRECTORY, and the store/client constructors create it unconditionally. So +// running any command from the wrong directory creates an empty store rather +// than reporting the mistake — and every later command run from there reads +// that empty store and reports zero messages, which is indistinguishable from +// data loss. An `auth` against such a directory is worse still: it leaves real +// session keys somewhere nobody is tracking. +// +// `auth` is exempt because first-time setup legitimately has nothing to point +// at yet. +func checkStorePresence(command, absStoreDir string) error { + if command == "auth" { + return nil + } + if _, err := os.Stat(absStoreDir); os.IsNotExist(err) { + return fmt.Errorf("store does not exist: %s (run 'auth' to create it, or pass --store)", absStoreDir) + } + return nil +} + func exitJSON(msg string) { fmt.Fprintf(os.Stderr, `{"success":false,"data":null,"error":"%s"}`+"\n", msg) os.Exit(1) @@ -130,6 +153,11 @@ func main() { fmt.Fprintf(os.Stderr, `{"success":false,"data":null,"error":"invalid store path: %v"}`+"\n", err) os.Exit(1) } + + if err := checkStorePresence(command, absStoreDir); err != nil { + exitJSON(err.Error()) + } + app, err := commands.NewApp(absStoreDir, version) if err != nil { fmt.Fprintf(os.Stderr, `{"success":false,"data":null,"error":"Failed to initialize: %v"} diff --git a/store_presence_test.go b/store_presence_test.go new file mode 100644 index 0000000..2876fad --- /dev/null +++ b/store_presence_test.go @@ -0,0 +1,72 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// A missing store directory must be reported, not created. `--store` defaults to +// a CWD-relative "./store", so without this guard running any command from the +// wrong directory produces an empty store that reads as zero messages — +// indistinguishable from data loss. +func TestCheckStorePresence(t *testing.T) { + existing := t.TempDir() + missing := filepath.Join(existing, "does-not-exist") + + tests := []struct { + name string + command string + storeDir string + wantError bool + }{ + { + name: "read command against a missing store is rejected", + command: "chats", + storeDir: missing, + wantError: true, + }, + { + name: "sync against a missing store is rejected", + command: "sync", + storeDir: missing, + wantError: true, + }, + { + name: "auth may create a store that does not exist yet", + command: "auth", + storeDir: missing, + wantError: false, + }, + { + name: "read command against an existing store is allowed", + command: "chats", + storeDir: existing, + wantError: false, + }, + { + name: "auth against an existing store is allowed", + command: "auth", + storeDir: existing, + wantError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkStorePresence(tt.command, tt.storeDir) + if tt.wantError { + require.Error(t, err) + require.Contains(t, err.Error(), "store does not exist") + } else { + require.NoError(t, err) + } + }) + } + + // The guard must not have created anything as a side effect. + _, err := os.Stat(missing) + require.True(t, os.IsNotExist(err), "checkStorePresence must not create the store directory") +}