From b99a19b066d7258c52870953d129ecbc80016d23 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 08:13:54 -0500
Subject: [PATCH 01/13] Add code review findings

---
 REVIEW.md | 449 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 449 insertions(+)
 create mode 100644 REVIEW.md

diff --git a/REVIEW.md b/REVIEW.md
new file mode 100644
index 0000000..ea590b7
--- /dev/null
+++ b/REVIEW.md
@@ -0,0 +1,449 @@
+# mogcli Code Review
+
+**Date:** 2026-02-13  
+**Scope:** Security, concurrency, UX/UI, code quality  
+**Codebase:** ~9,700 LOC (internal packages)
+
+---
+
+## Summary
+
+mogcli is a well-structured Go CLI with good separation of concerns. The codebase follows Go conventions and has reasonable test coverage. However, there are security concerns around file path handling, a race condition in the circuit breaker, and several UX improvements that could be made.
+
+**Critical Issues:** 2  
+**High Severity:** 2  
+**Medium Severity:** 5  
+**Low Severity:** 4  
+
+---
+
+## Security Issues
+
+### 🔴 CRITICAL: Path Traversal in OneDrive File Operations
+
+**Files:**
+- `internal/cmd/onedrive.go:93, 128, 133`
+- `internal/config/paths.go:120-145`
+
+**Description:**  
+User-supplied file paths for OneDrive upload/download are not validated against directory traversal attacks. The `ExpandPath()` function only handles `~` expansion but does not prevent `../` sequences.
+
+```go
+// internal/cmd/onedrive.go:128
+localPath, err := config.ExpandPath(strings.TrimSpace(c.LocalPath))
+
+// internal/cmd/onedrive.go:133
+content, err := os.ReadFile(localPath) //nolint:gosec // user-provided file path
+```
+
+The `nolint:gosec` comment acknowledges the risk but doesn't mitigate it.
+
+**Attack Vector:**
+```bash
+# Read arbitrary files during upload
+mog onedrive put ../../../etc/passwd --path /stolen.txt
+
+# Write to arbitrary locations during download
+mog onedrive get /file.txt --out ../../../tmp/malicious
+```
+
+**Recommendation:**  
+Add path canonicalization and validation:
+
+```go
+func SafeExpandPath(basePath, userPath string) (string, error) {
+    expanded, err := ExpandPath(userPath)
+    if err != nil {
+        return "", err
+    }
+    
+    cleaned := filepath.Clean(expanded)
+    absBase, _ := filepath.Abs(basePath)
+    absCleaned, _ := filepath.Abs(cleaned)
+    
+    if !strings.HasPrefix(absCleaned, absBase) {
+        return "", fmt.Errorf("path escapes base directory: %s", userPath)
+    }
+    
+    return cleaned, nil
+}
+```
+
+---
+
+### 🔴 CRITICAL: Race Condition in Circuit Breaker
+
+**File:** `internal/graph/circuitbreaker.go:41-55`
+
+**Description:**  
+The `IsOpen()` method reads and writes multiple fields under a single mutex lock, but there's a Time-of-Check-Time-of-Use (TOCTOU) issue. Between checking `IsOpen()` in `client.go:83` and recording success/failure, the circuit breaker state can change.
+
+```go
+// internal/graph/client.go:83-84
+if c.Breaker != nil && c.Breaker.IsOpen() {
+    return nil, nil, &CircuitBreakerError{}
+}
+// ... request happens here, breaker state can change ...
+if c.Breaker != nil {
+    c.Breaker.RecordSuccess()
+}
+```
+
+**Impact:**  
+Under high concurrency, requests may be allowed through an open breaker, or the failure count may not accurately reflect the actual error rate.
+
+**Recommendation:**  
+Use an atomic check-and-execute pattern:
+
+```go
+func (cb *CircuitBreaker) Execute(fn func() error) error {
+    cb.mu.Lock()
+    if cb.open && time.Since(cb.lastFailure) <= CircuitBreakerResetTime {
+        cb.mu.Unlock()
+        return &CircuitBreakerError{}
+    }
+    cb.mu.Unlock()
+    
+    err := fn()
+    
+    cb.mu.Lock()
+    defer cb.mu.Unlock()
+    if err != nil {
+        cb.failures++
+        cb.lastFailure = time.Now()
+        if cb.failures >= CircuitBreakerThreshold {
+            cb.open = true
+        }
+    } else {
+        cb.failures = 0
+        cb.open = false
+    }
+    return err
+}
+```
+
+---
+
+### 🟠 HIGH: No Validation of Pagination Token Host
+
+**File:** `internal/cmd/pagination.go:8-26`
+
+**Description:**  
+The pagination token URL is validated for format but not for host. An attacker could inject a malicious URL as a `--page` token to redirect API calls.
+
+```go
+func normalizePageToken(raw string) (string, error) {
+    // Only checks for valid URL format, not host
+    if u.Scheme != "http" && u.Scheme != "https" {
+        return "", usage("--page must be an absolute http(s) URL")
+    }
+    return u.String(), nil
+}
+```
+
+The Graph client has `sameHost()` validation (line 235), but pagination tokens bypass this in `Paginate()` when passed as a full URL.
+
+**Recommendation:**  
+Validate that pagination URLs match the expected Microsoft Graph host:
+
+```go
+var allowedHosts = []string{"graph.microsoft.com"}
+
+func normalizePageToken(raw string) (string, error) {
+    // ... existing validation ...
+    
+    if !isAllowedHost(u.Host) {
+        return "", usage("--page URL must be a Microsoft Graph URL")
+    }
+    return u.String(), nil
+}
+```
+
+---
+
+### 🟠 HIGH: Client Secret Visible in Process List
+
+**File:** `internal/cmd/auth.go:70`
+
+**Description:**  
+The `--client-secret` flag accepts secrets directly on the command line, which exposes them in process listings (`ps aux`).
+
+```go
+ClientSecret    string `name:"client-secret" help:"Client secret for app-only mode"`
+```
+
+While `--client-secret-env` is provided as an alternative, users may not realize the security implications.
+
+**Recommendation:**  
+1. Add a warning when `--client-secret` is used directly
+2. Consider deprecating the flag in favor of env-var-only input
+3. Add interactive secret prompting when neither is provided
+
+---
+
+### 🟡 MEDIUM: Unvalidated Profile Names in File Paths
+
+**Files:**
+- `internal/profile/store.go:33-39`
+- `internal/auth/manager.go:55-60`
+
+**Description:**  
+Profile names are used in secret key construction without strict validation. While `NormalizeName()` only trims whitespace, profile names could contain path-sensitive characters.
+
+```go
+func tokenCacheKey(profileName string) string {
+    return "mog:cache:" + strings.TrimSpace(profileName)
+}
+```
+
+The base64 encoding in `secretPath()` mitigates this for file paths, but profile names like `../malicious` could cause issues elsewhere.
+
+**Recommendation:**  
+Add character validation to `NormalizeName()`:
+
+```go
+func NormalizeName(name string) (string, error) {
+    normalized := strings.TrimSpace(name)
+    if normalized == "" {
+        return "", ErrInvalidName
+    }
+    
+    for _, r := range normalized {
+        if !((r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || 
+             (r >= '0' && r <= '9') || r == '-' || r == '_') {
+            return "", fmt.Errorf("%w: invalid character %q", ErrInvalidName, r)
+        }
+    }
+    
+    return normalized, nil
+}
+```
+
+---
+
+### 🟡 MEDIUM: Tokens Not Cleared from Memory
+
+**File:** `internal/auth/manager.go`
+
+**Description:**  
+Access tokens and refresh tokens are stored in regular Go strings, which cannot be securely cleared from memory. This is a common limitation in Go, but sensitive data remains in memory longer than necessary.
+
+**Recommendation:**  
+Consider using `[]byte` for tokens and implementing a secure clear function:
+
+```go
+func secureZero(b []byte) {
+    for i := range b {
+        b[i] = 0
+    }
+}
+```
+
+---
+
+## Concurrency Issues
+
+### 🟡 MEDIUM: Config File Read/Write Race
+
+**Files:**
+- `internal/config/config.go:43-70` (WriteConfig)
+- `internal/config/config.go:90-115` (ReadConfig)
+
+**Description:**  
+Multiple `mog` processes running concurrently could race on config file reads and writes. While atomic rename is used, there's no file locking.
+
+```go
+func WriteConfig(cfg File) error {
+    // No file locking
+    tmp := path + ".tmp"
+    if err := os.WriteFile(tmp, b, 0o600); err != nil { ... }
+    if err := os.Rename(tmp, path); err != nil { ... }
+}
+```
+
+**Impact:**  
+Lost updates if two processes modify profiles simultaneously.
+
+**Recommendation:**  
+Use file locking via `flock()` or a lockfile pattern for config modifications.
+
+---
+
+## UX Issues
+
+### 🟡 MEDIUM: No Confirmation for Destructive Operations
+
+**File:** `internal/cmd/onedrive.go:189`
+
+**Description:**  
+While `onedrive rm` has confirmation via `confirmDestructive()`, there's no undo mechanism and the confirmation message doesn't show file size or type.
+
+```go
+if err := confirmDestructive(ctx, flags, fmt.Sprintf("delete %s", c.Path)); err != nil {
+```
+
+**Recommendation:**  
+Fetch item metadata before confirmation to show:
+- File size
+- Last modified date  
+- Whether it's a folder (recursive delete warning)
+
+---
+
+### 🟢 LOW: Inconsistent Error Messages
+
+**Files:** Various
+
+**Description:**  
+Error messages mix styles:
+- `internal/graph/errors.go`: "Graph API error (404 NotFound): ..."
+- `internal/auth/manager.go`: "token exchange failed (error_code): description"
+- `internal/profile/store.go`: Bare sentinel errors like `ErrProfileNotFound`
+
+**Recommendation:**  
+Standardize error format across the codebase. Consider wrapping all user-facing errors through `errfmt.Format()`.
+
+---
+
+### 🟢 LOW: No Progress Indicator for Large File Transfers
+
+**File:** `internal/cmd/onedrive.go:76, 133`
+
+**Description:**  
+File uploads and downloads show no progress. For large files, the CLI appears frozen.
+
+**Recommendation:**  
+Add a progress bar using a library like `github.com/schollz/progressbar/v3`:
+
+```go
+bar := progressbar.DefaultBytes(fileSize, "Uploading")
+reader := progressbar.NewReader(file, bar)
+```
+
+---
+
+### 🟢 LOW: Missing `--dry-run` Flag for Write Operations
+
+**Files:**
+- `internal/cmd/mail.go` (send)
+- `internal/cmd/calendar.go` (create, update, delete)
+- `internal/cmd/onedrive.go` (put, rm, mkdir)
+
+**Description:**  
+Users cannot preview what will happen before executing write operations.
+
+**Recommendation:**  
+Add `--dry-run` flag that shows what would be done without making changes.
+
+---
+
+### 🟢 LOW: Hard-coded Pagination Limit
+
+**File:** `internal/cmd/onedrive.go:25`
+
+**Description:**  
+Default max of 100 items may not suit all use cases, and users might not realize there's more data.
+
+```go
+Max  int    `name:"max" default:"100" help:"Maximum items to return"`
+```
+
+**Recommendation:**  
+When results are truncated, always print a hint about pagination even in non-JSON mode.
+
+---
+
+## Code Quality Issues
+
+### 🟡 MEDIUM: Duplicated `decodeValuePage` Function
+
+**Files:**
+- `internal/services/calendar/service.go:82-99`
+- `internal/services/mail/service.go` (similar)
+- `internal/services/tasks/service.go` (similar)
+
+**Description:**  
+The same OData response parsing logic is duplicated across services.
+
+**Recommendation:**  
+Extract to a shared helper in `internal/graph/`:
+
+```go
+func DecodeODataPage(body []byte) ([]map[string]any, string, error)
+```
+
+---
+
+### 🟡 MEDIUM: Panic-based Exit Handling
+
+**File:** `internal/cmd/root.go:166`
+
+**Description:**  
+The CLI uses panics for exit code handling:
+
+```go
+kong.Exit(func(code int) { panic(exitPanic{code: code}) }),
+```
+
+While this is recovered, it's an unusual pattern that could confuse contributors.
+
+**Recommendation:**  
+Document this pattern clearly or refactor to use error wrapping with exit codes.
+
+---
+
+### 🟢 LOW: Unused `nolint` Comment Context
+
+**File:** `internal/cmd/onedrive.go:133`
+
+**Description:**  
+The `nolint:gosec` comment acknowledges the issue but doesn't document why it's acceptable:
+
+```go
+content, err := os.ReadFile(localPath) //nolint:gosec // user-provided file path
+```
+
+**Recommendation:**  
+Either fix the underlying issue or document why the risk is acceptable:
+
+```go
+//nolint:gosec // Path is validated by ExpandPath and intentionally user-controlled for CLI use
+```
+
+---
+
+### 🟢 LOW: Missing Godoc on Exported Types
+
+**Files:** Various
+
+**Description:**  
+Several exported types lack documentation:
+- `graph.Client`
+- `auth.Manager`
+- `profile.Store`
+
+**Recommendation:**  
+Add Godoc comments to all exported types and functions.
+
+---
+
+## Positive Observations
+
+1. **Good use of interfaces** - `authManager` and `profileStore` interfaces enable testing
+2. **Atomic config writes** - Using rename for safe config updates
+3. **Proper context propagation** - All operations accept `context.Context`
+4. **Consistent output modes** - `--json` and `--plain` are well-implemented
+5. **Good error wrapping** - `fmt.Errorf` with `%w` used consistently
+6. **Circuit breaker pattern** - Protects against cascading failures
+7. **Rate limit handling** - Proper retry-after and exponential backoff
+
+---
+
+## Recommended Priority
+
+1. **Immediate:** Fix path traversal vulnerability in OneDrive operations
+2. **High:** Address circuit breaker race condition
+3. **High:** Validate pagination token hosts
+4. **Medium:** Add profile name validation
+5. **Medium:** Add progress indicators for file transfers
+6. **Low:** Refactor duplicated code, add documentation

