diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 683462834..7650c182f 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -22,7 +22,7 @@ import { sumFilesSizeInBytes } from '../../lib/files.js'; import { useAbortJobOnSignal } from '../../lib/hooks/useAbortJobOnSignal.js'; import { useActorConfig } from '../../lib/hooks/useActorConfig.js'; import { error, info, run, simpleLog, warning } from '../../lib/outputs.js'; -import { transformEnvToEnvVars } from '../../lib/secrets.js'; +import { transformEnvToEnvVars, validateEnvironmentVariablesShape } from '../../lib/secrets.js'; import { createActZip, createSourceFiles, @@ -249,6 +249,17 @@ export class ActorsPushCommand extends ApifyCommand { const { config: actorConfig } = actorConfigResult.unwrap(); + // Preflight: reject an `environmentVariables` that isn't a key-value object. + // A common mistake is to use the API-wire shape `[{ name, value }]`, which + // the CLI would otherwise pass through unchanged — `@secret` references + // would never be resolved and the build would ship the literal string. + const envShapeError = validateEnvironmentVariablesShape(actorConfig?.environmentVariables); + if (envShapeError) { + error({ message: envShapeError }); + process.exitCode = CommandExitCodes.InvalidActorJson; + return; + } + const userInfo = await getLocalUserInfo(); const isOrganizationLoggedIn = !!userInfo.organizationOwnerUserId; const redirectUrlPart = isOrganizationLoggedIn ? `/organization/${userInfo.id}` : ''; diff --git a/src/commands/secrets/_index.ts b/src/commands/secrets/_index.ts index 95e5b030c..49d26db6d 100644 --- a/src/commands/secrets/_index.ts +++ b/src/commands/secrets/_index.ts @@ -1,6 +1,7 @@ import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; import { LOCAL_CONFIG_PATH } from '../../lib/consts.js'; import { SecretsAddCommand } from './add.js'; +import { SecretsCheckCommand } from './check.js'; import { SecretsLsCommand } from './ls.js'; import { SecretsRmCommand } from './rm.js'; @@ -20,11 +21,15 @@ export class SecretsIndexCommand extends ApifyCommand { + static override name = 'check' as const; + + static override description = + `Verifies that every "@name" secret reference in ${LOCAL_CONFIG_PATH} resolves to a locally stored secret, ` + + `and that "environmentVariables" is shaped as an object (key-value map). ` + + `Runs automatically as a preflight during 'apify push' — use this command to check standalone.`; + + static override examples = [ + { + description: 'Verify that all @-references in .actor/actor.json point to secrets you have added locally.', + command: 'apify secrets check', + }, + ]; + + static override docsUrl = 'https://docs.apify.com/cli/docs/reference#apify-secrets-check'; + + static override enableJsonFlag = true; + + static override flags = { + dir: Flags.string({ + description: 'Directory of the Actor whose .actor/actor.json should be checked.', + required: false, + }), + }; + + async run() { + const { json } = this.flags; + const cwd = this.flags.dir ?? process.cwd(); + + const actorConfigResult = await useActorConfig({ cwd }); + if (actorConfigResult.isErr()) { + error({ message: actorConfigResult.unwrapErr().message }); + process.exitCode = CommandExitCodes.InvalidActorJson; + return; + } + + const { config, exists } = actorConfigResult.unwrap(); + if (!exists) { + error({ + message: `No ${LOCAL_CONFIG_PATH} found in ${cwd}. Run 'apify init' first, or point at the Actor directory with --dir.`, + }); + process.exitCode = CommandExitCodes.InvalidActorJson; + return; + } + + const rawEnv = config?.environmentVariables; + + const shapeError = validateEnvironmentVariablesShape(rawEnv); + if (shapeError) { + if (json) { + printJsonToStdout({ ok: false, shapeError, missing: [], referenced: [] }); + } else { + error({ message: shapeError }); + } + process.exitCode = CommandExitCodes.InvalidActorJson; + return; + } + + if (!rawEnv || Object.keys(rawEnv as object).length === 0) { + if (json) { + printJsonToStdout({ ok: true, missing: [], referenced: [] }); + return; + } + info({ + message: `No "environmentVariables" declared in ${LOCAL_CONFIG_PATH}. Nothing to check.`, + stdout: true, + }); + return; + } + + const env = rawEnv as Record; + const refs = findSecretReferences(env); + const secrets = getSecretsFile(); + const missing = refs.filter((ref) => !(ref.name in secrets)); + + if (json) { + printJsonToStdout({ + ok: missing.length === 0, + referenced: refs, + missing, + }); + if (missing.length > 0) { + process.exitCode = CommandExitCodes.InvalidActorJson; + } + return; + } + + if (missing.length > 0) { + const listing = missing + .map(({ envKey, name }) => ` - ${name} (referenced by environmentVariables.${envKey})`) + .join('\n'); + const addCommands = missing.map(({ name }) => ` apify secrets add ${name} `).join('\n'); + error({ + message: + `The following secrets referenced in ${LOCAL_CONFIG_PATH} are missing from local storage:\n${listing}\n\n` + + `Add them by running:\n${addCommands}`, + }); + process.exitCode = CommandExitCodes.InvalidActorJson; + return; + } + + success({ + message: + refs.length === 0 + ? `No "@name" secret references found in ${LOCAL_CONFIG_PATH}.` + : `All ${refs.length} secret reference(s) in ${LOCAL_CONFIG_PATH} resolve to locally stored secrets.`, + stdout: true, + }); + } +} diff --git a/src/lib/secrets.ts b/src/lib/secrets.ts index 6d3f2654f..38b1b1f8a 100644 --- a/src/lib/secrets.ts +++ b/src/lib/secrets.ts @@ -1,6 +1,6 @@ import { readFileSync, writeFileSync } from 'node:fs'; -import { SECRETS_FILE_PATH } from './consts.js'; +import { LOCAL_CONFIG_PATH, SECRETS_FILE_PATH } from './consts.js'; import { ensureApifyDirectory } from './files.js'; import { warning } from './outputs.js'; @@ -9,6 +9,69 @@ const SECRET_KEY_PREFIX = '@'; const MAX_ENV_VAR_NAME_LENGTH = 100; const MAX_ENV_VAR_VALUE_LENGTH = 50000; +const isSecretKey = (envValue: string) => { + return new RegExp(`^${SECRET_KEY_PREFIX}.{1}`).test(envValue); +}; + +/** + * Ensures that `environmentVariables` in `.actor/actor.json` is shaped as a + * JSON object (key → value map). It is a common mistake to write it as an + * array of `{ name, value }` entries (the shape used by the Apify API on the + * wire), which the CLI would then silently pass through unresolved. + * + * Returns a human-readable error message when the shape is invalid, or `null` + * when it is valid (including when it is absent — the caller decides whether + * that is allowed). + */ +export const validateEnvironmentVariablesShape = (env: unknown): string | null => { + if (env === undefined || env === null) return null; + + if (Array.isArray(env)) { + return ( + `"environmentVariables" in ${LOCAL_CONFIG_PATH} must be a JSON object (key-value map), not an array.\n` + + `\n` + + `Change it from:\n` + + ` "environmentVariables": [{ "name": "MY_TOKEN", "value": "@mySecret" }]\n` + + `to:\n` + + ` "environmentVariables": { "MY_TOKEN": "@mySecret" }\n` + + `\n` + + `See https://docs.apify.com/platform/actors/development/actor-definition/actor-json#fields for the expected schema.` + ); + } + + if (typeof env !== 'object') { + return `"environmentVariables" in ${LOCAL_CONFIG_PATH} must be a JSON object (key-value map); got ${typeof env}.`; + } + + const badEntries = Object.entries(env as Record).filter(([, value]) => typeof value !== 'string'); + if (badEntries.length > 0) { + const listing = badEntries + .map(([key, value]) => ` - ${key}: expected string, got ${Array.isArray(value) ? 'array' : typeof value}`) + .join('\n'); + return ( + `"environmentVariables" in ${LOCAL_CONFIG_PATH} has non-string values:\n${listing}\n` + + `Each value must be a string (use "@secretName" to reference a locally stored secret).` + ); + } + + return null; +}; + +/** + * Returns every `@name` secret reference found in an `environmentVariables` + * map, along with the env-var key it appears under. Values that don't start + * with `@` are ignored. + */ +export const findSecretReferences = (env: Record): { envKey: string; name: string }[] => { + const refs: { envKey: string; name: string }[] = []; + for (const [envKey, value] of Object.entries(env)) { + if (typeof value === 'string' && isSecretKey(value)) { + refs.push({ envKey, name: value.replace(new RegExp(`^${SECRET_KEY_PREFIX}`), '') }); + } + } + return refs; +}; + export const getSecretsFile = () => { try { return JSON.parse(readFileSync(SECRETS_FILE_PATH(), 'utf-8')) || {}; @@ -46,10 +109,6 @@ export const removeSecret = (name: string) => { writeSecretsFile(secrets); }; -const isSecretKey = (envValue: string) => { - return new RegExp(`^${SECRET_KEY_PREFIX}.{1}`).test(envValue); -}; - /** * Replaces secure values in env with proper values from local secrets file. * @param env diff --git a/test/local/lib/secrets.test.ts b/test/local/lib/secrets.test.ts index 55568157e..263df963f 100644 --- a/test/local/lib/secrets.test.ts +++ b/test/local/lib/secrets.test.ts @@ -1,6 +1,59 @@ -import { replaceSecretsValue, transformEnvToEnvVars } from '../../../src/lib/secrets.js'; +import { + findSecretReferences, + replaceSecretsValue, + transformEnvToEnvVars, + validateEnvironmentVariablesShape, +} from '../../../src/lib/secrets.js'; describe('Secrets', () => { + describe('validateEnvironmentVariablesShape()', () => { + it('accepts undefined / null (nothing declared)', () => { + expect(validateEnvironmentVariablesShape(undefined)).toBeNull(); + expect(validateEnvironmentVariablesShape(null)).toBeNull(); + }); + + it('accepts an object of string values', () => { + expect(validateEnvironmentVariablesShape({ TOKEN: '@mySecret', PLAIN: 'value' })).toBeNull(); + }); + + it('rejects an array-shaped environmentVariables', () => { + const err = validateEnvironmentVariablesShape([{ name: 'TOKEN', value: '@mySecret' }]); + expect(err).toMatch(/must be a JSON object/i); + expect(err).toMatch(/not an array/i); + expect(err).toContain('"MY_TOKEN": "@mySecret"'); + }); + + it('rejects a scalar', () => { + expect(validateEnvironmentVariablesShape('foo')).toMatch(/must be a JSON object/i); + }); + + it('rejects an object with non-string values', () => { + const err = validateEnvironmentVariablesShape({ TOKEN: 123, NESTED: { a: 1 } }); + expect(err).toMatch(/non-string values/i); + expect(err).toContain('TOKEN: expected string, got number'); + expect(err).toContain('NESTED: expected string, got object'); + }); + }); + + describe('findSecretReferences()', () => { + it('returns every @name reference with its env key', () => { + expect( + findSecretReferences({ + TOKEN: '@myProdToken', + PLAIN: 'literal', + MONGO_URL: '@mongoUrl', + }), + ).toEqual([ + { envKey: 'TOKEN', name: 'myProdToken' }, + { envKey: 'MONGO_URL', name: 'mongoUrl' }, + ]); + }); + + it('returns an empty array when nothing is secret', () => { + expect(findSecretReferences({ PLAIN: 'value', OTHER: 'email@apify.com' })).toEqual([]); + }); + }); + describe('replaceSecretsValue()', () => { it('should replace secret references with their values', () => { const secrets = {