#89: grans owns its own Granola credentials

[dmwyatt]

Closes #89.

Granola moved its data-encryption key into the macOS data-protection keychain, gated on its own code signature, so grans sync can no longer read the token the desktop app stored. No amount of better decryption fixes that. grans now holds its own session.

What this adds

grans auth login              # Sign in via Granola's PKCE flow
grans auth login --provider microsoft
grans auth status             # Where credentials live, and expiry; never token material
grans auth logout             # Remove the local credentials

Token resolution order, with the local store kept last rather than removed, since it is still the only source on Windows and older macOS builds for anyone who has not signed in:

1. --token, or GRANS_TOKEN
2. grans's own credentials, refreshed when the access token has expired
3. The token Granola's desktop app stored locally

A failed refresh is reported, not quietly downgraded to the local store. If you signed grans in, a broken chain is a real problem and the message says to run grans auth login again.

Where the credentials live

This PR is what gives grans a long-lived secret. Before it, grans read an access token out of Granola's encrypted store on each run and kept nothing of its own. A refresh token is a bigger deal than the six-hour access token it replaces: it mints new access tokens until the session is revoked.

So it goes in the platform keychain, which grans auth status names:

$ grans auth status
Signed in.
  Credentials: Windows Credential Manager (grans)
  Session:     session_01KYE2HB66G210V6XK4W4SY8VY
  Access token: valid until 2026-07-26 03:03:54

An earlier revision of this branch wrote it to a 0600 TOML file instead. That was a step down from what it replaced: unlike Granola's DPAPI-wrapped store, a plaintext file copied from a backup or disk image is a working credential anywhere.

The file remains as a fallback, because a headless Linux box or a bare WSL install has no keychain and failing there would break setups that work today. It is announced at login and in auth status, and says what it does and does not protect. When a keychain does become available, the next command moves the credentials into it and deletes the file.

Keychain reachability is settled by reading, not by constructing an entry: a missing Secret Service or a locked keychain fails only on access.

On macOS the keychain item is bound to the binary that created it, so a rebuilt or self-updated grans may prompt for access again.

What the live API actually does

Four things in #89 turned out differently once tested against the real endpoints. All are now covered by tests.

The callback is not a granola:// URL. The login ends on https://www.granola.ai/app-redirect?code=..., which then hands off to the deep link. That page is what the browser shows and what the address bar offers to copy. Both carry the same code, so parsing takes the code parameter from any URL instead of enforcing a scheme.

The exchange response nests the tokens somewhere neither assumed path found. Extraction now looks for the object carrying an access_token/refresh_token pair, searching WorkOS-named keys first, because a response can carry the identity provider's tokens alongside Granola's and serde_json orders keys alphabetically. When nothing matches, the error prints the response's shape, keys and value types only, never values.

Refresh does not rotate the refresh token. #89 warned that it does and that failing to persist the rotation breaks the chain. A live refresh returns the same refresh token with a new access token and expiry. The code persists whatever comes back either way, so this changed only comments. The concurrent-rotation handling stays: one store read on a path that already failed, covering Granola turning rotation on later.

The version gate is about a minimum, not just presence. No version redirects to granola.ai/upgrade-app, and so does 0.0.1, but 6.518.0 still passes. A preflight request with redirects disabled catches that before a browser opens, since in a browser it just looks like a marketing page:

$ GRANS_GRANOLA_VERSION=0.0.1 grans auth login
Error: Granola rejected client version 0.0.1 as out of date.
Set GRANS_GRANOLA_VERSION=<current Granola version> and try again.

Verified on Windows

Signed in through the browser, then:

$ grans sync people -v
[DEBUG grans::api::auth] Using grans's own stored credentials
[grans] Fetched 1 people

$ grans sync people -v          # after forcing the stored token to expire
[DEBUG grans::api::auth] Stored access token is missing or expired; refreshing
[grans] Fetched 1 people

$ grans auth status -v          # with credentials still in the pre-keychain file
[DEBUG grans::api::credentials] Moving credentials from ...\auth.toml into the keychain

The migration removed the file, the session survived, and Windows lists the entry as LegacyGeneric:target=granola-session.grans.

Also verified: --refresh-token-stdin bootstrap; a dead refresh token surfacing Could not refresh grans's Granola session. Run 'grans auth login' to sign in again.; the local-store fallback still working after auth logout, so the Windows path has not regressed; and both --provider values mapping server-side to GoogleOAuth and MicrosoftOAuth.

Not verified: macOS, which is the platform the P0 is about. Nothing in the login flow is platform-specific, but that claim needs a real run there. Completing a Microsoft sign-in end to end, which needs a Microsoft-linked account; everything after the redirect is the same code path Google exercised. And a refresh against the keychain backend specifically, which is proven by composition, since the refresh logic is tested against the file backend and store.save() on the keychain is exercised by login and migration.