From 934c567670a89425099e9b04e985911c5ec1ce95 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:25:19 -0500
Subject: [PATCH 02/13] Harden OneDrive local path handling

---
 internal/cmd/onedrive.go      | 58 +++++++++++++++++++++---------
 internal/cmd/onedrive_test.go | 64 +++++++++++++++++++++++++++++++++
 internal/config/paths.go      | 41 +++++++++++++++++++++
 internal/config/paths_test.go | 68 +++++++++++++++++++++++++++++++++++
 4 files changed, 215 insertions(+), 16 deletions(-)
 create mode 100644 internal/cmd/onedrive_test.go
 create mode 100644 internal/config/paths_test.go

diff --git a/internal/cmd/onedrive.go b/internal/cmd/onedrive.go
index d403ead..ebe122b 100644
--- a/internal/cmd/onedrive.go
+++ b/internal/cmd/onedrive.go
@@ -77,23 +77,14 @@ func (c *OneDriveGetCmd) Run(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-
-	outPath := strings.TrimSpace(c.Out)
-	if outPath == "" {
-		outPath = filepath.Base(strings.TrimSpace(c.Path))
-		if outPath == "." || outPath == "/" || outPath == "" {
-			dir, dirErr := config.OneDriveDownloadsDir()
-			if dirErr != nil {
-				return dirErr
-			}
-			outPath = filepath.Join(dir, "download.bin")
-		}
-	}
-
-	expanded, err := config.ExpandPath(outPath)
+	expanded, err := resolveOneDriveOutPath(c.Path, c.Out)
 	if err != nil {
 		return err
 	}
+	if expanded == "" {
+		return usage("output path is required")
+	}
+
 	if err := os.MkdirAll(filepath.Dir(expanded), 0o755); err != nil {
 		return fmt.Errorf("create output directory: %w", err)
 	}
@@ -125,12 +116,15 @@ func (c *OneDrivePutCmd) Run(ctx context.Context) error {
 		return err
 	}
 
-	localPath, err := config.ExpandPath(strings.TrimSpace(c.LocalPath))
+	localPath, err := resolveOneDrivePutLocalPath(c.LocalPath)
 	if err != nil {
 		return err
 	}
+	if localPath == "" {
+		return usage("local file path is required")
+	}
 
-	content, err := os.ReadFile(localPath) //nolint:gosec // user-provided file path
+	content, err := os.ReadFile(localPath)
 	if err != nil {
 		return fmt.Errorf("read local file: %w", err)
 	}
@@ -211,3 +205,35 @@ func (c *OneDriveRmCmd) Run(ctx context.Context) error {
 	fmt.Fprintf(os.Stdout, "Deleted %s\n", c.Path)
 	return nil
 }
+
+func resolveOneDrivePutLocalPath(raw string) (string, error) {
+	cwd, err := os.Getwd()
+	if err != nil {
+		return "", fmt.Errorf("resolve current directory: %w", err)
+	}
+	return config.SafeExpandPath(cwd, strings.TrimSpace(raw))
+}
+
+func resolveOneDriveOutPath(remotePath string, outArg string) (string, error) {
+	outPath := strings.TrimSpace(outArg)
+	if outPath == "" {
+		downloadDir, err := config.EnsureOneDriveDownloadsDir()
+		if err != nil {
+			return "", err
+		}
+
+		fileName := filepath.Base(strings.TrimSpace(remotePath))
+		if fileName == "." || fileName == "/" || fileName == "" {
+			fileName = "download.bin"
+		}
+
+		return config.SafeExpandPath(downloadDir, fileName)
+	}
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		return "", fmt.Errorf("resolve current directory: %w", err)
+	}
+
+	return config.SafeExpandPath(cwd, outPath)
+}
diff --git a/internal/cmd/onedrive_test.go b/internal/cmd/onedrive_test.go
new file mode 100644
index 0000000..4617fc0
--- /dev/null
+++ b/internal/cmd/onedrive_test.go
@@ -0,0 +1,64 @@
+package cmd
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestResolveOneDrivePutLocalPathRejectsTraversal(t *testing.T) {
+	t.Parallel()
+
+	_, err := resolveOneDrivePutLocalPath("../secrets.txt")
+	if err == nil {
+		t.Fatal("expected traversal rejection")
+	}
+	if !strings.Contains(err.Error(), "path escapes base directory") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestResolveOneDrivePutLocalPathResolvesRelativePath(t *testing.T) {
+	t.Parallel()
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("os.Getwd failed: %v", err)
+	}
+
+	got, err := resolveOneDrivePutLocalPath("./fixtures/report.txt")
+	if err != nil {
+		t.Fatalf("resolveOneDrivePutLocalPath failed: %v", err)
+	}
+
+	want := filepath.Join(cwd, "fixtures", "report.txt")
+	if got != want {
+		t.Fatalf("unexpected resolved path: got %q want %q", got, want)
+	}
+}
+
+func TestResolveOneDriveOutPathDefaultsToDownloadsDir(t *testing.T) {
+	setTempUserConfigEnv(t)
+
+	got, err := resolveOneDriveOutPath("/Reports/report.pdf", "")
+	if err != nil {
+		t.Fatalf("resolveOneDriveOutPath failed: %v", err)
+	}
+
+	if !strings.HasSuffix(got, filepath.Join("mogcli", "onedrive-downloads", "report.pdf")) {
+		t.Fatalf("expected download path under mogcli downloads dir, got %q", got)
+	}
+}
+
+func TestResolveOneDriveOutPathRejectsTraversal(t *testing.T) {
+	t.Parallel()
+
+	_, err := resolveOneDriveOutPath("/Reports/report.pdf", "../outside.txt")
+	if err == nil {
+		t.Fatal("expected traversal rejection")
+	}
+	if !strings.Contains(err.Error(), "path escapes base directory") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
diff --git a/internal/config/paths.go b/internal/config/paths.go
index b9a3b8f..e3a13c0 100644
--- a/internal/config/paths.go
+++ b/internal/config/paths.go
@@ -143,3 +143,44 @@ func ExpandPath(path string) (string, error) {
 
 	return path, nil
 }
+
+// SafeExpandPath expands "~", normalizes the path, and blocks relative
+// traversal outside the provided base directory.
+// Absolute paths are treated as explicit user intent and are allowed.
+func SafeExpandPath(basePath string, userPath string) (string, error) {
+	expanded, err := ExpandPath(strings.TrimSpace(userPath))
+	if err != nil {
+		return "", err
+	}
+	if expanded == "" {
+		return "", nil
+	}
+
+	cleaned := filepath.Clean(expanded)
+	if filepath.IsAbs(cleaned) {
+		return cleaned, nil
+	}
+
+	absBase, err := filepath.Abs(strings.TrimSpace(basePath))
+	if err != nil {
+		return "", fmt.Errorf("resolve base path: %w", err)
+	}
+
+	resolved := filepath.Clean(filepath.Join(absBase, cleaned))
+	if escapesBasePath(absBase, resolved) {
+		return "", fmt.Errorf("path escapes base directory: %s", strings.TrimSpace(userPath))
+	}
+
+	return resolved, nil
+}
+
+func escapesBasePath(basePath string, targetPath string) bool {
+	rel, err := filepath.Rel(basePath, targetPath)
+	if err != nil {
+		return true
+	}
+	if rel == ".." {
+		return true
+	}
+	return strings.HasPrefix(rel, ".."+string(os.PathSeparator))
+}
diff --git a/internal/config/paths_test.go b/internal/config/paths_test.go
new file mode 100644
index 0000000..a00a424
--- /dev/null
+++ b/internal/config/paths_test.go
@@ -0,0 +1,68 @@
+package config
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestSafeExpandPathResolvesRelativePathsWithinBase(t *testing.T) {
+	t.Parallel()
+
+	base := t.TempDir()
+	got, err := SafeExpandPath(base, "docs/report.txt")
+	if err != nil {
+		t.Fatalf("SafeExpandPath failed: %v", err)
+	}
+
+	want := filepath.Join(base, "docs", "report.txt")
+	if got != want {
+		t.Fatalf("unexpected resolved path: got %q want %q", got, want)
+	}
+}
+
+func TestSafeExpandPathRejectsRelativeTraversal(t *testing.T) {
+	t.Parallel()
+
+	base := t.TempDir()
+	_, err := SafeExpandPath(base, "../secrets.txt")
+	if err == nil {
+		t.Fatal("expected traversal error")
+	}
+	if !strings.Contains(err.Error(), "path escapes base directory") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestSafeExpandPathAllowsAbsolutePath(t *testing.T) {
+	t.Parallel()
+
+	base := t.TempDir()
+	abs := filepath.Join(base, "..", "outside.txt")
+
+	got, err := SafeExpandPath(base, abs)
+	if err != nil {
+		t.Fatalf("SafeExpandPath failed for absolute path: %v", err)
+	}
+
+	want := filepath.Clean(abs)
+	if got != want {
+		t.Fatalf("unexpected cleaned absolute path: got %q want %q", got, want)
+	}
+}
+
+func TestSafeExpandPathExpandsTildeAndValidatesBase(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+
+	base := filepath.Join(home, "workspace")
+	got, err := SafeExpandPath(base, "~/notes.txt")
+	if err != nil {
+		t.Fatalf("SafeExpandPath failed for tilde path: %v", err)
+	}
+
+	want := filepath.Join(home, "notes.txt")
+	if got != want {
+		t.Fatalf("unexpected expanded tilde path: got %q want %q", got, want)
+	}
+}

From 0684e567ee87bb5209be6bd7c3c99df31df0c676 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:26:41 -0500
Subject: [PATCH 03/13] Refactor circuit breaker execution flow

---
 internal/graph/circuitbreaker.go      | 37 ++++++++++++-
 internal/graph/circuitbreaker_test.go | 79 +++++++++++++++++++++++++++
 internal/graph/client.go              | 52 ++++++++++++------
 internal/graph/client_test.go         | 34 ++++++++++++
 4 files changed, 183 insertions(+), 19 deletions(-)
 create mode 100644 internal/graph/circuitbreaker_test.go

diff --git a/internal/graph/circuitbreaker.go b/internal/graph/circuitbreaker.go
index e8a9d96..1c4f3c9 100644
--- a/internal/graph/circuitbreaker.go
+++ b/internal/graph/circuitbreaker.go
@@ -41,12 +41,47 @@ func (cb *CircuitBreaker) RecordFailure() {
 func (cb *CircuitBreaker) IsOpen() bool {
 	cb.mu.Lock()
 	defer cb.mu.Unlock()
+	return cb.isOpenLocked(time.Now())
+}
+
+// Execute atomically checks whether the breaker is open, runs fn, and then
+// records success/failure state.
+//
+// fn returns (recordFailure, err):
+//   - err == nil: records success and resets failures.
+//   - err != nil, recordFailure == true: records a breaker failure.
+//   - err != nil, recordFailure == false: returns err without changing breaker state.
+func (cb *CircuitBreaker) Execute(fn func() (bool, error)) error {
+	if fn == nil {
+		return nil
+	}
+
+	cb.mu.Lock()
+	if cb.isOpenLocked(time.Now()) {
+		cb.mu.Unlock()
+		return &CircuitBreakerError{}
+	}
+	cb.mu.Unlock()
+
+	recordFailure, err := fn()
+	if err == nil {
+		cb.RecordSuccess()
+		return nil
+	}
+	if !recordFailure {
+		return err
+	}
+
+	cb.RecordFailure()
+	return err
+}
 
+func (cb *CircuitBreaker) isOpenLocked(now time.Time) bool {
 	if !cb.open {
 		return false
 	}
 
-	if time.Since(cb.lastFailure) > CircuitBreakerResetTime {
+	if now.Sub(cb.lastFailure) > CircuitBreakerResetTime {
 		cb.open = false
 		cb.failures = 0
 		return false
diff --git a/internal/graph/circuitbreaker_test.go b/internal/graph/circuitbreaker_test.go
new file mode 100644
index 0000000..d4e67bc
--- /dev/null
+++ b/internal/graph/circuitbreaker_test.go
@@ -0,0 +1,79 @@
+package graph
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestCircuitBreakerExecuteReturnsOpenErrorWithoutRunningFn(t *testing.T) {
+	cb := NewCircuitBreaker()
+	for i := 0; i < CircuitBreakerThreshold; i++ {
+		cb.RecordFailure()
+	}
+
+	ran := false
+	err := cb.Execute(func() (bool, error) {
+		ran = true
+		return false, nil
+	})
+	if err == nil {
+		t.Fatal("expected circuit breaker error")
+	}
+
+	var breakerErr *CircuitBreakerError
+	if !errors.As(err, &breakerErr) {
+		t.Fatalf("expected CircuitBreakerError, got %T (%v)", err, err)
+	}
+	if ran {
+		t.Fatal("expected callback to be skipped while breaker is open")
+	}
+}
+
+func TestCircuitBreakerExecuteRecordsFailureWhenRequested(t *testing.T) {
+	cb := NewCircuitBreaker()
+
+	testErr := errors.New("server unavailable")
+	err := cb.Execute(func() (bool, error) {
+		return true, testErr
+	})
+	if !errors.Is(err, testErr) {
+		t.Fatalf("expected callback error, got %v", err)
+	}
+	if cb.failures != 1 {
+		t.Fatalf("expected one failure recorded, got %d", cb.failures)
+	}
+}
+
+func TestCircuitBreakerExecuteSkipsFailureRecordWhenNotRequested(t *testing.T) {
+	cb := NewCircuitBreaker()
+
+	testErr := errors.New("bad request")
+	err := cb.Execute(func() (bool, error) {
+		return false, testErr
+	})
+	if !errors.Is(err, testErr) {
+		t.Fatalf("expected callback error, got %v", err)
+	}
+	if cb.failures != 0 {
+		t.Fatalf("expected no failures recorded, got %d", cb.failures)
+	}
+}
+
+func TestCircuitBreakerExecuteResetsOnSuccess(t *testing.T) {
+	cb := NewCircuitBreaker()
+	cb.RecordFailure()
+	cb.RecordFailure()
+
+	if err := cb.Execute(func() (bool, error) {
+		return false, nil
+	}); err != nil {
+		t.Fatalf("Execute failed: %v", err)
+	}
+
+	if cb.failures != 0 {
+		t.Fatalf("expected failures reset to zero, got %d", cb.failures)
+	}
+	if cb.open {
+		t.Fatal("expected breaker to be closed after success")
+	}
+}
diff --git a/internal/graph/client.go b/internal/graph/client.go
index 7c36161..f93cdf3 100644
--- a/internal/graph/client.go
+++ b/internal/graph/client.go
@@ -80,61 +80,77 @@ func (c *Client) Do(ctx context.Context, method string, path string, query url.V
 		return nil, nil, errors.New("missing token provider")
 	}
 
-	if c.Breaker != nil && c.Breaker.IsOpen() {
-		return nil, nil, &CircuitBreakerError{}
+	if c.Breaker == nil {
+		resp, payload, _, err := c.doWithRetry(ctx, method, path, query, body, scopes, headers)
+		return resp, payload, err
+	}
+
+	var resp *http.Response
+	var payload []byte
+	err := c.Breaker.Execute(func() (bool, error) {
+		var recordFailure bool
+		var callErr error
+		resp, payload, recordFailure, callErr = c.doWithRetry(ctx, method, path, query, body, scopes, headers)
+		return recordFailure, callErr
+	})
+	if err != nil {
+		return resp, payload, err
 	}
 
+	return resp, payload, nil
+}
+
+func (c *Client) doWithRetry(
+	ctx context.Context,
+	method string,
+	path string,
+	query url.Values,
+	body any,
+	scopes []string,
+	headers http.Header,
+) (*http.Response, []byte, bool, error) {
 	retries429 := 0
 	retries5xx := 0
 
 	for {
 		resp, b, err := c.doOnce(ctx, method, path, query, body, scopes, headers)
 		if err != nil {
-			if c.Breaker != nil {
-				c.Breaker.RecordFailure()
-			}
-			return nil, nil, err
+			return nil, nil, true, err
 		}
 
 		if resp.StatusCode < 400 {
-			if c.Breaker != nil {
-				c.Breaker.RecordSuccess()
-			}
-			return resp, b, nil
+			return resp, b, false, nil
 		}
 
 		if resp.StatusCode == http.StatusTooManyRequests {
 			if retries429 >= c.MaxRetries429 {
 				apiErr := parseAPIError(resp.StatusCode, b)
-				return resp, b, apiErr
+				return resp, b, false, apiErr
 			}
 
 			delay := retryDelay(resp.Header.Get("Retry-After"), retries429, c.BaseDelay)
 			if err := sleepContext(ctx, delay); err != nil {
-				return nil, nil, err
+				return nil, nil, false, err
 			}
 			retries429++
 			continue
 		}
 
 		if resp.StatusCode >= 500 {
-			if c.Breaker != nil {
-				c.Breaker.RecordFailure()
-			}
 			if retries5xx >= c.MaxRetries5xx {
 				apiErr := parseAPIError(resp.StatusCode, b)
-				return resp, b, apiErr
+				return resp, b, true, apiErr
 			}
 
 			if err := sleepContext(ctx, ServerErrorRetryDelay); err != nil {
-				return nil, nil, err
+				return nil, nil, false, err
 			}
 			retries5xx++
 			continue
 		}
 
 		apiErr := parseAPIError(resp.StatusCode, b)
-		return resp, b, apiErr
+		return resp, b, false, apiErr
 	}
 }
 
diff --git a/internal/graph/client_test.go b/internal/graph/client_test.go
index 590ebea..5b35d74 100644
--- a/internal/graph/client_test.go
+++ b/internal/graph/client_test.go
@@ -1,6 +1,9 @@
 package graph
 
 import (
+	"context"
+	"errors"
+	"net/http"
 	"strings"
 	"testing"
 	"time"
@@ -41,3 +44,34 @@ func TestResolveURLRejectsCrossHostAbsoluteURL(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
+
+func TestDoShortCircuitsWhenCircuitBreakerOpen(t *testing.T) {
+	cb := NewCircuitBreaker()
+	for i := 0; i < CircuitBreakerThreshold; i++ {
+		cb.RecordFailure()
+	}
+
+	tokenCalls := 0
+	c := &Client{
+		BaseURL: "https://graph.microsoft.com/v1.0",
+		TokenProvider: func(context.Context, []string) (string, error) {
+			tokenCalls++
+			return "token", nil
+		},
+		HTTPClient: http.DefaultClient,
+		Breaker:    cb,
+	}
+
+	_, _, err := c.Do(context.Background(), http.MethodGet, "/me", nil, nil, nil, nil)
+	if err == nil {
+		t.Fatal("expected circuit breaker open error")
+	}
+
+	var breakerErr *CircuitBreakerError
+	if !errors.As(err, &breakerErr) {
+		t.Fatalf("expected CircuitBreakerError, got %T (%v)", err, err)
+	}
+	if tokenCalls != 0 {
+		t.Fatalf("expected token provider not to be called, got %d calls", tokenCalls)
+	}
+}

From 4a406f32eb1ee073b3344aa4b1d8c90a620e9cf4 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:27:06 -0500
Subject: [PATCH 04/13] Validate pagination hosts against Graph allow list

---
 internal/cmd/pagination.go      | 16 ++++++++++++++++
 internal/cmd/pagination_test.go |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/internal/cmd/pagination.go b/internal/cmd/pagination.go
index f4f7ffc..cb5308d 100644
--- a/internal/cmd/pagination.go
+++ b/internal/cmd/pagination.go
@@ -5,6 +5,14 @@ import (
 	"strings"
 )
 
+var allowedGraphPageHosts = map[string]struct{}{
+	"graph.microsoft.com":             {},
+	"graph.microsoft.us":              {},
+	"dod-graph.microsoft.us":          {},
+	"graph.microsoft.de":              {},
+	"microsoftgraph.chinacloudapi.cn": {},
+}
+
 func normalizePageToken(raw string) (string, error) {
 	page := strings.TrimSpace(raw)
 	if page == "" {
@@ -21,6 +29,14 @@ func normalizePageToken(raw string) (string, error) {
 	if u.Scheme != "http" && u.Scheme != "https" {
 		return "", usage("--page must be an absolute http(s) URL")
 	}
+	if !isAllowedGraphPageHost(u.Hostname()) {
+		return "", usage("--page URL must use a Microsoft Graph host")
+	}
 
 	return u.String(), nil
 }
+
+func isAllowedGraphPageHost(host string) bool {
+	_, ok := allowedGraphPageHosts[strings.ToLower(strings.TrimSpace(host))]
+	return ok
+}
diff --git a/internal/cmd/pagination_test.go b/internal/cmd/pagination_test.go
index ec40bcb..752dc9b 100644
--- a/internal/cmd/pagination_test.go
+++ b/internal/cmd/pagination_test.go
@@ -7,6 +7,7 @@ import (
 
 func TestNormalizePageToken(t *testing.T) {
 	valid := "https://graph.microsoft.com/v1.0/me/messages?$skiptoken=abc"
+	validGov := "https://graph.microsoft.us/v1.0/me/messages?$skiptoken=abc"
 
 	tests := []struct {
 		name    string
@@ -17,8 +18,10 @@ func TestNormalizePageToken(t *testing.T) {
 		{name: "empty", input: "", want: ""},
 		{name: "trim empty", input: "   ", want: ""},
 		{name: "absolute url", input: valid, want: valid},
+		{name: "absolute gov url", input: validGov, want: validGov},
 		{name: "relative url", input: "/me/messages?$skiptoken=abc", wantErr: true},
 		{name: "unsupported scheme", input: "ftp://graph.microsoft.com/v1.0/me/messages", wantErr: true},
+		{name: "untrusted host", input: "https://evil.example/v1.0/me/messages?$skiptoken=abc", wantErr: true},
 	}
 
 	for _, tc := range tests {

From 1170217e1f0739df91e559bc933aa758d417eb60 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:28:19 -0500
Subject: [PATCH 05/13] Validate profile names for cache and secret keys

---
 internal/auth/manager.go       | 66 +++++++++++++++++++++++++++-------
 internal/auth/manager_test.go  |  6 ++++
 internal/profile/store.go      |  8 +++++
 internal/profile/store_test.go | 18 ++++++++++
 4 files changed, 86 insertions(+), 12 deletions(-)

diff --git a/internal/auth/manager.go b/internal/auth/manager.go
index d3490b2..537e9ce 100644
--- a/internal/auth/manager.go
+++ b/internal/auth/manager.go
@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/jaredpalmer/mogcli/internal/errfmt"
+	"github.com/jaredpalmer/mogcli/internal/profile"
 	"github.com/jaredpalmer/mogcli/internal/secrets"
 )
 
@@ -52,12 +53,20 @@ func (m *Manager) nowUTC() time.Time {
 	return time.Now().UTC()
 }
 
-func tokenCacheKey(profileName string) string {
-	return "mog:cache:" + strings.TrimSpace(profileName)
+func tokenCacheKey(profileName string) (string, error) {
+	normalized, err := profile.NormalizeName(profileName)
+	if err != nil {
+		return "", fmt.Errorf("invalid profile name: %w", err)
+	}
+	return "mog:cache:" + normalized, nil
 }
 
-func appSecretKey(profileName string) string {
-	return "mog:appsecret:" + strings.TrimSpace(profileName)
+func appSecretKey(profileName string) (string, error) {
+	normalized, err := profile.NormalizeName(profileName)
+	if err != nil {
+		return "", fmt.Errorf("invalid profile name: %w", err)
+	}
+	return "mog:appsecret:" + normalized, nil
 }
 
 func graphDefaultScope() string {
@@ -230,11 +239,16 @@ func (m *Manager) LoginAppOnly(ctx context.Context, input AppOnlyLoginInput) err
 		return ErrMissingSecret
 	}
 
-	if err := secrets.SetSecret(appSecretKey(input.ProfileName), []byte(input.Secret)); err != nil {
+	secretKey, err := appSecretKey(input.ProfileName)
+	if err != nil {
+		return err
+	}
+
+	if err := secrets.SetSecret(secretKey, []byte(input.Secret)); err != nil {
 		return fmt.Errorf("store client secret: %w", err)
 	}
 
-	_, err := m.AcquireAppOnlyToken(ctx, input.ProfileName, input.ClientID, input.Authority)
+	_, err = m.AcquireAppOnlyToken(ctx, input.ProfileName, input.ClientID, input.Authority)
 	return err
 }
 
@@ -321,7 +335,12 @@ func (m *Manager) AcquireDelegatedToken(
 }
 
 func (m *Manager) AcquireAppOnlyToken(ctx context.Context, profileName string, clientID string, authority string) (string, error) {
-	secret, err := secrets.GetSecret(appSecretKey(profileName))
+	secretKey, err := appSecretKey(profileName)
+	if err != nil {
+		return "", err
+	}
+
+	secret, err := secrets.GetSecret(secretKey)
 	if err != nil {
 		return "", fmt.Errorf("read client secret: %w", err)
 	}
@@ -369,10 +388,19 @@ func (m *Manager) ReadAccount(profileName string) (AccountInfo, error) {
 }
 
 func (m *Manager) Logout(profileName string) error {
-	if err := secrets.DeleteSecret(tokenCacheKey(profileName)); err != nil {
+	cacheKey, err := tokenCacheKey(profileName)
+	if err != nil {
 		return err
 	}
-	if err := secrets.DeleteSecret(appSecretKey(profileName)); err != nil {
+	if err := secrets.DeleteSecret(cacheKey); err != nil {
+		return err
+	}
+
+	secretKey, err := appSecretKey(profileName)
+	if err != nil {
+		return err
+	}
+	if err := secrets.DeleteSecret(secretKey); err != nil {
 		return err
 	}
 
@@ -506,7 +534,11 @@ func looksLikeTenantDomain(value string) bool {
 }
 
 func (m *Manager) purgeDelegatedTokenCache(profileName string) error {
-	if err := secrets.DeleteSecret(tokenCacheKey(profileName)); err != nil {
+	cacheKey, err := tokenCacheKey(profileName)
+	if err != nil {
+		return err
+	}
+	if err := secrets.DeleteSecret(cacheKey); err != nil {
 		return fmt.Errorf("purge delegated token cache: %w", err)
 	}
 	return nil
@@ -518,7 +550,12 @@ func (m *Manager) saveToken(profileName string, cache TokenCache) error {
 		return fmt.Errorf("encode token cache: %w", err)
 	}
 
-	if err := secrets.SetSecret(tokenCacheKey(profileName), payload); err != nil {
+	cacheKey, err := tokenCacheKey(profileName)
+	if err != nil {
+		return err
+	}
+
+	if err := secrets.SetSecret(cacheKey, payload); err != nil {
 		return fmt.Errorf("store token cache: %w", err)
 	}
 
@@ -526,7 +563,12 @@ func (m *Manager) saveToken(profileName string, cache TokenCache) error {
 }
 
 func (m *Manager) loadToken(profileName string) (TokenCache, error) {
-	payload, err := secrets.GetSecret(tokenCacheKey(profileName))
+	cacheKey, err := tokenCacheKey(profileName)
+	if err != nil {
+		return TokenCache{}, err
+	}
+
+	payload, err := secrets.GetSecret(cacheKey)
 	if err != nil {
 		return TokenCache{}, fmt.Errorf("read token cache: %w", err)
 	}
diff --git a/internal/auth/manager_test.go b/internal/auth/manager_test.go
index 7fb0cef..54f2713 100644
--- a/internal/auth/manager_test.go
+++ b/internal/auth/manager_test.go
@@ -280,3 +280,9 @@ func idTokenFor(t *testing.T, accountID string, tenantID string) string {
 	payload := base64.RawURLEncoding.EncodeToString(payloadBytes)
 	return header + "." + payload + "."
 }
+
+func TestTokenCacheKeyRejectsInvalidProfileName(t *testing.T) {
+	if _, err := tokenCacheKey("../bad-profile"); err == nil {
+		t.Fatal("expected invalid profile name error")
+	}
+}
diff --git a/internal/profile/store.go b/internal/profile/store.go
index ead7f73..ddc5c20 100644
--- a/internal/profile/store.go
+++ b/internal/profile/store.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"unicode"
 
 	"github.com/jaredpalmer/mogcli/internal/config"
 )
@@ -36,6 +37,13 @@ func NormalizeName(name string) (string, error) {
 		return "", ErrInvalidName
 	}
 
+	for _, r := range normalized {
+		if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' || r == '_' {
+			continue
+		}
+		return "", fmt.Errorf("%w: invalid character %q", ErrInvalidName, r)
+	}
+
 	return normalized, nil
 }
 
diff --git a/internal/profile/store_test.go b/internal/profile/store_test.go
index d5ee0a0..8288148 100644
--- a/internal/profile/store_test.go
+++ b/internal/profile/store_test.go
@@ -17,6 +17,24 @@ func TestNormalizeName(t *testing.T) {
 	if name != "work" {
 		t.Fatalf("unexpected normalized name: %q", name)
 	}
+
+	invalid := []string{"work profile", "../work", "work/profile", "work.profile"}
+	for _, value := range invalid {
+		if _, err := NormalizeName(value); err == nil {
+			t.Fatalf("expected invalid profile name error for %q", value)
+		}
+	}
+
+	allowed := []string{"work", "work_1", "Work-Prod"}
+	for _, value := range allowed {
+		got, err := NormalizeName(value)
+		if err != nil {
+			t.Fatalf("expected %q to be valid, got %v", value, err)
+		}
+		if got != value {
+			t.Fatalf("expected normalized name %q, got %q", value, got)
+		}
+	}
 }
 
 func TestValidateRecord(t *testing.T) {

From ed4d5b3ec6375b7b412a9c9224d737b66126846c Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:29:14 -0500
Subject: [PATCH 06/13] Serialize config reads and writes with lockfile

---
 internal/config/config.go      | 114 ++++++++++++++++++---------
 internal/config/config_test.go | 137 +++++++++++++++++++++++++++++++++
 2 files changed, 216 insertions(+), 35 deletions(-)
 create mode 100644 internal/config/config_test.go

diff --git a/internal/config/config.go b/internal/config/config.go
index f7d1190..06ffddf 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"time"
 )
 
 type ProfileRecord struct {
@@ -31,6 +32,11 @@ type File struct {
 	ActiveProfile   string                   `json:"active_profile,omitempty"`
 }
 
+var (
+	configLockRetryInterval = 10 * time.Millisecond
+	configLockTimeout       = 5 * time.Second
+)
+
 func ConfigPath() (string, error) {
 	dir, err := Dir()
 	if err != nil {
@@ -41,33 +47,30 @@ func ConfigPath() (string, error) {
 }
 
 func WriteConfig(cfg File) error {
-	_, err := EnsureDir()
-	if err != nil {
-		return fmt.Errorf("ensure config dir: %w", err)
-	}
-
-	path, err := ConfigPath()
-	if err != nil {
-		return err
-	}
+	return withConfigLock(func() error {
+		path, err := ConfigPath()
+		if err != nil {
+			return err
+		}
 
-	b, err := json.MarshalIndent(cfg, "", "  ")
-	if err != nil {
-		return fmt.Errorf("encode config json: %w", err)
-	}
+		b, err := json.MarshalIndent(cfg, "", "  ")
+		if err != nil {
+			return fmt.Errorf("encode config json: %w", err)
+		}
 
-	b = append(b, '\n')
-	tmp := path + ".tmp"
+		b = append(b, '\n')
+		tmp := path + ".tmp"
 
-	if err := os.WriteFile(tmp, b, 0o600); err != nil {
-		return fmt.Errorf("write config: %w", err)
-	}
+		if err := os.WriteFile(tmp, b, 0o600); err != nil {
+			return fmt.Errorf("write config: %w", err)
+		}
 
-	if err := os.Rename(tmp, path); err != nil {
-		return fmt.Errorf("commit config: %w", err)
-	}
+		if err := os.Rename(tmp, path); err != nil {
+			return fmt.Errorf("commit config: %w", err)
+		}
 
-	return nil
+		return nil
+	})
 }
 
 func ConfigExists() (bool, error) {
@@ -88,23 +91,31 @@ func ConfigExists() (bool, error) {
 }
 
 func ReadConfig() (File, error) {
-	path, err := ConfigPath()
-	if err != nil {
-		return File{}, err
-	}
+	var cfg File
+	err := withConfigLock(func() error {
+		path, pathErr := ConfigPath()
+		if pathErr != nil {
+			return pathErr
+		}
 
-	b, err := os.ReadFile(path) //nolint:gosec // config file path
-	if err != nil {
-		if os.IsNotExist(err) {
-			return File{}, nil
+		b, readErr := os.ReadFile(path) //nolint:gosec // config file path
+		if readErr != nil {
+			if os.IsNotExist(readErr) {
+				cfg = File{}
+				return nil
+			}
+
+			return fmt.Errorf("read config: %w", readErr)
 		}
 
-		return File{}, fmt.Errorf("read config: %w", err)
-	}
+		if unmarshalErr := json.Unmarshal(b, &cfg); unmarshalErr != nil {
+			return fmt.Errorf("parse config %s: %w", path, unmarshalErr)
+		}
 
-	var cfg File
-	if err := json.Unmarshal(b, &cfg); err != nil {
-		return File{}, fmt.Errorf("parse config %s: %w", path, err)
+		return nil
+	})
+	if err != nil {
+		return File{}, err
 	}
 
 	if cfg.Profiles == nil {
@@ -113,3 +124,36 @@ func ReadConfig() (File, error) {
 
 	return cfg, nil
 }
+
+func withConfigLock(fn func() error) error {
+	if fn == nil {
+		return nil
+	}
+
+	dir, err := EnsureDir()
+	if err != nil {
+		return fmt.Errorf("ensure config dir: %w", err)
+	}
+	lockPath := filepath.Join(dir, "config.lock")
+	deadline := time.Now().Add(configLockTimeout)
+
+	for {
+		lockFile, openErr := os.OpenFile(lockPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)
+		if openErr == nil {
+			defer func() {
+				_ = lockFile.Close()
+				_ = os.Remove(lockPath)
+			}()
+			return fn()
+		}
+
+		if !os.IsExist(openErr) {
+			return fmt.Errorf("acquire config lock: %w", openErr)
+		}
+		if time.Now().After(deadline) {
+			return fmt.Errorf("acquire config lock timeout: %s", lockPath)
+		}
+
+		time.Sleep(configLockRetryInterval)
+	}
+}
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
new file mode 100644
index 0000000..a13d327
--- /dev/null
+++ b/internal/config/config_test.go
@@ -0,0 +1,137 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func setTempConfigEnv(t *testing.T) {
+	t.Helper()
+
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
+}
+
+func TestWriteAndReadConfigRoundTrip(t *testing.T) {
+	setTempConfigEnv(t)
+
+	input := File{
+		ActiveProfile: "work",
+		Profiles: map[string]ProfileRecord{
+			"work": {
+				Name:     "work",
+				Audience: "enterprise",
+				ClientID: "client-id",
+			},
+		},
+	}
+
+	if err := WriteConfig(input); err != nil {
+		t.Fatalf("WriteConfig failed: %v", err)
+	}
+
+	got, err := ReadConfig()
+	if err != nil {
+		t.Fatalf("ReadConfig failed: %v", err)
+	}
+
+	if got.ActiveProfile != input.ActiveProfile {
+		t.Fatalf("unexpected active profile: got %q want %q", got.ActiveProfile, input.ActiveProfile)
+	}
+	if _, ok := got.Profiles["work"]; !ok {
+		t.Fatalf("expected profile \"work\" to exist")
+	}
+}
+
+func TestWithConfigLockSerializesCallers(t *testing.T) {
+	setTempConfigEnv(t)
+
+	prevTimeout := configLockTimeout
+	prevRetry := configLockRetryInterval
+	configLockTimeout = 2 * time.Second
+	configLockRetryInterval = 5 * time.Millisecond
+	t.Cleanup(func() {
+		configLockTimeout = prevTimeout
+		configLockRetryInterval = prevRetry
+	})
+
+	started := make(chan struct{})
+	release := make(chan struct{})
+	firstDone := make(chan error, 1)
+	secondEntered := make(chan struct{})
+	secondDone := make(chan error, 1)
+
+	go func() {
+		firstDone <- withConfigLock(func() error {
+			close(started)
+			<-release
+			return nil
+		})
+	}()
+
+	<-started
+
+	go func() {
+		secondDone <- withConfigLock(func() error {
+			close(secondEntered)
+			return nil
+		})
+	}()
+
+	select {
+	case <-secondEntered:
+		t.Fatal("expected second caller to wait for lock release")
+	case <-time.After(100 * time.Millisecond):
+	}
+
+	close(release)
+
+	if err := <-firstDone; err != nil {
+		t.Fatalf("first lock holder failed: %v", err)
+	}
+
+	select {
+	case <-secondEntered:
+	case <-time.After(2 * time.Second):
+		t.Fatal("second caller did not enter lock after release")
+	}
+
+	if err := <-secondDone; err != nil {
+		t.Fatalf("second lock holder failed: %v", err)
+	}
+}
+
+func TestWithConfigLockTimesOut(t *testing.T) {
+	setTempConfigEnv(t)
+
+	dir, err := EnsureDir()
+	if err != nil {
+		t.Fatalf("EnsureDir failed: %v", err)
+	}
+	lockPath := filepath.Join(dir, "config.lock")
+	if err := os.WriteFile(lockPath, []byte("locked"), 0o600); err != nil {
+		t.Fatalf("write lock file: %v", err)
+	}
+	t.Cleanup(func() { _ = os.Remove(lockPath) })
+
+	prevTimeout := configLockTimeout
+	prevRetry := configLockRetryInterval
+	configLockTimeout = 50 * time.Millisecond
+	configLockRetryInterval = 10 * time.Millisecond
+	t.Cleanup(func() {
+		configLockTimeout = prevTimeout
+		configLockRetryInterval = prevRetry
+	})
+
+	err = withConfigLock(func() error { return nil })
+	if err == nil {
+		t.Fatal("expected lock timeout error")
+	}
+	if !strings.Contains(err.Error(), "acquire config lock timeout") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}

From 29fdebea4c2b4eb1e1741e7b7958e63fc9a53f9e Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:31:47 -0500
Subject: [PATCH 07/13] Add OneDrive dry-run, progress, and delete metadata

---
 internal/cmd/onedrive.go                   | 201 ++++++++++++++++++++-
 internal/cmd/onedrive_test.go              |  42 +++++
 internal/services/onedrive/service.go      |   6 +
 internal/services/onedrive/service_test.go |   5 +
 4 files changed, 245 insertions(+), 9 deletions(-)

diff --git a/internal/cmd/onedrive.go b/internal/cmd/onedrive.go
index ebe122b..ec45e85 100644
--- a/internal/cmd/onedrive.go
+++ b/internal/cmd/onedrive.go
@@ -1,8 +1,10 @@
 package cmd
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -88,7 +90,7 @@ func (c *OneDriveGetCmd) Run(ctx context.Context) error {
 	if err := os.MkdirAll(filepath.Dir(expanded), 0o755); err != nil {
 		return fmt.Errorf("create output directory: %w", err)
 	}
-	if err := os.WriteFile(expanded, content, 0o600); err != nil {
+	if err := writeLocalFileWithProgress(ctx, expanded, content); err != nil {
 		return fmt.Errorf("write file: %w", err)
 	}
 
@@ -104,6 +106,7 @@ type OneDrivePutCmd struct {
 	LocalPath  string `arg:"" required:"" help:"Local file path"`
 	RemotePath string `name:"path" required:"" help:"Remote destination path"`
 	User       string `name:"user" help:"App-only target user override (UPN or user ID)"`
+	DryRun     bool   `name:"dry-run" help:"Preview upload without modifying OneDrive"`
 }
 
 func (c *OneDrivePutCmd) Run(ctx context.Context) error {
@@ -124,7 +127,27 @@ func (c *OneDrivePutCmd) Run(ctx context.Context) error {
 		return usage("local file path is required")
 	}
 
-	content, err := os.ReadFile(localPath)
+	info, err := os.Stat(localPath)
+	if err != nil {
+		return fmt.Errorf("stat local file: %w", err)
+	}
+	size := info.Size()
+
+	if c.DryRun {
+		if outfmt.IsJSON(ctx) {
+			return outfmt.WriteJSON(os.Stdout, map[string]any{
+				"dry_run":     true,
+				"action":      "onedrive.put",
+				"local_path":  localPath,
+				"remote_path": c.RemotePath,
+				"bytes":       size,
+			})
+		}
+		fmt.Fprintf(os.Stdout, "Dry run: would upload %s (%d bytes) -> %s\n", localPath, size, c.RemotePath)
+		return nil
+	}
+
+	content, err := readLocalFileWithProgress(ctx, localPath)
 	if err != nil {
 		return fmt.Errorf("read local file: %w", err)
 	}
@@ -143,8 +166,9 @@ func (c *OneDrivePutCmd) Run(ctx context.Context) error {
 }
 
 type OneDriveMkdirCmd struct {
-	Path string `name:"path" required:"" help:"Remote folder path"`
-	User string `name:"user" help:"App-only target user override (UPN or user ID)"`
+	Path   string `name:"path" required:"" help:"Remote folder path"`
+	User   string `name:"user" help:"App-only target user override (UPN or user ID)"`
+	DryRun bool   `name:"dry-run" help:"Preview create without modifying OneDrive"`
 }
 
 func (c *OneDriveMkdirCmd) Run(ctx context.Context) error {
@@ -156,6 +180,17 @@ func (c *OneDriveMkdirCmd) Run(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
+	if c.DryRun {
+		if outfmt.IsJSON(ctx) {
+			return outfmt.WriteJSON(os.Stdout, map[string]any{
+				"dry_run": true,
+				"action":  "onedrive.mkdir",
+				"path":    c.Path,
+			})
+		}
+		fmt.Fprintf(os.Stdout, "Dry run: would create folder %s\n", c.Path)
+		return nil
+	}
 
 	svc := onedrive.New(rt.Graph, targetUser)
 	if err := svc.Mkdir(ctx, c.Path); err != nil {
@@ -171,8 +206,9 @@ func (c *OneDriveMkdirCmd) Run(ctx context.Context) error {
 }
 
 type OneDriveRmCmd struct {
-	Path string `name:"path" required:"" help:"Remote path to delete"`
-	User string `name:"user" help:"App-only target user override (UPN or user ID)"`
+	Path   string `name:"path" required:"" help:"Remote path to delete"`
+	User   string `name:"user" help:"App-only target user override (UPN or user ID)"`
+	DryRun bool   `name:"dry-run" help:"Preview delete without modifying OneDrive"`
 }
 
 func (c *OneDriveRmCmd) Run(ctx context.Context) error {
@@ -180,9 +216,6 @@ func (c *OneDriveRmCmd) Run(ctx context.Context) error {
 	if flags == nil {
 		flags = &RootFlags{}
 	}
-	if err := confirmDestructive(ctx, flags, fmt.Sprintf("delete %s", c.Path)); err != nil {
-		return err
-	}
 
 	rt, err := resolveRuntime(ctx, capOneDriveRM)
 	if err != nil {
@@ -194,6 +227,26 @@ func (c *OneDriveRmCmd) Run(ctx context.Context) error {
 	}
 
 	svc := onedrive.New(rt.Graph, targetUser)
+	if c.DryRun {
+		if outfmt.IsJSON(ctx) {
+			return outfmt.WriteJSON(os.Stdout, map[string]any{
+				"dry_run": true,
+				"action":  "onedrive.rm",
+				"path":    c.Path,
+			})
+		}
+		fmt.Fprintf(os.Stdout, "Dry run: would delete %s\n", c.Path)
+		return nil
+	}
+
+	action := fmt.Sprintf("delete %s", c.Path)
+	if metadata, statErr := svc.Stat(ctx, c.Path); statErr == nil {
+		action = describeOneDriveDeleteAction(c.Path, metadata)
+	}
+	if err := confirmDestructive(ctx, flags, action); err != nil {
+		return err
+	}
+
 	if err := svc.Remove(ctx, c.Path); err != nil {
 		return err
 	}
@@ -237,3 +290,133 @@ func resolveOneDriveOutPath(remotePath string, outArg string) (string, error) {
 
 	return config.SafeExpandPath(cwd, outPath)
 }
+
+func readLocalFileWithProgress(ctx context.Context, path string) ([]byte, error) {
+	file, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer file.Close()
+
+	info, err := file.Stat()
+	if err != nil {
+		return nil, err
+	}
+
+	var content bytes.Buffer
+	if _, err := copyWithProgress(ctx, &content, file, info.Size(), "Reading"); err != nil {
+		return nil, err
+	}
+
+	return content.Bytes(), nil
+}
+
+func writeLocalFileWithProgress(ctx context.Context, path string, content []byte) error {
+	file, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600)
+	if err != nil {
+		return err
+	}
+	defer file.Close()
+
+	_, err = copyWithProgress(ctx, file, bytes.NewReader(content), int64(len(content)), "Writing")
+	return err
+}
+
+func copyWithProgress(ctx context.Context, dst io.Writer, src io.Reader, total int64, label string) (int64, error) {
+	const reportEvery = int64(1 << 20) // 1 MiB
+
+	buffer := make([]byte, 32*1024)
+	var copied int64
+	nextReport := reportEvery
+
+	for {
+		n, readErr := src.Read(buffer)
+		if n > 0 {
+			written, writeErr := dst.Write(buffer[:n])
+			if writeErr != nil {
+				return copied, writeErr
+			}
+			if written != n {
+				return copied, io.ErrShortWrite
+			}
+
+			copied += int64(written)
+			if copied >= nextReport || (total > 0 && copied == total) {
+				reportOneDriveProgress(ctx, label, copied, total)
+				for copied >= nextReport {
+					nextReport += reportEvery
+				}
+			}
+		}
+
+		if readErr == io.EOF {
+			break
+		}
+		if readErr != nil {
+			return copied, readErr
+		}
+	}
+
+	if copied == 0 {
+		reportOneDriveProgress(ctx, label, copied, total)
+	}
+
+	return copied, nil
+}
+
+func reportOneDriveProgress(ctx context.Context, label string, copied int64, total int64) {
+	u := uiFromContext(ctx)
+	if u == nil {
+		return
+	}
+
+	if total > 0 {
+		percent := float64(copied) / float64(total) * 100
+		u.Err().Printf("%s: %d/%d bytes (%.0f%%)", label, copied, total, percent)
+		return
+	}
+
+	u.Err().Printf("%s: %d bytes", label, copied)
+}
+
+func describeOneDriveDeleteAction(path string, metadata map[string]any) string {
+	action := fmt.Sprintf("delete %s", strings.TrimSpace(path))
+	if len(metadata) == 0 {
+		return action
+	}
+
+	details := make([]string, 0, 4)
+	if _, ok := metadata["folder"].(map[string]any); ok {
+		details = append(details, "folder", "recursive")
+	} else {
+		details = append(details, "file")
+	}
+
+	if size, ok := oneDriveInt64(metadata["size"]); ok {
+		details = append(details, fmt.Sprintf("%d bytes", size))
+	}
+	if modified, ok := metadata["lastModifiedDateTime"].(string); ok && strings.TrimSpace(modified) != "" {
+		details = append(details, "modified "+strings.TrimSpace(modified))
+	}
+
+	if len(details) == 0 {
+		return action
+	}
+
+	return fmt.Sprintf("%s (%s)", action, strings.Join(details, ", "))
+}
+
+func oneDriveInt64(value any) (int64, bool) {
+	switch typed := value.(type) {
+	case int:
+		return int64(typed), true
+	case int32:
+		return int64(typed), true
+	case int64:
+		return typed, true
+	case float64:
+		return int64(typed), true
+	default:
+		return 0, false
+	}
+}
diff --git a/internal/cmd/onedrive_test.go b/internal/cmd/onedrive_test.go
index 4617fc0..2533c69 100644
--- a/internal/cmd/onedrive_test.go
+++ b/internal/cmd/onedrive_test.go
@@ -1,6 +1,8 @@
 package cmd
 
 import (
+	"bytes"
+	"context"
 	"os"
 	"path/filepath"
 	"strings"
@@ -62,3 +64,43 @@ func TestResolveOneDriveOutPathRejectsTraversal(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
+
+func TestDescribeOneDriveDeleteActionIncludesMetadata(t *testing.T) {
+	t.Parallel()
+
+	action := describeOneDriveDeleteAction("/Reports", map[string]any{
+		"folder":               map[string]any{},
+		"size":                 float64(2048),
+		"lastModifiedDateTime": "2026-02-13T12:00:00Z",
+	})
+	if !strings.Contains(action, "folder") {
+		t.Fatalf("expected folder detail, got %q", action)
+	}
+	if !strings.Contains(action, "recursive") {
+		t.Fatalf("expected recursive warning, got %q", action)
+	}
+	if !strings.Contains(action, "2048 bytes") {
+		t.Fatalf("expected size detail, got %q", action)
+	}
+	if !strings.Contains(action, "modified 2026-02-13T12:00:00Z") {
+		t.Fatalf("expected modified timestamp, got %q", action)
+	}
+}
+
+func TestCopyWithProgressCopiesPayload(t *testing.T) {
+	t.Parallel()
+
+	payload := "hello world"
+	var dst bytes.Buffer
+
+	n, err := copyWithProgress(context.Background(), &dst, strings.NewReader(payload), int64(len(payload)), "Writing")
+	if err != nil {
+		t.Fatalf("copyWithProgress failed: %v", err)
+	}
+	if n != int64(len(payload)) {
+		t.Fatalf("unexpected byte count: got %d want %d", n, len(payload))
+	}
+	if dst.String() != payload {
+		t.Fatalf("unexpected copied payload: got %q want %q", dst.String(), payload)
+	}
+}
diff --git a/internal/services/onedrive/service.go b/internal/services/onedrive/service.go
index 9988b89..a4888f3 100644
--- a/internal/services/onedrive/service.go
+++ b/internal/services/onedrive/service.go
@@ -55,6 +55,12 @@ func (s *Service) Get(ctx context.Context, remotePath string) ([]byte, error) {
 	return body, nil
 }
 
+func (s *Service) Stat(ctx context.Context, remotePath string) (map[string]any, error) {
+	var payload map[string]any
+	err := s.client.DoJSON(ctx, http.MethodGet, s.driveEndpoint()+"/root:"+normalizeRemotePath(remotePath), nil, nil, getOneDriveScopes, &payload)
+	return payload, err
+}
+
 func (s *Service) Put(ctx context.Context, remotePath string, content []byte) error {
 	endpoint := s.driveEndpoint() + "/root:" + normalizeRemotePath(remotePath) + ":/content"
 	_, _, err := s.client.Do(ctx, http.MethodPut, endpoint, nil, content, putOneDriveScopes, nil)
diff --git a/internal/services/onedrive/service_test.go b/internal/services/onedrive/service_test.go
index 5793fe8..ad62a78 100644
--- a/internal/services/onedrive/service_test.go
+++ b/internal/services/onedrive/service_test.go
@@ -80,6 +80,8 @@ func TestEndpointsRouteByAuthMode(t *testing.T) {
 					_, _ = fmt.Fprint(w, `{"value":[]}`)
 				case r.Method == http.MethodGet && r.URL.Path == tc.basePrefix+"/root:/docs/report.txt:/content":
 					_, _ = fmt.Fprint(w, `hello`)
+				case r.Method == http.MethodGet && r.URL.Path == tc.basePrefix+"/root:/docs/report.txt":
+					_, _ = fmt.Fprint(w, `{"id":"item-id","size":5}`)
 				case r.Method == http.MethodPut && r.URL.Path == tc.basePrefix+"/root:/docs/report.txt:/content":
 					w.WriteHeader(http.StatusCreated)
 				case r.Method == http.MethodPost && r.URL.Path == tc.basePrefix+"/root:/projects:/children":
@@ -103,6 +105,9 @@ func TestEndpointsRouteByAuthMode(t *testing.T) {
 			if _, err := svc.Get(context.Background(), "/docs/report.txt"); err != nil {
 				t.Fatalf("Get failed: %v", err)
 			}
+			if _, err := svc.Stat(context.Background(), "/docs/report.txt"); err != nil {
+				t.Fatalf("Stat failed: %v", err)
+			}
 			if err := svc.Put(context.Background(), "/docs/report.txt", []byte("hello")); err != nil {
 				t.Fatalf("Put failed: %v", err)
 			}

From 8b10277063077edcef68d7ec18bd2be386de60c0 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:32:40 -0500
Subject: [PATCH 08/13] Add dry-run support for mail and calendar writes

---
 internal/cmd/calendar.go           | 48 ++++++++++++++++++---
 internal/cmd/dry_run_flags_test.go | 69 ++++++++++++++++++++++++++++++
 internal/cmd/mail.go               | 14 ++++++
 3 files changed, 126 insertions(+), 5 deletions(-)
 create mode 100644 internal/cmd/dry_run_flags_test.go

diff --git a/internal/cmd/calendar.go b/internal/cmd/calendar.go
index 5ed1805..0ec6dea 100644
--- a/internal/cmd/calendar.go
+++ b/internal/cmd/calendar.go
@@ -81,6 +81,7 @@ type CalendarCreateCmd struct {
 	Body      string   `name:"body" help:"Optional body text"`
 	Attendees []string `name:"attendee" help:"Attendee email (repeat for multiple)"`
 	Teams     bool     `name:"teams" help:"Add Teams meeting link"`
+	DryRun    bool     `name:"dry-run" help:"Preview create without creating the event"`
 }
 
 func (c *CalendarCreateCmd) Run(ctx context.Context) error {
@@ -121,6 +122,17 @@ func (c *CalendarCreateCmd) Run(ctx context.Context) error {
 		payload["isOnlineMeeting"] = true
 		payload["onlineMeetingProvider"] = "teamsForBusiness"
 	}
+	if c.DryRun {
+		if outfmt.IsJSON(ctx) {
+			return outfmt.WriteJSON(os.Stdout, map[string]any{
+				"dry_run": true,
+				"action":  "calendar.create",
+				"event":   payload,
+			})
+		}
+		fmt.Fprintf(os.Stdout, "Dry run: would create event %q from %s to %s\n", c.Subject, c.Start, c.End)
+		return nil
+	}
 
 	svc := calendar.New(rt.Graph)
 	item, err := svc.Create(ctx, payload)
@@ -143,6 +155,7 @@ type CalendarUpdateCmd struct {
 	Body      string   `name:"body" help:"Body text"`
 	Attendees []string `name:"attendee" help:"Attendee email (repeat for multiple)"`
 	Teams     bool     `name:"teams" help:"Add Teams meeting link"`
+	DryRun    bool     `name:"dry-run" help:"Preview update without modifying the event"`
 }
 
 func (c *CalendarUpdateCmd) Run(ctx context.Context) error {
@@ -191,6 +204,18 @@ func (c *CalendarUpdateCmd) Run(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
+	if c.DryRun {
+		if outfmt.IsJSON(ctx) {
+			return outfmt.WriteJSON(os.Stdout, map[string]any{
+				"dry_run": true,
+				"action":  "calendar.update",
+				"id":      c.ID,
+				"changes": payload,
+			})
+		}
+		fmt.Fprintf(os.Stdout, "Dry run: would update event %s\n", c.ID)
+		return nil
+	}
 
 	svc := calendar.New(rt.Graph)
 	if err := svc.Update(ctx, c.ID, payload); err != nil {
@@ -205,10 +230,27 @@ func (c *CalendarUpdateCmd) Run(ctx context.Context) error {
 }
 
 type CalendarDeleteCmd struct {
-	ID string `arg:"" required:"" help:"Event ID"`
+	ID     string `arg:"" required:"" help:"Event ID"`
+	DryRun bool   `name:"dry-run" help:"Preview delete without deleting the event"`
 }
 
 func (c *CalendarDeleteCmd) Run(ctx context.Context) error {
+	rt, err := resolveRuntime(ctx, capCalendarDelete)
+	if err != nil {
+		return err
+	}
+	if c.DryRun {
+		if outfmt.IsJSON(ctx) {
+			return outfmt.WriteJSON(os.Stdout, map[string]any{
+				"dry_run": true,
+				"action":  "calendar.delete",
+				"id":      c.ID,
+			})
+		}
+		fmt.Fprintf(os.Stdout, "Dry run: would delete event %s\n", c.ID)
+		return nil
+	}
+
 	flags := rootFlagsFromContext(ctx)
 	if flags == nil {
 		flags = &RootFlags{}
@@ -217,10 +259,6 @@ func (c *CalendarDeleteCmd) Run(ctx context.Context) error {
 		return err
 	}
 
-	rt, err := resolveRuntime(ctx, capCalendarDelete)
-	if err != nil {
-		return err
-	}
 	if err := calendar.New(rt.Graph).Delete(ctx, c.ID); err != nil {
 		return err
 	}
diff --git a/internal/cmd/dry_run_flags_test.go b/internal/cmd/dry_run_flags_test.go
new file mode 100644
index 0000000..76311ce
--- /dev/null
+++ b/internal/cmd/dry_run_flags_test.go
@@ -0,0 +1,69 @@
+package cmd
+
+import "testing"
+
+func TestDryRunFlagsParse(t *testing.T) {
+	testCases := []struct {
+		name string
+		args []string
+		get  func(*CLI) bool
+	}{
+		{
+			name: "mail send --dry-run",
+			args: []string{"mail", "send", "--to", "dev@example.com", "--subject", "Test", "--body", "Body", "--dry-run"},
+			get:  func(cli *CLI) bool { return cli.Mail.Send.DryRun },
+		},
+		{
+			name: "calendar create --dry-run",
+			args: []string{
+				"calendar", "create",
+				"--subject", "Planning",
+				"--start", "2026-02-13T16:00:00Z",
+				"--end", "2026-02-13T16:30:00Z",
+				"--dry-run",
+			},
+			get: func(cli *CLI) bool { return cli.Calendar.Create.DryRun },
+		},
+		{
+			name: "calendar update --dry-run",
+			args: []string{"calendar", "update", "event-id", "--subject", "Updated", "--dry-run"},
+			get:  func(cli *CLI) bool { return cli.Calendar.Update.DryRun },
+		},
+		{
+			name: "calendar delete --dry-run",
+			args: []string{"calendar", "delete", "event-id", "--dry-run"},
+			get:  func(cli *CLI) bool { return cli.Calendar.Delete.DryRun },
+		},
+		{
+			name: "onedrive put --dry-run",
+			args: []string{"onedrive", "put", "./local.txt", "--path", "/remote.txt", "--dry-run"},
+			get:  func(cli *CLI) bool { return cli.OneDrive.Put.DryRun },
+		},
+		{
+			name: "onedrive mkdir --dry-run",
+			args: []string{"onedrive", "mkdir", "--path", "/reports", "--dry-run"},
+			get:  func(cli *CLI) bool { return cli.OneDrive.Mkdir.DryRun },
+		},
+		{
+			name: "onedrive rm --dry-run",
+			args: []string{"onedrive", "rm", "--path", "/reports/old.txt", "--dry-run"},
+			get:  func(cli *CLI) bool { return cli.OneDrive.RM.DryRun },
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			parser, cli, err := newParser("test")
+			if err != nil {
+				t.Fatalf("newParser failed: %v", err)
+			}
+			if _, err := parser.Parse(tc.args); err != nil {
+				t.Fatalf("parse failed: %v", err)
+			}
+			if !tc.get(cli) {
+				t.Fatalf("expected --dry-run to set flag true for args: %#v", tc.args)
+			}
+		})
+	}
+}
diff --git a/internal/cmd/mail.go b/internal/cmd/mail.go
index 6b9f850..a9b7418 100644
--- a/internal/cmd/mail.go
+++ b/internal/cmd/mail.go
@@ -86,6 +86,7 @@ type MailSendCmd struct {
 	Subject string   `name:"subject" required:"" help:"Email subject"`
 	Body    string   `name:"body" required:"" help:"Plain text body"`
 	User    string   `name:"user" help:"App-only target user override (UPN or user ID)"`
+	DryRun  bool     `name:"dry-run" help:"Preview send without sending the message"`
 }
 
 func (c *MailSendCmd) Run(ctx context.Context) error {
@@ -102,6 +103,19 @@ func (c *MailSendCmd) Run(ctx context.Context) error {
 	if len(recipients) == 0 {
 		return usage("at least one --to recipient is required")
 	}
+	if c.DryRun {
+		if outfmt.IsJSON(ctx) {
+			return outfmt.WriteJSON(os.Stdout, map[string]any{
+				"dry_run":  true,
+				"action":   "mail.send",
+				"to":       recipients,
+				"subject":  c.Subject,
+				"body_len": len(c.Body),
+			})
+		}
+		fmt.Fprintf(os.Stdout, "Dry run: would send message to %s with subject %q\n", strings.Join(recipients, ", "), c.Subject)
+		return nil
+	}
 
 	svc := mail.New(rt.Graph, targetUser)
 	if err := svc.Send(ctx, recipients, c.Subject, c.Body); err != nil {

From 7d8b00f988eb2a79554029ccf6c4e8126de66ee4 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:33:41 -0500
Subject: [PATCH 09/13] Deduplicate OData page decoding across services

---
 internal/graph/odata.go               | 27 +++++++++++++++++++++++++
 internal/graph/odata_test.go          | 27 +++++++++++++++++++++++++
 internal/services/calendar/service.go | 22 +-------------------
 internal/services/mail/service.go     | 29 +--------------------------
 internal/services/tasks/service.go    | 24 ++--------------------
 5 files changed, 58 insertions(+), 71 deletions(-)
 create mode 100644 internal/graph/odata.go
 create mode 100644 internal/graph/odata_test.go

diff --git a/internal/graph/odata.go b/internal/graph/odata.go
new file mode 100644
index 0000000..d8b032b
--- /dev/null
+++ b/internal/graph/odata.go
@@ -0,0 +1,27 @@
+package graph
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+)
+
+// DecodeODataPage decodes standard Graph collection payloads.
+func DecodeODataPage(body []byte) ([]map[string]any, string, error) {
+	var payload map[string]any
+	if err := json.Unmarshal(body, &payload); err != nil {
+		return nil, "", fmt.Errorf("decode json response: %w", err)
+	}
+
+	items := make([]map[string]any, 0)
+	if values, ok := payload["value"].([]any); ok {
+		for _, value := range values {
+			if item, ok := value.(map[string]any); ok {
+				items = append(items, item)
+			}
+		}
+	}
+
+	next, _ := payload["@odata.nextLink"].(string)
+	return items, strings.TrimSpace(next), nil
+}
diff --git a/internal/graph/odata_test.go b/internal/graph/odata_test.go
new file mode 100644
index 0000000..0dc3e55
--- /dev/null
+++ b/internal/graph/odata_test.go
@@ -0,0 +1,27 @@
+package graph
+
+import "testing"
+
+func TestDecodeODataPage(t *testing.T) {
+	body := []byte(`{
+		"value": [{"id":"1"},{"id":"2"}],
+		"@odata.nextLink":" https://graph.microsoft.com/v1.0/me/messages?$skiptoken=abc "
+	}`)
+
+	items, next, err := DecodeODataPage(body)
+	if err != nil {
+		t.Fatalf("DecodeODataPage failed: %v", err)
+	}
+	if len(items) != 2 {
+		t.Fatalf("expected two items, got %d", len(items))
+	}
+	if next != "https://graph.microsoft.com/v1.0/me/messages?$skiptoken=abc" {
+		t.Fatalf("unexpected next link: %q", next)
+	}
+}
+
+func TestDecodeODataPageInvalidJSON(t *testing.T) {
+	if _, _, err := DecodeODataPage([]byte(`{`)); err == nil {
+		t.Fatal("expected decode error")
+	}
+}
diff --git a/internal/services/calendar/service.go b/internal/services/calendar/service.go
index 0dae95a..cce1ada 100644
--- a/internal/services/calendar/service.go
+++ b/internal/services/calendar/service.go
@@ -2,7 +2,6 @@ package calendar
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -48,7 +47,7 @@ func (s *Service) List(ctx context.Context, from string, to string, max int, pag
 		return nil, "", err
 	}
 
-	items, next, err := decodeValuePage(body)
+	items, next, err := graph.DecodeODataPage(body)
 	if err != nil {
 		return nil, "", err
 	}
@@ -79,25 +78,6 @@ func (s *Service) Delete(ctx context.Context, id string) error {
 	return err
 }
 
-func decodeValuePage(body []byte) ([]map[string]any, string, error) {
-	var payload map[string]any
-	if err := json.Unmarshal(body, &payload); err != nil {
-		return nil, "", fmt.Errorf("decode json response: %w", err)
-	}
-
-	items := make([]map[string]any, 0)
-	if values, ok := payload["value"].([]any); ok {
-		for _, value := range values {
-			if item, ok := value.(map[string]any); ok {
-				items = append(items, item)
-			}
-		}
-	}
-
-	next, _ := payload["@odata.nextLink"].(string)
-	return items, strings.TrimSpace(next), nil
-}
-
 func trimPage(items []map[string]any, next string, max int) ([]map[string]any, string) {
 	if max <= 0 || len(items) <= max {
 		return items, next
diff --git a/internal/services/mail/service.go b/internal/services/mail/service.go
index 1b3b863..46fbf28 100644
--- a/internal/services/mail/service.go
+++ b/internal/services/mail/service.go
@@ -2,7 +2,6 @@ package mail
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -51,7 +50,7 @@ func (s *Service) List(ctx context.Context, max int, queryText string, page stri
 		return nil, "", err
 	}
 
-	items, next, err := decodeValuePage(body)
+	items, next, err := graph.DecodeODataPage(body)
 	if err != nil {
 		return nil, "", err
 	}
@@ -108,32 +107,6 @@ func (s *Service) sendMailEndpoint() string {
 	return "/me/sendMail"
 }
 
-func decodeValuePage(body []byte) ([]map[string]any, string, error) {
-	var payload map[string]any
-	if err := jsonUnmarshal(body, &payload); err != nil {
-		return nil, "", err
-	}
-
-	items := make([]map[string]any, 0)
-	if values, ok := payload["value"].([]any); ok {
-		for _, value := range values {
-			if item, ok := value.(map[string]any); ok {
-				items = append(items, item)
-			}
-		}
-	}
-
-	next, _ := payload["@odata.nextLink"].(string)
-	return items, strings.TrimSpace(next), nil
-}
-
-func jsonUnmarshal(data []byte, v any) error {
-	if err := json.Unmarshal(data, v); err != nil {
-		return fmt.Errorf("decode json response: %w", err)
-	}
-	return nil
-}
-
 func trimPage(items []map[string]any, next string, max int) ([]map[string]any, string) {
 	if max <= 0 || len(items) <= max {
 		return items, next
diff --git a/internal/services/tasks/service.go b/internal/services/tasks/service.go
index 0dade3f..1f40992 100644
--- a/internal/services/tasks/service.go
+++ b/internal/services/tasks/service.go
@@ -2,7 +2,6 @@ package tasks
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -32,7 +31,7 @@ func (s *Service) Lists(ctx context.Context) ([]map[string]any, error) {
 	if err != nil {
 		return nil, err
 	}
-	items, _, err := decodeValuePage(body)
+	items, _, err := graph.DecodeODataPage(body)
 	return items, err
 }
 
@@ -53,7 +52,7 @@ func (s *Service) ListTasks(ctx context.Context, listID string, max int, page st
 		return nil, "", err
 	}
 
-	items, next, err := decodeValuePage(body)
+	items, next, err := graph.DecodeODataPage(body)
 	if err != nil {
 		return nil, "", err
 	}
@@ -88,25 +87,6 @@ func (s *Service) DeleteTask(ctx context.Context, listID string, taskID string)
 	return normalizeTaskMutationError(err)
 }
 
-func decodeValuePage(body []byte) ([]map[string]any, string, error) {
-	var payload map[string]any
-	if err := json.Unmarshal(body, &payload); err != nil {
-		return nil, "", fmt.Errorf("decode json response: %w", err)
-	}
-
-	items := make([]map[string]any, 0)
-	if values, ok := payload["value"].([]any); ok {
-		for _, value := range values {
-			if item, ok := value.(map[string]any); ok {
-				items = append(items, item)
-			}
-		}
-	}
-
-	next, _ := payload["@odata.nextLink"].(string)
-	return items, strings.TrimSpace(next), nil
-}
-
 func normalizeTaskMutationError(err error) error {
 	if err == nil {
 		return nil

From fca0f73196ad5bdc1b8901072b3804fda7e444a5 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:34:53 -0500
Subject: [PATCH 10/13] Zero sensitive auth byte buffers after use

---
 internal/auth/manager.go      | 16 +++++++++++++++-
 internal/auth/manager_test.go | 11 +++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/internal/auth/manager.go b/internal/auth/manager.go
index 537e9ce..802fc45 100644
--- a/internal/auth/manager.go
+++ b/internal/auth/manager.go
@@ -135,6 +135,7 @@ func (m *Manager) LoginDelegated(ctx context.Context, input DelegatedLoginInput,
 	if err != nil {
 		return AccountInfo{}, err
 	}
+	defer secureZero(body)
 
 	var dcr deviceCodeResponse
 	if err := json.Unmarshal(body, &dcr); err != nil {
@@ -244,7 +245,9 @@ func (m *Manager) LoginAppOnly(ctx context.Context, input AppOnlyLoginInput) err
 		return err
 	}
 
-	if err := secrets.SetSecret(secretKey, []byte(input.Secret)); err != nil {
+	secretBytes := []byte(input.Secret)
+	defer secureZero(secretBytes)
+	if err := secrets.SetSecret(secretKey, secretBytes); err != nil {
 		return fmt.Errorf("store client secret: %w", err)
 	}
 
@@ -295,6 +298,7 @@ func (m *Manager) AcquireDelegatedToken(
 	if err != nil {
 		return "", err
 	}
+	defer secureZero(body)
 	if status != http.StatusOK {
 		var oe oauthErrorResponse
 		_ = json.Unmarshal(body, &oe)
@@ -344,6 +348,7 @@ func (m *Manager) AcquireAppOnlyToken(ctx context.Context, profileName string, c
 	if err != nil {
 		return "", fmt.Errorf("read client secret: %w", err)
 	}
+	defer secureZero(secret)
 	if len(secret) == 0 {
 		return "", ErrMissingSecret
 	}
@@ -358,6 +363,7 @@ func (m *Manager) AcquireAppOnlyToken(ctx context.Context, profileName string, c
 	if err != nil {
 		return "", err
 	}
+	defer secureZero(body)
 
 	if status != http.StatusOK {
 		var oe oauthErrorResponse
@@ -549,6 +555,7 @@ func (m *Manager) saveToken(profileName string, cache TokenCache) error {
 	if err != nil {
 		return fmt.Errorf("encode token cache: %w", err)
 	}
+	defer secureZero(payload)
 
 	cacheKey, err := tokenCacheKey(profileName)
 	if err != nil {
@@ -572,6 +579,7 @@ func (m *Manager) loadToken(profileName string) (TokenCache, error) {
 	if err != nil {
 		return TokenCache{}, fmt.Errorf("read token cache: %w", err)
 	}
+	defer secureZero(payload)
 
 	var cache TokenCache
 	if err := json.Unmarshal(payload, &cache); err != nil {
@@ -667,3 +675,9 @@ func sleepContext(ctx context.Context, delay time.Duration) error {
 		return nil
 	}
 }
+
+func secureZero(value []byte) {
+	for i := range value {
+		value[i] = 0
+	}
+}
diff --git a/internal/auth/manager_test.go b/internal/auth/manager_test.go
index 54f2713..664c447 100644
--- a/internal/auth/manager_test.go
+++ b/internal/auth/manager_test.go
@@ -286,3 +286,14 @@ func TestTokenCacheKeyRejectsInvalidProfileName(t *testing.T) {
 		t.Fatal("expected invalid profile name error")
 	}
 }
+
+func TestSecureZero(t *testing.T) {
+	value := []byte("super-secret")
+	secureZero(value)
+
+	for i, b := range value {
+		if b != 0 {
+			t.Fatalf("expected index %d to be zeroed, got %d", i, b)
+		}
+	}
+}

From 642309466bd2b05b624218da70fd8c5db4e4b2b6 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:35:13 -0500
Subject: [PATCH 11/13] Add coverage for next-page hint output

---
 internal/cmd/output_helpers_test.go | 47 +++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 internal/cmd/output_helpers_test.go

diff --git a/internal/cmd/output_helpers_test.go b/internal/cmd/output_helpers_test.go
new file mode 100644
index 0000000..d166fa0
--- /dev/null
+++ b/internal/cmd/output_helpers_test.go
@@ -0,0 +1,47 @@
+package cmd
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/jaredpalmer/mogcli/internal/ui"
+)
+
+func TestPrintNextPageHint(t *testing.T) {
+	t.Parallel()
+
+	var stderr bytes.Buffer
+	printer := ui.New(ui.Options{Stderr: &stderr})
+	token := "https://graph.microsoft.com/v1.0/me/messages?$skiptoken=abc"
+
+	printNextPageHint(printer, token)
+
+	got := strings.TrimSpace(stderr.String())
+	want := "# Next page: --page 'https://graph.microsoft.com/v1.0/me/messages?$skiptoken=abc'"
+	if got != want {
+		t.Fatalf("unexpected hint output:\n got: %q\nwant: %q", got, want)
+	}
+}
+
+func TestPrintNextPageHintSkipsEmptyToken(t *testing.T) {
+	t.Parallel()
+
+	var stderr bytes.Buffer
+	printer := ui.New(ui.Options{Stderr: &stderr})
+
+	printNextPageHint(printer, "")
+
+	if stderr.Len() != 0 {
+		t.Fatalf("expected no output for empty token, got %q", stderr.String())
+	}
+}
+
+func TestShellQuoteEscapesSingleQuote(t *testing.T) {
+	t.Parallel()
+
+	got := shellQuote("a'b")
+	if got != "'a'\\''b'" {
+		t.Fatalf("unexpected shell-quoted value: %q", got)
+	}
+}

From 7cf6c9bc5c5b1ac77844727f6904c530352b0b0d Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:36:51 -0500
Subject: [PATCH 12/13] Polish error formatting and add API documentation

---
 README.md                      |  6 ++++++
 docs/microsoft-port-plan.md    |  2 ++
 internal/auth/manager.go       |  2 ++
 internal/cmd/root.go           |  3 +++
 internal/errfmt/errfmt.go      | 14 ++++++++++++++
 internal/errfmt/errfmt_test.go | 22 ++++++++++++++++++++++
 internal/graph/client.go       |  2 ++
 internal/profile/store.go      |  2 ++
 8 files changed, 53 insertions(+)

diff --git a/README.md b/README.md
index 3694e28..904d214 100644
--- a/README.md
+++ b/README.md
@@ -12,6 +12,7 @@ Microsoft 365 in your terminal.
 - Stable scripting output modes: `--json` and `--plain`.
 - Interactive delegated wizard (`mog auth`), advanced app-only wizard (`mog auth app`), settings editor (`mog auth update`), and non-interactive login (`mog auth login`).
 - Per-command scope requests in delegated mode (progressive consent).
+- `--dry-run` previews for write operations in Mail, Calendar, and OneDrive.
 
 ### Workload support matrix
 
@@ -174,6 +175,7 @@ Mail:
 mog mail list --max 50 --query "from:alerts@example.com"
 mog mail get <message-id>
 mog mail send --to dev@contoso.com --subject "Deploy complete" --body "Finished."
+mog mail send --to dev@contoso.com --subject "Deploy complete" --body "Finished." --dry-run
 ```
 
 Calendar:
@@ -185,6 +187,7 @@ mog calendar create \
   --start "2026-02-13T16:00:00-08:00" \
   --end "2026-02-13T16:30:00-08:00" \
   --body "Weekly sync"
+mog calendar delete <event-id> --dry-run
 ```
 
 Contacts:
@@ -218,8 +221,11 @@ mog onedrive put ./report.pdf --path /Reports/report.pdf
 mog onedrive get /Reports/report.pdf --out ./report.pdf
 mog onedrive mkdir --path /Reports/Archive
 mog onedrive rm --path /Reports/old-report.pdf
+mog onedrive rm --path /Reports/old-report.pdf --dry-run
 ```
 
+If `mog onedrive get` is run without `--out`, files are saved under the local `onedrive-downloads` directory in your mogcli config path.
+
 App-only target user override (mail/contacts/onedrive):
 
 ```bash
diff --git a/docs/microsoft-port-plan.md b/docs/microsoft-port-plan.md
index 0a5f883..967bfc1 100644
--- a/docs/microsoft-port-plan.md
+++ b/docs/microsoft-port-plan.md
@@ -18,6 +18,8 @@ Implemented in codebase:
 8. App-only endpoint routing for mail/contacts/onedrive using `/users/{id}` with `--user` override and profile fallback.
 9. Explicit fail-fast app-only rejection for calendar/tasks with deterministic user-facing guidance.
 10. Task mutation mutability error normalization in `internal/services/tasks/service.go` for built-in/well-known Microsoft To Do list constraints.
+11. `--dry-run` previews for write operations in mail, calendar, and onedrive command surfaces.
+12. OneDrive local path hardening (`SafeExpandPath`), metadata-rich delete confirmation, and progress output for local file transfers.
 
 ## 0. Critique of the prior plan
 
diff --git a/internal/auth/manager.go b/internal/auth/manager.go
index 802fc45..b0c0288 100644
--- a/internal/auth/manager.go
+++ b/internal/auth/manager.go
@@ -25,11 +25,13 @@ var (
 	ErrMissingSecret       = errors.New("missing client secret")
 )
 
+// Manager coordinates login, token acquisition, and secure token persistence.
 type Manager struct {
 	HTTPClient *http.Client
 	now        func() time.Time
 }
 
+// NewManager returns an auth manager with default HTTP/time providers.
 func NewManager() *Manager {
 	return &Manager{
 		HTTPClient: http.DefaultClient,
diff --git a/internal/cmd/root.go b/internal/cmd/root.go
index d73d852..709431d 100644
--- a/internal/cmd/root.go
+++ b/internal/cmd/root.go
@@ -55,6 +55,9 @@ func Execute(args []string) (err error) {
 		args = []string{"--help"}
 	}
 
+	// Kong exits by invoking the configured kong.Exit callback.
+	// We translate that callback into a panic carrying only an exit code,
+	// then recover here so Execute can return a regular Go error value.
 	defer func() {
 		if r := recover(); r != nil {
 			if ep, ok := r.(exitPanic); ok {
diff --git a/internal/errfmt/errfmt.go b/internal/errfmt/errfmt.go
index a9f487c..0c6f85b 100644
--- a/internal/errfmt/errfmt.go
+++ b/internal/errfmt/errfmt.go
@@ -10,6 +10,7 @@ import (
 	"github.com/alecthomas/kong"
 
 	"github.com/jaredpalmer/mogcli/internal/graph"
+	"github.com/jaredpalmer/mogcli/internal/profile"
 )
 
 var aadstsRe = regexp.MustCompile(`AADSTS\d+`)
@@ -28,6 +29,19 @@ func Format(err error) string {
 		return prefixError(err.Error())
 	}
 
+	var breakerErr *graph.CircuitBreakerError
+	if errors.As(err, &breakerErr) {
+		return prefixError("Graph requests are temporarily paused after repeated failures. Retry in a few seconds")
+	}
+
+	if errors.Is(err, profile.ErrProfileNotFound) {
+		return prefixError("Profile not found. Run `mog auth accounts` to list available profiles")
+	}
+
+	if errors.Is(err, profile.ErrNoActiveProfile) {
+		return prefixError("No active profile. Run `mog auth use <profile>` or `mog auth login ...`")
+	}
+
 	var apiErr *graph.APIError
 	if errors.As(err, &apiErr) {
 		if apiErr.Code != "" {
diff --git a/internal/errfmt/errfmt_test.go b/internal/errfmt/errfmt_test.go
index 5ca4e06..34e83d3 100644
--- a/internal/errfmt/errfmt_test.go
+++ b/internal/errfmt/errfmt_test.go
@@ -4,6 +4,9 @@ import (
 	"errors"
 	"strings"
 	"testing"
+
+	"github.com/jaredpalmer/mogcli/internal/graph"
+	"github.com/jaredpalmer/mogcli/internal/profile"
 )
 
 func TestFormatAADSTS(t *testing.T) {
@@ -18,3 +21,22 @@ func TestFormatAADSTS(t *testing.T) {
 		t.Fatalf("expected actionable guidance, got: %s", msg)
 	}
 }
+
+func TestFormatProfileErrors(t *testing.T) {
+	notFound := Format(profile.ErrProfileNotFound)
+	if !strings.Contains(notFound, "Profile not found") {
+		t.Fatalf("unexpected profile-not-found message: %s", notFound)
+	}
+
+	noActive := Format(profile.ErrNoActiveProfile)
+	if !strings.Contains(noActive, "No active profile") {
+		t.Fatalf("unexpected no-active-profile message: %s", noActive)
+	}
+}
+
+func TestFormatCircuitBreakerError(t *testing.T) {
+	msg := Format(&graph.CircuitBreakerError{})
+	if !strings.Contains(strings.ToLower(msg), "temporarily paused") {
+		t.Fatalf("unexpected circuit-breaker message: %s", msg)
+	}
+}
diff --git a/internal/graph/client.go b/internal/graph/client.go
index f93cdf3..f9459e2 100644
--- a/internal/graph/client.go
+++ b/internal/graph/client.go
@@ -17,6 +17,7 @@ import (
 
 type TokenProvider func(ctx context.Context, scopes []string) (string, error)
 
+// Client wraps Microsoft Graph HTTP behavior including auth, retry, and paging.
 type Client struct {
 	BaseURL       string
 	HTTPClient    *http.Client
@@ -27,6 +28,7 @@ type Client struct {
 	Breaker       *CircuitBreaker
 }
 
+// NewClient builds a Graph client with conservative retry and breaker defaults.
 func NewClient(tokenProvider TokenProvider) *Client {
 	return &Client{
 		BaseURL:       "https://graph.microsoft.com/v1.0",
diff --git a/internal/profile/store.go b/internal/profile/store.go
index ddc5c20..902c191 100644
--- a/internal/profile/store.go
+++ b/internal/profile/store.go
@@ -27,8 +27,10 @@ const (
 	AuthModeAppOnly   = "app_only"
 )
 
+// Store manages profile records persisted in local config.
 type Store struct{}
 
+// NewStore creates a profile store backed by local config.
 func NewStore() *Store { return &Store{} }
 
 func NormalizeName(name string) (string, error) {

From 723bd83b1a596a6c3c248a9f79becd1f16283778 Mon Sep 17 00:00:00 2001
From: Jared Palmer <jaredpalmer@github.com>
Date: Fri, 13 Feb 2026 09:45:02 -0500
Subject: [PATCH 13/13] Remove review.md file

---
 REVIEW.md | 449 ------------------------------------------------------
 1 file changed, 449 deletions(-)
 delete mode 100644 REVIEW.md

diff --git a/REVIEW.md b/REVIEW.md
deleted file mode 100644
index ea590b7..0000000
--- a/REVIEW.md
+++ /dev/null
@@ -1,449 +0,0 @@
-# mogcli Code Review
-
-**Date:** 2026-02-13  
-**Scope:** Security, concurrency, UX/UI, code quality  
-**Codebase:** ~9,700 LOC (internal packages)
-
----
-
-## Summary
-
-mogcli is a well-structured Go CLI with good separation of concerns. The codebase follows Go conventions and has reasonable test coverage. However, there are security concerns around file path handling, a race condition in the circuit breaker, and several UX improvements that could be made.
-
-**Critical Issues:** 2  
-**High Severity:** 2  
-**Medium Severity:** 5  
-**Low Severity:** 4  
-
----
-
-## Security Issues
-
-### 🔴 CRITICAL: Path Traversal in OneDrive File Operations
-
-**Files:**
-- `internal/cmd/onedrive.go:93, 128, 133`
-- `internal/config/paths.go:120-145`
-
-**Description:**  
-User-supplied file paths for OneDrive upload/download are not validated against directory traversal attacks. The `ExpandPath()` function only handles `~` expansion but does not prevent `../` sequences.
-
-```go
-// internal/cmd/onedrive.go:128
-localPath, err := config.ExpandPath(strings.TrimSpace(c.LocalPath))
-
-// internal/cmd/onedrive.go:133
-content, err := os.ReadFile(localPath) //nolint:gosec // user-provided file path
-```
-
-The `nolint:gosec` comment acknowledges the risk but doesn't mitigate it.
-
-**Attack Vector:**
-```bash
-# Read arbitrary files during upload
-mog onedrive put ../../../etc/passwd --path /stolen.txt
-
-# Write to arbitrary locations during download
-mog onedrive get /file.txt --out ../../../tmp/malicious
-```
-
-**Recommendation:**  
-Add path canonicalization and validation:
-
-```go
-func SafeExpandPath(basePath, userPath string) (string, error) {
-    expanded, err := ExpandPath(userPath)
-    if err != nil {
-        return "", err
-    }
-    
-    cleaned := filepath.Clean(expanded)
-    absBase, _ := filepath.Abs(basePath)
-    absCleaned, _ := filepath.Abs(cleaned)
-    
-    if !strings.HasPrefix(absCleaned, absBase) {
-        return "", fmt.Errorf("path escapes base directory: %s", userPath)
-    }
-    
-    return cleaned, nil
-}
-```
-
----
-
-### 🔴 CRITICAL: Race Condition in Circuit Breaker
-
-**File:** `internal/graph/circuitbreaker.go:41-55`
-
-**Description:**  
-The `IsOpen()` method reads and writes multiple fields under a single mutex lock, but there's a Time-of-Check-Time-of-Use (TOCTOU) issue. Between checking `IsOpen()` in `client.go:83` and recording success/failure, the circuit breaker state can change.
-
-```go
-// internal/graph/client.go:83-84
-if c.Breaker != nil && c.Breaker.IsOpen() {
-    return nil, nil, &CircuitBreakerError{}
-}
-// ... request happens here, breaker state can change ...
-if c.Breaker != nil {
-    c.Breaker.RecordSuccess()
-}
-```
-
-**Impact:**  
-Under high concurrency, requests may be allowed through an open breaker, or the failure count may not accurately reflect the actual error rate.
-
-**Recommendation:**  
-Use an atomic check-and-execute pattern:
-
-```go
-func (cb *CircuitBreaker) Execute(fn func() error) error {
-    cb.mu.Lock()
-    if cb.open && time.Since(cb.lastFailure) <= CircuitBreakerResetTime {
-        cb.mu.Unlock()
-        return &CircuitBreakerError{}
-    }
-    cb.mu.Unlock()
-    
-    err := fn()
-    
-    cb.mu.Lock()
-    defer cb.mu.Unlock()
-    if err != nil {
-        cb.failures++
-        cb.lastFailure = time.Now()
-        if cb.failures >= CircuitBreakerThreshold {
-            cb.open = true
-        }
-    } else {
-        cb.failures = 0
-        cb.open = false
-    }
-    return err
-}
-```
-
----
-
-### 🟠 HIGH: No Validation of Pagination Token Host
-
-**File:** `internal/cmd/pagination.go:8-26`
-
-**Description:**  
-The pagination token URL is validated for format but not for host. An attacker could inject a malicious URL as a `--page` token to redirect API calls.
-
-```go
-func normalizePageToken(raw string) (string, error) {
-    // Only checks for valid URL format, not host
-    if u.Scheme != "http" && u.Scheme != "https" {
-        return "", usage("--page must be an absolute http(s) URL")
-    }
-    return u.String(), nil
-}
-```
-
-The Graph client has `sameHost()` validation (line 235), but pagination tokens bypass this in `Paginate()` when passed as a full URL.
-
-**Recommendation:**  
-Validate that pagination URLs match the expected Microsoft Graph host:
-
-```go
-var allowedHosts = []string{"graph.microsoft.com"}
-
-func normalizePageToken(raw string) (string, error) {
-    // ... existing validation ...
-    
-    if !isAllowedHost(u.Host) {
-        return "", usage("--page URL must be a Microsoft Graph URL")
-    }
-    return u.String(), nil
-}
-```
-
----
-
-### 🟠 HIGH: Client Secret Visible in Process List
-
-**File:** `internal/cmd/auth.go:70`
-
-**Description:**  
-The `--client-secret` flag accepts secrets directly on the command line, which exposes them in process listings (`ps aux`).
-
-```go
-ClientSecret    string `name:"client-secret" help:"Client secret for app-only mode"`
-```
-
-While `--client-secret-env` is provided as an alternative, users may not realize the security implications.
-
-**Recommendation:**  
-1. Add a warning when `--client-secret` is used directly
-2. Consider deprecating the flag in favor of env-var-only input
-3. Add interactive secret prompting when neither is provided
-
----
-
-### 🟡 MEDIUM: Unvalidated Profile Names in File Paths
-
-**Files:**
-- `internal/profile/store.go:33-39`
-- `internal/auth/manager.go:55-60`
-
-**Description:**  
-Profile names are used in secret key construction without strict validation. While `NormalizeName()` only trims whitespace, profile names could contain path-sensitive characters.
-
-```go
-func tokenCacheKey(profileName string) string {
-    return "mog:cache:" + strings.TrimSpace(profileName)
-}
-```
-
-The base64 encoding in `secretPath()` mitigates this for file paths, but profile names like `../malicious` could cause issues elsewhere.
-
-**Recommendation:**  
-Add character validation to `NormalizeName()`:
-
-```go
-func NormalizeName(name string) (string, error) {
-    normalized := strings.TrimSpace(name)
-    if normalized == "" {
-        return "", ErrInvalidName
-    }
-    
-    for _, r := range normalized {
-        if !((r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || 
-             (r >= '0' && r <= '9') || r == '-' || r == '_') {
-            return "", fmt.Errorf("%w: invalid character %q", ErrInvalidName, r)
-        }
-    }
-    
-    return normalized, nil
-}
-```
-
----
-
-### 🟡 MEDIUM: Tokens Not Cleared from Memory
-
-**File:** `internal/auth/manager.go`
-
-**Description:**  
-Access tokens and refresh tokens are stored in regular Go strings, which cannot be securely cleared from memory. This is a common limitation in Go, but sensitive data remains in memory longer than necessary.
-
-**Recommendation:**  
-Consider using `[]byte` for tokens and implementing a secure clear function:
-
-```go
-func secureZero(b []byte) {
-    for i := range b {
-        b[i] = 0
-    }
-}
-```
-
----
-
-## Concurrency Issues
-
-### 🟡 MEDIUM: Config File Read/Write Race
-
-**Files:**
-- `internal/config/config.go:43-70` (WriteConfig)
-- `internal/config/config.go:90-115` (ReadConfig)
-
-**Description:**  
-Multiple `mog` processes running concurrently could race on config file reads and writes. While atomic rename is used, there's no file locking.
-
-```go
-func WriteConfig(cfg File) error {
-    // No file locking
-    tmp := path + ".tmp"
-    if err := os.WriteFile(tmp, b, 0o600); err != nil { ... }
-    if err := os.Rename(tmp, path); err != nil { ... }
-}
-```
-
-**Impact:**  
-Lost updates if two processes modify profiles simultaneously.
-
-**Recommendation:**  
-Use file locking via `flock()` or a lockfile pattern for config modifications.
-
----
-
-## UX Issues
-
-### 🟡 MEDIUM: No Confirmation for Destructive Operations
-
-**File:** `internal/cmd/onedrive.go:189`
-
-**Description:**  
-While `onedrive rm` has confirmation via `confirmDestructive()`, there's no undo mechanism and the confirmation message doesn't show file size or type.
-
-```go
-if err := confirmDestructive(ctx, flags, fmt.Sprintf("delete %s", c.Path)); err != nil {
-```
-
-**Recommendation:**  
-Fetch item metadata before confirmation to show:
-- File size
-- Last modified date  
-- Whether it's a folder (recursive delete warning)
-
----
-
-### 🟢 LOW: Inconsistent Error Messages
-
-**Files:** Various
-
-**Description:**  
-Error messages mix styles:
-- `internal/graph/errors.go`: "Graph API error (404 NotFound): ..."
-- `internal/auth/manager.go`: "token exchange failed (error_code): description"
-- `internal/profile/store.go`: Bare sentinel errors like `ErrProfileNotFound`
-
-**Recommendation:**  
-Standardize error format across the codebase. Consider wrapping all user-facing errors through `errfmt.Format()`.
-
----
-
-### 🟢 LOW: No Progress Indicator for Large File Transfers
-
-**File:** `internal/cmd/onedrive.go:76, 133`
-
-**Description:**  
-File uploads and downloads show no progress. For large files, the CLI appears frozen.
-
-**Recommendation:**  
-Add a progress bar using a library like `github.com/schollz/progressbar/v3`:
-
-```go
-bar := progressbar.DefaultBytes(fileSize, "Uploading")
-reader := progressbar.NewReader(file, bar)
-```
-
----
-
-### 🟢 LOW: Missing `--dry-run` Flag for Write Operations
-
-**Files:**
-- `internal/cmd/mail.go` (send)
-- `internal/cmd/calendar.go` (create, update, delete)
-- `internal/cmd/onedrive.go` (put, rm, mkdir)
-
-**Description:**  
-Users cannot preview what will happen before executing write operations.
-
-**Recommendation:**  
-Add `--dry-run` flag that shows what would be done without making changes.
-
----
-
-### 🟢 LOW: Hard-coded Pagination Limit
-
-**File:** `internal/cmd/onedrive.go:25`
-
-**Description:**  
-Default max of 100 items may not suit all use cases, and users might not realize there's more data.
-
-```go
-Max  int    `name:"max" default:"100" help:"Maximum items to return"`
-```
-
-**Recommendation:**  
-When results are truncated, always print a hint about pagination even in non-JSON mode.
-
----
-
-## Code Quality Issues
-
-### 🟡 MEDIUM: Duplicated `decodeValuePage` Function
-
-**Files:**
-- `internal/services/calendar/service.go:82-99`
-- `internal/services/mail/service.go` (similar)
-- `internal/services/tasks/service.go` (similar)
-
-**Description:**  
-The same OData response parsing logic is duplicated across services.
-
-**Recommendation:**  
-Extract to a shared helper in `internal/graph/`:
-
-```go
-func DecodeODataPage(body []byte) ([]map[string]any, string, error)
-```
-
----
-
-### 🟡 MEDIUM: Panic-based Exit Handling
-
-**File:** `internal/cmd/root.go:166`
-
-**Description:**  
-The CLI uses panics for exit code handling:
-
-```go
-kong.Exit(func(code int) { panic(exitPanic{code: code}) }),
-```
-
-While this is recovered, it's an unusual pattern that could confuse contributors.
-
-**Recommendation:**  
-Document this pattern clearly or refactor to use error wrapping with exit codes.
-
----
-
-### 🟢 LOW: Unused `nolint` Comment Context
-
-**File:** `internal/cmd/onedrive.go:133`
-
-**Description:**  
-The `nolint:gosec` comment acknowledges the issue but doesn't document why it's acceptable:
-
-```go
-content, err := os.ReadFile(localPath) //nolint:gosec // user-provided file path
-```
-
-**Recommendation:**  
-Either fix the underlying issue or document why the risk is acceptable:
-
-```go
-//nolint:gosec // Path is validated by ExpandPath and intentionally user-controlled for CLI use
-```
-
----
-
-### 🟢 LOW: Missing Godoc on Exported Types
-
-**Files:** Various
-
-**Description:**  
-Several exported types lack documentation:
-- `graph.Client`
-- `auth.Manager`
-- `profile.Store`
-
-**Recommendation:**  
-Add Godoc comments to all exported types and functions.
-
----
-
-## Positive Observations
-
-1. **Good use of interfaces** - `authManager` and `profileStore` interfaces enable testing
-2. **Atomic config writes** - Using rename for safe config updates
-3. **Proper context propagation** - All operations accept `context.Context`
-4. **Consistent output modes** - `--json` and `--plain` are well-implemented
-5. **Good error wrapping** - `fmt.Errorf` with `%w` used consistently
-6. **Circuit breaker pattern** - Protects against cascading failures
-7. **Rate limit handling** - Proper retry-after and exponential backoff
-
----
-
-## Recommended Priority
-
-1. **Immediate:** Fix path traversal vulnerability in OneDrive operations
-2. **High:** Address circuit breaker race condition
-3. **High:** Validate pagination token hosts
-4. **Medium:** Add profile name validation
-5. **Medium:** Add progress indicators for file transfers
-6. **Low:** Refactor duplicated code, add documentation