From 966c365664f854c9e09d4d39245aaab5955ca25f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Tue, 19 May 2026 11:07:24 +0200 Subject: [PATCH 01/11] feat(auth): use os keyring for secure credential storage Store sensitive API tokens and proxy passwords in OS keyring for improved security, falling back to file. --- package.json | 1 + pnpm-lock.yaml | 135 +++++++++++++++ src/commands/actors/search.ts | 2 +- src/commands/auth/login.ts | 8 +- src/commands/auth/logout.ts | 2 + src/lib/actor.ts | 2 +- src/lib/credentials.ts | 237 +++++++++++++++++++++++++++ src/lib/utils.ts | 64 +++++--- test/__setup__/hooks/useAuthSetup.ts | 6 + test/local/lib/credentials.test.ts | 188 +++++++++++++++++++++ 10 files changed, 623 insertions(+), 22 deletions(-) create mode 100644 src/lib/credentials.ts create mode 100644 test/local/lib/credentials.test.ts diff --git a/package.json b/package.json index 899b8b644..a1f4b84e8 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "@inquirer/input": "^5.0.10", "@inquirer/password": "^5.0.10", "@inquirer/select": "^5.1.2", + "@napi-rs/keyring": "^1.3.0", "@root/walk": "~1.1.0", "@sapphire/duration": "^1.2.0", "@sapphire/result": "^2.8.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a5834240e..c1a30695a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -44,6 +44,9 @@ importers: '@inquirer/select': specifier: ^5.1.2 version: 5.1.2(@types/node@24.12.0) + '@napi-rs/keyring': + specifier: ^1.3.0 + version: 1.3.0 '@root/walk': specifier: ~1.1.0 version: 1.1.0 @@ -2477,6 +2480,87 @@ packages: '@module-federation/webpack-bundler-runtime@0.22.0': resolution: {integrity: sha512-aM8gCqXu+/4wBmJtVeMeeMN5guw3chf+2i6HajKtQv7SJfxV/f4IyNQJUeUQu9HfiAZHjqtMV5Lvq/Lvh8LdyA==} + '@napi-rs/keyring-darwin-arm64@1.3.0': + resolution: {integrity: sha512-pl76hJvdYUBn6I24bXiOBMA9nbDapo3I5B+f3OorjDU4dUMSypXeKbOVehJe8fhgTiH24flMyTS3aAIy43xegQ==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [darwin] + + '@napi-rs/keyring-darwin-x64@1.3.0': + resolution: {integrity: sha512-YcJtEV5LA3cvA4z3BurgxH5IhTsW1JfIvcAAcqcecwk06Si9F9NqkxbZVIfDwQ8oRHgaBmT3zZJnLAotCrVahw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [darwin] + + '@napi-rs/keyring-freebsd-x64@1.3.0': + resolution: {integrity: sha512-vlLf31TGhfRAaxLDBhg8b89ss0HHD/lyNmL5F3UjSaz5CUXElsJmKYq9fqA/B+cZKUEUcLHHGhF0I/CqcFdaVw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [freebsd] + + '@napi-rs/keyring-linux-arm-gnueabihf@1.3.0': + resolution: {integrity: sha512-KiWdMMu/Inz/bHHIAGrnF7r54FZDYXuHO6UFF/rhIrshUsxbMG1Rl9lEymNtqqsVo927G0VYcb02FzWQ3iBQRQ==} + engines: {node: '>= 10'} + cpu: [arm] + os: [linux] + + '@napi-rs/keyring-linux-arm64-gnu@1.3.0': + resolution: {integrity: sha512-eyKGpY40lm9Jvs1aD294XRH4y7+TlJM0YVAryZeXA6TX0mb4gMkxVXwSQv7MCwgah7raeUd0dKUb4BPAYIgcMg==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-arm64-musl@1.3.0': + resolution: {integrity: sha512-iIK6JWHXAJqDrEyLY3TmswwloVyt2vj+04TZnew+uSJ9gnDO8EwRbp3/iw3LpWaXiDO7VomGO6y8I0Id8uBZSw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-linux-riscv64-gnu@1.3.0': + resolution: {integrity: sha512-/PGqrwn6EwgtK6vccASSXJRfOSP4vN1F4ASsIQ+7MdrK6hNvAJ1FZPrIuD5gGGdxezo3F++To2Wq7DbuGIeuNQ==} + engines: {node: '>= 10'} + cpu: [riscv64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-gnu@1.3.0': + resolution: {integrity: sha512-2PDK1WKWTu9lBGq9VvNEkSlQD3O7YwVpmnyN2M3cy4v7NJ/8gDMd9GXv3G+FVXN13uhp4gnnPBS+ScefmEeD2A==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-musl@1.3.0': + resolution: {integrity: sha512-oJ2HkX8YUo46QBkn0pG+HuIKQNqr523q6vBobCn+P95s4C4K6/kLBqHY/1bg5J4ap31DzsznhnFKcfBNBsjCnw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-win32-arm64-msvc@1.3.0': + resolution: {integrity: sha512-tOd3c/uAaeoE4ycVlmAdSvygz0Zt3zdca6Y7gokBeIbaRDWpjDIUOpU3MvML59XAaqyuKGsVVu0F/DZb1lHPmw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [win32] + + '@napi-rs/keyring-win32-ia32-msvc@1.3.0': + resolution: {integrity: sha512-sPSqeAFZMGqP1R++M2JTza7GQJJ/TpCo6JU6Vcd4jnebvOaEDs9b7eipakU1PJdSvhpC2yXMCNRk9gXfrhuwHQ==} + engines: {node: '>= 10'} + cpu: [ia32] + os: [win32] + + '@napi-rs/keyring-win32-x64-msvc@1.3.0': + resolution: {integrity: sha512-4DnCWXwDc0HRKwyRlG5y0VhKZW2tNRQfKKfyj6IX/KWfDNyq9hn4n+GL1auyDcOO/v8PwnhmYo2+rOOqCkvvOg==} + engines: {node: '>= 10'} + cpu: [x64] + os: [win32] + + '@napi-rs/keyring@1.3.0': + resolution: {integrity: sha512-WrOw/bcXm0f9qHkumlT1QlArXSTWqaY9sunsDpOk+yCCorCKMxvWT/a3xko4EYHVdeZoh00yI2TydXn6eyICDA==} + engines: {node: '>= 10'} + '@napi-rs/wasm-runtime@1.0.7': resolution: {integrity: sha512-SeDnOO0Tk7Okiq6DbXmmBODgOAb9dp9gjlphokTUxmt8U3liIP1ZsozBahH69j/RJv+Rfs6IwUKHTgQYJ/HBAw==} @@ -12213,6 +12297,57 @@ snapshots: '@module-federation/runtime': 0.22.0 '@module-federation/sdk': 0.22.0 + '@napi-rs/keyring-darwin-arm64@1.3.0': + optional: true + + '@napi-rs/keyring-darwin-x64@1.3.0': + optional: true + + '@napi-rs/keyring-freebsd-x64@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm-gnueabihf@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm64-musl@1.3.0': + optional: true + + '@napi-rs/keyring-linux-riscv64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-x64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-x64-musl@1.3.0': + optional: true + + '@napi-rs/keyring-win32-arm64-msvc@1.3.0': + optional: true + + '@napi-rs/keyring-win32-ia32-msvc@1.3.0': + optional: true + + '@napi-rs/keyring-win32-x64-msvc@1.3.0': + optional: true + + '@napi-rs/keyring@1.3.0': + optionalDependencies: + '@napi-rs/keyring-darwin-arm64': 1.3.0 + '@napi-rs/keyring-darwin-x64': 1.3.0 + '@napi-rs/keyring-freebsd-x64': 1.3.0 + '@napi-rs/keyring-linux-arm-gnueabihf': 1.3.0 + '@napi-rs/keyring-linux-arm64-gnu': 1.3.0 + '@napi-rs/keyring-linux-arm64-musl': 1.3.0 + '@napi-rs/keyring-linux-riscv64-gnu': 1.3.0 + '@napi-rs/keyring-linux-x64-gnu': 1.3.0 + '@napi-rs/keyring-linux-x64-musl': 1.3.0 + '@napi-rs/keyring-win32-arm64-msvc': 1.3.0 + '@napi-rs/keyring-win32-ia32-msvc': 1.3.0 + '@napi-rs/keyring-win32-x64-msvc': 1.3.0 + '@napi-rs/wasm-runtime@1.0.7': dependencies: '@emnapi/core': 1.10.0 diff --git a/src/commands/actors/search.ts b/src/commands/actors/search.ts index d1a576e18..8f0bd94ed 100644 --- a/src/commands/actors/search.ts +++ b/src/commands/actors/search.ts @@ -96,7 +96,7 @@ export class ActorsSearchCommand extends ApifyCommand { if (isUserLogged) { await updateUserId(userInfo.id!); + const backend = await getBackend(); + const tokenLocation = + backend === 'keyring' + ? 'your OS keyring' + : `${AUTH_FILE_PATH()} (OS keyring unavailable; set APIFY_DISABLE_KEYRING=1 to silence)`; success({ - message: `You are logged in to Apify as ${userInfo.username || userInfo.id}. ${chalk.gray(`Your token is stored at ${AUTH_FILE_PATH()}.`)}`, + message: `You are logged in to Apify as ${userInfo.username || userInfo.id}. ${chalk.gray(`Your token is stored in ${tokenLocation}.`)}`, }); } else { error({ diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index a324e9878..19029efbd 100644 --- a/src/commands/auth/logout.ts +++ b/src/commands/auth/logout.ts @@ -1,5 +1,6 @@ import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; import { AUTH_FILE_PATH } from '../../lib/consts.js'; +import { clearSecrets } from '../../lib/credentials.js'; import { rimrafPromised } from '../../lib/files.js'; import { updateUserId } from '../../lib/hooks/telemetry/useTelemetryState.js'; import { success } from '../../lib/outputs.js'; @@ -24,6 +25,7 @@ export class AuthLogoutCommand extends ApifyCommand { static override docsUrl = 'https://docs.apify.com/cli/docs/reference#apify-logout'; async run() { + await clearSecrets(); await rimrafPromised(AUTH_FILE_PATH()); await updateUserId(null); diff --git a/src/lib/actor.ts b/src/lib/actor.ts index c896fd638..f0823c2c7 100644 --- a/src/lib/actor.ts +++ b/src/lib/actor.ts @@ -57,7 +57,7 @@ export const getApifyStorageClient = async ( const apifyToken = await getApifyTokenFromEnvOrAuthFile(); return new ApifyClient({ - ...getApifyClientOptions(apifyToken), + ...(await getApifyClientOptions(apifyToken)), ...options, }); }; diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts new file mode 100644 index 000000000..fd434b5ba --- /dev/null +++ b/src/lib/credentials.ts @@ -0,0 +1,237 @@ +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; +import process from 'node:process'; + +import { AUTH_FILE_PATH } from './consts.js'; +import { ensureApifyDirectory } from './utils.js'; +import { cliDebugPrint } from './utils/cliDebugPrint.js'; + +const KEYRING_SERVICE = 'com.apify.cli'; +const TOKEN_ACCOUNT = 'token'; +const PROXY_PASSWORD_ACCOUNT = 'proxy-password'; +const PROBE_ACCOUNT = '__probe__'; + +export type CredentialsBackend = 'keyring' | 'file'; + +interface KeyringEntry { + getPassword(): string | null; + setPassword(password: string): void; + deletePassword(): boolean; +} + +interface KeyringModule { + Entry: new (service: string, account: string) => KeyringEntry; +} + +interface StoredAuthFile { + token?: string; + proxy?: { password: string }; + secretsBackend?: CredentialsBackend; + [k: string]: unknown; +} + +let cachedKeyringModule: KeyringModule | null | undefined; +let cachedBackend: CredentialsBackend | undefined; +let migrationPromise: Promise | undefined; + +/** Test-only: clear cached module/backend/migration so each test starts fresh. */ +export function __resetCredentialsForTests() { + cachedKeyringModule = undefined; + cachedBackend = undefined; + migrationPromise = undefined; +} + +async function loadKeyringModule(): Promise { + if (cachedKeyringModule !== undefined) return cachedKeyringModule; + try { + // Indirect specifier so tsc doesn't try to resolve the module at compile time. + const specifier = '@napi-rs/keyring'; + cachedKeyringModule = (await import(specifier)) as KeyringModule; + } catch (err) { + cliDebugPrint('credentials', 'failed to load @napi-rs/keyring', err); + cachedKeyringModule = null; + } + return cachedKeyringModule; +} + +function probeKeyring(mod: KeyringModule): boolean { + try { + const entry = new mod.Entry(KEYRING_SERVICE, PROBE_ACCOUNT); + entry.setPassword('1'); + entry.getPassword(); + entry.deletePassword(); + return true; + } catch (err) { + cliDebugPrint('credentials', 'keyring probe failed', err); + return false; + } +} + +/** + * Picks a backend the first time it's called and caches the result for the rest of the process. + * Order: APIFY_DISABLE_KEYRING env override -> module load -> runtime probe. + */ +export async function getBackend(): Promise { + if (cachedBackend) return cachedBackend; + + if (process.env.APIFY_DISABLE_KEYRING === '1') { + cachedBackend = 'file'; + return cachedBackend; + } + + const mod = await loadKeyringModule(); + if (!mod) { + cachedBackend = 'file'; + return cachedBackend; + } + + cachedBackend = probeKeyring(mod) ? 'keyring' : 'file'; + return cachedBackend; +} + +function readAuthFile(): StoredAuthFile { + if (!existsSync(AUTH_FILE_PATH())) return {}; + try { + const raw = readFileSync(AUTH_FILE_PATH(), 'utf-8'); + return JSON.parse(raw) as StoredAuthFile; + } catch { + return {}; + } +} + +function writeAuthFile(data: StoredAuthFile) { + ensureApifyDirectory(AUTH_FILE_PATH()); + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(data, null, '\t')); +} + +async function getKeyringEntry(account: string): Promise { + const mod = await loadKeyringModule(); + if (!mod) return null; + return new mod.Entry(KEYRING_SERVICE, account); +} + +async function readKeyring(account: string): Promise { + try { + const entry = await getKeyringEntry(account); + if (!entry) return undefined; + return entry.getPassword() ?? undefined; + } catch (err) { + cliDebugPrint('credentials', `failed to read ${account} from keyring`, err); + return undefined; + } +} + +async function writeKeyring(account: string, value: string): Promise { + const entry = await getKeyringEntry(account); + if (!entry) { + throw new Error('OS keyring is not available.'); + } + entry.setPassword(value); +} + +async function deleteKeyring(account: string): Promise { + try { + const entry = await getKeyringEntry(account); + if (!entry) return; + entry.deletePassword(); + } catch (err) { + cliDebugPrint('credentials', `failed to delete ${account} from keyring`, err); + } +} + +export async function getToken(): Promise { + const backend = await getBackend(); + if (backend === 'keyring') return readKeyring(TOKEN_ACCOUNT); + return readAuthFile().token; +} + +export async function getProxyPassword(): Promise { + const backend = await getBackend(); + if (backend === 'keyring') return readKeyring(PROXY_PASSWORD_ACCOUNT); + return readAuthFile().proxy?.password; +} + +/** + * Persist token. When `skipIfUnchanged` is true and the stored value already matches, + * the write is skipped. This avoids macOS Keychain prompts on every command. + */ +export async function setToken(token: string, opts: { skipIfUnchanged?: boolean } = {}): Promise { + const backend = await getBackend(); + if (opts.skipIfUnchanged) { + const existing = backend === 'keyring' ? await readKeyring(TOKEN_ACCOUNT) : readAuthFile().token; + if (existing === token) return; + } + + if (backend === 'keyring') { + await writeKeyring(TOKEN_ACCOUNT, token); + return; + } + + const data = readAuthFile(); + data.token = token; + data.secretsBackend = 'file'; + writeAuthFile(data); +} + +export async function setProxyPassword(password: string, opts: { skipIfUnchanged?: boolean } = {}): Promise { + const backend = await getBackend(); + if (opts.skipIfUnchanged) { + const existing = backend === 'keyring' ? await readKeyring(PROXY_PASSWORD_ACCOUNT) : readAuthFile().proxy?.password; + if (existing === password) return; + } + + if (backend === 'keyring') { + await writeKeyring(PROXY_PASSWORD_ACCOUNT, password); + return; + } + + const data = readAuthFile(); + data.proxy = { password }; + data.secretsBackend = 'file'; + writeAuthFile(data); +} + +/** Remove all stored secrets from the active backend. */ +export async function clearSecrets(): Promise { + const backend = await getBackend(); + if (backend === 'keyring') { + await deleteKeyring(TOKEN_ACCOUNT); + await deleteKeyring(PROXY_PASSWORD_ACCOUNT); + } +} + +/** + * One-shot, idempotent migration of legacy plaintext auth.json to the keyring. + * + * - `secretsBackend` marker in auth.json makes re-entry a no-op. + * - On `file` backend the marker is written but secrets stay where they are. + * - On `keyring` backend secrets are moved out of auth.json. + * - Wrapped in try/catch so a migration failure never blocks the CLI. + */ +export async function ensureMigrated(): Promise { + if (migrationPromise) return migrationPromise; + migrationPromise = (async () => { + try { + const file = readAuthFile(); + if (file.secretsBackend) return; + if (!file.token && !file.proxy?.password) return; + + const backend = await getBackend(); + if (backend === 'file') { + file.secretsBackend = 'file'; + writeAuthFile(file); + return; + } + + if (file.token) await writeKeyring(TOKEN_ACCOUNT, file.token); + if (file.proxy?.password) await writeKeyring(PROXY_PASSWORD_ACCOUNT, file.proxy.password); + + delete file.token; + delete file.proxy; + file.secretsBackend = 'keyring'; + writeAuthFile(file); + } catch (err) { + cliDebugPrint('credentials', 'migration failed', err); + } + })(); + return migrationPromise; +} diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 852d1b242..53defe861 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -40,11 +40,11 @@ import { AUTH_FILE_PATH, CommandExitCodes, DEFAULT_LOCAL_STORAGE_DIR, - GLOBAL_CONFIGS_FOLDER, LOCAL_CONFIG_PATH, MINIMUM_SUPPORTED_PYTHON_VERSION, SUPPORTED_NODEJS_VERSION, } from './consts.js'; +import { ensureMigrated, getProxyPassword, getToken, setProxyPassword, setToken } from './credentials.js'; import { deleteFile, ensureFolderExistsSync, rimrafPromised } from './files.js'; import { inputFileRegExp, TEMP_INPUT_KEY_PREFIX } from './input-key.js'; import type { AuthJSON } from './types.js'; @@ -101,17 +101,29 @@ export const getLocalRequestQueuePath = (storeId?: string) => { }; /** - * Returns object from auth file or empty object. + * Returns object from auth file or empty object. Secrets (token, proxy.password) are pulled + * from the keyring when that backend is active; non-sensitive metadata stays in auth.json. */ export const getLocalUserInfo = async (): Promise => { + await ensureMigrated(); + let result: AuthJSON = {}; try { const raw = await readFile(AUTH_FILE_PATH(), 'utf-8'); result = JSON.parse(raw) as AuthJSON; } catch { - return {}; + // auth.json may not exist yet (fresh keyring-only state); fall through } + const token = await getToken(); + if (token) result.token = token; + + const proxyPassword = await getProxyPassword(); + if (proxyPassword) result.proxy = { password: proxyPassword }; + + const hasSomething = result.username || result.id || result.token; + if (!hasSomething) return {}; + if (!result.username && !result.id) { throw new Error('Corrupted local user info was found. Please run "apify login" to fix it.'); } @@ -132,13 +144,10 @@ export async function getLoggedClientOrThrow() { return loggedClient; } -const getTokenWithAuthFileFallback = (existingToken?: string) => { - if (!existingToken && existsSync(GLOBAL_CONFIGS_FOLDER()) && existsSync(AUTH_FILE_PATH())) { - const raw = readFileSync(AUTH_FILE_PATH(), 'utf-8'); - return JSON.parse(raw).token; - } - - return existingToken; +const resolveToken = async (existingToken?: string): Promise => { + if (existingToken) return existingToken; + await ensureMigrated(); + return getToken(); }; type CJSAxiosHeaders = import('axios', { with: { 'resolution-mode': 'require' } }).AxiosRequestConfig['headers']; @@ -146,8 +155,8 @@ type CJSAxiosHeaders = import('axios', { with: { 'resolution-mode': 'require' } /** * Returns options for ApifyClient */ -export const getApifyClientOptions = (token?: string, apiBaseUrl?: string): ApifyClientOptions => { - token = getTokenWithAuthFileFallback(token); +export const getApifyClientOptions = async (token?: string, apiBaseUrl?: string): Promise => { + token = await resolveToken(token); return { token, @@ -168,13 +177,14 @@ export const getApifyClientOptions = (token?: string, apiBaseUrl?: string): Apif /** * Gets instance of ApifyClient for token or for params from global auth file. - * NOTE: It refreshes global auth file each run - * @param [token] + * + * Refreshes the user metadata in auth.json each run. Secrets (token, proxy.password) only + * get written when their value actually changes — avoids macOS Keychain prompts on every command. */ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { - token = getTokenWithAuthFileFallback(token); + token = await resolveToken(token); - const apifyClient = new ApifyClient(getApifyClientOptions(token, apiBaseUrl)); + const apifyClient = new ApifyClient(await getApifyClientOptions(token, apiBaseUrl)); let userInfo; try { @@ -184,10 +194,26 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { return null; } - // Always refresh Auth file - ensureApifyDirectory(AUTH_FILE_PATH()); + if (apifyClient.token) { + await setToken(apifyClient.token, { skipIfUnchanged: true }); + } + if (userInfo.proxy?.password) { + await setProxyPassword(userInfo.proxy.password, { skipIfUnchanged: true }); + } - writeFileSync(AUTH_FILE_PATH(), JSON.stringify({ token: apifyClient.token, ...userInfo }, null, '\t')); + const { proxy: _proxy, ...metadataOnly } = userInfo as unknown as AuthJSON & Record; + ensureApifyDirectory(AUTH_FILE_PATH()); + const existingFile = (() => { + try { + return JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')) as Record; + } catch { + return {}; + } + })(); + writeFileSync( + AUTH_FILE_PATH(), + JSON.stringify({ ...existingFile, ...metadataOnly, secretsBackend: existingFile.secretsBackend }, null, '\t'), + ); return apifyClient; } diff --git a/test/__setup__/hooks/useAuthSetup.ts b/test/__setup__/hooks/useAuthSetup.ts index 841d54d07..78a55b32d 100644 --- a/test/__setup__/hooks/useAuthSetup.ts +++ b/test/__setup__/hooks/useAuthSetup.ts @@ -8,6 +8,7 @@ import { cryptoRandomObjectId } from '@apify/utilities'; import { LoginCommand } from '../../../src/commands/login.js'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; import { GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; +import { __resetCredentialsForTests } from '../../../src/lib/credentials.js'; import { getLocalUserInfo } from '../../../src/lib/utils.js'; export interface UseAuthSetupOptions { @@ -39,6 +40,10 @@ export function useAuthSetup({ cleanup = true, perTest = true }: UseAuthSetupOpt before(() => { vitest.stubEnv(envVariable, envValue()); + // Tests pin to the file backend so they don't touch the real OS keyring. + // Unit tests for credentials.ts override this explicitly. + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + __resetCredentialsForTests(); }); after(async () => { @@ -46,6 +51,7 @@ export function useAuthSetup({ cleanup = true, perTest = true }: UseAuthSetupOpt await rm(GLOBAL_CONFIGS_FOLDER(), { recursive: true, force: true }); } + __resetCredentialsForTests(); vitest.unstubAllEnvs(); }); } diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts new file mode 100644 index 000000000..d29e7778a --- /dev/null +++ b/test/local/lib/credentials.test.ts @@ -0,0 +1,188 @@ +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { rm } from 'node:fs/promises'; + +import { cryptoRandomObjectId } from '@apify/utilities'; + +import { AUTH_FILE_PATH, GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; +import { + __resetCredentialsForTests, + clearSecrets, + ensureMigrated, + getBackend, + getProxyPassword, + getToken, + setProxyPassword, + setToken, +} from '../../../src/lib/credentials.js'; + +const keyringStore = new Map(); + +vi.mock('@napi-rs/keyring', () => { + class Entry { + private key: string; + constructor(service: string, account: string) { + this.key = `${service}:${account}`; + } + getPassword(): string | null { + return keyringStore.get(this.key) ?? null; + } + setPassword(password: string): void { + keyringStore.set(this.key, password); + } + deletePassword(): boolean { + return keyringStore.delete(this.key); + } + } + return { Entry }; +}); + +const writeAuthFile = (data: Record) => { + mkdirSync(GLOBAL_CONFIGS_FOLDER(), { recursive: true }); + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(data)); +}; + +const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); + +describe('credentials', () => { + beforeEach(() => { + vitest.stubEnv('__APIFY_INTERNAL_TEST_AUTH_PATH__', cryptoRandomObjectId(12)); + keyringStore.clear(); + __resetCredentialsForTests(); + }); + + afterEach(async () => { + await rm(GLOBAL_CONFIGS_FOLDER(), { recursive: true, force: true }); + vitest.unstubAllEnvs(); + __resetCredentialsForTests(); + }); + + describe('getBackend()', () => { + it('returns "file" when APIFY_DISABLE_KEYRING=1', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + expect(await getBackend()).toBe('file'); + }); + + it('returns "keyring" when the keyring probe succeeds', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + expect(await getBackend()).toBe('keyring'); + }); + + it('caches the backend choice for the rest of the process', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + expect(await getBackend()).toBe('file'); + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + expect(await getBackend()).toBe('file'); + }); + }); + + describe('file backend', () => { + beforeEach(() => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + }); + + it('round-trips the token through auth.json', async () => { + await setToken('tok_123'); + expect(await getToken()).toBe('tok_123'); + const file = readAuthFile(); + expect(file.token).toBe('tok_123'); + expect(file.secretsBackend).toBe('file'); + }); + + it('round-trips the proxy password through auth.json', async () => { + await setProxyPassword('pw_abc'); + expect(await getProxyPassword()).toBe('pw_abc'); + expect(readAuthFile().proxy).toEqual({ password: 'pw_abc' }); + }); + + it('skipIfUnchanged is a no-op when the stored value matches', async () => { + await setToken('tok_123'); + const before = readFileSync(AUTH_FILE_PATH(), 'utf-8'); + await setToken('tok_123', { skipIfUnchanged: true }); + const after = readFileSync(AUTH_FILE_PATH(), 'utf-8'); + expect(after).toBe(before); + }); + + it('skipIfUnchanged still writes when the value differs', async () => { + await setToken('tok_123'); + await setToken('tok_456', { skipIfUnchanged: true }); + expect(await getToken()).toBe('tok_456'); + }); + }); + + describe('keyring backend', () => { + beforeEach(() => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + }); + + it('round-trips the token through the keyring and keeps it out of auth.json', async () => { + await setToken('tok_123'); + expect(await getToken()).toBe('tok_123'); + expect(keyringStore.get('com.apify.cli:token')).toBe('tok_123'); + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + }); + + it('round-trips the proxy password through the keyring', async () => { + await setProxyPassword('pw_abc'); + expect(await getProxyPassword()).toBe('pw_abc'); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw_abc'); + }); + + it('clearSecrets() removes both entries', async () => { + await setToken('tok_123'); + await setProxyPassword('pw_abc'); + await clearSecrets(); + expect(await getToken()).toBeUndefined(); + expect(await getProxyPassword()).toBeUndefined(); + }); + }); + + describe('ensureMigrated()', () => { + it('is a no-op when secretsBackend marker is already set', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ token: 'tok', secretsBackend: 'file' }); + await ensureMigrated(); + expect(readAuthFile().token).toBe('tok'); + }); + + it('is a no-op when there are no secrets to migrate', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + await ensureMigrated(); + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + }); + + it('on the file backend, stamps the marker without moving data', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ token: 'tok', username: 'u' }); + await ensureMigrated(); + const file = readAuthFile(); + expect(file.token).toBe('tok'); + expect(file.username).toBe('u'); + expect(file.secretsBackend).toBe('file'); + }); + + it('on the keyring backend, moves secrets out of auth.json', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + writeAuthFile({ token: 'tok', proxy: { password: 'pw' }, username: 'u' }); + await ensureMigrated(); + expect(keyringStore.get('com.apify.cli:token')).toBe('tok'); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw'); + const file = readAuthFile(); + expect(file.token).toBeUndefined(); + expect(file.proxy).toBeUndefined(); + expect(file.username).toBe('u'); + expect(file.secretsBackend).toBe('keyring'); + }); + + it('is memoized within a process', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ token: 'tok' }); + await ensureMigrated(); + expect(readAuthFile().secretsBackend).toBe('file'); + + // Overwrite the marker and call again — the memoized promise should short-circuit. + writeAuthFile({ token: 'tok2' }); + await ensureMigrated(); + expect(readAuthFile().secretsBackend).toBeUndefined(); + }); + }); +}); From 603a08699a9340717976138848cb7eb1cba9f5cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 11:31:09 +0000 Subject: [PATCH 02/11] fix(tests): await async getApifyClientOptions in test clients getApifyClientOptions became async but several test sites still passed the returned Promise directly to `new ApifyClient(...)`, leaving the client with undefined token/baseUrl. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/__setup__/config.ts | 4 ++-- test/e2e/commands/builds/lifecycle.test.ts | 2 +- test/e2e/commands/datasets/lifecycle.test.ts | 2 +- test/e2e/commands/key-value-stores/lifecycle.test.ts | 2 +- test/e2e/commands/runs/lifecycle.test.ts | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/__setup__/config.ts b/test/__setup__/config.ts index b2435f994..0c14fdf3a 100644 --- a/test/__setup__/config.ts +++ b/test/__setup__/config.ts @@ -13,9 +13,9 @@ if (!ENV_TEST_USER_TOKEN) { throw Error('You must configure "TEST_USER_TOKEN" environment variable to run tests!'); } -export const testUserClient = new ApifyClient(getApifyClientOptions(ENV_TEST_USER_TOKEN)); +export const testUserClient = new ApifyClient(await getApifyClientOptions(ENV_TEST_USER_TOKEN)); -export const badUserClient = new ApifyClient(getApifyClientOptions(TEST_USER_BAD_TOKEN)); +export const badUserClient = new ApifyClient(await getApifyClientOptions(TEST_USER_BAD_TOKEN)); export const TEST_USER_TOKEN = ENV_TEST_USER_TOKEN; diff --git a/test/e2e/commands/builds/lifecycle.test.ts b/test/e2e/commands/builds/lifecycle.test.ts index b54f65bdc..4697ec1ef 100644 --- a/test/e2e/commands/builds/lifecycle.test.ts +++ b/test/e2e/commands/builds/lifecycle.test.ts @@ -25,7 +25,7 @@ describe('[e2e][api] builds namespace', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); actor = await createTestActor('e2e-builds'); diff --git a/test/e2e/commands/datasets/lifecycle.test.ts b/test/e2e/commands/datasets/lifecycle.test.ts index 838a67bc0..b0b2deecf 100644 --- a/test/e2e/commands/datasets/lifecycle.test.ts +++ b/test/e2e/commands/datasets/lifecycle.test.ts @@ -24,7 +24,7 @@ describe('[e2e][api] datasets namespace', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); }); afterAll(async () => { diff --git a/test/e2e/commands/key-value-stores/lifecycle.test.ts b/test/e2e/commands/key-value-stores/lifecycle.test.ts index 191e76062..f01399a4b 100644 --- a/test/e2e/commands/key-value-stores/lifecycle.test.ts +++ b/test/e2e/commands/key-value-stores/lifecycle.test.ts @@ -24,7 +24,7 @@ describe('[e2e][api] key-value-stores namespace', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); }); afterAll(async () => { diff --git a/test/e2e/commands/runs/lifecycle.test.ts b/test/e2e/commands/runs/lifecycle.test.ts index 9c40111aa..da294a83f 100644 --- a/test/e2e/commands/runs/lifecycle.test.ts +++ b/test/e2e/commands/runs/lifecycle.test.ts @@ -30,7 +30,7 @@ describe('[e2e][api] runs lifecycle', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); const me = await client.user('me').get(); actor = await createTestActor('e2e-runs'); From b47629fd9067e38c3a1fc780309a81567b72303d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 12:07:40 +0000 Subject: [PATCH 03/11] fix(auth): skip keyring probe when backend already chosen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persist the backend marker in auth.json on first login and honor it on subsequent runs so we don't re-probe the OS keyring on every CLI invocation. Also drop the write/delete side of the probe — getPassword on a non-existent entry is enough to detect an unavailable backend and avoids unnecessary Keychain access. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/credentials.ts | 15 ++++++++++++--- src/lib/utils.ts | 5 +++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index fd434b5ba..75ae052e0 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -56,9 +56,7 @@ async function loadKeyringModule(): Promise { function probeKeyring(mod: KeyringModule): boolean { try { const entry = new mod.Entry(KEYRING_SERVICE, PROBE_ACCOUNT); - entry.setPassword('1'); entry.getPassword(); - entry.deletePassword(); return true; } catch (err) { cliDebugPrint('credentials', 'keyring probe failed', err); @@ -68,7 +66,7 @@ function probeKeyring(mod: KeyringModule): boolean { /** * Picks a backend the first time it's called and caches the result for the rest of the process. - * Order: APIFY_DISABLE_KEYRING env override -> module load -> runtime probe. + * Order: APIFY_DISABLE_KEYRING env override -> persisted marker in auth.json -> module load -> read-only probe. */ export async function getBackend(): Promise { if (cachedBackend) return cachedBackend; @@ -78,6 +76,17 @@ export async function getBackend(): Promise { return cachedBackend; } + const marker = readAuthFile().secretsBackend; + if (marker === 'file') { + cachedBackend = 'file'; + return cachedBackend; + } + if (marker === 'keyring') { + const mod = await loadKeyringModule(); + cachedBackend = mod ? 'keyring' : 'file'; + return cachedBackend; + } + const mod = await loadKeyringModule(); if (!mod) { cachedBackend = 'file'; diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 53defe861..713b1002f 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -44,7 +44,7 @@ import { MINIMUM_SUPPORTED_PYTHON_VERSION, SUPPORTED_NODEJS_VERSION, } from './consts.js'; -import { ensureMigrated, getProxyPassword, getToken, setProxyPassword, setToken } from './credentials.js'; +import { ensureMigrated, getBackend, getProxyPassword, getToken, setProxyPassword, setToken } from './credentials.js'; import { deleteFile, ensureFolderExistsSync, rimrafPromised } from './files.js'; import { inputFileRegExp, TEMP_INPUT_KEY_PREFIX } from './input-key.js'; import type { AuthJSON } from './types.js'; @@ -210,9 +210,10 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { return {}; } })(); + const backend = await getBackend(); writeFileSync( AUTH_FILE_PATH(), - JSON.stringify({ ...existingFile, ...metadataOnly, secretsBackend: existingFile.secretsBackend }, null, '\t'), + JSON.stringify({ ...existingFile, ...metadataOnly, secretsBackend: backend }, null, '\t'), ); return apifyClient; From 0e8c27c7a95307f4393a881f02849247a0dff5d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 12:40:49 +0000 Subject: [PATCH 04/11] fix(tests): pin e2e CLI subprocesses to file credential backend The OS keyring is machine-global, so the unique __APIFY_INTERNAL_TEST_AUTH_PATH__ per test only isolates auth.json, not the keyring. After one test logged in, the leaked token made later tests see getToken() return a value with no matching username/id and throw "Corrupted local user info". useAuthSetup already pins in-process tests to the file backend; do the same for the dist subprocesses runCli spawns. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/e2e/__helpers__/run-cli.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/__helpers__/run-cli.ts b/test/e2e/__helpers__/run-cli.ts index 8d1d05dca..fcaff98e6 100644 --- a/test/e2e/__helpers__/run-cli.ts +++ b/test/e2e/__helpers__/run-cli.ts @@ -45,6 +45,8 @@ export async function runCli( env: { APIFY_CLI_DISABLE_TELEMETRY: '1', APIFY_CLI_SKIP_UPDATE_CHECK: '1', + // Pin the file backend so e2e subprocesses don't share the host's OS keyring across tests. + APIFY_DISABLE_KEYRING: '1', ...options.env, }, }); From 9479f488101cba2f67ddf7dc3d09e4e37c179296 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 12:59:05 +0000 Subject: [PATCH 05/11] fix(auth): always clear keyring entries on logout Previously, clearSecrets() was a no-op when the active backend was 'file', so a user who logged in normally and later set APIFY_DISABLE_KEYRING=1 before running logout would leave their token in the OS keyring with no in-CLI way to remove it. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/credentials.ts | 13 +++++++------ test/local/lib/credentials.test.ts | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 75ae052e0..93613915f 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -199,13 +199,14 @@ export async function setProxyPassword(password: string, opts: { skipIfUnchanged writeAuthFile(data); } -/** Remove all stored secrets from the active backend. */ +/** + * Remove all stored secrets. Always attempts to clear the keyring even when the + * current backend is `file`, so toggling `APIFY_DISABLE_KEYRING=1` between login + * and logout does not orphan entries the user has no in-CLI way to discover. + */ export async function clearSecrets(): Promise { - const backend = await getBackend(); - if (backend === 'keyring') { - await deleteKeyring(TOKEN_ACCOUNT); - await deleteKeyring(PROXY_PASSWORD_ACCOUNT); - } + await deleteKeyring(TOKEN_ACCOUNT); + await deleteKeyring(PROXY_PASSWORD_ACCOUNT); } /** diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts index d29e7778a..228242c48 100644 --- a/test/local/lib/credentials.test.ts +++ b/test/local/lib/credentials.test.ts @@ -136,6 +136,23 @@ describe('credentials', () => { }); }); + describe('clearSecrets()', () => { + it('clears keyring entries even when APIFY_DISABLE_KEYRING=1 is set at logout time', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + await setToken('tok_123'); + await setProxyPassword('pw_abc'); + expect(keyringStore.get('com.apify.cli:token')).toBe('tok_123'); + + __resetCredentialsForTests(); + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + expect(await getBackend()).toBe('file'); + + await clearSecrets(); + expect(keyringStore.get('com.apify.cli:token')).toBeUndefined(); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBeUndefined(); + }); + }); + describe('ensureMigrated()', () => { it('is a no-op when secretsBackend marker is already set', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); From ecb4790bac66bb17069a89732d02a67dcab3b627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 13:01:52 +0000 Subject: [PATCH 06/11] fix(auth): distinguish env-disabled keyring from probe failure in login message When the user explicitly opts out via APIFY_DISABLE_KEYRING=1, calling the keyring "unavailable" and telling them to set the var they already set is misleading. Split the file-backend branch into two: env-disabled vs probe failure. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/auth/login.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index e75c19154..ceb1498d3 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -1,5 +1,6 @@ import type { Server } from 'node:http'; import type { AddressInfo } from 'node:net'; +import process from 'node:process'; import chalk from 'chalk'; import computerName from 'computer-name'; @@ -36,10 +37,14 @@ const tryToLogin = async (token: string) => { await updateUserId(userInfo.id!); const backend = await getBackend(); - const tokenLocation = - backend === 'keyring' - ? 'your OS keyring' - : `${AUTH_FILE_PATH()} (OS keyring unavailable; set APIFY_DISABLE_KEYRING=1 to silence)`; + let tokenLocation: string; + if (backend === 'keyring') { + tokenLocation = 'your OS keyring'; + } else if (process.env.APIFY_DISABLE_KEYRING === '1') { + tokenLocation = `${AUTH_FILE_PATH()} (OS keyring disabled via APIFY_DISABLE_KEYRING)`; + } else { + tokenLocation = `${AUTH_FILE_PATH()} (OS keyring unavailable; set APIFY_DISABLE_KEYRING=1 to silence)`; + } success({ message: `You are logged in to Apify as ${userInfo.username || userInfo.id}. ${chalk.gray(`Your token is stored in ${tokenLocation}.`)}`, }); From f9eaed9cf1df45ebc814c2aa9f7c25f5da9795a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 13:39:57 +0000 Subject: [PATCH 07/11] fix(auth): keep proxy.password in auth.json, route only token through keyring Scoping the keyring change down: stripping the entire proxy object from the userInfo write in getLoggedClient also dropped proxy.groups, which breaks the log_in_out API test that compares auth.json to the API user response. Leave proxy in the file as it was before and exclude the internal secretsBackend marker from the test comparison. --- src/lib/credentials.ts | 29 +++++++--------------------- src/lib/utils.ts | 20 ++++++------------- test/api/commands/log_in_out.test.ts | 2 ++ test/local/lib/credentials.test.ts | 19 ++++++++---------- 4 files changed, 23 insertions(+), 47 deletions(-) diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 93613915f..32db4f6b4 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -7,7 +7,6 @@ import { cliDebugPrint } from './utils/cliDebugPrint.js'; const KEYRING_SERVICE = 'com.apify.cli'; const TOKEN_ACCOUNT = 'token'; -const PROXY_PASSWORD_ACCOUNT = 'proxy-password'; const PROBE_ACCOUNT = '__probe__'; export type CredentialsBackend = 'keyring' | 'file'; @@ -154,8 +153,6 @@ export async function getToken(): Promise { } export async function getProxyPassword(): Promise { - const backend = await getBackend(); - if (backend === 'keyring') return readKeyring(PROXY_PASSWORD_ACCOUNT); return readAuthFile().proxy?.password; } @@ -182,20 +179,10 @@ export async function setToken(token: string, opts: { skipIfUnchanged?: boolean } export async function setProxyPassword(password: string, opts: { skipIfUnchanged?: boolean } = {}): Promise { - const backend = await getBackend(); - if (opts.skipIfUnchanged) { - const existing = backend === 'keyring' ? await readKeyring(PROXY_PASSWORD_ACCOUNT) : readAuthFile().proxy?.password; - if (existing === password) return; - } - - if (backend === 'keyring') { - await writeKeyring(PROXY_PASSWORD_ACCOUNT, password); - return; - } + if (opts.skipIfUnchanged && readAuthFile().proxy?.password === password) return; const data = readAuthFile(); data.proxy = { password }; - data.secretsBackend = 'file'; writeAuthFile(data); } @@ -206,15 +193,16 @@ export async function setProxyPassword(password: string, opts: { skipIfUnchanged */ export async function clearSecrets(): Promise { await deleteKeyring(TOKEN_ACCOUNT); - await deleteKeyring(PROXY_PASSWORD_ACCOUNT); } /** * One-shot, idempotent migration of legacy plaintext auth.json to the keyring. * + * Only the API token is moved into the keyring; proxy password stays in auth.json. + * * - `secretsBackend` marker in auth.json makes re-entry a no-op. - * - On `file` backend the marker is written but secrets stay where they are. - * - On `keyring` backend secrets are moved out of auth.json. + * - On `file` backend the marker is written but the token stays in auth.json. + * - On `keyring` backend the token is moved out of auth.json. * - Wrapped in try/catch so a migration failure never blocks the CLI. */ export async function ensureMigrated(): Promise { @@ -223,7 +211,7 @@ export async function ensureMigrated(): Promise { try { const file = readAuthFile(); if (file.secretsBackend) return; - if (!file.token && !file.proxy?.password) return; + if (!file.token) return; const backend = await getBackend(); if (backend === 'file') { @@ -232,11 +220,8 @@ export async function ensureMigrated(): Promise { return; } - if (file.token) await writeKeyring(TOKEN_ACCOUNT, file.token); - if (file.proxy?.password) await writeKeyring(PROXY_PASSWORD_ACCOUNT, file.proxy.password); - + await writeKeyring(TOKEN_ACCOUNT, file.token); delete file.token; - delete file.proxy; file.secretsBackend = 'keyring'; writeAuthFile(file); } catch (err) { diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 713b1002f..352b85055 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -44,7 +44,7 @@ import { MINIMUM_SUPPORTED_PYTHON_VERSION, SUPPORTED_NODEJS_VERSION, } from './consts.js'; -import { ensureMigrated, getBackend, getProxyPassword, getToken, setProxyPassword, setToken } from './credentials.js'; +import { ensureMigrated, getBackend, getToken, setToken } from './credentials.js'; import { deleteFile, ensureFolderExistsSync, rimrafPromised } from './files.js'; import { inputFileRegExp, TEMP_INPUT_KEY_PREFIX } from './input-key.js'; import type { AuthJSON } from './types.js'; @@ -101,8 +101,8 @@ export const getLocalRequestQueuePath = (storeId?: string) => { }; /** - * Returns object from auth file or empty object. Secrets (token, proxy.password) are pulled - * from the keyring when that backend is active; non-sensitive metadata stays in auth.json. + * Returns object from auth file or empty object. The token is pulled from the keyring + * when that backend is active; all other fields (including proxy) live in auth.json. */ export const getLocalUserInfo = async (): Promise => { await ensureMigrated(); @@ -118,9 +118,6 @@ export const getLocalUserInfo = async (): Promise => { const token = await getToken(); if (token) result.token = token; - const proxyPassword = await getProxyPassword(); - if (proxyPassword) result.proxy = { password: proxyPassword }; - const hasSomething = result.username || result.id || result.token; if (!hasSomething) return {}; @@ -197,11 +194,7 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { if (apifyClient.token) { await setToken(apifyClient.token, { skipIfUnchanged: true }); } - if (userInfo.proxy?.password) { - await setProxyPassword(userInfo.proxy.password, { skipIfUnchanged: true }); - } - const { proxy: _proxy, ...metadataOnly } = userInfo as unknown as AuthJSON & Record; ensureApifyDirectory(AUTH_FILE_PATH()); const existingFile = (() => { try { @@ -211,10 +204,9 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { } })(); const backend = await getBackend(); - writeFileSync( - AUTH_FILE_PATH(), - JSON.stringify({ ...existingFile, ...metadataOnly, secretsBackend: backend }, null, '\t'), - ); + const fileContents: Record = { ...existingFile, ...userInfo, secretsBackend: backend }; + if (backend === 'keyring') delete fileContents.token; + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(fileContents, null, '\t')); return apifyClient; } diff --git a/test/api/commands/log_in_out.test.ts b/test/api/commands/log_in_out.test.ts index 6987d4cfc..5ae48d0ed 100644 --- a/test/api/commands/log_in_out.test.ts +++ b/test/api/commands/log_in_out.test.ts @@ -51,6 +51,7 @@ describe('[api] apify login and logout', () => { currentBillingPeriod: _4, plan: _5, createdAt: _6, + secretsBackend: _7, ...userInfoFromConfigWithoutFloatFields } = userInfoFromConfig; @@ -101,6 +102,7 @@ describe('[api] apify login and logout', () => { currentBillingPeriod: _4, plan: _5, createdAt: _6, + secretsBackend: _7, ...userInfoFromConfigWithoutFloatFields } = userInfoFromConfig; diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts index 228242c48..2f8fb3b8f 100644 --- a/test/local/lib/credentials.test.ts +++ b/test/local/lib/credentials.test.ts @@ -121,26 +121,24 @@ describe('credentials', () => { expect(existsSync(AUTH_FILE_PATH())).toBe(false); }); - it('round-trips the proxy password through the keyring', async () => { + it('proxy password always lives in auth.json, not the keyring', async () => { await setProxyPassword('pw_abc'); expect(await getProxyPassword()).toBe('pw_abc'); - expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw_abc'); + expect(readAuthFile().proxy).toEqual({ password: 'pw_abc' }); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBeUndefined(); }); - it('clearSecrets() removes both entries', async () => { + it('clearSecrets() removes the token entry from the keyring', async () => { await setToken('tok_123'); - await setProxyPassword('pw_abc'); await clearSecrets(); expect(await getToken()).toBeUndefined(); - expect(await getProxyPassword()).toBeUndefined(); }); }); describe('clearSecrets()', () => { - it('clears keyring entries even when APIFY_DISABLE_KEYRING=1 is set at logout time', async () => { + it('clears the keyring token entry even when APIFY_DISABLE_KEYRING=1 is set at logout time', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); await setToken('tok_123'); - await setProxyPassword('pw_abc'); expect(keyringStore.get('com.apify.cli:token')).toBe('tok_123'); __resetCredentialsForTests(); @@ -149,7 +147,6 @@ describe('credentials', () => { await clearSecrets(); expect(keyringStore.get('com.apify.cli:token')).toBeUndefined(); - expect(keyringStore.get('com.apify.cli:proxy-password')).toBeUndefined(); }); }); @@ -177,15 +174,15 @@ describe('credentials', () => { expect(file.secretsBackend).toBe('file'); }); - it('on the keyring backend, moves secrets out of auth.json', async () => { + it('on the keyring backend, moves the token out of auth.json but leaves proxy in place', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); writeAuthFile({ token: 'tok', proxy: { password: 'pw' }, username: 'u' }); await ensureMigrated(); expect(keyringStore.get('com.apify.cli:token')).toBe('tok'); - expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw'); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBeUndefined(); const file = readAuthFile(); expect(file.token).toBeUndefined(); - expect(file.proxy).toBeUndefined(); + expect(file.proxy).toEqual({ password: 'pw' }); expect(file.username).toBe('u'); expect(file.secretsBackend).toBe('keyring'); }); From 6891911311e5dfc97cd1f1bb9d023d0c520160a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 16:18:23 +0200 Subject: [PATCH 08/11] wire up proxy.password --- src/lib/credentials.ts | 26 ++++++++++++++++++++++---- src/lib/utils.ts | 19 +++++++++++++++---- test/local/lib/credentials.test.ts | 16 +++++++++------- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 32db4f6b4..30a749e57 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -7,6 +7,7 @@ import { cliDebugPrint } from './utils/cliDebugPrint.js'; const KEYRING_SERVICE = 'com.apify.cli'; const TOKEN_ACCOUNT = 'token'; +const PROXY_PASSWORD_ACCOUNT = 'proxy-password'; const PROBE_ACCOUNT = '__probe__'; export type CredentialsBackend = 'keyring' | 'file'; @@ -153,6 +154,8 @@ export async function getToken(): Promise { } export async function getProxyPassword(): Promise { + const backend = await getBackend(); + if (backend === 'keyring') return readKeyring(PROXY_PASSWORD_ACCOUNT); return readAuthFile().proxy?.password; } @@ -179,10 +182,20 @@ export async function setToken(token: string, opts: { skipIfUnchanged?: boolean } export async function setProxyPassword(password: string, opts: { skipIfUnchanged?: boolean } = {}): Promise { - if (opts.skipIfUnchanged && readAuthFile().proxy?.password === password) return; + const backend = await getBackend(); + if (opts.skipIfUnchanged) { + const existing = backend === 'keyring' ? await readKeyring(PROXY_PASSWORD_ACCOUNT) : readAuthFile().proxy?.password; + if (existing === password) return; + } + + if (backend === 'keyring') { + await writeKeyring(PROXY_PASSWORD_ACCOUNT, password); + return; + } const data = readAuthFile(); data.proxy = { password }; + data.secretsBackend = 'file'; writeAuthFile(data); } @@ -193,16 +206,17 @@ export async function setProxyPassword(password: string, opts: { skipIfUnchanged */ export async function clearSecrets(): Promise { await deleteKeyring(TOKEN_ACCOUNT); + await deleteKeyring(PROXY_PASSWORD_ACCOUNT); } /** * One-shot, idempotent migration of legacy plaintext auth.json to the keyring. * - * Only the API token is moved into the keyring; proxy password stays in auth.json. + * Both the API token and the proxy password are moved into the keyring on the keyring backend. * * - `secretsBackend` marker in auth.json makes re-entry a no-op. - * - On `file` backend the marker is written but the token stays in auth.json. - * - On `keyring` backend the token is moved out of auth.json. + * - On `file` backend the marker is written but secrets stay in auth.json. + * - On `keyring` backend the token and proxy password are moved out of auth.json. * - Wrapped in try/catch so a migration failure never blocks the CLI. */ export async function ensureMigrated(): Promise { @@ -222,6 +236,10 @@ export async function ensureMigrated(): Promise { await writeKeyring(TOKEN_ACCOUNT, file.token); delete file.token; + if (file.proxy?.password) { + await writeKeyring(PROXY_PASSWORD_ACCOUNT, file.proxy.password); + delete file.proxy; + } file.secretsBackend = 'keyring'; writeAuthFile(file); } catch (err) { diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 352b85055..25f8edf84 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -44,7 +44,7 @@ import { MINIMUM_SUPPORTED_PYTHON_VERSION, SUPPORTED_NODEJS_VERSION, } from './consts.js'; -import { ensureMigrated, getBackend, getToken, setToken } from './credentials.js'; +import { ensureMigrated, getBackend, getProxyPassword, getToken, setProxyPassword, setToken } from './credentials.js'; import { deleteFile, ensureFolderExistsSync, rimrafPromised } from './files.js'; import { inputFileRegExp, TEMP_INPUT_KEY_PREFIX } from './input-key.js'; import type { AuthJSON } from './types.js'; @@ -101,8 +101,8 @@ export const getLocalRequestQueuePath = (storeId?: string) => { }; /** - * Returns object from auth file or empty object. The token is pulled from the keyring - * when that backend is active; all other fields (including proxy) live in auth.json. + * Returns object from auth file or empty object. Secrets (token, proxy password) are + * pulled from the keyring when that backend is active; user metadata lives in auth.json. */ export const getLocalUserInfo = async (): Promise => { await ensureMigrated(); @@ -118,6 +118,9 @@ export const getLocalUserInfo = async (): Promise => { const token = await getToken(); if (token) result.token = token; + const proxyPassword = await getProxyPassword(); + if (proxyPassword) result.proxy = { password: proxyPassword }; + const hasSomething = result.username || result.id || result.token; if (!hasSomething) return {}; @@ -195,6 +198,11 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { await setToken(apifyClient.token, { skipIfUnchanged: true }); } + const proxyPassword = (userInfo as { proxy?: { password?: string } } | undefined)?.proxy?.password; + if (proxyPassword) { + await setProxyPassword(proxyPassword, { skipIfUnchanged: true }); + } + ensureApifyDirectory(AUTH_FILE_PATH()); const existingFile = (() => { try { @@ -205,7 +213,10 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { })(); const backend = await getBackend(); const fileContents: Record = { ...existingFile, ...userInfo, secretsBackend: backend }; - if (backend === 'keyring') delete fileContents.token; + if (backend === 'keyring') { + delete fileContents.token; + delete fileContents.proxy; + } writeFileSync(AUTH_FILE_PATH(), JSON.stringify(fileContents, null, '\t')); return apifyClient; diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts index 2f8fb3b8f..6a9d7c0d1 100644 --- a/test/local/lib/credentials.test.ts +++ b/test/local/lib/credentials.test.ts @@ -121,17 +121,19 @@ describe('credentials', () => { expect(existsSync(AUTH_FILE_PATH())).toBe(false); }); - it('proxy password always lives in auth.json, not the keyring', async () => { + it('round-trips the proxy password through the keyring and keeps it out of auth.json', async () => { await setProxyPassword('pw_abc'); expect(await getProxyPassword()).toBe('pw_abc'); - expect(readAuthFile().proxy).toEqual({ password: 'pw_abc' }); - expect(keyringStore.get('com.apify.cli:proxy-password')).toBeUndefined(); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw_abc'); + expect(existsSync(AUTH_FILE_PATH())).toBe(false); }); - it('clearSecrets() removes the token entry from the keyring', async () => { + it('clearSecrets() removes the token and proxy entries from the keyring', async () => { await setToken('tok_123'); + await setProxyPassword('pw_abc'); await clearSecrets(); expect(await getToken()).toBeUndefined(); + expect(await getProxyPassword()).toBeUndefined(); }); }); @@ -174,15 +176,15 @@ describe('credentials', () => { expect(file.secretsBackend).toBe('file'); }); - it('on the keyring backend, moves the token out of auth.json but leaves proxy in place', async () => { + it('on the keyring backend, moves the token and proxy password out of auth.json', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); writeAuthFile({ token: 'tok', proxy: { password: 'pw' }, username: 'u' }); await ensureMigrated(); expect(keyringStore.get('com.apify.cli:token')).toBe('tok'); - expect(keyringStore.get('com.apify.cli:proxy-password')).toBeUndefined(); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw'); const file = readAuthFile(); expect(file.token).toBeUndefined(); - expect(file.proxy).toEqual({ password: 'pw' }); + expect(file.proxy).toBeUndefined(); expect(file.username).toBe('u'); expect(file.secretsBackend).toBe('keyring'); }); From 79a983a780c625d66a4f3e1eb5593f2fe64c2a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Wed, 20 May 2026 16:52:29 +0200 Subject: [PATCH 09/11] Align with mcpc --- src/lib/credentials.ts | 62 +++++++++++++++++++++--------------------- src/lib/utils.ts | 2 +- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 30a749e57..df95be6f6 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -30,13 +30,13 @@ interface StoredAuthFile { } let cachedKeyringModule: KeyringModule | null | undefined; -let cachedBackend: CredentialsBackend | undefined; +let backendPromise: Promise | undefined; let migrationPromise: Promise | undefined; /** Test-only: clear cached module/backend/migration so each test starts fresh. */ export function __resetCredentialsForTests() { cachedKeyringModule = undefined; - cachedBackend = undefined; + backendPromise = undefined; migrationPromise = undefined; } @@ -53,11 +53,20 @@ async function loadKeyringModule(): Promise { return cachedKeyringModule; } +/** + * Full write/read-back/delete round-trip on a unique random account. + * A read-only probe would pass on Secret Service backends that reject writes + * (e.g. read-only sessions), giving us a false positive at login time. + */ function probeKeyring(mod: KeyringModule): boolean { + const probeAccount = `${PROBE_ACCOUNT}_${Date.now()}_${Math.random().toString(36).slice(2)}`; + const probeValue = `probe-${Date.now()}`; try { - const entry = new mod.Entry(KEYRING_SERVICE, PROBE_ACCOUNT); - entry.getPassword(); - return true; + const entry = new mod.Entry(KEYRING_SERVICE, probeAccount); + entry.setPassword(probeValue); + const readBack = entry.getPassword(); + entry.deletePassword(); + return readBack === probeValue; } catch (err) { cliDebugPrint('credentials', 'keyring probe failed', err); return false; @@ -66,35 +75,26 @@ function probeKeyring(mod: KeyringModule): boolean { /** * Picks a backend the first time it's called and caches the result for the rest of the process. - * Order: APIFY_DISABLE_KEYRING env override -> persisted marker in auth.json -> module load -> read-only probe. + * Single-flight via a promise so concurrent callers share the same probe. + * Order: APIFY_DISABLE_KEYRING env override -> persisted marker in auth.json -> module load -> probe. */ export async function getBackend(): Promise { - if (cachedBackend) return cachedBackend; - - if (process.env.APIFY_DISABLE_KEYRING === '1') { - cachedBackend = 'file'; - return cachedBackend; - } + if (backendPromise) return backendPromise; + backendPromise = (async (): Promise => { + if (process.env.APIFY_DISABLE_KEYRING === '1') return 'file'; + + const marker = readAuthFile().secretsBackend; + if (marker === 'file') return 'file'; + if (marker === 'keyring') { + const mod = await loadKeyringModule(); + return mod ? 'keyring' : 'file'; + } - const marker = readAuthFile().secretsBackend; - if (marker === 'file') { - cachedBackend = 'file'; - return cachedBackend; - } - if (marker === 'keyring') { const mod = await loadKeyringModule(); - cachedBackend = mod ? 'keyring' : 'file'; - return cachedBackend; - } - - const mod = await loadKeyringModule(); - if (!mod) { - cachedBackend = 'file'; - return cachedBackend; - } - - cachedBackend = probeKeyring(mod) ? 'keyring' : 'file'; - return cachedBackend; + if (!mod) return 'file'; + return probeKeyring(mod) ? 'keyring' : 'file'; + })(); + return backendPromise; } function readAuthFile(): StoredAuthFile { @@ -109,7 +109,7 @@ function readAuthFile(): StoredAuthFile { function writeAuthFile(data: StoredAuthFile) { ensureApifyDirectory(AUTH_FILE_PATH()); - writeFileSync(AUTH_FILE_PATH(), JSON.stringify(data, null, '\t')); + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(data, null, '\t'), { mode: 0o600 }); } async function getKeyringEntry(account: string): Promise { diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 25f8edf84..06f35ea4e 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -217,7 +217,7 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { delete fileContents.token; delete fileContents.proxy; } - writeFileSync(AUTH_FILE_PATH(), JSON.stringify(fileContents, null, '\t')); + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(fileContents, null, '\t'), { mode: 0o600 }); return apifyClient; } From 385451fe6061f1d8a4d27ed891a5f35351ef8116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Thu, 28 May 2026 12:31:18 +0000 Subject: [PATCH 10/11] refactor(credentials): break import cycle and defer keyring probe - Move `ensureApifyDirectory` from utils.ts to files.ts to break the credentials.ts <-> utils.ts circular import. - Drop the synthetic write/read/delete probe at backend selection time and treat the first real keyring write as the probe. On failure, downgrade to the file backend and persist the marker so future runs skip the keyring entirely. Avoids a duplicate macOS Keychain prompt on first login. - Reword the "Corrupted local user info" error to accurately describe the stale-keyring-without-metadata state. --- src/lib/credentials.ts | 70 +++++++++++++++++++++++------------------- src/lib/files.ts | 11 ++++++- src/lib/secrets.ts | 2 +- src/lib/utils.ts | 15 ++------- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index df95be6f6..5b3799b43 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -2,13 +2,12 @@ import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import process from 'node:process'; import { AUTH_FILE_PATH } from './consts.js'; -import { ensureApifyDirectory } from './utils.js'; +import { ensureApifyDirectory } from './files.js'; import { cliDebugPrint } from './utils/cliDebugPrint.js'; const KEYRING_SERVICE = 'com.apify.cli'; const TOKEN_ACCOUNT = 'token'; const PROXY_PASSWORD_ACCOUNT = 'proxy-password'; -const PROBE_ACCOUNT = '__probe__'; export type CredentialsBackend = 'keyring' | 'file'; @@ -53,30 +52,14 @@ async function loadKeyringModule(): Promise { return cachedKeyringModule; } -/** - * Full write/read-back/delete round-trip on a unique random account. - * A read-only probe would pass on Secret Service backends that reject writes - * (e.g. read-only sessions), giving us a false positive at login time. - */ -function probeKeyring(mod: KeyringModule): boolean { - const probeAccount = `${PROBE_ACCOUNT}_${Date.now()}_${Math.random().toString(36).slice(2)}`; - const probeValue = `probe-${Date.now()}`; - try { - const entry = new mod.Entry(KEYRING_SERVICE, probeAccount); - entry.setPassword(probeValue); - const readBack = entry.getPassword(); - entry.deletePassword(); - return readBack === probeValue; - } catch (err) { - cliDebugPrint('credentials', 'keyring probe failed', err); - return false; - } -} - /** * Picks a backend the first time it's called and caches the result for the rest of the process. - * Single-flight via a promise so concurrent callers share the same probe. - * Order: APIFY_DISABLE_KEYRING env override -> persisted marker in auth.json -> module load -> probe. + * Single-flight via a promise so concurrent callers share the same lookup. + * Order: APIFY_DISABLE_KEYRING env override -> persisted marker in auth.json -> module load. + * + * No write-probe runs here: on macOS that would pop a keychain prompt before the user has + * authorized one. The first real write is the probe — failure is caught and downgraded + * via `downgradeBackendToFile()`, persisting the file marker so future runs skip the keyring. */ export async function getBackend(): Promise { if (backendPromise) return backendPromise; @@ -91,12 +74,19 @@ export async function getBackend(): Promise { } const mod = await loadKeyringModule(); - if (!mod) return 'file'; - return probeKeyring(mod) ? 'keyring' : 'file'; + return mod ? 'keyring' : 'file'; })(); return backendPromise; } +/** + * Called when a keyring write fails at runtime. Flips the cached backend so subsequent + * reads/writes use the file path immediately, without waiting for the marker on disk. + */ +function downgradeBackendToFile() { + backendPromise = Promise.resolve('file'); +} + function readAuthFile(): StoredAuthFile { if (!existsSync(AUTH_FILE_PATH())) return {}; try { @@ -171,8 +161,13 @@ export async function setToken(token: string, opts: { skipIfUnchanged?: boolean } if (backend === 'keyring') { - await writeKeyring(TOKEN_ACCOUNT, token); - return; + try { + await writeKeyring(TOKEN_ACCOUNT, token); + return; + } catch (err) { + cliDebugPrint('credentials', 'keyring write failed; falling back to file', err); + downgradeBackendToFile(); + } } const data = readAuthFile(); @@ -189,8 +184,13 @@ export async function setProxyPassword(password: string, opts: { skipIfUnchanged } if (backend === 'keyring') { - await writeKeyring(PROXY_PASSWORD_ACCOUNT, password); - return; + try { + await writeKeyring(PROXY_PASSWORD_ACCOUNT, password); + return; + } catch (err) { + cliDebugPrint('credentials', 'keyring write failed; falling back to file', err); + downgradeBackendToFile(); + } } const data = readAuthFile(); @@ -234,7 +234,15 @@ export async function ensureMigrated(): Promise { return; } - await writeKeyring(TOKEN_ACCOUNT, file.token); + try { + await writeKeyring(TOKEN_ACCOUNT, file.token); + } catch (err) { + cliDebugPrint('credentials', 'keyring write failed during migration; falling back to file', err); + downgradeBackendToFile(); + file.secretsBackend = 'file'; + writeAuthFile(file); + return; + } delete file.token; if (file.proxy?.password) { await writeKeyring(PROXY_PASSWORD_ACCOUNT, file.proxy.password); diff --git a/src/lib/files.ts b/src/lib/files.ts index e1a565b45..ff7f975ae 100644 --- a/src/lib/files.ts +++ b/src/lib/files.ts @@ -1,6 +1,6 @@ import { existsSync, mkdirSync } from 'node:fs'; import { readFile, stat, unlink, writeFile } from 'node:fs/promises'; -import { join, sep } from 'node:path'; +import { dirname, join, sep } from 'node:path'; import { rimraf } from 'rimraf'; @@ -61,6 +61,15 @@ export const deleteFile = async (filePath: string) => { } }; +/** + * Ensures the Apify directory exists, as well as nested folders (for tests) + */ +export function ensureApifyDirectory(file: string) { + const path = dirname(file); + + mkdirSync(path, { recursive: true }); +} + export const sumFilesSizeInBytes = async (pathToFiles: string[], cwd: string) => { const filesStats = await Promise.all(pathToFiles.map(async (filePath) => stat(join(cwd, filePath)))); diff --git a/src/lib/secrets.ts b/src/lib/secrets.ts index ed1a35fe6..6d3f2654f 100644 --- a/src/lib/secrets.ts +++ b/src/lib/secrets.ts @@ -1,8 +1,8 @@ import { readFileSync, writeFileSync } from 'node:fs'; import { SECRETS_FILE_PATH } from './consts.js'; +import { ensureApifyDirectory } from './files.js'; import { warning } from './outputs.js'; -import { ensureApifyDirectory } from './utils.js'; const SECRET_KEY_PREFIX = '@'; // TODO: Moved to shared diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 06f35ea4e..df4cd1020 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -1,5 +1,5 @@ import { execSync } from 'node:child_process'; -import { createWriteStream, existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { createWriteStream, existsSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { mkdir, readFile } from 'node:fs/promises'; import type { IncomingMessage } from 'node:http'; import { get } from 'node:https'; @@ -45,7 +45,7 @@ import { SUPPORTED_NODEJS_VERSION, } from './consts.js'; import { ensureMigrated, getBackend, getProxyPassword, getToken, setProxyPassword, setToken } from './credentials.js'; -import { deleteFile, ensureFolderExistsSync, rimrafPromised } from './files.js'; +import { deleteFile, ensureApifyDirectory, ensureFolderExistsSync, rimrafPromised } from './files.js'; import { inputFileRegExp, TEMP_INPUT_KEY_PREFIX } from './input-key.js'; import type { AuthJSON } from './types.js'; import { cliDebugPrint } from './utils/cliDebugPrint.js'; @@ -125,7 +125,7 @@ export const getLocalUserInfo = async (): Promise => { if (!hasSomething) return {}; if (!result.username && !result.id) { - throw new Error('Corrupted local user info was found. Please run "apify login" to fix it.'); + throw new Error('Stale credentials found without user metadata. Please run "apify login" again.'); } return result; @@ -716,15 +716,6 @@ export const downloadAndUnzip = async ({ url, pathTo }: { url: string; pathTo: s zip.extractAllTo(pathTo, true); }; -/** - * Ensures the Apify directory exists, as well as nested folders (for tests) - */ -export function ensureApifyDirectory(file: string) { - const path = dirname(file); - - mkdirSync(path, { recursive: true }); -} - export const TimestampFormatter = new Timestamp('YYYY-MM-DD [at] HH:mm:ss'); export const MultilineTimestampFormatter = new Timestamp(`YYYY-MM-DD[\n]HH:mm:ss`); From e3af68b9eb039fd1741c7bee22fed36c9c5b560e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20Sol=C3=A1r?= Date: Thu, 28 May 2026 12:59:10 +0000 Subject: [PATCH 11/11] fix(tests): make runs abort e2e test resilient to fast-finishing actor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared runId in the runs lifecycle test points at a hello-world actor that finishes in well under a second. After the resurrect step, the abort CLI's startup overhead (1–3s) meant the resurrected run had typically already SUCCEEDED by the time abort fired its API call, causing abort to print `Error: Run with ID "..." is already aborted.` to stdout with exit code 0 — which then broke `JSON.parse` in the test. Start a dedicated abort target run with a 30s `sleepMs` input so the abort always wins the race. Teach the basic test actor to honor the new `sleepMs` input. --- test/e2e/__fixtures__/basic-actor.js | 4 ++++ test/e2e/commands/runs/lifecycle.test.ts | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/test/e2e/__fixtures__/basic-actor.js b/test/e2e/__fixtures__/basic-actor.js index 69c121495..c76cc74b6 100644 --- a/test/e2e/__fixtures__/basic-actor.js +++ b/test/e2e/__fixtures__/basic-actor.js @@ -12,4 +12,8 @@ if (input) { await Actor.setValue('RECEIVED_INPUT', input); } +if (input && typeof input.sleepMs === 'number') { + await new Promise((resolve) => setTimeout(resolve, input.sleepMs)); +} + await Actor.exit(); diff --git a/test/e2e/commands/runs/lifecycle.test.ts b/test/e2e/commands/runs/lifecycle.test.ts index da294a83f..bbe80a439 100644 --- a/test/e2e/commands/runs/lifecycle.test.ts +++ b/test/e2e/commands/runs/lifecycle.test.ts @@ -115,13 +115,23 @@ describe('[e2e][api] runs lifecycle', () => { }); it('runs abort — aborts a running run', async () => { - const result = await runCli('apify', ['runs', 'abort', runId, '--json'], { + // Start a dedicated long-running run so abort always wins the race against + // the hello-world actor finishing on its own. + const startResult = await runCli( + 'apify', + ['actors', 'start', actorFullName, '--input', JSON.stringify({ sleepMs: 30_000 }), '--json'], + { cwd: actor.dir, env: authEnv }, + ); + expect(startResult.exitCode, `stderr: ${startResult.stderr}`).toBe(0); + const { id: abortRunId } = JSON.parse(startResult.stdout); + + const result = await runCli('apify', ['runs', 'abort', abortRunId, '--json'], { env: authEnv, }); expect(result.exitCode, `stderr: ${result.stderr}`).toBe(0); const data = JSON.parse(result.stdout); - expect(data.id).toBe(runId); + expect(data.id).toBe(abortRunId); }); it('runs rm — deletes a run', async () => {