From b3ff68bb1d5fd393d8fb8c019384e14b0f69f6b8 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Sun, 30 Aug 2026 10:12:18 -0600 Subject: [PATCH] fix: persist rotated refresh tokens, close silent-empty auth gaps Supabase rotates the refresh token on every refresh and invalidates the previous one immediately. createSupabaseClient used client defaults (autoRefreshToken: true) with Node's in-memory session store, so the long-lived MCP server kept spending the CLI's on-disk refresh token on its 30s auto-refresh tick without ever writing the rotation back to the ~/.config/tages/auth.json the CLI and server share. The on-disk credential eventually hit refresh_token_already_used and stayed dead. rlee@mersive.com's session died about an hour after login and stayed dead for four days; both pending phoenix teammates would have hit the same wall within an hour of joining. - packages/shared/src/auth-store.ts (new): single auth.json reader/writer shared by CLI and server. Atomic writes (temp file + rename), unconditional 0600, cleans up the temp file on failure. - packages/shared/src/auth-persist.ts (new): persistSessionOnRefresh listens for TOKEN_REFRESHED and writes it to disk; persistRotatedTokens refuses to overwrite a disk token with a later expiry than the incoming one, so a stale background process can't clobber a fresh `tages login` from another terminal. Registered before setSession() at every long-lived call site (server index.ts/config.ts, CLI auth/session.ts, both backfill-*.ts scripts), since an expired access token makes setSession refresh immediately and that first rotation was the one spending the disk token. - init.ts, link.ts, migrate.ts now route their auth.json writes through the shared writer instead of a bare writeFileSync. link is the command a teammate runs to join a project; the old truncate-then-write could race the server's rename and drop a freshly minted OAuth token into an orphaned inode while still reporting success. - New requireLiveSession guard on team (all 4 subcommands), status, and onboard. A dead session previously fell back silently to an anonymous client, read zero rows through RLS, and exited 0 with "No team members" / "Memories: 0" / an empty briefing. Covers anonymous as well as expired (`tages logout && tages team list` reproduces it in one step). - Versions: @tages/shared 0.2.3->0.2.4, @tages/server 0.3.4->0.3.5, @tages/cli 0.5.5->0.5.6. Docs refreshed to match, plus a new trap section on this bug. Not fixed, noted in the PR: persistRotatedTokens has no lock, so two simultaneous refreshes can still lose one rotation; no fsync before rename; 29 other CLI commands still share the old empty-vs-expired ambiguity (recall-context, pending, brief, query next). Tests: +12, each mutation-tested against its own removal. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012j48bNYeNWSu4dorw5wG6V --- README.md | 14 + docs/quickstart.md | 4 +- docs/team-onboarding.md | 35 ++- packages/cli/package.json | 2 +- .../cli/src/__tests__/commands-smoke.test.ts | 16 +- packages/cli/src/__tests__/status.test.ts | 20 +- packages/cli/src/__tests__/team.test.ts | 115 +++++++- packages/cli/src/auth/session.ts | 45 +++- packages/cli/src/auth/store.ts | 71 ++--- packages/cli/src/commands/init.ts | 11 +- packages/cli/src/commands/link.ts | 12 +- packages/cli/src/commands/migrate.ts | 5 +- packages/cli/src/commands/onboard.ts | 8 +- packages/cli/src/commands/status.ts | 8 +- packages/cli/src/commands/team.ts | 26 +- packages/cli/src/config/paths.ts | 15 +- packages/server/package.json | 2 +- .../scripts/backfill-chunk-embeddings.ts | 6 +- .../server/scripts/backfill-embeddings.ts | 6 +- .../src/__tests__/resolve-project.test.ts | 7 + packages/server/src/config.ts | 12 +- packages/server/src/index.ts | 7 +- packages/shared/package.json | 7 +- .../shared/src/__tests__/auth-persist.test.ts | 253 ++++++++++++++++++ packages/shared/src/auth-persist.ts | 100 +++++++ packages/shared/src/auth-store.ts | 107 ++++++++ packages/shared/src/index.ts | 9 + packages/shared/tsconfig.build.json | 7 + packages/shared/tsconfig.json | 13 +- pnpm-lock.yaml | 5 +- 30 files changed, 837 insertions(+), 111 deletions(-) create mode 100644 packages/shared/src/__tests__/auth-persist.test.ts create mode 100644 packages/shared/src/auth-persist.ts create mode 100644 packages/shared/src/auth-store.ts create mode 100644 packages/shared/tsconfig.build.json diff --git a/README.md b/README.md index 1c17e94..9339085 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,20 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for development setup and guidelines. ## Release Notes +### 2026-08-30 — `v0.5.6`: rotated refresh tokens were only kept in memory + +Supabase rotates the refresh token on every refresh and invalidates the previous one immediately. `createSupabaseClient` uses the client defaults (`autoRefreshToken: true`), and in Node the session store is in-memory only. The long-lived MCP server calls `setSession()` at boot and auto-refreshes on a 30s tick, so the rotated token lived in the server's memory and nowhere else — while the CLI and the server share the same `~/.config/tages/auth.json` on disk. The server was spending the CLI's on-disk refresh token every ~30 seconds without ever writing the new one back, so the on-disk credential eventually hit `refresh_token_already_used` and stayed dead. Confirmed empirically: `auth.json`'s mtime was still the original login while a direct call to the token endpoint returned `refresh_token_already_used`. `rlee@mersive.com`'s session died about an hour after login and stayed dead for four days; both pending `phoenix` teammates would have hit the same wall within an hour of joining. + +- **`packages/shared/src/auth-store.ts` (new).** The single `auth.json` reader/writer, moved out of the CLI so the server uses the same code path. Writes atomically (temp file + `rename`), sets file mode `0600` unconditionally rather than relying on `writeFileSync`'s creation-only mode, and cleans up the temp file on any write failure. +- **`packages/shared/src/auth-persist.ts` (new).** `persistSessionOnRefresh` registers a Supabase `onAuthStateChange` listener that writes `TOKEN_REFRESHED` sessions back to `auth.json`. `persistRotatedTokens` refuses to write when the token already on disk has a later expiry than the incoming one, so a long-lived background process can't stomp a fresh `tages login` run from another terminal. Registered before `setSession()` at every long-lived call site — server `index.ts`/`config.ts`, CLI `auth/session.ts`, both `backfill-*.ts` scripts — because an expired access token makes `setSession` refresh immediately, and that first rotation is the one that was spending the disk token. +- **`init.ts`, `link.ts`, `migrate.ts`** had their own bare `writeFileSync` calls on `auth.json` and now route through the shared writer. `link` is the command a teammate runs to join a project; a truncate-then-write racing the server's `rename` could drop freshly minted OAuth tokens into an orphaned inode while still reporting success. +- **New `requireLiveSession` guard** on `team` (all 4 subcommands), `status`, and `onboard`. On a dead session these previously fell back to an anonymous client, read zero rows through RLS, and printed a clean "No team members" / "Memories: 0" / an empty briefing while exiting `0` — `tages team list` reported no members for a project that had two pending invites. The guard covers `anonymous` sessions as well as `expired` ones (`tages logout && tages team list` reproduces it in one step). +- **Not fixed, and worth stating plainly:** `persistRotatedTokens` is read-compare-write with no lock, so two processes refreshing at the same instant can still lose one rotation (better than always losing it, not zero). A hard kill between write and `rename` can leave an orphaned `auth.json.tmp.` at `0600` that nothing reaps. There's no `fsync` before `rename`, so the write is atomic against concurrent readers but not against power loss. 29 other CLI commands still go through `createAuthenticatedClient` with the same empty-vs-expired ambiguity this PR fixed for `team`/`status`/`onboard` — `recall-context`, `pending`, `brief`, and `query` are next. + +**Versions:** `@tages/shared` 0.2.3 → 0.2.4, `@tages/server` 0.3.4 → 0.3.5, `@tages/cli` 0.5.5 → 0.5.6. + +**Tests:** +12, each mutation-tested (verified to fail when the behavior under test was removed): the staleness guard, the `TOKEN_REFRESHED` event filter, the `team list` guard, the `anonymous` branch, the missing-token guard, the listener's write-failure catch, `readAuthFile`'s null returns, and temp-file cleanup on failure. + ### 2026-08-25 — `v0.5.5`: six onboarding defects, found while provisioning a real team project Every item here was found by actually provisioning a new project and inviting two teammates, not by reading the code. All six are on the first-run path, which is why none of the 1,500+ unit tests caught them. diff --git a/docs/quickstart.md b/docs/quickstart.md index 73112c6..6463ade 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -7,10 +7,10 @@ This is the path for **creating a new project**. Joining a project a teammate al ```bash npm install -g @tages/cli tages --version -# 0.5.4 +# 0.5.6 ``` -That is the whole install. The published packages are current — `@tages/cli` 0.5.4, `@tages/server` 0.3.4, `@tages/shared` 0.2.2 — and the end-to-end suite gates every release against the **published** artifacts, not the source tree, so npm is the path that is actually tested. +That is the whole install. The published packages are current — `@tages/cli` 0.5.6, `@tages/server` 0.3.5, `@tages/shared` 0.2.4 — and the end-to-end suite gates every release against the **published** artifacts, not the source tree, so npm is the path that is actually tested. Your agent is wired to `npx -y @tages/server`, which needs no clone. Nothing to keep on disk, nothing to rebuild. diff --git a/docs/team-onboarding.md b/docs/team-onboarding.md index 52b421f..b3427ef 100644 --- a/docs/team-onboarding.md +++ b/docs/team-onboarding.md @@ -24,10 +24,10 @@ You need: ```bash npm install -g @tages/cli tages --version -# 0.5.4 +# 0.5.6 ``` -The published packages are current (`@tages/cli` 0.5.4, `@tages/server` 0.3.4) and are what the end-to-end release suite actually tests — it drives the published artifacts, not the source tree, precisely because a defect that killed an earlier release candidate was invisible to 1,200+ unit tests and only appeared when the built entrypoint was run by `node`. +The published packages are current (`@tages/cli` 0.5.6, `@tages/server` 0.3.5) and are what the end-to-end release suite actually tests — it drives the published artifacts, not the source tree, precisely because a defect that killed an earlier release candidate was invisible to 1,200+ unit tests and only appeared when the built entrypoint was run by `node`. You do **not** need a source clone. Your agent will be wired to `npx -y @tages/server`. @@ -91,7 +91,7 @@ MCP server: npx -y @tages/server (published package) MCP server: node /Users/you/src/tages/packages/server/dist/index.js (local build) ``` -On an npm install you get the first, and that is correct — it resolves `@tages/server` 0.3.4, the same artifact the release suite gates on. You only get the second if you built from source, in which case `link` prefers your local build. +On an npm install you get the first, and that is correct — it resolves `@tages/server` 0.3.5, the same artifact the release suite gates on. You only get the second if you built from source, in which case `link` prefers your local build. --- @@ -152,11 +152,38 @@ Project slugs are **globally unique across all owners** (`supabase/migrations/00 **If you are already in this state:** delete `~/.config/tages/projects/.json`, then re-run `tages link --project-id `. Or join under a different local name with `tages link --project-id --slug `. +### A session that dies on its own (fixed in 0.5.6 / 0.3.5 — upgrade) + +Symptom: you log in successfully, everything works, and roughly an hour later +every command prints `Session expired. Run tages login`. Nobody logged out and +`auth.json` still has its original timestamp. + +Cause: Supabase rotates the refresh token on every refresh and immediately +invalidates the previous one. The Supabase client auto-refreshes on a 30-second +tick, and the MCP server is long-lived, so it refreshed in the background and +kept the replacement **in memory only**. The token left on disk was spent, and +presenting it again fails with `refresh_token_already_used` — permanently. The +CLI and the MCP server share one `auth.json`, so the server quietly ended the +CLI's session. + +Fixed by having every process that holds a session write the rotated token back +(`persistSessionOnRefresh`, `@tages/shared`). Older builds cannot be worked +around, only re-logged-into — so upgrade rather than re-running `tages login` +every hour: + +```bash +npm install -g @tages/cli@latest +tages login +``` + +Nothing needs to change in `.mcp.json`; `npx -y @tages/server` picks up the new +server on your next Claude Code restart. + ### Roles: you need `admin`, not `member` Writes require owner or `admin`. `is_write_authorized` (`supabase/migrations/0031_rbac_write_policies.sql:14-25`) returns true only for the project owner or a `team_members` row with role `owner`/`admin`. -A `member` can read everything and write nothing. Their memories land in local SQLite and never sync — via the CLI you at least get the yellow `Stored locally only` warning, but **through the MCP `remember` tool the agent is told `Stored memory: ...` with no error at all** (`packages/server/src/tools/remember.ts:139-143` ignores the remote-write result). Since your agent uses the MCP path, a `member` will appear to be contributing to team memory for as long as nobody checks. +A `member` can read everything and write nothing. Their memories land in local SQLite and never sync. Both paths now say so: the CLI prints the yellow `Stored locally only` warning, and the MCP `remember` tool returns `Stored memory in the local cache only: ... teammates will NOT see this memory`, which is deliberately not the plain success wording so the agent relays the limitation instead of reporting a save (`packages/server/src/tools/remember.ts`). Earlier builds did ignore the remote-write result and reported plain success, so a `member` appeared to be contributing to team memory until somebody checked; if you are on a build older than 0.3.5, assume that is still true. Both the CLI default (`tages team invite `) and `tages init --team` invite as **`member`**. Owners must invite as admin explicitly: diff --git a/packages/cli/package.json b/packages/cli/package.json index 0be4a07..d5dfba4 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@tages/cli", - "version": "0.5.5", + "version": "0.5.6", "description": "CLI for Tages AI agent memory", "type": "module", "bin": { diff --git a/packages/cli/src/__tests__/commands-smoke.test.ts b/packages/cli/src/__tests__/commands-smoke.test.ts index b8ffa5a..1a25b72 100644 --- a/packages/cli/src/__tests__/commands-smoke.test.ts +++ b/packages/cli/src/__tests__/commands-smoke.test.ts @@ -106,6 +106,10 @@ const mockAuth = { setSession: vi.fn().mockResolvedValue({ data: { session: { access_token: 'm' } }, error: null }), getSession: vi.fn().mockResolvedValue({ data: { session: { access_token: 'm' } }, error: null }), refreshSession: vi.fn().mockResolvedValue({ data: { session: { access_token: 'm', refresh_token: 'm' } }, error: null }), + // Every session-bearing client registers a TOKEN_REFRESHED listener so the + // rotated refresh token gets written back to auth.json. The real client + // always has this; a stub without it fails the command outright. + onAuthStateChange: vi.fn(() => ({ data: { subscription: { unsubscribe: vi.fn() } } })), } const mockSupabase = { @@ -122,9 +126,15 @@ function resetMockSupabase() { mockAuth.setSession.mockClear() mockAuth.getSession.mockClear() mockAuth.refreshSession.mockClear() + mockAuth.onAuthStateChange.mockClear() } -vi.mock('@tages/shared', () => ({ +// Partial mock on purpose. A whole-module replacement has to be updated every +// time `shared` grows an export the CLI uses — it broke once already when the +// auth-file writer and the refresh-persistence hook moved into `shared` — and +// the failure surfaces as an unrelated command test, not as a missing import. +vi.mock('@tages/shared', async (importOriginal) => ({ + ...(await importOriginal()), createSupabaseClient: vi.fn(() => mockSupabase), })) @@ -555,6 +565,10 @@ describe('onboard command', () => { const setup = setupTempConfigDir() tempConfigDir = setup.configDir cleanupFn = setup.cleanup + // `onboard` refuses to build a briefing without a live session, because an + // anonymous client reads zero rows and renders as a project with nothing in + // it. These tests assert on briefing CONTENT, so they need a real session. + writeAuthConfig(tempConfigDir) console_ = captureConsole() resetMockSupabase() }) diff --git a/packages/cli/src/__tests__/status.test.ts b/packages/cli/src/__tests__/status.test.ts index 600a73a..53aa747 100644 --- a/packages/cli/src/__tests__/status.test.ts +++ b/packages/cli/src/__tests__/status.test.ts @@ -4,6 +4,7 @@ import * as path from 'path' import { setupTempConfigDir, writeProjectConfig, + writeAuthConfig, captureConsole, TEST_PROJECT_CONFIG, TEST_LOCAL_CONFIG, @@ -26,12 +27,28 @@ Object.defineProperty(mockChain, 'then', { }, }) +// `status` now refuses to run on a session that cannot read through RLS, so +// these tests have to present a live one. Without the `auth` surface below the +// real session resolver reports `anonymous`, and the command correctly exits 1 +// rather than printing counts an anonymous client could never have fetched. const mockSupabase = { from: vi.fn().mockReturnValue(mockChain), rpc: vi.fn(), + auth: { + setSession: vi.fn().mockResolvedValue({ data: { session: { access_token: 'm' } }, error: null }), + getSession: vi.fn().mockResolvedValue({ data: { session: { access_token: 'm' } }, error: null }), + refreshSession: vi + .fn() + .mockResolvedValue({ data: { session: { access_token: 'm', refresh_token: 'm' } }, error: null }), + onAuthStateChange: vi.fn(() => ({ data: { subscription: { unsubscribe: vi.fn() } } })), + }, } -vi.mock('@tages/shared', () => ({ +// Partial mock: a whole-module replacement has to be updated every time +// `shared` grows an export the CLI reaches, and the breakage shows up as an +// unrelated command test rather than a missing import. +vi.mock('@tages/shared', async (importOriginal) => ({ + ...(await importOriginal()), createSupabaseClient: vi.fn(() => mockSupabase), })) @@ -55,6 +72,7 @@ describe('status command', () => { const setup = setupTempConfigDir() tempConfigDir = setup.configDir cleanupFn = setup.cleanup + writeAuthConfig(tempConfigDir) console_ = captureConsole() vi.clearAllMocks() // Reset mock data diff --git a/packages/cli/src/__tests__/team.test.ts b/packages/cli/src/__tests__/team.test.ts index 897b9c3..8ab2b91 100644 --- a/packages/cli/src/__tests__/team.test.ts +++ b/packages/cli/src/__tests__/team.test.ts @@ -8,20 +8,41 @@ import { } from './helpers.js' // vi.mock factories are hoisted — use vi.hoisted() to share mocks. -const { mockLoadProjectConfig, mockCreateAuthenticatedClient, mockInviteTeamMembers } = - vi.hoisted(() => { - const mockLoadProjectConfig = vi.fn() - const mockCreateAuthenticatedClient = vi.fn() - const mockInviteTeamMembers = vi.fn() - return { mockLoadProjectConfig, mockCreateAuthenticatedClient, mockInviteTeamMembers } - }) +const { + mockLoadProjectConfig, + mockCreateAuthenticatedClient, + mockInviteTeamMembers, + sessionStatus, +} = vi.hoisted(() => { + const mockLoadProjectConfig = vi.fn() + const mockCreateAuthenticatedClient = vi.fn() + const mockInviteTeamMembers = vi.fn() + // Boxed so a test can flip it after the hoisted mock factory has run. + const sessionStatus = { + value: 'authenticated' as 'authenticated' | 'expired' | 'anonymous', + } + return { + mockLoadProjectConfig, + mockCreateAuthenticatedClient, + mockInviteTeamMembers, + sessionStatus, + } +}) vi.mock('../config/project.js', () => ({ loadProjectConfig: mockLoadProjectConfig, })) -vi.mock('../auth/session.js', () => ({ +// Partial mock so `requireLiveSession` stays REAL. Stubbing it would make these +// tests pass against a guard that does nothing; only the client resolution is +// faked here, and the guard runs for real against `sessionStatus`. +vi.mock('../auth/session.js', async (importOriginal) => ({ + ...(await importOriginal()), createAuthenticatedClient: mockCreateAuthenticatedClient, + createAuthenticatedClientWithStatus: async (...args: unknown[]) => ({ + supabase: await mockCreateAuthenticatedClient(...args), + status: sessionStatus.value, + }), })) vi.mock('../auth/invite.js', () => ({ @@ -38,7 +59,7 @@ vi.mock('../config/paths.js', () => ({ getCacheDir: () => path.join(tempConfigDir, 'cache'), })) -import { teamInviteCommand } from '../commands/team.js' +import { teamInviteCommand, teamListCommand } from '../commands/team.js' describe('teamInviteCommand — invitable roles', () => { let console_: ReturnType @@ -208,3 +229,79 @@ describe('teamInviteCommand — invitable roles', () => { ) }) }) + +describe('teamListCommand — expired session', () => { + let console_: ReturnType + let cleanupFn: () => void + + beforeEach(() => { + const setup = setupTempConfigDir() + tempConfigDir = setup.configDir + cleanupFn = setup.cleanup + writeAuthConfig(tempConfigDir) + console_ = captureConsole() + mockLoadProjectConfig.mockReturnValue(TEST_PROJECT_CONFIG) + }) + + afterEach(() => { + sessionStatus.value = 'authenticated' + console_.restore() + cleanupFn() + vi.clearAllMocks() + }) + + it.each([ + ['expired', /session has expired/i], + ['anonymous', /not signed in/i], + ] as const)('fails loudly on a %s session instead of reporting an empty team', async ( + state, + expected, + ) => { + // `anonymous` matters as much as `expired`: it is one step away via + // `tages logout && tages team list`, and it produces byte-identical + // output through a different door. + sessionStatus.value = state + mockCreateAuthenticatedClient.mockResolvedValue({ + from: () => { + throw new Error('must not query without a live session') + }, + }) + + const exit = vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('process.exit') + }) as never) + + await expect(teamListCommand({} as never)).rejects.toThrow('process.exit') + + expect(exit).toHaveBeenCalledWith(1) + const output = console_.errors.join('\n') + console_.logs.join('\n') + expect(output).toMatch(expected) + expect(output).not.toMatch(/No team members/) + exit.mockRestore() + }) + + it('fails loudly instead of reporting an empty team', async () => { + // The bug this pins: an expired session hands back an anonymous client, + // RLS returns zero rows, and `team list` printed + // "No team members. Run `tages team invite ` to add one." + // for a project that had two pending invites — then exited 0. + sessionStatus.value = 'expired' + mockCreateAuthenticatedClient.mockResolvedValue({ + from: () => { + throw new Error('must not query on an expired session') + }, + }) + + const exit = vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('process.exit') + }) as never) + + await expect(teamListCommand({} as never)).rejects.toThrow('process.exit') + + expect(exit).toHaveBeenCalledWith(1) + const output = console_.errors.join('\n') + console_.logs.join('\n') + expect(output).toMatch(/session has expired/i) + expect(output).not.toMatch(/No team members/) + exit.mockRestore() + }) +}) diff --git a/packages/cli/src/auth/session.ts b/packages/cli/src/auth/session.ts index 5771746..7070689 100644 --- a/packages/cli/src/auth/session.ts +++ b/packages/cli/src/auth/session.ts @@ -1,6 +1,7 @@ import * as fs from 'fs' -import { createSupabaseClient } from '@tages/shared' -import { getAuthPath } from '../config/paths.js' +import chalk from 'chalk' +import { createSupabaseClient, persistSessionOnRefresh } from '@tages/shared' +import { getAuthPath, getConfigDir } from '../config/paths.js' import { writeAuthFile } from './store.js' /** @@ -50,6 +51,14 @@ export async function createAuthenticatedClientWithStatus( if (fs.existsSync(authPath)) { const auth = JSON.parse(fs.readFileSync(authPath, 'utf-8')) if (auth.accessToken && auth.refreshToken) { + // The explicit refreshSession() below persists its result, but it is not + // the only thing that can rotate this token: the client auto-refreshes on + // a 30s tick, so any command that outlives one tick (`index`, `snapshot`, + // `query`) rotates in the background. Whoever spends the refresh token + // owns writing the replacement, or the next command finds it already + // used — the failure mode is a session that dies without anyone logging + // out. + persistSessionOnRefresh(supabase, { configDir: getConfigDir() }) await supabase.auth.setSession({ access_token: auth.accessToken, refresh_token: auth.refreshToken, @@ -93,3 +102,35 @@ export async function createAuthenticatedClientWithStatus( return { supabase, status } } + +/** + * Refuse to run a command whose output would be indistinguishable on a dead + * session from a correct answer. + * + * Both `anonymous` and `expired` hand back the anonymous client. Every + * RLS-protected read then returns zero rows, and writes affect zero rows, with + * no error either way. `tages team list` printed + * "No team members. Run `tages team invite ` to add one." + * for a project that had two pending invites, and exited 0 — a wrong answer + * that reads as a working tool. `tages logout && tages team list` reproduces it + * in one step, which is why guarding only `expired` is not enough. + * + * `service-key` is deliberately allowed through: it bypasses RLS entirely and + * is the supported CI/headless path. + * + * @param action verb phrase completing "Cannot ___:", e.g. "list the team". + * @param beforeExit runs immediately before the error prints. Callers holding + * a live ora spinner pass `() => spinner.stop()`, otherwise the red message + * renders on top of a spinner frame. + */ +export function requireLiveSession( + status: SessionStatus, + action: string, + beforeExit?: () => void, +): void { + if (status === 'authenticated' || status === 'service-key') return + beforeExit?.() + const why = status === 'expired' ? 'your session has expired' : 'you are not signed in' + console.error(chalk.red(`Cannot ${action}: ${why}. Run \`tages login\`.`)) + process.exit(1) +} diff --git a/packages/cli/src/auth/store.ts b/packages/cli/src/auth/store.ts index 08e9bfa..f8817ca 100644 --- a/packages/cli/src/auth/store.ts +++ b/packages/cli/src/auth/store.ts @@ -1,64 +1,27 @@ -import * as fs from 'fs' -import { getAuthPath, getConfigDir } from '../config/paths.js' +import { + writeAuthFile as sharedWriteAuthFile, + readAuthFile as sharedReadAuthFile, + type StoredAuth, +} from '@tages/shared' +import { getConfigDir } from '../config/paths.js' -export interface StoredAuth { - accessToken: string - refreshToken: string - userId: string -} +export type { StoredAuth } /** - * The single writer for `~/.config/tages/auth.json`. - * - * It exists because there were two, and only one of them was correct. Both - * `login` and the silent token-refresh in `auth/session.ts` write a live - * Supabase refresh token here, and both used - * `writeFileSync(path, data, { mode: 0o600 })` — where `mode` is the `open(2)` - * CREATION mode, applied only when the call actually creates the file. On an - * existing `auth.json` it is ignored, so a file that was once 0644 stayed 0644 - * while fresh tokens were written into it. Fixing `login` alone missed the path - * that matters more: refresh runs on nearly every command now that - * auto-reconcile is wired into a `preAction` hook, while `login` runs rarely. + * `auth.json` access for the CLI. * - * Permissions are therefore set unconditionally rather than hopefully, and the - * file is opened and truncated BEFORE the mode is corrected and the token - * written, so a fresh secret never sits on disk at looser permissions even for - * an instant. + * The implementation moved to `@tages/shared` when the MCP server had to write + * this file too: Supabase rotates the refresh token on every refresh and marks + * the previous one used, so a long-lived server that refreshes without saving + * the replacement spends the CLI's credential and leaves it permanently + * invalid. Two independent writers for one credential file is exactly the bug + * that once left `auth.json` at 0644, so there is one implementation and these + * wrappers only supply the CLI's own notion of where the config directory is. */ export function writeAuthFile(auth: StoredAuth): void { - const dir = getConfigDir() - fs.mkdirSync(dir, { recursive: true }) - // mkdirSync's mode is likewise creation-only, and it leaves an existing - // directory at whatever it was (0755 by default). - fs.chmodSync(dir, 0o700) - - const path = getAuthPath() - // 'w' truncates first, so any previous token is gone before fchmod runs; the - // window that remains exposes an empty file, never a credential. - const fd = fs.openSync(path, 'w', 0o600) - try { - fs.fchmodSync(fd, 0o600) - fs.writeFileSync(fd, JSON.stringify(auth, null, 2) + '\n') - } finally { - fs.closeSync(fd) - } + sharedWriteAuthFile(auth, { configDir: getConfigDir() }) } -/** - * Read the stored session, or null when absent/unreadable. - * - * Deliberately total: every caller treats a missing identity as "unknown", not - * as an error. A corrupt auth.json must not take down a command that only - * wanted to stamp authorship on a write. - */ export function readAuthFile(): StoredAuth | null { - try { - const raw = fs.readFileSync(getAuthPath(), 'utf-8') - const parsed = JSON.parse(raw) as Partial - return parsed && typeof parsed.userId === 'string' && parsed.userId - ? (parsed as StoredAuth) - : null - } catch { - return null - } + return sharedReadAuthFile({ configDir: getConfigDir() }) } diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 96f00a6..9c20166 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -7,6 +7,7 @@ import { createSupabaseClient, createCloudProject, createLocalProject } from '@t import { getConfigDir, getProjectsDir, getCacheDir, getAuthPath } from '../config/paths.js' import { injectMcpConfig } from '../config/mcp-inject.js' import { runGithubOAuth } from '../auth/github-oauth.js' +import { writeAuthFile } from '../auth/store.js' import { createAuthenticatedClientWithStatus } from '../auth/session.js' import { installPostCommitHook } from '../indexer/install-hook.js' @@ -237,9 +238,13 @@ export async function initCommand(options: InitOptions) { } } - // Save auth credentials - const authData = { accessToken, refreshToken, userId } - fs.writeFileSync(getAuthPath(), JSON.stringify(authData, null, 2) + '\n', { mode: 0o600 }) + // Save auth credentials through the shared writer, never `writeFileSync` + // directly: its `mode` is the open(2) CREATION mode, so on a pre-existing + // 0644 auth.json it is ignored and a live refresh token lands world-readable. + // The write also has to be atomic now that the MCP server writes this file on + // its own schedule — a truncate-then-write racing the server's rename drops + // these freshly-minted tokens into an orphaned inode and reports success. + writeAuthFile({ accessToken, refreshToken, userId }) // Create or find project in Supabase spinner.start('Setting up project...') diff --git a/packages/cli/src/commands/link.ts b/packages/cli/src/commands/link.ts index 8a6fb64..4642bb3 100644 --- a/packages/cli/src/commands/link.ts +++ b/packages/cli/src/commands/link.ts @@ -7,6 +7,7 @@ import { injectMcpConfig } from '../config/mcp-inject.js' import { installPostCommitHook } from '../indexer/install-hook.js' import { createAuthenticatedClient } from '../auth/session.js' import { runGithubOAuth } from '../auth/github-oauth.js' +import { writeAuthFile } from '../auth/store.js' // `init` owns the server-resolution helper; `link` must wire an agent exactly // the way `init` does, so it reuses that one implementation rather than // carrying a second copy that could drift. @@ -150,11 +151,12 @@ async function linkByProjectId(projectId: string, slugOverride: string | undefin spinner.start('Opening browser for GitHub authentication...') try { const auth = await runGithubOAuth(DASHBOARD_URL) - const authDir = path.dirname(authPath) - if (!fs.existsSync(authDir)) { - fs.mkdirSync(authDir, { recursive: true }) - } - fs.writeFileSync(authPath, JSON.stringify(auth, null, 2) + '\n', { mode: 0o600 }) + // Shared writer: it creates the config dir at 0700, sets the file to 0600 + // unconditionally (writeFileSync's `mode` is creation-only and is ignored + // on a pre-existing 0644 file), and writes atomically. Atomicity matters + // most here: this is the command a teammate runs to join, and it can race + // the MCP server, which writes this same file whenever it rotates a token. + writeAuthFile(auth) userId = auth.userId spinner.succeed('Authenticated with GitHub') } catch (err) { diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index eb011ab..23dc125 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -6,6 +6,7 @@ import { createAuthenticatedClient } from '../auth/session.js' import { loadProjectConfig } from '../config/project.js' import { getProjectsDir, getCacheDir, getAuthPath } from '../config/paths.js' import { runGithubOAuth } from '../auth/github-oauth.js' +import { writeAuthFile } from '../auth/store.js' const DASHBOARD_URL = process.env.TAGES_DASHBOARD_URL || 'https://app.tages.ai' const SUPABASE_URL = process.env.TAGES_SUPABASE_URL || 'https://wezagdgpvwfywjoxztfs.supabase.co' @@ -63,7 +64,9 @@ export async function migrateCommand(options: MigrateOptions) { userId = auth.userId spinner.succeed('Authenticated') - fs.writeFileSync(authPath, JSON.stringify({ accessToken, refreshToken, userId }, null, 2) + '\n', { mode: 0o600 }) + // Shared writer: atomic, and it sets 0600 unconditionally rather than + // relying on writeFileSync's creation-only `mode`. See auth/store.ts. + writeAuthFile({ accessToken, refreshToken, userId }) } catch (err) { spinner.fail('Authentication failed') console.error(chalk.red(` ${(err as Error).message}`)) diff --git a/packages/cli/src/commands/onboard.ts b/packages/cli/src/commands/onboard.ts index cb7f073..68128a4 100644 --- a/packages/cli/src/commands/onboard.ts +++ b/packages/cli/src/commands/onboard.ts @@ -1,6 +1,6 @@ import chalk from 'chalk' import ora from 'ora' -import { createAuthenticatedClient } from '../auth/session.js' +import { createAuthenticatedClientWithStatus, requireLiveSession } from '../auth/session.js' import { loadProjectConfig } from '../config/project.js' interface OnboardOptions { @@ -20,7 +20,11 @@ export async function onboardCommand(options: OnboardOptions) { } const spinner = ora('Loading project knowledge...').start() - const supabase = await createAuthenticatedClient(config.supabaseUrl, config.supabaseAnonKey) + const { supabase, status } = await createAuthenticatedClientWithStatus( + config.supabaseUrl, + config.supabaseAnonKey, + ) + requireLiveSession(status, 'build a briefing', () => spinner.stop()) const { data: memories } = await supabase .from('memories') diff --git a/packages/cli/src/commands/status.ts b/packages/cli/src/commands/status.ts index 085324f..c223f81 100644 --- a/packages/cli/src/commands/status.ts +++ b/packages/cli/src/commands/status.ts @@ -2,7 +2,7 @@ import * as fs from 'fs' import * as path from 'path' import { execSync } from 'child_process' import chalk from 'chalk' -import { createAuthenticatedClient } from '../auth/session.js' +import { createAuthenticatedClientWithStatus, requireLiveSession } from '../auth/session.js' import { loadProjectConfig } from '../config/project.js' import { getCachePath, getProjectsDir } from '../config/paths.js' @@ -77,7 +77,11 @@ export async function statusCommand(options: StatusOptions) { } if (config.supabaseUrl && config.supabaseAnonKey) { - const supabase = await createAuthenticatedClient(config.supabaseUrl, config.supabaseAnonKey) + const { supabase, status } = await createAuthenticatedClientWithStatus( + config.supabaseUrl, + config.supabaseAnonKey, + ) + requireLiveSession(status, 'read project status') // Memory counts by type (live only) const { data, error } = await supabase diff --git a/packages/cli/src/commands/team.ts b/packages/cli/src/commands/team.ts index e9ae691..b335b5c 100644 --- a/packages/cli/src/commands/team.ts +++ b/packages/cli/src/commands/team.ts @@ -1,5 +1,5 @@ import chalk from 'chalk' -import { createAuthenticatedClient } from '../auth/session.js' +import { createAuthenticatedClientWithStatus, requireLiveSession } from '../auth/session.js' import { loadProjectConfig } from '../config/project.js' import { getAuthPath } from '../config/paths.js' import { inviteTeamMembers } from '../auth/invite.js' @@ -47,7 +47,11 @@ export async function teamInviteCommand(email: string, options: TeamOptions) { const role = rawRole as ValidRole const auth = JSON.parse(fs.readFileSync(getAuthPath(), 'utf-8')) - const supabase = await createAuthenticatedClient(config.supabaseUrl, config.supabaseAnonKey) + const { supabase, status } = await createAuthenticatedClientWithStatus( + config.supabaseUrl, + config.supabaseAnonKey, + ) + requireLiveSession(status, 'invite') const result = await inviteTeamMembers(supabase, config.projectId, [email], auth.userId, role) @@ -66,7 +70,11 @@ export async function teamListCommand(options: TeamOptions) { process.exit(1) } - const supabase = await createAuthenticatedClient(config.supabaseUrl, config.supabaseAnonKey) + const { supabase, status } = await createAuthenticatedClientWithStatus( + config.supabaseUrl, + config.supabaseAnonKey, + ) + requireLiveSession(status, 'list the team') const { data: members, error } = await supabase .from('team_members') @@ -104,7 +112,11 @@ export async function teamRemoveCommand(emailOrId: string, options: TeamOptions) process.exit(1) } - const supabase = await createAuthenticatedClient(config.supabaseUrl, config.supabaseAnonKey) + const { supabase, status } = await createAuthenticatedClientWithStatus( + config.supabaseUrl, + config.supabaseAnonKey, + ) + requireLiveSession(status, 'remove a member') // Soft-revoke: update status to 'revoked' instead of hard delete const { error } = await supabase @@ -133,7 +145,11 @@ export async function teamRoleCommand(emailOrId: string, role: string, options: process.exit(1) } - const supabase = await createAuthenticatedClient(config.supabaseUrl, config.supabaseAnonKey) + const { supabase, status } = await createAuthenticatedClientWithStatus( + config.supabaseUrl, + config.supabaseAnonKey, + ) + requireLiveSession(status, 'change a role') const { error } = await supabase .from('team_members') diff --git a/packages/cli/src/config/paths.ts b/packages/cli/src/config/paths.ts index 8061ecb..8cb9c13 100644 --- a/packages/cli/src/config/paths.ts +++ b/packages/cli/src/config/paths.ts @@ -1,13 +1,14 @@ import * as path from 'path' import * as os from 'os' -export function getConfigDir(): string { - return path.join(os.homedir(), '.config', 'tages') -} - -export function getAuthPath(): string { - return path.join(getConfigDir(), 'auth.json') -} +// Re-exported, not redefined. The MCP server writes auth.json too, so the +// canonical location lives in `@tages/shared` alongside the writer. Keeping a +// byte-identical copy here worked only by coincidence: the day this module +// grew a TAGES_CONFIG_DIR override, the server's background token-persist +// would have kept writing the default path while every command read the +// overridden one, silently restoring the bug the writer exists to fix. +export { getConfigDir, getAuthPath } from '@tages/shared' +import { getConfigDir } from '@tages/shared' export function getProjectsDir(): string { return path.join(getConfigDir(), 'projects') diff --git a/packages/server/package.json b/packages/server/package.json index 1f456e2..de88058 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -1,6 +1,6 @@ { "name": "@tages/server", - "version": "0.3.4", + "version": "0.3.5", "description": "MCP server for persistent AI agent memory", "main": "./dist/index.js", "bin": { diff --git a/packages/server/scripts/backfill-chunk-embeddings.ts b/packages/server/scripts/backfill-chunk-embeddings.ts index 85a6237..aee758b 100644 --- a/packages/server/scripts/backfill-chunk-embeddings.ts +++ b/packages/server/scripts/backfill-chunk-embeddings.ts @@ -40,7 +40,7 @@ import * as fs from 'fs' import * as path from 'path' import * as os from 'os' import type { SupabaseClient } from '@supabase/supabase-js' -import { createSupabaseClient } from '@tages/shared' +import { createSupabaseClient, persistSessionOnRefresh } from '@tages/shared' import { generateChunkEmbeddings, resolveEmbeddingProvider, @@ -376,6 +376,10 @@ async function buildAuthenticatedClient(supabaseUrl: string, supabaseAnonKey: st if (fs.existsSync(authPath)) { const auth = JSON.parse(fs.readFileSync(authPath, 'utf-8')) if (auth.accessToken && auth.refreshToken) { + // Long-running backfill: the client auto-refreshes on a 30s tick and + // Supabase invalidates the previous refresh token, so persist the + // rotation or this script silently ends the user's CLI session. + persistSessionOnRefresh(supabase) await supabase.auth.setSession({ access_token: auth.accessToken, refresh_token: auth.refreshToken, diff --git a/packages/server/scripts/backfill-embeddings.ts b/packages/server/scripts/backfill-embeddings.ts index d0d58d7..f3f0e8c 100644 --- a/packages/server/scripts/backfill-embeddings.ts +++ b/packages/server/scripts/backfill-embeddings.ts @@ -40,7 +40,7 @@ import * as fs from 'fs' import * as path from 'path' import * as os from 'os' import type { SupabaseClient } from '@supabase/supabase-js' -import { createSupabaseClient } from '@tages/shared' +import { createSupabaseClient, persistSessionOnRefresh } from '@tages/shared' import { generateEmbedding, generateHostedEmbeddingsBatch, @@ -611,6 +611,10 @@ async function buildAuthenticatedClient(supabaseUrl: string, supabaseAnonKey: st if (fs.existsSync(authPath)) { const auth = JSON.parse(fs.readFileSync(authPath, 'utf-8')) if (auth.accessToken && auth.refreshToken) { + // Long-running backfill: the client auto-refreshes on a 30s tick and + // Supabase invalidates the previous refresh token, so persist the + // rotation or this script silently ends the user's CLI session. + persistSessionOnRefresh(supabase) await supabase.auth.setSession({ access_token: auth.accessToken, refresh_token: auth.refreshToken, diff --git a/packages/server/src/__tests__/resolve-project.test.ts b/packages/server/src/__tests__/resolve-project.test.ts index 3cdafa8..1bfb119 100644 --- a/packages/server/src/__tests__/resolve-project.test.ts +++ b/packages/server/src/__tests__/resolve-project.test.ts @@ -9,10 +9,17 @@ const mockCreateCloudProject = vi.fn() const mockCreateLocalProject = vi.fn() const mockCreateSupabaseClient = vi.fn() +const mockPersistSessionOnRefresh = vi.fn(() => () => {}) + vi.mock('@tages/shared', () => ({ createCloudProject: (...args: unknown[]) => mockCreateCloudProject(...args), createLocalProject: (...args: unknown[]) => mockCreateLocalProject(...args), createSupabaseClient: (...args: unknown[]) => mockCreateSupabaseClient(...args), + // The auto-create path registers this before setSession so a rotated refresh + // token reaches auth.json. Omitting it from the mock makes the call throw, + // which resolveProject catches as "cloud auto-create failed" and silently + // downgrades to local mode — the assertion below is what catches that. + persistSessionOnRefresh: () => mockPersistSessionOnRefresh(), })) import { resolveProject } from '../config.js' diff --git a/packages/server/src/config.ts b/packages/server/src/config.ts index 4a81b21..d9acbd0 100644 --- a/packages/server/src/config.ts +++ b/packages/server/src/config.ts @@ -2,7 +2,12 @@ import * as fs from 'fs' import * as path from 'path' import * as os from 'os' import { execSync } from 'child_process' -import { createSupabaseClient, createCloudProject, createLocalProject } from '@tages/shared' +import { + createSupabaseClient, + createCloudProject, + createLocalProject, + persistSessionOnRefresh, +} from '@tages/shared' import type { ProjectConfig } from '@tages/shared' export type { ProjectConfig } @@ -214,6 +219,11 @@ export async function resolveProject(cwd: string): Promise { const supabaseUrl = process.env.TAGES_SUPABASE_URL || DEFAULT_SUPABASE_URL const supabaseAnonKey = process.env.TAGES_SUPABASE_ANON_KEY || DEFAULT_SUPABASE_ANON_KEY const supabase = createSupabaseClient(supabaseUrl, supabaseAnonKey) + // Same reason as the boot path in index.ts: setSession refreshes on the + // spot when the stored access token has expired, and the rotated refresh + // token has to reach disk or auth.json is spent. Registering twice on the + // shared client instance is harmless — the second write is a no-op. + persistSessionOnRefresh(supabase) await supabase.auth.setSession({ access_token: auth.accessToken, refresh_token: auth.refreshToken, diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 15f27d6..92cccfc 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -1,7 +1,7 @@ #!/usr/bin/env node import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' -import { createSupabaseClient } from '@tages/shared' +import { createSupabaseClient, persistSessionOnRefresh } from '@tages/shared' import { z } from 'zod' import * as fs from 'fs' @@ -120,6 +120,11 @@ async function main() { if (fs.existsSync(authPath)) { const auth = JSON.parse(fs.readFileSync(authPath, 'utf-8')) if (auth.accessToken && auth.refreshToken) { + // Register BEFORE setSession. An expired access token makes + // setSession refresh immediately, and that first rotation is the one + // that invalidates the token on disk — miss it and auth.json is + // already dead by the time the listener exists. + persistSessionOnRefresh(supabaseClient) await supabaseClient.auth.setSession({ access_token: auth.accessToken, refresh_token: auth.refreshToken, diff --git a/packages/shared/package.json b/packages/shared/package.json index d070a8f..60fd9bc 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -1,14 +1,14 @@ { "name": "@tages/shared", - "version": "0.2.3", + "version": "0.2.4", "main": "./dist/index.js", "types": "./dist/index.d.ts", "files": [ "dist" ], "scripts": { - "build": "tsc", - "prepublishOnly": "tsc", + "build": "tsc -p tsconfig.build.json", + "prepublishOnly": "tsc -p tsconfig.build.json", "test": "vitest run --passWithNoTests", "typecheck": "tsc --noEmit" }, @@ -35,6 +35,7 @@ "@supabase/supabase-js": "^2.49.0" }, "devDependencies": { + "@types/node": "^20.19.0", "typescript": "^6.0.2", "vitest": "^4.1.5" } diff --git a/packages/shared/src/__tests__/auth-persist.test.ts b/packages/shared/src/__tests__/auth-persist.test.ts new file mode 100644 index 0000000..db3867a --- /dev/null +++ b/packages/shared/src/__tests__/auth-persist.test.ts @@ -0,0 +1,253 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import * as fs from 'fs' +import * as os from 'os' +import * as path from 'path' + +/** + * These cover the defect that killed a real session four days after login: + * Supabase rotates the refresh token on every refresh and marks the old one + * used, so a process that refreshes without writing the replacement to + * auth.json leaves the on-disk credential permanently spent. The observable + * symptom is `refresh_token_already_used` with an auth.json whose mtime is + * still the original login. + */ + +let tmpHome: string +let realHome: string | undefined + +function jwt(exp: number): string { + const body = Buffer.from(JSON.stringify({ exp, sub: 'u1' })).toString('base64url') + return `h.${body}.sig` +} + +function authPath(): string { + return path.join(tmpHome, '.config', 'tages', 'auth.json') +} + +function seed(auth: Record): void { + fs.mkdirSync(path.dirname(authPath()), { recursive: true }) + fs.writeFileSync(authPath(), JSON.stringify(auth)) +} + +beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'tages-auth-')) + // os.homedir() reads $HOME on POSIX, so redirecting the env var reroutes the + // real path helpers. vi.spyOn cannot touch `os` here — an ESM namespace + // object is not configurable — and mocking the module would also stub the + // fs-adjacent helpers these tests rely on. + realHome = process.env.HOME + process.env.HOME = tmpHome +}) + +afterEach(() => { + if (realHome === undefined) delete process.env.HOME + else process.env.HOME = realHome + fs.rmSync(tmpHome, { recursive: true, force: true }) +}) + +describe('persistRotatedTokens', () => { + it('writes a newer token over the stored one', async () => { + const { persistRotatedTokens } = await import('../auth-persist') + const { readAuthFile } = await import('../auth-store') + seed({ accessToken: jwt(1000), refreshToken: 'old', userId: 'u1' }) + + persistRotatedTokens(jwt(2000), 'rotated', 'u1') + + const stored = readAuthFile() + expect(stored?.refreshToken).toBe('rotated') + }) + + it('refuses to overwrite a fresher session written by another process', async () => { + const { persistRotatedTokens } = await import('../auth-persist') + const { readAuthFile } = await import('../auth-store') + // A `tages login` in another terminal has just written a newer session. + seed({ accessToken: jwt(9000), refreshToken: 'from-fresh-login', userId: 'u1' }) + + // A long-lived server that booted on the OLD session rotates its own copy. + persistRotatedTokens(jwt(2000), 'from-stale-server', 'u1') + + expect(readAuthFile()?.refreshToken).toBe('from-fresh-login') + }) + + it('treats an unparseable stored token as oldest rather than blocking the write', async () => { + const { persistRotatedTokens } = await import('../auth-persist') + const { readAuthFile } = await import('../auth-store') + seed({ accessToken: 'not-a-jwt', refreshToken: 'junk', userId: 'u1' }) + + persistRotatedTokens(jwt(2000), 'rotated', 'u1') + + expect(readAuthFile()?.refreshToken).toBe('rotated') + }) + + it('keeps the stored userId when the refreshed session carries none', async () => { + const { persistRotatedTokens } = await import('../auth-persist') + const { readAuthFile } = await import('../auth-store') + seed({ accessToken: jwt(1000), refreshToken: 'old', userId: 'u-keep' }) + + persistRotatedTokens(jwt(2000), 'rotated', '') + + expect(readAuthFile()?.userId).toBe('u-keep') + }) +}) + +describe('writeAuthFile', () => { + it('writes 0600 even when auth.json already exists as 0644', async () => { + const { writeAuthFile } = await import('../auth-store') + seed({ accessToken: 'a', refreshToken: 'b', userId: 'u1' }) + fs.chmodSync(authPath(), 0o644) + + writeAuthFile({ accessToken: jwt(2000), refreshToken: 'r', userId: 'u1' }) + + expect(fs.statSync(authPath()).mode & 0o777).toBe(0o600) + }) + + it('leaves no readable window: a concurrent reader never sees a truncated file', async () => { + const { writeAuthFile } = await import('../auth-store') + seed({ accessToken: jwt(1000), refreshToken: 'old', userId: 'u1' }) + + // The write is temp-file + rename, so at every instant the final path holds + // one complete document. Assert no temp file survives and the content parses. + writeAuthFile({ accessToken: jwt(2000), refreshToken: 'new', userId: 'u1' }) + + const dir = path.dirname(authPath()) + expect(fs.readdirSync(dir).filter((f) => f.includes('.tmp.'))).toEqual([]) + expect(() => JSON.parse(fs.readFileSync(authPath(), 'utf-8'))).not.toThrow() + }) +}) + +describe('persistSessionOnRefresh', () => { + it('persists on TOKEN_REFRESHED and ignores other auth events', async () => { + const { persistSessionOnRefresh } = await import('../auth-persist') + const { readAuthFile } = await import('../auth-store') + seed({ accessToken: jwt(1000), refreshToken: 'old', userId: 'u1' }) + + let handler: ((e: string, s: unknown) => void) | null = null + const supabase = { + auth: { + onAuthStateChange: (cb: (e: string, s: unknown) => void) => { + handler = cb + return { data: { subscription: { unsubscribe: () => {} } } } + }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any + + persistSessionOnRefresh(supabase) + expect(handler).not.toBeNull() + + handler!('SIGNED_IN', { + access_token: jwt(5000), + refresh_token: 'ignored', + user: { id: 'u1' }, + }) + expect(readAuthFile()?.refreshToken).toBe('old') + + handler!('TOKEN_REFRESHED', { + access_token: jwt(5000), + refresh_token: 'rotated', + user: { id: 'u1' }, + }) + expect(readAuthFile()?.refreshToken).toBe('rotated') + }) + + it('ignores a refreshed session missing access_token or refresh_token', async () => { + const { persistSessionOnRefresh } = await import('../auth-persist') + const { readAuthFile } = await import('../auth-store') + seed({ accessToken: jwt(1000), refreshToken: 'old', userId: 'u1' }) + + let handler: ((e: string, s: unknown) => void) | null = null + const supabase = { + auth: { + onAuthStateChange: (cb: (e: string, s: unknown) => void) => { + handler = cb + return { data: { subscription: { unsubscribe: () => {} } } } + }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any + + persistSessionOnRefresh(supabase) + expect(handler).not.toBeNull() + + // Supabase types mark both fields required, but a defensive guard exists + // because the callback receives whatever the client hands it at runtime. + handler!('TOKEN_REFRESHED', { refresh_token: 'rotated', user: { id: 'u1' } }) + expect(readAuthFile()?.refreshToken).toBe('old') + + handler!('TOKEN_REFRESHED', { access_token: jwt(5000), user: { id: 'u1' } }) + expect(readAuthFile()?.refreshToken).toBe('old') + }) + + it('logs and does not throw when the write fails, instead of crashing the process', async () => { + const { persistSessionOnRefresh } = await import('../auth-persist') + const { readAuthFile } = await import('../auth-store') + // Pre-create auth.json as a directory so the atomic rename inside + // writeAuthFile fails with EISDIR — a real write failure, no fs mocking. + fs.mkdirSync(authPath(), { recursive: true }) + + let handler: ((e: string, s: unknown) => void) | null = null + const supabase = { + auth: { + onAuthStateChange: (cb: (e: string, s: unknown) => void) => { + handler = cb + return { data: { subscription: { unsubscribe: () => {} } } } + }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any + + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + persistSessionOnRefresh(supabase) + expect(handler).not.toBeNull() + + expect(() => { + handler!('TOKEN_REFRESHED', { + access_token: jwt(5000), + refresh_token: 'rotated', + user: { id: 'u1' }, + }) + }).not.toThrow() + + expect(errorSpy).toHaveBeenCalledTimes(1) + expect(errorSpy.mock.calls[0][0]).toMatch(/could not persist refreshed session/) + // The in-memory session still works; only the on-disk copy is unaffected — + // confirm the directory sentinel is untouched, not silently replaced. + expect(fs.statSync(authPath()).isDirectory()).toBe(true) + expect(readAuthFile()).toBeNull() + + errorSpy.mockRestore() + }) +}) + +describe('readAuthFile', () => { + it('returns null when auth.json does not exist', async () => { + const { readAuthFile } = await import('../auth-store') + // No seed() call — the config dir itself is absent. + expect(readAuthFile()).toBeNull() + }) + + it('returns null when auth.json contains unparseable JSON', async () => { + const { readAuthFile } = await import('../auth-store') + fs.mkdirSync(path.dirname(authPath()), { recursive: true }) + fs.writeFileSync(authPath(), '{ not valid json') + + expect(readAuthFile()).toBeNull() + }) +}) + +describe('writeAuthFile rename failure', () => { + it('cleans up the temp file and propagates the error when rename fails', async () => { + const { writeAuthFile } = await import('../auth-store') + // Pre-create the final path as a directory: renameSync(file, dir) fails + // with EISDIR on POSIX, giving a real rename failure without mocking fs. + fs.mkdirSync(authPath(), { recursive: true }) + + expect(() => writeAuthFile({ accessToken: jwt(2000), refreshToken: 'r', userId: 'u1' })).toThrow() + + const dir = path.dirname(authPath()) + expect(fs.readdirSync(dir).filter((f) => f.includes('.tmp.'))).toEqual([]) + // The directory sentinel must still be there — writeAuthFile must not have + // clobbered it or left a partial artifact behind. + expect(fs.statSync(authPath()).isDirectory()).toBe(true) + }) +}) diff --git a/packages/shared/src/auth-persist.ts b/packages/shared/src/auth-persist.ts new file mode 100644 index 0000000..b134dc9 --- /dev/null +++ b/packages/shared/src/auth-persist.ts @@ -0,0 +1,100 @@ +import type { SupabaseClient } from '@supabase/supabase-js' +import { getAuthPath, readAuthFile, writeAuthFile, type AuthPathOptions } from './auth-store' + +/** + * Decode the `exp` claim of a Supabase access token without verifying it. + * + * Only used to order two tokens against each other, never to trust one. A + * malformed token sorts as "oldest" so it can never win the comparison in + * {@link persistRotatedTokens} and overwrite a good session. + */ +function tokenExpiry(accessToken: string | undefined): number { + if (!accessToken) return 0 + try { + const payload = accessToken.split('.')[1] + if (!payload) return 0 + const json = Buffer.from(payload, 'base64url').toString('utf-8') + const exp = (JSON.parse(json) as { exp?: number }).exp + return typeof exp === 'number' ? exp : 0 + } catch { + return 0 + } +} + +/** + * Write refreshed Supabase tokens back to `auth.json`. + * + * Supabase rotates the refresh token on every refresh and marks the previous + * one used; presenting it again fails with `refresh_token_already_used`. Any + * process that refreshes therefore OWNS the job of persisting the replacement, + * or it silently destroys the on-disk session for every other process. + * + * The staleness guard matters because more than one process shares this file. + * A long-lived MCP server that booted with an old session must not stamp its + * lineage over tokens a fresh `tages login` wrote a moment ago, so a write only + * happens when the incoming access token outlives the stored one. + */ +export function persistRotatedTokens( + accessToken: string, + refreshToken: string, + userId: string, + opts: AuthPathOptions = {}, +): void { + const stored = readAuthFile(opts) + // `>=`, not `>`, and that is load-bearing. `createSupabaseClient` memoises one + // client per process, so several callers can register a listener on the same + // instance and every one of them fires on a single TOKEN_REFRESHED. Equal + // expiries mean the token on disk is already the one being offered, so + // rejecting the tie makes the redundant handlers no-ops instead of N writes. + if (stored && tokenExpiry(stored.accessToken) >= tokenExpiry(accessToken)) return + writeAuthFile({ accessToken, refreshToken, userId: userId || stored?.userId || '' }, opts) +} + +/** + * Keep `auth.json` in step with a client that refreshes on its own. + * + * `createClient` defaults to `autoRefreshToken: true` with in-memory storage in + * Node, so a long-running process refreshes on a 30-second tick and keeps the + * result nowhere. Call this immediately after `setSession()` on any client that + * outlives a single command. + * + * Returns an unsubscribe function. + */ +const registered = new WeakSet() + +export function persistSessionOnRefresh( + supabase: SupabaseClient, + opts: AuthPathOptions = {}, +): () => void { + // `createSupabaseClient` memoises a single client per process, and several + // code paths call this on it (the preAction auto-reconcile hook, then the + // command itself). Registering once per client keeps one handler doing the + // work instead of N handlers racing to write the same file. + if (registered.has(supabase)) return () => {} + registered.add(supabase) + + const { data } = supabase.auth.onAuthStateChange((event, session) => { + if (event !== 'TOKEN_REFRESHED') return + if (!session?.access_token || !session?.refresh_token) return + try { + persistRotatedTokens( + session.access_token, + session.refresh_token, + session.user?.id ?? '', + opts, + ) + } catch (err) { + // Never take the process down over a credential-cache write. The session + // still works in memory; only the next process pays for the loss. + console.error( + `[tages] Warning: could not persist refreshed session to ` + + `${getAuthPath(opts.configDir)} — ${(err as Error).message}`, + ) + } + }) + return () => { + registered.delete(supabase) + data.subscription.unsubscribe() + } +} + diff --git a/packages/shared/src/auth-store.ts b/packages/shared/src/auth-store.ts new file mode 100644 index 0000000..cfe91a4 --- /dev/null +++ b/packages/shared/src/auth-store.ts @@ -0,0 +1,107 @@ +import * as fs from 'fs' +import * as os from 'os' +import * as path from 'path' + +export interface StoredAuth { + accessToken: string + refreshToken: string + userId: string +} + +export function getConfigDir(): string { + return path.join(os.homedir(), '.config', 'tages') +} + +export function getAuthPath(configDir?: string): string { + return path.join(configDir ?? getConfigDir(), 'auth.json') +} + +/** + * Where the credential lives. + * + * `configDir` is injectable because the CLI owns its own `config/paths` module + * and its tests redirect the whole thing; without the seam there would be two + * implementations of one path, which is how the file ended up with two writers + * in the first place. Callers that do not care get the real location. + */ +export interface AuthPathOptions { + configDir?: string +} + +/** + * The single writer for `~/.config/tages/auth.json`. + * + * It lives in `shared`, not in the CLI, because the MCP server must write here + * too. Supabase rotates the refresh token on every use and invalidates the old + * one immediately; a process that refreshes without persisting the replacement + * leaves the on-disk token permanently spent. The server is long-lived and + * `auto-refresh` is on by default, so it refreshes on a 30s tick — which is how + * a session that was minted by `tages login` died roughly an hour later with + * `refresh_token_already_used` and no user action at all. + * + * Permissions are set unconditionally rather than hopefully. Both `login` and + * the silent refresh in `auth/session.ts` used + * `writeFileSync(path, data, { mode: 0o600 })` — where `mode` is the `open(2)` + * CREATION mode, applied only when the call actually creates the file. On an + * existing `auth.json` it is ignored, so a file that was once 0644 stayed 0644 + * while fresh tokens were written into it. + * + * The write is atomic (temp file in the same directory, then `rename`). It has + * to be: now that a background server writes this file on its own schedule, + * a truncate-then-write would let a concurrently-starting CLI command read a + * half-written or empty file. `rename(2)` within one filesystem is atomic, so + * a reader sees either the whole old file or the whole new one. The temp file + * is created 0600 so a live token never exists at looser permissions, not even + * for an instant. + */ +export function writeAuthFile(auth: StoredAuth, opts: AuthPathOptions = {}): void { + const dir = opts.configDir ?? getConfigDir() + fs.mkdirSync(dir, { recursive: true }) + // mkdirSync's mode is likewise creation-only, and it leaves an existing + // directory at whatever it was (0755 by default). + fs.chmodSync(dir, 0o700) + + const finalPath = getAuthPath(dir) + // Same directory, so the rename below stays on one filesystem. The pid keeps + // two concurrent writers from sharing a temp file. + const tmpPath = `${finalPath}.tmp.${process.pid}` + // One try covering write AND rename, not just rename: if writeFileSync fails + // (ENOSPC, EIO, EDQUOT) the temp file is already created and would otherwise + // be left on disk holding a partial live refresh token that nothing reaps. + try { + const fd = fs.openSync(tmpPath, 'w', 0o600) + try { + fs.fchmodSync(fd, 0o600) + fs.writeFileSync(fd, JSON.stringify(auth, null, 2) + '\n') + } finally { + fs.closeSync(fd) + } + fs.renameSync(tmpPath, finalPath) + } catch (err) { + try { + fs.unlinkSync(tmpPath) + } catch { + // Best effort — the original failure is the one worth reporting. + } + throw err + } +} + +/** + * Read the stored session, or null when absent/unreadable. + * + * Deliberately total: every caller treats a missing identity as "unknown", not + * as an error. A corrupt auth.json must not take down a command that only + * wanted to stamp authorship on a write. + */ +export function readAuthFile(opts: AuthPathOptions = {}): StoredAuth | null { + try { + const raw = fs.readFileSync(getAuthPath(opts.configDir ?? getConfigDir()), 'utf-8') + const parsed = JSON.parse(raw) as Partial + return parsed && typeof parsed.userId === 'string' && parsed.userId + ? (parsed as StoredAuth) + : null + } catch { + return null + } +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index b2e7930..8f3d483 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -17,3 +17,12 @@ export { RELEVANCE_MIN_CANDIDATES, } from './relevance' export type { RelevanceVerdict } from './relevance' + +export { + writeAuthFile, + readAuthFile, + getAuthPath, + getConfigDir, +} from './auth-store' +export type { StoredAuth } from './auth-store' +export { persistSessionOnRefresh, persistRotatedTokens } from './auth-persist' diff --git a/packages/shared/tsconfig.build.json b/packages/shared/tsconfig.build.json new file mode 100644 index 0000000..5799d0c --- /dev/null +++ b/packages/shared/tsconfig.build.json @@ -0,0 +1,7 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": false + }, + "exclude": ["src/**/__tests__/**"] +} diff --git a/packages/shared/tsconfig.json b/packages/shared/tsconfig.json index 753762f..8400032 100644 --- a/packages/shared/tsconfig.json +++ b/packages/shared/tsconfig.json @@ -2,7 +2,9 @@ "compilerOptions": { "target": "ES2022", "module": "commonjs", - "lib": ["ES2022"], + "lib": [ + "ES2022" + ], "declaration": true, "outDir": "./dist", "rootDir": "./src", @@ -10,7 +12,12 @@ "esModuleInterop": true, "skipLibCheck": true, "forceConsistentCasingInFileNames": true, - "resolveJsonModule": true + "resolveJsonModule": true, + "types": [ + "node" + ] }, - "include": ["src/**/*"] + "include": [ + "src/**/*" + ] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ea6a16c..529f272 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -220,12 +220,15 @@ importers: specifier: ^2.49.0 version: 2.101.1 devDependencies: + '@types/node': + specifier: ^20.19.0 + version: 20.19.39 typescript: specifier: ^6.0.2 version: 6.0.2 vitest: specifier: ^4.1.5 - version: 4.1.5(@opentelemetry/api@1.9.1)(@types/node@25.6.0)(@vitest/coverage-v8@4.1.2)(vite@8.0.3(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)(@types/node@25.6.0)(esbuild@0.27.7)(jiti@2.6.1)(terser@5.46.2)(tsx@4.21.0)) + version: 4.1.5(@opentelemetry/api@1.9.1)(@types/node@20.19.39)(vite@8.0.3(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)(@types/node@20.19.39)(esbuild@0.27.7)(jiti@2.6.1)(terser@5.46.2)(tsx@4.21.0)) packages: