From 584ce39c822ab504af3eb2b3cc29d59e50f8be3e Mon Sep 17 00:00:00 2001
From: Erwann Mest <m@kud.io>
Date: Fri, 31 Jul 2026 15:33:33 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix(browse):=20require=20stored?=
 =?UTF-8?q?=20auth=20before=20entering=20interactive=20browser?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Add requireStoredAuth() in src/cli.ts, using resolveStoredAuth (not resolveAuth) so the check never triggers an interactive OAuth round-trip when PCLOUD_CLIENT_ID/_SECRET are set
- Call requireStoredAuth() in the browse command's .action() before startBrowse(), since checking inside the render happens after the alternate screen is up and the error is written to a buffer torn down microseconds later
- Refactor the not-authenticated message into exitNotAuthenticated() and reuse it from getAuthenticatedAPI()
- Add src/browse.test.ts with runBrowseLoggedOut(), which spawns the CLI in a fresh HOME with no PCLOUD_* env vars and asserts the auth error prints while the ink-picture/TerminalInfo render-crash stack is absent
---
 src/browse.test.ts | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 src/cli.ts         | 30 ++++++++++++++++++++------
 2 files changed, 77 insertions(+), 6 deletions(-)
 create mode 100644 src/browse.test.ts

diff --git a/src/browse.test.ts b/src/browse.test.ts
new file mode 100644
index 0000000..9bae71c
--- /dev/null
+++ b/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)
+  })
+})
diff --git a/src/cli.ts b/src/cli.ts
index 5119c50..a009ceb 100644
--- a/src/cli.ts
+++ b/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()
   })