Notes

• src/api/auth.rs reached 601 lines, so reading Granola's local token store moved to src/api/local_store.rs; credentials.rs then reached 528, so storage moved to credential_store.rs, leaving the credential type separate from where it is kept. Both were pure moves with the suite passing on either side.
• PKCE generation moved out of the Dropbox-specific OAuth module into src/pkce.rs, parameterized by entropy: Dropbox keeps 64 bytes, Granola sends 32 to match its client.
• The client version was hardcoded in two places and needed a third, so it is now src/api/identity.rs, overridable via GRANS_GRANOLA_VERSION. Moving 6.518.0 to 7.441.6 was checked against the data endpoints by syncing.
• The credential file is created with 0600 from the open rather than chmod'd afterward, which closed a window where the token was briefly world-readable. Found by the security review on this PR.
• No file lock around refresh. The failure it prevents needs two concurrent grans runs, is loud, and is recoverable by re-login. Easy to add if anyone hits it.
• src/sync/config.rs stores the Dropbox refresh token with the write-then-chmod pattern this PR replaced, and in plaintext. Untouched here to keep the diff honest, but it deserves the same treatment.

831 unit tests and 67 integration tests pass. No screenshots: every change here is terminal text, which reads better as the code blocks above.

https://claude.ai/code/session_012c8WU6fhQDaVekXJX5hGhH

[claude]

Claude finished @dmwyatt's task in 2m 51s —— View job

---

Code Review

• Gather context (read diff, CLAUDE.md, changed files)
• Review for bugs and code quality issues
• Post findings

Reviewed the full diff: src/api/granola_auth.rs, src/api/credentials.rs, src/api/local_store.rs, src/api/auth.rs, src/commands/auth.rs, src/pkce.rs, src/api/identity.rs, src/cli/args/mod.rs, src/main.rs, src/api/client.rs, src/commands/sync.rs, src/sync/oauth.rs, plus the CLAUDE.md/README doc updates.

Particular attention went to the highest-risk logic: the token-resolution/refresh chain and its concurrent-rotation handling in src/api/auth.rs, the JSON token-extraction search (locate_tokens/extract_token_set) in src/api/granola_auth.rs, the atomic credential file write in src/api/credentials.rs, and the local_store.rs split (confirmed a pure move via git show 8f67e55/f9a2357 — no behavioral change).

