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 1/9] 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 2/9] 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 3/9] 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 4/9] 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 5/9] 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 6/9] 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 7/9] 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 8/9] 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 9/9] 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; }