From 0ad09faccc5747a40e170528054ea6dd7dd007a6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Sep 2025 11:03:57 +0000 Subject: [PATCH 1/4] Initial plan From ca45319cb1ed034744be1a0f7372dfc9bfcbd8df Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Sep 2025 11:19:38 +0000 Subject: [PATCH 2/4] Implement error throwing for missing secrets in transformEnvToEnvVars and replaceSecretsValue Co-authored-by: vladfrangu <17960496+vladfrangu@users.noreply.github.com> --- src/lib/secrets.ts | 28 ++++++++--- test/local/lib/secrets.test.ts | 92 +++++++++++++++++++++++++++++++--- 2 files changed, 107 insertions(+), 13 deletions(-) diff --git a/src/lib/secrets.ts b/src/lib/secrets.ts index a373d8470..7e662ca0d 100644 --- a/src/lib/secrets.ts +++ b/src/lib/secrets.ts @@ -58,6 +58,8 @@ const isSecretKey = (envValue: string) => { export const replaceSecretsValue = (env: Record, secrets?: Record) => { secrets = secrets || getSecretsFile(); const updatedEnv = {}; + const missingSecrets: string[] = []; + Object.keys(env).forEach((key) => { if (isSecretKey(env[key])) { const secretKey = env[key].replace(new RegExp(`^${SECRET_KEY_PREFIX}`), ''); @@ -65,15 +67,21 @@ export const replaceSecretsValue = (env: Record, secrets?: Recor // @ts-expect-error - we are replacing the value updatedEnv[key] = secrets[secretKey]; } else { - warning({ - message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`, - }); + missingSecrets.push(secretKey); } } else { // @ts-expect-error - we are replacing the value updatedEnv[key] = env[key]; } }); + + if (missingSecrets.length > 0) { + throw new Error( + `Missing secrets: ${missingSecrets.join(', ')}. ` + + `Set them by calling "apify secrets add ".` + ); + } + return updatedEnv; }; @@ -91,6 +99,8 @@ interface EnvVar { export const transformEnvToEnvVars = (env: Record, secrets?: Record) => { secrets = secrets || getSecretsFile(); const envVars: EnvVar[] = []; + const missingSecrets: string[] = []; + Object.keys(env).forEach((key) => { if (isSecretKey(env[key])) { const secretKey = env[key].replace(new RegExp(`^${SECRET_KEY_PREFIX}`), ''); @@ -101,9 +111,7 @@ export const transformEnvToEnvVars = (env: Record, secrets?: Rec isSecret: true, }); } else { - warning({ - message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`, - }); + missingSecrets.push(secretKey); } } else { envVars.push({ @@ -112,5 +120,13 @@ export const transformEnvToEnvVars = (env: Record, secrets?: Rec }); } }); + + if (missingSecrets.length > 0) { + throw new Error( + `Missing secrets: ${missingSecrets.join(', ')}. ` + + `Set them by calling "apify secrets add ".` + ); + } + return envVars; }; diff --git a/test/local/lib/secrets.test.ts b/test/local/lib/secrets.test.ts index cbfea832f..bc9d61f9e 100644 --- a/test/local/lib/secrets.test.ts +++ b/test/local/lib/secrets.test.ts @@ -1,10 +1,8 @@ -import { replaceSecretsValue } from '../../../src/lib/secrets.js'; +import { replaceSecretsValue, transformEnvToEnvVars } from '../../../src/lib/secrets.js'; describe('Secrets', () => { describe('replaceSecretsValue()', () => { - it('should work', () => { - const spy = vitest.spyOn(console, 'error'); - + it('should work with valid secrets', () => { const secrets = { myProdToken: 'mySecretToken', mongoUrl: 'mongo://bla@bla:supermongo.com:27017', @@ -13,7 +11,6 @@ describe('Secrets', () => { TOKEN: '@myProdToken', USER: 'jakub.drobnik@apify.com', MONGO_URL: '@mongoUrl', - WARNING: '@doesNotExist', }; const updatedEnv = replaceSecretsValue(env, secrets); @@ -22,9 +19,90 @@ describe('Secrets', () => { USER: 'jakub.drobnik@apify.com', MONGO_URL: secrets.mongoUrl, }); + }); + + it('should throw error when secret is missing', () => { + const secrets = { + myProdToken: 'mySecretToken', + }; + const env = { + TOKEN: '@myProdToken', + WARNING: '@doesNotExist', + }; + + expect(() => replaceSecretsValue(env, secrets)).toThrow( + 'Missing secrets: doesNotExist. Set them by calling "apify secrets add ".' + ); + }); + + it('should throw error with multiple missing secrets', () => { + const secrets = {}; + const env = { + TOKEN: '@missingSecret1', + API_KEY: '@missingSecret2', + }; + + expect(() => replaceSecretsValue(env, secrets)).toThrow( + 'Missing secrets: missingSecret1, missingSecret2. Set them by calling "apify secrets add ".' + ); + }); + }); + + describe('transformEnvToEnvVars()', () => { + it('should work with valid secrets', () => { + const secrets = { + myProdToken: 'mySecretToken', + mongoUrl: 'mongo://bla@bla:supermongo.com:27017', + }; + const env = { + TOKEN: '@myProdToken', + USER: 'jakub.drobnik@apify.com', + MONGO_URL: '@mongoUrl', + }; + const envVars = transformEnvToEnvVars(env, secrets); + + expect(envVars).toStrictEqual([ + { + name: 'TOKEN', + value: secrets.myProdToken, + isSecret: true, + }, + { + name: 'USER', + value: 'jakub.drobnik@apify.com', + }, + { + name: 'MONGO_URL', + value: secrets.mongoUrl, + isSecret: true, + }, + ]); + }); + + it('should throw error when secret is missing', () => { + const secrets = { + myProdToken: 'mySecretToken', + }; + const env = { + TOKEN: '@myProdToken', + WARNING: '@doesNotExist', + }; + + expect(() => transformEnvToEnvVars(env, secrets)).toThrow( + 'Missing secrets: doesNotExist. Set them by calling "apify secrets add ".' + ); + }); + + it('should throw error with multiple missing secrets', () => { + const secrets = {}; + const env = { + TOKEN: '@missingSecret1', + API_KEY: '@missingSecret2', + }; - expect(spy).toHaveBeenCalled(); - expect(spy.mock.calls[0][0]).to.include('Warning:'); + expect(() => transformEnvToEnvVars(env, secrets)).toThrow( + 'Missing secrets: missingSecret1, missingSecret2. Set them by calling "apify secrets add ".' + ); }); }); }); From 0162b60258797d8a4455df8e7c3304bd0163b652 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Sep 2025 11:25:49 +0000 Subject: [PATCH 3/4] Add comprehensive test coverage for secrets error handling and edge cases Co-authored-by: vladfrangu <17960496+vladfrangu@users.noreply.github.com> --- src/lib/secrets.ts | 1 - test/local/lib/secrets.test.ts | 55 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/lib/secrets.ts b/src/lib/secrets.ts index 7e662ca0d..02885e6a1 100644 --- a/src/lib/secrets.ts +++ b/src/lib/secrets.ts @@ -1,7 +1,6 @@ import { readFileSync, writeFileSync } from 'node:fs'; import { SECRETS_FILE_PATH } from './consts.js'; -import { warning } from './outputs.js'; import { ensureApifyDirectory } from './utils.js'; const SECRET_KEY_PREFIX = '@'; diff --git a/test/local/lib/secrets.test.ts b/test/local/lib/secrets.test.ts index bc9d61f9e..0287ff794 100644 --- a/test/local/lib/secrets.test.ts +++ b/test/local/lib/secrets.test.ts @@ -105,4 +105,59 @@ describe('Secrets', () => { ); }); }); + + describe('Integration scenarios', () => { + it('should handle mixed environment variables correctly', () => { + const secrets = { + validSecret: 'validValue', + }; + const env = { + VALID_SECRET: '@validSecret', + NORMAL_VAR: 'normalValue', + INVALID_SECRET: '@missingSecret', + }; + + // Should fail because of missing secret + expect(() => transformEnvToEnvVars(env, secrets)).toThrow('Missing secrets: missingSecret'); + }); + + it('should work with empty environment variables', () => { + const result = transformEnvToEnvVars({}); + expect(result).toEqual([]); + }); + + it('should work with environment variables that contain no secrets', () => { + const env = { + NORMAL_VAR1: 'value1', + NORMAL_VAR2: 'value2', + }; + const result = transformEnvToEnvVars(env); + + expect(result).toEqual([ + { + name: 'NORMAL_VAR1', + value: 'value1', + }, + { + name: 'NORMAL_VAR2', + value: 'value2', + }, + ]); + }); + + it('should maintain order of environment variables', () => { + const secrets = { + secret1: 'value1', + secret2: 'value2', + }; + const env = { + THIRD: 'third', + FIRST: '@secret1', + SECOND: '@secret2', + }; + const result = transformEnvToEnvVars(env, secrets); + + expect(result.map(r => r.name)).toEqual(['THIRD', 'FIRST', 'SECOND']); + }); + }); }); From 816ca6d26b02f4b2eb5a01cb2531699d1e291ef3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Sep 2025 11:37:48 +0000 Subject: [PATCH 4/4] Add --ignore-missing-secrets flag to push and run commands with comprehensive test coverage Co-authored-by: vladfrangu <17960496+vladfrangu@users.noreply.github.com> --- src/commands/actors/push.ts | 11 +++- src/commands/run.ts | 11 +++- src/lib/secrets.ts | 42 +++++++++++---- test/local/lib/secrets.test.ts | 97 ++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 12 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index f0c2fc781..7cfc67114 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -79,6 +79,11 @@ export class ActorsPushCommand extends ApifyCommand { description: 'Directory where the Actor is located', required: false, }), + 'ignore-missing-secrets': Flags.boolean({ + description: 'Ignore missing secrets and show warnings instead of failing. Environment variables referencing missing secrets will be omitted.', + default: false, + required: false, + }), }; static override args = { @@ -280,7 +285,11 @@ Skipping push. Use --force to override.`, // Update Actor version const actorCurrentVersion = await actorClient.version(version).get(); const envVars = actorConfig!.environmentVariables - ? transformEnvToEnvVars(actorConfig!.environmentVariables as Record) + ? transformEnvToEnvVars( + actorConfig!.environmentVariables as Record, + undefined, + this.flags.ignoreMissingSecrets + ) : undefined; if (actorCurrentVersion) { diff --git a/src/commands/run.ts b/src/commands/run.ts index e72436e02..b9ffad48b 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -98,6 +98,11 @@ export class RunCommand extends ApifyCommand { stdin: StdinMode.Stringified, exclusive: ['input'], }), + 'ignore-missing-secrets': Flags.boolean({ + description: 'Ignore missing secrets and show warnings instead of failing. Environment variables referencing missing secrets will be omitted.', + default: false, + required: false, + }), }; async run() { @@ -259,7 +264,11 @@ export class RunCommand extends ApifyCommand { if (userId) localEnvVars[APIFY_ENV_VARS.USER_ID] = userId; if (token) localEnvVars[APIFY_ENV_VARS.TOKEN] = token; if (localConfig!.environmentVariables) { - const updatedEnv = replaceSecretsValue(localConfig!.environmentVariables as Record); + const updatedEnv = replaceSecretsValue( + localConfig!.environmentVariables as Record, + undefined, + this.flags.ignoreMissingSecrets + ); Object.assign(localEnvVars, updatedEnv); } diff --git a/src/lib/secrets.ts b/src/lib/secrets.ts index 02885e6a1..3fdd1507e 100644 --- a/src/lib/secrets.ts +++ b/src/lib/secrets.ts @@ -1,6 +1,7 @@ import { readFileSync, writeFileSync } from 'node:fs'; import { SECRETS_FILE_PATH } from './consts.js'; +import { warning } from './outputs.js'; import { ensureApifyDirectory } from './utils.js'; const SECRET_KEY_PREFIX = '@'; @@ -53,8 +54,9 @@ const isSecretKey = (envValue: string) => { * Replaces secure values in env with proper values from local secrets file. * @param env * @param secrets - Object with secrets, if not set, will be load from secrets file. + * @param ignoreMissingSecrets - If true, emit warnings for missing secrets instead of throwing errors */ -export const replaceSecretsValue = (env: Record, secrets?: Record) => { +export const replaceSecretsValue = (env: Record, secrets?: Record, ignoreMissingSecrets?: boolean) => { secrets = secrets || getSecretsFile(); const updatedEnv = {}; const missingSecrets: string[] = []; @@ -75,10 +77,19 @@ export const replaceSecretsValue = (env: Record, secrets?: Recor }); if (missingSecrets.length > 0) { - throw new Error( - `Missing secrets: ${missingSecrets.join(', ')}. ` + - `Set them by calling "apify secrets add ".` - ); + if (ignoreMissingSecrets) { + // Emit warnings for each missing secret, keeping original behavior + missingSecrets.forEach(secretKey => { + warning({ + message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`, + }); + }); + } else { + throw new Error( + `Missing secrets: ${missingSecrets.join(', ')}. ` + + `Set them by calling "apify secrets add ".` + ); + } } return updatedEnv; @@ -93,9 +104,11 @@ interface EnvVar { /** * Transform env to envVars format attribute, which uses Apify API * It replaces secrets to values from secrets file. + * @param env * @param secrets - Object with secrets, if not set, will be load from secrets file. + * @param ignoreMissingSecrets - If true, emit warnings for missing secrets instead of throwing errors */ -export const transformEnvToEnvVars = (env: Record, secrets?: Record) => { +export const transformEnvToEnvVars = (env: Record, secrets?: Record, ignoreMissingSecrets?: boolean) => { secrets = secrets || getSecretsFile(); const envVars: EnvVar[] = []; const missingSecrets: string[] = []; @@ -121,10 +134,19 @@ export const transformEnvToEnvVars = (env: Record, secrets?: Rec }); if (missingSecrets.length > 0) { - throw new Error( - `Missing secrets: ${missingSecrets.join(', ')}. ` + - `Set them by calling "apify secrets add ".` - ); + if (ignoreMissingSecrets) { + // Emit warnings for each missing secret, keeping original behavior + missingSecrets.forEach(secretKey => { + warning({ + message: `Value for ${secretKey} not found in local secrets. Set it by calling "apify secrets add ${secretKey} [SECRET_VALUE]"`, + }); + }); + } else { + throw new Error( + `Missing secrets: ${missingSecrets.join(', ')}. ` + + `Set them by calling "apify secrets add ".` + ); + } } return envVars; diff --git a/test/local/lib/secrets.test.ts b/test/local/lib/secrets.test.ts index 0287ff794..625b38339 100644 --- a/test/local/lib/secrets.test.ts +++ b/test/local/lib/secrets.test.ts @@ -160,4 +160,101 @@ describe('Secrets', () => { expect(result.map(r => r.name)).toEqual(['THIRD', 'FIRST', 'SECOND']); }); }); + + describe('ignoreMissingSecrets flag behavior', () => { + it('transformEnvToEnvVars should emit warnings when ignoreMissingSecrets is true', () => { + const spy = vitest.spyOn(console, 'error'); + + const secrets = { + validSecret: 'validValue', + }; + const env = { + VALID_SECRET: '@validSecret', + INVALID_SECRET: '@missingSecret', + NORMAL_VAR: 'normalValue', + }; + + const result = transformEnvToEnvVars(env, secrets, true); + + // Should include valid secret and normal var, but omit missing secret + expect(result).toEqual([ + { + name: 'VALID_SECRET', + value: 'validValue', + isSecret: true, + }, + { + name: 'NORMAL_VAR', + value: 'normalValue', + }, + ]); + + // Should have emitted a warning + expect(spy).toHaveBeenCalled(); + expect(spy.mock.calls[0][0]).to.include('Warning:'); + }); + + it('replaceSecretsValue should emit warnings when ignoreMissingSecrets is true', () => { + const spy = vitest.spyOn(console, 'error'); + + const secrets = { + validSecret: 'validValue', + }; + const env = { + VALID_SECRET: '@validSecret', + INVALID_SECRET: '@missingSecret', + NORMAL_VAR: 'normalValue', + }; + + const result = replaceSecretsValue(env, secrets, true); + + // Should include valid secret and normal var, but omit missing secret + expect(result).toEqual({ + VALID_SECRET: 'validValue', + NORMAL_VAR: 'normalValue', + }); + + // Should have emitted a warning + expect(spy).toHaveBeenCalled(); + expect(spy.mock.calls[0][0]).to.include('Warning:'); + }); + + it('should still throw error when ignoreMissingSecrets is false (default behavior)', () => { + const secrets = {}; + const env = { + INVALID_SECRET: '@missingSecret', + }; + + // Should throw when ignoreMissingSecrets is false (default) + expect(() => transformEnvToEnvVars(env, secrets, false)).toThrow('Missing secrets: missingSecret'); + + // Should also throw when parameter is not provided (default) + expect(() => transformEnvToEnvVars(env, secrets)).toThrow('Missing secrets: missingSecret'); + }); + + it('should handle multiple missing secrets with ignoreMissingSecrets', () => { + const spy = vitest.spyOn(console, 'error'); + + const secrets = {}; + const env = { + SECRET1: '@missing1', + SECRET2: '@missing2', + NORMAL_VAR: 'value', + }; + + const result = transformEnvToEnvVars(env, secrets, true); + + expect(result).toEqual([ + { + name: 'NORMAL_VAR', + value: 'value', + }, + ]); + + // Should have emitted warnings for both missing secrets + expect(spy).toHaveBeenCalledTimes(2); + expect(spy.mock.calls[0][0]).to.include('Warning:'); + expect(spy.mock.calls[1][0]).to.include('Warning:'); + }); + }); });