Note: I was unable to run cargo check/cargo test in this sandbox (the Bash tool required interactive approval I don't have here), so I relied on manual tracing rather than compiling. Given the PR author reports 828 unit + 67 integration tests passing, and my read of the logic (recursion depth guards, expiry-skew handling, concurrent-refresh fallback, credentials round-tripping) lines up with that, I don't have reason to doubt it.

Code review: no issues found.

[claude]

Claude finished @dmwyatt's task in 2m 29s —— View job

---

Security Review complete

• Gather context (diff, changed files)
• Review auth token handling (supabase.json, local_store.rs, credentials.rs, granola_auth.rs)
• Review SQL/FTS5 query construction — no db/ changes in this PR, not applicable
• Review command injection surfaces — none introduced (WSL cmd.exe call in local_store.rs is a pure move, no new user input)
• Review unsafe Rust blocks — none new; existing unsafe in token_store.rs/update/mod.rs/main.rs untouched by this PR
• Review path traversal in file/db path resolution — data_dir() unchanged, out of scope
• Review sensitive data in logs/errors/panics — token values are consistently kept out of debug!/error output (only lengths/expiry logged); describe_expiry has a test asserting no token leakage
• Review new/changed Cargo dependencies — url, uuid added; no known advisories, no new unsafe/native code surface
• Post findings

Findings

Posted 1 inline comment (scored ≥75 confidence):

src/api/credentials.rs (lines 93–102) — refresh-token file briefly world/group-readable before permissions are tightened. save_to writes the plaintext refresh token to auth.toml.tmp with fs::write (default umask-derived mode, typically 0644), then calls fs::set_permissions(..., 0o600) afterward. Between those two calls the token sits on disk in a directory (data_dir()) that isn't 0700 by default, so another local user able to traverse it and watching for the file (e.g. via inotify) could read the refresh token before the chmod lands. Suggested fix: open the temp file with OpenOptions::mode(0o600) on unix so it's created with the restrictive permissions from the start, instead of tightening them after content is written.

Everything else reviewed — response-body inclusion in granola_auth::post_json error messages, Debug-derived credential structs, the CLI --token/GRANS_TOKEN resolution, the PKCE implementation, and the local-store fallback — looked sound or below the confidence bar for this review's scope (either not attacker-reachable, or already mitigated, e.g. errors describe JSON shape only, never values).

[claude]

Refresh-token file briefly written with world/group-readable permissions (TOCTOU).

fs::write creates auth.toml.tmp with the process's default (umask-derived) mode — typically 0644 — and 0600 is applied only after the content, including the plaintext refresh token, is already on disk. Any other local user able to traverse data_dir() (commonly ~/.local/share/grans, not 0700 by default) can read the token during that window, e.g. by watching for the file with inotify and reading it before the chmod lands.

Fix by creating the file with the restrictive mode from the start instead of tightening it after the fact:

Suggested change

	let temp_path = path.with_extension("toml.tmp");
	fs::write(&temp_path, &content)
	.with_context(|| format!("Failed to write {}", temp_path.display()))?;
	#[cfg(unix)]
	{
	use std::os::unix::fs::PermissionsExt;
	fs::set_permissions(&temp_path, fs::Permissions::from_mode(0o600))
	.with_context(|| format!("Failed to set permissions on {}", temp_path.display()))?;
	}
	let temp_path = path.with_extension("toml.tmp");
	#[cfg(unix)]
	{
	use std::io::Write as _;
	use std::os::unix::fs::OpenOptionsExt;
	let mut file = fs::OpenOptions::new()
	.write(true)
	.create(true)
	.truncate(true)
	.mode(0o600)
	.open(&temp_path)
	.with_context(|| format!("Failed to open {}", temp_path.display()))?;
	file.write_all(content.as_bytes())
	.with_context(|| format!("Failed to write {}", temp_path.display()))?;
	}
	#[cfg(not(unix))]
	fs::write(&temp_path, &content)
	.with_context(|| format!("Failed to write {}", temp_path.display()))?;

Fix this →

[claude]

Claude finished @dmwyatt's task in 1m 38s —— View job

---

Security Review complete

• Gather context (diff, changed files)
• Review auth token handling (supabase.json, local_store.rs, credentials.rs, granola_auth.rs, auth.rs)
• Review SQL/FTS5 query construction — no db/ changes in this PR, not applicable
• Review command injection surfaces — none introduced (WSL cmd.exe call in local_store.rs is a pure move, no new user input)
• Review unsafe Rust blocks — none new in this diff
• Review path traversal in file/db path resolution — unchanged, out of scope
• Review sensitive data in logs/errors/panics — token values consistently kept out of debug!/error output; GranolaCredentials is never logged via {:?}
• Review new/changed Cargo dependencies — url, uuid (already assessed, no advisories)
• Post findings

Findings

The previously flagged TOCTOU issue is fixed. Commit 34b57af ("Create the credential file private, rather than tightening it after") rewrote src/api/credentials.rs's save_to/create_private_file: the temp file is now opened with OpenOptions::new().write(true).create_new(true).mode(0o600) on unix, so the restrictive permissions apply at creation rather than being applied via a later chmod. create_new also means a pre-existing file or symlink at the temp path (planted between the remove_file cleanup and the open) causes the open to fail rather than being written through. New tests (test_private_file_is_owner_only_from_creation, test_private_file_tightens_permissions_on_reuse) cover this directly. No remaining gap here.

Reviewed the rest of the diff (granola_auth.rs's token-location search and error-shape-only reporting, local_store.rs, auth.rs's refresh/persist chain, commands/auth.rs, identity.rs, pkce.rs using OsRng, client.rs) — nothing else met the ≥75 confidence bar for a concrete, in-scope attack scenario.

Security review: no issues found in this PR.

[claude]

Claude finished @dmwyatt's task in 1m 41s —— View job

---

Code Review

• Gather context (read diff, CLAUDE.md, changed files)
• Review for bugs and code quality issues
• Post findings

Re-reviewed the full diff against origin/main, with particular attention to the commits added since the last pass: the app-redirect callback parsing (parse_callback), the rewritten token-extraction search (locate_tokens/extract_token_set/describe_json_shape) and its depth-bounded recursion, the refresh-token-rotation comment corrections in auth.rs, and the create_private_file fix in src/api/credentials.rs that addresses the TOCTOU finding from the earlier security review.

Traced the recursion/depth-limit logic in locate_tokens and describe_json_shape by hand (including the depth-0 edge cases) and didn't find an off-by-one or early-return bug; the WorkOS-key-preference sort and the JSON-string-decoding path both match their test coverage. The refresh_and_persist/refresh_once concurrent-rotation handling in src/api/auth.rs is unchanged in behavior, only comments were corrected there. create_private_file's remove-then-create_new sequence correctly refuses to follow a symlink planted between the two calls.

Code review: no issues found.

[dmwyatt]

Fixed in 34b57af.

The permissions now come from the open rather than a later chmod, via OpenOptions::mode(0o600) on unix.

Writing the test for it surfaced a second hole in the same fix: OpenOptions::mode applies only when the open creates the file, so a temp file left behind by an interrupted run would have been reused with whatever permissions it already had, and the token written into it. So the leftover is removed first, and create_new(true) then refuses to write through anything, including a symlink, that appears between the remove and the open.

Three unix-only tests cover it, confirmed running on CI (the dev machine here is Windows, where they compile out):

test api::credentials::tests::test_private_file_is_owner_only_from_creation ... ok
test api::credentials::tests::test_private_file_tightens_permissions_on_reuse ... ok
test api::credentials::tests::test_saved_file_is_owner_only ... ok

Two things worth recording rather than fixing here:

src/sync/config.rs stores the Dropbox refresh token with the same write-then-chmod pattern this replaced. It is untouched by this PR, so I have left it alone rather than widening the diff, but it is the same class of bug for the same class of secret and deserves the same fix.

On Windows the file inherits the directory ACL and the refresh token sits there unprotected, which 0600 never addressed either. The real fix is the platform keychain, already noted in the PR description as a follow-up: grans would own that item, so the entitlement problem that blocked reading Granola's does not apply.

https://claude.ai/code/session_012c8WU6fhQDaVekXJX5hGhH

[claude]

Claude finished @dmwyatt's task in 2m 17s —— View job

---

Security Review complete

• Gather context (diff since last review, changed files)
• Review new commit 0368b32 — "Keep the refresh token in the platform keychain"
• Post findings

This push adds one new commit on top of what was already reviewed (34b57af, which fixed the earlier TOCTOU finding): src/api/credential_store.rs (new CredentialStore abstraction with Keychain/File backends) and the corresponding wiring in src/api/auth.rs and src/commands/auth.rs.

Checked specifically:

• create_private_file (the previously-fixed TOCTOU logic) moved unchanged into credential_store.rs's file backend — create_new(true) + mode(0o600) still applies at creation, not after the fact, and the fallback path is exercised by the existing test_private_file_* unit tests.
• Keychain writes (entry.set_password) go straight through the OS-native credential store (Windows Credential Manager / macOS Keychain / Secret Service via zbus-secret-service-keyring-store), bypassing the filesystem entirely — confirmed all three backends are actually compiled in via Cargo.lock (apple-native-keyring-store, windows-native-keyring-store, zbus-secret-service-keyring-store all appear as keyring's resolved dependencies), so this isn't a case of the Cargo feature flag only covering macOS while claiming cross-platform support.
• absorb_credentials_file (migrates an existing plaintext auth.toml into the keychain) only deletes the file after the keychain save() succeeds, so a failed migration can't strand or lose the credentials.
• No token material in output: describe(), the login/status/logout paths, and print_no_keychain_warning() only ever print the store's description (a path or "macOS Keychain (grans)"), never token values. reachable_keychain()'s error logging includes only the keyring::Error, not the secret.

No new findings at or above the 75-confidence bar for this delta.

Security review: no issues found in this PR.

[claude]

Claude finished @dmwyatt's task in 2m 43s —— View job

---

Code Review

• Gather context (diff since last review, CLAUDE.md, changed files)
• Review new commits for bugs and code quality issues
• Post findings

Reviewed the commit added since the last review pass, 0368b32 ("Keep the refresh token in the platform keychain"), which moves credential storage behind a new CredentialStore abstraction (src/api/credential_store.rs) with the platform keychain (keyring crate) as primary and the 0600 file as fallback, and the resulting changes to src/api/auth.rs, src/api/credentials.rs (now just the GranolaCredentials struct), src/commands/auth.rs, src/api/mod.rs, Cargo.toml, and the docs.

Particular attention went to:

• CredentialStore::open/reachable_keychain/absorb_credentials_file — the "test by reading, not constructing" reachability check, and the migrate-then-delete sequencing into the keychain (file only removed after the keychain write succeeds).
• create_private_file in credential_store.rs — confirmed the TOCTOU fix from the prior security review round survived this refactor unchanged (remove-then-create_new-with-mode(0o600), still covered by the three unix-only tests).
• The refresh/persist chain in auth.rs (refresh_and_persist/refresh_once/token_from_credentials) — traced the concurrent-rotation retry path by hand; logic and tests are consistent (one nitpick below the confidence bar: a second failed retry after a detected concurrent rotation returns the raw error without the "run grans auth login" hint, but this needs two concurrent refreshes racing and the retry also failing — too narrow to flag).
• Cargo.lock for the new keyring dependency — confirmed windows-native-keyring-store and zbus-secret-service-keyring-store are both pulled in alongside apple-native-keyring-store, so the "Windows Credential Manager / Secret Service" claims in the docs match what's actually compiled in, not just the macOS backend named in Cargo.toml.
• src/pkce.rs and src/sync/oauth.rs — confirmed the PKCE extraction is a clean move (Dropbox keeps 64 bytes of entropy, tests moved over intact).

Code review: no issues found.