Skip to content

Commit 3c14ede

Browse files
hughdbrownclaudewesm
authored
Add cross-platform secure file permissions with Windows DACL support (#73)
# Problem Unix permission modes (0600, 0700) used throughout the codebase are no-ops on Windows for access control. On a multi-user Windows machine, OAuth tokens, emails, and attachments are readable by other users. # Solution Create internal/fileutil/ package with build-tagged implementations: - Unix: thin wrappers around os.WriteFile, os.MkdirAll, os.Chmod, os.OpenFile - Windows: after calling the standard os function, set a DACL restricting access to the current user when the mode is owner-only (perm & 0077 == 0) # internal/fileutil/ - secure_unix.go (//go:build !windows) — thin wrappers around os.WriteFile, os.MkdirAll, os.Chmod, os.OpenFile - secure_windows.go (//go:build windows) — calls the standard os function, then for owner-only modes (perm & 0077 == 0) applies a DACL via golang.org/x/sys/windows that grants GENERIC_ALL only to the current user with PROTECTED_DACL_SECURITY_INFORMATION to block inherited ACEs. SID lookup failures log a warning and fall back to default behavior. - secure_test.go — 6 tests covering all 4 functions with owner-only and permissive modes, error paths, and read-only open # Usage | File | Internal calls | |---|---| | internal/oauth/oauth.go │|os.MkdirAll → SecureMkdirAll (0700), os.Chmod → SecureChmod (0600) | | internal/config/config.go | 2x os.MkdirAll → SecureMkdirAll (0700) | | internal/deletion/manifest.go | os.WriteFile → SecureWriteFile (0600) | | internal/sync/sync.go | os.WriteFile → SecureWriteFile (0600) | | cmd/msgvault/cmd/export_eml.go | os.WriteFile → SecureWriteFile (0600) | | cmd/msgvault/cmd/export_attachment.go | os.OpenFile → SecureOpenFile (0600) | | internal/export/attachments.go | 2x os.OpenFile → SecureOpenFile (perm param) | | internal/update/update.go | os.WriteFile → SecureWriteFile (0600) | --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>
1 parent 2305d72 commit 3c14ede

File tree

13 files changed

+440
-22
lines changed

13 files changed

+440
-22
lines changed

cmd/msgvault/cmd/export_attachment.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/spf13/cobra"
1212
"github.com/wesm/msgvault/internal/export"
13+
"github.com/wesm/msgvault/internal/fileutil"
1314
)
1415

1516
var (
@@ -133,7 +134,7 @@ func exportAttachmentBinary(storagePath, contentHash string) error {
133134
return err
134135
}
135136

136-
dst, err := os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
137+
dst, err := fileutil.SecureOpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
137138
if err != nil {
138139
return fmt.Errorf("create output file: %w", err)
139140
}

cmd/msgvault/cmd/export_eml.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strconv"
77

88
"github.com/spf13/cobra"
9+
"github.com/wesm/msgvault/internal/fileutil"
910
"github.com/wesm/msgvault/internal/query"
1011
"github.com/wesm/msgvault/internal/store"
1112
)
@@ -86,7 +87,7 @@ Examples:
8687
return err
8788
}
8889

89-
err = os.WriteFile(outputPath, rawData, 0600) // Restricted permissions for email content
90+
err = fileutil.SecureWriteFile(outputPath, rawData, 0600) // Restricted permissions for email content
9091
if err != nil {
9192
return fmt.Errorf("write file: %w", err)
9293
}

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,18 @@ require (
99
github.com/charmbracelet/lipgloss v1.1.0
1010
github.com/charmbracelet/x/ansi v0.10.1
1111
github.com/gogs/chardet v0.0.0-20211120154057-b7413eaefb8f
12+
github.com/google/go-cmp v0.7.0
1213
github.com/jhillyerd/enmime v1.3.0
1314
github.com/marcboeker/go-duckdb v1.8.5
1415
github.com/mark3labs/mcp-go v0.43.2
1516
github.com/mattn/go-runewidth v0.0.16
1617
github.com/mattn/go-sqlite3 v1.14.33
1718
github.com/muesli/termenv v0.16.0
1819
github.com/spf13/cobra v1.10.2
20+
golang.org/x/mod v0.30.0
1921
golang.org/x/oauth2 v0.34.0
2022
golang.org/x/sync v0.19.0
23+
golang.org/x/sys v0.37.0
2124
golang.org/x/text v0.30.0
2225
)
2326

