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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/commands/actor/charge.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { APIFY_ENV_VARS } from '@apify/consts';

import { getApifyTokenFromEnvOrAuthFile } from '../../lib/actor.js';
import { getApifyToken } from '../../lib/actor.js';
import { ApifyCommand } from '../../lib/command-framework/apify-command.js';
import { Args } from '../../lib/command-framework/args.js';
import { Flags } from '../../lib/command-framework/flags.js';
Expand Down Expand Up @@ -86,7 +86,7 @@ export class ActorChargeCommand extends ApifyCommand<typeof ActorChargeCommand>
return;
}

const apifyToken = await getApifyTokenFromEnvOrAuthFile();
const apifyToken = await getApifyToken();
const apifyClient = await getLoggedClient(apifyToken);
if (!apifyClient) {
throw new Error('Apify token is not set. Please set it using the environment variable APIFY_TOKEN.');
Expand Down
2 changes: 1 addition & 1 deletion src/commands/auth/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const API_VERSION = 'v1';

const tryToLogin = async (token: string) => {
const apiBaseUrl = getConsoleUrl().includes('localhost') ? LOCAL_API_BASE_URL : undefined;
const isUserLogged = await getLoggedClient(token, apiBaseUrl);
const isUserLogged = await getLoggedClient(token, apiBaseUrl, { persistCredentials: true });
const userInfo = await getLocalUserInfo();

if (isUserLogged) {
Expand Down
11 changes: 6 additions & 5 deletions src/commands/auth/token.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ApifyCommand } from '../../lib/command-framework/apify-command.js';
import { simpleLog } from '../../lib/outputs.js';
import { getLocalUserInfo, getLoggedClientOrThrow } from '../../lib/utils.js';
import { getLoggedClientOrThrow, resolveToken } from '../../lib/utils.js';

export class AuthTokenCommand extends ApifyCommand<typeof AuthTokenCommand> {
static override name = 'token' as const;
Expand All @@ -9,7 +9,7 @@ export class AuthTokenCommand extends ApifyCommand<typeof AuthTokenCommand> {

static override examples = [
{
description: 'Print the stored API token to stdout (use with care — it is a secret).',
description: 'Print the API token in use to stdout (use with care — it is a secret).',
command: 'apify auth token',
},
];
Expand All @@ -18,10 +18,11 @@ export class AuthTokenCommand extends ApifyCommand<typeof AuthTokenCommand> {

async run() {
await getLoggedClientOrThrow();
const userInfo = await getLocalUserInfo();
// Must match what the other commands actually authenticate with, so APIFY_TOKEN wins over the stored login.
const token = await resolveToken();

if (userInfo.token) {
simpleLog({ message: userInfo.token, stdout: true });
if (token) {
simpleLog({ message: token, stdout: true });
}
}
}
3 changes: 2 additions & 1 deletion src/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,8 @@ export class RunCommand extends ApifyCommand<typeof RunCommand> {

if (proxy && proxy.password) localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD] = proxy.password;
if (userId) localEnvVars[APIFY_ENV_VARS.USER_ID] = userId;
if (token) localEnvVars[APIFY_ENV_VARS.TOKEN] = token;
// Don't clobber an explicitly-set APIFY_TOKEN inherited from the environment — it must win over the stored login.
if (token && !process.env[APIFY_ENV_VARS.TOKEN]) localEnvVars[APIFY_ENV_VARS.TOKEN] = token;
if (localConfig!.environmentVariables) {
const updatedEnv = replaceSecretsValue(localConfig!.environmentVariables as Record<string, string>, undefined, {
allowMissing: this.flags.allowMissingSecrets,
Expand Down
19 changes: 7 additions & 12 deletions src/lib/actor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ApifyClient } from 'apify-client';

import { ACTOR_ENV_VARS, APIFY_ENV_VARS, KEY_VALUE_STORE_KEYS, LOCAL_ACTOR_ENV_VARS } from '@apify/consts';

import { getApifyClientOptions, getLocalStorageDir, getLocalUserInfo } from './utils.js';
import { getApifyClientOptions, getLocalStorageDir, resolveToken } from './utils.js';

export const APIFY_STORAGE_TYPES = {
KEY_VALUE_STORE: 'KEY_VALUE_STORE',
Expand All @@ -18,23 +18,18 @@ export const APIFY_STORAGE_TYPES = {
} as const;

/**
* Returns Apify token from environment variable or local auth file.
* Returns the Apify token to use — the `APIFY_TOKEN` env var, else the token stored by `apify login`.
* @returns Apify token
*/
export const getApifyTokenFromEnvOrAuthFile = async () => {
const apifyToken = process.env[APIFY_ENV_VARS.TOKEN];
if (apifyToken) {
return apifyToken;
}

const localUserInfo = await getLocalUserInfo();
if (!localUserInfo || !localUserInfo.token) {
export const getApifyToken = async () => {
const apifyToken = await resolveToken();
if (!apifyToken) {
throw new Error(
'Apify token is not set. Please set it using the environment variable APIFY_TOKEN or apify login command.',
);
}

return localUserInfo.token;
return apifyToken;
};

/**
Expand All @@ -54,7 +49,7 @@ export const getApifyStorageClient = async (
...options,
});
}
const apifyToken = await getApifyTokenFromEnvOrAuthFile();
const apifyToken = await getApifyToken();

return new ApifyClient({
...(await getApifyClientOptions(apifyToken)),
Expand Down
67 changes: 60 additions & 7 deletions src/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,19 @@ 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; user metadata lives in auth.json.
*
* auth.json only ever describes the account `apify login` stored, and `APIFY_TOKEN` exists to act
* as a different one (typically an organization), so the identity is read from the API whenever it
* is set.
*/
export const getLocalUserInfo = async (): Promise<AuthJSON> => {
await ensureMigrated();

const overrideToken = process.env[APIFY_ENV_VARS.TOKEN];
if (overrideToken) {
return { ...(await fetchActiveUserInfo(overrideToken)), token: overrideToken };
}

let result: AuthJSON = {};
try {
const raw = await readFile(AUTH_FILE_PATH(), 'utf-8');
Expand All @@ -98,10 +107,10 @@ export const getLocalUserInfo = async (): Promise<AuthJSON> => {
// auth.json may not exist yet (fresh keyring-only state); fall through
}

if ((await getBackend()) === 'keyring') {
const token = await getToken();
if (token) result.token = token;
const storedToken = await getToken();
if (storedToken) result.token = storedToken;

if ((await getBackend()) === 'keyring') {
const proxyPassword = await getProxyPassword();
if (proxyPassword) result.proxy = { ...result.proxy, password: proxyPassword };
}
Expand Down Expand Up @@ -129,8 +138,16 @@ export async function getLoggedClientOrThrow() {
return loggedClient;
}

const resolveToken = async (existingToken?: string): Promise<string | undefined> => {
/**
* Resolves the token to use, in order: explicitly passed token (e.g. `--token`) >
* `APIFY_TOKEN` env var > the token stored by `apify login`.
*
* The first two are one-time overrides and must never be written over the stored login —
* see the `persistCredentials` option of {@link getLoggedClient}.
*/
export const resolveToken = async (existingToken?: string): Promise<string | undefined> => {
if (existingToken) return existingToken;
if (process.env[APIFY_ENV_VARS.TOKEN]) return process.env[APIFY_ENV_VARS.TOKEN];

@DaveHanns DaveHanns Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APIFY_TOKEN now silently overwrites the stored apify login credentials

The read-side precedence added here is correct, but this line has a downstream side effect that I think makes it a blocker.

Because resolveToken() now returns process.env.APIFY_TOKEN, getLoggedClient(), reached by ~all authenticated commands via getLoggedClientOrThrow(), then persists it:

// src/lib/utils.ts, getLoggedClient()
const resolvedToken = await resolveToken(token);          // ← now the APIFY_TOKEN value

if (apifyClient.token) {
    await setToken(apifyClient.token, { skipIfUnchanged: true });   // ← writes it to keyring/auth.json
}

writeFileSync(AUTH_FILE_PATH(), JSON.stringify({ ...existingFile, ...userInfo }));  // ← rewrites username/id too

setToken(…, { skipIfUnchanged: true }) writes whenever the value differs from what's stored. Pre-PR, resolvedToken was always the stored token, so this was a no-op. Now:

apify login --token <A>   # stored login = account A
export APIFY_TOKEN=<B>    # a different, valid token
apify actors ls           # reads B (correct) — but setToken overwrites stored A with B,
                          #   and auth.json username/id are rewritten to account B
unset APIFY_TOKEN
apify actors ls           # resolveToken → getToken() → returns B  ← login A is gone

So a transient env var permanently mutates the durable login: the apify login account is silently replaced, and the change persists after unset. Two consequences:

  1. The env and stored tiers stop being independent — using APIFY_TOKEN destroys the stored login. This also contradicts the intent of the new run.ts guard's own comment ("must win over the stored login", i.e. without replacing it).
  2. It re-triggers the macOS Keychain write prompt that skipIfUnchanged was added to avoid — every command run with a differing APIFY_TOKEN rewrites the keyring.

Suggested fix: in getLoggedClient, only call setToken / setProxyPassword when the token came from an explicit apify login, not when it originated from APIFY_TOKEN or --token flag of command other than apify login (the --token on other commands should be one-time overwrite as well), mirroring the guard already added in run.ts:332.

(Heads-up: if you make this change, apify auth token will then print the stored token while other commands use the env token — it reads getLocalUserInfo().token directly and only looks correct today because of this overwrite. It'd need to become env-aware too.)

🤖 Generated with Claude Code

@l2ysho l2ysho Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you, this is a good catch and I am wondering how I missed this when I self reviewed 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I am inspecting getLoggedClient() and there are few things I really do not like (for example it is doing mutations but name suggests it is only get), I will create a follow up issue if I find it is worth of it.

await ensureMigrated();
return getToken();
};
Expand Down Expand Up @@ -160,13 +177,41 @@ export const getApifyClientOptions = async (token?: string, apiBaseUrl?: string)
};
};

/**
* User info of the account the process authenticates as, keyed by the token it was fetched with.
* Seeded by {@link getLoggedClient} from the user info it fetches anyway, so commands that get a
* client first (nearly all of them) pay no extra API call.
*/
let activeUserInfo: { token: string; info: AuthJSON } | undefined;

async function fetchActiveUserInfo(token: string): Promise<AuthJSON> {
if (activeUserInfo?.token !== token) {
const client = new ApifyClient(await getApifyClientOptions(token));
try {
activeUserInfo = { token, info: await client.user('me').get() };
} catch (err) {
cliDebugPrint('[fetchActiveUserInfo] error getting user info', { error: err });
throw new Error(`The token in ${APIFY_ENV_VARS.TOKEN} was rejected by the Apify API. Is it still valid?`);
}
}

return activeUserInfo.info;
}

/**
* Gets instance of ApifyClient for token or for params from global auth file.
*
* 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.
* Secrets (token, proxy.password) and user metadata are only written when `persistCredentials`
* is set — i.e. from `apify login`. Every other caller resolves a possibly-overridden token
* (`--token`, `APIFY_TOKEN`), and persisting that would silently replace the stored login.
* When writing, secrets only change on disk if their value differs — avoids macOS Keychain
* prompts on every command.
*/
export async function getLoggedClient(token?: string, apiBaseUrl?: string) {
export async function getLoggedClient(
token?: string,
apiBaseUrl?: string,
{ persistCredentials = false }: { persistCredentials?: boolean } = {},
) {
const resolvedToken = await resolveToken(token);

const apifyClient = new ApifyClient(await getApifyClientOptions(resolvedToken, apiBaseUrl));
Expand All @@ -179,6 +224,14 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) {
return null;
}

if (apifyClient.token) {
activeUserInfo = { token: apifyClient.token, info: userInfo };
}

if (!persistCredentials) {
return apifyClient;
}

if (apifyClient.token) {
await setToken(apifyClient.token, { skipIfUnchanged: true });
}
Expand Down
27 changes: 27 additions & 0 deletions test/local/commands/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,33 @@
expect(actOutput2).toStrictEqual('can play');
});

// Regression: an inherited APIFY_TOKEN must reach the Actor unchanged, not be clobbered by the stored login token.
it(`[api] does not override an inherited ${APIFY_ENV_VARS.TOKEN} with the stored token`, async () => {
await safeLogin();

const auth = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8'));
const inheritedToken = `${auth.token}_inherited`;
vitest.stubEnv(APIFY_ENV_VARS.TOKEN, inheritedToken);

const actCode = `
import { Actor } from 'apify';

Actor.main(async () => {
await Actor.setValue('OUTPUT', process.env);
console.log('Done.');
});
`;
writeFileSync(joinPath('src/main.js'), actCode, { flag: 'w' });

await testRunCommand(RunCommand, {});

const actOutputPath = joinPath(getLocalKeyValueStorePath(), 'OUTPUT.json');
const localEnvVars = JSON.parse(readFileSync(actOutputPath, 'utf8'));

expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(inheritedToken);

Check failure on line 242 in test/local/commands/run.test.ts

View workflow job for this annotation

GitHub Actions / API Tests

test/local/commands/run.test.ts > apify run > [api] does not override an inherited APIFY_TOKEN with the stored token

AssertionError: expected 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' to strictly equal 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' Expected: "***_inherited" Received: "***" ❯ test/local/commands/run.test.ts:242:46

Check failure on line 242 in test/local/commands/run.test.ts

View workflow job for this annotation

GitHub Actions / API Tests

test/local/commands/run.test.ts > apify run > [api] does not override an inherited APIFY_TOKEN with the stored token

AssertionError: expected 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' to strictly equal 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' Expected: "***_inherited" Received: "***" ❯ test/local/commands/run.test.ts:242:46

Check failure on line 242 in test/local/commands/run.test.ts

View workflow job for this annotation

GitHub Actions / API Tests

test/local/commands/run.test.ts > apify run > [api] does not override an inherited APIFY_TOKEN with the stored token

AssertionError: expected 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' to strictly equal 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' Expected: "***_inherited" Received: "***" ❯ test/local/commands/run.test.ts:242:46

Check failure on line 242 in test/local/commands/run.test.ts

View workflow job for this annotation

GitHub Actions / API Tests

test/local/commands/run.test.ts > apify run > [api] does not override an inherited APIFY_TOKEN with the stored token

AssertionError: expected 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' to strictly equal 'apify_api_rOJIytgOmpawaFxNKFDJXRYl0Gr…' Expected: "***_inherited" Received: "***" ❯ test/local/commands/run.test.ts:242:46
expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).not.toStrictEqual(auth.token);
});

it('run purge stores', async () => {
const input = {
myInput: 'value',
Expand Down
107 changes: 106 additions & 1 deletion test/local/lib/credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,27 @@ import {
setProxyPassword,
setToken,
} from '../../../src/lib/credentials.js';
import { getLocalUserInfo } from '../../../src/lib/utils.js';
import { getApifyClientOptions, getLocalUserInfo, getLoggedClient } from '../../../src/lib/utils.js';

// Stubs out the `user('me').get()` round-trip so getLoggedClient() can be tested without the API.
vi.mock('apify-client', () => {
class ApifyClient {
token?: string;
constructor(options: { token?: string }) {
this.token = options.token;
}
user() {
return {
get: async () => ({
id: `id_for_${this.token}`,
username: `user_for_${this.token}`,
proxy: { password: `pw_for_${this.token}` },
}),
};
}
}
return { ApifyClient };
});

const keyringStore = new Map<string, string>();
const keyringFailures = new Set<string>();
Expand Down Expand Up @@ -49,6 +69,7 @@ const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8'));
describe('credentials', () => {
beforeEach(() => {
vitest.stubEnv('__APIFY_INTERNAL_TEST_AUTH_PATH__', cryptoRandomObjectId(12));
vitest.stubEnv('APIFY_TOKEN', undefined);
keyringStore.clear();
keyringFailures.clear();
__resetCredentialsForTests();
Expand Down Expand Up @@ -277,5 +298,89 @@ describe('credentials', () => {
expect(info.token).toBe('tok_kr');
expect(info.proxy?.password).toBe('pw_kr');
});

it('describes the APIFY_TOKEN account, not the stored login', async () => {
vitest.stubEnv('APIFY_DISABLE_KEYRING', '1');
writeAuthFile({ username: 'stored', id: 'stored_id', token: 'stored_tok', secretsBackend: 'file' });
vitest.stubEnv('APIFY_TOKEN', 'env_tok_a');

const info = await getLocalUserInfo();
expect(info).toMatchObject({
username: 'user_for_env_tok_a',
id: 'id_for_env_tok_a',
token: 'env_tok_a',
});
expect(info.proxy?.password).toBe('pw_for_env_tok_a');
});

it('describes the APIFY_TOKEN account when there is no stored login at all', async () => {
vitest.stubEnv('APIFY_DISABLE_KEYRING', '1');
vitest.stubEnv('APIFY_TOKEN', 'env_tok_b');

expect(await getLocalUserInfo()).toMatchObject({ username: 'user_for_env_tok_b', id: 'id_for_env_tok_b' });
});
});

// Precedence: explicit token arg (e.g. --token) > APIFY_TOKEN env var > stored login token.
// Regression guard for the env var being ignored in favour of the stored token.
describe('token resolution precedence (getApifyClientOptions)', () => {
it('prefers the APIFY_TOKEN env var over the stored token', async () => {
vitest.stubEnv('APIFY_DISABLE_KEYRING', '1');
await setToken('stored_tok');
vitest.stubEnv('APIFY_TOKEN', 'env_tok');
const { token } = await getApifyClientOptions();
expect(token).toBe('env_tok');
});

it('prefers an explicitly passed token over the APIFY_TOKEN env var', async () => {
vitest.stubEnv('APIFY_TOKEN', 'env_tok');
const { token } = await getApifyClientOptions('explicit_tok');
expect(token).toBe('explicit_tok');
});

it('falls back to the stored token when APIFY_TOKEN is not set', async () => {
vitest.stubEnv('APIFY_DISABLE_KEYRING', '1');
vitest.stubEnv('APIFY_TOKEN', '');
await setToken('stored_tok');
const { token } = await getApifyClientOptions();
expect(token).toBe('stored_tok');
});
});

// A one-time token override (APIFY_TOKEN / --token) must never be written over the durable
// `apify login` credentials — only `apify login` itself persists.
describe('getLoggedClient() credential persistence', () => {
beforeEach(async () => {
vitest.stubEnv('APIFY_DISABLE_KEYRING', '1');
await setToken('stored_tok');
writeAuthFile({ ...readAuthFile(), username: 'stored_user', id: 'stored_id' });
});

it('uses APIFY_TOKEN without overwriting the stored login', async () => {
vitest.stubEnv('APIFY_TOKEN', 'env_tok');

const client = await getLoggedClient();

expect(client?.token).toBe('env_tok');
expect(await getToken()).toBe('stored_tok');
expect(readAuthFile().username).toBe('stored_user');
expect(readAuthFile().id).toBe('stored_id');
});

it('uses an explicitly passed token without overwriting the stored login', async () => {
const client = await getLoggedClient('flag_tok');

expect(client?.token).toBe('flag_tok');
expect(await getToken()).toBe('stored_tok');
expect(readAuthFile().username).toBe('stored_user');
});

it('persists the token and user metadata when persistCredentials is set (apify login)', async () => {
const client = await getLoggedClient('login_tok', undefined, { persistCredentials: true });

expect(client?.token).toBe('login_tok');
expect(await getToken()).toBe('login_tok');
expect(readAuthFile().username).toBe('user_for_login_tok');
});
});
});
Loading