From 95d616141d58afadb14ed923679f39165f7f41ed Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 21 May 2026 08:24:28 -0400 Subject: [PATCH 01/10] docs: design and plan for centralized OAuth --- .../plans/2026-05-20-centralized-oauth.md | 1829 +++++++++++++++++ .../2026-05-20-centralized-oauth-design.md | 518 +++++ 2 files changed, 2347 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-20-centralized-oauth.md create mode 100644 docs/superpowers/specs/2026-05-20-centralized-oauth-design.md diff --git a/docs/superpowers/plans/2026-05-20-centralized-oauth.md b/docs/superpowers/plans/2026-05-20-centralized-oauth.md new file mode 100644 index 000000000..1140cb722 --- /dev/null +++ b/docs/superpowers/plans/2026-05-20-centralized-oauth.md @@ -0,0 +1,1829 @@ +# Centralized Verified Google OAuth Client — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Eliminate the bring-your-own (BYO) Google Cloud OAuth setup as a first-run prerequisite by shipping a project-owned, Google-verified OAuth client baked into the msgvault binary. BYO + named-apps + service-account paths all remain as escape valves. + +**Architecture:** A new `internal/oauth/embedded.go` exposes package-level OAuth client credentials (overridable at build time via `-ldflags -X`) plus an `EmbeddedConfig` builder and `NewEmbeddedManager` factory. A new `cmd/msgvault/cmd/oauth_resolve.go` collapses today's BYO-only resolution into a single three-way helper (named BYO, global BYO, embedded). Service-account resolution remains at call sites since the SA manager has a different type. The `oauthManagerCache()` factory in `root.go` becomes the canonical refactor target; downstream consumers like `syncfull.go` inherit the new behavior automatically. Verification-window failures (100-user lifetime cap, unlisted test users) surface as a typed `access_denied` error in the OAuth callback handler; the Manager prints a fallback message when it sees this on the embedded path. + +**Tech Stack:** Go 1.x, `golang.org/x/oauth2`, `golang.org/x/oauth2/google`, Cobra CLI, stdlib `testing` (the existing msgvault test suite uses `t.Errorf`/`t.Fatalf`/`t.Run` patterns rather than testify, so plan tests follow that convention for consistency), GNU Make, GitHub Actions. + +--- + +## Spec reference + +The full design is at `docs/superpowers/specs/2026-05-20-centralized-oauth-design.md`. Read it before implementing. + +## Prerequisites (out of band, before merging) + +The dev and production Cloud projects need to exist before this work can ship usefully: + +- **Dev Google Cloud project** ("msgvault-dev"): create the project, register a Desktop OAuth client, list current contributors as test users, low API quota. Its `client_id` and `client_secret` become the source defaults in `internal/oauth/embedded.go` (Task 2). +- **Production Google Cloud project**: owned by the project maintainer. Its `client_id` and `client_secret` are injected at release time via GitHub Actions Secrets (Task 17, Task 18). Production verification (consent screen submission, brand verification, CASA Tier 2 assessment) is an out-of-band process tracked in the spec. + +The code in this plan compiles and tests pass with the placeholder string `"TBD-msgvault-dev-client-id"`. The dev project's real values must be substituted before the change is useful at runtime in source builds. + +## File structure + +New files: +- `internal/oauth/embedded.go` — package vars `oauthClientID` / `oauthClientSecret`, `EmbeddedConfig`, `NewEmbeddedManager`, `HasEmbeddedCredentials` +- `internal/oauth/embedded_test.go` — unit tests for the above +- `cmd/msgvault/cmd/oauth_resolve.go` — `resolveOAuthManager` three-way helper +- `cmd/msgvault/cmd/oauth_resolve_test.go` — unit tests for the helper + +Modified files: +- `internal/oauth/oauth.go` — add `ScopesEmbedded`, add `errAccessDenied` typed error, add `isEmbedded` field to `Manager`, modify `newCallbackHandler` to detect `error=access_denied`, modify `authorize` to print the embedded fallback message on access_denied +- `internal/oauth/oauth_test.go` — add `TestScopesEmbedded`, `TestCallbackHandlerAccessDenied`, `TestAuthorizeEmbeddedFallbackMessage`, `TestAuthorizeNonEmbeddedNoFallback` +- `internal/config/config.go` — remove `HasAnyConfig` method +- `internal/config/config_test.go` — remove `TestOAuthConfig_HasAnyConfig` and inline `HasAnyConfig` assertions +- `cmd/msgvault/cmd/root.go` — refactor `oauthManagerCache()` to call `resolveOAuthManager`; remove `errOAuthNotConfigured`, `tryFindClientSecrets`, `oauthSetupHint`, `wrapOAuthError` +- `cmd/msgvault/cmd/root_test.go` — remove tests for the deleted symbols +- `cmd/msgvault/cmd/addaccount.go` — replace BYO branch (`ClientSecretsFor` + `NewManager` + `errOAuthNotConfigured`/`wrapOAuthError`) with `resolveOAuthManager` call; remove the local `clientSecretsPath` variable +- `cmd/msgvault/cmd/addaccount_test.go` — add cases for the three resolver branches plus the named-app-not-found error +- `cmd/msgvault/cmd/deletions.go` — remove the `HasAnyConfig` gate, refactor the `!isServiceAccount` block, refactor the `getOAuthMgr` lambda, change `promptScopeEscalation`'s parameter from `clientSecretsPath` to `appName`, drop the local `clientSecretsPath` variable +- `cmd/msgvault/cmd/verify.go` — remove the `HasAnyConfig` gate; replace the OAuth fallback arm with `resolveOAuthManager` +- `cmd/msgvault/cmd/serve.go` — remove the `HasAnyConfig` startup gate +- `cmd/msgvault/cmd/syncfull.go` — remove the per-source `HasAnyConfig` skip +- `cmd/msgvault/cmd/sync.go` — remove the per-source `HasAnyConfig` skip +- `cmd/msgvault/cmd/setup.go` — remove `setupOAuthSecrets` and the call from `runSetup`; make the bundle's `[oauth] client_secrets` line conditional +- `cmd/msgvault/cmd/setup_test.go` — drop tests of the removed interactive prompt; update bundle tests to reflect the conditional `[oauth]` section +- `Makefile` — extend `LDFLAGS` with two more `-X` entries for the OAuth credentials +- `.github/workflows/release.yml` — inject `MSGVAULT_OAUTH_CLIENT_ID` and `MSGVAULT_OAUTH_CLIENT_SECRET` from GitHub Secrets into the release build step +- `README.md` — drop the "Follow the OAuth Setup Guide" prerequisite from Quick Start; add an "Advanced: bring your own OAuth client" subsection +- `cmd/msgvault/cmd/quickstart.md` — same shape as README updates + +--- + +## Task 1: Add `ScopesEmbedded` scope set + +**Files:** +- Modify: `internal/oauth/oauth.go` (around line 35, after `ScopesDeletion`) +- Test: `internal/oauth/oauth_test.go` (add a new test function) + +- [ ] **Step 1: Write the failing test** + +Add this test to `internal/oauth/oauth_test.go`: + +```go +func TestScopesEmbedded(t *testing.T) { + want := []string{ + "https://www.googleapis.com/auth/gmail.readonly", + "https://www.googleapis.com/auth/gmail.modify", + "https://mail.google.com/", + } + if len(ScopesEmbedded) != len(want) { + t.Fatalf("ScopesEmbedded has %d entries, want %d", len(ScopesEmbedded), len(want)) + } + for i, scope := range want { + if ScopesEmbedded[i] != scope { + t.Errorf("ScopesEmbedded[%d] = %q, want %q", i, ScopesEmbedded[i], scope) + } + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test -tags "fts5 sqlite_vec" -run TestScopesEmbedded ./internal/oauth/` + +Expected: FAIL with `undefined: ScopesEmbedded`. + +- [ ] **Step 3: Add the variable** + +In `internal/oauth/oauth.go`, right after the `ScopesDeletion` block (around line 37), insert: + +```go +// ScopesEmbedded is the scope set requested by the centralized verified +// msgvault OAuth client. It is the union of Scopes and ScopesDeletion so +// users on the embedded path never need scope escalation for permanent +// delete. +var ScopesEmbedded = []string{ + "https://www.googleapis.com/auth/gmail.readonly", + "https://www.googleapis.com/auth/gmail.modify", + "https://mail.google.com/", +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test -tags "fts5 sqlite_vec" -run TestScopesEmbedded ./internal/oauth/` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add internal/oauth/oauth.go internal/oauth/oauth_test.go +git commit -m "feat(oauth): add ScopesEmbedded for the verified client" +``` + +--- + +## Task 2: Create embedded credentials module + +**Files:** +- Create: `internal/oauth/embedded.go` +- Create: `internal/oauth/embedded_test.go` + +- [ ] **Step 1: Write the failing tests** + +Create `internal/oauth/embedded_test.go`: + +```go +package oauth + +import ( + "log/slog" + "testing" + + "golang.org/x/oauth2/google" +) + +func TestHasEmbeddedCredentials(t *testing.T) { + // Save and restore package vars around the test + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + + tests := []struct { + name string + id string + secret string + want bool + }{ + {"both set", "id", "secret", true}, + {"id only", "id", "", false}, + {"secret only", "", "secret", false}, + {"neither", "", "", false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + oauthClientID = tc.id + oauthClientSecret = tc.secret + if got := HasEmbeddedCredentials(); got != tc.want { + t.Errorf("HasEmbeddedCredentials() = %v, want %v", got, tc.want) + } + }) + } +} + +func TestEmbeddedConfig(t *testing.T) { + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + oauthClientID = "test-client-id" + oauthClientSecret = "test-client-secret" + + scopes := []string{"scope-a", "scope-b"} + cfg := EmbeddedConfig(scopes) + + if cfg.ClientID != "test-client-id" { + t.Errorf("ClientID = %q, want %q", cfg.ClientID, "test-client-id") + } + if cfg.ClientSecret != "test-client-secret" { + t.Errorf("ClientSecret = %q, want %q", cfg.ClientSecret, "test-client-secret") + } + if len(cfg.Scopes) != 2 || cfg.Scopes[0] != "scope-a" || cfg.Scopes[1] != "scope-b" { + t.Errorf("Scopes = %v, want %v", cfg.Scopes, scopes) + } + if cfg.Endpoint != google.Endpoint { + t.Errorf("Endpoint = %v, want google.Endpoint", cfg.Endpoint) + } +} + +func TestNewEmbeddedManager(t *testing.T) { + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + oauthClientID = "test-client-id" + oauthClientSecret = "test-client-secret" + + tokensDir := t.TempDir() + mgr, err := NewEmbeddedManager(tokensDir, slog.Default(), ScopesEmbedded) + if err != nil { + t.Fatalf("NewEmbeddedManager: %v", err) + } + if mgr == nil { + t.Fatal("NewEmbeddedManager returned nil manager") + } + if mgr.tokensDir != tokensDir { + t.Errorf("tokensDir = %q, want %q", mgr.tokensDir, tokensDir) + } + if !mgr.isEmbedded { + t.Error("isEmbedded = false, want true") + } +} + +func TestNewEmbeddedManagerWithoutCredentials(t *testing.T) { + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + oauthClientID = "" + oauthClientSecret = "" + + _, err := NewEmbeddedManager(t.TempDir(), slog.Default(), ScopesEmbedded) + if err == nil { + t.Fatal("NewEmbeddedManager: want error when credentials are empty, got nil") + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `go test -tags "fts5 sqlite_vec" -run 'TestHasEmbeddedCredentials|TestEmbeddedConfig|TestNewEmbeddedManager' ./internal/oauth/` + +Expected: FAIL with `undefined: oauthClientID`, `undefined: HasEmbeddedCredentials`, `undefined: EmbeddedConfig`, `undefined: NewEmbeddedManager`, and `undefined: isEmbedded`. + +- [ ] **Step 3: Add the `isEmbedded` field to the Manager struct** + +In `internal/oauth/oauth.go`, modify the `Manager` struct (around lines 57-67) to add `isEmbedded bool` at the end: + +```go +type Manager struct { + config *oauth2.Config + tokensDir string + logger *slog.Logger + profileURL string + + browserFlowFn func(ctx context.Context, email string, launchBrowser bool) (*oauth2.Token, error) + + // isEmbedded is true when this manager uses the centralized verified + // OAuth client (via NewEmbeddedManager) rather than a BYO + // client_secrets file. Used to enable the verification-window + // fallback message on access_denied. + isEmbedded bool +} +``` + +- [ ] **Step 4: Create the embedded module** + +Create `internal/oauth/embedded.go`: + +```go +package oauth + +import ( + "fmt" + "log/slog" + + "golang.org/x/oauth2" + "golang.org/x/oauth2/google" +) + +// oauthClientID and oauthClientSecret hold the centralized verified +// msgvault OAuth client credentials. They are package vars (not consts) +// so release builds can override them via: +// +// go build -ldflags "-X github.com/wesm/msgvault/internal/oauth.oauthClientID=..." +// +// Per https://developers.google.com/identity/protocols/oauth2 the desktop +// client secret is "obviously not treated as a secret"; PKCE provides the +// flow security. The values below are the dev project's credentials, +// suitable for contributor builds. Production binaries override both. +var ( + oauthClientID = "TBD-msgvault-dev-client-id" + oauthClientSecret = "TBD-msgvault-dev-client-secret" +) + +// HasEmbeddedCredentials reports whether the package vars are non-empty. +// Forks that strip the values out (or contributors testing the fallback) +// will see this return false, in which case NewEmbeddedManager refuses +// to construct an embedded manager. +func HasEmbeddedCredentials() bool { + return oauthClientID != "" && oauthClientSecret != "" +} + +// EmbeddedConfig returns the oauth2.Config built from the embedded +// credentials. RedirectURL is set later inside Manager.browserFlow when +// the loopback port is known; the rest of the config is fixed here. +func EmbeddedConfig(scopes []string) *oauth2.Config { + return &oauth2.Config{ + ClientID: oauthClientID, + ClientSecret: oauthClientSecret, + Endpoint: google.Endpoint, + Scopes: scopes, + } +} + +// NewEmbeddedManager constructs a Manager backed by the centralized +// verified OAuth client. Returns an error when credentials are missing +// (forks, stripped builds, or contributors who blanked the vars +// locally). +func NewEmbeddedManager(tokensDir string, logger *slog.Logger, scopes []string) (*Manager, error) { + if !HasEmbeddedCredentials() { + return nil, fmt.Errorf("no embedded OAuth credentials in this build") + } + if logger == nil { + logger = slog.Default() + } + return &Manager{ + config: EmbeddedConfig(scopes), + tokensDir: tokensDir, + logger: logger, + isEmbedded: true, + }, nil +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `go test -tags "fts5 sqlite_vec" -run 'TestHasEmbeddedCredentials|TestEmbeddedConfig|TestNewEmbeddedManager' ./internal/oauth/` + +Expected: PASS. + +- [ ] **Step 6: Run the full oauth package tests to confirm no regressions** + +Run: `go test -tags "fts5 sqlite_vec" ./internal/oauth/` + +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add internal/oauth/embedded.go internal/oauth/embedded_test.go internal/oauth/oauth.go +git commit -m "feat(oauth): add embedded credentials module" +``` + +--- + +## Task 3: Add `errAccessDenied` typed error and detect it in the OAuth callback + +**Files:** +- Modify: `internal/oauth/oauth.go` (add `errAccessDenied`, modify `newCallbackHandler`) +- Test: `internal/oauth/oauth_test.go` (add callback test) + +- [ ] **Step 1: Write the failing test** + +Add to `internal/oauth/oauth_test.go`: + +```go +func TestCallbackHandlerAccessDenied(t *testing.T) { + mgr := &Manager{logger: slog.Default()} + codeChan := make(chan string, 1) + errChan := make(chan error, 1) + handler := mgr.newCallbackHandler("expected-state", codeChan, errChan) + + req := httptest.NewRequest("GET", "/callback?error=access_denied&state=expected-state", nil) + rec := httptest.NewRecorder() + handler(rec, req) + + select { + case err := <-errChan: + if !errors.Is(err, errAccessDenied) { + t.Errorf("callback error = %v, want errAccessDenied", err) + } + default: + t.Fatal("callback handler did not send an error") + } +} +``` + +You may need to add `"errors"`, `"log/slog"`, `"net/http/httptest"` imports to the test file if not already present. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test -tags "fts5 sqlite_vec" -run TestCallbackHandlerAccessDenied ./internal/oauth/` + +Expected: FAIL with `undefined: errAccessDenied`. + +- [ ] **Step 3: Add the typed error and modify the callback handler** + +In `internal/oauth/oauth.go`, near the existing error definitions (e.g., right after `TokenMismatchError`), add: + +```go +// errAccessDenied is returned by the OAuth callback when Google +// signals that the authorization was rejected. On the embedded path +// this is the failure mode for "caller is not on the test-user list" +// and "100-user lifetime cap reached" during the verification window; +// on the BYO path it usually means the user clicked Deny. +var errAccessDenied = errors.New("oauth: access_denied") +``` + +Make sure `"errors"` is imported. + +Now modify `newCallbackHandler` (around lines 210-226). Insert the access_denied check before the state check (since Google sends the error param when consent is denied and the state may still be present). The updated handler: + +```go +func (m *Manager) newCallbackHandler(expectedState string, codeChan chan<- string, errChan chan<- error) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if errParam := r.URL.Query().Get("error"); errParam != "" { + if errParam == "access_denied" { + errChan <- errAccessDenied + _, _ = fmt.Fprintf(w, "Authorization denied. You can close this window.") + return + } + errChan <- fmt.Errorf("oauth callback error: %s", errParam) + _, _ = fmt.Fprintf(w, "Error: %s", errParam) + return + } + if r.URL.Query().Get("state") != expectedState { + errChan <- fmt.Errorf("state mismatch: possible CSRF attack") + _, _ = fmt.Fprintf(w, "Error: state mismatch") + return + } + code := r.URL.Query().Get("code") + if code == "" { + errChan <- fmt.Errorf("no code in callback") + _, _ = fmt.Fprintf(w, "Error: no authorization code received") + return + } + codeChan <- code + _, _ = fmt.Fprintf(w, "Authorization successful! You can close this window.") + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test -tags "fts5 sqlite_vec" -run TestCallbackHandlerAccessDenied ./internal/oauth/` + +Expected: PASS. + +- [ ] **Step 5: Run the full oauth package tests to confirm no regressions** + +Run: `go test -tags "fts5 sqlite_vec" ./internal/oauth/` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add internal/oauth/oauth.go internal/oauth/oauth_test.go +git commit -m "feat(oauth): detect access_denied in OAuth callback" +``` + +--- + +## Task 4: Print embedded-path fallback message on access_denied + +**Files:** +- Modify: `internal/oauth/oauth.go` (modify `authorize`) +- Test: `internal/oauth/oauth_test.go` (add test using `browserFlowFn` override) + +- [ ] **Step 1: Write the failing test** + +Add to `internal/oauth/oauth_test.go`: + +```go +func TestAuthorizeEmbeddedFallbackMessage(t *testing.T) { + tokensDir := t.TempDir() + mgr := &Manager{ + config: &oauth2.Config{ClientID: "x", ClientSecret: "y", Scopes: []string{"s"}}, + tokensDir: tokensDir, + logger: slog.Default(), + isEmbedded: true, + browserFlowFn: func(ctx context.Context, email string, launchBrowser bool) (*oauth2.Token, error) { + return nil, errAccessDenied + }, + } + + var buf bytes.Buffer + origStdout := stdout + stdout = &buf + defer func() { stdout = origStdout }() + + err := mgr.Authorize(context.Background(), "u@example.com") + if !errors.Is(err, errAccessDenied) { + t.Fatalf("Authorize error = %v, want errAccessDenied", err) + } + if !strings.Contains(buf.String(), "still in Google's verification") { + t.Errorf("expected fallback message, got: %q", buf.String()) + } +} + +func TestAuthorizeNonEmbeddedNoFallback(t *testing.T) { + mgr := &Manager{ + config: &oauth2.Config{ClientID: "x", ClientSecret: "y", Scopes: []string{"s"}}, + tokensDir: t.TempDir(), + logger: slog.Default(), + // isEmbedded: false (default) + browserFlowFn: func(ctx context.Context, email string, launchBrowser bool) (*oauth2.Token, error) { + return nil, errAccessDenied + }, + } + + var buf bytes.Buffer + origStdout := stdout + stdout = &buf + defer func() { stdout = origStdout }() + + err := mgr.Authorize(context.Background(), "u@example.com") + if !errors.Is(err, errAccessDenied) { + t.Fatalf("Authorize error = %v, want errAccessDenied", err) + } + if strings.Contains(buf.String(), "still in Google's verification") { + t.Errorf("did not expect fallback message for non-embedded, got: %q", buf.String()) + } +} +``` + +You may need to add `"bytes"`, `"strings"`, `"context"`, `"io"` imports if not already present. + +- [ ] **Step 2: Add a package-level `stdout io.Writer` so tests can capture output** + +In `internal/oauth/oauth.go`, near the top of the file (after the imports and existing package vars), add: + +```go +// stdout is the destination for user-facing messages printed during +// the OAuth flow. Replaceable in tests via the var to capture output. +var stdout io.Writer = os.Stdout +``` + +Add `"io"` to the imports if not already present. + +- [ ] **Step 3: Run tests to verify they fail** + +Run: `go test -tags "fts5 sqlite_vec" -run 'TestAuthorizeEmbedded|TestAuthorizeNonEmbedded' ./internal/oauth/` + +Expected: FAIL (the fallback message is not yet printed). + +- [ ] **Step 4: Modify `authorize` to print the fallback on embedded access_denied** + +In `internal/oauth/oauth.go`, modify the `authorize` method (around lines 180-202) to detect `errAccessDenied` on the embedded path and print the fallback message before returning. Replace the function body with: + +```go +func (m *Manager) authorize( + ctx context.Context, email string, launchBrowser bool, +) error { + flow := m.browserFlow + if m.browserFlowFn != nil { + flow = m.browserFlowFn + } + token, err := flow(ctx, email, launchBrowser) + if err != nil { + if m.isEmbedded && errors.Is(err, errAccessDenied) { + fmt.Fprint(stdout, embeddedFallbackMessage) + } + return err + } + + if _, err := m.resolveTokenEmail(ctx, email, token); err != nil { + return err + } + + return m.saveToken(email, token, m.config.Scopes) +} +``` + +And add the fallback message constant near `errAccessDenied`: + +```go +const embeddedFallbackMessage = ` +msgvault's centralized OAuth client is still in Google's verification +queue. Two options: + + 1. Use the bring-your-own setup (one-time, ~5 minutes): + https://msgvault.io/guides/oauth-setup/ + + 2. Request beta access (open a GitHub issue with your Gmail address): + https://github.com/wesm/msgvault/issues/new?template=beta-oauth.md + +` +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `go test -tags "fts5 sqlite_vec" -run 'TestAuthorizeEmbedded|TestAuthorizeNonEmbedded' ./internal/oauth/` + +Expected: PASS. + +- [ ] **Step 6: Run the full oauth package tests to confirm no regressions** + +Run: `go test -tags "fts5 sqlite_vec" ./internal/oauth/` + +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add internal/oauth/oauth.go internal/oauth/oauth_test.go +git commit -m "feat(oauth): print fallback message on embedded access_denied" +``` + +--- + +## Task 5: Create `resolveOAuthManager` helper + +**Files:** +- Create: `cmd/msgvault/cmd/oauth_resolve.go` +- Create: `cmd/msgvault/cmd/oauth_resolve_test.go` + +- [ ] **Step 1: Write the failing tests** + +Create `cmd/msgvault/cmd/oauth_resolve_test.go`: + +```go +package cmd + +import ( + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/wesm/msgvault/internal/config" + "github.com/wesm/msgvault/internal/oauth" +) + +// writeStubClientSecrets writes a minimal valid client_secret.json that +// parseClientSecrets will accept. We only need this to verify the BYO +// path returns a non-nil manager — we don't run any OAuth flow. +func writeStubClientSecrets(t *testing.T, dir, name string) string { + t.Helper() + path := filepath.Join(dir, name) + const stub = `{"installed":{"client_id":"abc","client_secret":"xyz","redirect_uris":["http://localhost"]}}` + if err := os.WriteFile(path, []byte(stub), 0600); err != nil { + t.Fatalf("write %s: %v", path, err) + } + return path +} + +// newTestConfig returns a Config with Data.DataDir set to a fresh temp +// directory. TokensDir() returns /tokens, which is what the +// resolver passes to the OAuth manager constructors. +func newTestConfig(t *testing.T) *config.Config { + t.Helper() + return &config.Config{ + Data: config.DataConfig{DataDir: t.TempDir()}, + } +} + +func TestResolveOAuthManager_NamedBYO(t *testing.T) { + cfg := newTestConfig(t) + secrets := writeStubClientSecrets(t, cfg.Data.DataDir, "acme.json") + cfg.OAuth.Apps = map[string]config.OAuthApp{"acme": {ClientSecrets: secrets}} + mgr, err := resolveOAuthManager(cfg, "acme", oauth.Scopes, slog.Default()) + if err != nil { + t.Fatalf("resolveOAuthManager: %v", err) + } + if mgr == nil { + t.Fatal("manager is nil") + } +} + +func TestResolveOAuthManager_NamedNotConfigured(t *testing.T) { + cfg := newTestConfig(t) + _, err := resolveOAuthManager(cfg, "nonexistent", oauth.Scopes, slog.Default()) + if err == nil { + t.Fatal("expected error for unknown app name") + } + if !strings.Contains(err.Error(), "nonexistent") { + t.Errorf("error %q should mention the app name", err.Error()) + } +} + +func TestResolveOAuthManager_GlobalBYO(t *testing.T) { + cfg := newTestConfig(t) + cfg.OAuth.ClientSecrets = writeStubClientSecrets(t, cfg.Data.DataDir, "default.json") + mgr, err := resolveOAuthManager(cfg, "", oauth.Scopes, slog.Default()) + if err != nil { + t.Fatalf("resolveOAuthManager: %v", err) + } + if mgr == nil { + t.Fatal("manager is nil") + } +} + +func TestResolveOAuthManager_Embedded(t *testing.T) { + // Embedded credentials must be non-empty in this test (they are by + // default — the source has the dev placeholder strings). + cfg := newTestConfig(t) + mgr, err := resolveOAuthManager(cfg, "", oauth.Scopes, slog.Default()) + if err != nil { + t.Fatalf("resolveOAuthManager: %v", err) + } + if mgr == nil { + t.Fatal("manager is nil") + } +} +``` + +You may need to check the exact name of the Data config struct in `internal/config/config.go` — the field on Config is called `Data` and its `DataDir` field holds the path. If the struct type name differs (e.g., `DataConfig` vs another name), adjust the import or the literal accordingly. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `go test -tags "fts5 sqlite_vec" -run TestResolveOAuthManager ./cmd/msgvault/cmd/` + +Expected: FAIL with `undefined: resolveOAuthManager`. + +- [ ] **Step 3: Create the resolver helper** + +Create `cmd/msgvault/cmd/oauth_resolve.go`: + +```go +package cmd + +import ( + "fmt" + "log/slog" + + "github.com/wesm/msgvault/internal/config" + "github.com/wesm/msgvault/internal/oauth" +) + +// resolveOAuthManager builds the *oauth.Manager appropriate for the +// account+config+scopes triple. Resolution order: +// +// 1. Named BYO: appName is non-empty and cfg.OAuth.Apps[appName] has +// client_secrets set — use that BYO OAuth client. +// 2. (If appName is non-empty but no client_secrets is registered for +// it) — return an error rather than falling through to embedded, +// because the user explicitly named a binding that doesn't exist. +// 3. Global BYO: appName is empty and cfg.OAuth.ClientSecrets is set — +// use the global BYO client. +// 4. Embedded: otherwise, use the centralized verified client. On the +// embedded path the manager is always built with oauth.ScopesEmbedded, +// ignoring the caller's per-call scope choice, because the embedded +// grant is broader than any per-call need. +// +// Callers handle service-account resolution themselves (via +// cfg.OAuth.ServiceAccountKeyFor(appName)) before calling this helper, +// because *oauth.Manager and the service-account manager have +// different interfaces. +func resolveOAuthManager( + cfg *config.Config, + appName string, + scopes []string, + logger *slog.Logger, +) (*oauth.Manager, error) { + if appName != "" { + app, ok := cfg.OAuth.Apps[appName] + if !ok || app.ClientSecrets == "" { + return nil, fmt.Errorf("OAuth app %q not configured (add [oauth.apps.%s] client_secrets to config.toml, or omit --oauth-app to use the embedded client)", appName, appName) + } + return oauth.NewManagerWithScopes(app.ClientSecrets, cfg.TokensDir(), logger, scopes) + } + + if cfg.OAuth.ClientSecrets != "" { + return oauth.NewManagerWithScopes(cfg.OAuth.ClientSecrets, cfg.TokensDir(), logger, scopes) + } + + return oauth.NewEmbeddedManager(cfg.TokensDir(), logger, oauth.ScopesEmbedded) +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestResolveOAuthManager ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add cmd/msgvault/cmd/oauth_resolve.go cmd/msgvault/cmd/oauth_resolve_test.go +git commit -m "feat(oauth): add resolveOAuthManager three-way helper" +``` + +--- + +## Task 6: Wire `oauthManagerCache` to use `resolveOAuthManager` + +**Files:** +- Modify: `cmd/msgvault/cmd/root.go` (the `oauthManagerCache` function at lines 419-442) + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Refactor `oauthManagerCache` to delegate to the resolver** + +In `cmd/msgvault/cmd/root.go`, replace the existing `oauthManagerCache` (around line 419) with: + +```go +// oauthManagerCache returns a resolver function that lazily creates and +// caches oauth.Manager instances keyed by app name. The cache is safe +// for concurrent use (serve runs scheduled syncs in goroutines). The +// underlying resolution is delegated to resolveOAuthManager. +func oauthManagerCache() func(appName string) (*oauth.Manager, error) { + var mu sync.Mutex + managers := map[string]*oauth.Manager{} + return func(appName string) (*oauth.Manager, error) { + mu.Lock() + defer mu.Unlock() + if mgr, ok := managers[appName]; ok { + return mgr, nil + } + mgr, err := resolveOAuthManager(cfg, appName, oauth.Scopes, logger) + if err != nil { + return nil, err + } + managers[appName] = mgr + return mgr, nil + } +} +``` + +(`wrapOAuthError` is no longer needed here; we'll remove it in Task 13.) + +- [ ] **Step 3: Run tests to verify behavior is preserved** + +Run: `go test -tags "fts5 sqlite_vec" ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 4: Commit** + +```bash +git add cmd/msgvault/cmd/root.go +git commit -m "refactor(oauth): route oauthManagerCache through resolveOAuthManager" +``` + +--- + +## Task 7: Refactor `addaccount.go` BYO branch + +**Files:** +- Modify: `cmd/msgvault/cmd/addaccount.go` (around lines 164-176) +- Test: `cmd/msgvault/cmd/addaccount_test.go` + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestAddAccount ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Replace the BYO branch and remove the unused `clientSecretsPath` variable** + +`clientSecretsPath` in `addaccount.go` is declared at line 51 and only used at lines 165 and 174 (both in the branch we're replacing). After the refactor it has no callers, so delete the declaration too. + +a. Delete the `var clientSecretsPath string` line (around line 51). + +b. Replace the BYO branch (around lines 164-176): + +```go +// Resolve client secrets path (standard OAuth flow) +clientSecretsPath, err = cfg.OAuth.ClientSecretsFor(resolvedApp) +if err != nil { + if !cfg.OAuth.HasAnyConfig() { + return errOAuthNotConfigured() + } + return err +} + +// Create OAuth manager +oauthMgr, err := oauth.NewManager(clientSecretsPath, cfg.TokensDir(), logger) +if err != nil { + return wrapOAuthError(fmt.Errorf("create oauth manager: %w", err)) +} +``` + +with: + +```go +// Build the OAuth manager. resolveOAuthManager handles named BYO, +// global BYO, and the embedded fallback automatically. +oauthMgr, err := resolveOAuthManager(cfg, resolvedApp, oauth.Scopes, logger) +if err != nil { + return err +} +``` + +- [ ] **Step 3: Add resolution-branch tests to addaccount_test.go** + +Add to `cmd/msgvault/cmd/addaccount_test.go`: + +```go +func TestAddAccount_ResolverBranches(t *testing.T) { + tests := []struct { + name string + appName string + setup func(cfg *config.Config, t *testing.T) + wantErr bool + errContains string + }{ + { + name: "named BYO with client_secrets", + appName: "acme", + setup: func(cfg *config.Config, t *testing.T) { + path := writeStubClientSecrets(t, cfg.Data.DataDir, "acme.json") + cfg.OAuth.Apps = map[string]config.OAuthApp{"acme": {ClientSecrets: path}} + }, + wantErr: false, + }, + { + name: "named app without client_secrets", + appName: "missing", + setup: func(cfg *config.Config, t *testing.T) {}, + wantErr: true, + errContains: "missing", + }, + { + name: "global BYO", + appName: "", + setup: func(cfg *config.Config, t *testing.T) { + cfg.OAuth.ClientSecrets = writeStubClientSecrets(t, cfg.Data.DataDir, "default.json") + }, + wantErr: false, + }, + { + name: "no config falls through to embedded", + appName: "", + setup: func(cfg *config.Config, t *testing.T) {}, + wantErr: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := newTestConfig(t) + tc.setup(cfg, t) + _, err := resolveOAuthManager(cfg, tc.appName, oauth.Scopes, slog.Default()) + if tc.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + if tc.errContains != "" && !strings.Contains(err.Error(), tc.errContains) { + t.Errorf("error %q should contain %q", err.Error(), tc.errContains) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} +``` + +(`writeStubClientSecrets` and `newTestConfig` were added in Task 5. Both test files can share them because they're in the same package.) + +- [ ] **Step 4: Run tests to verify everything passes** + +Run: `go test -tags "fts5 sqlite_vec" -run 'TestAddAccount|TestResolveOAuthManager' ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 5: Build the project to catch unused-variable errors** + +Run: `go build -tags "fts5 sqlite_vec" ./...` + +Expected: success. + +- [ ] **Step 6: Commit** + +```bash +git add cmd/msgvault/cmd/addaccount.go cmd/msgvault/cmd/addaccount_test.go +git commit -m "refactor(oauth): route addaccount BYO branch through resolveOAuthManager" +``` + +--- + +## Task 8: Refactor `deletions.go` BYO branches + +**Files:** +- Modify: `cmd/msgvault/cmd/deletions.go` (HasAnyConfig gate ~line 428, BYO branch ~lines 434-456, `getOAuthMgr` lambda ~lines 460-474, `promptScopeEscalation` body ~lines 680+) + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestDeletion ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Refactor the `!isServiceAccount` block** + +In `cmd/msgvault/cmd/deletions.go`, the relevant section today reads (around lines 426-457): + +```go +var clientSecretsPath string +if src.SourceType == "gmail" { + if !cfg.OAuth.HasAnyConfig() { + return errOAuthNotConfigured() + } + appName := sourceOAuthApp(src) + isServiceAccount := cfg.OAuth.ServiceAccountKeyFor(appName) != "" + + if !isServiceAccount { + clientSecretsPath, err = cfg.OAuth.ClientSecretsFor(appName) + if err != nil { + return err + } + + needsBatchDelete := !deleteTrash + if needsBatchDelete { + requiredScopes := oauth.ScopesDeletion + oauthMgr, err := oauth.NewManagerWithScopes(clientSecretsPath, cfg.TokensDir(), logger, requiredScopes) + if err != nil { + return wrapOAuthError(fmt.Errorf("create oauth manager: %w", err)) + } + if !oauthMgr.HasScope(account, "https://mail.google.com/") && oauthMgr.HasScopeMetadata(account) { + if err := promptScopeEscalation(ctx, oauthMgr, account, needsBatchDelete, clientSecretsPath); err != nil { + if errors.Is(err, errUserCanceled) { + return nil + } + return err + } + } + } + } +} +``` + +Replace with (deleting both the `HasAnyConfig` gate AND the `clientSecretsPath` declaration, and updating the `promptScopeEscalation` call to pass `appName`): + +```go +if src.SourceType == "gmail" { + appName := sourceOAuthApp(src) + isServiceAccount := cfg.OAuth.ServiceAccountKeyFor(appName) != "" + + if !isServiceAccount { + needsBatchDelete := !deleteTrash + if needsBatchDelete { + oauthMgr, err := resolveOAuthManager(cfg, appName, oauth.ScopesDeletion, logger) + if err != nil { + return err + } + if !oauthMgr.HasScope(account, "https://mail.google.com/") && oauthMgr.HasScopeMetadata(account) { + if err := promptScopeEscalation(ctx, oauthMgr, account, needsBatchDelete, appName); err != nil { + if errors.Is(err, errUserCanceled) { + return nil + } + return err + } + } + } + } +} +``` + +The outer `var clientSecretsPath string` declaration at line 426 goes away. The `if !cfg.OAuth.HasAnyConfig() { return errOAuthNotConfigured() }` gate goes away. The `appName` variable still lives inside the `if src.SourceType == "gmail"` block. + +- [ ] **Step 3: Refactor the `getOAuthMgr` lambda** + +The lambda at lines 460-474 captures the outer `clientSecretsPath` (which no longer exists after Step 2). Replace the lambda body: + +```go +getOAuthMgr := func(appName string) (*oauth.Manager, error) { + secretsPath := clientSecretsPath + if secretsPath == "" { + var err error + secretsPath, err = cfg.OAuth.ClientSecretsFor(appName) + if err != nil { + return nil, err + } + } + scopes := oauth.Scopes + if !deleteTrash { + scopes = oauth.ScopesDeletion + } + return oauth.NewManagerWithScopes(secretsPath, cfg.TokensDir(), logger, scopes) +} +``` + +with: + +```go +getOAuthMgr := func(appName string) (*oauth.Manager, error) { + scopes := oauth.Scopes + if !deleteTrash { + scopes = oauth.ScopesDeletion + } + return resolveOAuthManager(cfg, appName, scopes, logger) +} +``` + +- [ ] **Step 4: Change `promptScopeEscalation` signature from `clientSecretsPath` to `appName`** + +In `cmd/msgvault/cmd/deletions.go`, find `func promptScopeEscalation` (around line 680). Today its signature ends with `, clientSecretsPath string`: + +```go +func promptScopeEscalation(ctx context.Context, oauthMgr *oauth.Manager, account string, batchDelete bool, clientSecretsPath string) error { +``` + +Change to: + +```go +func promptScopeEscalation(ctx context.Context, oauthMgr *oauth.Manager, account string, batchDelete bool, appName string) error { +``` + +Inside the function body, find the manager re-creation after the user opts into elevated scopes (it calls `oauth.NewManagerWithScopes(clientSecretsPath, cfg.TokensDir(), logger, requiredScopes)`). Replace it with: + +```go +newMgr, err := resolveOAuthManager(cfg, appName, requiredScopes, logger) +if err != nil { + return err +} +``` + +(Adapt the variable name `newMgr` to whatever the function uses internally.) + +Both callers of `promptScopeEscalation` already have `appName` available (the first one as a local var in the `if src.SourceType == "gmail"` block, the second one as `sourceOAuthApp(src)` computed inline). The first caller updated in Step 2 already passes `appName`; the second caller (around line 537) currently passes `clientSecretsPath` and needs updating: + +```go +// Before +if err := promptScopeEscalation(ctx, oauthMgr, account, !useTrash, clientSecretsPath); err != nil { +// After +if err := promptScopeEscalation(ctx, oauthMgr, account, !useTrash, sourceOAuthApp(src)); err != nil { +``` + +- [ ] **Step 5: Build to catch any remaining references** + +Run: `go build -tags "fts5 sqlite_vec" ./...` + +Expected: success. Any leftover references to `clientSecretsPath` or `wrapOAuthError` in `deletions.go` mean a step was skipped. + +- [ ] **Step 6: Run all deletions tests** + +Run: `go test -tags "fts5 sqlite_vec" -run TestDeletion ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add cmd/msgvault/cmd/deletions.go +git commit -m "refactor(oauth): route deletions through resolveOAuthManager" +``` + +--- + +## Task 9: Refactor `verify.go` — remove HasAnyConfig gate and route OAuth fallback through resolver + +**Files:** +- Modify: `cmd/msgvault/cmd/verify.go` (HasAnyConfig gate ~line 95, OAuth fallback ~lines 126-132) + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestVerify ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Remove the HasAnyConfig gate** + +In `cmd/msgvault/cmd/verify.go`, find and delete the block (around lines 94-96): + +```go +if !cfg.OAuth.HasAnyConfig() { + return errOAuthNotConfigured() +} +``` + +This gate prevented `verify` from running without OAuth config; with the embedded fallthrough every Gmail source can be verified. + +- [ ] **Step 3: Replace the OAuth fallback arm** + +Find the block (around lines 126-132): + +```go +} else { + clientSecretsPath, secretsErr := cfg.OAuth.ClientSecretsFor(appName) + if secretsErr != nil { + return secretsErr + } + oauthMgr, mgrErr := oauth.NewManager(clientSecretsPath, cfg.TokensDir(), logger) + if mgrErr != nil { + return wrapOAuthError(fmt.Errorf("create oauth manager: %w", mgrErr)) + } + // ... rest of the OAuth path +} +``` + +Replace the resolution lines with: + +```go +} else { + oauthMgr, mgrErr := resolveOAuthManager(cfg, appName, oauth.Scopes, logger) + if mgrErr != nil { + return mgrErr + } + // ... rest of the OAuth path +} +``` + +- [ ] **Step 4: Build to catch unused-variable errors** + +Run: `go build -tags "fts5 sqlite_vec" ./...` + +Expected: success. + +- [ ] **Step 5: Run verify tests** + +Run: `go test -tags "fts5 sqlite_vec" -run TestVerify ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add cmd/msgvault/cmd/verify.go +git commit -m "refactor(oauth): route verify through resolveOAuthManager" +``` + +--- + +## Task 10: Remove `HasAnyConfig` startup gate in `serve.go` + +**Files:** +- Modify: `cmd/msgvault/cmd/serve.go` (around line 68) + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestServe ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Delete the gate** + +In `cmd/msgvault/cmd/serve.go`, find and delete the block (around lines 68-70): + +```go +if !cfg.OAuth.HasAnyConfig() { + return errOAuthNotConfigured() +} +``` + +- [ ] **Step 3: Build and test** + +Run: `go build -tags "fts5 sqlite_vec" ./...` +Run: `go test -tags "fts5 sqlite_vec" -run TestServe ./cmd/msgvault/cmd/` + +Expected: both succeed. + +- [ ] **Step 4: Commit** + +```bash +git add cmd/msgvault/cmd/serve.go +git commit -m "refactor(oauth): remove HasAnyConfig gate from serve" +``` + +--- + +## Task 11: Remove per-source `HasAnyConfig` skip in `syncfull.go` + +**Files:** +- Modify: `cmd/msgvault/cmd/syncfull.go` (around line 118) + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestSyncFull ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Delete the skip block** + +In `cmd/msgvault/cmd/syncfull.go`, find the multi-source loop and delete the block (around lines 118-121): + +```go +if !cfg.OAuth.HasAnyConfig() { + fmt.Printf("Skipping %s (OAuth not configured)\n", src.Identifier) + continue +} +``` + +The loop continues with the `appName := sourceOAuthApp(src)` line and downstream calls to `getOAuthMgr(appName)`. + +- [ ] **Step 3: Build and test** + +Run: `go build -tags "fts5 sqlite_vec" ./...` +Run: `go test -tags "fts5 sqlite_vec" -run TestSyncFull ./cmd/msgvault/cmd/` + +Expected: both succeed. + +- [ ] **Step 4: Commit** + +```bash +git add cmd/msgvault/cmd/syncfull.go +git commit -m "refactor(oauth): drop per-source skip in syncfull" +``` + +--- + +## Task 12: Remove per-source `HasAnyConfig` skip in `sync.go` + +**Files:** +- Modify: `cmd/msgvault/cmd/sync.go` (around line 128) + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestSync ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Delete the skip block** + +In `cmd/msgvault/cmd/sync.go`, find the multi-source loop and delete the block (around lines 128-131): + +```go +if !cfg.OAuth.HasAnyConfig() { + fmt.Printf("Skipping %s (OAuth not configured)\n", src.Identifier) + continue +} +``` + +- [ ] **Step 3: Build and test** + +Run: `go build -tags "fts5 sqlite_vec" ./...` +Run: `go test -tags "fts5 sqlite_vec" -run TestSync ./cmd/msgvault/cmd/` + +Expected: both succeed. + +- [ ] **Step 4: Commit** + +```bash +git add cmd/msgvault/cmd/sync.go +git commit -m "refactor(oauth): drop per-source skip in sync-incremental" +``` + +--- + +## Task 13: Remove dead OAuth-setup helpers from `root.go` + +**Files:** +- Modify: `cmd/msgvault/cmd/root.go` (delete `errOAuthNotConfigured`, `tryFindClientSecrets`, `oauthSetupHint`, `wrapOAuthError`) +- Modify: `cmd/msgvault/cmd/root_test.go` (delete tests for the deleted symbols) + +- [ ] **Step 1: Verify there are no remaining callers** + +Run: `grep -rn 'errOAuthNotConfigured\|tryFindClientSecrets\|oauthSetupHint\|wrapOAuthError' cmd/ internal/` + +Expected output: only matches inside `cmd/msgvault/cmd/root.go` (the definitions) and `cmd/msgvault/cmd/root_test.go` (their tests). If anything else shows up, refactor those sites before continuing. + +- [ ] **Step 2: Delete the helpers and their tests** + +In `cmd/msgvault/cmd/root.go`, delete the following functions (the line ranges from the previous tasks may have shifted; locate by symbol): + +- `errOAuthNotConfigured` (around line 277) +- `tryFindClientSecrets` (around line 288) +- `oauthSetupHint` (around line 256) +- `wrapOAuthError` (around line 322) + +In `cmd/msgvault/cmd/root_test.go`, delete the corresponding tests: + +- `TestErrOAuthNotConfigured` +- Any tests that call `wrapOAuthError` (search for the name) + +- [ ] **Step 3: Build and test** + +Run: `go build -tags "fts5 sqlite_vec" ./...` +Run: `go test -tags "fts5 sqlite_vec" ./...` + +Expected: both succeed. + +- [ ] **Step 4: Commit** + +```bash +git add cmd/msgvault/cmd/root.go cmd/msgvault/cmd/root_test.go +git commit -m "refactor(oauth): remove dead OAuth-setup error helpers" +``` + +--- + +## Task 14: Remove `OAuthConfig.HasAnyConfig` + +**Files:** +- Modify: `internal/config/config.go` (delete the `HasAnyConfig` method) +- Modify: `internal/config/config_test.go` (delete `TestOAuthConfig_HasAnyConfig` and inline assertions) + +- [ ] **Step 1: Verify there are no remaining callers** + +Run: `grep -rn 'HasAnyConfig' --include='*.go'` + +Expected: only matches inside `internal/config/config.go` (definition) and `internal/config/config_test.go` (tests, plus comments). + +- [ ] **Step 2: Delete the method and its tests** + +In `internal/config/config.go`, delete the `HasAnyConfig` method (around lines 178-188). + +In `internal/config/config_test.go`: + +- Delete `TestOAuthConfig_HasAnyConfig` (lines around 1240-1308). +- In `TestLoadWithNamedOAuthApps` (around line 1310), remove the block (around lines 1363-1367) that asserts on `HasAnyConfig`. +- In `TestLoadWithNamedOAuthApps_RelativePaths` (around line 1484), remove the block (around lines 1604-1608) that asserts on `HasAnyConfig`. + +- [ ] **Step 3: Build and test** + +Run: `go build -tags "fts5 sqlite_vec" ./...` +Run: `go test -tags "fts5 sqlite_vec" ./internal/config/` + +Expected: both succeed. + +- [ ] **Step 4: Commit** + +```bash +git add internal/config/config.go internal/config/config_test.go +git commit -m "refactor(oauth): remove OAuthConfig.HasAnyConfig" +``` + +--- + +## Task 15: Remove interactive OAuth prompt from `setup` and adjust bundle behavior + +**Files:** +- Modify: `cmd/msgvault/cmd/setup.go` +- Modify: `cmd/msgvault/cmd/setup_test.go` + +- [ ] **Step 1: Verify existing tests pass** + +Run: `go test -tags "fts5 sqlite_vec" -run TestSetup ./cmd/msgvault/cmd/` + +Expected: PASS. + +- [ ] **Step 2: Remove `setupOAuthSecrets` and update `runSetup`** + +In `cmd/msgvault/cmd/setup.go`: + +a. Delete `func setupOAuthSecrets(reader *bufio.Reader) (string, error)` (lines 100-143). + +b. Update `runSetup` to remove the OAuth step. Replace the body with: + +```go +func runSetup(cmd *cobra.Command, args []string) error { + reader := bufio.NewReader(os.Stdin) + + fmt.Println("Welcome to msgvault setup!") + fmt.Println() + + if err := cfg.EnsureHomeDir(); err != nil { + return fmt.Errorf("create home directory: %w", err) + } + + // Configure remote NAS (optional). msgvault now ships with an + // embedded verified OAuth client, so the old "Step 1: OAuth + // credentials" prompt is gone. Operators who want their own OAuth + // client can still set [oauth] client_secrets in config.toml + // manually. + remoteURL, remoteAPIKey, err := setupRemoteServer(reader) + if err != nil { + return err + } + + if remoteURL != "" { + cfg.Remote.URL = remoteURL + cfg.Remote.APIKey = remoteAPIKey + if strings.HasPrefix(remoteURL, "http://") { + cfg.Remote.AllowInsecure = true + } + } + + if remoteURL != "" { + if err := cfg.Save(); err != nil { + return fmt.Errorf("save config: %w", err) + } + fmt.Printf("\nConfiguration saved to %s\n", cfg.ConfigFilePath()) + } + + fmt.Println() + fmt.Println("Setup complete! Next steps:") + fmt.Println() + fmt.Println(" 1. Add a Gmail account:") + fmt.Println(" msgvault add-account you@gmail.com") + fmt.Println() + fmt.Println(" 2. Sync your emails:") + fmt.Println(" msgvault sync-full you@gmail.com") + fmt.Println() + if remoteURL != "" { + fmt.Println(" 3. Export token to your NAS (after add-account):") + fmt.Println(" msgvault export-token you@gmail.com") + fmt.Println() + } + fmt.Println("For more help: msgvault --help") + + return nil +} +``` + +c. Update `setupRemoteServer` signature to drop the unused `oauthSecretsPath` parameter: + +```go +func setupRemoteServer(reader *bufio.Reader) (string, string, error) { + // ... same body but reference cfg.OAuth.ClientSecrets directly +} +``` + +Inside `setupRemoteServer`, replace the `effectiveSecrets` lines (around 203-205) with: + +```go +effectiveSecrets := cfg.OAuth.ClientSecrets // empty when operator uses embedded +``` + +d. Update `createNASBundle` so the generated `config.toml` only includes the `[oauth]` section when `oauthSecretsPath != ""`. Replace the `nasConfig` string construction with: + +```go +nasConfig := fmt.Sprintf(`[server] +bind_addr = "0.0.0.0" +api_port = 8080 +api_key = %q + +[sync] +rate_limit_qps = 5 + +# Accounts will be added automatically when you export tokens. +# You can also add them manually: +# [[accounts]] +# email = "you@gmail.com" +# schedule = "0 2 * * *" +# enabled = true +`, apiKey) + +if oauthSecretsPath != "" { + nasConfig += ` +[oauth] +client_secrets = "/data/client_secret.json" +` +} +``` + +Update the printed instructions inside `setupRemoteServer` to only mention `client_secret.json` when one was bundled: + +```go +fmt.Printf("\nNAS deployment files created in: %s\n", bundleDir) +fmt.Println(" - config.toml (ready for NAS)") +if effectiveSecrets != "" { + fmt.Println(" - client_secret.json (copy of OAuth credentials)") +} +fmt.Println(" - docker-compose.yml (ready to deploy)") +``` + +(That last bit is already the existing structure; verify it still reads correctly after the surrounding edits.) + +- [ ] **Step 3: Update `setup_test.go`** + +In `cmd/msgvault/cmd/setup_test.go`: + +- Delete any tests that directly exercise `setupOAuthSecrets`. +- Update the bundle tests so the "no secrets path given" case verifies the generated `config.toml` does NOT contain a `[oauth]` section. +- Update the "secrets path given" case to verify the bundle contains both `client_secret.json` AND a `config.toml` with `[oauth] client_secrets = "/data/client_secret.json"`. + +Concretely, for the "no secrets" case, add an assertion: + +```go +configBytes, err := os.ReadFile(filepath.Join(bundleDir, "config.toml")) +if err != nil { + t.Fatalf("read bundle config: %v", err) +} +if strings.Contains(string(configBytes), "[oauth]") { + t.Error("config.toml should not contain [oauth] section when no secrets provided") +} +``` + +- [ ] **Step 4: Build and test** + +Run: `go build -tags "fts5 sqlite_vec" ./...` +Run: `go test -tags "fts5 sqlite_vec" -run TestSetup ./cmd/msgvault/cmd/` + +Expected: both succeed. + +- [ ] **Step 5: Commit** + +```bash +git add cmd/msgvault/cmd/setup.go cmd/msgvault/cmd/setup_test.go +git commit -m "refactor(setup): remove interactive OAuth step; bundle [oauth] section is now opt-in" +``` + +--- + +## Task 16: Wire `-ldflags` injection in Makefile + +**Files:** +- Modify: `Makefile` + +- [ ] **Step 1: Extend `LDFLAGS`** + +In `Makefile`, find the `LDFLAGS` definition (lines 9-11): + +```make +LDFLAGS := -X github.com/wesm/msgvault/cmd/msgvault/cmd.Version=$(VERSION) \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.Commit=$(COMMIT) \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.BuildDate=$(BUILD_DATE) +``` + +Replace with: + +```make +LDFLAGS := -X github.com/wesm/msgvault/cmd/msgvault/cmd.Version=$(VERSION) \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.Commit=$(COMMIT) \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.BuildDate=$(BUILD_DATE) \ + -X github.com/wesm/msgvault/internal/oauth.oauthClientID=$(MSGVAULT_OAUTH_CLIENT_ID) \ + -X github.com/wesm/msgvault/internal/oauth.oauthClientSecret=$(MSGVAULT_OAUTH_CLIENT_SECRET) +``` + +When `MSGVAULT_OAUTH_CLIENT_ID` and `MSGVAULT_OAUTH_CLIENT_SECRET` are unset (contributor builds via `make build`), Make expands them to empty strings and the `-X` flags become effectively no-ops; the package vars keep their compiled-in dev defaults. + +- [ ] **Step 2: Verify contributor build still works** + +Run: `make build` + +Expected: success. The resulting binary uses the source-default (dev) OAuth credentials. + +- [ ] **Step 3: Verify release-style build works** + +Run: `MSGVAULT_OAUTH_CLIENT_ID=test-prod-id MSGVAULT_OAUTH_CLIENT_SECRET=test-prod-secret make build` + +Expected: success. + +- [ ] **Step 4: Commit** + +```bash +git add Makefile +git commit -m "build: inject embedded OAuth credentials via ldflags" +``` + +--- + +## Task 17: Wire production credential injection in release workflow + +**Files:** +- Modify: `.github/workflows/release.yml` + +- [ ] **Step 1: Find the build step(s)** + +Open `.github/workflows/release.yml`. The Linux build step runs `go build` directly (not `make build-release`). Locate the step (it's the one with the `go build` command) and also any other platform build steps. + +Each `go build` invocation that produces a release binary needs `MSGVAULT_OAUTH_CLIENT_ID` and `MSGVAULT_OAUTH_CLIENT_SECRET` set in its environment. The simplest pattern is to set them as job-level env vars so every step inherits them. + +- [ ] **Step 2: Add the env injection at the job level** + +For each release-producing job (Linux amd64, Linux arm64, macOS, Windows), add an `env:` block at the job level (or at each build step) that pulls from GitHub Secrets: + +```yaml +env: + MSGVAULT_OAUTH_CLIENT_ID: ${{ secrets.MSGVAULT_OAUTH_CLIENT_ID }} + MSGVAULT_OAUTH_CLIENT_SECRET: ${{ secrets.MSGVAULT_OAUTH_CLIENT_SECRET }} +``` + +If the build step currently runs `go build` with an explicit `-ldflags` argument (not `make build-release`), update the `-ldflags` string to include the two new `-X` entries: + +```yaml +go build \ + -ldflags "-s -w \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.Version=$VERSION \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.Commit=$COMMIT \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.BuildDate=$BUILD_DATE \ + -X github.com/wesm/msgvault/internal/oauth.oauthClientID=$MSGVAULT_OAUTH_CLIENT_ID \ + -X github.com/wesm/msgvault/internal/oauth.oauthClientSecret=$MSGVAULT_OAUTH_CLIENT_SECRET" \ + ... +``` + +Preserve the existing build flags. The simplest pattern is to switch the workflow to call `make build-release` (which already picks up the env vars after Task 16). Use whichever is closer to the existing workflow style. + +- [ ] **Step 3: Confirm CI doesn't try to use the secrets** + +Open `.github/workflows/ci.yml`. CI should NOT set `MSGVAULT_OAUTH_CLIENT_ID` or `MSGVAULT_OAUTH_CLIENT_SECRET`. CI builds use the source defaults (dev client), which is fine for unit tests since they stub the OAuth manager. + +- [ ] **Step 4: Local sanity check (workflow not actually run here)** + +You cannot run the release workflow without a tag push. As a smoke test, run the equivalent command locally: + +Run: `MSGVAULT_OAUTH_CLIENT_ID=fake-prod-id MSGVAULT_OAUTH_CLIENT_SECRET=fake-prod-secret make build-release` + +Expected: success. + +- [ ] **Step 5: Commit** + +```bash +git add .github/workflows/release.yml +git commit -m "ci: inject embedded OAuth credentials into release builds" +``` + +--- + +## Task 18: Update README and quickstart docs + +**Files:** +- Modify: `README.md` +- Modify: `cmd/msgvault/cmd/quickstart.md` + +- [ ] **Step 1: Update the Quick Start section in `README.md`** + +In `README.md`, find the section that today instructs users to "Follow the OAuth Setup Guide" before running `add-account`. Drop that prerequisite and replace with the embedded-default flow. + +Replace the existing Quick Start lines (the ones beginning with "Follow the OAuth Setup Guide…") with: + +```markdown +### Quick Start + +```sh +# Initialize the database +msgvault init-db + +# Add a Gmail account — opens your browser for consent +msgvault add-account you@gmail.com + +# Sync mail +msgvault sync-full you@gmail.com + +# Browse the archive +msgvault tui +``` + +No Google Cloud Console setup required: msgvault ships with a verified OAuth client. +``` + +Add a new subsection later in the README (after the basic usage block) titled "Advanced: bring your own OAuth client": + +```markdown +### Advanced: bring your own OAuth client + +The default flow uses msgvault's centralized verified OAuth client. You only need your own Cloud project if: + +- Your Workspace organization prohibits authorizing third-party OAuth apps +- You prefer your own Cloud project's third-party-access listing to show +- You need your own Gmail API quota for very large mailboxes +- You want a fallback before msgvault's centralized client finishes Google verification + +Follow the [OAuth setup guide](https://msgvault.io/guides/oauth-setup/) to create a Desktop OAuth client, then add it to `~/.msgvault/config.toml`: + +```toml +[oauth] +client_secrets = "/path/to/client_secret.json" +``` + +Use `--oauth-app NAME` for per-account named-app routing — see the OAuth setup guide for details. +``` + +- [ ] **Step 2: Update `quickstart.md`** + +In `cmd/msgvault/cmd/quickstart.md`, apply the same shape: drop the "Follow OAuth Setup Guide" prerequisite, mention the centralized client as default, and note BYO as advanced. + +- [ ] **Step 3: Confirm no broken doc links** + +Run: `grep -n 'oauth-setup\|client_secret.json' README.md cmd/msgvault/cmd/quickstart.md` + +Expected: only matches inside the "Advanced" subsection, not the Quick Start. + +- [ ] **Step 4: Commit** + +```bash +git add README.md cmd/msgvault/cmd/quickstart.md +git commit -m "docs: drop OAuth setup prerequisite from Quick Start" +``` + +--- + +## Task 19: Final integration check + +**Files:** none (verification only) + +- [ ] **Step 1: Run the full test suite** + +Run: `make test` + +Expected: PASS. + +- [ ] **Step 2: Run linters** + +Run: `make lint-ci` + +Expected: clean. + +- [ ] **Step 3: Run go vet** + +Run: `go vet -tags "fts5 sqlite_vec" ./...` + +Expected: clean. + +- [ ] **Step 4: Confirm the dead symbols are truly gone** + +Run: `grep -rn 'errOAuthNotConfigured\|tryFindClientSecrets\|oauthSetupHint\|wrapOAuthError\|HasAnyConfig\|setupOAuthSecrets' --include='*.go'` + +Expected: no matches in any production or test file. + +- [ ] **Step 5: Confirm the new symbols are in place** + +Run: `grep -rn 'ScopesEmbedded\|EmbeddedConfig\|NewEmbeddedManager\|HasEmbeddedCredentials\|resolveOAuthManager\|errAccessDenied\|embeddedFallbackMessage' --include='*.go'` + +Expected: matches in `internal/oauth/embedded.go`, `internal/oauth/oauth.go`, `cmd/msgvault/cmd/oauth_resolve.go`, and their respective test files. + +- [ ] **Step 6: Manual walkthrough (smoke test)** + +This step does not pass/fail automatically; it's a sanity check against a real Gmail account. + +1. Build: `make build`. +2. Run `./msgvault init-db` in an isolated `MSGVAULT_HOME=/tmp/mvtest`. +3. Run `./msgvault --home /tmp/mvtest add-account you@gmail.com`. +4. The browser should open to Google's OAuth consent screen. If your Gmail account is on the dev project's test-user list, consent should succeed. If not, you should see the verification-window fallback message in the terminal. +5. Confirm a token file appears under `/tmp/mvtest/tokens/`. +6. Run `./msgvault --home /tmp/mvtest sync-full you@gmail.com --limit 5` and confirm five messages sync. + +Document any deviations in the PR description. + +- [ ] **Step 7: Mark plan complete in the worktree** + +No commit needed for this step. The plan is fully executed once Steps 1-6 pass. + +--- + +## Self-review checklist + +After implementing every task, run through this checklist before declaring done: + +1. **Embedded client works in source builds?** `make build` then `./msgvault add-account` should reach the consent screen (assuming a non-empty dev `oauthClientID`). +2. **BYO still works?** Set `[oauth] client_secrets = "..."` in `config.toml` and confirm `add-account` uses it. +3. **Named BYO still works?** Add `[oauth.apps.acme]` and confirm `add-account --oauth-app acme` uses it. +4. **Service account still works?** Configure `[oauth] service_account_key`, confirm `add-account` short-circuits to the SA path. +5. **`--oauth-app nonexistent` errors?** Confirm the error mentions the app name. +6. **Token-refresh access_denied prints the fallback?** Manual test only — temporarily revoke consent in your Google account dashboard and re-run a sync; the fallback message should appear. +7. **`make test` clean?** No failing tests, no skipped tests that previously passed. +8. **`make lint-ci` clean?** No new lints. +9. **Spec coverage?** Every code change in the spec maps to a task above. The out-of-band verification prep (privacy policy, demo video, CASA assessment) is handled separately from this plan. +10. **No references to deleted symbols?** Grep across the repo. diff --git a/docs/superpowers/specs/2026-05-20-centralized-oauth-design.md b/docs/superpowers/specs/2026-05-20-centralized-oauth-design.md new file mode 100644 index 000000000..37ed50b6f --- /dev/null +++ b/docs/superpowers/specs/2026-05-20-centralized-oauth-design.md @@ -0,0 +1,518 @@ +# Centralized verified Google OAuth client for msgvault + +Status: Design, ready for review +Date: 2026-05-20 + +## Summary + +Replace the bring-your-own (BYO) Google Cloud OAuth setup that every new user +must complete today with a centralized, Google-verified OAuth client baked into +the msgvault binary. New users go from "create a Cloud project, configure the +consent screen, register yourself as a test user, download client_secret.json, +edit config.toml" to "run `msgvault add-account you@gmail.com`". BYO, +named-apps, and the service-account path all remain as escape valves for +Workspace orgs, privacy-conscious users, and high-volume mailboxes. + +## Decisions resolved during brainstorming + +| Decision | Choice | +|---|---| +| Strategy | Eliminate the user-owned OAuth app step entirely via a project-owned verified client | +| Verified scopes | `gmail.readonly` + `gmail.modify` + `mail.google.com/` | +| BYO path | Stay as a peer option (hybrid-full): BYO + named apps + per-account binding all keep working | +| Embed mechanism | gh-style hybrid: package-level defaults in source, overridable via `-ldflags -X` at build time | +| Microsoft 365 | Out of scope for this design; tackled in a separate effort later | +| Rollout | Land in `main` and ship continuously while Google verification runs in parallel. BYO is the documented fallback for anyone who hits the 100-user lifetime cap before verification completes. | + +## Goals + +- A clean-install user can run `msgvault add-account you@gmail.com` with no + prior Google Cloud Console work and reach the browser consent screen. +- BYO config (`[oauth] client_secrets`, `[oauth.apps.*]`, + `[oauth] service_account_key`) continues to work unchanged. +- The embedded credentials live as package variables, default to a known value + in source, and can be overridden at build time via Go `-ldflags`. +- The verified client requests scopes equivalent to today's combined + `oauth.Scopes` + `oauth.ScopesDeletion` so the centralized path supports + every existing feature including permanent delete. +- Dead code that only existed to handle the "no OAuth credentials configured" + first-run cliff is removed. +- Documentation reframes setup: centralized is the default path, BYO is an + "advanced" footnote. + +## Non-goals + +- Microsoft 365 / Graph centralization. The `[microsoft]` config block, + `add-o365` command, and the existing BYO Azure flow are untouched. +- IMAP changes. `add-imap` and its app-password flow are unrelated. +- The service-account path. It already serves Workspace admins via + domain-wide delegation and stays as-is. +- Removing any BYO surface (named apps, `--oauth-app` flag, per-account + binding column, `TokenMatchesClient`, scope-escalation prompt for BYO + users). All of it stays for the mixed personal+Workspace user case. + +## Background + +### Current setup pain + +A new user today must, before they can sync a single message: + +1. Create a Google Cloud project in the console. +2. Enable the Gmail API. +3. Configure the OAuth consent screen, including `gmail.modify` and listing + themselves as a test user. +4. Create an OAuth client ID (Desktop type). +5. Download `client_secret.json`. +6. Create or edit `~/.msgvault/config.toml` to point `[oauth] client_secrets` + at the file. +7. Live with Google's 7-day refresh-token expiry that applies to unverified + restricted-scope clients. + +Steps 1 through 6 take 5 to 15 minutes for someone who has never used Cloud +Console and is hostile to anyone who has not. + +### Existing infrastructure that already supports the new design + +- `internal/oauth/oauth.go` already separates `NewManager` (default scopes, + reads a `client_secret.json` path) from `NewManagerWithScopes` (custom + scopes). The flow factor we are adding is "no client_secret.json path, + build the `oauth2.Config` from embedded values". +- The Makefile already wires version metadata into the binary via `-ldflags` + (`Makefile:9-11`). We extend the same pattern with two more `-X` entries. +- The release workflow at `.github/workflows/release.yml` is where production + credentials get injected from GitHub Actions Secrets. + +### Why hybrid-full and not "drop BYO entirely" + +The brainstorming session initially settled on dropping BYO entirely. That +broke down for three reasons that surfaced once we looked at the code: + +1. **Workspace orgs that mandate internal OAuth apps.** Some IT policies + prohibit employees from authorizing third-party OAuth apps. BYO is one + path for those users; the service account is another. +2. **The 100-user lifetime cap during verification.** Google caps unverified + restricted-scope clients at 100 distinct grants over the project lifetime + regardless of publishing status. msgvault's install path (Homebrew, + conda-forge, `install.sh`) can fill that in a single Hacker News thread. + BYO is the only escape valve during the verification window that does not + require running a long-lived branch. +3. **The named-apps complexity is independent of BYO-as-default.** Group B + features (named apps, `--oauth-app`, per-account binding column) exist to + support users with multiple Gmail accounts pointing at different OAuth + clients (personal + work). That use case persists in the hybrid world, so + the code stays. + +The work that goes away is the Group A "you forgot to configure OAuth" +plumbing, not the Group B per-account-binding system. + +## Design + +### Credential resolution + +OAuth credential resolution becomes a three-way decision inside +`resolveOAuthManager`. Service-account resolution is unchanged and stays at +the call sites that need it: they short-circuit on +`cfg.OAuth.ServiceAccountKeyFor(appName) != ""` before invoking +`resolveOAuthManager`. So the resolver only sees OAuth cases. + +``` +Given an effective app name (from --oauth-app on add-account, or from +sources.oauth_app for commands operating on an existing account), and +assuming the caller has already short-circuited any service-account path: + +1. Effective app name is non-empty: + - cfg.OAuth.Apps[name].ClientSecrets is set + -> Use BYO OAuth via the named app's client_secrets. + - Otherwise + -> Return error "OAuth app NAME not configured". This preserves + today's behavior when --oauth-app or sources.oauth_app references + a name that has no OAuth client_secrets entry. Silent fallthrough + to embedded for an explicitly-named app would be a footgun. + +2. Effective app name is empty and cfg.OAuth.ClientSecrets is set: + -> Use BYO OAuth via the global default client_secrets. + +3. Otherwise: + -> Use the embedded client. +``` + +The branch is captured in a single helper so the if-chain does not get +duplicated at every call site. Existing accounts whose `sources.oauth_app` +column points at a named BYO binding continue to use that binding. Existing +accounts authorized under the global BYO default continue to use it. The +embedded path is reached only when nothing is configured. + +### Embedded credentials module + +New file: `internal/oauth/embedded.go`. + +```go +// Package vars are intentional (not consts) so -ldflags -X can override. +// Per https://developers.google.com/identity/protocols/oauth2 the desktop +// client secret is "obviously not treated as a secret"; PKCE provides the +// flow security. +var ( + oauthClientID = "TBD-msgvault-dev-client-id" + oauthClientSecret = "TBD-msgvault-dev-client-secret" +) + +// EmbeddedConfig returns the oauth2.Config built from the embedded +// credentials, using the loopback flow that the existing browserFlow code +// already implements. +func EmbeddedConfig(scopes []string) *oauth2.Config { ... } + +// NewEmbeddedManager mirrors NewManagerWithScopes but uses embedded +// credentials instead of reading client_secret.json from disk. +func NewEmbeddedManager(tokensDir string, logger *slog.Logger, scopes []string) (*Manager, error) { ... } + +// HasEmbeddedCredentials reports whether the package vars are non-empty. +// Used by selection logic to suppress the fall-through-to-embedded branch +// when a fork has stripped the values out. +func HasEmbeddedCredentials() bool { ... } +``` + +Source defaults: + +- A separate "msgvault-dev" Google Cloud project owns the source-default + client. Its consent screen lists current contributors as test users; its + Gmail API quota is small. The defaults are *not* the production values, + so forks and source builds do not burn the production project's 100-user + cap during the verification window. +- Production values are injected at release time via `-ldflags` from GitHub + Actions Secrets and never appear in the repo. + +`HasEmbeddedCredentials` is true in any normal build (release or source); +both have non-empty package vars. A fork that strips the values out, or a +contributor who clears them locally to test the fallback path, would see it +return false; in that case `resolveOAuthManager` returns a typed +"no embedded OAuth credentials in this build" error. Callers print a +"build with embedded credentials, or configure BYO in config.toml" +message. This is a separate failure mode from the verification-window +"access_denied" case described under Verification window UX below; they +do not need to share an error type. + +### Scope set for the embedded client + +New variable in `internal/oauth/oauth.go`: + +```go +// ScopesEmbedded is the scope set requested by the centralized verified +// msgvault OAuth client. It is the union of Scopes and ScopesDeletion so +// users on the embedded path never need scope escalation for permanent +// delete. +var ScopesEmbedded = []string{ + "https://www.googleapis.com/auth/gmail.readonly", + "https://www.googleapis.com/auth/gmail.modify", + "https://mail.google.com/", +} +``` + +`Scopes` (`internal/oauth/oauth.go:28`) and `ScopesDeletion` +(`internal/oauth/oauth.go:35`) keep their current values. The escalation flow +in `cmd/msgvault/cmd/deletions.go` keeps working unchanged for BYO users +whose own OAuth clients are registered for `gmail.readonly + gmail.modify` +only. On the embedded path, `HasScope("https://mail.google.com/")` returns +true at first auth, so the escalation prompt is never reached. + +### Resolver helper + +New helper in a small new file `cmd/msgvault/cmd/oauth_resolve.go`: + +```go +// resolveOAuthManager builds the *oauth.Manager appropriate for the +// account+config+scopes triple. Resolution order matches the section +// above: named BYO OAuth, global BYO OAuth, embedded. Returns an error +// when an explicitly-named app has no client_secrets, or (rare) when +// embedded credentials are absent (forks, stripped builds). +// Callers handle service-account resolution themselves before calling +// this helper, because *oauth.Manager and the service-account manager +// have different interfaces. +func resolveOAuthManager( + cfg *config.Config, + appName string, + scopes []string, + logger *slog.Logger, +) (*oauth.Manager, error) { ... } +``` + +The canonical refactor target is `oauthManagerCache()` at +`cmd/msgvault/cmd/root.go:419-442`. That function builds and caches +`*oauth.Manager` instances per app name, and is consumed by the sync +codepath via `getOAuthMgr := oauthManagerCache()` (syncfull.go:79). Its +internals collapse to a single call to `resolveOAuthManager`; the outer +caching shell stays. + +Other call sites that today inline the +`ClientSecretsFor(appName)` + `oauth.NewManagerWithScopes(...)` pair move +to `resolveOAuthManager` directly: + +- `cmd/msgvault/cmd/addaccount.go:165-176` (the BYO branch that today + emits `errOAuthNotConfigured` / `wrapOAuthError`) +- `cmd/msgvault/cmd/deletions.go:435-479` and `:695-708` (manager + resolution before scope checks, plus the escalation re-resolve) +- `cmd/msgvault/cmd/verify.go:126-132` (the OAuth-fallback arm of + verify's resolution) + +Service-account call sites that today branch on +`cfg.OAuth.ServiceAccountKeyFor(appName)` first +(`cmd/msgvault/cmd/sync.go:228`, `cmd/msgvault/cmd/serve.go:351`, +`cmd/msgvault/cmd/verify.go:117`, `cmd/msgvault/cmd/addaccount.go:91`, +`cmd/msgvault/cmd/deletions.go:432`) keep that early branch; only their +"no service account configured, fall through to OAuth manager" arm uses +the new helper. + +The `buildAPIClient` site at `cmd/msgvault/cmd/syncfull.go:228-241` +already routes through `getOAuthMgr` and so picks up the new behavior +automatically once `oauthManagerCache()` is refactored. + +Two multi-source loops (`cmd/msgvault/cmd/syncfull.go:118` and +`cmd/msgvault/cmd/sync.go:128`) currently skip per-source with +`if !cfg.OAuth.HasAnyConfig() { fmt.Printf("Skipping %s ..."); continue }`. +With the embedded fallthrough, that skip becomes dead — every Gmail +source can reach the resolver. Both blocks are removed; the loop simply +calls `getOAuthMgr(appName)` for each Gmail source. + +Each of those sites currently picks scopes (`oauth.Scopes` vs +`oauth.ScopesDeletion`). The helper accepts the requested scopes from the +caller; on the embedded path it ignores the request and always uses +`ScopesEmbedded` because the embedded grant is broader than any per-call +need. + +### Code removed + +These exist only to handle the "no OAuth configured" first-run cliff and +become dead once the embedded path is the no-config default: + +| Symbol | File | Reason | +|---|---|---| +| `errOAuthNotConfigured` | `cmd/msgvault/cmd/root.go` | Embedded path is always available in release builds | +| `tryFindClientSecrets` | `cmd/msgvault/cmd/root.go` | Same | +| `oauthSetupHint` | `cmd/msgvault/cmd/root.go` | Same | +| `wrapOAuthError` | `cmd/msgvault/cmd/root.go` | Same | +| `OAuthConfig.HasAnyConfig` | `internal/config/config.go` | All six call sites are "if !HasAnyConfig { error or skip }" gates; embedded fallthrough makes each one unreachable. | +| Interactive OAuth prompt in `setup` (`setupOAuthSecrets`) | `cmd/msgvault/cmd/setup.go` | The step asks for `client_secret.json`; no longer required because the embedded client is the default. The function itself is removed. | +| Mandatory `client_secret.json` copy into `setup`'s deployment bundle | `cmd/msgvault/cmd/setup.go` | Setup's bundle mode still supports copying a BYO `client_secret.json` into the bundle when the operator opts in; it just no longer requires one. The required-copy code path goes away. | +| Tests covering the above | `cmd/msgvault/cmd/root_test.go`, `cmd/msgvault/cmd/setup_test.go`, `internal/config/config_test.go` (`TestOAuthConfig_HasAnyConfig` plus the inline `HasAnyConfig` assertions in `TestLoadWithNamedOAuthApps` and `TestLoadWithNamedOAuthApps_RelativePaths`) | Follow the symbols | + +The `setup` command's other responsibilities (data dir creation, optional +remote configuration) stay. Whether to retire `setup` entirely in favor of a +slimmer `init` is a separable decision and not part of this design. + +### What stays untouched + +- `[oauth] client_secrets` field in config +- `[oauth.apps.NAME]` named-apps map and the entire `OAuthApp` struct +- `[oauth] service_account_key` field and `[oauth.apps.NAME] + service_account_key` +- `--oauth-app NAME` flag on every command that has it today +- `sources.oauth_app` column and binding-change detection +- `oauth.TokenMatchesClient`, `HasScope`, `HasScopeMetadata` +- Scope-escalation prompt (`promptScopeEscalation` in + `cmd/msgvault/cmd/deletions.go`) for BYO accounts whose own clients are + read+modify-only +- `OAuthConfig.ClientSecretsFor` (the resolver helper calls it on the BYO + branches) +- `OAuthConfig.ServiceAccountKeyFor` (call sites still use it for their + early service-account short-circuit) + +## Documentation changes + +- `README.md`: drop the "Follow the OAuth Setup Guide" prerequisite from the + Quick Start. Replace with a one-liner: "`msgvault add-account + you@gmail.com`, that's it". Add a short subsection later in the README + titled "Advanced: bring your own OAuth client" that links to the existing + setup guide for the Workspace-mandate, privacy-conscious, and quota + cases. +- `cmd/msgvault/cmd/quickstart.md`: same shape. Embedded is the default; + BYO is a footnote. +- `https://msgvault.io/guides/oauth-setup/`: not removed (BYO users still + need it), but the page intro changes to "Most users do not need this + page; the default `msgvault add-account` flow works without any Cloud + Console setup. This guide is for Workspace orgs that mandate internal + OAuth apps, users who prefer their own Cloud project, and similar + advanced cases." +- `https://msgvault.io/` landing page: drop the "~5 minutes" OAuth-setup + callout from the install path. + +The msgvault.io site lives in a separate repo; site changes are out of +scope for the implementation PR in this codebase, but the Quick Start +docs that live in this repo are in scope. + +## Build and release integration + +Extend `LDFLAGS` in the Makefile: + +```make +LDFLAGS := -X github.com/wesm/msgvault/cmd/msgvault/cmd.Version=$(VERSION) \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.Commit=$(COMMIT) \ + -X github.com/wesm/msgvault/cmd/msgvault/cmd.BuildDate=$(BUILD_DATE) \ + -X github.com/wesm/msgvault/internal/oauth.oauthClientID=$(MSGVAULT_OAUTH_CLIENT_ID) \ + -X github.com/wesm/msgvault/internal/oauth.oauthClientSecret=$(MSGVAULT_OAUTH_CLIENT_SECRET) +``` + +When the env vars are unset (e.g., `make build` by a contributor), the `-X` +flags become no-ops and the package vars keep their source defaults. + +In `.github/workflows/release.yml`, the release build step sets: + +```yaml +- name: Build release binary + env: + MSGVAULT_OAUTH_CLIENT_ID: ${{ secrets.MSGVAULT_OAUTH_CLIENT_ID }} + MSGVAULT_OAUTH_CLIENT_SECRET: ${{ secrets.MSGVAULT_OAUTH_CLIENT_SECRET }} + run: make build-release +``` + +The CI workflow at `.github/workflows/ci.yml` does *not* set these. CI builds +use the source defaults (the dev client), which is fine for unit tests since +they stub the OAuth manager. + +## Verification window UX + +While Google verification is in progress (estimated 2 to 3 months from +submission), the embedded client lives in Cloud Console "In Production +(unverified)" status. Two constraints apply: + +- **100-user lifetime cap.** Across the project lifetime, no more than 100 + distinct Google accounts can authorize the client. The counter does not + reset. +- **7-day refresh-token expiry.** Tokens issued by an unverified + restricted-scope client expire after 7 days. Users re-authorize weekly. + +When `add-account` on the embedded path receives an `access_denied` +response from Google's OAuth endpoints (the failure mode for both +"caller is not on the test-user list" and "100-user lifetime cap reached"), +it prints: + +``` +msgvault's centralized OAuth client is still in Google's verification +queue. Two options: + + 1. Use the bring-your-own setup (one-time, ~5 minutes): + https://msgvault.io/guides/oauth-setup/ + + 2. Request beta access (open a GitHub issue with your Gmail address): + https://github.com/wesm/msgvault/issues/new?template=beta-oauth.md +``` + +The implementer should not key off Google's specific error sub-codes (those +have changed over time and are not contractually stable). `access_denied` +plus the absence of a returned token from the loopback flow is signal +enough to print the fallback message. + +No silent failures, no infinite retry loops. Users on the embedded path who +re-authorize weekly during the window do not see this message; only users +hitting the cap or unlisted-test-user wall do. + +The same fallback message is printed by any command that triggers a +browser-OAuth flow on the embedded path and receives `access_denied`, not +just `add-account`. This includes sync commands that fail token refresh +after the 7-day window and re-enter the loopback flow, and `verify` when +it has to re-authorize. + +After verification lands, Google flips a flag on their end. The cap lifts; +refresh tokens become long-lived. No msgvault code change is needed at that +moment. + +## Testing strategy + +- `internal/oauth/embedded_test.go` (new): unit test the `EmbeddedConfig` + builder and `HasEmbeddedCredentials` detection. Verify the ldflags + override mechanism by setting the package vars in a test fixture. +- `internal/oauth/oauth_test.go`: existing tests stay; they exercise BYO + paths via `parseClientSecrets`. +- `cmd/msgvault/cmd/addaccount_test.go`: add cases covering the new + resolution paths visible from `add-account`: the service-account + short-circuit, the three branches inside `resolveOAuthManager` (named + BYO OAuth, global BYO OAuth, embedded), and the error case where + `--oauth-app NAME` references an app without `client_secrets`. +- `cmd/msgvault/cmd/oauth_resolve_test.go` (new): unit-test the resolver + helper in isolation with each combination of config inputs. +- End-to-end: existing fixtures cover BYO; add a stub embedded fixture that + fakes `oauth.NewEmbeddedManager` to skip the browser-flow step. +- Manual test plan documented in the implementation plan: walk through + clean-install on a fresh machine, verify both the embedded happy path and + the cap-exceeded message. + +## Migration + +No migration code needed. + +- Users with `[oauth] client_secrets` set: keep working via BYO. +- Users with `[oauth.apps.*]` named apps: keep working via named BYO. +- Users with `[oauth] service_account_key`: keep working via service account. +- Users with empty/missing OAuth config on a clean install: previously got + `errOAuthNotConfigured`; now get the embedded client. + +The only implicit transition is when an existing user deletes their +`[oauth] client_secrets` line. Their next `add-account` for that email runs +the embedded consent flow. The resulting token overwrites the existing +token file for that email (under `/.json`), +replacing the old BYO `client_id` metadata with the embedded one. From +that point forward, every command resolving credentials for that account +uses the embedded path. The user's old BYO Cloud project becomes +irrelevant but is not touched. + +## Out of scope + +- Microsoft 365 centralization. Microsoft has its own verification process + (publisher verification, permissions justification, separate cost + structure). Tackle in a follow-up effort. +- A "msgvault init wizard" that bundles `init-db` + first `add-account` + into a single command. Separable improvement. +- Encryption at rest (database, attachments). Tracked in the existing + "Not Yet Implemented" list in `CLAUDE.md`. +- Removing the `setup` command. The OAuth-credentials step is removed; the + rest of `setup` is a separate cleanup. + +## Out-of-band verification prep + +These do not block landing the code, but the verified client cannot be used +in production by anyone outside the 100-user test cohort until they are +complete. + +| Item | Owner | Notes | +|---|---|---| +| Privacy policy hosted on msgvault.io (first-party) | Project maintainer | Must be on a verified-owned domain. GitHub Pages does not count. | +| OAuth consent screen branding | Project maintainer | App name "msgvault", logo, support email, scope justifications | +| Brand verification (Google confirms msgvault.io ownership) | Project maintainer | 2 to 3 business days | +| Demo video (unlisted YouTube, ~5 minutes) | Project maintainer | Shows OAuth flow plus each scope's usage in production-level domain | +| OAuth consent screen submission for restricted scopes | Project maintainer | 2 to 8 weeks of review iteration | +| CASA assessor contract | Project maintainer | TAC Security is cheapest (~$500/yr) via Google's negotiated deal | +| OWASP ZAP pre-scan in CI | Project maintainer | Add as a GitHub Actions job before formal DAST | +| CASA Tier 2 SAQ draft | Project maintainer | 54 questions; mostly reusable across years | +| DAST scan against production app | Assessor | Likely targets msgvault.io plus OAuth client source audit | +| Letter of Validation submitted to Cloud Console | Project maintainer | Final step | +| Annual recertification | Project maintainer | Recert email arrives 12 months from LOV approval | + +## Project ownership and verification deadline + +- **Production OAuth client.** Owned by the project maintainer. Recert + emails go to the account on file. The specific account is tracked + outside this repo. +- **Verification deadline is the 100-user cap, not the calendar.** Google + caps unverified restricted-scope clients at 100 distinct grants over the + project lifetime, and the counter does not reset. Verification (consent + screen submission, brand verification, CASA Tier 2 assessment, LOV) must + complete before the production project accumulates 100 grants. If it + does not, new users hit the BYO fallback path until verification lands. + This is treated as a hard scheduling constraint, not a soft target. +- **Dev project cap.** The dev Cloud project also has the 100-user lifetime + cap. Not worth solving until it actually becomes a problem; contributors + who hit it can BYO. + +## References + +- Google docs on the desktop client secret not being treated as a secret: + https://developers.google.com/identity/protocols/oauth2 +- Google docs on restricted scope verification: + https://developers.google.com/identity/protocols/oauth2/production-readiness/restricted-scope-verification +- Google docs on app audience and user cap behavior: + https://support.google.com/cloud/answer/15549945 +- Google Gmail API scope classifications (confirms readonly, modify, and + mail.google.com/ are all in the Restricted bucket): + https://developers.google.com/workspace/gmail/api/auth/scopes +- gh CLI precedent for ldflags-overridable embedded OAuth client_id: + https://sourcegraph.com/r/github.com/cli/cli/-/blob/internal/authflow/flow.go +- CASA Tier 2 process overview: + https://appdefensealliance.dev/casa/tier-2/tier2-overview From e667a688c48550a79debe48a920fd00a38b7a8fc Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 21 May 2026 08:24:33 -0400 Subject: [PATCH 02/10] feat(oauth): add embedded credentials with access_denied fallback --- internal/oauth/embedded.go | 63 +++++++++++++++++++ internal/oauth/embedded_test.go | 104 ++++++++++++++++++++++++++++++++ internal/oauth/oauth.go | 57 +++++++++++++++++ internal/oauth/oauth_test.go | 88 +++++++++++++++++++++++++++ 4 files changed, 312 insertions(+) create mode 100644 internal/oauth/embedded.go create mode 100644 internal/oauth/embedded_test.go diff --git a/internal/oauth/embedded.go b/internal/oauth/embedded.go new file mode 100644 index 000000000..bad7e9833 --- /dev/null +++ b/internal/oauth/embedded.go @@ -0,0 +1,63 @@ +package oauth + +import ( + "fmt" + "log/slog" + + "golang.org/x/oauth2" + "golang.org/x/oauth2/google" +) + +// oauthClientID and oauthClientSecret hold the centralized verified +// msgvault OAuth client credentials. They are package vars (not consts) +// so release builds can override them via: +// +// go build -ldflags "-X github.com/wesm/msgvault/internal/oauth.oauthClientID=..." +// +// Per https://developers.google.com/identity/protocols/oauth2 the desktop +// client secret is "obviously not treated as a secret"; PKCE provides the +// flow security. The values below are the dev project's credentials, +// suitable for contributor builds. Production binaries override both. +var ( + oauthClientID = "TBD-msgvault-dev-client-id" + oauthClientSecret = "TBD-msgvault-dev-client-secret" +) + +// HasEmbeddedCredentials reports whether the package vars are non-empty. +// Forks that strip the values out (or contributors testing the fallback) +// will see this return false, in which case NewEmbeddedManager refuses +// to construct an embedded manager. +func HasEmbeddedCredentials() bool { + return oauthClientID != "" && oauthClientSecret != "" +} + +// EmbeddedConfig returns the oauth2.Config built from the embedded +// credentials. RedirectURL is set later inside Manager.browserFlow when +// the loopback port is known; the rest of the config is fixed here. +func EmbeddedConfig(scopes []string) *oauth2.Config { + return &oauth2.Config{ + ClientID: oauthClientID, + ClientSecret: oauthClientSecret, + Endpoint: google.Endpoint, + Scopes: scopes, + } +} + +// NewEmbeddedManager constructs a Manager backed by the centralized +// verified OAuth client. Returns an error when credentials are missing +// (forks, stripped builds, or contributors who blanked the vars +// locally). +func NewEmbeddedManager(tokensDir string, logger *slog.Logger, scopes []string) (*Manager, error) { + if !HasEmbeddedCredentials() { + return nil, fmt.Errorf("no embedded OAuth credentials in this build") + } + if logger == nil { + logger = slog.Default() + } + return &Manager{ + config: EmbeddedConfig(scopes), + tokensDir: tokensDir, + logger: logger, + isEmbedded: true, + }, nil +} diff --git a/internal/oauth/embedded_test.go b/internal/oauth/embedded_test.go new file mode 100644 index 000000000..93c12db1c --- /dev/null +++ b/internal/oauth/embedded_test.go @@ -0,0 +1,104 @@ +package oauth + +import ( + "log/slog" + "testing" + + "golang.org/x/oauth2/google" +) + +func TestHasEmbeddedCredentials(t *testing.T) { + // Save and restore package vars around the test + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + + tests := []struct { + name string + id string + secret string + want bool + }{ + {"both set", "id", "secret", true}, + {"id only", "id", "", false}, + {"secret only", "", "secret", false}, + {"neither", "", "", false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + oauthClientID = tc.id + oauthClientSecret = tc.secret + if got := HasEmbeddedCredentials(); got != tc.want { + t.Errorf("HasEmbeddedCredentials() = %v, want %v", got, tc.want) + } + }) + } +} + +func TestEmbeddedConfig(t *testing.T) { + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + oauthClientID = "test-client-id" + oauthClientSecret = "test-client-secret" + + scopes := []string{"scope-a", "scope-b"} + cfg := EmbeddedConfig(scopes) + + if cfg.ClientID != "test-client-id" { + t.Errorf("ClientID = %q, want %q", cfg.ClientID, "test-client-id") + } + if cfg.ClientSecret != "test-client-secret" { + t.Errorf("ClientSecret = %q, want %q", cfg.ClientSecret, "test-client-secret") + } + if len(cfg.Scopes) != 2 || cfg.Scopes[0] != "scope-a" || cfg.Scopes[1] != "scope-b" { + t.Errorf("Scopes = %v, want %v", cfg.Scopes, scopes) + } + if cfg.Endpoint != google.Endpoint { + t.Errorf("Endpoint = %v, want google.Endpoint", cfg.Endpoint) + } +} + +func TestNewEmbeddedManager(t *testing.T) { + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + oauthClientID = "test-client-id" + oauthClientSecret = "test-client-secret" + + tokensDir := t.TempDir() + mgr, err := NewEmbeddedManager(tokensDir, slog.Default(), ScopesEmbedded) + if err != nil { + t.Fatalf("NewEmbeddedManager: %v", err) + } + if mgr == nil { + t.Fatal("NewEmbeddedManager returned nil manager") + } + if mgr.tokensDir != tokensDir { + t.Errorf("tokensDir = %q, want %q", mgr.tokensDir, tokensDir) + } + if !mgr.isEmbedded { + t.Error("isEmbedded = false, want true") + } +} + +func TestNewEmbeddedManagerWithoutCredentials(t *testing.T) { + origID, origSecret := oauthClientID, oauthClientSecret + defer func() { + oauthClientID = origID + oauthClientSecret = origSecret + }() + oauthClientID = "" + oauthClientSecret = "" + + _, err := NewEmbeddedManager(t.TempDir(), slog.Default(), ScopesEmbedded) + if err == nil { + t.Fatal("NewEmbeddedManager: want error when credentials are empty, got nil") + } +} diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index 2cb2c23db..dea4aa02b 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -38,6 +38,16 @@ var ScopesDeletion = []string{ "https://mail.google.com/", } +// ScopesEmbedded is the scope set requested by the centralized verified +// msgvault OAuth client. It is the union of Scopes and ScopesDeletion so +// users on the embedded path never need scope escalation for permanent +// delete. +var ScopesEmbedded = []string{ + "https://www.googleapis.com/auth/gmail.readonly", + "https://www.googleapis.com/auth/gmail.modify", + "https://mail.google.com/", +} + const defaultProfileURL = "https://gmail.googleapis.com/gmail/v1/users/me/profile" // TokenMismatchError is returned when the authorized Google account @@ -55,6 +65,34 @@ func (e *TokenMismatchError) Error() string { ) } +// errAccessDenied is returned by the OAuth callback when Google +// signals that the authorization was rejected. On the embedded path +// this is the failure mode for "caller is not on the test-user list" +// and "100-user lifetime cap reached" during the verification window; +// on the BYO path it usually means the user clicked Deny. +var errAccessDenied = errors.New("oauth: access_denied") + +// embeddedFallbackMessage is printed to the user when the embedded +// OAuth client receives an access_denied response. This indicates the +// user is not on the test-user list or the 100-user lifetime cap was +// reached during the verification window. The message points users to +// the BYO setup or beta-access channels. +const embeddedFallbackMessage = ` +msgvault's centralized OAuth client is still in Google's verification +queue. Two options: + + 1. Use the bring-your-own setup (one-time, ~5 minutes): + https://msgvault.io/guides/oauth-setup/ + + 2. Request beta access (open a GitHub issue with your Gmail address): + https://github.com/wesm/msgvault/issues/new?template=beta-oauth.md + +` + +// stdout is the destination for user-facing messages printed during +// the OAuth flow. Replaceable in tests via the var to capture output. +var stdout io.Writer = os.Stdout + // Manager handles OAuth2 token acquisition and storage. type Manager struct { config *oauth2.Config @@ -66,6 +104,12 @@ type Manager struct { // a real HTTP server and browser. When nil, the real browserFlow // is used. browserFlowFn func(ctx context.Context, email string, launchBrowser bool) (*oauth2.Token, error) + + // isEmbedded is true when this manager uses the centralized verified + // OAuth client (via NewEmbeddedManager) rather than a BYO + // client_secrets file. Used to enable the verification-window + // fallback message on access_denied. + isEmbedded bool } // NewManager creates an OAuth manager from client secrets. @@ -188,6 +232,9 @@ func (m *Manager) authorize( } token, err := flow(ctx, email, launchBrowser) if err != nil { + if m.isEmbedded && errors.Is(err, errAccessDenied) { + _, _ = fmt.Fprint(stdout, embeddedFallbackMessage) + } return err } @@ -211,6 +258,16 @@ const ( // newCallbackHandler returns an HTTP handler that processes the OAuth callback. func (m *Manager) newCallbackHandler(expectedState string, codeChan chan<- string, errChan chan<- error) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + if errParam := r.URL.Query().Get("error"); errParam != "" { + if errParam == "access_denied" { + errChan <- errAccessDenied + _, _ = fmt.Fprintf(w, "Authorization denied. You can close this window.") + return + } + errChan <- fmt.Errorf("oauth callback error: %s", errParam) + _, _ = fmt.Fprintf(w, "Error: %s", errParam) + return + } if r.URL.Query().Get("state") != expectedState { errChan <- errors.New("state mismatch: possible CSRF attack") _, _ = fmt.Fprintf(w, "Error: state mismatch") diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index 0c1d83241..304b0231e 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -1,6 +1,7 @@ package oauth import ( + "bytes" "context" "crypto/sha256" "encoding/json" @@ -531,6 +532,26 @@ func TestNewCallbackHandler(t *testing.T) { } } +func TestCallbackHandlerAccessDenied(t *testing.T) { + mgr := &Manager{logger: slog.Default()} + codeChan := make(chan string, 1) + errChan := make(chan error, 1) + handler := mgr.newCallbackHandler("expected-state", codeChan, errChan) + + req := httptest.NewRequest("GET", "/callback?error=access_denied&state=expected-state", nil) + rec := httptest.NewRecorder() + handler(rec, req) + + select { + case err := <-errChan: + if !errors.Is(err, errAccessDenied) { + t.Errorf("callback error = %v, want errAccessDenied", err) + } + default: + t.Fatal("callback handler did not send an error") + } +} + // TestAuthorize_SavesUnderOriginalIdentifier exercises the real // authorize() method end-to-end (with injected browserFlow and // profile server) to verify the token is saved under the original @@ -786,3 +807,70 @@ func TestValidateBrowserURL(t *testing.T) { }) } } + +func TestScopesEmbedded(t *testing.T) { + want := []string{ + "https://www.googleapis.com/auth/gmail.readonly", + "https://www.googleapis.com/auth/gmail.modify", + "https://mail.google.com/", + } + if len(ScopesEmbedded) != len(want) { + t.Fatalf("ScopesEmbedded has %d entries, want %d", len(ScopesEmbedded), len(want)) + } + for i, scope := range want { + if ScopesEmbedded[i] != scope { + t.Errorf("ScopesEmbedded[%d] = %q, want %q", i, ScopesEmbedded[i], scope) + } + } +} + +func TestAuthorizeEmbeddedFallbackMessage(t *testing.T) { + tokensDir := t.TempDir() + mgr := &Manager{ + config: &oauth2.Config{ClientID: "x", ClientSecret: "y", Scopes: []string{"s"}}, + tokensDir: tokensDir, + logger: slog.Default(), + isEmbedded: true, + browserFlowFn: func(ctx context.Context, email string, launchBrowser bool) (*oauth2.Token, error) { + return nil, errAccessDenied + }, + } + + var buf bytes.Buffer + origStdout := stdout + stdout = &buf + defer func() { stdout = origStdout }() + + err := mgr.Authorize(context.Background(), "u@example.com") + if !errors.Is(err, errAccessDenied) { + t.Fatalf("Authorize error = %v, want errAccessDenied", err) + } + if !strings.Contains(buf.String(), "still in Google's verification") { + t.Errorf("expected fallback message, got: %q", buf.String()) + } +} + +func TestAuthorizeNonEmbeddedNoFallback(t *testing.T) { + mgr := &Manager{ + config: &oauth2.Config{ClientID: "x", ClientSecret: "y", Scopes: []string{"s"}}, + tokensDir: t.TempDir(), + logger: slog.Default(), + // isEmbedded: false (default) + browserFlowFn: func(ctx context.Context, email string, launchBrowser bool) (*oauth2.Token, error) { + return nil, errAccessDenied + }, + } + + var buf bytes.Buffer + origStdout := stdout + stdout = &buf + defer func() { stdout = origStdout }() + + err := mgr.Authorize(context.Background(), "u@example.com") + if !errors.Is(err, errAccessDenied) { + t.Fatalf("Authorize error = %v, want errAccessDenied", err) + } + if strings.Contains(buf.String(), "still in Google's verification") { + t.Errorf("did not expect fallback message for non-embedded, got: %q", buf.String()) + } +} From cf59223ac0b7d6c36f15925c7b8ac459e0975c50 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 21 May 2026 08:24:47 -0400 Subject: [PATCH 03/10] refactor(oauth): centralize manager resolution and drop HasAnyConfig gating --- cmd/msgvault/cmd/addaccount.go | 15 +---- cmd/msgvault/cmd/addaccount_test.go | 61 ++++++++++++++++++ cmd/msgvault/cmd/deletions.go | 34 +++------- cmd/msgvault/cmd/oauth_resolve.go | 49 ++++++++++++++ cmd/msgvault/cmd/oauth_resolve_test.go | 84 ++++++++++++++++++++++++ cmd/msgvault/cmd/root.go | 88 ++------------------------ cmd/msgvault/cmd/root_test.go | 73 --------------------- cmd/msgvault/cmd/serve.go | 5 -- cmd/msgvault/cmd/sync.go | 4 -- cmd/msgvault/cmd/sync_test.go | 2 +- cmd/msgvault/cmd/syncfull.go | 15 +---- cmd/msgvault/cmd/verify.go | 12 +--- internal/config/config.go | 14 ---- internal/config/config_test.go | 73 --------------------- 14 files changed, 213 insertions(+), 316 deletions(-) create mode 100644 cmd/msgvault/cmd/oauth_resolve.go create mode 100644 cmd/msgvault/cmd/oauth_resolve_test.go diff --git a/cmd/msgvault/cmd/addaccount.go b/cmd/msgvault/cmd/addaccount.go index bb36a4cc5..8099e36c3 100644 --- a/cmd/msgvault/cmd/addaccount.go +++ b/cmd/msgvault/cmd/addaccount.go @@ -58,7 +58,6 @@ Examples: // Resolve which client secrets to use resolvedApp := oauthAppName oauthAppExplicit := cmd.Flags().Changed("oauth-app") - var clientSecretsPath string // Initialize database (in case it's new) dbPath := cfg.DatabaseDSN() @@ -174,21 +173,13 @@ Examples: return nil } - // Resolve client secrets path (standard OAuth flow) - clientSecretsPath, err = cfg.OAuth.ClientSecretsFor(resolvedApp) + // Build the OAuth manager. resolveOAuthManager handles named BYO, + // global BYO, and the embedded fallback automatically. + oauthMgr, err := resolveOAuthManager(cfg, resolvedApp, oauth.Scopes, logger) if err != nil { - if !cfg.OAuth.HasAnyConfig() { - return errOAuthNotConfigured() - } return err } - // Create OAuth manager - oauthMgr, err := oauth.NewManager(clientSecretsPath, cfg.TokensDir(), logger) - if err != nil { - return wrapOAuthError(fmt.Errorf("create oauth manager: %w", err)) - } - // If --force, delete existing token so we re-authorize if forceReauth { if oauthMgr.HasToken(email) { diff --git a/cmd/msgvault/cmd/addaccount_test.go b/cmd/msgvault/cmd/addaccount_test.go index 327e706d4..6747a7c40 100644 --- a/cmd/msgvault/cmd/addaccount_test.go +++ b/cmd/msgvault/cmd/addaccount_test.go @@ -14,6 +14,7 @@ import ( assertpkg "github.com/stretchr/testify/assert" requirepkg "github.com/stretchr/testify/require" "go.kenn.io/msgvault/internal/config" + "go.kenn.io/msgvault/internal/oauth" "go.kenn.io/msgvault/internal/store" ) @@ -1027,3 +1028,63 @@ func TestAddAccount_ForceServiceAccountReturnsActionableError(t *testing.T) { requirepkg.Error(t, err, "expected --force service account error") requirepkg.ErrorContains(t, err, "service accounts do not use --force") } + +func TestAddAccount_ResolverBranches(t *testing.T) { + tests := []struct { + name string + appName string + setup func(cfg *config.Config, t *testing.T) + wantErr bool + errContains string + }{ + { + name: "named BYO with client_secrets", + appName: "acme", + setup: func(cfg *config.Config, t *testing.T) { + path := writeStubClientSecrets(t, cfg.Data.DataDir, "acme.json") + cfg.OAuth.Apps = map[string]config.OAuthApp{"acme": {ClientSecrets: path}} + }, + wantErr: false, + }, + { + name: "named app without client_secrets", + appName: "missing", + setup: func(cfg *config.Config, t *testing.T) {}, + wantErr: true, + errContains: "missing", + }, + { + name: "global BYO", + appName: "", + setup: func(cfg *config.Config, t *testing.T) { + cfg.OAuth.ClientSecrets = writeStubClientSecrets(t, cfg.Data.DataDir, "default.json") + }, + wantErr: false, + }, + { + name: "no config falls through to embedded", + appName: "", + setup: func(cfg *config.Config, t *testing.T) {}, + wantErr: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := newTestConfig(t) + tc.setup(cfg, t) + _, err := resolveOAuthManager(cfg, tc.appName, oauth.Scopes, slog.Default()) + if tc.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + if tc.errContains != "" && !strings.Contains(err.Error(), tc.errContains) { + t.Errorf("error %q should contain %q", err.Error(), tc.errContains) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/cmd/msgvault/cmd/deletions.go b/cmd/msgvault/cmd/deletions.go index 3f7098813..aff7a738e 100644 --- a/cmd/msgvault/cmd/deletions.go +++ b/cmd/msgvault/cmd/deletions.go @@ -491,29 +491,19 @@ Examples: // buildAPIClient uses standard scopes; deletion may need elevated ones. // Service-account flows get scopes via the JWT assertion (no stored // token), so the scope-escalation prompt only applies to browser OAuth. - var clientSecretsPath string if src.SourceType == sourceTypeGmail { - if !cfg.OAuth.HasAnyConfig() { - return errOAuthNotConfigured() - } appName := sourceOAuthApp(src) isServiceAccount := cfg.OAuth.ServiceAccountKeyFor(appName) != "" if !isServiceAccount { - clientSecretsPath, err = cfg.OAuth.ClientSecretsFor(appName) - if err != nil { - return err - } - needsBatchDelete := deletePermanent if needsBatchDelete { - requiredScopes := oauth.ScopesDeletion - oauthMgr, err := oauth.NewManagerWithScopes(clientSecretsPath, cfg.TokensDir(), logger, requiredScopes) + oauthMgr, err := resolveOAuthManager(cfg, appName, oauth.ScopesDeletion, logger) if err != nil { - return wrapOAuthError(fmt.Errorf("create oauth manager: %w", err)) + return err } if !oauthMgr.HasScope(account, "https://mail.google.com/") && oauthMgr.HasScopeMetadata(account) { - if err := promptScopeEscalation(ctx, oauthMgr, account, needsBatchDelete, clientSecretsPath); err != nil { + if err := promptScopeEscalation(ctx, oauthMgr, account, needsBatchDelete, appName); err != nil { if errors.Is(err, errUserCanceled) { return nil } @@ -526,19 +516,11 @@ Examples: // Build API client — reuses the same factory as sync. getOAuthMgr := func(appName string) (*oauth.Manager, error) { - secretsPath := clientSecretsPath - if secretsPath == "" { - var err error - secretsPath, err = cfg.OAuth.ClientSecretsFor(appName) - if err != nil { - return nil, err - } - } scopes := oauth.Scopes if deletePermanent { scopes = oauth.ScopesDeletion } - return oauth.NewManagerWithScopes(secretsPath, cfg.TokensDir(), logger, scopes) + return resolveOAuthManager(cfg, appName, scopes, logger) } // For permanent deletion (not trash), service-account flows need the // elevated mail.google.com scope; trash-only uses the standard set. @@ -602,7 +584,7 @@ Examples: if mgrErr != nil { return mgrErr } - if err := promptScopeEscalation(ctx, oauthMgr, account, !useTrash, clientSecretsPath); err != nil { + if err := promptScopeEscalation(ctx, oauthMgr, account, !useTrash, sourceOAuthApp(src)); err != nil { if errors.Is(err, errUserCanceled) { return nil } @@ -745,7 +727,7 @@ var errUserCanceled = errors.New("user canceled scope escalation") // promptScopeEscalation prompts the user to re-authorize with elevated scopes. // It deletes the old token, runs the OAuth browser flow, and returns nil on // success. The caller should re-create the OAuth manager after this returns. -func promptScopeEscalation(ctx context.Context, oauthMgr *oauth.Manager, account string, batchDelete bool, clientSecretsPath string) error { +func promptScopeEscalation(ctx context.Context, oauthMgr *oauth.Manager, account string, batchDelete bool, appName string) error { fmt.Println("\n" + strings.Repeat("=", 70)) fmt.Println("PERMISSION UPGRADE REQUIRED") fmt.Println(strings.Repeat("=", 70)) @@ -795,9 +777,9 @@ func promptScopeEscalation(ctx context.Context, oauthMgr *oauth.Manager, account fmt.Println("Starting OAuth flow...") fmt.Println() - newMgr, err := oauth.NewManagerWithScopes(clientSecretsPath, cfg.TokensDir(), logger, requiredScopes) + newMgr, err := resolveOAuthManager(cfg, appName, requiredScopes, logger) if err != nil { - return fmt.Errorf("create oauth manager: %w", err) + return err } if err := newMgr.Authorize(ctx, account); err != nil { diff --git a/cmd/msgvault/cmd/oauth_resolve.go b/cmd/msgvault/cmd/oauth_resolve.go new file mode 100644 index 000000000..b2275c91c --- /dev/null +++ b/cmd/msgvault/cmd/oauth_resolve.go @@ -0,0 +1,49 @@ +package cmd + +import ( + "fmt" + "log/slog" + + "go.kenn.io/msgvault/internal/config" + "go.kenn.io/msgvault/internal/oauth" +) + +// resolveOAuthManager builds the *oauth.Manager appropriate for the +// account+config+scopes triple. Resolution order: +// +// 1. Named BYO: appName is non-empty and cfg.OAuth.Apps[appName] has +// client_secrets set — use that BYO OAuth client. +// 2. (If appName is non-empty but no client_secrets is registered for +// it) — return an error rather than falling through to embedded, +// because the user explicitly named a binding that doesn't exist. +// 3. Global BYO: appName is empty and cfg.OAuth.ClientSecrets is set — +// use the global BYO client. +// 4. Embedded: otherwise, use the centralized verified client. On the +// embedded path the manager is always built with oauth.ScopesEmbedded, +// ignoring the caller's per-call scope choice, because the embedded +// grant is broader than any per-call need. +// +// Callers handle service-account resolution themselves (via +// cfg.OAuth.ServiceAccountKeyFor(appName)) before calling this helper, +// because *oauth.Manager and the service-account manager have +// different interfaces. +func resolveOAuthManager( + cfg *config.Config, + appName string, + scopes []string, + logger *slog.Logger, +) (*oauth.Manager, error) { + if appName != "" { + app, ok := cfg.OAuth.Apps[appName] + if !ok || app.ClientSecrets == "" { + return nil, fmt.Errorf("OAuth app %q not configured: add [oauth.apps.%s] client_secrets to config.toml, or rebind the account with 'msgvault add-account ' (without --oauth-app) to use the embedded client", appName, appName) + } + return oauth.NewManagerWithScopes(app.ClientSecrets, cfg.TokensDir(), logger, scopes) + } + + if cfg.OAuth.ClientSecrets != "" { + return oauth.NewManagerWithScopes(cfg.OAuth.ClientSecrets, cfg.TokensDir(), logger, scopes) + } + + return oauth.NewEmbeddedManager(cfg.TokensDir(), logger, oauth.ScopesEmbedded) +} diff --git a/cmd/msgvault/cmd/oauth_resolve_test.go b/cmd/msgvault/cmd/oauth_resolve_test.go new file mode 100644 index 000000000..faffd9c4d --- /dev/null +++ b/cmd/msgvault/cmd/oauth_resolve_test.go @@ -0,0 +1,84 @@ +package cmd + +import ( + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "go.kenn.io/msgvault/internal/config" + "go.kenn.io/msgvault/internal/oauth" +) + +// writeStubClientSecrets writes a minimal valid client_secret.json that +// parseClientSecrets will accept. We only need this to verify the BYO +// path returns a non-nil manager — we don't run any OAuth flow. +func writeStubClientSecrets(t *testing.T, dir, name string) string { + t.Helper() + path := filepath.Join(dir, name) + const stub = `{"installed":{"client_id":"abc","client_secret":"xyz","redirect_uris":["http://localhost"]}}` + if err := os.WriteFile(path, []byte(stub), 0600); err != nil { + t.Fatalf("write %s: %v", path, err) + } + return path +} + +// newTestConfig returns a Config with Data.DataDir set to a fresh temp +// directory. TokensDir() returns /tokens, which is what the +// resolver passes to the OAuth manager constructors. +func newTestConfig(t *testing.T) *config.Config { + t.Helper() + return &config.Config{ + Data: config.DataConfig{DataDir: t.TempDir()}, + } +} + +func TestResolveOAuthManager_NamedBYO(t *testing.T) { + cfg := newTestConfig(t) + secrets := writeStubClientSecrets(t, cfg.Data.DataDir, "acme.json") + cfg.OAuth.Apps = map[string]config.OAuthApp{"acme": {ClientSecrets: secrets}} + mgr, err := resolveOAuthManager(cfg, "acme", oauth.Scopes, slog.Default()) + if err != nil { + t.Fatalf("resolveOAuthManager: %v", err) + } + if mgr == nil { + t.Fatal("manager is nil") + } +} + +func TestResolveOAuthManager_NamedNotConfigured(t *testing.T) { + cfg := newTestConfig(t) + _, err := resolveOAuthManager(cfg, "nonexistent", oauth.Scopes, slog.Default()) + if err == nil { + t.Fatal("expected error for unknown app name") + } + if !strings.Contains(err.Error(), "nonexistent") { + t.Errorf("error %q should mention the app name", err.Error()) + } +} + +func TestResolveOAuthManager_GlobalBYO(t *testing.T) { + cfg := newTestConfig(t) + cfg.OAuth.ClientSecrets = writeStubClientSecrets(t, cfg.Data.DataDir, "default.json") + mgr, err := resolveOAuthManager(cfg, "", oauth.Scopes, slog.Default()) + if err != nil { + t.Fatalf("resolveOAuthManager: %v", err) + } + if mgr == nil { + t.Fatal("manager is nil") + } +} + +func TestResolveOAuthManager_Embedded(t *testing.T) { + // Embedded credentials must be non-empty in this test (they are by + // default — the source has the dev placeholder strings). + cfg := newTestConfig(t) + mgr, err := resolveOAuthManager(cfg, "", oauth.Scopes, slog.Default()) + if err != nil { + t.Fatalf("resolveOAuthManager: %v", err) + } + if mgr == nil { + t.Fatal("manager is nil") + } +} diff --git a/cmd/msgvault/cmd/root.go b/cmd/msgvault/cmd/root.go index 0bbb16bba..9db639e24 100644 --- a/cmd/msgvault/cmd/root.go +++ b/cmd/msgvault/cmd/root.go @@ -6,7 +6,6 @@ import ( "fmt" "log/slog" "os" - "path/filepath" "runtime" "runtime/debug" "strings" @@ -196,8 +195,7 @@ func sanitizeArgs(args []string) []string { redactNext = false continue } - if before, _, ok := strings.Cut(a, "="); ok { - key := before + if key, _, found := strings.Cut(a, "="); found { if sensitive[key] { out = append(out, key+"=") continue @@ -322,81 +320,6 @@ func silenceUsageInRunE(cmd *cobra.Command) { } } -// oauthSetupHint returns help text for OAuth configuration issues, -// using the actual config file path so it's clear on all platforms. -func oauthSetupHint() string { - configPath := "" - if cfg != nil { - configPath = cfg.ConfigFilePath() - } - hint := fmt.Sprintf(` -To use msgvault, you need a Google Cloud OAuth credential: - 1. Follow the setup guide: https://msgvault.io/guides/oauth-setup/ - 2. Download the client_secret.json file - 3. Create or edit %s: - [oauth] - client_secrets = "/path/to/client_secret.json"`, configPath) - if cfg != nil && len(cfg.OAuth.Apps) > 0 { - hint += "\n\nNamed OAuth apps are configured. " + - "Use 'add-account --oauth-app ' to bind an account." - } - return hint -} - -// errOAuthNotConfigured returns a helpful error when OAuth client secrets are missing. -// It also searches for client_secret*.json files in common locations. -func errOAuthNotConfigured() error { - // Check common locations for client_secret*.json - hint := tryFindClientSecrets() - if hint != "" { - return fmt.Errorf("OAuth client secrets not configured.%s", hint) - } - return fmt.Errorf("OAuth client secrets not configured.%s", oauthSetupHint()) -} - -// tryFindClientSecrets looks for client_secret*.json in common locations -// and returns a hint if found. -func tryFindClientSecrets() string { - home, _ := os.UserHomeDir() - candidates := []string{ - filepath.Join(home, "Downloads", "client_secret*.json"), - "client_secret*.json", - } - if cfg != nil { - candidates = append(candidates, filepath.Join(cfg.HomeDir, "client_secret*.json")) - } - - for _, pattern := range candidates { - matches, _ := filepath.Glob(pattern) - if len(matches) > 0 { - configPath := "" - if cfg != nil { - configPath = cfg.ConfigFilePath() - } - return fmt.Sprintf(` - -Found OAuth credentials at: %s - -To use this file, add to %s: - [oauth] - client_secrets = %q - -Or copy the file to your msgvault home directory: - cp %q ~/.msgvault/client_secret.json`, matches[0], configPath, matches[0], matches[0]) - } - } - return "" -} - -// wrapOAuthError wraps an oauth/client-secrets error with setup instructions -// if the root cause is a missing or unreadable secrets file. -func wrapOAuthError(err error) error { - if errors.Is(err, os.ErrNotExist) || errors.Is(err, os.ErrPermission) { - return fmt.Errorf("OAuth client secrets file not accessible.%s", oauthSetupHint()) - } - return err -} - // isAuthInvalidError returns true if the error indicates the OAuth token is // permanently invalid (expired or revoked), as opposed to a transient failure // like a network error or context cancellation. @@ -489,7 +412,8 @@ func getTokenSourceWithReauth( // oauthManagerCache returns a resolver function that lazily creates and // caches oauth.Manager instances keyed by app name. The cache is safe -// for concurrent use (serve runs scheduled syncs in goroutines). +// for concurrent use (serve runs scheduled syncs in goroutines). The +// underlying resolution is delegated to resolveOAuthManager. func oauthManagerCache() func(appName string) (*oauth.Manager, error) { var mu sync.Mutex managers := map[string]*oauth.Manager{} @@ -499,14 +423,10 @@ func oauthManagerCache() func(appName string) (*oauth.Manager, error) { if mgr, ok := managers[appName]; ok { return mgr, nil } - secretsPath, err := cfg.OAuth.ClientSecretsFor(appName) + mgr, err := resolveOAuthManager(cfg, appName, oauth.Scopes, logger) if err != nil { return nil, err } - mgr, err := oauth.NewManager(secretsPath, cfg.TokensDir(), logger) - if err != nil { - return nil, wrapOAuthError(fmt.Errorf("create oauth manager: %w", err)) - } managers[appName] = mgr return mgr, nil } diff --git a/cmd/msgvault/cmd/root_test.go b/cmd/msgvault/cmd/root_test.go index 742dcb13b..92d67a60a 100644 --- a/cmd/msgvault/cmd/root_test.go +++ b/cmd/msgvault/cmd/root_test.go @@ -5,8 +5,6 @@ import ( "errors" "fmt" "net" - "os" - "strings" "sync/atomic" "testing" "time" @@ -18,77 +16,6 @@ import ( extOAuth2 "golang.org/x/oauth2" ) -func TestErrOAuthNotConfigured(t *testing.T) { - assert := assertpkg.New(t) - err := errOAuthNotConfigured() - requirepkg.Error(t, err, "errOAuthNotConfigured()") - - msg := err.Error() - - // Should contain the main message - assert.Contains(msg, "OAuth client secrets not configured", "missing 'not configured'") - - // Should contain either: - // 1. A "Found OAuth credentials" hint (if client_secret*.json exists on this machine) - // 2. The setup URL (if no credentials found) - hasFoundHint := strings.Contains(msg, "Found OAuth credentials at:") - hasSetupURL := strings.Contains(msg, "https://msgvault.io/guides/oauth-setup/") - - assert.True(hasFoundHint || hasSetupURL, - "error message missing both 'Found OAuth credentials' hint and setup URL: %q", msg) - - // Should contain config file instructions (either "config.toml" or "" placeholder) - assert.Contains(msg, "config", "error message missing config reference") -} - -func TestWrapOAuthError_NotExist(t *testing.T) { - originalErr := fmt.Errorf("open /path/to/secrets.json: %w", os.ErrNotExist) - - wrapped := wrapOAuthError(originalErr) - - msg := wrapped.Error() - - // Should contain accessible message (not "not found" anymore) - assertpkg.Contains(t, msg, "not accessible", "missing 'not accessible'") - // Should contain setup hint - assertpkg.Contains(t, msg, "https://msgvault.io/guides/oauth-setup/", "missing setup URL") -} - -func TestWrapOAuthError_Permission(t *testing.T) { - originalErr := fmt.Errorf("open /path/to/secrets.json: %w", os.ErrPermission) - - wrapped := wrapOAuthError(originalErr) - - msg := wrapped.Error() - - // Should contain accessible message - assertpkg.Contains(t, msg, "not accessible", "missing 'not accessible'") - // Should contain setup hint - assertpkg.Contains(t, msg, "https://msgvault.io/guides/oauth-setup/", "missing setup URL") -} - -func TestWrapOAuthError_OtherError(t *testing.T) { - originalErr := errors.New("some other error") - - wrapped := wrapOAuthError(originalErr) - - // Should return the original error unchanged - assertpkg.Equal(t, originalErr, wrapped, "wrapOAuthError() changed unrelated error") -} - -func TestWrapOAuthError_NestedNotExist(t *testing.T) { - // Test that errors.Is can find nested os.ErrNotExist - innerErr := fmt.Errorf("file error: %w", os.ErrNotExist) - outerErr := fmt.Errorf("oauth manager: %w", innerErr) - - wrapped := wrapOAuthError(outerErr) - - msg := wrapped.Error() - - // Should detect the nested os.ErrNotExist and wrap appropriately - assertpkg.Contains(t, msg, "not accessible", "failed to detect nested os.ErrNotExist") -} - // newTestRootCmd creates a fresh root command for testing, avoiding mutation // of the global rootCmd which could cause race conditions in parallel tests. func newTestRootCmd() *cobra.Command { diff --git a/cmd/msgvault/cmd/serve.go b/cmd/msgvault/cmd/serve.go index 7089770ee..dc10d367b 100644 --- a/cmd/msgvault/cmd/serve.go +++ b/cmd/msgvault/cmd/serve.go @@ -66,11 +66,6 @@ func runServe(cmd *cobra.Command, args []string) error { logger.Warn("api_key is very short — use a randomly generated key of at least 32 characters") } - // Validate config - if !cfg.OAuth.HasAnyConfig() { - return errOAuthNotConfigured() - } - // Check for scheduled accounts (warn but don't fail - allows token upload first) scheduled := cfg.ScheduledAccounts() if len(scheduled) == 0 { diff --git a/cmd/msgvault/cmd/sync.go b/cmd/msgvault/cmd/sync.go index 460b8ff03..49fcbfa9c 100644 --- a/cmd/msgvault/cmd/sync.go +++ b/cmd/msgvault/cmd/sync.go @@ -128,10 +128,6 @@ Examples: for _, src := range allSources { switch src.SourceType { case sourceTypeGmail: - if !cfg.OAuth.HasAnyConfig() { - fmt.Printf("Skipping %s (OAuth not configured)\n", src.Identifier) - continue - } appName := sourceOAuthApp(src) if !src.SyncCursor.Valid || src.SyncCursor.String == "" { fmt.Printf("Skipping %s (no history ID - run 'sync-full' first)\n", src.Identifier) diff --git a/cmd/msgvault/cmd/sync_test.go b/cmd/msgvault/cmd/sync_test.go index 4c4f694d8..bfb74382b 100644 --- a/cmd/msgvault/cmd/sync_test.go +++ b/cmd/msgvault/cmd/sync_test.go @@ -410,7 +410,7 @@ func TestSyncFullCmd_MalformedDateRejectsBeforeSync(t *testing.T) { _ = s.Close() // Write OAuth client secrets and a fake token so the Gmail - // source passes discovery checks (HasAnyConfig + HasToken). + // source passes discovery checks. secretsPath := filepath.Join(tmpDir, "client_secret.json") require.NoError(os.WriteFile(secretsPath, []byte(fakeClientSecrets), 0600), "write client secrets") tokensDir := filepath.Join(tmpDir, "tokens") diff --git a/cmd/msgvault/cmd/syncfull.go b/cmd/msgvault/cmd/syncfull.go index 5ab78fd75..ecca2f0be 100644 --- a/cmd/msgvault/cmd/syncfull.go +++ b/cmd/msgvault/cmd/syncfull.go @@ -119,10 +119,6 @@ Examples: for _, src := range allSources { switch src.SourceType { case sourceTypeGmail: - if !cfg.OAuth.HasAnyConfig() { - fmt.Printf("Skipping %s (OAuth not configured)\n", src.Identifier) - continue - } appName := sourceOAuthApp(src) // Service accounts are always ready — no per-user token needed if cfg.OAuth.ServiceAccountKeyFor(appName) == "" { @@ -444,16 +440,7 @@ func buildSyncQuery() string { parts = append(parts, syncQuery) } - result := "" - var resultSb447 strings.Builder - for i, p := range parts { - if i > 0 { - resultSb447.WriteString(" ") - } - resultSb447.WriteString(p) - } - result += resultSb447.String() - return result + return strings.Join(parts, " ") } // CLIProgress implements gmail.SyncProgressWithDate for terminal output. diff --git a/cmd/msgvault/cmd/verify.go b/cmd/msgvault/cmd/verify.go index 19661e5af..297accc49 100644 --- a/cmd/msgvault/cmd/verify.go +++ b/cmd/msgvault/cmd/verify.go @@ -179,10 +179,6 @@ Examples: appName = sourceOAuthApp(src) } - if !cfg.OAuth.HasAnyConfig() { - return errOAuthNotConfigured() - } - // Set up context with cancellation ctx, cancel := context.WithCancel(cmd.Context()) defer cancel() @@ -210,13 +206,9 @@ Examples: return fmt.Errorf("service account token for %s: %w", email, err) } } else { - clientSecretsPath, secretsErr := cfg.OAuth.ClientSecretsFor(appName) - if secretsErr != nil { - return secretsErr - } - oauthMgr, mgrErr := oauth.NewManager(clientSecretsPath, cfg.TokensDir(), logger) + oauthMgr, mgrErr := resolveOAuthManager(cfg, appName, oauth.Scopes, logger) if mgrErr != nil { - return wrapOAuthError(fmt.Errorf("create oauth manager: %w", mgrErr)) + return mgrErr } // Machine-readable mode must not enter an interactive OAuth // flow that writes prompts to stdout before the JSON object. diff --git a/internal/config/config.go b/internal/config/config.go index 9180e1520..027fa7342 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -204,20 +204,6 @@ func (o *OAuthConfig) ServiceAccountKeyFor(name string) string { return "" } -// HasAnyConfig returns true if any OAuth configuration exists -// (default or named apps). -func (o *OAuthConfig) HasAnyConfig() bool { - if o.ClientSecrets != "" || o.ServiceAccountKey != "" { - return true - } - for _, app := range o.Apps { - if app.ClientSecrets != "" || app.ServiceAccountKey != "" { - return true - } - } - return false -} - // MicrosoftConfig holds Microsoft 365 / Azure AD OAuth configuration. type MicrosoftConfig struct { ClientID string `toml:"client_id"` diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b0ba93c54..bc7ae0631 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1006,73 +1006,6 @@ func TestOAuthConfig_ServiceAccountKeyFor(t *testing.T) { } } -func TestOAuthConfig_HasAnyConfig(t *testing.T) { - tests := []struct { - name string - config OAuthConfig - want bool - }{ - { - name: "empty config", - config: OAuthConfig{}, - want: false, - }, - { - name: "default only", - config: OAuthConfig{ClientSecrets: "/path/to/default.json"}, - want: true, - }, - { - name: "named app only", - config: OAuthConfig{ - Apps: map[string]OAuthApp{ - "acme": {ClientSecrets: "/path/to/acme.json"}, - }, - }, - want: true, - }, - { - name: "named app with empty path", - config: OAuthConfig{ - Apps: map[string]OAuthApp{ - "acme": {ClientSecrets: ""}, - }, - }, - want: false, - }, - { - name: "default service account only", - config: OAuthConfig{ServiceAccountKey: "/path/to/service-account.json"}, - want: true, - }, - { - name: "named service account only", - config: OAuthConfig{ - Apps: map[string]OAuthApp{ - "workspace": {ServiceAccountKey: "/path/to/workspace.json"}, - }, - }, - want: true, - }, - { - name: "mixed oauth and service account", - config: OAuthConfig{ - ClientSecrets: "/path/to/default.json", - Apps: map[string]OAuthApp{ - "workspace": {ServiceAccountKey: "/path/to/workspace.json"}, - }, - }, - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assertpkg.Equal(t, tt.want, tt.config.HasAnyConfig()) - }) - } -} - func TestLoadWithNamedOAuthApps(t *testing.T) { require := requirepkg.New(t) assert := assertpkg.New(t) @@ -1112,9 +1045,6 @@ client_secrets = "/absolute/personal.json" personal, ok := cfg.OAuth.Apps["personal"] require.True(ok, "Apps[personal] not found") assert.Equal("/absolute/personal.json", personal.ClientSecrets) - - // HasAnyConfig should be true - assert.True(cfg.OAuth.HasAnyConfig()) } func TestLoadExpandsVectorDBPath(t *testing.T) { @@ -1304,9 +1234,6 @@ client_secrets = "/path/to/acme.json" // Default should be empty assert.Empty(cfg.OAuth.ClientSecrets) - // HasAnyConfig should still be true - assert.True(cfg.OAuth.HasAnyConfig()) - // ClientSecretsFor("") should fail _, err = cfg.OAuth.ClientSecretsFor("") require.Error(err, "ClientSecretsFor(\"\") should error with no default") From d4cac985be76772b466fb7432dd75c9e50a2516a Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 21 May 2026 08:24:50 -0400 Subject: [PATCH 04/10] refactor(setup): drop interactive OAuth step --- cmd/msgvault/cmd/setup.go | 100 ++++++++------------------------- cmd/msgvault/cmd/setup_test.go | 12 +++- 2 files changed, 33 insertions(+), 79 deletions(-) diff --git a/cmd/msgvault/cmd/setup.go b/cmd/msgvault/cmd/setup.go index cbcf3f867..d9bce8664 100644 --- a/cmd/msgvault/cmd/setup.go +++ b/cmd/msgvault/cmd/setup.go @@ -4,7 +4,6 @@ import ( "bufio" "crypto/rand" "encoding/hex" - "errors" "fmt" "io" "net" @@ -21,10 +20,10 @@ var setupCmd = &cobra.Command{ Short: "Interactive setup wizard for first-run configuration", Long: `Interactive setup wizard to configure msgvault for first use. -This command helps you: - 1. Locate or configure Google OAuth credentials - 2. Create the config.toml file - 3. Optionally configure a remote NAS server for token export +This command helps you optionally configure a remote NAS server for +token export and headless deployment. msgvault ships with an embedded +verified OAuth client, so no client_secret.json is required for the +standard setup. Run this once after installing msgvault to get started quickly.`, Args: cobra.NoArgs, @@ -41,45 +40,35 @@ func runSetup(cmd *cobra.Command, args []string) error { fmt.Println("Welcome to msgvault setup!") fmt.Println() - // Ensure home directory exists if err := cfg.EnsureHomeDir(); err != nil { return fmt.Errorf("create home directory: %w", err) } - // Step 1: Find or prompt for OAuth credentials - secretsPath, err := setupOAuthSecrets(reader) + // Configure remote NAS (optional). msgvault now ships with an + // embedded verified OAuth client, so the old "Step 1: OAuth + // credentials" prompt is gone. Operators who want their own OAuth + // client can still set [oauth] client_secrets in config.toml + // manually. + remoteURL, remoteAPIKey, err := setupRemoteServer(reader) if err != nil { return err } - // Step 2: Optionally configure remote NAS - remoteURL, remoteAPIKey, err := setupRemoteServer(reader, secretsPath) - if err != nil { - return err - } - - // Step 3: Update config - if secretsPath != "" { - cfg.OAuth.ClientSecrets = secretsPath - } if remoteURL != "" { cfg.Remote.URL = remoteURL cfg.Remote.APIKey = remoteAPIKey - // Auto-set for HTTP: target is Tailscale/LAN, not public internet. if strings.HasPrefix(remoteURL, "http://") { cfg.Remote.AllowInsecure = true } } - // Only save if we configured something - if secretsPath != "" || remoteURL != "" { + if remoteURL != "" { if err := cfg.Save(); err != nil { return fmt.Errorf("save config: %w", err) } fmt.Printf("\nConfiguration saved to %s\n", cfg.ConfigFilePath()) } - // Print next steps fmt.Println() fmt.Println("Setup complete! Next steps:") fmt.Println() @@ -99,55 +88,10 @@ func runSetup(cmd *cobra.Command, args []string) error { return nil } -func setupOAuthSecrets(reader *bufio.Reader) (string, error) { - fmt.Println("Step 1: OAuth Credentials") - fmt.Println("--------------------------") - - // Check if already configured - if cfg.OAuth.ClientSecrets != "" { - fmt.Printf("OAuth credentials already configured: %s\n", cfg.OAuth.ClientSecrets) - if promptYesNo(reader, "Keep existing configuration?") { - return "", nil - } - } - - fmt.Println() - fmt.Println("You need a Google Cloud OAuth credential (client_secret.json).") - fmt.Println() - fmt.Println("To get one:") - fmt.Println(" 1. Go to https://console.cloud.google.com/apis/credentials") - fmt.Println(" 2. Create OAuth client ID (Desktop app)") - fmt.Println(" 3. Download the JSON file") - fmt.Println() - - // Prompt for path - fmt.Print("Path to client_secret.json: ") - path, _ := reader.ReadString('\n') - path = strings.TrimSpace(path) - - if path == "" { - return "", errors.New("OAuth credentials path is required") - } - - // Expand ~ in path - if strings.HasPrefix(path, "~/") { - home, _ := os.UserHomeDir() - path = filepath.Join(home, path[2:]) - } - - // Verify file exists - if _, err := os.Stat(path); os.IsNotExist(err) { - return "", fmt.Errorf("file not found: %s", path) - } - - fmt.Printf("Using: %s\n", path) - return path, nil -} - -func setupRemoteServer(reader *bufio.Reader, oauthSecretsPath string) (string, string, error) { +func setupRemoteServer(reader *bufio.Reader) (string, string, error) { fmt.Println() - fmt.Println("Step 2: Remote NAS Server (Optional)") - fmt.Println("-------------------------------------") + fmt.Println("Remote NAS Server (Optional)") + fmt.Println("----------------------------") fmt.Println("Configure a remote msgvault server to export tokens for headless deployment.") fmt.Println() @@ -201,11 +145,7 @@ func setupRemoteServer(reader *bufio.Reader, oauthSecretsPath string) (string, s fmt.Printf("\nGenerated API key: %s\n", apiKey) // Create NAS deployment bundle - // Use existing secrets path if user kept their current OAuth config - effectiveSecrets := oauthSecretsPath - if effectiveSecrets == "" { - effectiveSecrets = cfg.OAuth.ClientSecrets - } + effectiveSecrets := cfg.OAuth.ClientSecrets // empty when operator uses embedded bundleDir := filepath.Join(cfg.HomeDir, "nas-bundle") if err := createNASBundle(bundleDir, apiKey, effectiveSecrets, port); err != nil { fmt.Printf("Warning: Could not create NAS bundle: %v\n", err) @@ -246,9 +186,6 @@ bind_addr = "0.0.0.0" api_port = 8080 api_key = %q -[oauth] -client_secrets = "/data/client_secret.json" - [sync] rate_limit_qps = 5 @@ -260,6 +197,13 @@ rate_limit_qps = 5 # enabled = true `, apiKey) + if oauthSecretsPath != "" { + nasConfig += ` +[oauth] +client_secrets = "/data/client_secret.json" +` + } + configPath := filepath.Join(bundleDir, "config.toml") if err := os.WriteFile(configPath, []byte(nasConfig), 0600); err != nil { return fmt.Errorf("write config.toml: %w", err) diff --git a/cmd/msgvault/cmd/setup_test.go b/cmd/msgvault/cmd/setup_test.go index 128e4f19c..61819445f 100644 --- a/cmd/msgvault/cmd/setup_test.go +++ b/cmd/msgvault/cmd/setup_test.go @@ -33,6 +33,8 @@ func TestCreateNASBundle(t *testing.T) { configStr := string(configData) assert.Contains(configStr, apiKey, "config.toml should contain the API key") assert.Contains(configStr, "0.0.0.0", "config.toml should bind to 0.0.0.0") + assert.Contains(configStr, "[oauth]", "config.toml should contain [oauth] section when secrets are provided") + assert.Contains(configStr, `client_secrets = "/data/client_secret.json"`, "config.toml should set client_secrets") // Verify config.toml has secure permissions // Windows doesn't support Unix file permissions. @@ -73,6 +75,12 @@ func TestCreateNASBundle_NoSecrets(t *testing.T) { // client_secret.json should NOT exist (no source path given) _, err = os.Stat(filepath.Join(bundleDir, "client_secret.json")) assert.True(os.IsNotExist(err), "client_secret.json should not exist when no secrets path given") + + // config.toml should NOT contain [oauth] section when no secrets provided; + // the NAS instance falls back to the embedded verified OAuth client. + configBytes, err := os.ReadFile(filepath.Join(bundleDir, "config.toml")) + requirepkg.NoError(t, err, "read bundle config") + assert.NotContains(string(configBytes), "[oauth]", "config.toml should not contain [oauth] section when no secrets provided") } func TestCreateNASBundle_CopiesSecrets(t *testing.T) { @@ -95,7 +103,9 @@ func TestCreateNASBundle_CopiesSecrets(t *testing.T) { // config.toml should reference /data/client_secret.json cfgData, err := os.ReadFile(filepath.Join(bundleDir, "config.toml")) require.NoError(err, "read config.toml") - assert.Contains(string(cfgData), `/data/client_secret.json`, "config.toml should reference /data/client_secret.json") + cfgStr := string(cfgData) + assert.Contains(cfgStr, "[oauth]", "config.toml should contain [oauth] section when secrets are provided") + assert.Contains(cfgStr, `/data/client_secret.json`, "config.toml should reference /data/client_secret.json") } func TestCreateNASBundle_InvalidSecretPath(t *testing.T) { From beff875f345f98efab5cf416ab26760468c09c07 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 21 May 2026 08:24:53 -0400 Subject: [PATCH 05/10] build: inject embedded OAuth credentials via ldflags --- .github/workflows/release.yml | 24 +++++++++++++++++++++--- Makefile | 7 +++++++ internal/oauth/embedded.go | 2 +- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2a14a544f..0e48371bc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -52,12 +52,18 @@ jobs: GOOS: linux GOARCH: ${{ matrix.goarch }} CGO_ENABLED: '1' + MSGVAULT_OAUTH_CLIENT_ID: ${{ secrets.MSGVAULT_OAUTH_CLIENT_ID }} + MSGVAULT_OAUTH_CLIENT_SECRET: ${{ secrets.MSGVAULT_OAUTH_CLIENT_SECRET }} run: | + if [ -z "$MSGVAULT_OAUTH_CLIENT_ID" ] || [ -z "$MSGVAULT_OAUTH_CLIENT_SECRET" ]; then + echo "FATAL: MSGVAULT_OAUTH_CLIENT_ID and MSGVAULT_OAUTH_CLIENT_SECRET repository secrets must be set" >&2 + exit 1 + fi export PATH="/usr/local/go/bin:$HOME/go/bin:$PATH" VERSION=${GITHUB_REF#refs/tags/v} mkdir -p dist - LDFLAGS="-s -w -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=v${VERSION} -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=$(printf '%s' "$GITHUB_SHA" | cut -c1-8) -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=$(date -u +%Y-%m-%dT%H:%M:%SZ) -extldflags '-lstdc++ -lm'" + LDFLAGS="-s -w -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=v${VERSION} -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=$(printf '%s' "$GITHUB_SHA" | cut -c1-8) -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=$(date -u +%Y-%m-%dT%H:%M:%SZ) -X go.kenn.io/msgvault/internal/oauth.oauthClientID=${MSGVAULT_OAUTH_CLIENT_ID} -X go.kenn.io/msgvault/internal/oauth.oauthClientSecret=${MSGVAULT_OAUTH_CLIENT_SECRET} -extldflags '-lstdc++ -lm'" go build -tags "fts5 sqlite_vec" -trimpath -buildvcs=false -ldflags="$LDFLAGS" -o dist/msgvault ./cmd/msgvault echo "--- Binary info ---" @@ -106,11 +112,17 @@ jobs: GOOS: darwin GOARCH: ${{ matrix.goarch }} CGO_ENABLED: 1 + MSGVAULT_OAUTH_CLIENT_ID: ${{ secrets.MSGVAULT_OAUTH_CLIENT_ID }} + MSGVAULT_OAUTH_CLIENT_SECRET: ${{ secrets.MSGVAULT_OAUTH_CLIENT_SECRET }} run: | + if [ -z "$MSGVAULT_OAUTH_CLIENT_ID" ] || [ -z "$MSGVAULT_OAUTH_CLIENT_SECRET" ]; then + echo "FATAL: MSGVAULT_OAUTH_CLIENT_ID and MSGVAULT_OAUTH_CLIENT_SECRET repository secrets must be set" >&2 + exit 1 + fi VERSION=${GITHUB_REF#refs/tags/v} mkdir -p dist - LDFLAGS="-s -w -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=v${VERSION} -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=$(printf '%s' "$GITHUB_SHA" | cut -c1-8) -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=$(date -u +%Y-%m-%dT%H:%M:%SZ)" + LDFLAGS="-s -w -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=v${VERSION} -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=$(printf '%s' "$GITHUB_SHA" | cut -c1-8) -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=$(date -u +%Y-%m-%dT%H:%M:%SZ) -X go.kenn.io/msgvault/internal/oauth.oauthClientID=${MSGVAULT_OAUTH_CLIENT_ID} -X go.kenn.io/msgvault/internal/oauth.oauthClientSecret=${MSGVAULT_OAUTH_CLIENT_SECRET}" go build -tags "fts5 sqlite_vec" -trimpath -ldflags="$LDFLAGS" -o dist/msgvault ./cmd/msgvault echo "--- Binary info ---" @@ -172,11 +184,17 @@ jobs: CGO_ENABLED: '1' CGO_CFLAGS: "-IC:/msys64/mingw64/include -fgnu89-inline" CGO_LDFLAGS: "-Wl,--allow-multiple-definition" + MSGVAULT_OAUTH_CLIENT_ID: ${{ secrets.MSGVAULT_OAUTH_CLIENT_ID }} + MSGVAULT_OAUTH_CLIENT_SECRET: ${{ secrets.MSGVAULT_OAUTH_CLIENT_SECRET }} run: | + if [ -z "$MSGVAULT_OAUTH_CLIENT_ID" ] || [ -z "$MSGVAULT_OAUTH_CLIENT_SECRET" ]; then + echo "FATAL: MSGVAULT_OAUTH_CLIENT_ID and MSGVAULT_OAUTH_CLIENT_SECRET repository secrets must be set" >&2 + exit 1 + fi VERSION="${GITHUB_REF#refs/tags/v}" mkdir -p dist - LDFLAGS="-s -w -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=v${VERSION} -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=$(printf '%s' "$GITHUB_SHA" | cut -c1-8) -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=$(date -u +%Y-%m-%dT%H:%M:%SZ)" + LDFLAGS="-s -w -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=v${VERSION} -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=$(printf '%s' "$GITHUB_SHA" | cut -c1-8) -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=$(date -u +%Y-%m-%dT%H:%M:%SZ) -X go.kenn.io/msgvault/internal/oauth.oauthClientID=${MSGVAULT_OAUTH_CLIENT_ID} -X go.kenn.io/msgvault/internal/oauth.oauthClientSecret=${MSGVAULT_OAUTH_CLIENT_SECRET}" go build -tags "fts5 sqlite_vec" -trimpath -ldflags="$LDFLAGS" -o dist/msgvault.exe ./cmd/msgvault # Smoke test diff --git a/Makefile b/Makefile index c1d9df7e7..cab6912df 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,13 @@ LDFLAGS := -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=$(VERSION) \ -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=$(COMMIT) \ -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=$(BUILD_DATE) +# Only inject embedded OAuth credentials when both env vars are set; +# otherwise leave the compiled-in defaults from internal/oauth/embedded.go. +ifneq ($(and $(MSGVAULT_OAUTH_CLIENT_ID),$(MSGVAULT_OAUTH_CLIENT_SECRET)),) +LDFLAGS += -X go.kenn.io/msgvault/internal/oauth.oauthClientID=$(MSGVAULT_OAUTH_CLIENT_ID) \ + -X go.kenn.io/msgvault/internal/oauth.oauthClientSecret=$(MSGVAULT_OAUTH_CLIENT_SECRET) +endif + LDFLAGS_RELEASE := $(LDFLAGS) -s -w # Default build tags applied to every go build/test/bench invocation. diff --git a/internal/oauth/embedded.go b/internal/oauth/embedded.go index bad7e9833..d27684b11 100644 --- a/internal/oauth/embedded.go +++ b/internal/oauth/embedded.go @@ -12,7 +12,7 @@ import ( // msgvault OAuth client credentials. They are package vars (not consts) // so release builds can override them via: // -// go build -ldflags "-X github.com/wesm/msgvault/internal/oauth.oauthClientID=..." +// go build -ldflags "-X go.kenn.io/msgvault/internal/oauth.oauthClientID=..." // // Per https://developers.google.com/identity/protocols/oauth2 the desktop // client secret is "obviously not treated as a secret"; PKCE provides the From 7f3916ffbfc1601ced5acecea9ad8f0efb80bb28 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 21 May 2026 08:24:55 -0400 Subject: [PATCH 06/10] docs: drop OAuth setup prerequisite from Quick Start --- README.md | 41 +++++++++++++++++++++++++--------- cmd/msgvault/cmd/quickstart.md | 17 +++++++++----- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 0cecf35ec..aaa692f7a 100644 --- a/README.md +++ b/README.md @@ -64,16 +64,22 @@ conda install -c conda-forge msgvault ## Quick Start -> **Prerequisites:** You need a Google Cloud OAuth credential before adding an account. -> Follow the **[OAuth Setup Guide](https://msgvault.io/guides/oauth-setup/)** to create one (~5 minutes). - -```bash +```sh +# Initialize the database msgvault init-db -msgvault add-account you@gmail.com # opens browser for OAuth -msgvault sync-full you@gmail.com --limit 100 + +# Add a Gmail account — opens your browser for consent +msgvault add-account you@gmail.com + +# Sync mail +msgvault sync-full you@gmail.com + +# Browse the archive msgvault tui ``` +No Google Cloud Console setup required: msgvault ships with a verified OAuth client. + ## Commands | Command | Description | @@ -174,14 +180,29 @@ All data lives in `~/.msgvault/` by default (override with `MSGVAULT_HOME`). ```toml # ~/.msgvault/config.toml -[oauth] -client_secrets = "/path/to/client_secret.json" - [sync] rate_limit_qps = 5 ``` -See the [Configuration Guide](https://msgvault.io/configuration/) for all options. +See the [Configuration Guide](https://msgvault.io/configuration/) for all options. To override the embedded OAuth client, see [Advanced: bring your own OAuth client](#advanced-bring-your-own-oauth-client) below. + +### Advanced: bring your own OAuth client + +The default flow uses msgvault's centralized verified OAuth client. You only need your own Cloud project if: + +- Your Workspace organization prohibits authorizing third-party OAuth apps +- You prefer your own Cloud project's third-party-access listing to show +- You need your own Gmail API quota for very large mailboxes +- You want a fallback before msgvault's centralized client finishes Google verification + +Follow the [OAuth setup guide](https://msgvault.io/guides/oauth-setup/) to create a Desktop OAuth client, then add it to `~/.msgvault/config.toml`: + +```toml +[oauth] +client_secrets = "/path/to/client_secret.json" +``` + +Use `--oauth-app NAME` for per-account named-app routing — see the OAuth setup guide for details. ### Multiple OAuth Apps (Google Workspace) diff --git a/cmd/msgvault/cmd/quickstart.md b/cmd/msgvault/cmd/quickstart.md index fd60c930c..3830431ff 100644 --- a/cmd/msgvault/cmd/quickstart.md +++ b/cmd/msgvault/cmd/quickstart.md @@ -28,21 +28,28 @@ msgvault add-account user@gmail.com msgvault add-account user@gmail.com --headless ``` -Requires `oauth.client_secrets` to be set in `~/.msgvault/config.toml` pointing -to a Google Cloud OAuth client secrets JSON file. +msgvault ships with a verified OAuth client, so no Google Cloud Console +setup is required. Bringing your own OAuth app is optional and advanced — +see the Configuration section below. ### Configuration The config file is at `~/.msgvault/config.toml`: ```toml -[oauth] -client_secrets = "/path/to/client_secret.json" - [sync] rate_limit_qps = 5 ``` +Advanced: to override the embedded OAuth client with your own Google Cloud +Desktop OAuth client (Workspace orgs that block third-party apps, custom +quota, or a verification-window fallback), add: + +```toml +[oauth] +client_secrets = "/path/to/client_secret.json" +``` + ## Syncing email ### Full sync From e089adba2b3966fe8a8b578f4faa41544b15b5af Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 21 May 2026 12:57:39 -0400 Subject: [PATCH 07/10] chore: setup oauth info --- internal/oauth/embedded.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/oauth/embedded.go b/internal/oauth/embedded.go index d27684b11..d1d76ca72 100644 --- a/internal/oauth/embedded.go +++ b/internal/oauth/embedded.go @@ -19,8 +19,8 @@ import ( // flow security. The values below are the dev project's credentials, // suitable for contributor builds. Production binaries override both. var ( - oauthClientID = "TBD-msgvault-dev-client-id" - oauthClientSecret = "TBD-msgvault-dev-client-secret" + oauthClientID = "913114107126-tfruv1983bsv811mbjkqjvtd23io5b93.apps.googleusercontent.com" + oauthClientSecret = "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" ) // HasEmbeddedCredentials reports whether the package vars are non-empty. From c3290d45ddfe09dad40e618e42ab9b9d569ca780 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 18 Jun 2026 08:57:33 -0400 Subject: [PATCH 08/10] fix: clean up OAuth lint after rebase The rebase kept the embedded OAuth path but exposed CI lint failures in the merged test coverage and callback handler. Keep the OAuth resolver behavior unchanged while converting the affected tests to the testify helpers expected by this repo. The callback error response now escapes provider-supplied error text before writing it to the browser, and the embedded credential constants carry the gosec rationale that these are public desktop client identifiers overridden in release builds. --- cmd/msgvault/cmd/addaccount_test.go | 27 ++++++++-------- cmd/msgvault/cmd/oauth_resolve_test.go | 39 +++++++--------------- cmd/msgvault/cmd/setup_test.go | 9 +++--- internal/oauth/embedded.go | 6 ++-- internal/oauth/embedded_test.go | 45 +++++++++----------------- internal/oauth/oauth.go | 3 +- internal/oauth/oauth_test.go | 40 +++++++++-------------- 7 files changed, 65 insertions(+), 104 deletions(-) diff --git a/cmd/msgvault/cmd/addaccount_test.go b/cmd/msgvault/cmd/addaccount_test.go index 6747a7c40..b16eedde9 100644 --- a/cmd/msgvault/cmd/addaccount_test.go +++ b/cmd/msgvault/cmd/addaccount_test.go @@ -1033,14 +1033,15 @@ func TestAddAccount_ResolverBranches(t *testing.T) { tests := []struct { name string appName string - setup func(cfg *config.Config, t *testing.T) + setup func(t *testing.T, cfg *config.Config) wantErr bool errContains string }{ { name: "named BYO with client_secrets", appName: "acme", - setup: func(cfg *config.Config, t *testing.T) { + setup: func(t *testing.T, cfg *config.Config) { + t.Helper() path := writeStubClientSecrets(t, cfg.Data.DataDir, "acme.json") cfg.OAuth.Apps = map[string]config.OAuthApp{"acme": {ClientSecrets: path}} }, @@ -1049,14 +1050,15 @@ func TestAddAccount_ResolverBranches(t *testing.T) { { name: "named app without client_secrets", appName: "missing", - setup: func(cfg *config.Config, t *testing.T) {}, + setup: func(t *testing.T, cfg *config.Config) { t.Helper() }, wantErr: true, errContains: "missing", }, { name: "global BYO", appName: "", - setup: func(cfg *config.Config, t *testing.T) { + setup: func(t *testing.T, cfg *config.Config) { + t.Helper() cfg.OAuth.ClientSecrets = writeStubClientSecrets(t, cfg.Data.DataDir, "default.json") }, wantErr: false, @@ -1064,27 +1066,24 @@ func TestAddAccount_ResolverBranches(t *testing.T) { { name: "no config falls through to embedded", appName: "", - setup: func(cfg *config.Config, t *testing.T) {}, + setup: func(t *testing.T, cfg *config.Config) { t.Helper() }, wantErr: false, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + require := requirepkg.New(t) cfg := newTestConfig(t) - tc.setup(cfg, t) + tc.setup(t, cfg) _, err := resolveOAuthManager(cfg, tc.appName, oauth.Scopes, slog.Default()) if tc.wantErr { - if err == nil { - t.Fatal("expected error, got nil") - } - if tc.errContains != "" && !strings.Contains(err.Error(), tc.errContains) { - t.Errorf("error %q should contain %q", err.Error(), tc.errContains) + require.Error(err, "expected error") + if tc.errContains != "" { + require.ErrorContains(err, tc.errContains) } return } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoError(err, "resolveOAuthManager") }) } } diff --git a/cmd/msgvault/cmd/oauth_resolve_test.go b/cmd/msgvault/cmd/oauth_resolve_test.go index faffd9c4d..fe0a7c195 100644 --- a/cmd/msgvault/cmd/oauth_resolve_test.go +++ b/cmd/msgvault/cmd/oauth_resolve_test.go @@ -4,9 +4,10 @@ import ( "log/slog" "os" "path/filepath" - "strings" "testing" + assertpkg "github.com/stretchr/testify/assert" + requirepkg "github.com/stretchr/testify/require" "go.kenn.io/msgvault/internal/config" "go.kenn.io/msgvault/internal/oauth" ) @@ -18,9 +19,7 @@ func writeStubClientSecrets(t *testing.T, dir, name string) string { t.Helper() path := filepath.Join(dir, name) const stub = `{"installed":{"client_id":"abc","client_secret":"xyz","redirect_uris":["http://localhost"]}}` - if err := os.WriteFile(path, []byte(stub), 0600); err != nil { - t.Fatalf("write %s: %v", path, err) - } + requirepkg.NoError(t, os.WriteFile(path, []byte(stub), 0600), "write %s", path) return path } @@ -39,35 +38,23 @@ func TestResolveOAuthManager_NamedBYO(t *testing.T) { secrets := writeStubClientSecrets(t, cfg.Data.DataDir, "acme.json") cfg.OAuth.Apps = map[string]config.OAuthApp{"acme": {ClientSecrets: secrets}} mgr, err := resolveOAuthManager(cfg, "acme", oauth.Scopes, slog.Default()) - if err != nil { - t.Fatalf("resolveOAuthManager: %v", err) - } - if mgr == nil { - t.Fatal("manager is nil") - } + requirepkg.NoError(t, err, "resolveOAuthManager") + requirepkg.NotNil(t, mgr, "manager") } func TestResolveOAuthManager_NamedNotConfigured(t *testing.T) { cfg := newTestConfig(t) _, err := resolveOAuthManager(cfg, "nonexistent", oauth.Scopes, slog.Default()) - if err == nil { - t.Fatal("expected error for unknown app name") - } - if !strings.Contains(err.Error(), "nonexistent") { - t.Errorf("error %q should mention the app name", err.Error()) - } + requirepkg.Error(t, err, "expected error for unknown app name") + assertpkg.ErrorContains(t, err, "nonexistent") } func TestResolveOAuthManager_GlobalBYO(t *testing.T) { cfg := newTestConfig(t) cfg.OAuth.ClientSecrets = writeStubClientSecrets(t, cfg.Data.DataDir, "default.json") mgr, err := resolveOAuthManager(cfg, "", oauth.Scopes, slog.Default()) - if err != nil { - t.Fatalf("resolveOAuthManager: %v", err) - } - if mgr == nil { - t.Fatal("manager is nil") - } + requirepkg.NoError(t, err, "resolveOAuthManager") + requirepkg.NotNil(t, mgr, "manager") } func TestResolveOAuthManager_Embedded(t *testing.T) { @@ -75,10 +62,6 @@ func TestResolveOAuthManager_Embedded(t *testing.T) { // default — the source has the dev placeholder strings). cfg := newTestConfig(t) mgr, err := resolveOAuthManager(cfg, "", oauth.Scopes, slog.Default()) - if err != nil { - t.Fatalf("resolveOAuthManager: %v", err) - } - if mgr == nil { - t.Fatal("manager is nil") - } + requirepkg.NoError(t, err, "resolveOAuthManager") + requirepkg.NotNil(t, mgr, "manager") } diff --git a/cmd/msgvault/cmd/setup_test.go b/cmd/msgvault/cmd/setup_test.go index 61819445f..002c6cc04 100644 --- a/cmd/msgvault/cmd/setup_test.go +++ b/cmd/msgvault/cmd/setup_test.go @@ -60,17 +60,18 @@ func TestCreateNASBundle(t *testing.T) { } func TestCreateNASBundle_NoSecrets(t *testing.T) { + require := requirepkg.New(t) assert := assertpkg.New(t) bundleDir := filepath.Join(t.TempDir(), "nas-bundle") err := createNASBundle(bundleDir, "key", "", 8080) - requirepkg.NoError(t, err, "createNASBundle") + require.NoError(err, "createNASBundle") // config.toml and docker-compose.yml should exist _, err = os.Stat(filepath.Join(bundleDir, "config.toml")) - requirepkg.NoError(t, err, "config.toml should exist") + require.NoError(err, "config.toml should exist") _, err = os.Stat(filepath.Join(bundleDir, "docker-compose.yml")) - requirepkg.NoError(t, err, "docker-compose.yml should exist") + require.NoError(err, "docker-compose.yml should exist") // client_secret.json should NOT exist (no source path given) _, err = os.Stat(filepath.Join(bundleDir, "client_secret.json")) @@ -79,7 +80,7 @@ func TestCreateNASBundle_NoSecrets(t *testing.T) { // config.toml should NOT contain [oauth] section when no secrets provided; // the NAS instance falls back to the embedded verified OAuth client. configBytes, err := os.ReadFile(filepath.Join(bundleDir, "config.toml")) - requirepkg.NoError(t, err, "read bundle config") + require.NoError(err, "read bundle config") assert.NotContains(string(configBytes), "[oauth]", "config.toml should not contain [oauth] section when no secrets provided") } diff --git a/internal/oauth/embedded.go b/internal/oauth/embedded.go index d1d76ca72..5707c4c46 100644 --- a/internal/oauth/embedded.go +++ b/internal/oauth/embedded.go @@ -1,7 +1,7 @@ package oauth import ( - "fmt" + "errors" "log/slog" "golang.org/x/oauth2" @@ -18,6 +18,8 @@ import ( // client secret is "obviously not treated as a secret"; PKCE provides the // flow security. The values below are the dev project's credentials, // suitable for contributor builds. Production binaries override both. +// #nosec G101 -- Google desktop OAuth client credentials are public client +// identifiers; PKCE secures the flow and release builds override these values. var ( oauthClientID = "913114107126-tfruv1983bsv811mbjkqjvtd23io5b93.apps.googleusercontent.com" oauthClientSecret = "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" @@ -49,7 +51,7 @@ func EmbeddedConfig(scopes []string) *oauth2.Config { // locally). func NewEmbeddedManager(tokensDir string, logger *slog.Logger, scopes []string) (*Manager, error) { if !HasEmbeddedCredentials() { - return nil, fmt.Errorf("no embedded OAuth credentials in this build") + return nil, errors.New("no embedded OAuth credentials in this build") } if logger == nil { logger = slog.Default() diff --git a/internal/oauth/embedded_test.go b/internal/oauth/embedded_test.go index 93c12db1c..2d6d968b5 100644 --- a/internal/oauth/embedded_test.go +++ b/internal/oauth/embedded_test.go @@ -4,6 +4,8 @@ import ( "log/slog" "testing" + assertpkg "github.com/stretchr/testify/assert" + requirepkg "github.com/stretchr/testify/require" "golang.org/x/oauth2/google" ) @@ -30,14 +32,13 @@ func TestHasEmbeddedCredentials(t *testing.T) { t.Run(tc.name, func(t *testing.T) { oauthClientID = tc.id oauthClientSecret = tc.secret - if got := HasEmbeddedCredentials(); got != tc.want { - t.Errorf("HasEmbeddedCredentials() = %v, want %v", got, tc.want) - } + assertpkg.Equal(t, tc.want, HasEmbeddedCredentials(), "HasEmbeddedCredentials()") }) } } func TestEmbeddedConfig(t *testing.T) { + assert := assertpkg.New(t) origID, origSecret := oauthClientID, oauthClientSecret defer func() { oauthClientID = origID @@ -49,21 +50,15 @@ func TestEmbeddedConfig(t *testing.T) { scopes := []string{"scope-a", "scope-b"} cfg := EmbeddedConfig(scopes) - if cfg.ClientID != "test-client-id" { - t.Errorf("ClientID = %q, want %q", cfg.ClientID, "test-client-id") - } - if cfg.ClientSecret != "test-client-secret" { - t.Errorf("ClientSecret = %q, want %q", cfg.ClientSecret, "test-client-secret") - } - if len(cfg.Scopes) != 2 || cfg.Scopes[0] != "scope-a" || cfg.Scopes[1] != "scope-b" { - t.Errorf("Scopes = %v, want %v", cfg.Scopes, scopes) - } - if cfg.Endpoint != google.Endpoint { - t.Errorf("Endpoint = %v, want google.Endpoint", cfg.Endpoint) - } + assert.Equal("test-client-id", cfg.ClientID, "ClientID") + assert.Equal("test-client-secret", cfg.ClientSecret, "ClientSecret") + assert.Equal(scopes, cfg.Scopes, "Scopes") + assert.Equal(google.Endpoint, cfg.Endpoint, "Endpoint") } func TestNewEmbeddedManager(t *testing.T) { + require := requirepkg.New(t) + assert := assertpkg.New(t) origID, origSecret := oauthClientID, oauthClientSecret defer func() { oauthClientID = origID @@ -74,18 +69,10 @@ func TestNewEmbeddedManager(t *testing.T) { tokensDir := t.TempDir() mgr, err := NewEmbeddedManager(tokensDir, slog.Default(), ScopesEmbedded) - if err != nil { - t.Fatalf("NewEmbeddedManager: %v", err) - } - if mgr == nil { - t.Fatal("NewEmbeddedManager returned nil manager") - } - if mgr.tokensDir != tokensDir { - t.Errorf("tokensDir = %q, want %q", mgr.tokensDir, tokensDir) - } - if !mgr.isEmbedded { - t.Error("isEmbedded = false, want true") - } + require.NoError(err, "NewEmbeddedManager") + require.NotNil(mgr, "NewEmbeddedManager") + assert.Equal(tokensDir, mgr.tokensDir, "tokensDir") + assert.True(mgr.isEmbedded, "isEmbedded") } func TestNewEmbeddedManagerWithoutCredentials(t *testing.T) { @@ -98,7 +85,5 @@ func TestNewEmbeddedManagerWithoutCredentials(t *testing.T) { oauthClientSecret = "" _, err := NewEmbeddedManager(t.TempDir(), slog.Default(), ScopesEmbedded) - if err == nil { - t.Fatal("NewEmbeddedManager: want error when credentials are empty, got nil") - } + requirepkg.Error(t, err, "NewEmbeddedManager should fail when credentials are empty") } diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index dea4aa02b..62b356f26 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "html" "io" "log/slog" "net/http" @@ -265,7 +266,7 @@ func (m *Manager) newCallbackHandler(expectedState string, codeChan chan<- strin return } errChan <- fmt.Errorf("oauth callback error: %s", errParam) - _, _ = fmt.Fprintf(w, "Error: %s", errParam) + _, _ = fmt.Fprintf(w, "Error: %s", html.EscapeString(errParam)) return } if r.URL.Query().Get("state") != expectedState { diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index 304b0231e..003ded831 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -538,17 +538,15 @@ func TestCallbackHandlerAccessDenied(t *testing.T) { errChan := make(chan error, 1) handler := mgr.newCallbackHandler("expected-state", codeChan, errChan) - req := httptest.NewRequest("GET", "/callback?error=access_denied&state=expected-state", nil) + req := httptest.NewRequest(http.MethodGet, "/callback?error=access_denied&state=expected-state", nil) rec := httptest.NewRecorder() handler(rec, req) select { case err := <-errChan: - if !errors.Is(err, errAccessDenied) { - t.Errorf("callback error = %v, want errAccessDenied", err) - } + requirepkg.ErrorIs(t, err, errAccessDenied) default: - t.Fatal("callback handler did not send an error") + requirepkg.Fail(t, "callback handler did not send an error") } } @@ -809,22 +807,20 @@ func TestValidateBrowserURL(t *testing.T) { } func TestScopesEmbedded(t *testing.T) { + require := requirepkg.New(t) + assert := assertpkg.New(t) want := []string{ "https://www.googleapis.com/auth/gmail.readonly", "https://www.googleapis.com/auth/gmail.modify", "https://mail.google.com/", } - if len(ScopesEmbedded) != len(want) { - t.Fatalf("ScopesEmbedded has %d entries, want %d", len(ScopesEmbedded), len(want)) - } - for i, scope := range want { - if ScopesEmbedded[i] != scope { - t.Errorf("ScopesEmbedded[%d] = %q, want %q", i, ScopesEmbedded[i], scope) - } - } + require.Len(ScopesEmbedded, len(want), "ScopesEmbedded length") + assert.Equal(want, ScopesEmbedded, "ScopesEmbedded") } func TestAuthorizeEmbeddedFallbackMessage(t *testing.T) { + require := requirepkg.New(t) + assert := assertpkg.New(t) tokensDir := t.TempDir() mgr := &Manager{ config: &oauth2.Config{ClientID: "x", ClientSecret: "y", Scopes: []string{"s"}}, @@ -842,15 +838,13 @@ func TestAuthorizeEmbeddedFallbackMessage(t *testing.T) { defer func() { stdout = origStdout }() err := mgr.Authorize(context.Background(), "u@example.com") - if !errors.Is(err, errAccessDenied) { - t.Fatalf("Authorize error = %v, want errAccessDenied", err) - } - if !strings.Contains(buf.String(), "still in Google's verification") { - t.Errorf("expected fallback message, got: %q", buf.String()) - } + require.ErrorIs(err, errAccessDenied, "Authorize") + assert.Contains(buf.String(), "still in Google's verification") } func TestAuthorizeNonEmbeddedNoFallback(t *testing.T) { + require := requirepkg.New(t) + assert := assertpkg.New(t) mgr := &Manager{ config: &oauth2.Config{ClientID: "x", ClientSecret: "y", Scopes: []string{"s"}}, tokensDir: t.TempDir(), @@ -867,10 +861,6 @@ func TestAuthorizeNonEmbeddedNoFallback(t *testing.T) { defer func() { stdout = origStdout }() err := mgr.Authorize(context.Background(), "u@example.com") - if !errors.Is(err, errAccessDenied) { - t.Fatalf("Authorize error = %v, want errAccessDenied", err) - } - if strings.Contains(buf.String(), "still in Google's verification") { - t.Errorf("did not expect fallback message for non-embedded, got: %q", buf.String()) - } + require.ErrorIs(err, errAccessDenied, "Authorize") + assert.NotContains(buf.String(), "still in Google's verification") } From 6bf68482950871487a978e6ac919070d86f06fc4 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 18 Jun 2026 09:00:09 -0400 Subject: [PATCH 09/10] test: cover OAuth callback error escaping The callback handler now escapes provider-supplied error text before writing it to the browser, but the previous test only verified the error channel. Add a regression test that drives the generic callback error path with HTML metacharacters and checks the response body preserves only the escaped form. --- internal/oauth/oauth_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/internal/oauth/oauth_test.go b/internal/oauth/oauth_test.go index 003ded831..0dc025216 100644 --- a/internal/oauth/oauth_test.go +++ b/internal/oauth/oauth_test.go @@ -550,6 +550,31 @@ func TestCallbackHandlerAccessDenied(t *testing.T) { } } +func TestCallbackHandlerEscapesErrorResponse(t *testing.T) { + require := requirepkg.New(t) + assert := assertpkg.New(t) + mgr := &Manager{logger: slog.Default()} + codeChan := make(chan string, 1) + errChan := make(chan error, 1) + handler := mgr.newCallbackHandler("expected-state", codeChan, errChan) + + const rawError = "" + req := httptest.NewRequest(http.MethodGet, "/callback?error=%3Cscript%3Ealert(1)%3C%2Fscript%3E&state=expected-state", nil) + rec := httptest.NewRecorder() + handler(rec, req) + + select { + case err := <-errChan: + require.ErrorContains(err, rawError) + default: + require.Fail("callback handler did not send an error") + } + + body := rec.Body.String() + assert.Contains(body, "Error: <script>alert(1)</script>") + assert.NotContains(body, rawError) +} + // TestAuthorize_SavesUnderOriginalIdentifier exercises the real // authorize() method end-to-end (with injected browserFlow and // profile server) to verify the token is saved under the original From 64f8b474a431c00e7a1706c729e2de79475a8912 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Thu, 18 Jun 2026 09:14:34 -0400 Subject: [PATCH 10/10] fix: keep embedded OAuth clients consistent roborev-ci found that NAS bundles could rely on a published Docker image built with the source/default OAuth client while desktop releases used the production client. Docker publish now requires the same OAuth secrets as release builds and passes them through the existing ldflag path; local and PR image builds leave those values empty instead of baking the development client into containers. The default no-config add-account path now treats the embedded manager like an app switch for token reuse, so a stored token minted by a different client forces reauthorization instead of being reported as already authorized. --- .github/workflows/docker.yml | 14 +++++++ Dockerfile | 9 ++++- cmd/msgvault/cmd/addaccount.go | 6 +-- cmd/msgvault/cmd/addaccount_test.go | 58 +++++++++++++++++++++++++++++ internal/oauth/oauth.go | 6 +++ 5 files changed, 89 insertions(+), 4 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 173f8c52d..3dcd4ada4 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -101,6 +101,7 @@ jobs: - name: Prepare build args id: build_args run: | + set -euo pipefail if [[ "$GITHUB_REF" == refs/tags/v* ]]; then VERSION="${GITHUB_REF#refs/tags/}" else @@ -112,6 +113,17 @@ jobs: echo "build_date=$(date -u +%Y-%m-%dT%H:%M:%SZ)" } >> "$GITHUB_OUTPUT" + - name: Validate OAuth build args + env: + MSGVAULT_OAUTH_CLIENT_ID: ${{ secrets.MSGVAULT_OAUTH_CLIENT_ID }} + MSGVAULT_OAUTH_CLIENT_SECRET: ${{ secrets.MSGVAULT_OAUTH_CLIENT_SECRET }} + run: | + set -euo pipefail + if [[ -z "$MSGVAULT_OAUTH_CLIENT_ID" || -z "$MSGVAULT_OAUTH_CLIENT_SECRET" ]]; then + echo "MSGVAULT OAuth secrets are required for Docker publish" >&2 + exit 1 + fi + - name: Build and push uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7.2.0 with: @@ -124,5 +136,7 @@ jobs: VERSION=${{ steps.build_args.outputs.version }} COMMIT=${{ steps.build_args.outputs.commit }} BUILD_DATE=${{ steps.build_args.outputs.build_date }} + MSGVAULT_OAUTH_CLIENT_ID=${{ secrets.MSGVAULT_OAUTH_CLIENT_ID }} + MSGVAULT_OAUTH_CLIENT_SECRET=${{ secrets.MSGVAULT_OAUTH_CLIENT_SECRET }} cache-from: type=gha cache-to: type=gha,mode=max diff --git a/Dockerfile b/Dockerfile index 48d55547d..afac3a805 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,15 +22,22 @@ COPY . . ARG VERSION=dev ARG COMMIT=unknown ARG BUILD_DATE=unknown +ARG MSGVAULT_OAUTH_CLIENT_ID= +ARG MSGVAULT_OAUTH_CLIENT_SECRET= # Note: Module path must match go.mod (go.kenn.io/msgvault) +# Docker builds receive production OAuth values from the publish workflow. +# Local/PR Docker builds default these to empty so they do not accidentally +# embed the source development client in container images. RUN CGO_ENABLED=1 go build \ -tags fts5 \ -trimpath \ -ldflags="-s -w \ -X go.kenn.io/msgvault/cmd/msgvault/cmd.Version=${VERSION} \ -X go.kenn.io/msgvault/cmd/msgvault/cmd.Commit=${COMMIT} \ - -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=${BUILD_DATE}" \ + -X go.kenn.io/msgvault/cmd/msgvault/cmd.BuildDate=${BUILD_DATE} \ + -X go.kenn.io/msgvault/internal/oauth.oauthClientID=${MSGVAULT_OAUTH_CLIENT_ID} \ + -X go.kenn.io/msgvault/internal/oauth.oauthClientSecret=${MSGVAULT_OAUTH_CLIENT_SECRET}" \ -o /msgvault \ ./cmd/msgvault diff --git a/cmd/msgvault/cmd/addaccount.go b/cmd/msgvault/cmd/addaccount.go index 8099e36c3..a41074930 100644 --- a/cmd/msgvault/cmd/addaccount.go +++ b/cmd/msgvault/cmd/addaccount.go @@ -195,10 +195,10 @@ Examples: // If a valid token exists, check if we can reuse it. // Validate the token's client identity when any named app is // involved — whether from an explicit flag, a binding change, - // or inherited from the DB. A mismatched token would fail on - // next refresh. + // inherited from the DB — or when falling back to the embedded + // client. A mismatched token would fail on next refresh. needsClientCheck := bindingChanged || oauthAppExplicit || - resolvedApp != "" + resolvedApp != "" || oauthMgr.UsesEmbeddedClient() tokenReusable := !forceReauth && oauthMgr.HasToken(email) && (!needsClientCheck || oauthMgr.TokenMatchesClient(email)) if tokenReusable { diff --git a/cmd/msgvault/cmd/addaccount_test.go b/cmd/msgvault/cmd/addaccount_test.go index b16eedde9..23a12ce6f 100644 --- a/cmd/msgvault/cmd/addaccount_test.go +++ b/cmd/msgvault/cmd/addaccount_test.go @@ -370,6 +370,64 @@ func TestAddAccount_ExplicitDefaultRejectsMismatchedToken(t *testing.T) { require.Error(err, "mismatched token should be rejected with explicit --oauth-app \"\"") } +// TestAddAccount_EmbeddedDefaultRejectsMismatchedToken verifies that the +// no-config embedded fallback does not silently reuse a token minted by a +// different OAuth client. +func TestAddAccount_EmbeddedDefaultRejectsMismatchedToken(t *testing.T) { + require := requirepkg.New(t) + tmpDir := t.TempDir() + + tokensDir := filepath.Join(tmpDir, "tokens") + require.NoError(os.MkdirAll(tokensDir, 0700), "mkdir tokens") + tokenData, err := json.Marshal(map[string]string{ + "access_token": "fake-access", + "refresh_token": "fake-refresh", + "token_type": "Bearer", + "client_id": "wrong-client.apps.googleusercontent.com", + }) + require.NoError(err, "marshal token") + require.NoError(os.WriteFile( + filepath.Join(tokensDir, "user@example.com.json"), + tokenData, 0600, + ), "write token") + + savedCfg := cfg + savedLogger := logger + savedOAuthApp := oauthAppName + defer func() { + cfg = savedCfg + logger = savedLogger + oauthAppName = savedOAuthApp + }() + + cfg = &config.Config{ + HomeDir: tmpDir, + Data: config.DataConfig{DataDir: tmpDir}, + } + logger = slog.New(slog.NewTextHandler(os.Stderr, nil)) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + testCmd := &cobra.Command{ + Use: "add-account ", + Args: cobra.ExactArgs(1), + RunE: addAccountCmd.RunE, + } + testCmd.Flags().StringVar(&oauthAppName, "oauth-app", "", "") + testCmd.Flags().BoolVar(&headless, "headless", false, "") + testCmd.Flags().BoolVar(&forceReauth, "force", false, "") + testCmd.Flags().StringVar(&accountDisplayName, "display-name", "", "") + testCmd.Flags().BoolVar(&noDefaultIdentityAddAccount, "no-default-identity", false, "") + + root := newTestRootCmd() + root.AddCommand(testCmd) + root.SetArgs([]string{"add-account", "user@example.com"}) + + err = root.ExecuteContext(ctx) + require.Error(err, "embedded client should reject a token minted by another OAuth client") +} + // TestAddAccount_ExplicitDefaultAcceptsMatchingToken verifies that // --oauth-app "" accepts a token minted by the default client. func TestAddAccount_ExplicitDefaultAcceptsMatchingToken(t *testing.T) { diff --git a/internal/oauth/oauth.go b/internal/oauth/oauth.go index 62b356f26..a7f0256d7 100644 --- a/internal/oauth/oauth.go +++ b/internal/oauth/oauth.go @@ -487,6 +487,12 @@ func (m *Manager) TokenMatchesClient(email string) bool { return tf.ClientID == m.config.ClientID } +// UsesEmbeddedClient reports whether this manager uses msgvault's embedded +// OAuth client rather than user-provided client secrets. +func (m *Manager) UsesEmbeddedClient() bool { + return m.isEmbedded +} + // HasScopeMetadata returns true if the token file for this account has any // scope metadata stored. Legacy tokens (saved before scope tracking) return false. func (m *Manager) HasScopeMetadata(email string) bool {