From 585de4c71b2e4e0d9608f918c51f3421c339932b Mon Sep 17 00:00:00 2001 From: wydrox <79707825+wydrox@users.noreply.github.com> Date: Mon, 20 Apr 2026 18:50:50 +0200 Subject: [PATCH] Enforce 0600 permissions on secret files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session files and config.json stored under ~/.martmart-cli/ contain bearer tokens, refresh tokens, and cookies that effectively grant API access. Ensure they are never world- or group-readable: - Call os.Chmod(path, 0600) after os.WriteFile in config.Save and session.SaveProvider, since os.WriteFile only honours the mode argument when it creates a new file — pre-existing files keep their old mode and need an explicit chmod to be narrowed. - Add TestSave_EnforcesFileMode0600 and TestSaveProvider_EnforcesFileMode0600 covering the fresh-write and overwrite paths for both Frisco and Delio session files plus the shared config file. - Document the 0600 guarantee in SECURITY.md. --- SECURITY.md | 1 + internal/config/config.go | 9 ++++++- internal/config/config_test.go | 34 ++++++++++++++++++++++++ internal/session/session.go | 10 +++++++- internal/session/session_test.go | 44 ++++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 2 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 4beaf2d..4281a32 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -25,4 +25,5 @@ I will acknowledge the report as soon as possible and aim to provide an initial - Credentials/tokens are stored locally in `~/.martmart-cli/frisco-session.json` for Frisco and `~/.martmart-cli/delio-session.json` for Delio (with legacy read fallback from older `session.json` locations). - Access to that file effectively grants API access in the user context. +- Files in `~/.martmart-cli/` (session files and `config.json`) are written with `0600` permissions on every save so only the owning user can read them; any pre-existing file with wider permissions is narrowed back to `0600`. - `martmart mcp` uses the same local session; run it only in trusted local environments. diff --git a/internal/config/config.go b/internal/config/config.go index dc2a164..1fba05d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -122,6 +122,10 @@ func Normalize(cfg *Config) (*Config, error) { } // Save persists the config file with 0600 permissions. +// +// os.WriteFile only applies the mode when creating a new file, so we also +// call os.Chmod afterwards to narrow permissions on pre-existing files that +// may have been written with wider modes by older versions. func Save(cfg *Config) error { norm, err := Normalize(cfg) if err != nil { @@ -134,5 +138,8 @@ func Save(cfg *Config) error { if err != nil { return err } - return os.WriteFile(configFile, data, 0o600) + if err := os.WriteFile(configFile, data, 0o600); err != nil { + return err + } + return os.Chmod(configFile, 0o600) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9973d6c..355f1b7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -62,3 +62,37 @@ func TestSave_WritesCurrentConfigPath(t *testing.T) { t.Fatalf("legacy config should not be written, stat err=%v", err) } } + +func TestSave_EnforcesFileMode0600(t *testing.T) { + base := t.TempDir() + current := filepath.Join(base, "martmart-cli", "config.json") + legacy := filepath.Join(base, "frisco-cli", "config.json") + setTempConfigPaths(t, current, legacy) + + // Fresh write creates the file with 0600. + if err := Save(&Config{DefaultProvider: "frisco", RateLimitRPS: 1, RateLimitBurst: 1}); err != nil { + t.Fatalf("Save (fresh): %v", err) + } + fi, err := os.Stat(current) + if err != nil { + t.Fatalf("stat after fresh Save: %v", err) + } + if got := fi.Mode().Perm(); got != 0o600 { + t.Errorf("fresh file mode: got %o, want 600", got) + } + + // Pre-existing file with wider permissions must be narrowed back to 0600. + if err := os.Chmod(current, 0o644); err != nil { + t.Fatalf("Chmod 0644: %v", err) + } + if err := Save(&Config{DefaultProvider: "frisco", RateLimitRPS: 2, RateLimitBurst: 2}); err != nil { + t.Fatalf("Save (overwrite): %v", err) + } + fi, err = os.Stat(current) + if err != nil { + t.Fatalf("stat after overwrite Save: %v", err) + } + if got := fi.Mode().Perm(); got != 0o600 { + t.Errorf("overwrite file mode: got %o, want 600", got) + } +} diff --git a/internal/session/session.go b/internal/session/session.go index e55af77..cf4b7a3 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -204,6 +204,10 @@ func Load() (*Session, error) { } // SaveProvider persists s to the provider session file with 0600 permissions. +// +// os.WriteFile only applies the mode when creating a new file, so we also +// call os.Chmod afterwards to narrow permissions on pre-existing files that +// may have been written with wider modes by older versions. func SaveProvider(provider string, s *Session) error { provider = NormalizeProvider(provider) if provider == "" { @@ -226,7 +230,11 @@ func SaveProvider(provider string, s *Session) error { if err != nil { return err } - return os.WriteFile(SessionFilePath(provider), data, 0o600) + path := SessionFilePath(provider) + if err := os.WriteFile(path, data, 0o600); err != nil { + return err + } + return os.Chmod(path, 0o600) } // Save persists s to the active provider's session file with 0600 permissions. diff --git a/internal/session/session_test.go b/internal/session/session_test.go index f451ffa..cd35f87 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -1008,6 +1008,50 @@ func TestLoadProvider_FallsBackToCurrentLegacyFilename(t *testing.T) { } } +func TestSaveProvider_EnforcesFileMode0600(t *testing.T) { + for _, provider := range []string{ProviderFrisco, ProviderDelio} { + t.Run(provider, func(t *testing.T) { + dir := t.TempDir() + setTempSession(t, dir) + + s := &Session{ + BaseURL: DefaultBaseURLForProvider(provider), + Token: "tok_" + provider, + Headers: map[string]string{}, + } + + // Fresh write creates the file with 0600. + if err := SaveProvider(provider, s); err != nil { + t.Fatalf("SaveProvider (fresh): %v", err) + } + path := SessionFilePath(provider) + fi, err := os.Stat(path) + if err != nil { + t.Fatalf("stat after fresh SaveProvider: %v", err) + } + if got := fi.Mode().Perm(); got != 0o600 { + t.Errorf("fresh file mode: got %o, want 600", got) + } + + // Pre-existing file with wider permissions must be narrowed to 0600. + if err := os.Chmod(path, 0o644); err != nil { + t.Fatalf("Chmod 0644: %v", err) + } + s.Token = "tok_" + provider + "_v2" + if err := SaveProvider(provider, s); err != nil { + t.Fatalf("SaveProvider (overwrite): %v", err) + } + fi, err = os.Stat(path) + if err != nil { + t.Fatalf("stat after overwrite SaveProvider: %v", err) + } + if got := fi.Mode().Perm(); got != 0o600 { + t.Errorf("overwrite file mode: got %o, want 600", got) + } + }) + } +} + func TestLoadProvider_FallsBackToLegacyDir(t *testing.T) { base := t.TempDir() newDir := filepath.Join(base, "martmart-cli")