Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
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
1 change: 1 addition & 0 deletions src/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ export async function getLoggedClientOrThrow() {

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
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 @@ describe('apify run', () => {
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);
expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).not.toStrictEqual(auth.token);
});

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

const keyringStore = new Map<string, string>();
const keyringFailures = new Set<string>();
Expand Down Expand Up @@ -278,4 +278,30 @@ describe('credentials', () => {
expect(info.proxy?.password).toBe('pw_kr');
});
});

// 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');
});
});
});