From a61cea4eea56677b9a56d55e34ceddfeba5163f2 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:09:23 -0400 Subject: [PATCH 01/15] feat(auth): add internal/profile package for account resolution New package resolves the active notion-cli profile from --profile flag, NOTION_CLI_PROFILE env, settings.json default_profile, and legacy top-level credentials, and exposes helpers for per-profile token/config paths. Includes a resolver that returns ErrNoProfile when no profile is selected and no legacy top-level files exist, so callers can fail up front instead of silently running unauthenticated. Profile names must match ^[a-z0-9][a-z0-9_-]*\$ so they stay safe to embed in filesystem paths. --- internal/cli/profile.go | 21 ++++ internal/profile/profile.go | 202 ++++++++++++++++++++++++++++++ internal/profile/profile_test.go | 207 +++++++++++++++++++++++++++++++ 3 files changed, 430 insertions(+) create mode 100644 internal/cli/profile.go create mode 100644 internal/profile/profile.go create mode 100644 internal/profile/profile_test.go diff --git a/internal/cli/profile.go b/internal/cli/profile.go new file mode 100644 index 0000000..3ebe949 --- /dev/null +++ b/internal/cli/profile.go @@ -0,0 +1,21 @@ +package cli + +import ( + "github.com/lox/notion-cli/internal/profile" +) + +// active holds the profile resolved by main.go for the current invocation. +// Helpers in this package that create token stores or load config read +// from it so commands don't need to thread the profile through every call. +var active = profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault} + +// SetActiveProfile records the profile selected for this invocation. +func SetActiveProfile(p profile.Profile) { + active = p +} + +// ActiveProfile returns the profile selected for this invocation, or the +// implicit default profile if none was set. +func ActiveProfile() profile.Profile { + return active +} diff --git a/internal/profile/profile.go b/internal/profile/profile.go new file mode 100644 index 0000000..9791e04 --- /dev/null +++ b/internal/profile/profile.go @@ -0,0 +1,202 @@ +// Package profile resolves which Notion account a notion-cli invocation +// targets and returns the on-disk locations for that account's credentials. +package profile + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" +) + +const ( + // EnvVar is the environment variable that selects a profile when no + // --profile flag is passed. + EnvVar = "NOTION_CLI_PROFILE" + // DefaultName is the implicit profile name used when the caller has + // not selected one and legacy top-level credentials exist. + DefaultName = "default" + // SettingsFileName is the cross-profile settings file at the top of + // the notion-cli config dir. + SettingsFileName = "settings.json" + // TokenFileName is the OAuth token filename inside a profile dir. + TokenFileName = "token.json" + // ConfigFileName is the API config filename inside a profile dir. + ConfigFileName = "config.json" + configDirName = "notion-cli" +) + +// Source records where the active profile came from, so auth status can +// tell the user how it resolved. +type Source string + +const ( + SourceFlag Source = "--profile flag" + SourceEnv Source = EnvVar + SourceSettings Source = SettingsFileName + SourceDefault Source = "implicit default" +) + +// Profile identifies a selected notion-cli profile and where it was +// selected from. +type Profile struct { + Name string + Source Source +} + +// ErrNoProfile is returned by Resolve when no profile is selected by flag, +// env, or settings, and no legacy top-level credentials exist to fall back +// to. +var ErrNoProfile = errors.New("no profile specified; pass --profile or set " + EnvVar) + +// nameRE enforces the accepted profile-name alphabet: lowercase ASCII, +// leading letter or digit, `_` and `-` allowed in the tail. +var nameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]*$`) + +// Validate reports whether name is acceptable as a profile identifier. +func Validate(name string) error { + if name == "" { + return errors.New("profile name must not be empty") + } + if !nameRE.MatchString(name) { + return fmt.Errorf("invalid profile name %q: must match %s", name, nameRE.String()) + } + return nil +} + +// Resolve picks the active profile using the precedence: +// +// 1. --profile flag value +// 2. NOTION_CLI_PROFILE environment variable +// 3. default_profile in ~/.config/notion-cli/settings.json +// 4. legacy implicit default (top-level ~/.config/notion-cli/{token,config}.json) +// +// If none of those apply, Resolve returns ErrNoProfile. +func Resolve(flagValue string) (Profile, error) { + if v := strings.TrimSpace(flagValue); v != "" { + if err := Validate(v); err != nil { + return Profile{}, err + } + return Profile{Name: v, Source: SourceFlag}, nil + } + if v := strings.TrimSpace(os.Getenv(EnvVar)); v != "" { + if err := Validate(v); err != nil { + return Profile{}, fmt.Errorf("%s: %w", EnvVar, err) + } + return Profile{Name: v, Source: SourceEnv}, nil + } + if v, err := loadSettingsDefault(); err != nil { + return Profile{}, err + } else if v != "" { + if err := Validate(v); err != nil { + return Profile{}, fmt.Errorf("settings.json default_profile: %w", err) + } + return Profile{Name: v, Source: SourceSettings}, nil + } + if legacyTopLevelExists() { + return Profile{Name: DefaultName, Source: SourceDefault}, nil + } + return Profile{}, ErrNoProfile +} + +// TokenRoot returns the top-level directory that contains OAuth token files. +// notion-cli historically stores tokens under $HOME/.config/notion-cli on +// every OS, so we keep that path here to avoid orphaning existing tokens on +// macOS where UserConfigDir would otherwise resolve to +// ~/Library/Application Support. +func TokenRoot() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(home, ".config", configDirName), nil +} + +// ConfigRoot returns the top-level directory that contains API config and +// cross-profile settings files. This follows os.UserConfigDir, matching the +// existing config.json layout. +func ConfigRoot() (string, error) { + base, err := os.UserConfigDir() + if err != nil { + return "", err + } + return filepath.Join(base, configDirName), nil +} + +// TokenPath returns the OAuth token file path for the given profile. The +// implicit default profile keeps using the top-level token.json. +func TokenPath(p Profile) (string, error) { + root, err := TokenRoot() + if err != nil { + return "", err + } + if p.Name == DefaultName { + return filepath.Join(root, TokenFileName), nil + } + return filepath.Join(root, p.Name, TokenFileName), nil +} + +// ConfigPath returns the API config file path for the given profile. The +// implicit default profile keeps using the top-level config.json. +func ConfigPath(p Profile) (string, error) { + root, err := ConfigRoot() + if err != nil { + return "", err + } + if p.Name == DefaultName { + return filepath.Join(root, ConfigFileName), nil + } + return filepath.Join(root, p.Name, ConfigFileName), nil +} + +// SettingsPath returns the cross-profile settings.json path. It lives next +// to the API config files. +func SettingsPath() (string, error) { + root, err := ConfigRoot() + if err != nil { + return "", err + } + return filepath.Join(root, SettingsFileName), nil +} + +type settingsFile struct { + DefaultProfile string `json:"default_profile,omitempty"` +} + +func loadSettingsDefault() (string, error) { + path, err := SettingsPath() + if err != nil { + return "", err + } + data, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return "", nil + } + return "", fmt.Errorf("read %s: %w", path, err) + } + var s settingsFile + if err := json.Unmarshal(data, &s); err != nil { + return "", fmt.Errorf("parse %s: %w", path, err) + } + return strings.TrimSpace(s.DefaultProfile), nil +} + +func legacyTopLevelExists() bool { + tokenRoot, err := TokenRoot() + if err == nil { + if _, err := os.Stat(filepath.Join(tokenRoot, TokenFileName)); err == nil { + return true + } + } + configRoot, err := ConfigRoot() + if err == nil { + if _, err := os.Stat(filepath.Join(configRoot, ConfigFileName)); err == nil { + return true + } + } + return false +} diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go new file mode 100644 index 0000000..a4553dc --- /dev/null +++ b/internal/profile/profile_test.go @@ -0,0 +1,207 @@ +package profile + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "testing" +) + +// isolateProfileDirs redirects HOME and UserConfigDir to a temp dir so the +// resolver only sees state we set up for the test. +func isolateProfileDirs(t *testing.T) string { + t.Helper() + tmp := t.TempDir() + t.Setenv("HOME", tmp) + // os.UserConfigDir on Linux reads XDG_CONFIG_HOME, on macOS it reads + // nothing and composes from HOME. Set both so the lookup is deterministic. + t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmp, ".config")) + return tmp +} + +func TestValidateRejectsEmpty(t *testing.T) { + if err := Validate(""); err == nil { + t.Fatalf("Validate(\"\") returned nil, expected error") + } +} + +func TestValidateRejectsBadCharacters(t *testing.T) { + bad := []string{ + "Work", // uppercase + "work/sub", // slash + "-leading-dash", // leading dash + "_leading-under", // leading underscore + "has spaces", // whitespace + "has.dot", // dot + "has!bang", // punctuation + "../escape", // path traversal + "work/../other", // path traversal + "", // empty + } + for _, name := range bad { + if err := Validate(name); err == nil { + t.Errorf("Validate(%q) returned nil, expected error", name) + } + } +} + +func TestValidateAcceptsCommonNames(t *testing.T) { + good := []string{"work", "home", "default", "brianle", "proj1", "proj_1", "a-b-c", "0xble"} + for _, name := range good { + if err := Validate(name); err != nil { + t.Errorf("Validate(%q) returned %v, expected nil", name, err) + } + } +} + +func TestResolvePrefersFlagOverEnvAndSettings(t *testing.T) { + isolateProfileDirs(t) + t.Setenv(EnvVar, "env-profile") + writeSettings(t, "settings-profile") + + p, err := Resolve("flag-profile") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if p.Name != "flag-profile" || p.Source != SourceFlag { + t.Fatalf("Resolve = %+v, want flag-profile/SourceFlag", p) + } +} + +func TestResolvePrefersEnvOverSettings(t *testing.T) { + isolateProfileDirs(t) + t.Setenv(EnvVar, "env-profile") + writeSettings(t, "settings-profile") + + p, err := Resolve("") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if p.Name != "env-profile" || p.Source != SourceEnv { + t.Fatalf("Resolve = %+v, want env-profile/SourceEnv", p) + } +} + +func TestResolveUsesSettingsWhenFlagAndEnvUnset(t *testing.T) { + isolateProfileDirs(t) + t.Setenv(EnvVar, "") + writeSettings(t, "settings-profile") + + p, err := Resolve("") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if p.Name != "settings-profile" || p.Source != SourceSettings { + t.Fatalf("Resolve = %+v, want settings-profile/SourceSettings", p) + } +} + +func TestResolveFallsBackToImplicitDefaultWhenLegacyFilesExist(t *testing.T) { + tmp := isolateProfileDirs(t) + t.Setenv(EnvVar, "") + + tokenRoot := filepath.Join(tmp, ".config", configDirName) + if err := os.MkdirAll(tokenRoot, 0o700); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Join(tokenRoot, TokenFileName), []byte(`{"access_token":"legacy"}`), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + p, err := Resolve("") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if p.Name != DefaultName || p.Source != SourceDefault { + t.Fatalf("Resolve = %+v, want default/SourceDefault", p) + } +} + +func TestResolveReturnsErrNoProfileWhenNothingResolves(t *testing.T) { + isolateProfileDirs(t) + t.Setenv(EnvVar, "") + + _, err := Resolve("") + if !errors.Is(err, ErrNoProfile) { + t.Fatalf("Resolve = %v, want ErrNoProfile", err) + } +} + +func TestResolveValidatesFlagValue(t *testing.T) { + isolateProfileDirs(t) + t.Setenv(EnvVar, "") + + if _, err := Resolve("BadName"); err == nil { + t.Fatalf("Resolve(\"BadName\") returned nil, expected validation error") + } +} + +func TestResolveValidatesEnvValue(t *testing.T) { + isolateProfileDirs(t) + t.Setenv(EnvVar, "BadName") + + if _, err := Resolve(""); err == nil { + t.Fatalf("Resolve with bad env returned nil, expected validation error") + } +} + +func TestTokenPathDefaultPointsToLegacyTopLevel(t *testing.T) { + tmp := isolateProfileDirs(t) + + path, err := TokenPath(Profile{Name: DefaultName, Source: SourceDefault}) + if err != nil { + t.Fatalf("TokenPath: %v", err) + } + want := filepath.Join(tmp, ".config", configDirName, TokenFileName) + if path != want { + t.Fatalf("TokenPath = %q, want %q", path, want) + } +} + +func TestTokenPathNamedProfileUsesSubdir(t *testing.T) { + tmp := isolateProfileDirs(t) + + path, err := TokenPath(Profile{Name: "work", Source: SourceFlag}) + if err != nil { + t.Fatalf("TokenPath: %v", err) + } + want := filepath.Join(tmp, ".config", configDirName, "work", TokenFileName) + if path != want { + t.Fatalf("TokenPath = %q, want %q", path, want) + } +} + +func TestConfigPathNamedProfileIsolatedFromDefault(t *testing.T) { + isolateProfileDirs(t) + + defaultPath, err := ConfigPath(Profile{Name: DefaultName}) + if err != nil { + t.Fatalf("ConfigPath default: %v", err) + } + workPath, err := ConfigPath(Profile{Name: "work"}) + if err != nil { + t.Fatalf("ConfigPath work: %v", err) + } + if defaultPath == workPath { + t.Fatalf("default and named profile share config path: %q", defaultPath) + } +} + +func writeSettings(t *testing.T, defaultProfile string) { + t.Helper() + path, err := SettingsPath() + if err != nil { + t.Fatalf("SettingsPath: %v", err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + data, err := json.Marshal(settingsFile{DefaultProfile: defaultProfile}) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } +} From adab300d5f6c72a96f5d3ccca6c16dd0b86e507b Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:09:33 -0400 Subject: [PATCH 02/15] feat(auth): namespace token store and API config by profile Accept a profile in NewFileTokenStore and config.Load/Save so named profiles store credentials under ~/.config/notion-cli// while the implicit default profile keeps using the legacy top-level paths. Existing single-account installs read and write the same files as before. Adds round-trip tests that two profiles can hold their own tokens and configs without colliding. --- internal/config/config.go | 57 +++++++++++++++++++++++++--------- internal/config/config_test.go | 38 +++++++++++++++++++++++ internal/mcp/client_test.go | 26 ++++++++++++++++ internal/mcp/token_store.go | 18 ++++++----- 4 files changed, 117 insertions(+), 22 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 1c7c6b1..323d937 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -7,10 +7,11 @@ import ( "os" "path/filepath" "strings" + + "github.com/lox/notion-cli/internal/profile" ) const ( - configDirName = "notion-cli" configFileName = "config.json" defaultAPIBaseURL = "https://api.notion.com/v1" defaultNotionAPIVer = "2026-03-11" @@ -54,12 +55,20 @@ func Default() Config { } } +// defaultProfile is the implicit default profile used when a caller does +// not pass one. Matches the legacy single-profile layout. +func defaultProfile() profile.Profile { + return profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault} +} + +// Path returns the API config file path for the default profile. func Path() (string, error) { - configDir, err := os.UserConfigDir() - if err != nil { - return "", err - } - return filepath.Join(configDir, configDirName, configFileName), nil + return PathFor(defaultProfile()) +} + +// PathFor returns the API config file path for the given profile. +func PathFor(p profile.Profile) (string, error) { + return profile.ConfigPath(p) } func Load() (Config, error) { @@ -71,8 +80,14 @@ func Load() (Config, error) { } func LoadWithMeta(overrides APIOverrides) (LoadedConfig, error) { + return LoadWithMetaForProfile(defaultProfile(), overrides) +} + +// LoadWithMetaForProfile loads the API config file that belongs to the +// given profile and applies the provided overrides. +func LoadWithMetaForProfile(p profile.Profile, overrides APIOverrides) (LoadedConfig, error) { cfg := Default() - path, err := Path() + path, err := PathFor(p) if err != nil { return LoadedConfig{}, err } @@ -102,7 +117,13 @@ func LoadWithMeta(overrides APIOverrides) (LoadedConfig, error) { } func Save(cfg Config) error { - path, err := Path() + return SaveForProfile(defaultProfile(), cfg) +} + +// SaveForProfile writes the given config to the profile's config file +// atomically. +func SaveForProfile(p profile.Profile, cfg Config) error { + path, err := PathFor(p) if err != nil { return err } @@ -156,25 +177,33 @@ func Save(cfg Config) error { } func SetAPIToken(token string) error { - cfg, err := loadForMutation() + return SetAPITokenForProfile(defaultProfile(), token) +} + +func SetAPITokenForProfile(p profile.Profile, token string) error { + cfg, err := loadForMutation(p) if err != nil { return err } cfg.API.Token = strings.TrimSpace(token) - return Save(cfg) + return SaveForProfile(p, cfg) } func UnsetAPIToken() error { - cfg, err := loadForMutation() + return UnsetAPITokenForProfile(defaultProfile()) +} + +func UnsetAPITokenForProfile(p profile.Profile) error { + cfg, err := loadForMutation(p) if err != nil { return err } cfg.API.Token = "" - return Save(cfg) + return SaveForProfile(p, cfg) } -func loadForMutation() (Config, error) { - path, err := Path() +func loadForMutation(p profile.Profile) (Config, error) { + path, err := PathFor(p) if err != nil { return Config{}, err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f566b4b..8431bf8 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -4,6 +4,8 @@ import ( "os" "path/filepath" "testing" + + "github.com/lox/notion-cli/internal/profile" ) func TestLoadWithMetaDefaults(t *testing.T) { @@ -90,6 +92,42 @@ func TestUnsetAPITokenClearsStoredToken(t *testing.T) { } } +func TestSetAPITokenIsIsolatedPerProfile(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmp, ".config")) + + work := profile.Profile{Name: "work", Source: profile.SourceFlag} + home := profile.Profile{Name: "home", Source: profile.SourceFlag} + + if err := SetAPITokenForProfile(work, "work-token"); err != nil { + t.Fatalf("SetAPITokenForProfile(work): %v", err) + } + if err := SetAPITokenForProfile(home, "home-token"); err != nil { + t.Fatalf("SetAPITokenForProfile(home): %v", err) + } + + loadedWork, err := LoadWithMetaForProfile(work, APIOverrides{}) + if err != nil { + t.Fatalf("LoadWithMetaForProfile(work): %v", err) + } + if loadedWork.Config.API.Token != "work-token" { + t.Fatalf("work token = %q, want work-token", loadedWork.Config.API.Token) + } + + loadedHome, err := LoadWithMetaForProfile(home, APIOverrides{}) + if err != nil { + t.Fatalf("LoadWithMetaForProfile(home): %v", err) + } + if loadedHome.Config.API.Token != "home-token" { + t.Fatalf("home token = %q, want home-token", loadedHome.Config.API.Token) + } + + if loadedWork.Path == loadedHome.Path { + t.Fatalf("work and home configs share path: %q", loadedWork.Path) + } +} + func TestSaveSecuresConfigFile(t *testing.T) { t.Setenv("HOME", t.TempDir()) diff --git a/internal/mcp/client_test.go b/internal/mcp/client_test.go index da811d1..e25497d 100644 --- a/internal/mcp/client_test.go +++ b/internal/mcp/client_test.go @@ -1,10 +1,36 @@ package mcp import ( + "path/filepath" "reflect" "testing" + + "github.com/lox/notion-cli/internal/profile" ) +func TestNewFileTokenStoreForProfileIsolatesPaths(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + + defaultStore, err := NewFileTokenStoreForProfile(profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault}) + if err != nil { + t.Fatalf("NewFileTokenStoreForProfile(default): %v", err) + } + workStore, err := NewFileTokenStoreForProfile(profile.Profile{Name: "work", Source: profile.SourceFlag}) + if err != nil { + t.Fatalf("NewFileTokenStoreForProfile(work): %v", err) + } + + defaultWant := filepath.Join(tmp, ".config", "notion-cli", "token.json") + workWant := filepath.Join(tmp, ".config", "notion-cli", "work", "token.json") + if defaultStore.Path() != defaultWant { + t.Fatalf("default store path = %q, want %q", defaultStore.Path(), defaultWant) + } + if workStore.Path() != workWant { + t.Fatalf("work store path = %q, want %q", workStore.Path(), workWant) + } +} + func TestBuildSearchToolArgsOmitsBlankQuery(t *testing.T) { got := buildSearchToolArgs("", &SearchOptions{ContentSearchMode: "workspace_search"}) want := map[string]any{ diff --git a/internal/mcp/token_store.go b/internal/mcp/token_store.go index 9b86cac..00db4dc 100644 --- a/internal/mcp/token_store.go +++ b/internal/mcp/token_store.go @@ -9,14 +9,10 @@ import ( "sync" "time" + "github.com/lox/notion-cli/internal/profile" "github.com/mark3labs/mcp-go/client/transport" ) -const ( - configDir = ".config/notion-cli" - configFile = "token.json" -) - var ErrNoToken = errors.New("no token available") type FileTokenStore struct { @@ -24,13 +20,19 @@ type FileTokenStore struct { mu sync.RWMutex } +// NewFileTokenStore returns a token store backed by the default profile's +// token.json, matching the legacy behavior before multi-profile support. func NewFileTokenStore() (*FileTokenStore, error) { - homeDir, err := os.UserHomeDir() + return NewFileTokenStoreForProfile(profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault}) +} + +// NewFileTokenStoreForProfile returns a token store rooted at the given +// profile's on-disk location. +func NewFileTokenStoreForProfile(p profile.Profile) (*FileTokenStore, error) { + path, err := profile.TokenPath(p) if err != nil { return nil, err } - - path := filepath.Join(homeDir, configDir, configFile) return &FileTokenStore{path: path}, nil } From 35a3d0dc9cea57116163a8f98a5d5af43a315742 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:09:42 -0400 Subject: [PATCH 03/15] feat(auth): wire --profile flag and surface active profile in status Add a persistent --profile flag (and NOTION_CLI_PROFILE env) on the root command, resolve it once in main, and thread it through every token store and config load via internal/cli.ActiveProfile. auth status gains a Profile line that names the active profile and where the selection came from (flag, env, settings, or implicit default) so users can verify which account the CLI is hitting. When no profile is selected and no legacy top-level credentials exist, notion-cli now fails up front with "No profile specified. Pass --profile or set NOTION_CLI_PROFILE." instead of silently treating the caller as unauthenticated. --- cmd/auth.go | 30 ++++++++++++++++++------------ cmd/root.go | 7 ++++++- internal/cli/context.go | 2 +- internal/cli/official_api.go | 2 +- main.go | 20 ++++++++++++++++++-- 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/cmd/auth.go b/cmd/auth.go index 963f11f..872bc2f 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -37,7 +37,7 @@ const officialAPIIntegrationsURL = "https://www.notion.so/profile/integrations/i type AuthLoginCmd struct{} func (c *AuthLoginCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStore() + tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) if err != nil { output.PrintError(err) return err @@ -55,7 +55,7 @@ func (c *AuthLoginCmd) Run(ctx *Context) error { type AuthRefreshCmd struct{} func (c *AuthRefreshCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStore() + tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) if err != nil { output.PrintError(err) return err @@ -95,7 +95,7 @@ type AuthStatusCmd struct { func (c *AuthStatusCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - tokenStore, err := mcp.NewFileTokenStore() + tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) if err != nil { output.PrintError(err) return err @@ -112,16 +112,19 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { } hasValidToken := token.AccessToken != "" && !token.IsExpired() + activeProfile := cli.ActiveProfile() if ctx.JSON { enc := json.NewEncoder(os.Stdout) enc.SetIndent("", " ") return enc.Encode(map[string]any{ - "authenticated": hasValidToken, - "token_type": token.TokenType, - "has_token": token.AccessToken != "", - "expires_at": token.ExpiresAt, - "config_path": tokenStore.Path(), + "authenticated": hasValidToken, + "token_type": token.TokenType, + "has_token": token.AccessToken != "", + "expires_at": token.ExpiresAt, + "config_path": tokenStore.Path(), + "profile": activeProfile.Name, + "profile_source": string(activeProfile.Source), }) } @@ -134,6 +137,9 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { } fmt.Println() + _, _ = labelStyle.Print("Profile: ") + fmt.Printf("%s (from %s)\n", activeProfile.Name, activeProfile.Source) + _, _ = labelStyle.Print("Config path: ") fmt.Println(tokenStore.Path()) @@ -151,7 +157,7 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { type AuthLogoutCmd struct{} func (c *AuthLogoutCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStore() + tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) if err != nil { output.PrintError(err) return err @@ -190,7 +196,7 @@ func (c *AuthAPISetupCmd) Run(ctx *Context) error { output.PrintWarning("Official API token does not match the expected Notion token format") _, _ = fmt.Fprintln(authAPIOutput, "Expected format: ntn_") } - if err := config.SetAPIToken(token); err != nil { + if err := config.SetAPITokenForProfile(cli.ActiveProfile(), token); err != nil { output.PrintError(err) return err } @@ -285,7 +291,7 @@ func (c *AuthAPIUnsetCmd) Run(ctx *Context) error { return nil } - if err := config.UnsetAPIToken(); err != nil { + if err := config.UnsetAPITokenForProfile(cli.ActiveProfile()); err != nil { output.PrintError(err) return err } @@ -375,7 +381,7 @@ func printOfficialAPITokenSetupHint(out io.Writer, shouldOpenBrowser bool) { } func mustConfigPath() string { - path, err := config.Path() + path, err := config.PathFor(cli.ActiveProfile()) if err != nil { return "" } diff --git a/cmd/root.go b/cmd/root.go index f5c1db5..189c795 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,6 +1,9 @@ package cmd -import "github.com/lox/notion-cli/internal/config" +import ( + "github.com/lox/notion-cli/internal/config" + "github.com/lox/notion-cli/internal/profile" +) type Context struct { JSON bool @@ -8,6 +11,7 @@ type Context struct { APIToken string APIBaseURL string APINotionVersion string + Profile profile.Profile } type CLI struct { @@ -15,6 +19,7 @@ type CLI struct { APIToken string `env:"NOTION_API_TOKEN" hidden:""` APIBaseURL string `env:"NOTION_API_BASE_URL" hidden:""` APINotionVersion string `env:"NOTION_API_NOTION_VERSION" hidden:""` + Profile string `help:"Notion account profile to use (also reads $NOTION_CLI_PROFILE)" env:"NOTION_CLI_PROFILE" name:"profile"` Auth AuthCmd `cmd:"" help:"Authentication commands"` Page PageCmd `cmd:"" help:"Page commands"` diff --git a/internal/cli/context.go b/internal/cli/context.go index d69b867..d6ec058 100644 --- a/internal/cli/context.go +++ b/internal/cli/context.go @@ -51,7 +51,7 @@ func GetClient() (*mcp.Client, error) { } func autoRefreshIfNeeded(ctx context.Context) error { - tokenStore, err := mcp.NewFileTokenStore() + tokenStore, err := mcp.NewFileTokenStoreForProfile(ActiveProfile()) if err != nil { return err } diff --git a/internal/cli/official_api.go b/internal/cli/official_api.go index f159fef..1ce7b31 100644 --- a/internal/cli/official_api.go +++ b/internal/cli/official_api.go @@ -15,7 +15,7 @@ type OfficialAPIConfig struct { } func LoadOfficialAPIConfig(overrides config.APIOverrides) (*OfficialAPIConfig, error) { - loaded, err := config.LoadWithMeta(overrides) + loaded, err := config.LoadWithMetaForProfile(ActiveProfile(), overrides) if err != nil { return nil, fmt.Errorf("load config: %w", err) } diff --git a/main.go b/main.go index a7a7232..37e8625 100644 --- a/main.go +++ b/main.go @@ -1,11 +1,14 @@ package main import ( + "errors" + "fmt" "os" "github.com/alecthomas/kong" "github.com/lox/notion-cli/cmd" "github.com/lox/notion-cli/internal/cli" + "github.com/lox/notion-cli/internal/profile" ) var version = "dev" @@ -23,14 +26,27 @@ func main() { kong.UsageOnError(), kong.Vars{"version": version}, ) + + active, err := profile.Resolve(c.Profile) + if err != nil { + if errors.Is(err, profile.ErrNoProfile) { + _, _ = fmt.Fprintln(os.Stderr, "\u2717 No profile specified. Pass --profile or set NOTION_CLI_PROFILE.") + } else { + _, _ = fmt.Fprintf(os.Stderr, "\u2717 %s\n", err) + } + os.Exit(1) + } + cli.SetActiveProfile(active) cli.SetAccessToken(c.Token) - err := ctx.Run(&cmd.Context{ + + runErr := ctx.Run(&cmd.Context{ Token: c.Token, APIToken: c.APIToken, APIBaseURL: c.APIBaseURL, APINotionVersion: c.APINotionVersion, + Profile: active, }) - ctx.FatalIfErrorf(err) + ctx.FatalIfErrorf(runErr) os.Exit(0) } From 82e9575ef7185c2c415414f32aabbaf692be082a Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:09:49 -0400 Subject: [PATCH 04/15] docs: document --profile, settings.json, and profile layout Add a Profiles section to the README explaining --profile, NOTION_CLI_PROFILE, settings.json default_profile, resolution precedence, the no-profile error, name validation, and on-disk layout. Mirror the short form in the bundled notion skill and add NOTION_CLI_PROFILE to the environment variables table. --- README.md | 36 ++++++++++++++++++++++++++++++++++++ skills/notion/SKILL.md | 14 +++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d5eac52..982e5f4 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,41 @@ The CLI uses Notion's remote MCP server with OAuth authentication. On first run, **Note:** Access tokens expire after 1 hour. The CLI automatically refreshes tokens when they expire or are about to expire, so you typically don't need to think about this. Use `notion-cli auth refresh` to manually refresh if needed. +### Profiles + +Every command accepts `--profile ` (or `NOTION_CLI_PROFILE`) to scope the OAuth token and official API config to a specific Notion account, so you can keep separate logins for `work`, `home`, etc. + +```bash +# Log in to a named profile +notion-cli auth login --profile work + +# Use the profile for a single command +notion-cli page list --profile work + +# Pin a profile for the shell session +export NOTION_CLI_PROFILE=work + +# Pin a profile for every invocation (survives across shells) +cat > ~/.config/notion-cli/settings.json <<'JSON' +{"default_profile": "work"} +JSON +``` + +Profile resolution, highest priority first: + +1. `--profile ` flag +2. `NOTION_CLI_PROFILE` environment variable +3. `default_profile` in `~/.config/notion-cli/settings.json` +4. Implicit default (the pre-existing top-level `~/.config/notion-cli/{token,config}.json`) + +When none of those resolve (no top-level files, no settings, no flag, no env), every command fails up front with `No profile specified. Pass --profile or set NOTION_CLI_PROFILE.` rather than silently treating the caller as unauthenticated. If you want to force `--profile` on every invocation (for example to keep an agent from ever touching the wrong workspace), remove the top-level `{token,config}.json`. + +Profile names must match `^[a-z0-9][a-z0-9_-]*$`. + +Named profiles store their credentials under `~/.config/notion-cli//{token,config}.json`. The implicit default profile keeps using the existing top-level paths, so existing single-account installs need no migration. + +`notion-cli auth status` always prints the active profile and where it was resolved from, so you can verify which account the CLI is about to hit. + ## Environment Variables | Variable | Description | @@ -182,6 +217,7 @@ The CLI uses Notion's remote MCP server with OAuth authentication. On first run, | `NOTION_API_TOKEN` | Official Notion API token used for upload fallback and verification | | `NOTION_API_BASE_URL` | Override the official Notion API base URL | | `NOTION_API_NOTION_VERSION` | Override the official Notion API version | +| `NOTION_CLI_PROFILE` | Default profile when `--profile` is not passed | ## How It Works diff --git a/skills/notion/SKILL.md b/skills/notion/SKILL.md index 6502630..b16c074 100644 --- a/skills/notion/SKILL.md +++ b/skills/notion/SKILL.md @@ -30,7 +30,7 @@ The CLI uses OAuth authentication for MCP-backed commands. On first use, it open ```bash notion-cli auth login # Authenticate with Notion -notion-cli auth status # Check authentication status +notion-cli auth status # Check authentication status (also shows active profile) notion-cli auth refresh # Refresh token if status shows expired token notion-cli auth logout # Clear credentials ``` @@ -48,6 +48,18 @@ notion-cli auth api unset For CI/headless environments, set `NOTION_API_TOKEN`. +### Multiple accounts + +Every command accepts `--profile ` (or `NOTION_CLI_PROFILE`) to target a specific Notion account. Named profiles keep credentials isolated under `~/.config/notion-cli//`; the implicit default profile uses the existing top-level paths. + +```bash +notion-cli auth login --profile work +notion-cli page list --profile work +export NOTION_CLI_PROFILE=work # pin for the shell session +``` + +Resolution precedence: `--profile` > `NOTION_CLI_PROFILE` > `default_profile` in `~/.config/notion-cli/settings.json` > implicit top-level default. If none resolve, the CLI fails with `No profile specified.` instead of acting silently. + ## Available Commands ``` From 502fd17e22c6f07683e77537d8b0f2d49234b385 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:20:21 -0400 Subject: [PATCH 05/15] fix(auth): honor NOTION_ACCESS_TOKEN and route MCP client through profile Headless callers with only NOTION_ACCESS_TOKEN no longer trip the "No profile specified" gate; profile resolution is only fatal when there is no access token to authenticate with. Also route the MCP client token store through the active profile. Before this, every GetClient call constructed mcp.NewClient() without profile context, so it always opened the default profile's store; a run with --profile could refresh one profile and then authenticate with another, or fail as unauthenticated when only the named profile had credentials. --- internal/cli/context.go | 2 +- internal/mcp/client.go | 21 ++++++++++++++++++++- main.go | 26 +++++++++++++++++--------- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/internal/cli/context.go b/internal/cli/context.go index d6ec058..036243d 100644 --- a/internal/cli/context.go +++ b/internal/cli/context.go @@ -29,7 +29,7 @@ func GetClient() (*mcp.Client, error) { } } - var opts []mcp.ClientOption + opts := []mcp.ClientOption{mcp.WithProfile(ActiveProfile())} if accessToken != "" { opts = append(opts, mcp.WithAccessToken(accessToken)) } diff --git a/internal/mcp/client.go b/internal/mcp/client.go index a536b48..f4c5237 100644 --- a/internal/mcp/client.go +++ b/internal/mcp/client.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/lox/notion-cli/internal/profile" "github.com/mark3labs/mcp-go/client" "github.com/mark3labs/mcp-go/client/transport" "github.com/mark3labs/mcp-go/mcp" @@ -31,6 +32,8 @@ type ClientOption func(*clientConfig) type clientConfig struct { endpoint string accessToken string + profile profile.Profile + hasProfile bool } func WithEndpoint(endpoint string) ClientOption { @@ -45,6 +48,16 @@ func WithAccessToken(token string) ClientOption { } } +// WithProfile routes the OAuth token store to the given profile so named +// profiles read and refresh their own credentials instead of falling back +// to the default profile's top-level files. +func WithProfile(p profile.Profile) ClientOption { + return func(c *clientConfig) { + c.profile = p + c.hasProfile = true + } +} + func NewClient(opts ...ClientOption) (*Client, error) { cfg := &clientConfig{ endpoint: DefaultEndpoint, @@ -53,7 +66,13 @@ func NewClient(opts ...ClientOption) (*Client, error) { opt(cfg) } - tokenStore, err := NewFileTokenStore() + var tokenStore *FileTokenStore + var err error + if cfg.hasProfile { + tokenStore, err = NewFileTokenStoreForProfile(cfg.profile) + } else { + tokenStore, err = NewFileTokenStore() + } if err != nil { return nil, fmt.Errorf("create token store: %w", err) } diff --git a/main.go b/main.go index 37e8625..2fbe9c2 100644 --- a/main.go +++ b/main.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "strings" "github.com/alecthomas/kong" "github.com/lox/notion-cli/cmd" @@ -27,16 +28,23 @@ func main() { kong.Vars{"version": version}, ) - active, err := profile.Resolve(c.Profile) - if err != nil { - if errors.Is(err, profile.ErrNoProfile) { - _, _ = fmt.Fprintln(os.Stderr, "\u2717 No profile specified. Pass --profile or set NOTION_CLI_PROFILE.") - } else { - _, _ = fmt.Fprintf(os.Stderr, "\u2717 %s\n", err) + active, profileErr := profile.Resolve(c.Profile) + if profileErr != nil { + hasAccessToken := strings.TrimSpace(c.Token) != "" + if !hasAccessToken || !errors.Is(profileErr, profile.ErrNoProfile) { + if errors.Is(profileErr, profile.ErrNoProfile) { + _, _ = fmt.Fprintln(os.Stderr, "\u2717 No profile specified. Pass --profile or set NOTION_CLI_PROFILE.") + } else { + _, _ = fmt.Fprintf(os.Stderr, "\u2717 %s\n", profileErr) + } + os.Exit(1) } - os.Exit(1) + // Headless path: NOTION_ACCESS_TOKEN authenticates MCP directly, so + // the profile gate can be skipped. Profile-scoped operations (like + // auth api setup) will still use the implicit default layout. + } else { + cli.SetActiveProfile(active) } - cli.SetActiveProfile(active) cli.SetAccessToken(c.Token) runErr := ctx.Run(&cmd.Context{ @@ -44,7 +52,7 @@ func main() { APIToken: c.APIToken, APIBaseURL: c.APIBaseURL, APINotionVersion: c.APINotionVersion, - Profile: active, + Profile: cli.ActiveProfile(), }) ctx.FatalIfErrorf(runErr) os.Exit(0) From 70ef95cfcfac2e7e32203b5f575bc08df65b071e Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:50:14 -0400 Subject: [PATCH 06/15] fix(auth): preserve source attribution for env-selected profile Drop the env:"NOTION_CLI_PROFILE" tag from CLI.Profile. Kong was merging the flag and env values into one field, which meant profile.Resolve always saw a non-empty flagValue and classified env-selected profiles as SourceFlag; auth status would then report "from --profile flag" when the user had only set the environment variable. profile.Resolve already handles NOTION_CLI_PROFILE in its fallback chain, so main passes the bare flag value and the resolver records SourceEnv correctly. --- cmd/root.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index 189c795..f7d0e24 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -19,7 +19,12 @@ type CLI struct { APIToken string `env:"NOTION_API_TOKEN" hidden:""` APIBaseURL string `env:"NOTION_API_BASE_URL" hidden:""` APINotionVersion string `env:"NOTION_API_NOTION_VERSION" hidden:""` - Profile string `help:"Notion account profile to use (also reads $NOTION_CLI_PROFILE)" env:"NOTION_CLI_PROFILE" name:"profile"` + // Profile intentionally does not use Kong's env:"NOTION_CLI_PROFILE" tag. + // profile.Resolve needs to see the flag value and env variable as + // separate inputs so auth status can attribute the selection to + // --profile vs NOTION_CLI_PROFILE; Kong would merge them into one value + // and we'd always report SourceFlag. + Profile string `help:"Notion account profile to use (also reads $NOTION_CLI_PROFILE)" name:"profile"` Auth AuthCmd `cmd:"" help:"Authentication commands"` Page PageCmd `cmd:"" help:"Page commands"` From d4d5266128a62c5b5ee7a2cb04598608281205ec Mon Sep 17 00:00:00 2001 From: Brian Le Date: Thu, 30 Apr 2026 12:08:44 -0400 Subject: [PATCH 07/15] refactor(auth): pass profile through command context --- cmd/auth.go | 28 ++++++++++++++-------------- cmd/comment.go | 4 ++-- cmd/db.go | 6 +++--- cmd/page.go | 14 +++++++------- cmd/search.go | 3 ++- cmd/tools.go | 2 +- internal/cli/context.go | 15 ++++++++------- internal/cli/official_api.go | 9 +++++---- internal/cli/profile.go | 21 --------------------- main.go | 5 ++--- 10 files changed, 44 insertions(+), 63 deletions(-) delete mode 100644 internal/cli/profile.go diff --git a/cmd/auth.go b/cmd/auth.go index 872bc2f..12ab160 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -37,7 +37,7 @@ const officialAPIIntegrationsURL = "https://www.notion.so/profile/integrations/i type AuthLoginCmd struct{} func (c *AuthLoginCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) + tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) if err != nil { output.PrintError(err) return err @@ -55,7 +55,7 @@ func (c *AuthLoginCmd) Run(ctx *Context) error { type AuthRefreshCmd struct{} func (c *AuthRefreshCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) + tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) if err != nil { output.PrintError(err) return err @@ -95,7 +95,7 @@ type AuthStatusCmd struct { func (c *AuthStatusCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) + tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) if err != nil { output.PrintError(err) return err @@ -112,7 +112,7 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { } hasValidToken := token.AccessToken != "" && !token.IsExpired() - activeProfile := cli.ActiveProfile() + activeProfile := ctx.Profile if ctx.JSON { enc := json.NewEncoder(os.Stdout) @@ -157,7 +157,7 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { type AuthLogoutCmd struct{} func (c *AuthLogoutCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(cli.ActiveProfile()) + tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) if err != nil { output.PrintError(err) return err @@ -196,13 +196,13 @@ func (c *AuthAPISetupCmd) Run(ctx *Context) error { output.PrintWarning("Official API token does not match the expected Notion token format") _, _ = fmt.Fprintln(authAPIOutput, "Expected format: ntn_") } - if err := config.SetAPITokenForProfile(cli.ActiveProfile(), token); err != nil { + if err := config.SetAPITokenForProfile(ctx.Profile, token); err != nil { output.PrintError(err) return err } output.PrintSuccess("Official API token saved") - _, _ = fmt.Fprintf(authAPIOutput, "Config path: %s\n", mustConfigPath()) + _, _ = fmt.Fprintf(authAPIOutput, "Config path: %s\n", mustConfigPath(ctx)) return nil } @@ -213,7 +213,7 @@ type AuthAPIStatusCmd struct { func (c *AuthAPIStatusCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - loaded, err := cli.LoadOfficialAPIConfig(officialAPIOverrides(ctx)) + loaded, err := cli.LoadOfficialAPIConfig(ctx.Profile, officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -228,12 +228,12 @@ type AuthAPIVerifyCmd struct { func (c *AuthAPIVerifyCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - loaded, err := cli.LoadOfficialAPIConfig(officialAPIOverrides(ctx)) + loaded, err := cli.LoadOfficialAPIConfig(ctx.Profile, officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err } - client, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) + client, err := cli.RequireOfficialAPIClient(ctx.Profile, officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -275,7 +275,7 @@ func (c *AuthAPIVerifyCmd) Run(ctx *Context) error { type AuthAPIUnsetCmd struct{} func (c *AuthAPIUnsetCmd) Run(ctx *Context) error { - loaded, err := cli.LoadOfficialAPIConfig(officialAPIOverrides(ctx)) + loaded, err := cli.LoadOfficialAPIConfig(ctx.Profile, officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -291,7 +291,7 @@ func (c *AuthAPIUnsetCmd) Run(ctx *Context) error { return nil } - if err := config.UnsetAPITokenForProfile(cli.ActiveProfile()); err != nil { + if err := config.UnsetAPITokenForProfile(ctx.Profile); err != nil { output.PrintError(err) return err } @@ -380,8 +380,8 @@ func printOfficialAPITokenSetupHint(out io.Writer, shouldOpenBrowser bool) { _, _ = fmt.Fprintln(out) } -func mustConfigPath() string { - path, err := config.PathFor(cli.ActiveProfile()) +func mustConfigPath(ctx *Context) string { + path, err := config.PathFor(ctx.Profile) if err != nil { return "" } diff --git a/cmd/comment.go b/cmd/comment.go index 6d8ef15..1bb4144 100644 --- a/cmd/comment.go +++ b/cmd/comment.go @@ -31,7 +31,7 @@ func (c *CommentListCmd) Run(ctx *Context) error { } func runCommentList(ctx *Context, page string, includeResolved bool) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -266,7 +266,7 @@ func (c *CommentCreateCmd) Run(ctx *Context) error { } func runCommentCreate(ctx *Context, page, content string) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } diff --git a/cmd/db.go b/cmd/db.go index b0554f4..0796e97 100644 --- a/cmd/db.go +++ b/cmd/db.go @@ -28,7 +28,7 @@ func (c *DBListCmd) Run(ctx *Context) error { } func runDBList(ctx *Context, query string, limit int) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -98,7 +98,7 @@ func runDBCreate(ctx *Context, database, title string, props []string, content, content = string(data) } - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -159,7 +159,7 @@ func runDBCreate(ctx *Context, database, title string, props []string, content, } func runDBQuery(ctx *Context, id string) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } diff --git a/cmd/page.go b/cmd/page.go index 2dec616..6a03ba0 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -41,7 +41,7 @@ func (c *PageListCmd) Run(ctx *Context) error { } func runPageList(ctx *Context, query string, limit int) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -90,7 +90,7 @@ func (c *PageViewCmd) Run(ctx *Context) error { } func runPageView(ctx *Context, page string, raw, includeComments bool) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -204,7 +204,7 @@ func (c *PageCreateCmd) Run(ctx *Context) error { } func runPageCreate(ctx *Context, title, parent, content string) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -288,7 +288,7 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski } } - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -430,7 +430,7 @@ func runPageArchive(ctx *Context, page string) error { return err } - apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) + apiClient, err := cli.RequireOfficialAPIClient(ctx.Profile, officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -446,7 +446,7 @@ func runPageArchive(ctx *Context, page string) error { } func runPageEdit(ctx *Context, page, replace, find, replaceWith, appendText string, props []string, allowDeletingContent bool) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } @@ -707,7 +707,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL } if client == nil { - client, err = requirePageClientFn() + client, err = requirePageClientFn(ctx.Profile) if err != nil { return err } diff --git a/cmd/search.go b/cmd/search.go index ca46085..6aa2b2a 100644 --- a/cmd/search.go +++ b/cmd/search.go @@ -21,10 +21,11 @@ func (c *SearchCmd) Run(ctx *Context) error { } func runSearch(ctx *Context, query string, limit int, searchMode string) error { - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } + defer func() { _ = client.Close() }() mode := "workspace_search" if searchMode == "ai" { diff --git a/cmd/tools.go b/cmd/tools.go index 94049ef..e39e440 100644 --- a/cmd/tools.go +++ b/cmd/tools.go @@ -23,7 +23,7 @@ type toolSummary struct { func (c *ToolsCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - client, err := cli.RequireClient() + client, err := cli.RequireClient(ctx.Profile) if err != nil { return err } diff --git a/internal/cli/context.go b/internal/cli/context.go index 036243d..3fa84cf 100644 --- a/internal/cli/context.go +++ b/internal/cli/context.go @@ -9,6 +9,7 @@ import ( "github.com/lox/notion-cli/internal/mcp" "github.com/lox/notion-cli/internal/output" + "github.com/lox/notion-cli/internal/profile" ) var accessToken string @@ -18,18 +19,18 @@ func SetAccessToken(token string) { accessToken = token } -func GetClient() (*mcp.Client, error) { +func GetClient(p profile.Profile) (*mcp.Client, error) { ctx := context.Background() // Auto-refresh if token is expired or expiring soon if accessToken == "" { - if err := autoRefreshIfNeeded(ctx); err != nil { + if err := autoRefreshIfNeeded(ctx, p); err != nil { // Non-fatal, but surface guidance to reduce auth-related command failures. printAuthRefreshGuidance(err) } } - opts := []mcp.ClientOption{mcp.WithProfile(ActiveProfile())} + opts := []mcp.ClientOption{mcp.WithProfile(p)} if accessToken != "" { opts = append(opts, mcp.WithAccessToken(accessToken)) } @@ -50,8 +51,8 @@ func GetClient() (*mcp.Client, error) { return client, nil } -func autoRefreshIfNeeded(ctx context.Context) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(ActiveProfile()) +func autoRefreshIfNeeded(ctx context.Context, p profile.Profile) error { + tokenStore, err := mcp.NewFileTokenStoreForProfile(p) if err != nil { return err } @@ -76,8 +77,8 @@ func autoRefreshIfNeeded(ctx context.Context) error { return nil } -func RequireClient() (*mcp.Client, error) { - return GetClient() +func RequireClient(p profile.Profile) (*mcp.Client, error) { + return GetClient(p) } func printAuthRefreshGuidance(err error) { diff --git a/internal/cli/official_api.go b/internal/cli/official_api.go index 1ce7b31..5cd0952 100644 --- a/internal/cli/official_api.go +++ b/internal/cli/official_api.go @@ -5,6 +5,7 @@ import ( "github.com/lox/notion-cli/internal/api" "github.com/lox/notion-cli/internal/config" + "github.com/lox/notion-cli/internal/profile" ) type OfficialAPIConfig struct { @@ -14,8 +15,8 @@ type OfficialAPIConfig struct { HasConfigToken bool } -func LoadOfficialAPIConfig(overrides config.APIOverrides) (*OfficialAPIConfig, error) { - loaded, err := config.LoadWithMetaForProfile(ActiveProfile(), overrides) +func LoadOfficialAPIConfig(p profile.Profile, overrides config.APIOverrides) (*OfficialAPIConfig, error) { + loaded, err := config.LoadWithMetaForProfile(p, overrides) if err != nil { return nil, fmt.Errorf("load config: %w", err) } @@ -27,8 +28,8 @@ func LoadOfficialAPIConfig(overrides config.APIOverrides) (*OfficialAPIConfig, e }, nil } -func RequireOfficialAPIClient(overrides config.APIOverrides) (*api.Client, error) { - loaded, err := LoadOfficialAPIConfig(overrides) +func RequireOfficialAPIClient(p profile.Profile, overrides config.APIOverrides) (*api.Client, error) { + loaded, err := LoadOfficialAPIConfig(p, overrides) if err != nil { return nil, err } diff --git a/internal/cli/profile.go b/internal/cli/profile.go deleted file mode 100644 index 3ebe949..0000000 --- a/internal/cli/profile.go +++ /dev/null @@ -1,21 +0,0 @@ -package cli - -import ( - "github.com/lox/notion-cli/internal/profile" -) - -// active holds the profile resolved by main.go for the current invocation. -// Helpers in this package that create token stores or load config read -// from it so commands don't need to thread the profile through every call. -var active = profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault} - -// SetActiveProfile records the profile selected for this invocation. -func SetActiveProfile(p profile.Profile) { - active = p -} - -// ActiveProfile returns the profile selected for this invocation, or the -// implicit default profile if none was set. -func ActiveProfile() profile.Profile { - return active -} diff --git a/main.go b/main.go index 2fbe9c2..9d258c0 100644 --- a/main.go +++ b/main.go @@ -42,8 +42,7 @@ func main() { // Headless path: NOTION_ACCESS_TOKEN authenticates MCP directly, so // the profile gate can be skipped. Profile-scoped operations (like // auth api setup) will still use the implicit default layout. - } else { - cli.SetActiveProfile(active) + active = profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault} } cli.SetAccessToken(c.Token) @@ -52,7 +51,7 @@ func main() { APIToken: c.APIToken, APIBaseURL: c.APIBaseURL, APINotionVersion: c.APINotionVersion, - Profile: cli.ActiveProfile(), + Profile: active, }) ctx.FatalIfErrorf(runErr) os.Exit(0) From d3727b1d4f132c81bb7c2949cf5bbd491a51e0ea Mon Sep 17 00:00:00 2001 From: Brian Le Date: Tue, 5 May 2026 11:30:17 -0400 Subject: [PATCH 08/15] feat(auth): refresh multi-profile support --- README.md | 18 +- cmd/auth.go | 231 +++++++++++++++++---- cmd/auth_api_test.go | 5 +- cmd/auth_test.go | 94 +++++++++ cmd/comment.go | 4 +- cmd/db.go | 6 +- cmd/page.go | 14 +- cmd/root.go | 15 +- cmd/search.go | 3 +- cmd/tools.go | 2 +- internal/cli/context.go | 23 ++- internal/cli/official_api.go | 11 +- internal/config/config.go | 311 +++++++++++++++++++++++++---- internal/config/config_test.go | 170 ++++++++++++---- internal/mcp/client.go | 25 +-- internal/mcp/client_test.go | 29 +-- internal/mcp/oauth.go | 78 +++++++- internal/mcp/oauth_test.go | 193 ++++++++++++++++++ internal/mcp/token_lock_unix.go | 30 +++ internal/mcp/token_lock_windows.go | 41 ++++ internal/mcp/token_store.go | 115 +++++++++-- internal/mcp/token_store_test.go | 73 +++++++ internal/profile/profile.go | 202 ------------------- internal/profile/profile_test.go | 207 ------------------- main.go | 32 +-- skills/notion/SKILL.md | 5 +- 26 files changed, 1264 insertions(+), 673 deletions(-) create mode 100644 cmd/auth_test.go create mode 100644 internal/mcp/oauth_test.go create mode 100644 internal/mcp/token_lock_unix.go create mode 100644 internal/mcp/token_lock_windows.go create mode 100644 internal/mcp/token_store_test.go delete mode 100644 internal/profile/profile.go delete mode 100644 internal/profile/profile_test.go diff --git a/README.md b/README.md index 982e5f4..bb13739 100644 --- a/README.md +++ b/README.md @@ -188,26 +188,24 @@ notion-cli page list --profile work # Pin a profile for the shell session export NOTION_CLI_PROFILE=work -# Pin a profile for every invocation (survives across shells) -cat > ~/.config/notion-cli/settings.json <<'JSON' -{"default_profile": "work"} -JSON +# Make a profile the default for future invocations +notion-cli auth use work ``` Profile resolution, highest priority first: 1. `--profile ` flag 2. `NOTION_CLI_PROFILE` environment variable -3. `default_profile` in `~/.config/notion-cli/settings.json` -4. Implicit default (the pre-existing top-level `~/.config/notion-cli/{token,config}.json`) +3. Active profile from `notion-cli auth use ` +4. Implicit default profile -When none of those resolve (no top-level files, no settings, no flag, no env), every command fails up front with `No profile specified. Pass --profile or set NOTION_CLI_PROFILE.` rather than silently treating the caller as unauthenticated. If you want to force `--profile` on every invocation (for example to keep an agent from ever touching the wrong workspace), remove the top-level `{token,config}.json`. +The default profile keeps using the existing top-level `~/.config/notion-cli/{token,config}.json` files, so existing single-account installs need no migration. `notion-cli auth use ` stores the active profile in `~/.config/notion-cli/state.json`. -Profile names must match `^[a-z0-9][a-z0-9_-]*$`. +Profile names may contain letters, numbers, at signs, dots, underscores, and hyphens. -Named profiles store their credentials under `~/.config/notion-cli//{token,config}.json`. The implicit default profile keeps using the existing top-level paths, so existing single-account installs need no migration. +Named profiles store their credentials under `~/.config/notion-cli/profiles//{token,config}.json`. -`notion-cli auth status` always prints the active profile and where it was resolved from, so you can verify which account the CLI is about to hit. +`notion-cli auth status` prints the selected profile and token path, and `notion-cli auth list` shows all known profiles with OAuth and API-token status. ## Environment Variables diff --git a/cmd/auth.go b/cmd/auth.go index 12ab160..8238138 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -9,6 +9,7 @@ import ( "os" "regexp" "strings" + "time" "github.com/fatih/color" "github.com/lox/notion-cli/internal/cli" @@ -22,6 +23,8 @@ type AuthCmd struct { Login AuthLoginCmd `cmd:"" help:"Authenticate with Notion via OAuth"` Refresh AuthRefreshCmd `cmd:"" help:"Refresh the access token"` Status AuthStatusCmd `cmd:"" default:"withargs" help:"Show authentication status"` + List AuthListCmd `cmd:"" help:"List profiles and authentication state"` + Use AuthUseCmd `cmd:"" help:"Set the active profile"` Logout AuthLogoutCmd `cmd:"" help:"Clear stored credentials"` API AuthAPICmd `cmd:"" name:"api" help:"Official API token commands"` } @@ -34,10 +37,21 @@ var notionAPITokenPattern = regexp.MustCompile(`^ntn_[A-Za-z0-9]{20,}$`) const officialAPIIntegrationsURL = "https://www.notion.so/profile/integrations/internal" +type authProfileStatus struct { + Profile string `json:"profile"` + Active bool `json:"active"` + HasOAuthToken bool `json:"has_oauth_token"` + OAuthStatus string `json:"oauth_status"` + OAuthExpiresAt *time.Time `json:"oauth_expires_at,omitempty"` + HasAPIToken bool `json:"has_api_token"` + TokenPath string `json:"token_path"` + ConfigPath string `json:"config_path"` +} + type AuthLoginCmd struct{} func (c *AuthLoginCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) + tokenStore, err := mcp.NewFileTokenStore(ctx.Profile) if err != nil { output.PrintError(err) return err @@ -55,7 +69,7 @@ func (c *AuthLoginCmd) Run(ctx *Context) error { type AuthRefreshCmd struct{} func (c *AuthRefreshCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) + tokenStore, err := mcp.NewFileTokenStore(ctx.Profile) if err != nil { output.PrintError(err) return err @@ -95,69 +109,144 @@ type AuthStatusCmd struct { func (c *AuthStatusCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) + status, err := inspectProfileStatus(ctx.Profile) if err != nil { output.PrintError(err) return err } - token, err := tokenStore.GetToken(context.Background()) - if err != nil { - if err == mcp.ErrNoToken { - fmt.Println("Not authenticated. Run 'notion-cli auth login' to authenticate.") - return nil + if ctx.JSON { + payload := map[string]any{ + "authenticated": status.OAuthStatus == "valid", + "profile": status.Profile, + "has_token": status.HasOAuthToken, + "token_path": status.TokenPath, + "oauth_status": status.OAuthStatus, + } + if status.OAuthExpiresAt != nil { + payload["expires_at"] = status.OAuthExpiresAt } + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + return enc.Encode(payload) + } + + labelStyle := color.New(color.Faint) + switch status.OAuthStatus { + case "valid": + output.PrintSuccess("Authenticated") + case "expired": + output.PrintWarning("Token expired") + default: + output.PrintWarning("Not authenticated") + } + fmt.Println() + + _, _ = labelStyle.Print("Profile: ") + fmt.Println(status.Profile) + _, _ = labelStyle.Print("Token path: ") + fmt.Println(status.TokenPath) + + if status.OAuthExpiresAt != nil { + _, _ = labelStyle.Print("Expires: ") + fmt.Println(status.OAuthExpiresAt.Format("2 Jan 2006 15:04")) + } + if status.OAuthStatus == "missing" { + _, _ = fmt.Fprintln(os.Stdout, "Run 'notion-cli auth login' to authenticate this profile.") + } + + return nil +} + +type AuthListCmd struct { + JSON bool `help:"Output as JSON" short:"j"` +} + +func (c *AuthListCmd) Run(ctx *Context) error { + profiles, err := config.ListProfiles() + if err != nil { output.PrintError(err) return err } - hasValidToken := token.AccessToken != "" && !token.IsExpired() - activeProfile := ctx.Profile + rows := make([]authProfileStatus, 0, len(profiles)) + for _, profile := range profiles { + row, err := inspectProfileStatus(profile) + if err != nil { + output.PrintError(err) + return err + } + rows = append(rows, row) + } - if ctx.JSON { + if c.JSON { enc := json.NewEncoder(os.Stdout) enc.SetIndent("", " ") - return enc.Encode(map[string]any{ - "authenticated": hasValidToken, - "token_type": token.TokenType, - "has_token": token.AccessToken != "", - "expires_at": token.ExpiresAt, - "config_path": tokenStore.Path(), - "profile": activeProfile.Name, - "profile_source": string(activeProfile.Source), - }) + return enc.Encode(rows) } labelStyle := color.New(color.Faint) - - if hasValidToken { - output.PrintSuccess("Authenticated") - } else { - output.PrintWarning("Token expired or not set") + for i, row := range rows { + header := row.Profile + if row.Active { + header += " (active)" + } + fmt.Println(header) + _, _ = labelStyle.Print(" OAuth: ") + fmt.Println(row.OAuthStatus) + if row.OAuthExpiresAt != nil { + _, _ = labelStyle.Print(" Expires: ") + fmt.Println(row.OAuthExpiresAt.Format("2 Jan 2006 15:04")) + } + _, _ = labelStyle.Print(" API token: ") + if row.HasAPIToken { + fmt.Println("configured") + } else { + fmt.Println("missing") + } + _, _ = labelStyle.Print(" Token path: ") + fmt.Println(row.TokenPath) + _, _ = labelStyle.Print(" Config path:") + fmt.Println(" " + row.ConfigPath) + if i < len(rows)-1 { + fmt.Println() + } } - fmt.Println() - _, _ = labelStyle.Print("Profile: ") - fmt.Printf("%s (from %s)\n", activeProfile.Name, activeProfile.Source) + return nil +} - _, _ = labelStyle.Print("Config path: ") - fmt.Println(tokenStore.Path()) +type AuthUseCmd struct { + Profile string `arg:"" help:"Profile name to make active"` +} - _, _ = labelStyle.Print("Token type: ") - fmt.Println(token.TokenType) +func (c *AuthUseCmd) Run(ctx *Context) error { + if err := config.SetActiveProfile(c.Profile); err != nil { + output.PrintError(err) + return err + } - if !token.ExpiresAt.IsZero() { - _, _ = labelStyle.Print("Expires: ") - fmt.Println(token.ExpiresAt.Format("2 Jan 2006 15:04")) + status, err := inspectProfileStatus(c.Profile) + if err != nil { + output.PrintError(err) + return err } + output.PrintSuccess("Active profile updated") + fmt.Printf("Profile: %s\n", status.Profile) + if status.OAuthStatus == "missing" { + fmt.Println("Run 'notion-cli auth login' to authenticate this profile.") + } + if !status.HasAPIToken { + fmt.Println("Run 'notion-cli auth api setup' if this profile needs official API features.") + } return nil } type AuthLogoutCmd struct{} func (c *AuthLogoutCmd) Run(ctx *Context) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(ctx.Profile) + tokenStore, err := mcp.NewFileTokenStore(ctx.Profile) if err != nil { output.PrintError(err) return err @@ -202,7 +291,7 @@ func (c *AuthAPISetupCmd) Run(ctx *Context) error { } output.PrintSuccess("Official API token saved") - _, _ = fmt.Fprintf(authAPIOutput, "Config path: %s\n", mustConfigPath(ctx)) + _, _ = fmt.Fprintf(authAPIOutput, "Config path: %s\n", mustConfigPath(ctx.Profile)) return nil } @@ -213,7 +302,7 @@ type AuthAPIStatusCmd struct { func (c *AuthAPIStatusCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - loaded, err := cli.LoadOfficialAPIConfig(ctx.Profile, officialAPIOverrides(ctx)) + loaded, err := cli.LoadOfficialAPIConfig(officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -228,12 +317,12 @@ type AuthAPIVerifyCmd struct { func (c *AuthAPIVerifyCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - loaded, err := cli.LoadOfficialAPIConfig(ctx.Profile, officialAPIOverrides(ctx)) + loaded, err := cli.LoadOfficialAPIConfig(officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err } - client, err := cli.RequireOfficialAPIClient(ctx.Profile, officialAPIOverrides(ctx)) + client, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -250,6 +339,7 @@ func (c *AuthAPIVerifyCmd) Run(ctx *Context) error { enc.SetIndent("", " ") return enc.Encode(map[string]any{ "verified": true, + "profile": loaded.Profile, "token_source": loaded.APITokenSource, "config_path": loaded.ConfigPath, "base_url": loaded.Config.API.BaseURL, @@ -259,6 +349,7 @@ func (c *AuthAPIVerifyCmd) Run(ctx *Context) error { } output.PrintSuccess("Official API token verified") + _, _ = fmt.Fprintf(authAPIOutput, "Profile: %s\n", loaded.Profile) _, _ = fmt.Fprintf(authAPIOutput, "Token source: %s\n", loaded.APITokenSource) _, _ = fmt.Fprintf(authAPIOutput, "Config path: %s\n", loaded.ConfigPath) _, _ = fmt.Fprintf(authAPIOutput, "Base URL: %s\n", loaded.Config.API.BaseURL) @@ -275,7 +366,7 @@ func (c *AuthAPIVerifyCmd) Run(ctx *Context) error { type AuthAPIUnsetCmd struct{} func (c *AuthAPIUnsetCmd) Run(ctx *Context) error { - loaded, err := cli.LoadOfficialAPIConfig(ctx.Profile, officialAPIOverrides(ctx)) + loaded, err := cli.LoadOfficialAPIConfig(officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -312,6 +403,7 @@ func printAuthAPIStatus(ctx *Context, loaded *cli.OfficialAPIConfig) error { enc.SetIndent("", " ") return enc.Encode(map[string]any{ "configured": hasToken, + "profile": loaded.Profile, "token_source": loaded.APITokenSource, "config_path": loaded.ConfigPath, "base_url": loaded.Config.API.BaseURL, @@ -325,6 +417,7 @@ func printAuthAPIStatus(ctx *Context, loaded *cli.OfficialAPIConfig) error { output.PrintWarning("Official API token not configured") } _, _ = fmt.Fprintln(authAPIOutput) + _, _ = fmt.Fprintf(authAPIOutput, "Profile: %s\n", loaded.Profile) _, _ = fmt.Fprintf(authAPIOutput, "Token source: %s\n", loaded.APITokenSource) _, _ = fmt.Fprintf(authAPIOutput, "Config path: %s\n", loaded.ConfigPath) _, _ = fmt.Fprintf(authAPIOutput, "Base URL: %s\n", loaded.Config.API.BaseURL) @@ -380,10 +473,62 @@ func printOfficialAPITokenSetupHint(out io.Writer, shouldOpenBrowser bool) { _, _ = fmt.Fprintln(out) } -func mustConfigPath(ctx *Context) string { - path, err := config.PathFor(ctx.Profile) +func mustConfigPath(profile string) string { + path, err := config.PathForProfile(profile) if err != nil { return "" } return path } + +func inspectProfileStatus(profile string) (authProfileStatus, error) { + resolvedProfile, err := config.ResolveProfile(profile) + if err != nil { + return authProfileStatus{}, err + } + active, err := config.ActiveProfile() + if err != nil { + return authProfileStatus{}, err + } + paths, err := config.PathsForProfile(resolvedProfile) + if err != nil { + return authProfileStatus{}, err + } + loaded, err := config.LoadWithMeta(config.APIOverrides{Profile: resolvedProfile}) + if err != nil { + return authProfileStatus{}, err + } + + status := authProfileStatus{ + Profile: resolvedProfile, + Active: resolvedProfile == active, + HasAPIToken: loaded.HasConfigToken, + TokenPath: paths.TokenPath, + ConfigPath: paths.ConfigPath, + OAuthStatus: "missing", + } + + tokenStore, err := mcp.NewFileTokenStore(resolvedProfile) + if err != nil { + return authProfileStatus{}, err + } + token, err := tokenStore.GetToken(context.Background()) + if err != nil { + if err == mcp.ErrNoToken { + return status, nil + } + return authProfileStatus{}, err + } + + status.HasOAuthToken = strings.TrimSpace(token.AccessToken) != "" + expiresAt := token.ExpiresAt + status.OAuthExpiresAt = &expiresAt + if status.HasOAuthToken { + if token.IsExpired() { + status.OAuthStatus = "expired" + } else { + status.OAuthStatus = "valid" + } + } + return status, nil +} diff --git a/cmd/auth_api_test.go b/cmd/auth_api_test.go index 9a4452e..37f59d6 100644 --- a/cmd/auth_api_test.go +++ b/cmd/auth_api_test.go @@ -138,12 +138,15 @@ func TestAuthAPIStatusJSONUsesLoadedConfig(t *testing.T) { }) cmd := &AuthAPIStatusCmd{JSON: true} - if err := cmd.Run(&Context{APIToken: "env-token"}); err != nil { + if err := cmd.Run(&Context{Profile: "work", APIToken: "env-token"}); err != nil { t.Fatalf("Run: %v", err) } if !strings.Contains(out.String(), `"configured": true`) { t.Fatalf("unexpected output: %s", out.String()) } + if !strings.Contains(out.String(), `"profile": "work"`) { + t.Fatalf("unexpected output: %s", out.String()) + } if !strings.Contains(out.String(), `"token_source": "env"`) { t.Fatalf("unexpected output: %s", out.String()) } diff --git a/cmd/auth_test.go b/cmd/auth_test.go new file mode 100644 index 0000000..3d5a2df --- /dev/null +++ b/cmd/auth_test.go @@ -0,0 +1,94 @@ +package cmd + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/lox/notion-cli/internal/config" + "github.com/lox/notion-cli/internal/mcp" + "github.com/mark3labs/mcp-go/client/transport" +) + +func TestAuthUsePersistsActiveProfile(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + cmd := &AuthUseCmd{Profile: "work"} + stdout := captureStdout(t, func() { + if err := cmd.Run(&Context{}); err != nil { + t.Fatalf("Run: %v", err) + } + }) + + active, err := config.ActiveProfile() + if err != nil { + t.Fatalf("ActiveProfile: %v", err) + } + if active != "work" { + t.Fatalf("active profile = %q, want work", active) + } + if !strings.Contains(stdout, "Profile: work") { + t.Fatalf("unexpected output: %q", stdout) + } +} + +func TestAuthListJSONShowsProfilesAndActiveState(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + if err := config.SetActiveProfile("work"); err != nil { + t.Fatalf("SetActiveProfile: %v", err) + } + if err := config.SetAPITokenForProfile("personal", "personal-token"); err != nil { + t.Fatalf("SetAPITokenForProfile: %v", err) + } + store, err := mcp.NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + AccessToken: "oauth-token", + TokenType: "Bearer", + ExpiresAt: time.Now().Add(time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + + cmd := &AuthListCmd{JSON: true} + stdout := captureStdout(t, func() { + if err := cmd.Run(&Context{}); err != nil { + t.Fatalf("Run: %v", err) + } + }) + + if !strings.Contains(stdout, `"profile": "work"`) { + t.Fatalf("unexpected output: %s", stdout) + } + if !strings.Contains(stdout, `"active": true`) { + t.Fatalf("unexpected output: %s", stdout) + } + if !strings.Contains(stdout, `"oauth_status": "valid"`) { + t.Fatalf("unexpected output: %s", stdout) + } + if !strings.Contains(stdout, `"profile": "personal"`) { + t.Fatalf("unexpected output: %s", stdout) + } +} + +func TestAuthStatusJSONReportsMissingTokenForProfile(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + cmd := &AuthStatusCmd{JSON: true} + stdout := captureStdout(t, func() { + if err := cmd.Run(&Context{Profile: "work"}); err != nil { + t.Fatalf("Run: %v", err) + } + }) + + if !strings.Contains(stdout, `"profile": "work"`) { + t.Fatalf("unexpected output: %s", stdout) + } + if !strings.Contains(stdout, `"oauth_status": "missing"`) { + t.Fatalf("unexpected output: %s", stdout) + } +} diff --git a/cmd/comment.go b/cmd/comment.go index 1bb4144..6d8ef15 100644 --- a/cmd/comment.go +++ b/cmd/comment.go @@ -31,7 +31,7 @@ func (c *CommentListCmd) Run(ctx *Context) error { } func runCommentList(ctx *Context, page string, includeResolved bool) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -266,7 +266,7 @@ func (c *CommentCreateCmd) Run(ctx *Context) error { } func runCommentCreate(ctx *Context, page, content string) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } diff --git a/cmd/db.go b/cmd/db.go index 0796e97..b0554f4 100644 --- a/cmd/db.go +++ b/cmd/db.go @@ -28,7 +28,7 @@ func (c *DBListCmd) Run(ctx *Context) error { } func runDBList(ctx *Context, query string, limit int) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -98,7 +98,7 @@ func runDBCreate(ctx *Context, database, title string, props []string, content, content = string(data) } - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -159,7 +159,7 @@ func runDBCreate(ctx *Context, database, title string, props []string, content, } func runDBQuery(ctx *Context, id string) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } diff --git a/cmd/page.go b/cmd/page.go index 6a03ba0..2dec616 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -41,7 +41,7 @@ func (c *PageListCmd) Run(ctx *Context) error { } func runPageList(ctx *Context, query string, limit int) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -90,7 +90,7 @@ func (c *PageViewCmd) Run(ctx *Context) error { } func runPageView(ctx *Context, page string, raw, includeComments bool) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -204,7 +204,7 @@ func (c *PageCreateCmd) Run(ctx *Context) error { } func runPageCreate(ctx *Context, title, parent, content string) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -288,7 +288,7 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski } } - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -430,7 +430,7 @@ func runPageArchive(ctx *Context, page string) error { return err } - apiClient, err := cli.RequireOfficialAPIClient(ctx.Profile, officialAPIOverrides(ctx)) + apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -446,7 +446,7 @@ func runPageArchive(ctx *Context, page string) error { } func runPageEdit(ctx *Context, page, replace, find, replaceWith, appendText string, props []string, allowDeletingContent bool) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } @@ -707,7 +707,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL } if client == nil { - client, err = requirePageClientFn(ctx.Profile) + client, err = requirePageClientFn() if err != nil { return err } diff --git a/cmd/root.go b/cmd/root.go index f7d0e24..3e63ade 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,30 +1,22 @@ package cmd -import ( - "github.com/lox/notion-cli/internal/config" - "github.com/lox/notion-cli/internal/profile" -) +import "github.com/lox/notion-cli/internal/config" type Context struct { + Profile string JSON bool Token string APIToken string APIBaseURL string APINotionVersion string - Profile profile.Profile } type CLI struct { + Profile string `help:"Config profile name" env:"NOTION_CLI_PROFILE"` Token string `help:"Access token (skips OAuth)" env:"NOTION_ACCESS_TOKEN" hidden:""` APIToken string `env:"NOTION_API_TOKEN" hidden:""` APIBaseURL string `env:"NOTION_API_BASE_URL" hidden:""` APINotionVersion string `env:"NOTION_API_NOTION_VERSION" hidden:""` - // Profile intentionally does not use Kong's env:"NOTION_CLI_PROFILE" tag. - // profile.Resolve needs to see the flag value and env variable as - // separate inputs so auth status can attribute the selection to - // --profile vs NOTION_CLI_PROFILE; Kong would merge them into one value - // and we'd always report SourceFlag. - Profile string `help:"Notion account profile to use (also reads $NOTION_CLI_PROFILE)" name:"profile"` Auth AuthCmd `cmd:"" help:"Authentication commands"` Page PageCmd `cmd:"" help:"Page commands"` @@ -49,6 +41,7 @@ func officialAPIOverrides(ctx *Context) config.APIOverrides { return config.APIOverrides{} } return config.APIOverrides{ + Profile: ctx.Profile, BaseURL: ctx.APIBaseURL, NotionVersion: ctx.APINotionVersion, Token: ctx.APIToken, diff --git a/cmd/search.go b/cmd/search.go index 6aa2b2a..ca46085 100644 --- a/cmd/search.go +++ b/cmd/search.go @@ -21,11 +21,10 @@ func (c *SearchCmd) Run(ctx *Context) error { } func runSearch(ctx *Context, query string, limit int, searchMode string) error { - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } - defer func() { _ = client.Close() }() mode := "workspace_search" if searchMode == "ai" { diff --git a/cmd/tools.go b/cmd/tools.go index e39e440..94049ef 100644 --- a/cmd/tools.go +++ b/cmd/tools.go @@ -23,7 +23,7 @@ type toolSummary struct { func (c *ToolsCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - client, err := cli.RequireClient(ctx.Profile) + client, err := cli.RequireClient() if err != nil { return err } diff --git a/internal/cli/context.go b/internal/cli/context.go index 3fa84cf..6794d10 100644 --- a/internal/cli/context.go +++ b/internal/cli/context.go @@ -9,31 +9,36 @@ import ( "github.com/lox/notion-cli/internal/mcp" "github.com/lox/notion-cli/internal/output" - "github.com/lox/notion-cli/internal/profile" ) var accessToken string +var profile string var authRefreshNoticeWriter io.Writer = os.Stderr func SetAccessToken(token string) { accessToken = token } -func GetClient(p profile.Profile) (*mcp.Client, error) { +func SetProfile(value string) { + profile = value +} + +func GetClient() (*mcp.Client, error) { ctx := context.Background() // Auto-refresh if token is expired or expiring soon if accessToken == "" { - if err := autoRefreshIfNeeded(ctx, p); err != nil { + if err := autoRefreshIfNeeded(ctx); err != nil { // Non-fatal, but surface guidance to reduce auth-related command failures. printAuthRefreshGuidance(err) } } - opts := []mcp.ClientOption{mcp.WithProfile(p)} + var opts []mcp.ClientOption if accessToken != "" { opts = append(opts, mcp.WithAccessToken(accessToken)) } + opts = append(opts, mcp.WithProfile(profile)) client, err := mcp.NewClient(opts...) if err != nil { @@ -51,8 +56,8 @@ func GetClient(p profile.Profile) (*mcp.Client, error) { return client, nil } -func autoRefreshIfNeeded(ctx context.Context, p profile.Profile) error { - tokenStore, err := mcp.NewFileTokenStoreForProfile(p) +func autoRefreshIfNeeded(ctx context.Context) error { + tokenStore, err := mcp.NewFileTokenStore(profile) if err != nil { return err } @@ -68,7 +73,7 @@ func autoRefreshIfNeeded(ctx context.Context, p profile.Profile) error { return fmt.Errorf("token expired and no refresh token available") } - _, err := mcp.RefreshToken(ctx, tokenStore) + _, err := mcp.RefreshTokenIfNeeded(ctx, tokenStore) if err != nil { return fmt.Errorf("auto-refresh failed: %w", err) } @@ -77,8 +82,8 @@ func autoRefreshIfNeeded(ctx context.Context, p profile.Profile) error { return nil } -func RequireClient(p profile.Profile) (*mcp.Client, error) { - return GetClient(p) +func RequireClient() (*mcp.Client, error) { + return GetClient() } func printAuthRefreshGuidance(err error) { diff --git a/internal/cli/official_api.go b/internal/cli/official_api.go index 5cd0952..452fc8b 100644 --- a/internal/cli/official_api.go +++ b/internal/cli/official_api.go @@ -5,31 +5,32 @@ import ( "github.com/lox/notion-cli/internal/api" "github.com/lox/notion-cli/internal/config" - "github.com/lox/notion-cli/internal/profile" ) type OfficialAPIConfig struct { Config config.Config + Profile string ConfigPath string APITokenSource string HasConfigToken bool } -func LoadOfficialAPIConfig(p profile.Profile, overrides config.APIOverrides) (*OfficialAPIConfig, error) { - loaded, err := config.LoadWithMetaForProfile(p, overrides) +func LoadOfficialAPIConfig(overrides config.APIOverrides) (*OfficialAPIConfig, error) { + loaded, err := config.LoadWithMeta(overrides) if err != nil { return nil, fmt.Errorf("load config: %w", err) } return &OfficialAPIConfig{ Config: loaded.Config, + Profile: loaded.Profile, ConfigPath: loaded.Path, APITokenSource: loaded.APITokenSource, HasConfigToken: loaded.HasConfigToken, }, nil } -func RequireOfficialAPIClient(p profile.Profile, overrides config.APIOverrides) (*api.Client, error) { - loaded, err := LoadOfficialAPIConfig(p, overrides) +func RequireOfficialAPIClient(overrides config.APIOverrides) (*api.Client, error) { + loaded, err := LoadOfficialAPIConfig(overrides) if err != nil { return nil, err } diff --git a/internal/config/config.go b/internal/config/config.go index 323d937..d0d6d5b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,13 +6,18 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" - - "github.com/lox/notion-cli/internal/profile" + "unicode" ) const ( + configDirName = "notion-cli" configFileName = "config.json" + tokenFileName = "token.json" + stateFileName = "state.json" + profilesDirName = "profiles" + defaultProfileName = "default" defaultAPIBaseURL = "https://api.notion.com/v1" defaultNotionAPIVer = "2026-03-11" ) @@ -29,17 +34,29 @@ type APIConfig struct { type LoadedConfig struct { Config Config + Profile string Path string APITokenSource string HasConfigToken bool } type APIOverrides struct { + Profile string BaseURL string NotionVersion string Token string } +type ProfilePaths struct { + Profile string + ConfigPath string + TokenPath string +} + +type State struct { + ActiveProfile string `json:"active_profile,omitempty"` +} + const ( APITokenSourceNone = "none" APITokenSourceConfig = "config" @@ -55,20 +72,252 @@ func Default() Config { } } -// defaultProfile is the implicit default profile used when a caller does -// not pass one. Matches the legacy single-profile layout. -func defaultProfile() profile.Profile { - return profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault} +func Path() (string, error) { + return PathForProfile("") } -// Path returns the API config file path for the default profile. -func Path() (string, error) { - return PathFor(defaultProfile()) +func ConfigDir() (string, error) { + configDir, err := os.UserConfigDir() + if err != nil { + return "", err + } + return filepath.Join(configDir, configDirName), nil } -// PathFor returns the API config file path for the given profile. -func PathFor(p profile.Profile) (string, error) { - return profile.ConfigPath(p) +func ProfilesDir() (string, error) { + baseDir, err := ConfigDir() + if err != nil { + return "", err + } + return filepath.Join(baseDir, profilesDirName), nil +} + +func StatePath() (string, error) { + baseDir, err := ConfigDir() + if err != nil { + return "", err + } + return filepath.Join(baseDir, stateFileName), nil +} + +func PathForProfile(profile string) (string, error) { + paths, err := PathsForProfile(profile) + if err != nil { + return "", err + } + return paths.ConfigPath, nil +} + +func PathsForProfile(profile string) (ProfilePaths, error) { + baseDir, err := ConfigDir() + if err != nil { + return ProfilePaths{}, err + } + + resolvedProfile, err := ResolveProfile(profile) + if err != nil { + return ProfilePaths{}, err + } + + profileDir := baseDir + if resolvedProfile != defaultProfileName { + profileDir = filepath.Join(baseDir, profilesDirName, resolvedProfile) + } + + return ProfilePaths{ + Profile: resolvedProfile, + ConfigPath: filepath.Join(profileDir, configFileName), + TokenPath: filepath.Join(profileDir, tokenFileName), + }, nil +} + +func DefaultProfile() string { + return defaultProfileName +} + +func ResolveProfile(profile string) (string, error) { + normalized := strings.TrimSpace(profile) + if normalized == "" { + return defaultProfileName, nil + } + if normalized == "." || normalized == ".." { + return "", fmt.Errorf("invalid profile %q", profile) + } + for _, r := range normalized { + if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '.' || r == '_' || r == '-' || r == '@' { + continue + } + return "", fmt.Errorf("invalid profile %q: use letters, numbers, at sign, dot, underscore, and hyphen", profile) + } + return normalized, nil +} + +func LoadState() (State, error) { + path, err := StatePath() + if err != nil { + return State{}, err + } + data, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return State{}, nil + } + return State{}, fmt.Errorf("read state: %w", err) + } + if len(data) == 0 { + return State{}, nil + } + var state State + if err := json.Unmarshal(data, &state); err != nil { + return State{}, fmt.Errorf("parse state: %w", err) + } + if state.ActiveProfile != "" { + resolved, err := ResolveProfile(state.ActiveProfile) + if err != nil { + return State{}, err + } + state.ActiveProfile = resolved + } + return state, nil +} + +func SaveState(state State) error { + if state.ActiveProfile != "" { + resolved, err := ResolveProfile(state.ActiveProfile) + if err != nil { + return err + } + state.ActiveProfile = resolved + } + + path, err := StatePath() + if err != nil { + return err + } + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("create state dir: %w", err) + } + if err := os.Chmod(dir, 0o700); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("secure state dir: %w", err) + } + + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + return fmt.Errorf("marshal state: %w", err) + } + data = append(data, '\n') + + tmp, err := os.CreateTemp(dir, stateFileName+".*.tmp") + if err != nil { + return fmt.Errorf("create temp state: %w", err) + } + + tmpPath := tmp.Name() + cleanup := func() { + _ = tmp.Close() + _ = os.Remove(tmpPath) + } + if err := tmp.Chmod(0o600); err != nil { + cleanup() + return fmt.Errorf("secure temp state: %w", err) + } + if _, err := tmp.Write(data); err != nil { + cleanup() + return fmt.Errorf("write temp state: %w", err) + } + if err := tmp.Close(); err != nil { + cleanup() + return fmt.Errorf("close temp state: %w", err) + } + if err := os.Rename(tmpPath, path); err != nil { + cleanup() + return fmt.Errorf("replace state: %w", err) + } + if err := os.Chmod(path, 0o600); err != nil { + return fmt.Errorf("secure state file: %w", err) + } + return nil +} + +func ResolveSelectedProfile(requested string) (string, error) { + if strings.TrimSpace(requested) != "" { + return ResolveProfile(requested) + } + state, err := LoadState() + if err != nil { + return "", err + } + if strings.TrimSpace(state.ActiveProfile) != "" { + return ResolveProfile(state.ActiveProfile) + } + return DefaultProfile(), nil +} + +func SetActiveProfile(profile string) error { + resolved, err := ResolveProfile(profile) + if err != nil { + return err + } + return SaveState(State{ActiveProfile: resolved}) +} + +func ActiveProfile() (string, error) { + state, err := LoadState() + if err != nil { + return "", err + } + if strings.TrimSpace(state.ActiveProfile) != "" { + return state.ActiveProfile, nil + } + return DefaultProfile(), nil +} + +func ListProfiles() ([]string, error) { + baseDir, err := ProfilesDir() + if err != nil { + return nil, err + } + + names := map[string]struct{}{ + DefaultProfile(): {}, + } + active, err := ActiveProfile() + if err != nil { + return nil, err + } + names[active] = struct{}{} + + entries, err := os.ReadDir(baseDir) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, fmt.Errorf("read profiles dir: %w", err) + } + for _, entry := range entries { + if !entry.IsDir() { + continue + } + resolved, err := ResolveProfile(entry.Name()) + if err != nil { + return nil, err + } + names[resolved] = struct{}{} + } + + var rest []string + for name := range names { + if name == active || name == DefaultProfile() { + continue + } + rest = append(rest, name) + } + slices.Sort(rest) + + profiles := []string{active} + if active != DefaultProfile() { + profiles = append(profiles, DefaultProfile()) + } + profiles = append(profiles, rest...) + return profiles, nil } func Load() (Config, error) { @@ -80,17 +329,12 @@ func Load() (Config, error) { } func LoadWithMeta(overrides APIOverrides) (LoadedConfig, error) { - return LoadWithMetaForProfile(defaultProfile(), overrides) -} - -// LoadWithMetaForProfile loads the API config file that belongs to the -// given profile and applies the provided overrides. -func LoadWithMetaForProfile(p profile.Profile, overrides APIOverrides) (LoadedConfig, error) { cfg := Default() - path, err := PathFor(p) + paths, err := PathsForProfile(overrides.Profile) if err != nil { return LoadedConfig{}, err } + path := paths.ConfigPath fileCfg, err := loadFile(path) if err != nil { @@ -110,6 +354,7 @@ func LoadWithMetaForProfile(p profile.Profile, overrides APIOverrides) (LoadedCo normalize(&cfg) return LoadedConfig{ Config: cfg, + Profile: paths.Profile, Path: path, APITokenSource: source, HasConfigToken: strings.TrimSpace(fileCfg.API.Token) != "", @@ -117,13 +362,11 @@ func LoadWithMetaForProfile(p profile.Profile, overrides APIOverrides) (LoadedCo } func Save(cfg Config) error { - return SaveForProfile(defaultProfile(), cfg) + return SaveForProfile("", cfg) } -// SaveForProfile writes the given config to the profile's config file -// atomically. -func SaveForProfile(p profile.Profile, cfg Config) error { - path, err := PathFor(p) +func SaveForProfile(profile string, cfg Config) error { + path, err := PathForProfile(profile) if err != nil { return err } @@ -177,33 +420,33 @@ func SaveForProfile(p profile.Profile, cfg Config) error { } func SetAPIToken(token string) error { - return SetAPITokenForProfile(defaultProfile(), token) + return SetAPITokenForProfile("", token) } -func SetAPITokenForProfile(p profile.Profile, token string) error { - cfg, err := loadForMutation(p) +func SetAPITokenForProfile(profile, token string) error { + cfg, err := loadForMutation(profile) if err != nil { return err } cfg.API.Token = strings.TrimSpace(token) - return SaveForProfile(p, cfg) + return SaveForProfile(profile, cfg) } func UnsetAPIToken() error { - return UnsetAPITokenForProfile(defaultProfile()) + return UnsetAPITokenForProfile("") } -func UnsetAPITokenForProfile(p profile.Profile) error { - cfg, err := loadForMutation(p) +func UnsetAPITokenForProfile(profile string) error { + cfg, err := loadForMutation(profile) if err != nil { return err } cfg.API.Token = "" - return SaveForProfile(p, cfg) + return SaveForProfile(profile, cfg) } -func loadForMutation(p profile.Profile) (Config, error) { - path, err := PathFor(p) +func loadForMutation(profile string) (Config, error) { + path, err := PathForProfile(profile) if err != nil { return Config{}, err } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8431bf8..5570ca2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,9 +3,9 @@ package config import ( "os" "path/filepath" + "reflect" + "strings" "testing" - - "github.com/lox/notion-cli/internal/profile" ) func TestLoadWithMetaDefaults(t *testing.T) { @@ -24,6 +24,9 @@ func TestLoadWithMetaDefaults(t *testing.T) { if loaded.APITokenSource != APITokenSourceNone { t.Fatalf("APITokenSource = %q, want %q", loaded.APITokenSource, APITokenSourceNone) } + if loaded.Profile != DefaultProfile() { + t.Fatalf("Profile = %q, want %q", loaded.Profile, DefaultProfile()) + } } func TestLoadWithMetaReportsConfigTokenSource(t *testing.T) { @@ -92,69 +95,162 @@ func TestUnsetAPITokenClearsStoredToken(t *testing.T) { } } -func TestSetAPITokenIsIsolatedPerProfile(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmp, ".config")) +func TestSaveSecuresConfigFile(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + cfg := Default() + cfg.API.Token = "secret-token" + if err := Save(cfg); err != nil { + t.Fatalf("Save: %v", err) + } - work := profile.Profile{Name: "work", Source: profile.SourceFlag} - home := profile.Profile{Name: "home", Source: profile.SourceFlag} + path, err := Path() + if err != nil { + t.Fatalf("Path: %v", err) + } - if err := SetAPITokenForProfile(work, "work-token"); err != nil { - t.Fatalf("SetAPITokenForProfile(work): %v", err) + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat: %v", err) } - if err := SetAPITokenForProfile(home, "home-token"); err != nil { - t.Fatalf("SetAPITokenForProfile(home): %v", err) + if perm := info.Mode().Perm(); perm != 0o600 { + t.Fatalf("config perm = %o, want 600", perm) } - loadedWork, err := LoadWithMetaForProfile(work, APIOverrides{}) + dirInfo, err := os.Stat(filepath.Dir(path)) if err != nil { - t.Fatalf("LoadWithMetaForProfile(work): %v", err) + t.Fatalf("Stat dir: %v", err) } - if loadedWork.Config.API.Token != "work-token" { - t.Fatalf("work token = %q, want work-token", loadedWork.Config.API.Token) + if perm := dirInfo.Mode().Perm(); perm != 0o700 { + t.Fatalf("config dir perm = %o, want 700", perm) } +} + +func TestPathsForProfileDefaultAndNamed(t *testing.T) { + t.Setenv("HOME", t.TempDir()) - loadedHome, err := LoadWithMetaForProfile(home, APIOverrides{}) + defaultPaths, err := PathsForProfile("") if err != nil { - t.Fatalf("LoadWithMetaForProfile(home): %v", err) + t.Fatalf("PathsForProfile default: %v", err) + } + if defaultPaths.Profile != DefaultProfile() { + t.Fatalf("default profile = %q, want %q", defaultPaths.Profile, DefaultProfile()) + } + if got := filepath.Base(defaultPaths.ConfigPath); got != configFileName { + t.Fatalf("default config filename = %q, want %q", got, configFileName) } - if loadedHome.Config.API.Token != "home-token" { - t.Fatalf("home token = %q, want home-token", loadedHome.Config.API.Token) + if got := filepath.Base(defaultPaths.TokenPath); got != tokenFileName { + t.Fatalf("default token filename = %q, want %q", got, tokenFileName) } - if loadedWork.Path == loadedHome.Path { - t.Fatalf("work and home configs share path: %q", loadedWork.Path) + workPaths, err := PathsForProfile("work") + if err != nil { + t.Fatalf("PathsForProfile work: %v", err) + } + if workPaths.Profile != "work" { + t.Fatalf("work profile = %q, want work", workPaths.Profile) + } + if !strings.Contains(workPaths.ConfigPath, filepath.Join(profilesDirName, "work")) { + t.Fatalf("work config path = %q, want profiles/work segment", workPaths.ConfigPath) + } + if !strings.Contains(workPaths.TokenPath, filepath.Join(profilesDirName, "work")) { + t.Fatalf("work token path = %q, want profiles/work segment", workPaths.TokenPath) } } -func TestSaveSecuresConfigFile(t *testing.T) { +func TestProfileSpecificAPITokensAreIsolated(t *testing.T) { t.Setenv("HOME", t.TempDir()) - cfg := Default() - cfg.API.Token = "secret-token" - if err := Save(cfg); err != nil { - t.Fatalf("Save: %v", err) + if err := SetAPIToken("default-token"); err != nil { + t.Fatalf("SetAPIToken default: %v", err) + } + if err := SetAPITokenForProfile("work", "work-token"); err != nil { + t.Fatalf("SetAPITokenForProfile: %v", err) } - path, err := Path() + defaultLoaded, err := LoadWithMeta(APIOverrides{}) if err != nil { - t.Fatalf("Path: %v", err) + t.Fatalf("LoadWithMeta default: %v", err) + } + if defaultLoaded.Config.API.Token != "default-token" { + t.Fatalf("default token = %q, want default-token", defaultLoaded.Config.API.Token) } - info, err := os.Stat(path) + workLoaded, err := LoadWithMeta(APIOverrides{Profile: "work"}) if err != nil { - t.Fatalf("Stat: %v", err) + t.Fatalf("LoadWithMeta work: %v", err) } - if perm := info.Mode().Perm(); perm != 0o600 { - t.Fatalf("config perm = %o, want 600", perm) + if workLoaded.Config.API.Token != "work-token" { + t.Fatalf("work token = %q, want work-token", workLoaded.Config.API.Token) } + if workLoaded.Path == defaultLoaded.Path { + t.Fatalf("profile config path should differ from default path") + } +} - dirInfo, err := os.Stat(filepath.Dir(path)) +func TestResolveProfileRejectsInvalidNames(t *testing.T) { + for _, value := range []string{"../oops", "work/team", "two words"} { + if _, err := ResolveProfile(value); err == nil { + t.Fatalf("ResolveProfile(%q) should fail", value) + } + } +} + +func TestResolveProfileAllowsEmailNames(t *testing.T) { + got, err := ResolveProfile("brian@brianle.xyz") if err != nil { - t.Fatalf("Stat dir: %v", err) + t.Fatalf("ResolveProfile: %v", err) } - if perm := dirInfo.Mode().Perm(); perm != 0o700 { - t.Fatalf("config dir perm = %o, want 700", perm) + if got != "brian@brianle.xyz" { + t.Fatalf("profile = %q, want brian@brianle.xyz", got) + } +} + +func TestResolveSelectedProfileUsesActiveStateWhenUnset(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + if err := SetActiveProfile("work"); err != nil { + t.Fatalf("SetActiveProfile: %v", err) + } + + profile, err := ResolveSelectedProfile("") + if err != nil { + t.Fatalf("ResolveSelectedProfile: %v", err) + } + if profile != "work" { + t.Fatalf("profile = %q, want work", profile) + } +} + +func TestResolveSelectedProfilePrefersExplicitValue(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + if err := SetActiveProfile("work"); err != nil { + t.Fatalf("SetActiveProfile: %v", err) + } + + profile, err := ResolveSelectedProfile("personal") + if err != nil { + t.Fatalf("ResolveSelectedProfile: %v", err) + } + if profile != "personal" { + t.Fatalf("profile = %q, want personal", profile) + } +} + +func TestListProfilesIncludesActiveDefaultAndNamedProfiles(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + if err := SetActiveProfile("work"); err != nil { + t.Fatalf("SetActiveProfile: %v", err) + } + if err := SetAPITokenForProfile("personal", "personal-token"); err != nil { + t.Fatalf("SetAPITokenForProfile: %v", err) + } + + got, err := ListProfiles() + if err != nil { + t.Fatalf("ListProfiles: %v", err) + } + want := []string{"work", "default", "personal"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("profiles = %#v, want %#v", got, want) } } diff --git a/internal/mcp/client.go b/internal/mcp/client.go index f4c5237..038d380 100644 --- a/internal/mcp/client.go +++ b/internal/mcp/client.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/lox/notion-cli/internal/profile" "github.com/mark3labs/mcp-go/client" "github.com/mark3labs/mcp-go/client/transport" "github.com/mark3labs/mcp-go/mcp" @@ -32,8 +31,7 @@ type ClientOption func(*clientConfig) type clientConfig struct { endpoint string accessToken string - profile profile.Profile - hasProfile bool + profile string } func WithEndpoint(endpoint string) ClientOption { @@ -48,13 +46,9 @@ func WithAccessToken(token string) ClientOption { } } -// WithProfile routes the OAuth token store to the given profile so named -// profiles read and refresh their own credentials instead of falling back -// to the default profile's top-level files. -func WithProfile(p profile.Profile) ClientOption { +func WithProfile(profile string) ClientOption { return func(c *clientConfig) { - c.profile = p - c.hasProfile = true + c.profile = profile } } @@ -66,13 +60,7 @@ func NewClient(opts ...ClientOption) (*Client, error) { opt(cfg) } - var tokenStore *FileTokenStore - var err error - if cfg.hasProfile { - tokenStore, err = NewFileTokenStoreForProfile(cfg.profile) - } else { - tokenStore, err = NewFileTokenStore() - } + tokenStore, err := NewFileTokenStore(cfg.profile) if err != nil { return nil, fmt.Errorf("create token store: %w", err) } @@ -209,9 +197,8 @@ func (c *Client) Search(ctx context.Context, query string, opts *SearchOptions) } func buildSearchToolArgs(query string, opts *SearchOptions) map[string]any { - args := map[string]any{} - if strings.TrimSpace(query) != "" { - args["query"] = query + args := map[string]any{ + "query": strings.TrimSpace(query), } if opts != nil && opts.ContentSearchMode != "" { args["content_search_mode"] = opts.ContentSearchMode diff --git a/internal/mcp/client_test.go b/internal/mcp/client_test.go index e25497d..5723a33 100644 --- a/internal/mcp/client_test.go +++ b/internal/mcp/client_test.go @@ -1,39 +1,14 @@ package mcp import ( - "path/filepath" "reflect" "testing" - - "github.com/lox/notion-cli/internal/profile" ) -func TestNewFileTokenStoreForProfileIsolatesPaths(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - - defaultStore, err := NewFileTokenStoreForProfile(profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault}) - if err != nil { - t.Fatalf("NewFileTokenStoreForProfile(default): %v", err) - } - workStore, err := NewFileTokenStoreForProfile(profile.Profile{Name: "work", Source: profile.SourceFlag}) - if err != nil { - t.Fatalf("NewFileTokenStoreForProfile(work): %v", err) - } - - defaultWant := filepath.Join(tmp, ".config", "notion-cli", "token.json") - workWant := filepath.Join(tmp, ".config", "notion-cli", "work", "token.json") - if defaultStore.Path() != defaultWant { - t.Fatalf("default store path = %q, want %q", defaultStore.Path(), defaultWant) - } - if workStore.Path() != workWant { - t.Fatalf("work store path = %q, want %q", workStore.Path(), workWant) - } -} - -func TestBuildSearchToolArgsOmitsBlankQuery(t *testing.T) { +func TestBuildSearchToolArgsIncludesBlankQuery(t *testing.T) { got := buildSearchToolArgs("", &SearchOptions{ContentSearchMode: "workspace_search"}) want := map[string]any{ + "query": "", "content_search_mode": "workspace_search", } diff --git a/internal/mcp/oauth.go b/internal/mcp/oauth.go index d47a6ee..54e2019 100644 --- a/internal/mcp/oauth.go +++ b/internal/mcp/oauth.go @@ -11,6 +11,7 @@ import ( "net/http" "os/exec" "runtime" + "strings" "time" "github.com/mark3labs/mcp-go/client" @@ -20,6 +21,12 @@ import ( const callbackPath = "/callback" +const refreshSkew = 5 * time.Minute + +type refreshTokenFunc func(context.Context, *FileTokenStore, *transport.Token) (*transport.Token, error) + +var refreshOAuthToken refreshTokenFunc = refreshTokenWithNotion + func GenerateCodeVerifier() (string, error) { b := make([]byte, 32) if _, err := rand.Read(b); err != nil { @@ -201,11 +208,74 @@ func RunOAuthFlow(ctx context.Context, tokenStore *FileTokenStore) error { } func RefreshToken(ctx context.Context, tokenStore *FileTokenStore) (*transport.Token, error) { - token, err := tokenStore.GetToken(ctx) + return refreshTokenLocked(ctx, tokenStore, true) +} + +func RefreshTokenIfNeeded(ctx context.Context, tokenStore *FileTokenStore) (*transport.Token, error) { + return refreshTokenLocked(ctx, tokenStore, false) +} + +func refreshTokenLocked(ctx context.Context, tokenStore *FileTokenStore, force bool) (*transport.Token, error) { + var refreshed *transport.Token + err := tokenStore.WithLock(ctx, func() error { + token, err := tokenStore.GetToken(ctx) + if err != nil { + return fmt.Errorf("get token: %w", err) + } + + if !force && tokenFresh(token) { + refreshed = token + return nil + } + + newToken, err := refreshOAuthToken(ctx, tokenStore, token) + if err != nil { + if isInvalidGrantError(err) { + latest, latestErr := tokenStore.GetToken(ctx) + if latestErr == nil && tokenFresh(latest) { + refreshed = latest + return nil + } + return fmt.Errorf("refresh token was rejected; browser login required: %w", err) + } + return err + } + + if err := tokenStore.SaveToken(ctx, newToken); err != nil { + return fmt.Errorf("save token: %w", err) + } + refreshed = newToken + return nil + }) if err != nil { - return nil, fmt.Errorf("get token: %w", err) + return nil, err + } + if refreshed == nil { + return nil, errors.New("refresh did not return a token") + } + return refreshed, nil +} + +func tokenFresh(token *transport.Token) bool { + return token != nil && + token.AccessToken != "" && + (token.ExpiresAt.IsZero() || token.ExpiresAt.After(time.Now().Add(refreshSkew))) +} + +func isInvalidGrantError(err error) bool { + if err == nil { + return false + } + + var oauthErr transport.OAuthError + if errors.As(err, &oauthErr) && oauthErr.ErrorCode == "invalid_grant" { + return true } + return strings.Contains(err.Error(), "invalid_grant") +} + +func refreshTokenWithNotion(ctx context.Context, tokenStore *FileTokenStore, token *transport.Token) (*transport.Token, error) { if token.RefreshToken == "" { return nil, errors.New("no refresh token available") } @@ -249,10 +319,6 @@ func RefreshToken(ctx context.Context, tokenStore *FileTokenStore) (*transport.T return nil, fmt.Errorf("refresh token: %w", err) } - if err := tokenStore.SaveToken(ctx, newToken); err != nil { - return nil, fmt.Errorf("save token: %w", err) - } - return newToken, nil } diff --git a/internal/mcp/oauth_test.go b/internal/mcp/oauth_test.go new file mode 100644 index 0000000..1a55d37 --- /dev/null +++ b/internal/mcp/oauth_test.go @@ -0,0 +1,193 @@ +package mcp + +import ( + "context" + "fmt" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/mark3labs/mcp-go/client/transport" +) + +func TestRefreshTokenSkipsRefreshWhenAnotherCallerAlreadyRefreshed(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + store, err := NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + saveExpiredToken(t, store) + + oldRefresh := refreshOAuthToken + t.Cleanup(func() { + refreshOAuthToken = oldRefresh + }) + + var refreshCalls atomic.Int32 + refreshOAuthToken = func(ctx context.Context, tokenStore *FileTokenStore, token *transport.Token) (*transport.Token, error) { + refreshCalls.Add(1) + time.Sleep(50 * time.Millisecond) + return &transport.Token{ + AccessToken: "new-access", + TokenType: "bearer", + RefreshToken: "new-refresh", + ExpiresAt: time.Now().Add(time.Hour), + }, nil + } + + var wg sync.WaitGroup + errs := make(chan error, 2) + for i := 0; i < 2; i++ { + wg.Add(1) + go func() { + defer wg.Done() + token, err := RefreshTokenIfNeeded(context.Background(), store) + if err != nil { + errs <- err + return + } + if token.AccessToken != "new-access" { + errs <- fmt.Errorf("access token = %q, want new-access", token.AccessToken) + } + }() + } + wg.Wait() + close(errs) + + for err := range errs { + if err != nil { + t.Fatal(err) + } + } + if got := refreshCalls.Load(); got != 1 { + t.Fatalf("refresh calls = %d, want 1", got) + } +} + +func TestRefreshTokenInvalidGrantUsesNewerSavedToken(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + store, err := NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + saveExpiredToken(t, store) + + oldRefresh := refreshOAuthToken + t.Cleanup(func() { + refreshOAuthToken = oldRefresh + }) + + refreshOAuthToken = func(ctx context.Context, tokenStore *FileTokenStore, token *transport.Token) (*transport.Token, error) { + if err := tokenStore.SaveToken(ctx, &transport.Token{ + AccessToken: "winner-access", + TokenType: "bearer", + RefreshToken: "winner-refresh", + ExpiresAt: time.Now().Add(time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + return nil, fmt.Errorf("refresh token: %w", transport.OAuthError{ErrorCode: "invalid_grant"}) + } + + token, err := RefreshToken(context.Background(), store) + if err != nil { + t.Fatalf("RefreshToken: %v", err) + } + if token.AccessToken != "winner-access" { + t.Fatalf("access token = %q, want winner-access", token.AccessToken) + } +} + +func TestRefreshTokenForcesRefreshWhenTokenIsFresh(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + store, err := NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + if err := store.SaveClientID(context.Background(), "client-123"); err != nil { + t.Fatalf("SaveClientID: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + AccessToken: "fresh-access", + TokenType: "bearer", + RefreshToken: "fresh-refresh", + ExpiresAt: time.Now().Add(time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + + oldRefresh := refreshOAuthToken + t.Cleanup(func() { + refreshOAuthToken = oldRefresh + }) + + var refreshCalls atomic.Int32 + refreshOAuthToken = func(context.Context, *FileTokenStore, *transport.Token) (*transport.Token, error) { + refreshCalls.Add(1) + return &transport.Token{ + AccessToken: "forced-access", + TokenType: "bearer", + RefreshToken: "forced-refresh", + ExpiresAt: time.Now().Add(time.Hour), + }, nil + } + + token, err := RefreshToken(context.Background(), store) + if err != nil { + t.Fatalf("RefreshToken: %v", err) + } + if token.AccessToken != "forced-access" { + t.Fatalf("access token = %q, want forced-access", token.AccessToken) + } + if got := refreshCalls.Load(); got != 1 { + t.Fatalf("refresh calls = %d, want 1", got) + } +} + +func TestRefreshTokenInvalidGrantRequiresLoginWhenNoNewerTokenExists(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + store, err := NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + saveExpiredToken(t, store) + + oldRefresh := refreshOAuthToken + t.Cleanup(func() { + refreshOAuthToken = oldRefresh + }) + + refreshOAuthToken = func(context.Context, *FileTokenStore, *transport.Token) (*transport.Token, error) { + return nil, fmt.Errorf("refresh token: %w", transport.OAuthError{ErrorCode: "invalid_grant"}) + } + + _, err = RefreshToken(context.Background(), store) + if err == nil { + t.Fatal("RefreshToken returned nil error, want browser login required") + } + if got := err.Error(); !strings.Contains(got, "browser login required") { + t.Fatalf("error = %q, want browser login required", got) + } +} + +func saveExpiredToken(t *testing.T, store *FileTokenStore) { + t.Helper() + + if err := store.SaveClientID(context.Background(), "client-123"); err != nil { + t.Fatalf("SaveClientID: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + AccessToken: "old-access", + TokenType: "bearer", + RefreshToken: "old-refresh", + ExpiresAt: time.Now().Add(-time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } +} diff --git a/internal/mcp/token_lock_unix.go b/internal/mcp/token_lock_unix.go new file mode 100644 index 0000000..b3055ab --- /dev/null +++ b/internal/mcp/token_lock_unix.go @@ -0,0 +1,30 @@ +//go:build !windows + +package mcp + +import ( + "os" + + "golang.org/x/sys/unix" +) + +func acquireFileLock(path string) (*os.File, error) { + lockFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0600) + if err != nil { + return nil, err + } + if err := unix.Flock(int(lockFile.Fd()), unix.LOCK_EX); err != nil { + _ = lockFile.Close() + return nil, err + } + return lockFile, nil +} + +func releaseFileLock(lockFile *os.File) error { + err := unix.Flock(int(lockFile.Fd()), unix.LOCK_UN) + closeErr := lockFile.Close() + if err != nil { + return err + } + return closeErr +} diff --git a/internal/mcp/token_lock_windows.go b/internal/mcp/token_lock_windows.go new file mode 100644 index 0000000..2a62f16 --- /dev/null +++ b/internal/mcp/token_lock_windows.go @@ -0,0 +1,41 @@ +//go:build windows + +package mcp + +import ( + "os" + + "golang.org/x/sys/windows" +) + +func acquireFileLock(path string) (*os.File, error) { + lockFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0600) + if err != nil { + return nil, err + } + + var overlapped windows.Overlapped + if err := windows.LockFileEx( + windows.Handle(lockFile.Fd()), + windows.LOCKFILE_EXCLUSIVE_LOCK, + 0, + 1, + 0, + &overlapped, + ); err != nil { + _ = lockFile.Close() + return nil, err + } + + return lockFile, nil +} + +func releaseFileLock(lockFile *os.File) error { + var overlapped windows.Overlapped + err := windows.UnlockFileEx(windows.Handle(lockFile.Fd()), 0, 1, 0, &overlapped) + closeErr := lockFile.Close() + if err != nil { + return err + } + return closeErr +} diff --git a/internal/mcp/token_store.go b/internal/mcp/token_store.go index 00db4dc..d2d6154 100644 --- a/internal/mcp/token_store.go +++ b/internal/mcp/token_store.go @@ -9,31 +9,28 @@ import ( "sync" "time" - "github.com/lox/notion-cli/internal/profile" + "github.com/lox/notion-cli/internal/config" "github.com/mark3labs/mcp-go/client/transport" ) var ErrNoToken = errors.New("no token available") +var ( + refreshLocksMu sync.Mutex + refreshLocks = map[string]*sync.Mutex{} +) + type FileTokenStore struct { path string mu sync.RWMutex } -// NewFileTokenStore returns a token store backed by the default profile's -// token.json, matching the legacy behavior before multi-profile support. -func NewFileTokenStore() (*FileTokenStore, error) { - return NewFileTokenStoreForProfile(profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault}) -} - -// NewFileTokenStoreForProfile returns a token store rooted at the given -// profile's on-disk location. -func NewFileTokenStoreForProfile(p profile.Profile) (*FileTokenStore, error) { - path, err := profile.TokenPath(p) +func NewFileTokenStore(profile string) (*FileTokenStore, error) { + paths, err := config.PathsForProfile(profile) if err != nil { return nil, err } - return &FileTokenStore{path: path}, nil + return &FileTokenStore{path: paths.TokenPath}, nil } func (s *FileTokenStore) GetToken(ctx context.Context) (*transport.Token, error) { @@ -93,12 +90,7 @@ func (s *FileTokenStore) SaveToken(ctx context.Context, token *transport.Token) ClientID: existing.ClientID, } - data, err := json.MarshalIndent(stored, "", " ") - if err != nil { - return err - } - - return os.WriteFile(s.path, data, 0600) + return s.writeStoredToken(ctx, stored) } func (s *FileTokenStore) Clear() error { @@ -115,6 +107,10 @@ func (s *FileTokenStore) Path() string { return s.path } +func (s *FileTokenStore) LockPath() string { + return s.path + ".lock" +} + type storedToken struct { AccessToken string `json:"access_token"` TokenType string `json:"token_type"` @@ -169,10 +165,89 @@ func (s *FileTokenStore) SaveClientID(ctx context.Context, clientID string) erro stored.ClientID = clientID - data, err = json.MarshalIndent(stored, "", " ") + return s.writeStoredToken(ctx, stored) +} + +func (s *FileTokenStore) writeStoredToken(ctx context.Context, stored storedToken) error { + if err := ctx.Err(); err != nil { + return err + } + + dir := filepath.Dir(s.path) + if err := os.MkdirAll(dir, 0700); err != nil { + return err + } + + data, err := json.MarshalIndent(stored, "", " ") if err != nil { return err } - return os.WriteFile(s.path, data, 0600) + tmp, err := os.CreateTemp(dir, ".token-*.json") + if err != nil { + return err + } + tmpPath := tmp.Name() + cleanup := true + defer func() { + if cleanup { + _ = os.Remove(tmpPath) + } + }() + + if err := tmp.Chmod(0600); err != nil { + _ = tmp.Close() + return err + } + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + + if err := os.Rename(tmpPath, s.path); err != nil { + return err + } + cleanup = false + return nil +} + +func (s *FileTokenStore) WithLock(ctx context.Context, fn func() error) error { + if err := ctx.Err(); err != nil { + return err + } + + processLock := processRefreshLock(s.LockPath()) + processLock.Lock() + defer processLock.Unlock() + + dir := filepath.Dir(s.path) + if err := os.MkdirAll(dir, 0700); err != nil { + return err + } + + lockFile, err := acquireFileLock(s.LockPath()) + if err != nil { + return err + } + defer func() { _ = releaseFileLock(lockFile) }() + + if err := ctx.Err(); err != nil { + return err + } + return fn() +} + +func processRefreshLock(path string) *sync.Mutex { + refreshLocksMu.Lock() + defer refreshLocksMu.Unlock() + + lock, ok := refreshLocks[path] + if !ok { + lock = &sync.Mutex{} + refreshLocks[path] = lock + } + return lock } diff --git a/internal/mcp/token_store_test.go b/internal/mcp/token_store_test.go new file mode 100644 index 0000000..c9c3517 --- /dev/null +++ b/internal/mcp/token_store_test.go @@ -0,0 +1,73 @@ +package mcp + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/mark3labs/mcp-go/client/transport" +) + +func TestNewFileTokenStoreUsesProfilePath(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + store, err := NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + + if got := filepath.Base(store.Path()); got != "token.json" { + t.Fatalf("token filename = %q, want token.json", got) + } + if !strings.Contains(store.Path(), filepath.Join("profiles", "work")) { + t.Fatalf("token path = %q, want profiles/work segment", store.Path()) + } +} + +func TestSaveTokenWritesAtomicallyAndPreservesClientID(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + store, err := NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + + if err := store.SaveClientID(context.Background(), "client-123"); err != nil { + t.Fatalf("SaveClientID: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + AccessToken: "access-123", + TokenType: "bearer", + RefreshToken: "refresh-123", + ExpiresAt: time.Now().Add(time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + + info, err := os.Stat(store.Path()) + if err != nil { + t.Fatalf("stat token file: %v", err) + } + if got := info.Mode().Perm(); got != 0600 { + t.Fatalf("token file mode = %o, want 0600", got) + } + + data, err := os.ReadFile(store.Path()) + if err != nil { + t.Fatalf("read token file: %v", err) + } + var stored storedToken + if err := json.Unmarshal(data, &stored); err != nil { + t.Fatalf("unmarshal token: %v", err) + } + if stored.ClientID != "client-123" { + t.Fatalf("client ID = %q, want client-123", stored.ClientID) + } + if stored.RefreshToken != "refresh-123" { + t.Fatalf("refresh token = %q, want refresh-123", stored.RefreshToken) + } +} diff --git a/internal/profile/profile.go b/internal/profile/profile.go deleted file mode 100644 index 9791e04..0000000 --- a/internal/profile/profile.go +++ /dev/null @@ -1,202 +0,0 @@ -// Package profile resolves which Notion account a notion-cli invocation -// targets and returns the on-disk locations for that account's credentials. -package profile - -import ( - "encoding/json" - "errors" - "fmt" - "os" - "path/filepath" - "regexp" - "strings" -) - -const ( - // EnvVar is the environment variable that selects a profile when no - // --profile flag is passed. - EnvVar = "NOTION_CLI_PROFILE" - // DefaultName is the implicit profile name used when the caller has - // not selected one and legacy top-level credentials exist. - DefaultName = "default" - // SettingsFileName is the cross-profile settings file at the top of - // the notion-cli config dir. - SettingsFileName = "settings.json" - // TokenFileName is the OAuth token filename inside a profile dir. - TokenFileName = "token.json" - // ConfigFileName is the API config filename inside a profile dir. - ConfigFileName = "config.json" - configDirName = "notion-cli" -) - -// Source records where the active profile came from, so auth status can -// tell the user how it resolved. -type Source string - -const ( - SourceFlag Source = "--profile flag" - SourceEnv Source = EnvVar - SourceSettings Source = SettingsFileName - SourceDefault Source = "implicit default" -) - -// Profile identifies a selected notion-cli profile and where it was -// selected from. -type Profile struct { - Name string - Source Source -} - -// ErrNoProfile is returned by Resolve when no profile is selected by flag, -// env, or settings, and no legacy top-level credentials exist to fall back -// to. -var ErrNoProfile = errors.New("no profile specified; pass --profile or set " + EnvVar) - -// nameRE enforces the accepted profile-name alphabet: lowercase ASCII, -// leading letter or digit, `_` and `-` allowed in the tail. -var nameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]*$`) - -// Validate reports whether name is acceptable as a profile identifier. -func Validate(name string) error { - if name == "" { - return errors.New("profile name must not be empty") - } - if !nameRE.MatchString(name) { - return fmt.Errorf("invalid profile name %q: must match %s", name, nameRE.String()) - } - return nil -} - -// Resolve picks the active profile using the precedence: -// -// 1. --profile flag value -// 2. NOTION_CLI_PROFILE environment variable -// 3. default_profile in ~/.config/notion-cli/settings.json -// 4. legacy implicit default (top-level ~/.config/notion-cli/{token,config}.json) -// -// If none of those apply, Resolve returns ErrNoProfile. -func Resolve(flagValue string) (Profile, error) { - if v := strings.TrimSpace(flagValue); v != "" { - if err := Validate(v); err != nil { - return Profile{}, err - } - return Profile{Name: v, Source: SourceFlag}, nil - } - if v := strings.TrimSpace(os.Getenv(EnvVar)); v != "" { - if err := Validate(v); err != nil { - return Profile{}, fmt.Errorf("%s: %w", EnvVar, err) - } - return Profile{Name: v, Source: SourceEnv}, nil - } - if v, err := loadSettingsDefault(); err != nil { - return Profile{}, err - } else if v != "" { - if err := Validate(v); err != nil { - return Profile{}, fmt.Errorf("settings.json default_profile: %w", err) - } - return Profile{Name: v, Source: SourceSettings}, nil - } - if legacyTopLevelExists() { - return Profile{Name: DefaultName, Source: SourceDefault}, nil - } - return Profile{}, ErrNoProfile -} - -// TokenRoot returns the top-level directory that contains OAuth token files. -// notion-cli historically stores tokens under $HOME/.config/notion-cli on -// every OS, so we keep that path here to avoid orphaning existing tokens on -// macOS where UserConfigDir would otherwise resolve to -// ~/Library/Application Support. -func TokenRoot() (string, error) { - home, err := os.UserHomeDir() - if err != nil { - return "", err - } - return filepath.Join(home, ".config", configDirName), nil -} - -// ConfigRoot returns the top-level directory that contains API config and -// cross-profile settings files. This follows os.UserConfigDir, matching the -// existing config.json layout. -func ConfigRoot() (string, error) { - base, err := os.UserConfigDir() - if err != nil { - return "", err - } - return filepath.Join(base, configDirName), nil -} - -// TokenPath returns the OAuth token file path for the given profile. The -// implicit default profile keeps using the top-level token.json. -func TokenPath(p Profile) (string, error) { - root, err := TokenRoot() - if err != nil { - return "", err - } - if p.Name == DefaultName { - return filepath.Join(root, TokenFileName), nil - } - return filepath.Join(root, p.Name, TokenFileName), nil -} - -// ConfigPath returns the API config file path for the given profile. The -// implicit default profile keeps using the top-level config.json. -func ConfigPath(p Profile) (string, error) { - root, err := ConfigRoot() - if err != nil { - return "", err - } - if p.Name == DefaultName { - return filepath.Join(root, ConfigFileName), nil - } - return filepath.Join(root, p.Name, ConfigFileName), nil -} - -// SettingsPath returns the cross-profile settings.json path. It lives next -// to the API config files. -func SettingsPath() (string, error) { - root, err := ConfigRoot() - if err != nil { - return "", err - } - return filepath.Join(root, SettingsFileName), nil -} - -type settingsFile struct { - DefaultProfile string `json:"default_profile,omitempty"` -} - -func loadSettingsDefault() (string, error) { - path, err := SettingsPath() - if err != nil { - return "", err - } - data, err := os.ReadFile(path) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - return "", nil - } - return "", fmt.Errorf("read %s: %w", path, err) - } - var s settingsFile - if err := json.Unmarshal(data, &s); err != nil { - return "", fmt.Errorf("parse %s: %w", path, err) - } - return strings.TrimSpace(s.DefaultProfile), nil -} - -func legacyTopLevelExists() bool { - tokenRoot, err := TokenRoot() - if err == nil { - if _, err := os.Stat(filepath.Join(tokenRoot, TokenFileName)); err == nil { - return true - } - } - configRoot, err := ConfigRoot() - if err == nil { - if _, err := os.Stat(filepath.Join(configRoot, ConfigFileName)); err == nil { - return true - } - } - return false -} diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go deleted file mode 100644 index a4553dc..0000000 --- a/internal/profile/profile_test.go +++ /dev/null @@ -1,207 +0,0 @@ -package profile - -import ( - "encoding/json" - "errors" - "os" - "path/filepath" - "testing" -) - -// isolateProfileDirs redirects HOME and UserConfigDir to a temp dir so the -// resolver only sees state we set up for the test. -func isolateProfileDirs(t *testing.T) string { - t.Helper() - tmp := t.TempDir() - t.Setenv("HOME", tmp) - // os.UserConfigDir on Linux reads XDG_CONFIG_HOME, on macOS it reads - // nothing and composes from HOME. Set both so the lookup is deterministic. - t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmp, ".config")) - return tmp -} - -func TestValidateRejectsEmpty(t *testing.T) { - if err := Validate(""); err == nil { - t.Fatalf("Validate(\"\") returned nil, expected error") - } -} - -func TestValidateRejectsBadCharacters(t *testing.T) { - bad := []string{ - "Work", // uppercase - "work/sub", // slash - "-leading-dash", // leading dash - "_leading-under", // leading underscore - "has spaces", // whitespace - "has.dot", // dot - "has!bang", // punctuation - "../escape", // path traversal - "work/../other", // path traversal - "", // empty - } - for _, name := range bad { - if err := Validate(name); err == nil { - t.Errorf("Validate(%q) returned nil, expected error", name) - } - } -} - -func TestValidateAcceptsCommonNames(t *testing.T) { - good := []string{"work", "home", "default", "brianle", "proj1", "proj_1", "a-b-c", "0xble"} - for _, name := range good { - if err := Validate(name); err != nil { - t.Errorf("Validate(%q) returned %v, expected nil", name, err) - } - } -} - -func TestResolvePrefersFlagOverEnvAndSettings(t *testing.T) { - isolateProfileDirs(t) - t.Setenv(EnvVar, "env-profile") - writeSettings(t, "settings-profile") - - p, err := Resolve("flag-profile") - if err != nil { - t.Fatalf("Resolve: %v", err) - } - if p.Name != "flag-profile" || p.Source != SourceFlag { - t.Fatalf("Resolve = %+v, want flag-profile/SourceFlag", p) - } -} - -func TestResolvePrefersEnvOverSettings(t *testing.T) { - isolateProfileDirs(t) - t.Setenv(EnvVar, "env-profile") - writeSettings(t, "settings-profile") - - p, err := Resolve("") - if err != nil { - t.Fatalf("Resolve: %v", err) - } - if p.Name != "env-profile" || p.Source != SourceEnv { - t.Fatalf("Resolve = %+v, want env-profile/SourceEnv", p) - } -} - -func TestResolveUsesSettingsWhenFlagAndEnvUnset(t *testing.T) { - isolateProfileDirs(t) - t.Setenv(EnvVar, "") - writeSettings(t, "settings-profile") - - p, err := Resolve("") - if err != nil { - t.Fatalf("Resolve: %v", err) - } - if p.Name != "settings-profile" || p.Source != SourceSettings { - t.Fatalf("Resolve = %+v, want settings-profile/SourceSettings", p) - } -} - -func TestResolveFallsBackToImplicitDefaultWhenLegacyFilesExist(t *testing.T) { - tmp := isolateProfileDirs(t) - t.Setenv(EnvVar, "") - - tokenRoot := filepath.Join(tmp, ".config", configDirName) - if err := os.MkdirAll(tokenRoot, 0o700); err != nil { - t.Fatalf("MkdirAll: %v", err) - } - if err := os.WriteFile(filepath.Join(tokenRoot, TokenFileName), []byte(`{"access_token":"legacy"}`), 0o600); err != nil { - t.Fatalf("WriteFile: %v", err) - } - - p, err := Resolve("") - if err != nil { - t.Fatalf("Resolve: %v", err) - } - if p.Name != DefaultName || p.Source != SourceDefault { - t.Fatalf("Resolve = %+v, want default/SourceDefault", p) - } -} - -func TestResolveReturnsErrNoProfileWhenNothingResolves(t *testing.T) { - isolateProfileDirs(t) - t.Setenv(EnvVar, "") - - _, err := Resolve("") - if !errors.Is(err, ErrNoProfile) { - t.Fatalf("Resolve = %v, want ErrNoProfile", err) - } -} - -func TestResolveValidatesFlagValue(t *testing.T) { - isolateProfileDirs(t) - t.Setenv(EnvVar, "") - - if _, err := Resolve("BadName"); err == nil { - t.Fatalf("Resolve(\"BadName\") returned nil, expected validation error") - } -} - -func TestResolveValidatesEnvValue(t *testing.T) { - isolateProfileDirs(t) - t.Setenv(EnvVar, "BadName") - - if _, err := Resolve(""); err == nil { - t.Fatalf("Resolve with bad env returned nil, expected validation error") - } -} - -func TestTokenPathDefaultPointsToLegacyTopLevel(t *testing.T) { - tmp := isolateProfileDirs(t) - - path, err := TokenPath(Profile{Name: DefaultName, Source: SourceDefault}) - if err != nil { - t.Fatalf("TokenPath: %v", err) - } - want := filepath.Join(tmp, ".config", configDirName, TokenFileName) - if path != want { - t.Fatalf("TokenPath = %q, want %q", path, want) - } -} - -func TestTokenPathNamedProfileUsesSubdir(t *testing.T) { - tmp := isolateProfileDirs(t) - - path, err := TokenPath(Profile{Name: "work", Source: SourceFlag}) - if err != nil { - t.Fatalf("TokenPath: %v", err) - } - want := filepath.Join(tmp, ".config", configDirName, "work", TokenFileName) - if path != want { - t.Fatalf("TokenPath = %q, want %q", path, want) - } -} - -func TestConfigPathNamedProfileIsolatedFromDefault(t *testing.T) { - isolateProfileDirs(t) - - defaultPath, err := ConfigPath(Profile{Name: DefaultName}) - if err != nil { - t.Fatalf("ConfigPath default: %v", err) - } - workPath, err := ConfigPath(Profile{Name: "work"}) - if err != nil { - t.Fatalf("ConfigPath work: %v", err) - } - if defaultPath == workPath { - t.Fatalf("default and named profile share config path: %q", defaultPath) - } -} - -func writeSettings(t *testing.T, defaultProfile string) { - t.Helper() - path, err := SettingsPath() - if err != nil { - t.Fatalf("SettingsPath: %v", err) - } - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - t.Fatalf("MkdirAll: %v", err) - } - data, err := json.Marshal(settingsFile{DefaultProfile: defaultProfile}) - if err != nil { - t.Fatalf("Marshal: %v", err) - } - if err := os.WriteFile(path, data, 0o600); err != nil { - t.Fatalf("WriteFile: %v", err) - } -} diff --git a/main.go b/main.go index 9d258c0..9ef9ab7 100644 --- a/main.go +++ b/main.go @@ -1,15 +1,12 @@ package main import ( - "errors" - "fmt" "os" - "strings" "github.com/alecthomas/kong" "github.com/lox/notion-cli/cmd" "github.com/lox/notion-cli/internal/cli" - "github.com/lox/notion-cli/internal/profile" + "github.com/lox/notion-cli/internal/config" ) var version = "dev" @@ -27,33 +24,18 @@ func main() { kong.UsageOnError(), kong.Vars{"version": version}, ) - - active, profileErr := profile.Resolve(c.Profile) - if profileErr != nil { - hasAccessToken := strings.TrimSpace(c.Token) != "" - if !hasAccessToken || !errors.Is(profileErr, profile.ErrNoProfile) { - if errors.Is(profileErr, profile.ErrNoProfile) { - _, _ = fmt.Fprintln(os.Stderr, "\u2717 No profile specified. Pass --profile or set NOTION_CLI_PROFILE.") - } else { - _, _ = fmt.Fprintf(os.Stderr, "\u2717 %s\n", profileErr) - } - os.Exit(1) - } - // Headless path: NOTION_ACCESS_TOKEN authenticates MCP directly, so - // the profile gate can be skipped. Profile-scoped operations (like - // auth api setup) will still use the implicit default layout. - active = profile.Profile{Name: profile.DefaultName, Source: profile.SourceDefault} - } + profile, err := config.ResolveSelectedProfile(c.Profile) + ctx.FatalIfErrorf(err) cli.SetAccessToken(c.Token) - - runErr := ctx.Run(&cmd.Context{ + cli.SetProfile(profile) + err = ctx.Run(&cmd.Context{ + Profile: profile, Token: c.Token, APIToken: c.APIToken, APIBaseURL: c.APIBaseURL, APINotionVersion: c.APINotionVersion, - Profile: active, }) - ctx.FatalIfErrorf(runErr) + ctx.FatalIfErrorf(err) os.Exit(0) } diff --git a/skills/notion/SKILL.md b/skills/notion/SKILL.md index b16c074..d67f81d 100644 --- a/skills/notion/SKILL.md +++ b/skills/notion/SKILL.md @@ -50,15 +50,16 @@ For CI/headless environments, set `NOTION_API_TOKEN`. ### Multiple accounts -Every command accepts `--profile ` (or `NOTION_CLI_PROFILE`) to target a specific Notion account. Named profiles keep credentials isolated under `~/.config/notion-cli//`; the implicit default profile uses the existing top-level paths. +Every command accepts `--profile ` (or `NOTION_CLI_PROFILE`) to target a specific Notion account. Named profiles keep credentials isolated under `~/.config/notion-cli/profiles//`; the implicit default profile uses the existing top-level paths. ```bash notion-cli auth login --profile work notion-cli page list --profile work export NOTION_CLI_PROFILE=work # pin for the shell session +notion-cli auth use work # make work the default profile ``` -Resolution precedence: `--profile` > `NOTION_CLI_PROFILE` > `default_profile` in `~/.config/notion-cli/settings.json` > implicit top-level default. If none resolve, the CLI fails with `No profile specified.` instead of acting silently. +Resolution precedence: `--profile` > `NOTION_CLI_PROFILE` > `notion-cli auth use ` > implicit default profile. ## Available Commands From 719e0816c39f2b0801bb0b5ab0cd04f850a4327c Mon Sep 17 00:00:00 2001 From: Brian Le Date: Tue, 5 May 2026 11:53:06 -0400 Subject: [PATCH 09/15] test(auth): isolate profile config dirs --- cmd/auth_test.go | 15 ++++++++++++--- internal/config/config_test.go | 28 ++++++++++++++++++---------- internal/mcp/oauth_test.go | 17 +++++++++++++---- internal/mcp/token_store_test.go | 4 ++-- 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/cmd/auth_test.go b/cmd/auth_test.go index 3d5a2df..e779317 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "path/filepath" "strings" "testing" "time" @@ -11,8 +12,16 @@ import ( "github.com/mark3labs/mcp-go/client/transport" ) +func isolateAuthConfig(t *testing.T) { + t.Helper() + + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) +} + func TestAuthUsePersistsActiveProfile(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateAuthConfig(t) cmd := &AuthUseCmd{Profile: "work"} stdout := captureStdout(t, func() { @@ -34,7 +43,7 @@ func TestAuthUsePersistsActiveProfile(t *testing.T) { } func TestAuthListJSONShowsProfilesAndActiveState(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateAuthConfig(t) if err := config.SetActiveProfile("work"); err != nil { t.Fatalf("SetActiveProfile: %v", err) @@ -76,7 +85,7 @@ func TestAuthListJSONShowsProfilesAndActiveState(t *testing.T) { } func TestAuthStatusJSONReportsMissingTokenForProfile(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateAuthConfig(t) cmd := &AuthStatusCmd{JSON: true} stdout := captureStdout(t, func() { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5570ca2..a6a3f1d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -8,8 +8,16 @@ import ( "testing" ) +func isolateConfigDir(t *testing.T) { + t.Helper() + + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) +} + func TestLoadWithMetaDefaults(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) loaded, err := LoadWithMeta(APIOverrides{}) if err != nil { @@ -30,7 +38,7 @@ func TestLoadWithMetaDefaults(t *testing.T) { } func TestLoadWithMetaReportsConfigTokenSource(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) if err := SetAPIToken("secret-token"); err != nil { t.Fatalf("SetAPIToken: %v", err) } @@ -48,7 +56,7 @@ func TestLoadWithMetaReportsConfigTokenSource(t *testing.T) { } func TestLoadWithMetaEnvOverrideWins(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) if err := SetAPIToken("config-token"); err != nil { t.Fatalf("SetAPIToken: %v", err) } @@ -75,7 +83,7 @@ func TestLoadWithMetaEnvOverrideWins(t *testing.T) { } func TestUnsetAPITokenClearsStoredToken(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) if err := SetAPIToken("secret-token"); err != nil { t.Fatalf("SetAPIToken: %v", err) } @@ -96,7 +104,7 @@ func TestUnsetAPITokenClearsStoredToken(t *testing.T) { } func TestSaveSecuresConfigFile(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) cfg := Default() cfg.API.Token = "secret-token" @@ -127,7 +135,7 @@ func TestSaveSecuresConfigFile(t *testing.T) { } func TestPathsForProfileDefaultAndNamed(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) defaultPaths, err := PathsForProfile("") if err != nil { @@ -159,7 +167,7 @@ func TestPathsForProfileDefaultAndNamed(t *testing.T) { } func TestProfileSpecificAPITokensAreIsolated(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) if err := SetAPIToken("default-token"); err != nil { t.Fatalf("SetAPIToken default: %v", err) @@ -207,7 +215,7 @@ func TestResolveProfileAllowsEmailNames(t *testing.T) { } func TestResolveSelectedProfileUsesActiveStateWhenUnset(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) if err := SetActiveProfile("work"); err != nil { t.Fatalf("SetActiveProfile: %v", err) } @@ -222,7 +230,7 @@ func TestResolveSelectedProfileUsesActiveStateWhenUnset(t *testing.T) { } func TestResolveSelectedProfilePrefersExplicitValue(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) if err := SetActiveProfile("work"); err != nil { t.Fatalf("SetActiveProfile: %v", err) } @@ -237,7 +245,7 @@ func TestResolveSelectedProfilePrefersExplicitValue(t *testing.T) { } func TestListProfilesIncludesActiveDefaultAndNamedProfiles(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateConfigDir(t) if err := SetActiveProfile("work"); err != nil { t.Fatalf("SetActiveProfile: %v", err) } diff --git a/internal/mcp/oauth_test.go b/internal/mcp/oauth_test.go index 1a55d37..0ea844c 100644 --- a/internal/mcp/oauth_test.go +++ b/internal/mcp/oauth_test.go @@ -3,6 +3,7 @@ package mcp import ( "context" "fmt" + "path/filepath" "strings" "sync" "sync/atomic" @@ -12,8 +13,16 @@ import ( "github.com/mark3labs/mcp-go/client/transport" ) +func isolateMCPConfig(t *testing.T) { + t.Helper() + + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) +} + func TestRefreshTokenSkipsRefreshWhenAnotherCallerAlreadyRefreshed(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateMCPConfig(t) store, err := NewFileTokenStore("work") if err != nil { @@ -68,7 +77,7 @@ func TestRefreshTokenSkipsRefreshWhenAnotherCallerAlreadyRefreshed(t *testing.T) } func TestRefreshTokenInvalidGrantUsesNewerSavedToken(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateMCPConfig(t) store, err := NewFileTokenStore("work") if err != nil { @@ -103,7 +112,7 @@ func TestRefreshTokenInvalidGrantUsesNewerSavedToken(t *testing.T) { } func TestRefreshTokenForcesRefreshWhenTokenIsFresh(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateMCPConfig(t) store, err := NewFileTokenStore("work") if err != nil { @@ -150,7 +159,7 @@ func TestRefreshTokenForcesRefreshWhenTokenIsFresh(t *testing.T) { } func TestRefreshTokenInvalidGrantRequiresLoginWhenNoNewerTokenExists(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateMCPConfig(t) store, err := NewFileTokenStore("work") if err != nil { diff --git a/internal/mcp/token_store_test.go b/internal/mcp/token_store_test.go index c9c3517..9004e19 100644 --- a/internal/mcp/token_store_test.go +++ b/internal/mcp/token_store_test.go @@ -13,7 +13,7 @@ import ( ) func TestNewFileTokenStoreUsesProfilePath(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateMCPConfig(t) store, err := NewFileTokenStore("work") if err != nil { @@ -29,7 +29,7 @@ func TestNewFileTokenStoreUsesProfilePath(t *testing.T) { } func TestSaveTokenWritesAtomicallyAndPreservesClientID(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + isolateMCPConfig(t) store, err := NewFileTokenStore("work") if err != nil { From e3d2cc15a670294372b81ca6cb8cab7e2f682625 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Tue, 5 May 2026 12:01:30 -0400 Subject: [PATCH 10/15] fix(auth): address profile refresh review feedback --- README.md | 2 +- cmd/auth.go | 4 ++-- internal/config/config.go | 21 ++++++++++++++++++- internal/config/config_test.go | 3 +++ internal/mcp/oauth.go | 11 +++++++++- internal/mcp/oauth_test.go | 37 ++++++++++++++++++++++++++++++++++ 6 files changed, 73 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index bb13739..66dd5b4 100644 --- a/README.md +++ b/README.md @@ -199,7 +199,7 @@ Profile resolution, highest priority first: 3. Active profile from `notion-cli auth use ` 4. Implicit default profile -The default profile keeps using the existing top-level `~/.config/notion-cli/{token,config}.json` files, so existing single-account installs need no migration. `notion-cli auth use ` stores the active profile in `~/.config/notion-cli/state.json`. +The default profile keeps using the existing OAuth token path, so existing single-account installs need no migration. `notion-cli auth use ` stores the active profile in the cross-profile config directory. Profile names may contain letters, numbers, at signs, dots, underscores, and hyphens. diff --git a/cmd/auth.go b/cmd/auth.go index 8238138..db573c4 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -206,8 +206,8 @@ func (c *AuthListCmd) Run(ctx *Context) error { } _, _ = labelStyle.Print(" Token path: ") fmt.Println(row.TokenPath) - _, _ = labelStyle.Print(" Config path:") - fmt.Println(" " + row.ConfigPath) + _, _ = labelStyle.Print(" Config path: ") + fmt.Println(row.ConfigPath) if i < len(rows)-1 { fmt.Println() } diff --git a/internal/config/config.go b/internal/config/config.go index d0d6d5b..e6598f1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -123,14 +123,33 @@ func PathsForProfile(profile string) (ProfilePaths, error) { if resolvedProfile != defaultProfileName { profileDir = filepath.Join(baseDir, profilesDirName, resolvedProfile) } + tokenPath := filepath.Join(profileDir, tokenFileName) + if resolvedProfile == defaultProfileName { + tokenPath, err = legacyDefaultTokenPath() + if err != nil { + return ProfilePaths{}, err + } + } return ProfilePaths{ Profile: resolvedProfile, ConfigPath: filepath.Join(profileDir, configFileName), - TokenPath: filepath.Join(profileDir, tokenFileName), + TokenPath: tokenPath, }, nil } +// legacyDefaultTokenPath preserves the pre-profile OAuth token location. +// API config files use ConfigDir, but upstream OAuth tokens historically +// lived under ~/.config/notion-cli even on platforms where os.UserConfigDir +// resolves somewhere else. +func legacyDefaultTokenPath() (string, error) { + homeDir, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(homeDir, ".config", configDirName, tokenFileName), nil +} + func DefaultProfile() string { return defaultProfileName } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a6a3f1d..e3385e0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -150,6 +150,9 @@ func TestPathsForProfileDefaultAndNamed(t *testing.T) { if got := filepath.Base(defaultPaths.TokenPath); got != tokenFileName { t.Fatalf("default token filename = %q, want %q", got, tokenFileName) } + if !strings.Contains(defaultPaths.TokenPath, filepath.Join(".config", configDirName, tokenFileName)) { + t.Fatalf("default token path = %q, want legacy .config/notion-cli path", defaultPaths.TokenPath) + } workPaths, err := PathsForProfile("work") if err != nil { diff --git a/internal/mcp/oauth.go b/internal/mcp/oauth.go index 54e2019..5465178 100644 --- a/internal/mcp/oauth.go +++ b/internal/mcp/oauth.go @@ -232,7 +232,7 @@ func refreshTokenLocked(ctx context.Context, tokenStore *FileTokenStore, force b if err != nil { if isInvalidGrantError(err) { latest, latestErr := tokenStore.GetToken(ctx) - if latestErr == nil && tokenFresh(latest) { + if latestErr == nil && tokenFresh(latest) && !sameToken(latest, token) { refreshed = latest return nil } @@ -262,6 +262,15 @@ func tokenFresh(token *transport.Token) bool { (token.ExpiresAt.IsZero() || token.ExpiresAt.After(time.Now().Add(refreshSkew))) } +func sameToken(a, b *transport.Token) bool { + if a == nil || b == nil { + return a == b + } + return a.AccessToken == b.AccessToken && + a.RefreshToken == b.RefreshToken && + a.ExpiresAt.Equal(b.ExpiresAt) +} + func isInvalidGrantError(err error) bool { if err == nil { return false diff --git a/internal/mcp/oauth_test.go b/internal/mcp/oauth_test.go index 0ea844c..c76baa0 100644 --- a/internal/mcp/oauth_test.go +++ b/internal/mcp/oauth_test.go @@ -185,6 +185,43 @@ func TestRefreshTokenInvalidGrantRequiresLoginWhenNoNewerTokenExists(t *testing. } } +func TestRefreshTokenInvalidGrantDoesNotAcceptSameFreshToken(t *testing.T) { + isolateMCPConfig(t) + + store, err := NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + if err := store.SaveClientID(context.Background(), "client-123"); err != nil { + t.Fatalf("SaveClientID: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + AccessToken: "fresh-access", + TokenType: "bearer", + RefreshToken: "fresh-refresh", + ExpiresAt: time.Now().Add(time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + + oldRefresh := refreshOAuthToken + t.Cleanup(func() { + refreshOAuthToken = oldRefresh + }) + + refreshOAuthToken = func(context.Context, *FileTokenStore, *transport.Token) (*transport.Token, error) { + return nil, fmt.Errorf("refresh token: %w", transport.OAuthError{ErrorCode: "invalid_grant"}) + } + + _, err = RefreshToken(context.Background(), store) + if err == nil { + t.Fatal("RefreshToken returned nil error, want browser login required") + } + if got := err.Error(); !strings.Contains(got, "browser login required") { + t.Fatalf("error = %q, want browser login required", got) + } +} + func saveExpiredToken(t *testing.T, store *FileTokenStore) { t.Helper() From bee7885d212ce40c001279a3ef926cf8ac1a21e9 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Tue, 5 May 2026 12:17:21 -0400 Subject: [PATCH 11/15] fix(auth): reject non-portable profile names --- README.md | 2 +- internal/config/config.go | 29 +++++++++++++++++++++++------ internal/config/config_test.go | 27 +++++++++++++++++++-------- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 66dd5b4..a560e67 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,7 @@ Profile resolution, highest priority first: The default profile keeps using the existing OAuth token path, so existing single-account installs need no migration. `notion-cli auth use ` stores the active profile in the cross-profile config directory. -Profile names may contain letters, numbers, at signs, dots, underscores, and hyphens. +Profile names must start and end with a lowercase ASCII letter or number. They may contain lowercase letters, numbers, at signs, dots, underscores, and hyphens. Named profiles store their credentials under `~/.config/notion-cli/profiles//{token,config}.json`. diff --git a/internal/config/config.go b/internal/config/config.go index e6598f1..f38cdc8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,7 +8,6 @@ import ( "path/filepath" "slices" "strings" - "unicode" ) const ( @@ -159,18 +158,36 @@ func ResolveProfile(profile string) (string, error) { if normalized == "" { return defaultProfileName, nil } - if normalized == "." || normalized == ".." { - return "", fmt.Errorf("invalid profile %q", profile) + + runes := []rune(normalized) + if !isProfileEndpoint(runes[0]) || !isProfileEndpoint(runes[len(runes)-1]) { + return "", fmt.Errorf("invalid profile %q: start and end with a lowercase letter or number", profile) } - for _, r := range normalized { - if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '.' || r == '_' || r == '-' || r == '@' { + for _, r := range runes { + if isProfileChar(r) { continue } - return "", fmt.Errorf("invalid profile %q: use letters, numbers, at sign, dot, underscore, and hyphen", profile) + return "", fmt.Errorf("invalid profile %q: use lowercase letters, numbers, at sign, dot, underscore, and hyphen", profile) } return normalized, nil } +func isProfileEndpoint(r rune) bool { + return isLowercaseASCII(r) || isDigitASCII(r) +} + +func isProfileChar(r rune) bool { + return isLowercaseASCII(r) || isDigitASCII(r) || r == '.' || r == '_' || r == '-' || r == '@' +} + +func isLowercaseASCII(r rune) bool { + return r >= 'a' && r <= 'z' +} + +func isDigitASCII(r rune) bool { + return r >= '0' && r <= '9' +} + func LoadState() (State, error) { path, err := StatePath() if err != nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e3385e0..026d068 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -200,20 +200,31 @@ func TestProfileSpecificAPITokensAreIsolated(t *testing.T) { } func TestResolveProfileRejectsInvalidNames(t *testing.T) { - for _, value := range []string{"../oops", "work/team", "two words"} { + for _, value := range []string{ + "../oops", + "work/team", + "two words", + "Work", + ".work", + "work.", + "work-", + "ümlaut", + } { if _, err := ResolveProfile(value); err == nil { t.Fatalf("ResolveProfile(%q) should fail", value) } } } -func TestResolveProfileAllowsEmailNames(t *testing.T) { - got, err := ResolveProfile("brian@brianle.xyz") - if err != nil { - t.Fatalf("ResolveProfile: %v", err) - } - if got != "brian@brianle.xyz" { - t.Fatalf("profile = %q, want brian@brianle.xyz", got) +func TestResolveProfileAllowsPortableNames(t *testing.T) { + for _, value := range []string{"work", "work-1", "personal_2", "brian@brianle.xyz"} { + got, err := ResolveProfile(value) + if err != nil { + t.Fatalf("ResolveProfile(%q): %v", value, err) + } + if got != value { + t.Fatalf("profile = %q, want %q", got, value) + } } } From b01b357e263746de4ddac5b8958898003448dd6e Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 6 May 2026 11:01:07 -0400 Subject: [PATCH 12/15] fix(auth): treat refreshable token expiry as valid `auth status` and `auth list` previously reported `OAuth: expired` whenever the access token was past its expiry, even though the README already promises the CLI auto-refreshes on use when a refresh token is on file. The "expired" line was cosmetic only; commands kept working, but it caused agents and humans to react to a non-problem. Collapse the recoverable case into `valid`, and surface the truly broken case as `login_required` so the actionable distinction is preserved. Update the bundled skill's "Auth preflight" tip and auth command annotations accordingly. JSON shape: `oauth_status` now reports `valid` | `login_required` | `missing` instead of `valid` | `expired` | `missing`. The `authenticated` boolean continues to track whether the next command will succeed without re-login, which is now actually accurate. --- cmd/auth.go | 21 ++++++++++----- cmd/auth_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++ skills/notion/SKILL.md | 6 ++--- 3 files changed, 78 insertions(+), 10 deletions(-) diff --git a/cmd/auth.go b/cmd/auth.go index db573c4..f73dcc1 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -135,8 +135,8 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { switch status.OAuthStatus { case "valid": output.PrintSuccess("Authenticated") - case "expired": - output.PrintWarning("Token expired") + case "login_required": + output.PrintWarning("Login required") default: output.PrintWarning("Not authenticated") } @@ -151,7 +151,7 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { _, _ = labelStyle.Print("Expires: ") fmt.Println(status.OAuthExpiresAt.Format("2 Jan 2006 15:04")) } - if status.OAuthStatus == "missing" { + if status.OAuthStatus == "missing" || status.OAuthStatus == "login_required" { _, _ = fmt.Fprintln(os.Stdout, "Run 'notion-cli auth login' to authenticate this profile.") } @@ -234,7 +234,7 @@ func (c *AuthUseCmd) Run(ctx *Context) error { output.PrintSuccess("Active profile updated") fmt.Printf("Profile: %s\n", status.Profile) - if status.OAuthStatus == "missing" { + if status.OAuthStatus == "missing" || status.OAuthStatus == "login_required" { fmt.Println("Run 'notion-cli auth login' to authenticate this profile.") } if !status.HasAPIToken { @@ -524,10 +524,17 @@ func inspectProfileStatus(profile string) (authProfileStatus, error) { expiresAt := token.ExpiresAt status.OAuthExpiresAt = &expiresAt if status.HasOAuthToken { - if token.IsExpired() { - status.OAuthStatus = "expired" - } else { + switch { + case !token.IsExpired(): + status.OAuthStatus = "valid" + case strings.TrimSpace(token.RefreshToken) != "": + // Access token is past expiry but a refresh token is on file, + // so the next command will silently refresh. Surface this as + // valid; the underlying expiry is still in OAuthExpiresAt for + // callers that want it. status.OAuthStatus = "valid" + default: + status.OAuthStatus = "login_required" } } return status, nil diff --git a/cmd/auth_test.go b/cmd/auth_test.go index e779317..baa4b06 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -101,3 +101,64 @@ func TestAuthStatusJSONReportsMissingTokenForProfile(t *testing.T) { t.Fatalf("unexpected output: %s", stdout) } } + +func TestAuthStatusJSONReportsValidWhenAccessExpiredButRefreshAvailable(t *testing.T) { + isolateAuthConfig(t) + + store, err := mcp.NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + AccessToken: "stale-access", + TokenType: "Bearer", + RefreshToken: "rotating-refresh", + ExpiresAt: time.Now().Add(-time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + + cmd := &AuthStatusCmd{JSON: true} + stdout := captureStdout(t, func() { + if err := cmd.Run(&Context{Profile: "work"}); err != nil { + t.Fatalf("Run: %v", err) + } + }) + + if !strings.Contains(stdout, `"oauth_status": "valid"`) { + t.Fatalf("expected valid status when refresh token present, got: %s", stdout) + } + if !strings.Contains(stdout, `"authenticated": true`) { + t.Fatalf("expected authenticated true when refresh token present, got: %s", stdout) + } +} + +func TestAuthStatusJSONReportsLoginRequiredWithoutRefreshToken(t *testing.T) { + isolateAuthConfig(t) + + store, err := mcp.NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + AccessToken: "stale-access", + TokenType: "Bearer", + ExpiresAt: time.Now().Add(-time.Hour), + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + + cmd := &AuthStatusCmd{JSON: true} + stdout := captureStdout(t, func() { + if err := cmd.Run(&Context{Profile: "work"}); err != nil { + t.Fatalf("Run: %v", err) + } + }) + + if !strings.Contains(stdout, `"oauth_status": "login_required"`) { + t.Fatalf("expected login_required without refresh token, got: %s", stdout) + } + if !strings.Contains(stdout, `"authenticated": false`) { + t.Fatalf("expected authenticated false without refresh token, got: %s", stdout) + } +} diff --git a/skills/notion/SKILL.md b/skills/notion/SKILL.md index d67f81d..6fbb5f1 100644 --- a/skills/notion/SKILL.md +++ b/skills/notion/SKILL.md @@ -30,8 +30,8 @@ The CLI uses OAuth authentication for MCP-backed commands. On first use, it open ```bash notion-cli auth login # Authenticate with Notion -notion-cli auth status # Check authentication status (also shows active profile) -notion-cli auth refresh # Refresh token if status shows expired token +notion-cli auth status # Show active profile and OAuth state (diagnostic) +notion-cli auth refresh # Force-refresh; commands auto-refresh on use, so rarely needed notion-cli auth logout # Clear credentials ``` @@ -204,6 +204,6 @@ notion-cli search "api" --json | jq '.[] | .title' 6. **Inline comments by default** - `page view` includes open page comments and inline block discussions unless `--no-comments` is set 7. **Raw output** - Use `--raw` with `page view` to see the original Notion markup 8. **JSON for parsing** - Use `--json` when you need to extract specific fields, including the `Comments` array from `page view` -9. **Auth preflight** - Run `notion-cli auth status --json` before a multi-step workflow and refresh/login if needed +9. **No auth preflight** - Just run the command; the CLI auto-refreshes tokens on use. `notion-cli auth status` and `notion-cli auth list` are diagnostic surfaces, not health gates - do not poll them as a sanity check before each call. Only run `notion-cli auth login` if a real command returns an authentication error. 10. **API fallback preflight** - Run `notion-cli auth api verify` before workflows that need local image upload 11. **Error handling** - If a targeted `page edit` call fails, rerun with `--replace` as a safe fallback From 5bdef6f6961938d9dd34e55fd8536c76ef3c6b48 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 6 May 2026 11:23:13 -0400 Subject: [PATCH 13/15] fix(config): keep PathForProfile working without HOME `PathForProfile` only returns a profile's config-file path, but it delegated to `PathsForProfile`, which always computed the default profile's legacy token path via `os.UserHomeDir()`. That made config-only flows (`auth api status/setup` and similar) fail in environments where HOME is unavailable, even though they only need `config.json` and previously resolved via `os.UserConfigDir()` alone. Extract a `profileBaseDir` helper that depends only on `ConfigDir()` so config-path resolution no longer transitively requires HOME. Token path resolution still uses `legacyDefaultTokenPath` for the default profile, since that path is genuinely under `~/.config/notion-cli` and cannot be served from `os.UserConfigDir()` on every platform. Reported by Codex review on PR #35. --- internal/config/config.go | 31 +++++++++++++++++++++++-------- internal/config/config_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index f38cdc8..bfea0f1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -100,28 +100,28 @@ func StatePath() (string, error) { } func PathForProfile(profile string) (string, error) { - paths, err := PathsForProfile(profile) + resolvedProfile, err := ResolveProfile(profile) + if err != nil { + return "", err + } + profileDir, err := profileBaseDir(resolvedProfile) if err != nil { return "", err } - return paths.ConfigPath, nil + return filepath.Join(profileDir, configFileName), nil } func PathsForProfile(profile string) (ProfilePaths, error) { - baseDir, err := ConfigDir() + resolvedProfile, err := ResolveProfile(profile) if err != nil { return ProfilePaths{}, err } - resolvedProfile, err := ResolveProfile(profile) + profileDir, err := profileBaseDir(resolvedProfile) if err != nil { return ProfilePaths{}, err } - profileDir := baseDir - if resolvedProfile != defaultProfileName { - profileDir = filepath.Join(baseDir, profilesDirName, resolvedProfile) - } tokenPath := filepath.Join(profileDir, tokenFileName) if resolvedProfile == defaultProfileName { tokenPath, err = legacyDefaultTokenPath() @@ -137,6 +137,21 @@ func PathsForProfile(profile string) (ProfilePaths, error) { }, nil } +// profileBaseDir returns the directory that holds a profile's config file. +// It only depends on ConfigDir (XDG_CONFIG_HOME or its platform fallback) +// and never resolves HOME, so config-only flows keep working in environments +// where os.UserHomeDir is unavailable (e.g. minimal CI containers). +func profileBaseDir(resolvedProfile string) (string, error) { + baseDir, err := ConfigDir() + if err != nil { + return "", err + } + if resolvedProfile == defaultProfileName { + return baseDir, nil + } + return filepath.Join(baseDir, profilesDirName, resolvedProfile), nil +} + // legacyDefaultTokenPath preserves the pre-profile OAuth token location. // API config files use ConfigDir, but upstream OAuth tokens historically // lived under ~/.config/notion-cli even on platforms where os.UserConfigDir diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 026d068..41c4850 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -276,3 +276,31 @@ func TestListProfilesIncludesActiveDefaultAndNamedProfiles(t *testing.T) { t.Fatalf("profiles = %#v, want %#v", got, want) } } + +func TestPathForProfileDefaultDoesNotRequireHome(t *testing.T) { + isolateConfigDir(t) + t.Setenv("HOME", "") + + // macOS and Windows resolve UserConfigDir from HOME, so this regression + // only surfaces on platforms (Linux containers, etc.) where ConfigDir + // can be satisfied via XDG_CONFIG_HOME without HOME. + if _, err := os.UserConfigDir(); err != nil { + t.Skipf("UserConfigDir requires HOME on this platform: %v", err) + } + + path, err := PathForProfile("") + if err != nil { + t.Fatalf("PathForProfile(default): %v", err) + } + if path == "" { + t.Fatalf("expected non-empty config path for default profile") + } + + path, err = PathForProfile("work") + if err != nil { + t.Fatalf("PathForProfile(work): %v", err) + } + if path == "" { + t.Fatalf("expected non-empty config path for named profile") + } +} From 5cdab5927a9b28ad53370d45c22cd89fbffaa3c2 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 6 May 2026 11:34:10 -0400 Subject: [PATCH 14/15] fix(auth): align status and list JSON shape `auth status --json` and `auth list --json` shipped with divergent key names for the same fields: status used `expires_at`/`has_token` while list used `oauth_expires_at`/`has_oauth_token`. Status also omitted `active`, `has_api_token`, and `config_path` despite having the data. Standardize on the list/struct key names and add the missing fields so a single-profile inspection and a rows-of-the-list-format inspection return the same shape (plus the existing `authenticated` synthetic at the top level). --- cmd/auth.go | 15 +++++++++------ cmd/auth_test.go | 6 ++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/cmd/auth.go b/cmd/auth.go index f73dcc1..be530dc 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -117,14 +117,17 @@ func (c *AuthStatusCmd) Run(ctx *Context) error { if ctx.JSON { payload := map[string]any{ - "authenticated": status.OAuthStatus == "valid", - "profile": status.Profile, - "has_token": status.HasOAuthToken, - "token_path": status.TokenPath, - "oauth_status": status.OAuthStatus, + "authenticated": status.OAuthStatus == "valid", + "profile": status.Profile, + "active": status.Active, + "has_oauth_token": status.HasOAuthToken, + "oauth_status": status.OAuthStatus, + "has_api_token": status.HasAPIToken, + "token_path": status.TokenPath, + "config_path": status.ConfigPath, } if status.OAuthExpiresAt != nil { - payload["expires_at"] = status.OAuthExpiresAt + payload["oauth_expires_at"] = status.OAuthExpiresAt } enc := json.NewEncoder(os.Stdout) enc.SetIndent("", " ") diff --git a/cmd/auth_test.go b/cmd/auth_test.go index baa4b06..8b882db 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -131,6 +131,12 @@ func TestAuthStatusJSONReportsValidWhenAccessExpiredButRefreshAvailable(t *testi if !strings.Contains(stdout, `"authenticated": true`) { t.Fatalf("expected authenticated true when refresh token present, got: %s", stdout) } + if !strings.Contains(stdout, `"has_oauth_token": true`) { + t.Fatalf("expected has_oauth_token field, got: %s", stdout) + } + if !strings.Contains(stdout, `"oauth_expires_at":`) { + t.Fatalf("expected oauth_expires_at field, got: %s", stdout) + } } func TestAuthStatusJSONReportsLoginRequiredWithoutRefreshToken(t *testing.T) { From 9dc02cc3fa14e0c4bc495c958a7f131184f67d24 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 6 May 2026 11:49:17 -0400 Subject: [PATCH 15/15] fix(auth): suppress zero expiry and reject windows reserved profiles Two follow-on fixes from Codex review of the multi-profile work: - inspectProfileStatus always populated OAuthExpiresAt from the stored token, even when the access token field was empty. An interrupted OAuth login can leave token.json holding only a client_id, so status output would report `oauth_status: missing` alongside a misleading `oauth_expires_at: 0001-01-01T00:00:00Z`. Only set the expiry when a real access token is present. - ResolveProfile accepted Windows reserved device basenames (con, prn, aux, nul, com1-9, lpt1-9, plus the same with extensions) which then flow into file paths under `~/.config/notion-cli/profiles//`. Allowing them lets a profile authenticate on macOS or Linux but fail the moment Windows tries to create the directory. Reject reserved basenames during validation. --- cmd/auth.go | 4 ++-- cmd/auth_test.go | 29 +++++++++++++++++++++++++++++ internal/config/config.go | 26 ++++++++++++++++++++++++++ internal/config/config_test.go | 18 ++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/cmd/auth.go b/cmd/auth.go index be530dc..285b1bd 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -524,9 +524,9 @@ func inspectProfileStatus(profile string) (authProfileStatus, error) { } status.HasOAuthToken = strings.TrimSpace(token.AccessToken) != "" - expiresAt := token.ExpiresAt - status.OAuthExpiresAt = &expiresAt if status.HasOAuthToken { + expiresAt := token.ExpiresAt + status.OAuthExpiresAt = &expiresAt switch { case !token.IsExpired(): status.OAuthStatus = "valid" diff --git a/cmd/auth_test.go b/cmd/auth_test.go index 8b882db..7ef6b44 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -139,6 +139,35 @@ func TestAuthStatusJSONReportsValidWhenAccessExpiredButRefreshAvailable(t *testi } } +func TestAuthStatusJSONOmitsExpiryWhenAccessTokenMissing(t *testing.T) { + isolateAuthConfig(t) + + store, err := mcp.NewFileTokenStore("work") + if err != nil { + t.Fatalf("NewFileTokenStore: %v", err) + } + if err := store.SaveToken(context.Background(), &transport.Token{ + TokenType: "Bearer", + RefreshToken: "leftover-refresh", + }); err != nil { + t.Fatalf("SaveToken: %v", err) + } + + cmd := &AuthStatusCmd{JSON: true} + stdout := captureStdout(t, func() { + if err := cmd.Run(&Context{Profile: "work"}); err != nil { + t.Fatalf("Run: %v", err) + } + }) + + if !strings.Contains(stdout, `"oauth_status": "missing"`) { + t.Fatalf("expected missing status when access token empty, got: %s", stdout) + } + if strings.Contains(stdout, `"oauth_expires_at"`) { + t.Fatalf("expected no oauth_expires_at when access token empty, got: %s", stdout) + } +} + func TestAuthStatusJSONReportsLoginRequiredWithoutRefreshToken(t *testing.T) { isolateAuthConfig(t) diff --git a/internal/config/config.go b/internal/config/config.go index bfea0f1..3173109 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -184,9 +184,35 @@ func ResolveProfile(profile string) (string, error) { } return "", fmt.Errorf("invalid profile %q: use lowercase letters, numbers, at sign, dot, underscore, and hyphen", profile) } + if isWindowsReservedName(normalized) { + return "", fmt.Errorf("invalid profile %q: name is reserved on Windows", profile) + } return normalized, nil } +// isWindowsReservedName reports whether the (already lowercased) profile +// matches a Windows reserved device name. Windows refuses to create files or +// directories with these basenames, so allowing them here would leave +// profiles that work on macOS/Linux but break on Windows the moment a +// command tries to read or write the profile's token/config files. +func isWindowsReservedName(name string) bool { + base := name + if dot := strings.IndexByte(base, '.'); dot >= 0 { + base = base[:dot] + } + switch base { + case "con", "prn", "aux", "nul": + return true + } + if len(base) == 4 && (base[:3] == "com" || base[:3] == "lpt") { + c := base[3] + if c >= '1' && c <= '9' { + return true + } + } + return false +} + func isProfileEndpoint(r rune) bool { return isLowercaseASCII(r) || isDigitASCII(r) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 41c4850..c71afa0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -216,6 +216,24 @@ func TestResolveProfileRejectsInvalidNames(t *testing.T) { } } +func TestResolveProfileRejectsWindowsReservedNames(t *testing.T) { + for _, value := range []string{ + "con", + "prn", + "aux", + "nul", + "com1", + "com9", + "lpt1", + "lpt9", + "con.txt", + } { + if _, err := ResolveProfile(value); err == nil { + t.Fatalf("ResolveProfile(%q) should fail as Windows reserved", value) + } + } +} + func TestResolveProfileAllowsPortableNames(t *testing.T) { for _, value := range []string{"work", "work-1", "personal_2", "brian@brianle.xyz"} { got, err := ResolveProfile(value)