From ea7108a191c8d634df9a1a5db09b8b1d354713c4 Mon Sep 17 00:00:00 2001 From: Steven777 Date: Wed, 2 Sep 2026 19:45:25 +0800 Subject: [PATCH 1/2] fix(auth): bound macOS credential status --- CHANGELOG.md | 6 +++ README.md | 4 ++ docs/AUTHENTICATION.md | 7 +++ src/core/keyring.ts | 97 +++++++++++++++++++++++++++++++--------- src/test/keyring.test.ts | 25 +++++++++++ 5 files changed, 118 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46dd40a..3706383 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to `sustech-cli` are documented in this file. ## [Unreleased] +### Fixed + +- Made `auth status` use a metadata-only macOS Keychain lookup instead of + reading the stored password, and bounded credential-helper subprocesses to + five seconds with a structured `CREDENTIAL_STORE_TIMEOUT` status. + ## [0.10.0] - 2026-08-29 ### Added diff --git a/README.md b/README.md index 89698a6..3e657aa 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,10 @@ command-line argument, and is never written to the CLI config. If no safe backend is available, the CLI returns `CREDENTIAL_STORE_UNAVAILABLE` instead of falling back to plaintext. +On macOS, `auth status` checks Keychain item metadata without reading the +password. Credential-helper commands are bounded to five seconds and report +`CREDENTIAL_STORE_TIMEOUT` without an automatic retry. + ```bash sustech auth login --profile main sustech auth check --profile main --service bb --json diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index c8a94fa..c0b42cc 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -68,6 +68,13 @@ Linux deliberately requires a desktop D-Bus session and the distribution's `secret-tool`/`libsecret-tools` package. It does not silently fall back to a plaintext file or a session-only kernel keyring. +`auth status` does not read the stored password when checking macOS Keychain. +It uses a metadata-only `security find-generic-password` lookup without `-w`. +Credential-helper subprocesses have a five-second deadline and are never +retried automatically. If a helper exceeds that deadline, structured status +sets `reasonCode` to `CREDENTIAL_STORE_TIMEOUT`, marks the backend unavailable +for that probe, and leaves the credential and profile metadata unchanged. + ## Profiles The default profile is named `default`. Multiple accounts use explicit names: diff --git a/src/core/keyring.ts b/src/core/keyring.ts index d7f43cd..fefc7da 100644 --- a/src/core/keyring.ts +++ b/src/core/keyring.ts @@ -9,6 +9,7 @@ import { defaultConfigDirectory } from "./local-store.js"; export const DEFAULT_CREDENTIAL_PROFILE = "default"; export const SUSTECH_CREDENTIAL_SERVICE = "cn.edu.sustech.cli.cas"; export const BLACKBOARD_CALENDAR_LINK_SERVICE = "cn.edu.sustech.cli.bb-calendar-link"; +export const DEFAULT_CREDENTIAL_COMMAND_TIMEOUT_MS = 5_000; export type CredentialBackend = | "macos-keychain" @@ -18,6 +19,7 @@ export type CredentialBackend = export interface SecretStore { readonly backend: CredentialBackend; readonly persistent: true; + has?(account: string): Promise; get(account: string): Promise; set(account: string, password: string): Promise; delete(account: string): Promise; @@ -28,6 +30,7 @@ export interface CredentialStoreOptions { env?: NodeJS.ProcessEnv; platform?: NodeJS.Platform; store?: SecretStore; + credentialCommandTimeoutMs?: number; } interface StoredProfile { @@ -60,6 +63,7 @@ export interface CredentialProfileStatus { persistent: boolean; storedAt?: string; profiles: string[]; + reasonCode?: "CREDENTIAL_STORE_ERROR" | "CREDENTIAL_STORE_TIMEOUT"; reason?: string; remediation?: string; } @@ -270,18 +274,20 @@ export async function getCredentialStatus( } try { - const password = await resolution.store.get(stored.account); + const credentialAvailable = resolution.store.has + ? await resolution.store.has(stored.account) + : Boolean(await resolution.store.get(stored.account)); return { profile, configured: true, - credentialAvailable: Boolean(password), + credentialAvailable, maskedSid: maskSid(stored.sid), backend: stored.backend, backendAvailable: true, persistent: true, storedAt: stored.storedAt, profiles, - ...(!password ? { reason: "The profile metadata exists, but the secret is missing from the credential store." } : {}), + ...(!credentialAvailable ? { reason: "The profile metadata exists, but the secret is missing from the credential store." } : {}), }; } catch (error) { return { @@ -294,6 +300,7 @@ export async function getCredentialStatus( persistent: true, storedAt: stored.storedAt, profiles, + reasonCode: credentialStatusReasonCode(error), reason: safeStoreReason(error), }; } @@ -433,9 +440,9 @@ async function resolveBackendForNamespace( }; } const platform = options.platform ?? process.platform; - if (platform === "darwin") return await resolveMacosKeychain(options.env, namespace); + if (platform === "darwin") return await resolveMacosKeychain(options, namespace); if (platform === "win32") return await resolveWindowsCredentialManager(namespace); - if (platform === "linux") return await resolveLinuxSecretService(options.env, namespace); + if (platform === "linux") return await resolveLinuxSecretService(options, namespace); return { backend: "unavailable", available: false, @@ -446,12 +453,13 @@ async function resolveBackendForNamespace( } async function resolveMacosKeychain( - customEnv: NodeJS.ProcessEnv | undefined, + options: CredentialStoreOptions, namespace: SecretNamespace, ): Promise { const backend = "macos-keychain" as const; const executable = "/usr/bin/security"; - const env = customEnv ?? process.env; + const env = options.env ?? process.env; + const timeoutMs = credentialCommandTimeoutMs(options); try { await access(executable, constants.X_OK); const { AsyncEntry } = await import("@napi-rs/keyring"); @@ -460,10 +468,13 @@ async function resolveMacosKeychain( const store: SecretStore = { backend, persistent: true, + async has(account) { + return await macosCredentialExists(executable, namespace.service, account, env, timeoutMs); + }, async get(account) { const password = await new AsyncEntry(namespace.service, account).getPassword() ?? undefined; if (password !== undefined) return password; - if (!await macosCredentialExists(executable, namespace.service, account, env)) return undefined; + if (!await macosCredentialExists(executable, namespace.service, account, env, timeoutMs)) return undefined; throw new Error("macOS Keychain item exists, but its secret could not be read."); }, async set(account, password) { @@ -471,7 +482,7 @@ async function resolveMacosKeychain( }, async delete(account) { const deleted = await new AsyncEntry(namespace.service, account).deleteCredential(); - if (await macosCredentialExists(executable, namespace.service, account, env)) { + if (await macosCredentialExists(executable, namespace.service, account, env, timeoutMs)) { throw new Error("macOS Keychain delete could not be verified."); } return deleted; @@ -494,10 +505,11 @@ async function macosCredentialExists( service: string, account: string, env: NodeJS.ProcessEnv, + timeoutMs: number, ): Promise { const result = await runCredentialCommand(executable, [ "find-generic-password", "-s", service, "-a", account, - ], undefined, env); + ], undefined, env, timeoutMs); if (macosItemNotFound(result)) return false; if (result.code !== 0) throw new Error("macOS Keychain metadata lookup failed."); return true; @@ -542,10 +554,11 @@ async function resolveWindowsCredentialManager(namespace: SecretNamespace): Prom } async function resolveLinuxSecretService( - customEnv: NodeJS.ProcessEnv | undefined, + options: CredentialStoreOptions, namespace: SecretNamespace, ): Promise { - const env = customEnv ?? process.env; + const env = options.env ?? process.env; + const timeoutMs = credentialCommandTimeoutMs(options); if (!env.DBUS_SESSION_BUS_ADDRESS) { return { backend: "linux-secret-service", @@ -571,7 +584,7 @@ async function resolveLinuxSecretService( async get(account) { const result = await runCredentialCommand(executable, [ "lookup", "service", namespace.service, "account", account, - ], undefined, env); + ], undefined, env, timeoutMs); if (result.code === 1 && !result.stdout.trim() && !result.stderr.trim()) return undefined; if (result.code !== 0) throw new Error("Secret Service lookup failed."); return result.stdout.replace(/\r?\n$/, "") || undefined; @@ -582,13 +595,13 @@ async function resolveLinuxSecretService( `--label=${namespace.linuxLabel} (${account.split(":", 1)[0]})`, "service", namespace.service, "account", account, - ], `${password}\n`, env); + ], `${password}\n`, env, timeoutMs); if (result.code !== 0) throw new Error("Secret Service write failed."); }, async delete(account) { const result = await runCredentialCommand(executable, [ "clear", "service", namespace.service, "account", account, - ], undefined, env); + ], undefined, env, timeoutMs); if (result.code === 1 && !result.stderr.trim()) return false; if (result.code !== 0) throw new Error("Secret Service delete failed."); return true; @@ -620,12 +633,26 @@ async function runCredentialCommand( args: string[], input: string | undefined, env: NodeJS.ProcessEnv, + timeoutMs = DEFAULT_CREDENTIAL_COMMAND_TIMEOUT_MS, ): Promise<{ code: number; stdout: string; stderr: string }> { return await new Promise((resolve, reject) => { const child = spawn(executable, args, { env, stdio: ["pipe", "pipe", "pipe"] }); const stdout: Buffer[] = []; const stderr: Buffer[] = []; let size = 0; + let settled = false; + const timer = setTimeout(() => { + if (settled) return; + settled = true; + child.kill("SIGKILL"); + reject(new CredentialCommandTimeoutError(timeoutMs)); + }, timeoutMs); + const rejectOnce = (error: Error) => { + if (settled) return; + settled = true; + clearTimeout(timer); + reject(error); + }; child.stdout.on("data", (chunk: Buffer) => { size += chunk.length; if (size <= 64 * 1024) stdout.push(chunk); @@ -634,17 +661,39 @@ async function runCredentialCommand( size += chunk.length; if (size <= 64 * 1024) stderr.push(chunk); }); - child.once("error", reject); - child.once("close", (code) => resolve({ - code: code ?? 1, - stdout: Buffer.concat(stdout).toString("utf8"), - stderr: Buffer.concat(stderr).toString("utf8"), - })); + child.once("error", rejectOnce); + child.once("close", (code) => { + if (settled) return; + settled = true; + clearTimeout(timer); + resolve({ + code: code ?? 1, + stdout: Buffer.concat(stdout).toString("utf8"), + stderr: Buffer.concat(stderr).toString("utf8"), + }); + }); if (input !== undefined) child.stdin.end(input, "utf8"); else child.stdin.end(); }); } +class CredentialCommandTimeoutError extends Error { + public readonly code = "CREDENTIAL_STORE_TIMEOUT"; + + public constructor(timeoutMs: number) { + super(`Credential-store command exceeded its ${timeoutMs} ms deadline.`); + this.name = "CredentialCommandTimeoutError"; + } +} + +function credentialCommandTimeoutMs(options: CredentialStoreOptions): number { + const timeoutMs = options.credentialCommandTimeoutMs ?? DEFAULT_CREDENTIAL_COMMAND_TIMEOUT_MS; + if (!Number.isSafeInteger(timeoutMs) || timeoutMs <= 0 || timeoutMs > 60_000) { + throw new Error("Credential-store command timeout must be an integer from 1 to 60000 milliseconds."); + } + return timeoutMs; +} + function requireAvailableStore(resolution: BackendResolution): SecretStore { if (resolution.store) return resolution.store; throw new CliError( @@ -731,6 +780,12 @@ function safeStoreReason(error: unknown): string { : "The operating-system credential store rejected or could not complete the request."; } +function credentialStatusReasonCode(error: unknown): "CREDENTIAL_STORE_ERROR" | "CREDENTIAL_STORE_TIMEOUT" { + return error && typeof error === "object" && "code" in error && error.code === "CREDENTIAL_STORE_TIMEOUT" + ? "CREDENTIAL_STORE_TIMEOUT" + : "CREDENTIAL_STORE_ERROR"; +} + async function readCredentialConfig(options: CredentialStoreOptions): Promise { const path = credentialConfigPath(options); let raw: string; diff --git a/src/test/keyring.test.ts b/src/test/keyring.test.ts index 832b395..321257f 100644 --- a/src/test/keyring.test.ts +++ b/src/test/keyring.test.ts @@ -20,8 +20,16 @@ class MemoryStore implements SecretStore { public readonly backend = "macos-keychain" as const; public readonly persistent = true as const; public readonly values = new Map(); + public getCalls = 0; + public hasCalls = 0; + + public async has(account: string): Promise { + this.hasCalls += 1; + return this.values.has(account); + } public async get(account: string): Promise { + this.getCalls += 1; return this.values.get(account); } @@ -60,11 +68,14 @@ test("system credential profiles keep only non-secret metadata on disk", async ( backend: "macos-keychain", }); + const getCallsBeforeStatus = store.getCalls; const status = await getCredentialStatus("personal", { configDir, store }); assert.equal(status.configured, true); assert.equal(status.credentialAvailable, true); assert.equal(status.maskedSid, "12****00"); assert.deepEqual(status.profiles, ["personal"]); + assert.equal(store.getCalls, getCallsBeforeStatus); + assert.equal(store.hasCalls, 1); const deleted = await deleteStoredCredentials("personal", { configDir, store }); assert.equal(deleted.removed, true); @@ -249,6 +260,7 @@ test("Linux Secret Service wiring performs store, lookup, and clear through secr configDir: join(root, "config"), platform: "linux" as const, env: fakeEnv, + credentialCommandTimeoutMs: 50, }; try { await mkdir(binDir); @@ -260,6 +272,9 @@ case "$1" in printf '%s' "$password" > "$FAKE_SECRET_STATE" ;; lookup) + if [ "$FAKE_SECRET_LOOKUP_HANG" = "1" ]; then + exec /bin/sleep 60 + fi if [ -s "$FAKE_SECRET_STATE" ]; then /bin/cat "$FAKE_SECRET_STATE" printf '\\n' @@ -290,6 +305,16 @@ esac assert.equal(loaded.password, "secret with spaces"); assert.equal(loaded.backend, "linux-secret-service"); + fakeEnv.FAKE_SECRET_LOOKUP_HANG = "1"; + const startedAt = Date.now(); + const timedOut = await getCredentialStatus(undefined, storeOptions); + assert.ok(Date.now() - startedAt < 2_000); + assert.equal(timedOut.credentialAvailable, false); + assert.equal(timedOut.backendAvailable, false); + assert.equal(timedOut.reasonCode, "CREDENTIAL_STORE_TIMEOUT"); + assert.match(timedOut.reason ?? "", /CREDENTIAL_STORE_TIMEOUT/); + delete fakeEnv.FAKE_SECRET_LOOKUP_HANG; + fakeEnv.FAKE_SECRET_CLEAR_ERROR = "1"; await assert.rejects( deleteStoredCredentials(undefined, storeOptions), From 1e39512593be318485971ca1473d61052461245c Mon Sep 17 00:00:00 2001 From: Grada Date: Sat, 5 Sep 2026 13:29:41 +0800 Subject: [PATCH 2/2] test: restrict short credential deadline to timeout fixture --- src/test/keyring.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/keyring.test.ts b/src/test/keyring.test.ts index 321257f..d47997f 100644 --- a/src/test/keyring.test.ts +++ b/src/test/keyring.test.ts @@ -260,7 +260,6 @@ test("Linux Secret Service wiring performs store, lookup, and clear through secr configDir: join(root, "config"), platform: "linux" as const, env: fakeEnv, - credentialCommandTimeoutMs: 50, }; try { await mkdir(binDir); @@ -307,7 +306,7 @@ esac fakeEnv.FAKE_SECRET_LOOKUP_HANG = "1"; const startedAt = Date.now(); - const timedOut = await getCredentialStatus(undefined, storeOptions); + const timedOut = await getCredentialStatus(undefined, { ...storeOptions, credentialCommandTimeoutMs: 50 }); assert.ok(Date.now() - startedAt < 2_000); assert.equal(timedOut.credentialAvailable, false); assert.equal(timedOut.backendAvailable, false);