Skip to content

Commit ba77eb8

Browse files
aljjang95lidge-jun
andcommitted
fix(kiro): resolve the Windows kiro-cli executable without PATH
Forced and add-account Kiro login shell out to the local CLI by bare name. #710 taught Windows where the SQLite store lives, so a Windows box can import tokens while PATH still has no `kiro-cli` -- and then login fails on a machine that is otherwise correctly installed. The resolver tries PATH first (both `PATH` and `Path`, since Windows uses the latter), then the layouts the installers actually produce: `%LOCALAPPDATA%\Kiro-Cli` and `C:\Program Files\Kiro-Cli` on Windows, `~/.local/bin` and the usual prefixes elsewhere. With nothing found it returns the bare command so spawn still reports the original error rather than a path this code invented. Two additions on top of the contributed patch. `existsSync` alone accepts a *directory* named `kiro-cli`, which then reaches spawn() and fails with EACCES instead of falling through to the next candidate -- candidates now have to be files. And the tests only ever injected `pathEntries`, so the environment parsing itself was never exercised; a case now drives `Path` and `PATH` through the real code. Both new tests were checked by ablation: removing the `isFile` guard fails the directory case (6 pass / 1 fail) and restoring it returns 7/7, so neither passes vacuously. Windows behavior is unverifiable from macOS; the matrix CI on this branch is what confirms it. Co-authored-by: bitkyc08-arch <bitkyc08@gmail.com>
1 parent 0856b0c commit ba77eb8

4 files changed

Lines changed: 198 additions & 3 deletions

File tree

docs-site/src/content/docs/guides/providers.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ read-only. Two environment variables make the source and token row selection exp
165165
- `KIROCLI_TOKEN_KEY` selects the exact `auth_kv` token key when a database contains multiple
166166
otherwise ambiguous token rows. A missing selection fails login instead of guessing.
167167

168+
On Windows, import looks for `%LOCALAPPDATA%\Kiro-Cli\data.sqlite3`. Forced/add-account login
169+
also needs the local CLI binary: opencodex first uses `PATH`, then falls back to
170+
`%LOCALAPPDATA%\Kiro-Cli\kiro-cli.exe` and `C:\Program Files\Kiro-Cli\kiro-cli.exe`.
171+
168172
After a successful import, opencodex persists the imported credential to
169173
`~/.opencodex/auth.json`.
170174

