diff --git a/e2e/tests/sdm-browser-login.test.ts b/e2e/tests/sdm-browser-login.test.ts index b70331da..893a406d 100644 --- a/e2e/tests/sdm-browser-login.test.ts +++ b/e2e/tests/sdm-browser-login.test.ts @@ -93,14 +93,6 @@ exit 0 `, ); chmodSync(fakeSdm, 0o755); - - // sdmEmail must be set or the orchestrator returns needs-manual before - // ever launching a browser (the email preflight). - mkdirSync(join(home, ".mattstack", "rt"), { recursive: true }); - writeFileSync( - join(home, ".mattstack", "rt", "secrets.json"), - JSON.stringify({ sdmEmail: "nobody@example.test" }, null, 2), - ); }); afterEach(async () => { @@ -117,10 +109,13 @@ exit 0 test("silent path drives the fake SAML flow to completion", async () => { const fakeSdm = join(home, "fakebin", "sdm"); + // sdmEmail must be set or the orchestrator returns needs-manual before + // ever launching a browser (the email preflight) — SDM_EMAIL overrides + // the encrypted store for exactly this preflight read. session = await startInteractive({ args: ["sdm", "login"], home, - env: { RT_SDM_BIN: fakeSdm }, + env: { RT_SDM_BIN: fakeSdm, SDM_EMAIL: "nobody@example.test" }, timeoutMs: 20_000, }); diff --git a/extensions/vscode/rt-context/src/secrets.ts b/extensions/vscode/rt-context/src/secrets.ts index 58d596fe..98c12e74 100644 --- a/extensions/vscode/rt-context/src/secrets.ts +++ b/extensions/vscode/rt-context/src/secrets.ts @@ -1,17 +1,15 @@ /** * Shared secrets reader for the VS Code extension. * - * Reads go through the rt daemon's `secrets:read` verb (RT-32) — the daemon - * owns the encrypted store and, during the migration, its own plaintext - * fallback (lib/linear.ts's loadSecrets), so this module no longer opens - * ~/.mattstack/rt/secrets.json directly. VS Code's secret store is the - * fallback when the daemon is unreachable or the key was never set. + * Reads go through the rt daemon's `secrets:read` verb — the daemon owns + * the encrypted sops/age store (lib/linear.ts's loadSecrets), the only + * source. VS Code's secret store is the fallback when the daemon is + * unreachable or the key was never set. * - * Writes: there is no secrets:write daemon verb yet, and this module no - * longer writes ~/.mattstack/rt/secrets.json either (a plaintext write here - * would be a silently-ignored no-op from rt's perspective — the file is - * being retired as a write target). `setSecret` tells the user what to run - * instead rather than pretending to save. + * Writes: there is no secrets:write daemon verb, and this module never + * writes to disk itself — the encrypted store is written only via + * `rt secrets set`. `showCantSaveSecretMessage` tells the user what to run + * instead of pretending to save. */ import * as vscode from 'vscode'; @@ -80,11 +78,10 @@ export async function getSecret( } /** - * RT-32: no longer writes ~/.mattstack/rt/secrets.json (that file is being - * retired as a write target — the encrypted store, written only via - * `rt secrets set`, is the source of truth). There is nothing left to - * collect from the user here, so the command shows this directed message - * immediately rather than prompting for a token it can't save. + * This module never writes a secret itself — the encrypted store is + * written only via `rt secrets set`. There is nothing left to collect from + * the user here, so the command shows this directed message immediately + * rather than prompting for a token it can't save. */ export function showCantSaveSecretMessage(key: DaemonSecretKey): void { vscode.window.showErrorMessage( diff --git a/lib/__tests__/linear.test.ts b/lib/__tests__/linear.test.ts index 27b1f500..51a8191e 100644 --- a/lib/__tests__/linear.test.ts +++ b/lib/__tests__/linear.test.ts @@ -1,12 +1,11 @@ -import { describe, test, expect, afterEach, beforeEach, spyOn } from "bun:test"; -import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "fs"; +import { describe, test, expect, afterEach, beforeEach } from "bun:test"; +import { mkdtempSync, realpathSync, rmSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { pickStartedState, fetchMyTodoTickets, searchTickets, loadSecrets, saveSecret, saveTeamConfig, getTeamConfig, } from "../linear.ts"; -import { rtDir } from "../rt-paths.ts"; import { secretsFilePath, resetSecretsMemo, type SecretsExecResult, type SecretsExecSeam, type SecretsSeams } from "../secrets/store.ts"; import type { AgeExecResult, AgeKeySeam } from "../home/age-key.ts"; @@ -101,7 +100,7 @@ function fakeSecretsSeams(seedDomains: Record> = return { ageKeySeam: fakeAgeKeySeam(), execSeam }; } -describe("loadSecrets / saveSecret / saveTeamConfig / getTeamConfig — encrypted store + plaintext fallback", () => { +describe("loadSecrets / saveSecret / saveTeamConfig / getTeamConfig — encrypted store", () => { const origHome = process.env.HOME; let home: string; @@ -116,39 +115,16 @@ describe("loadSecrets / saveSecret / saveTeamConfig / getTeamConfig — encrypte rmSync(home, { recursive: true, force: true }); }); - function writePlaintext(secrets: Record): void { - mkdirSync(rtDir(), { recursive: true }); - writeFileSync(join(rtDir(), "secrets.json"), JSON.stringify(secrets, null, 2)); - } - - test("encrypted store value wins over plaintext when both are present", async () => { - writePlaintext({ linearApiKey: "plaintext-key", gitlabToken: "plaintext-gitlab" }); - const seams = fakeSecretsSeams({ rt: { linearApiKey: "encrypted-key" } }); - - const secrets = await loadSecrets(seams); - - expect(secrets.linearApiKey).toBe("encrypted-key"); - expect(secrets.gitlabToken).toBe("plaintext-gitlab"); // not yet in the encrypted store - }); - - test("falls back to plaintext entirely when the encrypted domain doesn't exist yet (transition state)", async () => { - writePlaintext({ linearApiKey: "plaintext-key" }); - const seams = fakeSecretsSeams(); // no "rt" domain seeded -> file doesn't exist - - expect((await loadSecrets(seams)).linearApiKey).toBe("plaintext-key"); - }); - - test("returns {} when neither store has anything", async () => { + test("returns {} when the store has nothing", async () => { expect(await loadSecrets(fakeSecretsSeams())).toEqual({}); }); - test("saveSecret writes to the encrypted store and never touches the plaintext file", async () => { + test("saveSecret writes to the encrypted store", async () => { const seams = fakeSecretsSeams(); await saveSecret("linearApiKey", "new-key", seams); expect((await loadSecrets(seams)).linearApiKey).toBe("new-key"); - expect(existsSync(join(rtDir(), "secrets.json"))).toBe(false); }); test("saveTeamConfig writes both linearTeamId and linearTeamKey to the encrypted store", async () => { @@ -186,25 +162,7 @@ describe("loadSecrets / saveSecret / saveTeamConfig / getTeamConfig — encrypte }; } - test("an encrypted-store read failure logs what happened and falls back to plaintext WHEN the plaintext file still exists", async () => { - writePlaintext({ gitlabToken: "plaintext-gitlab" }); - const seams: SecretsSeams = { ageKeySeam: fakeAgeKeySeam(), execSeam: brokenExecSeam() }; - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - - try { - const secrets = await loadSecrets(seams); - expect(secrets.gitlabToken).toBe("plaintext-gitlab"); - expect(errorSpy).toHaveBeenCalledTimes(1); - const logged = errorSpy.mock.calls[0]!.join(" "); - expect(logged).toContain("encrypted store unreadable"); - expect(logged).toContain("using plaintext secrets.json"); - } finally { - errorSpy.mockRestore(); - } - }); - - test("an encrypted-store read failure THROWS when the plaintext file is absent — never silently returns {}", async () => { - // No writePlaintext() call: the transition-only fallback file doesn't exist. + test("an encrypted-store read failure propagates — never silently returns {}", async () => { const seams: SecretsSeams = { ageKeySeam: fakeAgeKeySeam(), execSeam: brokenExecSeam() }; await expect(loadSecrets(seams)).rejects.toThrow(/decryption failed/); diff --git a/lib/daemon/__tests__/secrets-handler.test.ts b/lib/daemon/__tests__/secrets-handler.test.ts index e2e50460..a0354c48 100644 --- a/lib/daemon/__tests__/secrets-handler.test.ts +++ b/lib/daemon/__tests__/secrets-handler.test.ts @@ -1,8 +1,6 @@ /** - * secrets:forge-token (MAT-33): the grant gate is the point of the verb, so - * every test here is about who gets refused. The old world was gitq opening - * ~/.mattstack/rt/secrets.json itself, where every caller got every token with no - * grant check anywhere. + * secrets:forge-token: the grant gate is the point of the verb, so every + * test here is about who gets refused. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "fs"; @@ -59,6 +57,15 @@ describe("secrets:forge-token", () => { expect((await h({ repoName: "", forge: "gitlab" })).ok).toBe(false); expect((await h({ repoName: "gitq", forge: "bitbucket" as any })).ok).toBe(false); }); + + test("a secrets-reader throw (e.g. an unreadable encrypted store) surfaces as a rejected promise, never a fallback token", async () => { + const h = createSecretsHandlers(fakeCtx, { + tracking: () => ({ gitq: { mode: "live", caches: ["branches"] } }) as any, + secrets: () => { throw new Error("decryption failed"); }, + })["secrets:forge-token"]; + + await expect(h({ repoName: "gitq", forge: "gitlab" })).rejects.toThrow("decryption failed"); + }); }); /** @@ -196,6 +203,12 @@ describe("secrets:read", () => { expect(res).toEqual({ ok: true, data: { linearApiKey: "lin_api_x" } }); expect(deckCalled).toBe(false); }); + + test("an extensionSecrets throw (e.g. an unreadable encrypted store) surfaces as a rejected promise, never a partial ok", async () => { + const h = readHandler({ extensionSecrets: async () => { throw new Error("decryption failed"); } }); + + await expect(h({ token: "test-token" })).rejects.toThrow("decryption failed"); + }); }); // scope: "deck" reads a wholly different encrypted domain (lib/secrets/ diff --git a/lib/daemon/freshness.ts b/lib/daemon/freshness.ts index 910b262b..756972d0 100644 --- a/lib/daemon/freshness.ts +++ b/lib/daemon/freshness.ts @@ -268,7 +268,7 @@ export async function getRepoContext( } const secrets = await loadSecrets(); if (!secrets.gitlabToken) { - throw new Error("missing gitlabToken in ~/.mattstack/rt/secrets.json (run rt secret set gitlabToken )"); + throw new Error("missing gitlabToken (run: rt secrets set rt gitlabToken)"); } const remoteUrl = getRemoteUrl(repoPath); if (!remoteUrl) { diff --git a/lib/daemon/handlers/secrets.ts b/lib/daemon/handlers/secrets.ts index d52310bd..969384f7 100644 --- a/lib/daemon/handlers/secrets.ts +++ b/lib/daemon/handlers/secrets.ts @@ -118,7 +118,7 @@ const SECRETS_KEY: Record = { export interface SecretsHandlerOverrides { tracking?: () => RepoTracking; secrets?: () => { gitlabToken?: string; githubToken?: string } | Promise<{ gitlabToken?: string; githubToken?: string }>; - /** Defaults to `loadSecrets` (the full encrypted-store + plaintext-fallback loader) for secrets:read's "extension" scope. */ + /** Defaults to `loadSecrets` (the `rt` encrypted domain) for secrets:read's "extension" scope. */ extensionSecrets?: () => Promise<{ linearApiKey?: string; gitlabToken?: string }>; /** Defaults to `loadDeckSecrets` (the `deck` encrypted domain) for secrets:read's "deck" scope. */ deckSecrets?: () => Promise<{ cfApiToken?: string; cfZoneId?: string }>; @@ -164,7 +164,7 @@ export function createSecretsHandlers( const token = (await secrets())[SECRETS_KEY[forge]]; if (!token) { - return { ok: false as const, error: `no ${forge} token in ~/.mattstack/rt/secrets.json (${SECRETS_KEY[forge]})` }; + return { ok: false as const, error: `no ${forge} token configured (run: rt secrets set rt ${SECRETS_KEY[forge]})` }; } ctx.log.info({ repoName, forge }, "secrets:forge-token grant-gated read"); diff --git a/lib/linear.ts b/lib/linear.ts index 30199c36..937bfae6 100644 --- a/lib/linear.ts +++ b/lib/linear.ts @@ -8,14 +8,11 @@ * 4. Cache results in memory (5-minute TTL) * * Secrets: the sops-backed store (lib/secrets/store.ts, domain "rt") is the - * source of truth; ~/.mattstack/rt/secrets.json is a transition-only fallback - * (RT-32) read through readPlaintextSecretsFallback — the one function to - * delete once the live import (a later lane) retires the plaintext file. + * only source. loadSecrets propagates a decrypt failure rather than + * degrading to any other source — callers must not treat a thrown error + * from it as "no secrets configured." */ -import { existsSync, readFileSync } from "fs"; -import { join } from "path"; -import { rtDir } from "./rt-paths.ts"; import { readSecret, writeSecret, createRealSecretsExecSeam, type SecretsSeams } from "./secrets/store.ts"; import { createRealAgeKeySeam } from "./home/age-key.ts"; @@ -47,25 +44,6 @@ function defaultSecretsSeams(): SecretsSeams { }; } -function plaintextSecretsPath(): string { - return join(rtDir(), "secrets.json"); -} - -/** - * RT-32 transition fallback: the plaintext file still holds the real values - * until the live import runs, and stays readable afterward only because - * nothing has deleted it yet. Delete this function (and its two call sites - * in loadSecrets) when that import retires ~/.mattstack/rt/secrets.json — no - * other code path should ever read this file directly. - */ -function readPlaintextSecretsFallback(): Secrets { - try { - return JSON.parse(readFileSync(plaintextSecretsPath(), "utf8")); - } catch { - return {}; - } -} - interface EncryptedRtSecretsResult { values: Partial; /** Set when a decrypt attempt threw (keychain unreachable, corrupt ciphertext, …) — loadSecrets decides whether that's fatal. */ @@ -96,30 +74,18 @@ async function readEncryptedRtSecrets(seams: SecretsSeams): Promise { - const { values: encrypted, failure } = await readEncryptedRtSecrets(seams); - if (!failure) { - return { ...readPlaintextSecretsFallback(), ...encrypted }; - } - - if (!existsSync(plaintextSecretsPath())) { - throw failure; - } - - console.error(`[secrets] encrypted store unreadable (${failure.message}); using plaintext secrets.json`); - return { ...readPlaintextSecretsFallback(), ...encrypted }; + const { values, failure } = await readEncryptedRtSecrets(seams); + if (failure) throw failure; + return values; } export async function saveSecret( diff --git a/lib/sdm/browser-login.ts b/lib/sdm/browser-login.ts index 7f89afeb..222dcabb 100644 --- a/lib/sdm/browser-login.ts +++ b/lib/sdm/browser-login.ts @@ -10,6 +10,11 @@ * an on-screen window is required for the click to land (an off-screen window * does not lay out, so getBoundingClientRect returns nothing); and StrongDM's * SAML button only responds to a TRUSTED CDP mouse click, not a synthetic one. + * + * `SDM_EMAIL` overrides the store's `sdmEmail` for the email preflight + * (`runBrowserLogin`'s `email` seam) — the one env escape hatch into an + * otherwise store-only read, kept narrow so e2e can drive the flow without + * writing to the encrypted secrets store. */ import { spawn } from "node:child_process"; @@ -316,7 +321,7 @@ export function runBrowserLogin(opts: { visible?: boolean; onLine?: (line: strin waitForCdp: realWaitForCdp, startLogin: startLoginCapture, showWindow: realShowWindow, - email: async () => (await loadSecrets()).sdmEmail ?? null, + email: async () => process.env.SDM_EMAIL || (await loadSecrets()).sdmEmail || null, onLine, }); }