Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/linux-secret-tool-probe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@cloudflare/workers-auth": patch
"wrangler": patch
---

Fix `wrangler login --use-keyring` incorrectly reporting that `secret-tool` is missing on Linux

Libsecret's `secret-tool` does not support `--version`; it prints usage and exits 2, which Wrangler previously interpreted as unavailable. Wrangler now reports it missing only when launching the executable fails.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ export function setLinuxSecretToolRunner(
/**
* Probe whether `secret-tool` is callable in the current environment.
*
* Returns `true` when `secret-tool --version` exits 0. The probe does not
* Returns `true` when `secret-tool` could be spawned at all, whatever its
* exit status. libsecret's `secret-tool` has no `--version` flag: it prints
* its usage and exits 2, so the exit status says nothing about whether the
* tool is installed. Only a failure to spawn the process (e.g. `ENOENT`
* when it is not on `PATH`) means it is missing. The probe does not
* exercise the keyring backend itself — a missing D-Bus session surfaces
* on the first real read/write rather than every consumer invocation, so
* we avoid the extra latency on every command for users whose desktop
Expand All @@ -72,7 +76,10 @@ export function probeSecretTool(): boolean {
let result: boolean;
try {
const r = runner(["--version"]);
result = r.status === 0;
// `spawnSync` does not throw when the executable is missing: it
// reports the failure via `error`. A result without an error proves
// the executable was launched, even if a signal terminated it.
result = r.error === undefined;
} catch {
result = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ function mockResult({
status = 0,
stdout = "",
stderr = "",
error,
}: {
status?: number | null;
stdout?: string;
stderr?: string;
error?: Error;
} = {}): SpawnSyncReturns<string> {
return {
status,
Expand All @@ -26,9 +28,22 @@ function mockResult({
signal: null,
output: [null, stdout, stderr],
pid: 1,
error,
} as SpawnSyncReturns<string>;
}

/**
* What `spawnSync` returns when `secret-tool` cannot be spawned at all (for
* example because it is not on `PATH`): it does not throw, it reports the
* failure via `error` and leaves `status` as `null`.
*/
function spawnErrorResult(code = "ENOENT"): SpawnSyncReturns<string> {
return mockResult({
status: null,
error: Object.assign(new Error(`spawn secret-tool ${code}`), { code }),
});
}

const SAMPLE_KEY = new Uint8Array(32).fill(0xab);

describe("LinuxSecretToolKeyProvider", () => {
Expand Down Expand Up @@ -59,10 +74,40 @@ describe("LinuxSecretToolKeyProvider", () => {
expect(probeSecretTool()).toBe(false);
});

it("returns false when secret-tool --version exits non-zero", ({
it("returns true when secret-tool prints its usage and exits 2 (libsecret has no --version flag)", ({
expect,
}) => {
setLinuxSecretToolRunner(() =>
mockResult({
status: 2,
stderr:
"usage: secret-tool store --label='label' attribute value ...",
})
);
expect(probeSecretTool()).toBe(true);
});

it("returns true on any other non-zero exit status, since the process ran", ({
expect,
}) => {
setLinuxSecretToolRunner(() => mockResult({ status: 127 }));
expect(probeSecretTool()).toBe(true);
});

it("returns true when secret-tool was launched but terminated by a signal", ({
expect,
}) => {
setLinuxSecretToolRunner(() => ({
...mockResult({ status: null }),
signal: "SIGTERM",
}));
expect(probeSecretTool()).toBe(true);
});

it("returns false when spawnSync reports ENOENT instead of throwing", ({
expect,
}) => {
setLinuxSecretToolRunner(() => spawnErrorResult());
expect(probeSecretTool()).toBe(false);
});

Expand Down Expand Up @@ -91,7 +136,7 @@ describe("LinuxSecretToolKeyProvider", () => {
setLinuxSecretToolRunner(() => mockResult({ status: 0 }));
expect(probeSecretTool()).toBe(true);

setLinuxSecretToolRunner(() => mockResult({ status: 127 }));
setLinuxSecretToolRunner(() => spawnErrorResult());
expect(probeSecretTool()).toBe(false);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,14 @@ describe("createCredentialStorageContext — resolver", () => {
}) => {
stubPlatform("linux");
vi.stubEnv("CLOUDFLARE_AUTH_USE_KEYRING", "true");
setLinuxSecretToolRunner(() => mockResult({ status: 127 }));
// `spawnSync` reports a missing executable via `error` with a `null`
// status rather than by throwing.
setLinuxSecretToolRunner(() => ({
...mockResult({ status: null }),
error: Object.assign(new Error("spawn secret-tool ENOENT"), {
code: "ENOENT",
}),
}));

expect(() => resolveStore({ isKeyringEnabled: true })).toThrow(
/CLOUDFLARE_AUTH_USE_KEYRING is set but `secret-tool` is required/
Expand Down
Loading