@@ -36,7 +39,6 @@ require (
3639
github.com/go-viper/mapstructure/v2 v2.2.1 // indirect
3740
github.com/goccy/go-json v0.10.5 // indirect
3841
github.com/google/flatbuffers v25.1.24+incompatible // indirect
39-
github.com/google/go-cmp v0.7.0 // indirect
4042
github.com/google/uuid v1.6.0 // indirect
4143
github.com/inconshreveable/mousetrap v1.1.0 // indirect
4244
github.com/invopop/jsonschema v0.13.0 // indirect
@@ -61,9 +63,7 @@ require (
6163
github.com/yosida95/uritemplate/v3 v3.0.2 // indirect
6264
github.com/zeebo/xxh3 v1.0.2 // indirect
6365
golang.org/x/exp v0.0.0-20250218142911-aa4b98e5adaa // indirect
64-
golang.org/x/mod v0.30.0 // indirect
6566
golang.org/x/net v0.46.0 // indirect
66-
golang.org/x/sys v0.37.0 // indirect
6767
golang.org/x/telemetry v0.0.0-20251008203120-078029d740a8 // indirect
6868
golang.org/x/tools v0.38.0 // indirect
6969
golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect

internal/config/config.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ package config
33

44
import (
55
"fmt"
6+
"log/slog"
67
"os"
78
"path/filepath"
89
"strings"
910

1011
"github.com/BurntSushi/toml"
12+
"github.com/wesm/msgvault/internal/fileutil"
1113
)
1214

1315
// Config represents the msgvault configuration.
@@ -142,7 +144,7 @@ func (c *Config) AnalyticsDir() string {
142144

143145
// EnsureHomeDir creates the msgvault home directory if it doesn't exist.
144146
func (c *Config) EnsureHomeDir() error {
145-
return os.MkdirAll(c.HomeDir, 0700)
147+
return fileutil.SecureMkdirAll(c.HomeDir, 0700)
146148
}
147149

148150
// ConfigFilePath returns the path to the config file.
@@ -174,28 +176,40 @@ func MkTempDir(pattern string, preferredDirs ...string) (string, error) {
174176
}
175177
dir, err := os.MkdirTemp(base, pattern)
176178
if err == nil {
179+
secureTempDir(dir)
177180
return dir, nil
178181
}
179182
}
180183

181184
// Try system temp dir
182185
dir, sysErr := os.MkdirTemp("", pattern)
183186
if sysErr == nil {
187+
secureTempDir(dir)
184188
return dir, nil
185189
}
186190

187191
// Fallback: use ~/.msgvault/tmp/
188192
fallbackBase := filepath.Join(DefaultHome(), "tmp")
189-
if err := os.MkdirAll(fallbackBase, 0700); err != nil {
193+
if err := fileutil.SecureMkdirAll(fallbackBase, 0700); err != nil {
190194
return "", fmt.Errorf("create temp dir: %w (fallback also failed: %v)", sysErr, err)
191195
}
192196
dir, err := os.MkdirTemp(fallbackBase, pattern)
193197
if err != nil {
194198
return "", fmt.Errorf("create temp dir: %w (fallback also failed: %v)", sysErr, err)
195199
}
200+
secureTempDir(dir)
196201
return dir, nil
197202
}
198203

204+
// secureTempDir applies owner-only permissions to a temp directory created by
205+
// os.MkdirTemp, which uses default permissions. On Windows, this also sets an
206+
// owner-only DACL. Failures are logged but non-fatal.
207+
func secureTempDir(dir string) {
208+
if err := fileutil.SecureChmod(dir, 0700); err != nil {
209+
slog.Warn("failed to secure temp directory permissions", "path", dir, "err", err)
210+
}
211+
}
212+
199213
// resolveRelative makes a relative path absolute by joining it with base.
200214
// Absolute paths and empty strings are returned unchanged.
201215
func resolveRelative(path, base string) string {

internal/config/config_test.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,23 @@ func TestDefaultHomeExpandsTilde(t *testing.T) {
344344
}
345345
}
346346

347+
// assertTempDirSecured checks that a temp dir has permissions no more
348+
// permissive than 0700. This is umask-tolerant (stricter is fine).
349+
func assertTempDirSecured(t *testing.T, dir string) {
350+
t.Helper()
351+
if runtime.GOOS == "windows" {
352+
return // Windows uses DACLs, not Unix permission bits
353+
}
354+
info, err := os.Stat(dir)
355+
if err != nil {
356+
t.Fatalf("Stat temp dir: %v", err)
357+
}
358+
got := info.Mode().Perm()
359+
if got&^os.FileMode(0700) != 0 {
360+
t.Errorf("temp dir perm = %04o, has bits beyond 0700 (extra: %04o)", got, got&^0700)
361+
}
362+
}
363+
347364
func TestMkTempDir(t *testing.T) {
348365
t.Run("uses system temp when no preferred dirs", func(t *testing.T) {
349366
dir, err := MkTempDir("test-*")
@@ -355,6 +372,7 @@ func TestMkTempDir(t *testing.T) {
355372
if _, err := os.Stat(dir); err != nil {
356373
t.Errorf("temp dir does not exist: %v", err)
357374
}
375+
assertTempDirSecured(t, dir)
358376
})
359377

360378
t.Run("uses preferred dir when available", func(t *testing.T) {
@@ -368,6 +386,7 @@ func TestMkTempDir(t *testing.T) {
368386
if !strings.HasPrefix(dir, preferred) {
369387
t.Errorf("temp dir %q not under preferred %q", dir, preferred)
370388
}
389+
assertTempDirSecured(t, dir)
371390
})
372391

373392
t.Run("skips empty preferred dir strings", func(t *testing.T) {
@@ -433,13 +452,8 @@ func TestMkTempDir(t *testing.T) {
433452
}
434453

435454
// Verify the tmp dir was created with restrictive permissions
436-
info, err := os.Stat(expectedBase)
437-
if err != nil {
438-
t.Fatalf("stat fallback dir: %v", err)
439-
}
440-
if perm := info.Mode().Perm(); perm != 0o700 {
441-
t.Errorf("fallback dir permissions = %04o, want 0700", perm)
442-
}
455+
assertTempDirSecured(t, expectedBase)
456+
assertTempDirSecured(t, dir)
443457
})
444458
}
445459

internal/deletion/manifest.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"sort"
1111
"strings"
1212
"time"
13+
14+
"github.com/wesm/msgvault/internal/fileutil"
1315
)
1416

1517
// Status represents the state of a deletion batch.
@@ -151,7 +153,7 @@ func (m *Manifest) Save(path string) error {
151153
return err
152154
}
153155

154-
return os.WriteFile(path, data, 0600)
156+
return fileutil.SecureWriteFile(path, data, 0600)
155157
}
156158

157159
// FormatSummary returns a human-readable summary of the deletion.

internal/export/attachments.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import (
1313
"strings"
1414
"syscall"
1515

16+
"github.com/wesm/msgvault/internal/fileutil"
17+
1618
"github.com/wesm/msgvault/internal/query"
1719
)
1820

@@ -322,7 +324,7 @@ func exportAttachmentToFile(outputDir, attachmentsDir, contentHash, filename str
322324
// trying path, path_1, path_2, etc. on conflict. Returns the open file and
323325
// the path that was actually used. Uses O_CREATE|O_EXCL to avoid TOCTOU races.
324326
func CreateExclusiveFile(p string, perm os.FileMode) (*os.File, string, error) {
325-
f, err := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
327+
f, err := fileutil.SecureOpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
326328
if err == nil {
327329
return f, p, nil
328330
}
@@ -333,7 +335,7 @@ func CreateExclusiveFile(p string, perm os.FileMode) (*os.File, string, error) {
333335
base := p[:len(p)-len(ext)]
334336
for i := 1; ; i++ {
335337
candidate := fmt.Sprintf("%s_%d%s", base, i, ext)
336-
f, err = os.OpenFile(candidate, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
338+
f, err = fileutil.SecureOpenFile(candidate, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
337339
if err == nil {
338340
return f, candidate, nil
339341
}

0 commit comments

Comments
 (0)