Skip to content

Commit 0e12e62

Browse files
EconoBenclaude
andcommitted
Merge upstream/main into feat/tui-remote-support
Resolve conflicts by: - Taking upstream for .roborev.toml, Dockerfile, cmd/msgvault/cmd/setup.go - Keeping our doRequestWithContext in internal/remote/store.go for context propagation - Keeping our TUI aggregate endpoints in internal/api/handlers.go - Keeping our test additions in internal/api/handlers_test.go - Keeping our engine field in internal/api/server.go All tests pass after merge. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 parents 65527eb + 68517c1 commit 0e12e62

25 files changed

+1548
-263
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ oauth_client*.json
2222

2323
# Local development state
2424
.beads/
25+
.githooks/post-commit
26+
.mcp.json
2527

2628
# Python
2729
__pycache__/

.roborev.toml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,11 @@ choice — do not flag it as a concern.
1313
HTTP remote defaults, plaintext key display in interactive CLI,
1414
enabled=true override on account creation, and page-aligned pagination
1515
are documented design decisions — see code comments at each site.
16+
17+
This is a single-user personal tool with no privilege separation, no
18+
setuid, no shared directories, and no multi-tenant access. Do not flag
19+
symlink-following, local file overwrites, or similar CWE patterns that
20+
require an attacker with the same privileges as the user — they already
21+
own everything this tool touches. Auth on loopback-only endpoints with
22+
no configured API key is also by design.
1623
"""

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ WORKDIR /data
6262

6363
# Health check using wget (curl not included to keep image small)
6464
HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \
65-
CMD wget -q --spider http://localhost:8080/health || exit 1
65+
CMD wget -qO/dev/null http://localhost:8080/health || exit 1
6666

6767
# Default port for HTTP API
6868
EXPOSE 8080

cmd/msgvault/cmd/mcp.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ var mcpCmd = &cobra.Command{
2121
2222
This allows Claude Desktop (or any MCP client) to query your email archive
2323
using tools like search_messages, get_message, list_messages, get_stats,
24-
and aggregate.
24+
aggregate, and stage_deletion.
2525
2626
Add to Claude Desktop config:
2727
{
@@ -76,7 +76,7 @@ Add to Claude Desktop config:
7676
ctx, cancel := context.WithCancel(context.Background())
7777
defer cancel()
7878

79-
return mcpserver.Serve(ctx, engine, cfg.AttachmentsDir())
79+
return mcpserver.Serve(ctx, engine, cfg.AttachmentsDir(), cfg.Data.DataDir)
8080
},
8181
}
8282

cmd/msgvault/cmd/setup.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,7 @@ rate_limit_qps = 5
272272
}
273273

274274
// Create docker-compose.yml
275-
dockerCompose := fmt.Sprintf(`version: "3.8"
276-
277-
services:
275+
dockerCompose := fmt.Sprintf(`services:
278276
msgvault:
279277
image: ghcr.io/wesm/msgvault:latest
280278
container_name: msgvault

internal/api/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ func (s *Server) setupRouter() chi.Router {
116116

117117
// Health check (no auth required)
118118
r.Get("/health", s.handleHealth)
119+
r.Head("/health", s.handleHealth)
119120

120121
// API routes (auth required)
121122
r.Route("/api/v1", func(r chi.Router) {

internal/api/server_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,24 @@ func TestHealthEndpoint(t *testing.T) {
115115
}
116116
}
117117

118+
func TestHealthEndpoint_HEAD(t *testing.T) {
119+
cfg := &config.Config{
120+
Server: config.ServerConfig{APIPort: 8080},
121+
}
122+
sched := newMockScheduler()
123+
srv := NewServer(cfg, nil, sched, testLogger())
124+
125+
req := httptest.NewRequest("HEAD", "/health", nil)
126+
w := httptest.NewRecorder()
127+
128+
srv.Router().ServeHTTP(w, req)
129+
130+
if w.Code != http.StatusOK {
131+
t.Errorf("HEAD /health status = %d, want %d",
132+
w.Code, http.StatusOK)
133+
}
134+
}
135+
118136
func TestAuthMiddleware(t *testing.T) {
119137
cfg := &config.Config{
120138
Server: config.ServerConfig{

internal/config/config.go

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -243,27 +243,66 @@ func (c *Config) ConfigFilePath() string {
243243
return filepath.Join(c.HomeDir, "config.toml")
244244
}
245245

246-
// Save writes the current configuration to disk.
247-
// Creates the config file if it doesn't exist, or updates it if it does.
248-
// Empty sections are omitted from the output.
246+
// Save writes the current configuration to disk atomically.
247+
// Uses temp file + rename to prevent partial writes on crash.
248+
// Enforces 0600 permissions regardless of existing file mode.
249249
func (c *Config) Save() error {
250250
path := c.ConfigFilePath()
251251

252+
// Resolve symlinks so atomic rename replaces the target, not
253+
// the symlink itself. EvalSymlinks fails on dangling symlinks
254+
// (target doesn't exist yet), so fall back to Readlink.
255+
if resolved, err := filepath.EvalSymlinks(path); err == nil {
256+
path = resolved
257+
} else if target, lErr := os.Readlink(path); lErr == nil {
258+
if !filepath.IsAbs(target) {
259+
target = filepath.Join(filepath.Dir(path), target)
260+
}
261+
path = target
262+
}
263+
252264
// Ensure home directory exists
253265
if err := c.EnsureHomeDir(); err != nil {
254266
return fmt.Errorf("create config directory: %w", err)
255267
}
256268

257-
f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
269+
dir := filepath.Dir(path)
270+
tmp, err := os.CreateTemp(dir, ".config-*.toml.tmp")
258271
if err != nil {
259-
return fmt.Errorf("create config file: %w", err)
272+
return fmt.Errorf("create temp config file: %w", err)
273+
}
274+
tmpPath := tmp.Name()
275+
276+
// Clean up temp file on any failure path
277+
success := false
278+
defer func() {
279+
if !success {
280+
tmp.Close()
281+
os.Remove(tmpPath)
282+
}
283+
}()
284+
285+
if err := tmp.Chmod(0600); err != nil {
286+
return fmt.Errorf("set config file permissions: %w", err)
260287
}
261-
defer f.Close()
262288

263-
if err := toml.NewEncoder(f).Encode(c); err != nil {
289+
if err := toml.NewEncoder(tmp).Encode(c); err != nil {
264290
return fmt.Errorf("encode config: %w", err)
265291
}
266292

293+
if err := tmp.Sync(); err != nil {
294+
return fmt.Errorf("sync config file: %w", err)
295+
}
296+
297+
if err := tmp.Close(); err != nil {
298+
return fmt.Errorf("close config file: %w", err)
299+
}
300+
301+
if err := os.Rename(tmpPath, path); err != nil {
302+
return fmt.Errorf("rename config file: %w", err)
303+
}
304+
305+
success = true
267306
return nil
268307
}
269308

internal/config/config_test.go

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -926,6 +926,187 @@ func TestSave_CreatesFileWithSecurePermissions(t *testing.T) {
926926
}
927927
}
928928

929+
func TestSave_TightensWeakPermissions(t *testing.T) {
930+
if runtime.GOOS == "windows" {
931+
t.Skip("Unix file permissions not supported on Windows")
932+
}
933+
934+
tmpDir := t.TempDir()
935+
cfg := NewDefaultConfig()
936+
cfg.HomeDir = tmpDir
937+
938+
// Pre-create config file with overly permissive mode
939+
path := cfg.ConfigFilePath()
940+
if err := os.WriteFile(path, []byte(""), 0644); err != nil {
941+
t.Fatalf("WriteFile: %v", err)
942+
}
943+
944+
if err := cfg.Save(); err != nil {
945+
t.Fatalf("Save() error = %v", err)
946+
}
947+
948+
info, err := os.Stat(path)
949+
if err != nil {
950+
t.Fatalf("Stat: %v", err)
951+
}
952+
if info.Mode().Perm()&0077 != 0 {
953+
t.Errorf("Save should tighten perms: got %04o, want 0600",
954+
info.Mode().Perm())
955+
}
956+
}
957+
958+
func TestSave_FollowsSymlink(t *testing.T) {
959+
if runtime.GOOS == "windows" {
960+
t.Skip("symlinks require elevated privileges on Windows")
961+
}
962+
963+
t.Run("absolute target", func(t *testing.T) {
964+
tmpDir := t.TempDir()
965+
targetDir := t.TempDir()
966+
targetPath := filepath.Join(targetDir, "actual-config.toml")
967+
linkPath := filepath.Join(tmpDir, "config.toml")
968+
969+
if err := os.Symlink(targetPath, linkPath); err != nil {
970+
t.Fatalf("Symlink: %v", err)
971+
}
972+
973+
cfg := NewDefaultConfig()
974+
cfg.HomeDir = tmpDir
975+
cfg.Sync.RateLimitQPS = 77
976+
977+
if err := cfg.Save(); err != nil {
978+
t.Fatalf("Save() error = %v", err)
979+
}
980+
981+
linkTarget, err := os.Readlink(linkPath)
982+
if err != nil {
983+
t.Fatalf("symlink was replaced: %v", err)
984+
}
985+
if linkTarget != targetPath {
986+
t.Errorf("symlink target = %q, want %q", linkTarget, targetPath)
987+
}
988+
989+
loaded, err := Load(targetPath, "")
990+
if err != nil {
991+
t.Fatalf("Load target: %v", err)
992+
}
993+
if loaded.Sync.RateLimitQPS != 77 {
994+
t.Errorf("RateLimitQPS = %d, want 77", loaded.Sync.RateLimitQPS)
995+
}
996+
})
997+
998+
t.Run("relative target", func(t *testing.T) {
999+
tmpDir := t.TempDir()
1000+
// Create subdir for the actual file
1001+
subDir := filepath.Join(tmpDir, "real")
1002+
if err := os.Mkdir(subDir, 0700); err != nil {
1003+
t.Fatalf("Mkdir: %v", err)
1004+
}
1005+
targetPath := filepath.Join(subDir, "config.toml")
1006+
linkPath := filepath.Join(tmpDir, "config.toml")
1007+
1008+
// Relative symlink: config.toml → real/config.toml
1009+
if err := os.Symlink("real/config.toml", linkPath); err != nil {
1010+
t.Fatalf("Symlink: %v", err)
1011+
}
1012+
1013+
cfg := NewDefaultConfig()
1014+
cfg.HomeDir = tmpDir
1015+
cfg.Sync.RateLimitQPS = 88
1016+
1017+
if err := cfg.Save(); err != nil {
1018+
t.Fatalf("Save() error = %v", err)
1019+
}
1020+
1021+
// Symlink must still be intact
1022+
linkTarget, err := os.Readlink(linkPath)
1023+
if err != nil {
1024+
t.Fatalf("symlink was replaced: %v", err)
1025+
}
1026+
if linkTarget != "real/config.toml" {
1027+
t.Errorf("symlink target = %q, want %q",
1028+
linkTarget, "real/config.toml")
1029+
}
1030+
1031+
// Target file should contain the saved config
1032+
loaded, err := Load(targetPath, "")
1033+
if err != nil {
1034+
t.Fatalf("Load target: %v", err)
1035+
}
1036+
if loaded.Sync.RateLimitQPS != 88 {
1037+
t.Errorf("RateLimitQPS = %d, want 88",
1038+
loaded.Sync.RateLimitQPS)
1039+
}
1040+
})
1041+
}
1042+
1043+
func TestSave_FailurePreservesExisting(t *testing.T) {
1044+
if runtime.GOOS == "windows" {
1045+
t.Skip("cannot make directory unwritable on Windows")
1046+
}
1047+
1048+
tmpDir := t.TempDir()
1049+
1050+
// Save initial valid config
1051+
cfg := NewDefaultConfig()
1052+
cfg.HomeDir = tmpDir
1053+
cfg.Sync.RateLimitQPS = 5
1054+
if err := cfg.Save(); err != nil {
1055+
t.Fatalf("initial Save: %v", err)
1056+
}
1057+
1058+
// Read back original content
1059+
originalBytes, err := os.ReadFile(cfg.ConfigFilePath())
1060+
if err != nil {
1061+
t.Fatalf("ReadFile: %v", err)
1062+
}
1063+
1064+
// Make directory unwritable so CreateTemp fails
1065+
if err := os.Chmod(tmpDir, 0500); err != nil {
1066+
t.Fatalf("Chmod: %v", err)
1067+
}
1068+
t.Cleanup(func() { _ = os.Chmod(tmpDir, 0700) })
1069+
1070+
// Probe whether the restriction actually works
1071+
probe, probeErr := os.CreateTemp(tmpDir, "probe-*")
1072+
if probeErr == nil {
1073+
probe.Close()
1074+
os.Remove(probe.Name())
1075+
t.Skip("chmod 0500 did not restrict writes (running as root)")
1076+
}
1077+
1078+
// Save should fail
1079+
cfg.Sync.RateLimitQPS = 99
1080+
if err := cfg.Save(); err == nil {
1081+
t.Fatal("Save should fail when directory is unwritable")
1082+
}
1083+
1084+
// Restore permissions to verify state
1085+
if err := os.Chmod(tmpDir, 0700); err != nil {
1086+
t.Fatalf("Chmod restore: %v", err)
1087+
}
1088+
1089+
// Original config should be intact
1090+
currentBytes, err := os.ReadFile(cfg.ConfigFilePath())
1091+
if err != nil {
1092+
t.Fatalf("ReadFile: %v", err)
1093+
}
1094+
if string(currentBytes) != string(originalBytes) {
1095+
t.Error("config file was corrupted after failed Save")
1096+
}
1097+
1098+
// No temp files should be left behind
1099+
entries, err := os.ReadDir(tmpDir)
1100+
if err != nil {
1101+
t.Fatalf("ReadDir: %v", err)
1102+
}
1103+
for _, e := range entries {
1104+
if strings.HasPrefix(e.Name(), ".config-") {
1105+
t.Errorf("leftover temp file: %s", e.Name())
1106+
}
1107+
}
1108+
}
1109+
9291110
func TestSave_OverwritesExisting(t *testing.T) {
9301111
tmpDir := t.TempDir()
9311112

0 commit comments

Comments
 (0)