🐛 fix(browse): require stored auth before entering interactive browser

src/browse.test.ts

@@ -0,0 +1,53 @@
+import { describe, expect, it } from "vitest"
+import { spawnSync } from "node:child_process"
+import { mkdtempSync, rmSync } from "node:fs"
+import { tmpdir } from "node:os"
+import { join } from "node:path"
+import { fileURLToPath } from "node:url"
+
+const CLI = fileURLToPath(new URL("./cli.ts", import.meta.url))
+const TSX = fileURLToPath(new URL("../node_modules/.bin/tsx", import.meta.url))
+
+// Reaching the render is precisely what the gate prevents, and it leaves a trace:
+// mounting the browser without a TTY drives ink-picture's terminal query into a
+// crash. Asserting that stack is absent is what makes this a real regression test
+// — exit status alone does not discriminate, since the crash also exits non-zero.
+const RENDER_REACHED = /ink-picture|TerminalInfo|at async/
+
+// TokenStore reads os.homedir(), which is $HOME on POSIX, so pointing HOME at an
+// empty directory is what makes "logged out" deterministic. cwd goes there too:
+// cli.ts calls dotenv.config(), and a .env in the repo root would otherwise hand
+// the subprocess the very credentials this test is trying to withhold.
+const runBrowseLoggedOut = () => {
+  const home = mkdtempSync(join(tmpdir(), "pcloud-cli-home-"))
+  const env: NodeJS.ProcessEnv = { ...process.env, HOME: home }
+  delete env.PCLOUD_AUTH
+  delete env.PCLOUD_ACCESS_TOKEN
+  delete env.PCLOUD_CLIENT_ID
+  delete env.PCLOUD_CLIENT_SECRET
+
+  try {
+    return spawnSync(TSX, [CLI, "browse"], {
+      cwd: home,
+      env,
+      encoding: "utf8",
+      timeout: 60_000,
+    })
+  } finally {
+    rmSync(home, { recursive: true, force: true })
+  }
+}
+
+describe("browse without credentials", () => {
+  it("exits non-zero instead of opening the browser", () => {
+    expect(runBrowseLoggedOut().status).toBe(1)
+  })
+
+  it("says how to authenticate", () => {
+    expect(runBrowseLoggedOut().stderr).toMatch(/Not authenticated/)
+  })
+
+  it("refuses before mounting the browser, not from inside it", () => {
+    expect(runBrowseLoggedOut().stderr).not.toMatch(RENDER_REACHED)
+  })
+})

src/cli.ts

@@ -17,6 +17,7 @@ import {
   OAuthFlow,
   sessionLogin,
   resolveAuth,
+  resolveStoredAuth,
 } from "@kud/pcloud"
 import { renderAccount, renderChanges, renderFileList } from "./render.js"
 import { planRewind, applyRewind } from "./rewind.js"
@@ -56,19 +57,31 @@ program
   .description("CLI tool for pCloud file operations")
   .version(pkg.version)
 
+const exitNotAuthenticated = (): never => {
+  console.error("\n❌ Not authenticated!\n")
+  console.error("It looks like you haven't set up pCloud CLI yet.\n")
+  console.error("Please run this command first:\n")
+  console.error("  pcloud login\n")
+  console.error("This is a one-time setup that takes less than a minute.\n")
+  process.exit(1)
+}
+
 const getAuthenticatedAPI = async (): Promise<PCloudAPI> => {
   try {
     return await resolveAuth({ defaultApiServer })
   } catch {
-    console.error("\n❌ Not authenticated!\n")
-    console.error("It looks like you haven't set up pCloud CLI yet.\n")
-    console.error("Please run this command first:\n")
-    console.error("  pcloud login\n")
-    console.error("This is a one-time setup that takes less than a minute.\n")
-    process.exit(1)
+    return exitNotAuthenticated()
   }
 }
 
+// resolveStoredAuth rather than resolveAuth: the latter falls through to an
+// interactive OAuth browser round-trip when PCLOUD_CLIENT_ID and _SECRET are set,
+// which is the wrong thing to trigger from a precondition check. This only asks
+// whether a credential is already on hand.
+const requireStoredAuth = (): void => {
+  if (!resolveStoredAuth({ defaultApiServer })) exitNotAuthenticated()
+}
+
 const handleError = (error: unknown): never => {
   console.error(`Error: ${error instanceof Error ? error.message : error}`)
   process.exit(1)
@@ -1477,6 +1490,11 @@ program
   .command("browse")
   .description("Interactive file browser")
   .action(async () => {
+    // Checked here rather than left to the browser component: that check runs
+    // inside the render, by which point the alternate screen is up, so its error
+    // is written to a buffer torn down microseconds later and the command appears
+    // to exit silently. The message only survives if it precedes the switch.
+    requireStoredAuth()
     const { startBrowse } = await import("./browse.js")
     await startBrowse()
   })