src/oauth/kiro-credentials.ts

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { randomUUID } from "node:crypto";
2-
import { chmodSync, closeSync, existsSync, fsyncSync, linkSync, openSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs";
2+
import { chmodSync, closeSync, existsSync, fsyncSync, linkSync, openSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs";
33
import { homedir } from "node:os";
44
import { isAbsolute, join, posix, win32 } from "node:path";
55
import { Database } from "bun:sqlite";
@@ -169,6 +169,77 @@ export function resolveKiroCliNativeSessionEntries(
169169
return [{ location: "kiro-cli-linux-data", path: posix.join(home, ".local", "share", "kiro-cli", "data.sqlite3") }];
170170
}
171171

172+
/**
173+
* Resolve the absolute kiro-cli executable for spawn/login helpers.
174+
*
175+
* Pure + parameterized like `resolveKiroCliNativeSessionEntries` so Windows install layouts can be
176+
* covered from any host. PATH remains the first choice; only when bare `kiro-cli` is missing do we
177+
* fall back to the platform-native install directories next to the session database.
178+
*
179+
* Windows: official MSI installs to `C:\Program Files\Kiro-Cli\kiro-cli.exe`, while some local
180+
* installs keep the binary next to `%LOCALAPPDATA%\Kiro-Cli\data.sqlite3`.
181+
* macOS/Linux: prefer PATH, then the usual user-local bin directories.
182+
*/
183+
export function resolveKiroCliExecutable(
184+
inputs: KiroCliNativeInputs & {
185+
pathEntries?: string[];
186+
exists?: (path: string) => boolean;
187+
isFile?: (path: string) => boolean;
188+
},
189+
): string {
190+
const exists = inputs.exists ?? existsSync;
191+
// A directory named `kiro-cli` on PATH satisfies existsSync and would then be handed to
192+
// spawn(), which fails with EACCES at login instead of falling through to the next candidate.
193+
// When a caller injects `exists` it owns the whole filesystem view, so the real stat would
194+
// reject every synthetic path; such callers inject `isFile` too when they care about it.
195+
const isFile = inputs.isFile ?? (inputs.exists ? () => true : ((path: string) => {
196+
try {
197+
return statSync(path).isFile();
198+
} catch {
199+
return false;
200+
}
201+
}));
202+
const pathEntries = inputs.pathEntries
203+
?? (inputs.env.PATH ?? inputs.env.Path ?? "").split(inputs.platform === "win32" ? ";" : ":")
204+
.map(entry => entry.trim())
205+
.filter(Boolean);
206+
207+
const pathCandidates = inputs.platform === "win32"
208+
? pathEntries.flatMap(entry => [
209+
win32.join(entry, "kiro-cli.exe"),
210+
win32.join(entry, "kiro-cli"),
211+
])
212+
: pathEntries.map(entry => posix.join(entry, "kiro-cli"));
213+
214+
const installCandidates: string[] = [];
215+
if (inputs.platform === "win32") {
216+
const localBase = inputs.env.LOCALAPPDATA?.trim()
217+
|| (inputs.env.USERPROFILE?.trim() ? win32.join(inputs.env.USERPROFILE.trim(), "AppData", "Local") : "")
218+
|| win32.join(inputs.home, "AppData", "Local");
219+
const programFiles = inputs.env["ProgramFiles"]?.trim() || "C:\\Program Files";
220+
installCandidates.push(
221+
win32.join(localBase, "Kiro-Cli", "kiro-cli.exe"),
222+
win32.join(programFiles, "Kiro-Cli", "kiro-cli.exe"),
223+
);
224+
} else if (inputs.platform === "darwin") {
225+
installCandidates.push(
226+
posix.join(inputs.home, ".local", "bin", "kiro-cli"),
227+
"/usr/local/bin/kiro-cli",
228+
"/opt/homebrew/bin/kiro-cli",
229+
);
230+
} else {
231+
installCandidates.push(
232+
posix.join(inputs.home, ".local", "bin", "kiro-cli"),
233+
"/usr/local/bin/kiro-cli",
234+
);
235+
}
236+
237+
for (const candidate of [...pathCandidates, ...installCandidates]) {
238+
if (exists(candidate) && isFile(candidate)) return candidate;
239+
}
240+
return inputs.platform === "win32" ? "kiro-cli.exe" : "kiro-cli";
241+
}
242+
172243
function nativeKiroCliSessionEntries(): Array<{ location: KiroCliNativeLocation; path: string }> {
173244
// Only the stores that `kiro-cli logout` / `kiro-cli login` themselves mutate. Import fallbacks
174245
// (Amazon Q / SSO cache) and KIROCLI_DB_PATH selectors must not be snapshotted for rollback.

src/oauth/kiro.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@ import {
1818
persistKiroCliSessionRecovery,
1919
readImportedKiroCredential,
2020
readKiroCliSqliteCredential,
21+
resolveKiroCliExecutable,
2122
restoreKiroCliSession,
2223
restoreStaleKiroCliSessionRecovery,
2324
requireKiroRegion,
2425
type ImportedKiroCredential,
2526
type KiroCliSessionSnapshot,
2627
type KiroImportDiagnostic,
2728
} from "./kiro-credentials";
29+
import { homedir } from "node:os";
2830
import { getAccountSet, saveAccountCredential } from "./store";
2931

3032
const DEFAULT_REGION = "us-east-1";
@@ -72,9 +74,18 @@ const pendingKiroLoginTransactions = new WeakMap<OAuthCredentials, KiroCliSessio
7274
/** Forced logins that started with no native CLI DB must logout on persistence failure. */
7375
const pendingKiroEmptyPriorSessions = new WeakSet<OAuthCredentials>();
7476

77+
78+
function resolveRuntimeKiroCliExecutable(): string {
79+
return resolveKiroCliExecutable({
80+
env: process.env,
81+
platform: process.platform,
82+
home: process.platform === "win32" ? homedir() : (process.env.HOME || homedir()),
83+
});
84+
}
85+
7586
function logoutKiroCliBestEffort(): void {
7687
try {
77-
Bun.spawnSync(["kiro-cli", "logout"], {
88+
Bun.spawnSync([resolveRuntimeKiroCliExecutable(), "logout"], {
7889
stdin: "ignore",
7990
stdout: "ignore",
8091
stderr: "ignore",
@@ -128,7 +139,7 @@ async function defaultKiroCliRunner(args: string[], signal?: AbortSignal): Promi
128139
throwIfKiroLoginCancelled(signal);
129140
let child: ReturnType<typeof Bun.spawn>;
130141
try {
131-
child = Bun.spawn(["kiro-cli", ...args], {
142+
child = Bun.spawn([resolveRuntimeKiroCliExecutable(), ...args], {
132143
stdin: "ignore",
133144
stdout: "pipe",
134145
stderr: "ignore",
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { resolveKiroCliExecutable } from "../src/oauth/kiro-credentials";
3+
4+
/**
5+
* Forced/add-account Kiro login still shells out to the local CLI. After #710 fixed Windows
6+
* SQLite discovery, Windows installs can import tokens while PATH still lacks `kiro-cli`.
7+
* The pure executable resolver covers that layout without launching the real binary.
8+
*/
9+
describe("kiro-cli executable resolution", () => {
10+
const WIN_HOME = "C:\\Users\\u";
11+
12+
test("prefers the first PATH hit before install-directory fallbacks", () => {
13+
const exists = (path: string) => path === "C:\\Tools\\kiro-cli.exe";
14+
expect(resolveKiroCliExecutable({
15+
env: {
16+
PATH: "C:\\Tools;C:\\Windows\\System32",
17+
LOCALAPPDATA: "C:\\Users\\u\\AppData\\Local",
18+
},
19+
platform: "win32",
20+
home: WIN_HOME,
21+
pathEntries: ["C:\\Tools", "C:\\Windows\\System32"],
22+
exists,
23+
})).toBe("C:\\Tools\\kiro-cli.exe");
24+
});
25+
26+
test("win32 falls back to %LOCALAPPDATA%\\Kiro-Cli\\kiro-cli.exe", () => {
27+
const exists = (path: string) => path === "C:\\Users\\u\\AppData\\Local\\Kiro-Cli\\kiro-cli.exe";
28+
expect(resolveKiroCliExecutable({
29+
env: {
30+
PATH: "C:\\Windows\\System32",
31+
LOCALAPPDATA: "C:\\Users\\u\\AppData\\Local",
32+
},
33+
platform: "win32",
34+
home: WIN_HOME,
35+
pathEntries: ["C:\\Windows\\System32"],
36+
exists,
37+
})).toBe("C:\\Users\\u\\AppData\\Local\\Kiro-Cli\\kiro-cli.exe");
38+
});
39+
40+
test("win32 falls back to Program Files\\Kiro-Cli when LOCALAPPDATA binary is absent", () => {
41+
const exists = (path: string) => path === "C:\\Program Files\\Kiro-Cli\\kiro-cli.exe";
42+
expect(resolveKiroCliExecutable({
43+
env: {
44+
PATH: "C:\\Windows\\System32",
45+
LOCALAPPDATA: "C:\\Users\\u\\AppData\\Local",
46+
ProgramFiles: "C:\\Program Files",
47+
},
48+
platform: "win32",
49+
home: WIN_HOME,
50+
pathEntries: ["C:\\Windows\\System32"],
51+
exists,
52+
})).toBe("C:\\Program Files\\Kiro-Cli\\kiro-cli.exe");
53+
});
54+
55+
test("linux keeps PATH-first resolution and falls back to ~/.local/bin", () => {
56+
const exists = (path: string) => path === "/home/u/.local/bin/kiro-cli";
57+
expect(resolveKiroCliExecutable({
58+
env: { PATH: "/usr/bin" },
59+
platform: "linux",
60+
home: "/home/u",
61+
pathEntries: ["/usr/bin"],
62+
exists,
63+
})).toBe("/home/u/.local/bin/kiro-cli");
64+
});
65+
66+
test("returns the bare command when no candidate exists so spawn can report the original error", () => {
67+
expect(resolveKiroCliExecutable({
68+
env: { PATH: "C:\\Windows\\System32" },
69+
platform: "win32",
70+
home: WIN_HOME,
71+
pathEntries: ["C:\\Windows\\System32"],
72+
exists: () => false,
73+
})).toBe("kiro-cli.exe");
74+
});
75+
76+
test("skips a directory named kiro-cli and keeps looking", () => {
77+
// A directory passes existsSync, so without the isFile guard this resolves to
78+
// C:\Tools\kiro-cli and spawn() fails with EACCES at login time.
79+
const exists = (path: string) =>
80+
path === "C:\\Tools\\kiro-cli" || path === "C:\\Program Files\\Kiro-Cli\\kiro-cli.exe";
81+
const isFile = (path: string) => path !== "C:\\Tools\\kiro-cli";
82+
expect(resolveKiroCliExecutable({
83+
env: { PATH: "C:\\Tools", ProgramFiles: "C:\\Program Files" },
84+
platform: "win32",
85+
home: WIN_HOME,
86+
pathEntries: ["C:\\Tools"],
87+
exists,
88+
isFile,
89+
})).toBe("C:\\Program Files\\Kiro-Cli\\kiro-cli.exe");
90+
});
91+
92+
test("parses PATH from the environment under both Windows casings", () => {
93+
// The resolver must read the env itself, not only an injected pathEntries array:
94+
// Windows exposes the variable as `Path`, POSIX as `PATH`.
95+
const exists = (path: string) => path === "C:\\Tools\\kiro-cli.exe";
96+
expect(resolveKiroCliExecutable({
97+
env: { Path: "C:\\Tools;C:\\Windows\\System32" },
98+
platform: "win32",
99+
home: WIN_HOME,
100+
exists,
101+
})).toBe("C:\\Tools\\kiro-cli.exe");
102+
expect(resolveKiroCliExecutable({
103+
env: { PATH: "C:\\Tools" },
104+
platform: "win32",
105+
home: WIN_HOME,
106+
exists,
107+
})).toBe("C:\\Tools\\kiro-cli.exe");
108+
});
109+
});

0 commit comments

Comments
 (0)