From d71b15807ec9bfc5b79e440d09a1232cb45aadf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 09:03:18 +0200 Subject: [PATCH 01/42] feat: add the dockerContextDir resolver and Docker context file collector --- src/lib/docker-context.ts | 165 ++++++++++++++++++++ test/local/lib/docker-context-files.test.ts | 142 +++++++++++++++++ test/local/lib/docker-context.test.ts | 130 +++++++++++++++ 3 files changed, 437 insertions(+) create mode 100644 src/lib/docker-context.ts create mode 100644 test/local/lib/docker-context-files.test.ts create mode 100644 test/local/lib/docker-context.test.ts diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts new file mode 100644 index 000000000..eb7650219 --- /dev/null +++ b/src/lib/docker-context.ts @@ -0,0 +1,165 @@ +import { execFileSync } from 'node:child_process'; +import { existsSync, lstatSync, realpathSync } from 'node:fs'; +import { isAbsolute, join, relative, resolve, sep } from 'node:path'; + +import { ACTOR_SPECIFICATION_FOLDER } from './consts.js'; +import { getActorLocalFilePaths } from './utils.js'; + +/** Tracks pushing Actors with `dockerContextDir` (monorepo Actors) to the Apify platform. */ +export const MONOREPO_PLATFORM_SUPPORT_ISSUE_URL = 'https://github.com/apify/apify-core/issues/28685'; + +export type DockerContextResolution = + | { kind: 'none' } + | { + kind: 'context'; + contextRoot: string; + /** The Git repository's root, else the context. */ + sourceRoot: string; + /** Relative to `sourceRoot`. */ + actorPath: string; + } + | { kind: 'invalid'; message: string }; + +/** + * `dockerContextDir` is relative to `.actor/`, as on the platform. A context at or below the Actor folder is an + * ordinary push, which the platform builds as it is; one above or beside it is a monorepo Actor. + */ +export function resolveDockerContext(actorDir: string, dockerContextDir: unknown): DockerContextResolution { + if (dockerContextDir === undefined || dockerContextDir === null || dockerContextDir === '') return { kind: 'none' }; + if (typeof dockerContextDir !== 'string') { + return { kind: 'invalid', message: `"dockerContextDir" in .actor/actor.json must be a string.` }; + } + + const contextRoot = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, dockerContextDir); + const contextPathInActor = relative(actorDir, contextRoot); + if (contextPathInActor === '' || !isOutside(contextPathInActor)) return { kind: 'none' }; + + if (!existsSync(contextRoot)) { + return { + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${contextRoot}, which does not exist.`, + }; + } + + // The platform's clone holds nothing above the repository root. + const repoRoot = git(actorDir, ['rev-parse', '--show-toplevel']); + if (repoRoot) { + const sourceRoot = realpathSync(repoRoot); + if (isOutside(relative(sourceRoot, realpathSync(contextRoot)))) { + return { + kind: 'invalid', + message: + `"dockerContextDir" in .actor/actor.json points to ${contextRoot}, outside the Git repository ${repoRoot}. ` + + 'The Apify platform builds from a clone of the repository, so it could not build this Actor.', + }; + } + return { + kind: 'context', + contextRoot, + sourceRoot, + actorPath: toPosix(relative(sourceRoot, realpathSync(actorDir))), + }; + } + + const actorPath = relative(contextRoot, actorDir); + if (isOutside(actorPath)) { + return { + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${contextRoot}, which does not contain the Actor's folder.`, + }; + } + return { kind: 'context', contextRoot, sourceRoot: contextRoot, actorPath: toPosix(actorPath) }; +} + +function isOutside(relativePath: string): boolean { + return isAbsolute(relativePath) || relativePath === '..' || relativePath.startsWith(`..${sep}`); +} + +function toPosix(filePath: string): string { + return filePath.split(sep).join('/'); +} + +export interface GitProvenance { + remoteUrl?: string; + branch?: string; + commit?: string; + dirty?: boolean; +} + +function git(cwd: string, args: string[]): string | undefined { + try { + const output = execFileSync('git', args, { cwd, encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] }); + return output.trim(); + } catch { + return undefined; + } +} + +/** A remote URL may carry credentials (`https://user:token@host/...`); they never leave this machine. */ +function withoutCredentials(remoteUrl: string): string { + try { + const url = new URL(remoteUrl); + url.username = ''; + url.password = ''; + return url.toString(); + } catch { + // scp-like `git@host:owner/repo.git` has no password to strip. + return remoteUrl; + } +} + +export function readGitProvenance(dir: string): GitProvenance { + const commit = git(dir, ['rev-parse', 'HEAD']); + if (!commit) return {}; + + const provenance: GitProvenance = { commit }; + const branch = git(dir, ['rev-parse', '--abbrev-ref', 'HEAD']); + if (branch && branch !== 'HEAD') provenance.branch = branch; + const remoteUrl = git(dir, ['remote', 'get-url', 'origin']); + if (remoteUrl) provenance.remoteUrl = withoutCredentials(remoteUrl); + const status = git(dir, ['status', '--porcelain', '--', '.']); + if (status !== undefined) provenance.dirty = status.length > 0; + return provenance; +} + +/** Untracked, these are never on the platform and too big to push by accident. */ +const UNTRACKED_IGNORED_SEGMENTS = new Set(['node_modules', 'storage', 'apify_storage', 'crawlee_storage']); + +function gitFileList(root: string, args: string[]): string[] | undefined { + const output = git(root, ['ls-files', '-z', ...args]); + return output?.split('\0').filter(Boolean); +} + +/** What a Git clone of `root` has, as on disk now, plus untracked files Git does not ignore; `.actorignore` + * does not apply, as on the platform. Outside Git, what an ordinary push of `root` sends. */ +export async function getContextFilePaths(root: string): Promise { + const tracked = gitFileList(root, ['--cached']); + const untracked = gitFileList(root, ['--others', '--exclude-standard']); + if (!tracked || !untracked) return getActorLocalFilePaths(root); + + // Skips files deleted locally and submodules (listed as directories). + const isPushable = (filePath: string) => { + try { + const stats = lstatSync(join(root, filePath)); + return stats.isSymbolicLink() || stats.isFile(); + } catch { + return false; + } + }; + return [ + ...tracked.filter(isPushable), + ...untracked.filter( + (filePath) => + isPushable(filePath) && !filePath.split('/').some((segment) => UNTRACKED_IGNORED_SEGMENTS.has(segment)), + ), + ].map((filePath) => filePath.split('/').join(sep)); +} + +export function monorepoUnsupportedMessage(actorName: string): string { + return [ + `Actor ${actorName} sets "dockerContextDir" in .actor/actor.json, so it builds from a Docker context outside its own folder (a monorepo Actor).`, + 'apify push does not support such Actors on the Apify platform yet. Follow the progress at:', + ` ${MONOREPO_PLATFORM_SUPPORT_ISSUE_URL}`, + `Until then, set the Actor's source in Apify Console to the Git repository with the folder (#:).`, + ].join('\n'); +} diff --git a/test/local/lib/docker-context-files.test.ts b/test/local/lib/docker-context-files.test.ts new file mode 100644 index 000000000..35260cf0b --- /dev/null +++ b/test/local/lib/docker-context-files.test.ts @@ -0,0 +1,142 @@ +import { execFileSync } from 'node:child_process'; +import { mkdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; + +import { getContextFilePaths } from '../../../src/lib/docker-context.js'; +import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; + +// vi.mock is hoisted before imports, so both the context collector and utils.ts get the mocked version. With +// `gitAvailable` off, every git call fails as it does without git, which sends the collector down its fallback. +const gitState = vi.hoisted(() => ({ gitAvailable: true })); +vi.mock('node:child_process', async (importOriginal) => { + const original = await importOriginal(); + const unavailable = () => { + throw new Error('not a git repository'); + }; + return { + ...original, + execSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execSync(...args) : unavailable()) as typeof original.execSync, + execFileSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execFileSync(...args) : unavailable()) as typeof original.execFileSync, + }; +}); + +const ACTOR_PATH = 'actors/x'; + +describe('getContextFilePaths', () => { + describe('inside a Git repository', () => { + const { tmpPath, joinPath, beforeAllCalls, afterAllCalls } = useTempPath('docker-context-files-test-dir', { + create: true, + remove: true, + cwd: false, + cwdParent: false, + }); + + const write = (relativePath: string, content = 'content') => { + mkdirSync(dirname(joinPath(relativePath)), { recursive: true }); + writeFileSync(joinPath(relativePath), content); + }; + const git = (...args: string[]) => execFileSync('git', args, { cwd: tmpPath, stdio: 'ignore' }); + + beforeAll(async () => { + await beforeAllCalls(); + + write('package.json', '{}'); + write('.gitignore', 'dist\n'); + write('shared/Dockerfile', 'FROM apify/actor-node:20\n'); + write(`${ACTOR_PATH}/.actor/actor.json`, '{}'); + write(`${ACTOR_PATH}/src/index.ts`); + write(`${ACTOR_PATH}/.actorignore`, '!dist/\nsrc/index.ts\n'); + write('packages/utils/src/index.ts'); + write('node_modules/tracked/index.js'); + git('init', '--quiet'); + git('add', '.'); + git('-c', 'user.name=Test', '-c', 'user.email=test@example.com', 'commit', '--quiet', '-m', 'init'); + + // After the commit: untracked, git-ignored, deleted and linked files, as a working copy has them. + write('shared/start.sh'); + write(`${ACTOR_PATH}/dist/main.js`); + write('node_modules/untracked/index.js'); + write('storage/key_value_stores/default/INPUT.json'); + rmSync(joinPath('packages/utils/src/index.ts')); + symlinkSync('../../shared', joinPath(`${ACTOR_PATH}/shared`)); + }); + + afterAll(async () => { + await afterAllCalls(); + }); + + it('lists the tracked files and the untracked files Git does not ignore', async () => { + const paths = await getContextFilePaths(tmpPath); + + expect(paths).toEqual( + expect.arrayContaining([ + 'package.json', + '.gitignore', + join('shared', 'Dockerfile'), + join(ACTOR_PATH, '.actor', 'actor.json'), + join(ACTOR_PATH, 'src', 'index.ts'), + join('shared', 'start.sh'), + ]), + ); + expect(paths).not.toContain(join(ACTOR_PATH, 'dist', 'main.js')); + }); + + it('skips a tracked file that is deleted locally', async () => { + expect(await getContextFilePaths(tmpPath)).not.toContain(join('packages', 'utils', 'src', 'index.ts')); + }); + + it('drops node_modules and storage folders from the untracked files only', async () => { + const paths = await getContextFilePaths(tmpPath); + + expect(paths).toContain(join('node_modules', 'tracked', 'index.js')); + expect(paths).not.toContain(join('node_modules', 'untracked', 'index.js')); + expect(paths).not.toContain(join('storage', 'key_value_stores', 'default', 'INPUT.json')); + }); + + it('does not apply .actorignore, as the platform clone has none of its effects', async () => { + const paths = await getContextFilePaths(tmpPath); + + // Tracked, so in the clone, although .actorignore excludes it. + expect(paths).toContain(join(ACTOR_PATH, 'src', 'index.ts')); + // Git-ignored, so not in the clone, whatever .actorignore force-includes. + expect(paths).not.toContain(join(ACTOR_PATH, 'dist', 'main.js')); + }); + + it('keeps a symlink, as a Git clone does', async () => { + expect(await getContextFilePaths(tmpPath)).toContain(join(ACTOR_PATH, 'shared')); + }); + }); + + describe('without git', () => { + const { tmpPath, joinPath, beforeAllCalls, afterAllCalls } = useTempPath('docker-context-files-no-git-dir', { + create: true, + remove: true, + cwd: false, + cwdParent: false, + }); + + beforeAll(async () => { + await beforeAllCalls(); + mkdirSync(joinPath('src'), { recursive: true }); + writeFileSync(joinPath('main.js'), 'content'); + writeFileSync(joinPath('src/index.js'), 'content'); + writeFileSync(joinPath('debug.log'), 'content'); + writeFileSync(joinPath('.actorignore'), 'debug.log\n'); + gitState.gitAvailable = false; + }); + + afterAll(async () => { + gitState.gitAvailable = true; + await afterAllCalls(); + }); + + it('falls back to the ordinary file list, which applies .actorignore', async () => { + const paths = await getContextFilePaths(tmpPath); + + expect(paths).toEqual(expect.arrayContaining(['main.js', 'src/index.js'])); + expect(paths).not.toContain('debug.log'); + }); + }); +}); diff --git a/test/local/lib/docker-context.test.ts b/test/local/lib/docker-context.test.ts new file mode 100644 index 000000000..4b1477b3e --- /dev/null +++ b/test/local/lib/docker-context.test.ts @@ -0,0 +1,130 @@ +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, realpathSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; + +import { resolveDockerContext } from '../../../src/lib/docker-context.js'; +import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; + +const ACTOR_PATH = 'actors/x'; + +describe('resolveDockerContext', () => { + describe('inside a Git repository', () => { + const { tmpPath, joinPath, beforeAllCalls, afterAllCalls } = useTempPath('docker-context-test-dir', { + create: true, + remove: true, + cwd: false, + cwdParent: false, + }); + + const actorDir = () => joinPath(ACTOR_PATH); + + beforeAll(async () => { + await beforeAllCalls(); + mkdirSync(joinPath(ACTOR_PATH, '.actor'), { recursive: true }); + mkdirSync(joinPath(ACTOR_PATH, 'src'), { recursive: true }); + mkdirSync(joinPath('shared'), { recursive: true }); + execFileSync('git', ['init', '--quiet'], { cwd: tmpPath, stdio: 'ignore' }); + }); + + afterAll(async () => { + await afterAllCalls(); + }); + + it('treats a missing field, or a context at or below the Actor folder, as an ordinary push', () => { + expect(resolveDockerContext(actorDir(), undefined)).toEqual({ kind: 'none' }); + expect(resolveDockerContext(actorDir(), '')).toEqual({ kind: 'none' }); + expect(resolveDockerContext(actorDir(), '..')).toEqual({ kind: 'none' }); + expect(resolveDockerContext(actorDir(), '.')).toEqual({ kind: 'none' }); + expect(resolveDockerContext(actorDir(), '../src')).toEqual({ kind: 'none' }); + }); + + it('resolves a context above the Actor folder to the repository root and the Actor path in it', () => { + expect(resolveDockerContext(actorDir(), '../../..')).toEqual({ + kind: 'context', + contextRoot: tmpPath, + sourceRoot: realpathSync(tmpPath), + actorPath: ACTOR_PATH, + }); + }); + + it('keeps the repository root as the source for a context elsewhere in the repository', () => { + expect(resolveDockerContext(actorDir(), '../../../shared')).toEqual({ + kind: 'context', + contextRoot: joinPath('shared'), + sourceRoot: realpathSync(tmpPath), + actorPath: ACTOR_PATH, + }); + }); + + it('rejects a context outside the Git repository', () => { + const repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { + cwd: actorDir(), + encoding: 'utf-8', + }).trim(); + + expect(resolveDockerContext(actorDir(), '../../../..')).toEqual({ + kind: 'invalid', + message: + `"dockerContextDir" in .actor/actor.json points to ${resolve(tmpPath, '..')}, outside the Git repository ${repoRoot}. ` + + 'The Apify platform builds from a clone of the repository, so it could not build this Actor.', + }); + }); + + it('rejects a context that does not exist', () => { + expect(resolveDockerContext(actorDir(), '../../../missing')).toEqual({ + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${joinPath('missing')}, which does not exist.`, + }); + }); + + it('rejects a field that is not a string', () => { + expect(resolveDockerContext(actorDir(), 42)).toEqual({ + kind: 'invalid', + message: '"dockerContextDir" in .actor/actor.json must be a string.', + }); + }); + }); + + describe('outside a Git repository', () => { + let root: string; + let actorDir: string; + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'apify-cli-docker-context-')); + actorDir = join(root, ACTOR_PATH); + mkdirSync(join(actorDir, '.actor'), { recursive: true }); + mkdirSync(join(root, 'other'), { recursive: true }); + }); + + afterAll(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('makes the context the source when it contains the Actor folder', () => { + expect(resolveDockerContext(actorDir, '../../..')).toEqual({ + kind: 'context', + contextRoot: root, + sourceRoot: root, + actorPath: ACTOR_PATH, + }); + }); + + it('rejects a sibling directory, which does not contain the Actor folder', () => { + expect(resolveDockerContext(actorDir, '../../../other')).toEqual({ + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${join(root, 'other')}, which does not contain the Actor's folder.`, + }); + }); + + it('rejects a context that does not exist', () => { + // `resolve`, not `join`: on Windows the resolved context carries the drive (`D:\repo`), as the command resolves it. + const repoDir = resolve('/repo'); + + expect(resolveDockerContext(join(repoDir, 'actors', 'a'), '../../..')).toEqual({ + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${repoDir}, which does not exist.`, + }); + }); + }); +}); From d49e15e21327721067917894dc4002bc3dd45fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 09:03:22 +0200 Subject: [PATCH 02/42] feat: refuse monorepo Actors in apify push before any platform write --- src/commands/actors/push.ts | 14 ++ test/local/commands/push-monorepo.test.ts | 167 ++++++++++++++++++++++ 2 files changed, 181 insertions(+) create mode 100644 test/local/commands/push-monorepo.test.ts diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 03c25e4ab..2489eacc4 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -19,6 +19,7 @@ import { Args } from '../../lib/command-framework/args.js'; import { Flags } from '../../lib/command-framework/flags.js'; import { getConsoleUrl } from '../../lib/console-url.js'; import { CommandExitCodes, DEPRECATED_LOCAL_CONFIG_NAME, LOCAL_CONFIG_PATH } from '../../lib/consts.js'; +import { monorepoUnsupportedMessage, resolveDockerContext } from '../../lib/docker-context.js'; import { sumFilesSizeInBytes } from '../../lib/files.js'; import { useAbortJobOnSignal } from '../../lib/hooks/useAbortJobOnSignal.js'; import { useActorConfig } from '../../lib/hooks/useActorConfig.js'; @@ -288,6 +289,19 @@ export class ActorsPushCommand extends ApifyCommand { const { config: actorConfig } = actorConfigResult.unwrap(); + // A monorepo Actor builds from a Docker context above its folder, which a push of that folder cannot carry. + const dockerContext = resolveDockerContext(cwd, actorConfig?.dockerContextDir); + if (dockerContext.kind === 'invalid') { + error({ message: dockerContext.message }); + process.exitCode = CommandExitCodes.InvalidActorJson; + return; + } + if (dockerContext.kind === 'context') { + error({ message: monorepoUnsupportedMessage(actorConfig!.name as string) }); + process.exitCode = CommandExitCodes.NotImplemented; + return; + } + const userInfo = await getCurrentUserInfo(); const isOrganizationLoggedIn = !!userInfo.organizationOwnerUserId; const redirectUrlPart = isOrganizationLoggedIn ? `/organization/${userInfo.id}` : ''; diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts new file mode 100644 index 000000000..6d3eab22d --- /dev/null +++ b/test/local/commands/push-monorepo.test.ts @@ -0,0 +1,167 @@ +import { execFileSync } from 'node:child_process'; +import { mkdir, writeFile } from 'node:fs/promises'; +import { dirname } from 'node:path'; + +import { ACTOR_SOURCE_TYPES } from '@apify/consts'; + +import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; +import { CommandExitCodes } from '../../../src/lib/consts.js'; +import { MONOREPO_PLATFORM_SUPPORT_ISSUE_URL } from '../../../src/lib/docker-context.js'; +import { cwdCache } from '../../../src/lib/hooks/useActorConfig.js'; +import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; +import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; + +const actName = 'push-monorepo-actor'; +const ACTOR_ID = 'mOnO1234efGh'; +const ACTOR_PATH = 'actors/typescript-actor'; + +const actor = { + id: ACTOR_ID, + name: actName, + // Older than any local file, so the "modified on the platform" check never trips. + modifiedAt: new Date(0), + taggedBuilds: { latest: { buildId: 'build1' } }, +}; +const build = { id: 'build1', actId: ACTOR_ID, buildNumber: '0.0.1', status: 'SUCCEEDED' }; +const currentVersion = { versionNumber: '0.0', sourceType: ACTOR_SOURCE_TYPES.SOURCE_FILES }; + +const actorGet = vitest.fn(async () => actor); +const versionUpdate = vitest.fn(async (_modifier: unknown) => ({})); +const versionCreate = vitest.fn(async (created: unknown) => created); +const keyValueStoreGetOrCreate = vitest.fn(async () => ({ id: 'store1' })); + +// Stands in for the platform: the Actor and its version exist, and every write is recorded. +vitest.mock('../../../src/lib/utils.js', async (importOriginal) => ({ + ...(await importOriginal()), + getCurrentUserInfo: vitest.fn(async () => ({ id: 'userId', username: 'user' })), + outputJobLog: vitest.fn(async () => {}), + getLoggedClientOrThrow: vitest.fn(async () => ({ + baseUrl: 'https://api.apify.com/v2', + token: 'my-token', + actor: () => ({ + get: actorGet, + version: () => ({ get: async () => currentVersion, update: versionUpdate }), + versions: () => ({ create: versionCreate }), + build: async () => build, + }), + keyValueStores: () => ({ getOrCreate: keyValueStoreGetOrCreate }), + build: () => ({ get: async () => build }), + })), +})); + +// The mocked `node:process` copy has no signal handlers; nothing here needs them. +vitest.mock('../../../src/lib/hooks/useAbortJobOnSignal.js', () => ({ + useAbortJobOnSignal: () => ({ [Symbol.dispose]() {} }), +})); + +const { beforeAllCalls, afterAllCalls, joinPath, tmpPath, forceNewCwd } = useTempPath(actName, { + create: true, + remove: true, + cwd: true, + cwdParent: false, +}); + +const { logMessages } = useConsoleSpy(); + +const { ActorsPushCommand } = await import('../../../src/commands/actors/push.js'); +// The command sets the exit code on this mocked copy of `node:process`, not on the global one. +const { default: commandProcess } = await import('node:process'); + +async function write(relativePath: string, content: string) { + await mkdir(dirname(joinPath(relativePath)), { recursive: true }); + await writeFile(joinPath(relativePath), content); +} + +async function writeActorConfig(fields: Record) { + await write( + `${ACTOR_PATH}/.actor/actor.json`, + JSON.stringify({ actorSpecification: 1, name: actName, version: '0.0', buildTag: 'latest', ...fields }), + ); +} + +function git(...args: string[]) { + execFileSync('git', args, { cwd: tmpPath, stdio: 'ignore' }); +} + +let originalExitCode: typeof process.exitCode; + +beforeEach(async () => { + originalExitCode = commandProcess.exitCode; + actorGet.mockClear(); + versionUpdate.mockClear(); + versionCreate.mockClear(); + keyValueStoreGetOrCreate.mockClear(); + cwdCache.clear(); + await beforeAllCalls(); + + // The layout of apify/actor-monorepo-example, as its own Git repository. + await write('package.json', '{"workspaces":["actors/**","packages/**"]}'); + await write('.gitignore', 'node_modules\n'); + await write('shared/TypeScript_Dockerfile', 'FROM apify/actor-node:20\n'); + await write('packages/typescript-utils/src/index.ts', 'export const x = 1;\n'); + await write('node_modules/ignored/index.js', ''); + await writeActorConfig({ dockerContextDir: '../../..', dockerfile: '../../../shared/TypeScript_Dockerfile' }); + await write(`${ACTOR_PATH}/src/index.ts`, 'console.log(1);\n'); + git('init', '--quiet', '--initial-branch=main'); + git('add', '.'); + git('-c', 'user.name=Test', '-c', 'user.email=test@example.com', 'commit', '--quiet', '-m', 'init'); + + forceNewCwd(ACTOR_PATH); +}); + +afterEach(async () => { + commandProcess.exitCode = originalExitCode; + await afterAllCalls(); +}); + +describe('apify push of a monorepo Actor', () => { + it('stops before touching the platform, pointing at the tracking issue', async () => { + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.NotImplemented); + const stderr = logMessages.error.join('\n'); + expect(stderr).toContain( + `Actor ${actName} sets "dockerContextDir" in .actor/actor.json, so it builds from a Docker context outside its own folder (a monorepo Actor).`, + ); + expect(stderr).toContain('apify push does not support such Actors on the Apify platform yet'); + expect(stderr).toContain(MONOREPO_PLATFORM_SUPPORT_ISSUE_URL); + expect(stderr).toContain("set the Actor's source in Apify Console to the Git repository with the folder"); + expect(actorGet).not.toHaveBeenCalled(); + expect(versionCreate).not.toHaveBeenCalled(); + expect(versionUpdate).not.toHaveBeenCalled(); + expect(keyValueStoreGetOrCreate).not.toHaveBeenCalled(); + }); + + it('keeps stdout clean with --json', async () => { + await testRunCommand(ActorsPushCommand, { flags_json: true }); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.NotImplemented); + expect(logMessages.log).toEqual([]); + expect(logMessages.error.join('\n')).toContain(MONOREPO_PLATFORM_SUPPORT_ISSUE_URL); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it('rejects a dockerContextDir outside the Git repository, which the platform could not build', async () => { + await writeActorConfig({ dockerContextDir: '../../../..' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.error.join('\n')).toContain('outside the Git repository'); + expect(actorGet).not.toHaveBeenCalled(); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it('pushes the Actor folder as usual when the Docker context lies inside it', async () => { + await writeActorConfig({ dockerContextDir: '../src' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(versionUpdate).toHaveBeenCalledTimes(1); + const modifier = versionUpdate.mock.calls[0][0] as { sourceType: string; sourceFiles: { name: string }[] }; + expect(modifier.sourceType).toBe(ACTOR_SOURCE_TYPES.SOURCE_FILES); + expect(modifier.sourceFiles.map((file) => file.name).sort()).toEqual(['.actor/actor.json', 'src/index.ts']); + expect(logMessages.log.join('\n')).toContain('Apify push result: SUCCEEDED'); + }); +}); From 3fa653bdc13deeb8649233aa5f3484d98d5a3400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 09:55:43 +0200 Subject: [PATCH 03/42] feat: validate actor.json paths before apify push uploads anything --- src/commands/actors/push.ts | 14 ++++ src/lib/actor-paths.ts | 137 ++++++++++++++++++++++++++++++++++++ src/lib/docker-context.ts | 4 +- 3 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 src/lib/actor-paths.ts diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 2489eacc4..4d50dbb1d 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -14,6 +14,7 @@ import { } from '@apify/consts'; import { createHmacSignature } from '@apify/utilities'; +import { checkActorPaths, formatActorPathProblems } from '../../lib/actor-paths.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'; @@ -302,6 +303,19 @@ export class ActorsPushCommand extends ApifyCommand { return; } + // The platform resolves these paths after the upload; a wrong one builds the wrong image or fails the build. + const pathProblems = checkActorPaths({ + actorDir: cwd, + uploadRoot: cwd, + uploadedFiles: filePathsToPush, + config: actorConfig, + }); + if (pathProblems.length > 0) { + error({ message: formatActorPathProblems(pathProblems) }); + process.exitCode = CommandExitCodes.InvalidActorJson; + return; + } + const userInfo = await getCurrentUserInfo(); const isOrganizationLoggedIn = !!userInfo.organizationOwnerUserId; const redirectUrlPart = isOrganizationLoggedIn ? `/organization/${userInfo.id}` : ''; diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts new file mode 100644 index 000000000..b7597df7a --- /dev/null +++ b/src/lib/actor-paths.ts @@ -0,0 +1,137 @@ +import { readdirSync, realpathSync, statSync } from 'node:fs'; +import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'; + +import { ACTOR_SPECIFICATION_FOLDER } from './consts.js'; +import { isOutside, toPosix } from './docker-context.js'; + +export interface ActorPathProblem { + field: string; + value: string; + resolvedPath: string; + reason: string; +} + +export interface ActorPathsOptions { + actorDir: string; + /** What push sends: the Actor folder, or the source root of a monorepo Actor. */ + uploadRoot: string; + /** Relative to `uploadRoot`, as the file collectors return them. */ + uploadedFiles: string[]; + config: Record; + /** When given, `dockerfile` must lie inside it, as BuildKit reads the Dockerfile from the context. */ + dockerContextRoot?: string; +} + +/** The platform finds these by a directory listing, so their names match case-insensitively. */ +const LISTED_FILE_FIELDS = ['dockerfile', 'readme', 'changelog']; +const SCHEMA_FIELDS = ['input', 'output', 'webServerSchema']; +const STORAGE_SCHEMA_FIELDS = ['keyValueStore', 'dataset']; + +interface PathField { + field: string; + value: string; + caseInsensitive: boolean; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +/** Every file-path field the platform resolves relative to `.actor/`; an inline schema object is not a path. */ +function collectPathFields(config: Record): PathField[] { + const fields: PathField[] = []; + const add = (field: string, value: unknown, caseInsensitive = false) => { + if (typeof value === 'string') fields.push({ field, value, caseInsensitive }); + }; + + for (const field of LISTED_FILE_FIELDS) add(field, config[field], true); + for (const field of SCHEMA_FIELDS) add(field, config[field]); + if (isRecord(config.storages)) { + for (const field of STORAGE_SCHEMA_FIELDS) add(`storages.${field}`, config.storages[field]); + if (isRecord(config.storages.datasets)) { + for (const [name, value] of Object.entries(config.storages.datasets)) { + add(`storages.datasets.${name}`, value); + } + } + } + return fields; +} + +function listDirectory(dir: string): string[] { + try { + return readdirSync(dir); + } catch { + return []; + } +} + +function checkPathField( + { field, value, caseInsensitive }: PathField, + { actorDir, uploadRoot, uploadedFiles, dockerContextRoot }: ActorPathsOptions, +): ActorPathProblem | undefined { + const problem = (resolvedPath: string, reason: string) => ({ field, value, resolvedPath, reason }); + if (isAbsolute(value)) return problem(value, `is absolute; the path is relative to ${ACTOR_SPECIFICATION_FOLDER}/`); + + const resolvedPath = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, value); + const root = resolve(uploadRoot); + if (isOutside(relative(root, resolvedPath))) return problem(resolvedPath, `lies outside the uploaded folder ${root}`); + + const dir = dirname(resolvedPath); + const wanted = basename(resolvedPath); + const entries = listDirectory(dir); + const sameCase = entries.find((entry) => entry === wanted); + const anyCase = entries.find((entry) => entry.toLowerCase() === wanted.toLowerCase()); + const name = sameCase ?? (caseInsensitive ? anyCase : undefined); + if (!name) { + return problem( + resolvedPath, + anyCase ? `does not exist (the name must match ${anyCase} exactly)` : 'does not exist', + ); + } + + const filePath = join(dir, name); + let realPath: string; + try { + realPath = realpathSync(filePath); + } catch { + return problem(resolvedPath, 'does not exist'); + } + if (isOutside(relative(realpathSync(root), realPath))) { + return problem(resolvedPath, `lies outside the uploaded folder ${root} (through a symlink)`); + } + if (statSync(realPath).isDirectory()) return problem(resolvedPath, 'is a directory, not a file'); + if (dockerContextRoot && field === 'dockerfile' && isOutside(relative(realpathSync(dockerContextRoot), realPath))) { + return problem(resolvedPath, `lies outside the Docker context ${dockerContextRoot}`); + } + + const uploadedPath = toPosix(relative(root, filePath)); + const uploaded = uploadedFiles.map(toPosix); + if (!uploaded.includes(uploadedPath)) { + const differentCase = uploaded.find((uploadedFile) => uploadedFile.toLowerCase() === uploadedPath.toLowerCase()); + return problem( + resolvedPath, + differentCase + ? `does not match the case of the uploaded file ${differentCase}` + : 'is not among the files apify push uploads (excluded by .gitignore or .actorignore?)', + ); + } + return undefined; +} + +/** + * Applies the platform's rules to every file path in `actor.json` before anything is uploaded: relative to + * `.actor/`, inside the upload root (symlinks resolved), an existing file, and one that push sends. + */ +export function checkActorPaths(options: ActorPathsOptions): ActorPathProblem[] { + return collectPathFields(options.config).flatMap((pathField) => checkPathField(pathField, options) ?? []); +} + +export function formatActorPathProblems(problems: ActorPathProblem[]): string { + return [ + '.actor/actor.json refers to files the Apify platform could not use:', + ...problems.map( + ({ field, value, resolvedPath, reason }) => ` "${field}": "${value}" (${resolvedPath}) ${reason}.`, + ), + `Paths are relative to ${ACTOR_SPECIFICATION_FOLDER}/ and must point to files inside the uploaded folder.`, + ].join('\n'); +} diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts index eb7650219..dc1c3dd43 100644 --- a/src/lib/docker-context.ts +++ b/src/lib/docker-context.ts @@ -71,11 +71,11 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown return { kind: 'context', contextRoot, sourceRoot: contextRoot, actorPath: toPosix(actorPath) }; } -function isOutside(relativePath: string): boolean { +export function isOutside(relativePath: string): boolean { return isAbsolute(relativePath) || relativePath === '..' || relativePath.startsWith(`..${sep}`); } -function toPosix(filePath: string): string { +export function toPosix(filePath: string): string { return filePath.split(sep).join('/'); } From afa1cb7e1014b1b1aa92ccaa7ab553150aaa7782 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 09:55:45 +0200 Subject: [PATCH 04/42] test: cover the actor.json path checks and the push refusal --- test/local/commands/push-actor-paths.test.ts | 151 +++++++++++++++ test/local/lib/actor-paths.test.ts | 186 +++++++++++++++++++ 2 files changed, 337 insertions(+) create mode 100644 test/local/commands/push-actor-paths.test.ts create mode 100644 test/local/lib/actor-paths.test.ts diff --git a/test/local/commands/push-actor-paths.test.ts b/test/local/commands/push-actor-paths.test.ts new file mode 100644 index 000000000..f55be9cef --- /dev/null +++ b/test/local/commands/push-actor-paths.test.ts @@ -0,0 +1,151 @@ +import { mkdir, writeFile } from 'node:fs/promises'; +import { dirname } from 'node:path'; + +import { ACTOR_SOURCE_TYPES } from '@apify/consts'; + +import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; +import { CommandExitCodes } from '../../../src/lib/consts.js'; +import { cwdCache } from '../../../src/lib/hooks/useActorConfig.js'; +import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; +import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; + +const actName = 'push-actor-paths-actor'; +const ACTOR_ID = 'pAtH1234efGh'; + +const actor = { + id: ACTOR_ID, + name: actName, + // Older than any local file, so the "modified on the platform" check never trips. + modifiedAt: new Date(0), + taggedBuilds: { latest: { buildId: 'build1' } }, +}; +const build = { id: 'build1', actId: ACTOR_ID, buildNumber: '0.0.1', status: 'SUCCEEDED' }; +const currentVersion = { versionNumber: '0.0', sourceType: ACTOR_SOURCE_TYPES.SOURCE_FILES }; + +const actorGet = vitest.fn(async () => actor); +const versionUpdate = vitest.fn(async (_modifier: unknown) => ({})); +const versionCreate = vitest.fn(async (created: unknown) => created); + +// Stands in for the platform: the Actor and its version exist, and every read and write is recorded. +vitest.mock('../../../src/lib/utils.js', async (importOriginal) => ({ + ...(await importOriginal()), + getCurrentUserInfo: vitest.fn(async () => ({ id: 'userId', username: 'user' })), + outputJobLog: vitest.fn(async () => {}), + getLoggedClientOrThrow: vitest.fn(async () => ({ + baseUrl: 'https://api.apify.com/v2', + actor: () => ({ + get: actorGet, + version: () => ({ get: async () => currentVersion, update: versionUpdate }), + versions: () => ({ create: versionCreate }), + build: async () => build, + }), + build: () => ({ get: async () => build }), + })), +})); + +// The mocked `node:process` copy has no signal handlers; nothing here needs them. +vitest.mock('../../../src/lib/hooks/useAbortJobOnSignal.js', () => ({ + useAbortJobOnSignal: () => ({ [Symbol.dispose]() {} }), +})); + +const { beforeAllCalls, afterAllCalls, joinPath, tmpPath } = useTempPath(actName, { + create: true, + remove: true, + cwd: true, + cwdParent: false, +}); + +const { logMessages } = useConsoleSpy(); + +const { ActorsPushCommand } = await import('../../../src/commands/actors/push.js'); +// The command sets the exit code on this mocked copy of `node:process`, not on the global one. +const { default: commandProcess } = await import('node:process'); + +async function write(relativePath: string, content: string) { + await mkdir(dirname(joinPath(relativePath)), { recursive: true }); + await writeFile(joinPath(relativePath), content); +} + +async function writeActorConfig(fields: Record) { + await write( + '.actor/actor.json', + JSON.stringify({ actorSpecification: 1, name: actName, version: '0.0', buildTag: 'latest', ...fields }), + ); +} + +let originalExitCode: typeof process.exitCode; + +beforeEach(async () => { + originalExitCode = commandProcess.exitCode; + actorGet.mockClear(); + versionUpdate.mockClear(); + versionCreate.mockClear(); + cwdCache.clear(); + await beforeAllCalls(); + + await write('.actor/INPUT_SCHEMA.json', '{}'); + await write('.actorignore', 'Dockerfile.ci\n'); + await write('Dockerfile', 'FROM apify/actor-node:20\n'); + await write('Dockerfile.ci', 'FROM apify/actor-node:20\n'); + await write('src/main.js', 'console.log(1);\n'); +}); + +afterEach(async () => { + commandProcess.exitCode = originalExitCode; + await afterAllCalls(); +}); + +describe('apify push with file paths in actor.json', () => { + it('stops before reading or writing the Actor, listing every wrong path', async () => { + await writeActorConfig({ + dockerfile: '../Dockerfile.prod', + readme: '../Dockerfile.ci', + input: '../../INPUT_SCHEMA.json', + }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + const stderr = logMessages.error.join('\n'); + expect(stderr).toContain('.actor/actor.json refers to files the Apify platform could not use:'); + expect(stderr).toContain(`"dockerfile": "../Dockerfile.prod" (${joinPath('Dockerfile.prod')}) does not exist.`); + expect(stderr).toContain( + `"readme": "../Dockerfile.ci" (${joinPath('Dockerfile.ci')}) is not among the files apify push uploads (excluded by .gitignore or .actorignore?).`, + ); + expect(stderr).toContain( + `"input": "../../INPUT_SCHEMA.json" (${joinPath('..', 'INPUT_SCHEMA.json')}) lies outside the uploaded folder ${tmpPath}.`, + ); + expect(actorGet).not.toHaveBeenCalled(); + expect(versionCreate).not.toHaveBeenCalled(); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it('keeps stdout clean with --json', async () => { + await writeActorConfig({ dockerfile: '../Dockerfile.prod' }); + + await testRunCommand(ActorsPushCommand, { flags_json: true }); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.log).toEqual([]); + expect(logMessages.error.join('\n')).toContain('"dockerfile": "../Dockerfile.prod"'); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it('pushes as usual when every path points to an uploaded file', async () => { + await writeActorConfig({ dockerfile: '../dockerfile', input: './INPUT_SCHEMA.json' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(versionUpdate).toHaveBeenCalledTimes(1); + const modifier = versionUpdate.mock.calls[0][0] as { sourceFiles: { name: string }[] }; + expect(modifier.sourceFiles.map((file) => file.name).sort()).toEqual([ + '.actor/INPUT_SCHEMA.json', + '.actor/actor.json', + '.actorignore', + 'Dockerfile', + 'src/main.js', + ]); + expect(logMessages.log.join('\n')).toContain('Apify push result: SUCCEEDED'); + }); +}); diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts new file mode 100644 index 000000000..c0ca4305e --- /dev/null +++ b/test/local/lib/actor-paths.test.ts @@ -0,0 +1,186 @@ +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; + +import { checkActorPaths, formatActorPathProblems } from '../../../src/lib/actor-paths.js'; +import { getActorLocalFilePaths } from '../../../src/lib/utils.js'; + +const ACTOR_PATH = 'actors/x'; + +describe('checkActorPaths', () => { + let root: string; + let actorDir: string; + let uploadedFiles: string[]; + + const write = (relativePath: string, content = 'content') => { + mkdirSync(dirname(join(root, relativePath)), { recursive: true }); + writeFileSync(join(root, relativePath), content); + }; + + // The Actor folder is what an ordinary push uploads. + const check = (config: Record, options: { uploadRoot?: string; dockerContextRoot?: string } = {}) => + checkActorPaths({ actorDir, uploadRoot: actorDir, uploadedFiles, config, ...options }); + + beforeAll(async () => { + root = mkdtempSync(join(tmpdir(), 'apify-cli-actor-paths-')); + actorDir = join(root, ACTOR_PATH); + write(`${ACTOR_PATH}/.actor/actor.json`, '{}'); + write(`${ACTOR_PATH}/.actor/INPUT_SCHEMA.json`, '{}'); + write(`${ACTOR_PATH}/.actor/README.md`); + write(`${ACTOR_PATH}/.actorignore`, 'Dockerfile.ci\n'); + write(`${ACTOR_PATH}/Dockerfile`, 'FROM apify/actor-node:20\n'); + write(`${ACTOR_PATH}/Dockerfile.ci`, 'FROM apify/actor-node:20\n'); + write(`${ACTOR_PATH}/src/index.js`); + write('shared/Dockerfile', 'FROM apify/actor-node:20\n'); + write('shared/input_schema.json', '{}'); + symlinkSync(join(root, 'shared', 'input_schema.json'), join(actorDir, '.actor', 'linked_schema.json')); + uploadedFiles = await getActorLocalFilePaths(actorDir); + }); + + afterAll(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('passes files inside the upload root, next to actor.json or above it', () => { + expect(check({ input: './INPUT_SCHEMA.json', readme: './README.md', dockerfile: '../Dockerfile' })).toEqual([]); + expect(check({ input: 'INPUT_SCHEMA.json' })).toEqual([]); + }); + + it('does nothing for a config without path fields', () => { + expect(check({ name: 'x', version: '0.0', environmentVariables: { A: '1' } })).toEqual([]); + expect(check({})).toEqual([]); + }); + + it('skips inline schema objects', () => { + expect( + check({ + input: { title: 'Input', type: 'object', properties: {} }, + storages: { dataset: { fields: {} }, datasets: { main: { fields: {} } } }, + }), + ).toEqual([]); + }); + + it('rejects a path above the upload root', () => { + expect(check({ dockerfile: '../../../shared/Dockerfile' })).toEqual([ + { + field: 'dockerfile', + value: '../../../shared/Dockerfile', + resolvedPath: join(root, 'shared', 'Dockerfile'), + reason: `lies outside the uploaded folder ${actorDir}`, + }, + ]); + }); + + it('rejects an absolute path', () => { + const value = resolve('/x'); + + expect(check({ input: value })).toEqual([ + { field: 'input', value, resolvedPath: value, reason: 'is absolute; the path is relative to .actor/' }, + ]); + }); + + it('rejects a missing file', () => { + expect(check({ input: './missing.json' })).toEqual([ + { + field: 'input', + value: './missing.json', + resolvedPath: join(actorDir, '.actor', 'missing.json'), + reason: 'does not exist', + }, + ]); + }); + + it('rejects a directory', () => { + expect(check({ dockerfile: '../src' })).toEqual([ + { + field: 'dockerfile', + value: '../src', + resolvedPath: join(actorDir, 'src'), + reason: 'is a directory, not a file', + }, + ]); + }); + + it('rejects a file push does not upload', () => { + expect(check({ dockerfile: '../Dockerfile.ci' })).toEqual([ + { + field: 'dockerfile', + value: '../Dockerfile.ci', + resolvedPath: join(actorDir, 'Dockerfile.ci'), + reason: 'is not among the files apify push uploads (excluded by .gitignore or .actorignore?)', + }, + ]); + }); + + it('rejects a symlink to a file outside the upload root', () => { + expect(check({ input: './linked_schema.json' })).toEqual([ + { + field: 'input', + value: './linked_schema.json', + resolvedPath: join(actorDir, '.actor', 'linked_schema.json'), + reason: `lies outside the uploaded folder ${actorDir} (through a symlink)`, + }, + ]); + }); + + it('matches the name of dockerfile, readme and changelog case-insensitively, and a schema exactly', () => { + expect(check({ dockerfile: '../dockerfile', readme: './readme.MD' })).toEqual([]); + expect(check({ input: './input_schema.json' })).toEqual([ + { + field: 'input', + value: './input_schema.json', + resolvedPath: join(actorDir, '.actor', 'input_schema.json'), + reason: 'does not exist (the name must match INPUT_SCHEMA.json exactly)', + }, + ]); + }); + + it('checks every string-valued schema reference', () => { + expect( + check({ + output: './output.json', + webServerSchema: './openapi.json', + storages: { keyValueStore: './kvs.json', dataset: './dataset.json', datasets: { main: './main.json' } }, + }).map((problem) => problem.field), + ).toEqual(['output', 'webServerSchema', 'storages.keyValueStore', 'storages.dataset', 'storages.datasets.main']); + }); + + it('requires dockerfile inside the Docker context when one is given', () => { + const monorepo = { uploadRoot: root, dockerContextRoot: join(root, 'shared') }; + uploadedFiles = [`${ACTOR_PATH}/Dockerfile`, `${ACTOR_PATH}/.actor/actor.json`, 'shared/Dockerfile']; + + expect(check({ dockerfile: '../../../shared/Dockerfile' }, monorepo)).toEqual([]); + expect(check({ dockerfile: '../Dockerfile' }, monorepo)).toEqual([ + { + field: 'dockerfile', + value: '../Dockerfile', + resolvedPath: join(actorDir, 'Dockerfile'), + reason: `lies outside the Docker context ${join(root, 'shared')}`, + }, + ]); + expect(check({ dockerfile: '../Dockerfile' }, { uploadRoot: root, dockerContextRoot: root })).toEqual([]); + }); +}); + +describe('formatActorPathProblems', () => { + it('lists every problem with its field, value, resolved path and reason', () => { + const message = formatActorPathProblems([ + { + field: 'dockerfile', + value: '../Dockerfile.prod', + resolvedPath: '/actor/Dockerfile.prod', + reason: 'does not exist', + }, + { field: 'input', value: '/schema.json', resolvedPath: '/schema.json', reason: 'is absolute' }, + ]); + + expect(message).toBe( + [ + '.actor/actor.json refers to files the Apify platform could not use:', + ' "dockerfile": "../Dockerfile.prod" (/actor/Dockerfile.prod) does not exist.', + ' "input": "/schema.json" (/schema.json) is absolute.', + 'Paths are relative to .actor/ and must point to files inside the uploaded folder.', + ].join('\n'), + ); + }); +}); From 2ac0be952a18bf499fbd561fa45333ca593bc194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 10:01:40 +0200 Subject: [PATCH 05/42] test: pass the monorepo file list to the Docker context case instead of mutating the shared one --- test/local/lib/actor-paths.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index c0ca4305e..e7dcc87a6 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -18,8 +18,10 @@ describe('checkActorPaths', () => { }; // The Actor folder is what an ordinary push uploads. - const check = (config: Record, options: { uploadRoot?: string; dockerContextRoot?: string } = {}) => - checkActorPaths({ actorDir, uploadRoot: actorDir, uploadedFiles, config, ...options }); + const check = ( + config: Record, + options: { uploadRoot?: string; uploadedFiles?: string[]; dockerContextRoot?: string } = {}, + ) => checkActorPaths({ actorDir, uploadRoot: actorDir, uploadedFiles, config, ...options }); beforeAll(async () => { root = mkdtempSync(join(tmpdir(), 'apify-cli-actor-paths-')); @@ -146,8 +148,8 @@ describe('checkActorPaths', () => { }); it('requires dockerfile inside the Docker context when one is given', () => { - const monorepo = { uploadRoot: root, dockerContextRoot: join(root, 'shared') }; - uploadedFiles = [`${ACTOR_PATH}/Dockerfile`, `${ACTOR_PATH}/.actor/actor.json`, 'shared/Dockerfile']; + const monorepoFiles = [`${ACTOR_PATH}/Dockerfile`, `${ACTOR_PATH}/.actor/actor.json`, 'shared/Dockerfile']; + const monorepo = { uploadRoot: root, uploadedFiles: monorepoFiles, dockerContextRoot: join(root, 'shared') }; expect(check({ dockerfile: '../../../shared/Dockerfile' }, monorepo)).toEqual([]); expect(check({ dockerfile: '../Dockerfile' }, monorepo)).toEqual([ @@ -158,7 +160,7 @@ describe('checkActorPaths', () => { reason: `lies outside the Docker context ${join(root, 'shared')}`, }, ]); - expect(check({ dockerfile: '../Dockerfile' }, { uploadRoot: root, dockerContextRoot: root })).toEqual([]); + expect(check({ dockerfile: '../Dockerfile' }, { ...monorepo, dockerContextRoot: root })).toEqual([]); }); }); From 8e0236979d314cb3c7ba1d66883e78ce781446fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 10:10:31 +0200 Subject: [PATCH 06/42] feat: push monorepo Actors by uploading the repository root with actorPath --- src/commands/actors/push.ts | 141 +++++++++++++++++++++++++----------- 1 file changed, 98 insertions(+), 43 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 4d50dbb1d..b6ba318ee 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -1,8 +1,10 @@ +import { randomUUID } from 'node:crypto'; import { readFileSync, statSync, unlinkSync } from 'node:fs'; +import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import process from 'node:process'; -import type { Actor, ActorCollectionCreateOptions, ActorDefaultRunOptions } from 'apify-client'; +import type { Actor, ActorCollectionCreateOptions, ActorDefaultRunOptions, ApifyApiError } from 'apify-client'; import open from 'open'; import { fetchManifest } from '@apify/actor-templates'; @@ -20,7 +22,13 @@ import { Args } from '../../lib/command-framework/args.js'; import { Flags } from '../../lib/command-framework/flags.js'; import { getConsoleUrl } from '../../lib/console-url.js'; import { CommandExitCodes, DEPRECATED_LOCAL_CONFIG_NAME, LOCAL_CONFIG_PATH } from '../../lib/consts.js'; -import { monorepoUnsupportedMessage, resolveDockerContext } from '../../lib/docker-context.js'; +import { + getContextFilePaths, + monorepoUnsupportedMessage, + readGitProvenance, + resolveDockerContext, + toPosix, +} from '../../lib/docker-context.js'; import { sumFilesSizeInBytes } from '../../lib/files.js'; import { useAbortJobOnSignal } from '../../lib/hooks/useAbortJobOnSignal.js'; import { useActorConfig } from '../../lib/hooks/useActorConfig.js'; @@ -38,7 +46,6 @@ import { printJsonToStdout, } from '../../lib/utils.js'; -const TEMP_ZIP_FILE_NAME = 'temp_file.zip'; const DEFAULT_RUN_OPTIONS = { build: 'latest', memoryMbytes: 4096, @@ -155,6 +162,12 @@ const confirmGitSourceSwitch = async ({ }); }; +/** A platform without the `actorPath` version field rejects the key with a schema error. */ +const rejectsActorPath = (err: unknown) => { + const { statusCode, message } = err as ApifyApiError; + return statusCode === 400 && typeof message === 'string' && message.includes('actorPath'); +}; + export class ActorsPushCommand extends ApifyCommand { static override name = 'push' as const; @@ -290,25 +303,30 @@ export class ActorsPushCommand extends ApifyCommand { const { config: actorConfig } = actorConfigResult.unwrap(); - // A monorepo Actor builds from a Docker context above its folder, which a push of that folder cannot carry. + // A monorepo Actor builds from a Docker context above its folder, so the push uploads its repository + // (or, outside Git, the context) and tells the platform where the Actor lies in it. const dockerContext = resolveDockerContext(cwd, actorConfig?.dockerContextDir); if (dockerContext.kind === 'invalid') { error({ message: dockerContext.message }); process.exitCode = CommandExitCodes.InvalidActorJson; return; } + let uploadRoot = cwd; + let uploadedFiles = filePathsToPush; + let actorPath: string | undefined; if (dockerContext.kind === 'context') { - error({ message: monorepoUnsupportedMessage(actorConfig!.name as string) }); - process.exitCode = CommandExitCodes.NotImplemented; - return; + uploadRoot = dockerContext.sourceRoot; + uploadedFiles = (await getContextFilePaths(uploadRoot)).map(toPosix); + actorPath = dockerContext.actorPath; } // The platform resolves these paths after the upload; a wrong one builds the wrong image or fails the build. const pathProblems = checkActorPaths({ actorDir: cwd, - uploadRoot: cwd, - uploadedFiles: filePathsToPush, + uploadRoot, + uploadedFiles, config: actorConfig, + dockerContextRoot: dockerContext.kind === 'context' ? dockerContext.contextRoot : undefined, }); if (pathProblems.length > 0) { error({ message: formatActorPathProblems(pathProblems) }); @@ -316,6 +334,17 @@ export class ActorsPushCommand extends ApifyCommand { return; } + if (actorPath && readGitProvenance(uploadRoot).dirty) { + warning({ + message: `The repository ${uploadRoot} has uncommitted changes; the push uploads the files as they are.`, + }); + } + + const refuseMonorepoPush = () => { + error({ message: monorepoUnsupportedMessage(actorConfig!.name as string) }); + process.exitCode = CommandExitCodes.NotImplemented; + }; + const userInfo = await getCurrentUserInfo(); const isOrganizationLoggedIn = !!userInfo.organizationOwnerUserId; const redirectUrlPart = isOrganizationLoggedIn ? `/organization/${userInfo.id}` : ''; @@ -368,6 +397,7 @@ export class ActorsPushCommand extends ApifyCommand { // TODO: export enum from apify-client sourceType: ACTOR_SOURCE_TYPES.SOURCE_FILES as never, sourceFiles: [], + ...(actorPath ? { actorPath } : {}), }, ], }; @@ -377,7 +407,13 @@ export class ActorsPushCommand extends ApifyCommand { newActor.actorStandby = { isEnabled: true }; } - actor = await apifyClient.actors().create(newActor); + try { + actor = await apifyClient.actors().create(newActor); + } catch (err) { + if (!rejectsActorPath(err)) throw err; + refuseMonorepoPush(); + return; + } actorId = actor.id; isActorCreatedNow = true; info({ message: `Created Actor with name ${actorConfig!.name} on Apify.` }); @@ -393,14 +429,14 @@ export class ActorsPushCommand extends ApifyCommand { info({ message: `Deploying Actor '${actorConfig!.name}' to Apify.` }); - const filesSize = await sumFilesSizeInBytes(filePathsToPush, cwd); + const filesSize = await sumFilesSizeInBytes(uploadedFiles, uploadRoot); - if (filesSize < MAX_MULTIFILE_BYTES && !isActorCreatedNow) { + if ((filesSize < MAX_MULTIFILE_BYTES || actorPath) && !isActorCreatedNow) { const client = await actorClient.get(); // Check when was files modified last - const mostRecentModifiedFileMs = filePathsToPush.reduce((modifiedMs, filePath) => { - const { mtimeMs, ctimeMs } = statSync(join(cwd, filePath)); + const mostRecentModifiedFileMs = uploadedFiles.reduce((modifiedMs, filePath) => { + const { mtimeMs, ctimeMs } = statSync(join(uploadRoot, filePath)); // Sometimes it's possible mtimeMs is some messed up value (like 2000/01/01 midnight), then we want to check created if it's newer const fileModifiedMs = mtimeMs > ctimeMs ? mtimeMs : ctimeMs; @@ -443,24 +479,28 @@ Skipping push. Use --force to override.`, let sourceFiles; let tarballUrl; if (filesSize < MAX_MULTIFILE_BYTES) { - sourceFiles = await createSourceFiles(filePathsToPush, cwd); + sourceFiles = await createSourceFiles(uploadedFiles, uploadRoot); sourceType = ACTOR_SOURCE_TYPES.SOURCE_FILES; } else { // Create zip run({ message: 'Zipping Actor files' }); - await createActZip(TEMP_ZIP_FILE_NAME, filePathsToPush, cwd); + const zipPath = join(tmpdir(), `apify-push-${randomUUID()}.zip`); + await createActZip(zipPath, uploadedFiles, uploadRoot); // Upload it to Apify.keyValueStores const store = await apifyClient.keyValueStores().getOrCreate(`actor-${actorId}-source`); const key = `version-${version}.zip`; - const buffer = readFileSync(TEMP_ZIP_FILE_NAME); - await apifyClient.keyValueStore(store.id).setRecord({ - key, - // TODO: fix this type too - value: buffer as never, - contentType: 'application/zip', - }); - unlinkSync(TEMP_ZIP_FILE_NAME); + try { + const buffer = readFileSync(zipPath); + await apifyClient.keyValueStore(store.id).setRecord({ + key, + // TODO: fix this type too + value: buffer as never, + contentType: 'application/zip', + }); + } finally { + unlinkSync(zipPath); + } const tempTarballUrl = new URL( `${apifyClient.baseUrl}/key-value-stores/${store.id}/records/${key}?disableRedirect=true`, ); @@ -493,27 +533,42 @@ Skipping push. Use --force to override.`, const applyEnvVarsToBuild = this.flags.applyEnvVarsToBuild ?? (actorConfig!.applyEnvVarsToBuild as boolean | undefined); - if (actorCurrentVersion) { - const actorVersionModifier = { tarballUrl, sourceFiles, buildTag, sourceType, envVars, applyEnvVarsToBuild }; - // TODO: fix this type too -.- - await actorClient.version(version).update(actorVersionModifier as never); - run({ message: `Updated version ${version} for Actor ${actor.name}.` }); - } else { - const actorNewVersion = { - versionNumber: version, - tarballUrl, - sourceFiles, - buildTag, - sourceType, - envVars, - applyEnvVarsToBuild, - }; + const actorVersionSource = { tarballUrl, sourceFiles, sourceType, ...(actorPath ? { actorPath } : {}) }; + try { + if (actorCurrentVersion) { + const actorVersionModifier = { ...actorVersionSource, buildTag, envVars, applyEnvVarsToBuild }; + // TODO: fix this type too -.- + await actorClient.version(version).update(actorVersionModifier as never); + run({ message: `Updated version ${version} for Actor ${actor.name}.` }); + } else { + const actorNewVersion = { + versionNumber: version, + ...actorVersionSource, + buildTag, + envVars, + applyEnvVarsToBuild, + }; - await actorClient.versions().create({ - ...actorNewVersion, - } as never); + await actorClient.versions().create({ + ...actorNewVersion, + } as never); - run({ message: `Created version ${version} for Actor ${actor.name}.` }); + run({ message: `Created version ${version} for Actor ${actor.name}.` }); + } + } catch (err) { + if (!rejectsActorPath(err)) throw err; + if (isActorCreatedNow) { + try { + await actorClient.delete(); + info({ message: `Deleted the just created Actor ${actor.name} again.` }); + } catch (deleteErr) { + warning({ + message: `Could not delete the just created Actor ${actor.name}: ${(deleteErr as Error).message}`, + }); + } + } + refuseMonorepoPush(); + return; } // Sync standby mode on existing actors with actor.json From 87c374b485f64dfb8489a548812598b23aeb654e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 10:10:32 +0200 Subject: [PATCH 07/42] test: cover the monorepo upload, the zip path, and the actorPath skew guard --- test/local/commands/push-monorepo.test.ts | 247 ++++++++++++++++++++-- 1 file changed, 224 insertions(+), 23 deletions(-) diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index 6d3eab22d..b92a59100 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -1,8 +1,9 @@ import { execFileSync } from 'node:child_process'; -import { mkdir, writeFile } from 'node:fs/promises'; -import { dirname } from 'node:path'; +import { existsSync } from 'node:fs'; +import { mkdir, rm, writeFile } from 'node:fs/promises'; +import { dirname, join } from 'node:path'; -import { ACTOR_SOURCE_TYPES } from '@apify/consts'; +import { ACTOR_SOURCE_TYPES, MAX_MULTIFILE_BYTES } from '@apify/consts'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; import { CommandExitCodes } from '../../../src/lib/consts.js'; @@ -25,10 +26,27 @@ const actor = { const build = { id: 'build1', actId: ACTOR_ID, buildNumber: '0.0.1', status: 'SUCCEEDED' }; const currentVersion = { versionNumber: '0.0', sourceType: ACTOR_SOURCE_TYPES.SOURCE_FILES }; -const actorGet = vitest.fn(async () => actor); +interface VersionPayload { + sourceType: string; + sourceFiles?: { name: string }[]; + tarballUrl?: string; + actorPath?: string; +} + +const actorGet = vitest.fn(async (): Promise => actor); +const actorDelete = vitest.fn(async () => {}); +const actorsCreate = vitest.fn(async (_created: unknown) => actor); const versionUpdate = vitest.fn(async (_modifier: unknown) => ({})); const versionCreate = vitest.fn(async (created: unknown) => created); -const keyValueStoreGetOrCreate = vitest.fn(async () => ({ id: 'store1' })); +const keyValueStoreGetOrCreate = vitest.fn(async () => ({ id: 'store1', urlSigningSecretKey: 'secret' })); +const setRecord = vitest.fn(async (_record: unknown) => {}); + +// What apify-client throws when a platform without the `actorPath` version field rejects the key. +const actorPathRejected = () => + Object.assign(new Error('Invalid actor version: actorPath is not allowed by the schema'), { + statusCode: 400, + type: 'invalid-request', + }); // Stands in for the platform: the Actor and its version exist, and every write is recorded. vitest.mock('../../../src/lib/utils.js', async (importOriginal) => ({ @@ -40,20 +58,43 @@ vitest.mock('../../../src/lib/utils.js', async (importOriginal) => ({ token: 'my-token', actor: () => ({ get: actorGet, + delete: actorDelete, version: () => ({ get: async () => currentVersion, update: versionUpdate }), versions: () => ({ create: versionCreate }), build: async () => build, }), + actors: () => ({ create: actorsCreate }), keyValueStores: () => ({ getOrCreate: keyValueStoreGetOrCreate }), + keyValueStore: () => ({ setRecord }), build: () => ({ get: async () => build }), })), })); +// Creating an Actor looks its template up in the manifest; nothing here needs the network. +vitest.mock('@apify/actor-templates', () => ({ fetchManifest: async () => ({ templates: [] }) })); + // The mocked `node:process` copy has no signal handlers; nothing here needs them. vitest.mock('../../../src/lib/hooks/useAbortJobOnSignal.js', () => ({ useAbortJobOnSignal: () => ({ [Symbol.dispose]() {} }), })); +// With `gitAvailable` off, every git call fails as it does without git, so the fixture counts as outside Git +// even though it lives inside this repository's checkout. +const gitState = vitest.hoisted(() => ({ gitAvailable: true })); +vitest.mock('node:child_process', async (importOriginal) => { + const original = await importOriginal(); + const unavailable = () => { + throw new Error('not a git repository'); + }; + return { + ...original, + execSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execSync(...args) : unavailable()) as typeof original.execSync, + execFileSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execFileSync(...args) : unavailable()) as typeof original.execFileSync, + }; +}); + const { beforeAllCalls, afterAllCalls, joinPath, tmpPath, forceNewCwd } = useTempPath(actName, { create: true, remove: true, @@ -83,14 +124,31 @@ function git(...args: string[]) { execFileSync('git', args, { cwd: tmpPath, stdio: 'ignore' }); } +function versionPayload(mock: typeof versionUpdate | typeof actorsCreate) { + expect(mock).toHaveBeenCalledTimes(1); + return mock.mock.calls[0][0] as VersionPayload; +} + +function expectRefusal() { + expect(commandProcess.exitCode).toBe(CommandExitCodes.NotImplemented); + const stderr = logMessages.error.join('\n'); + expect(stderr).toContain('apify push does not support such Actors on the Apify platform yet'); + expect(stderr).toContain(MONOREPO_PLATFORM_SUPPORT_ISSUE_URL); + expect(logMessages.log.join('\n')).not.toContain('Apify push result'); +} + let originalExitCode: typeof process.exitCode; beforeEach(async () => { originalExitCode = commandProcess.exitCode; + gitState.gitAvailable = true; actorGet.mockClear(); + actorDelete.mockClear(); + actorsCreate.mockClear(); versionUpdate.mockClear(); versionCreate.mockClear(); keyValueStoreGetOrCreate.mockClear(); + setRecord.mockClear(); cwdCache.clear(); await beforeAllCalls(); @@ -115,30 +173,173 @@ afterEach(async () => { }); describe('apify push of a monorepo Actor', () => { - it('stops before touching the platform, pointing at the tracking issue', async () => { + it('uploads the repository root as source files with the Actor folder as actorPath', async () => { await testRunCommand(ActorsPushCommand, {}); - expect(commandProcess.exitCode).toBe(CommandExitCodes.NotImplemented); - const stderr = logMessages.error.join('\n'); - expect(stderr).toContain( - `Actor ${actName} sets "dockerContextDir" in .actor/actor.json, so it builds from a Docker context outside its own folder (a monorepo Actor).`, + expect(commandProcess.exitCode).toBeFalsy(); + const modifier = versionPayload(versionUpdate); + expect(modifier.sourceType).toBe(ACTOR_SOURCE_TYPES.SOURCE_FILES); + expect(modifier.actorPath).toBe(ACTOR_PATH); + expect(modifier.tarballUrl).toBeUndefined(); + const names = modifier.sourceFiles!.map((file) => file.name); + expect(names).toEqual( + expect.arrayContaining([ + `${ACTOR_PATH}/.actor/actor.json`, + `${ACTOR_PATH}/src/index.ts`, + 'package.json', + 'shared/TypeScript_Dockerfile', + 'packages/typescript-utils/src/index.ts', + ]), ); - expect(stderr).toContain('apify push does not support such Actors on the Apify platform yet'); - expect(stderr).toContain(MONOREPO_PLATFORM_SUPPORT_ISSUE_URL); - expect(stderr).toContain("set the Actor's source in Apify Console to the Git repository with the folder"); - expect(actorGet).not.toHaveBeenCalled(); - expect(versionCreate).not.toHaveBeenCalled(); - expect(versionUpdate).not.toHaveBeenCalled(); + expect(names.some((name) => name.startsWith('node_modules'))).toBe(false); + expect(names.some((name) => name.includes('\\'))).toBe(false); expect(keyValueStoreGetOrCreate).not.toHaveBeenCalled(); + expect(logMessages.error.join('\n')).not.toContain('uncommitted changes'); + expect(logMessages.log.join('\n')).toContain('Apify push result: SUCCEEDED'); }); - it('keeps stdout clean with --json', async () => { + it('keeps the --json envelope on stdout', async () => { await testRunCommand(ActorsPushCommand, { flags_json: true }); - expect(commandProcess.exitCode).toBe(CommandExitCodes.NotImplemented); - expect(logMessages.log).toEqual([]); - expect(logMessages.error.join('\n')).toContain(MONOREPO_PLATFORM_SUPPORT_ISSUE_URL); + expect(commandProcess.exitCode).toBeFalsy(); + expect(versionPayload(versionUpdate).actorPath).toBe(ACTOR_PATH); + expect(logMessages.log).toHaveLength(1); + expect(JSON.parse(logMessages.log[0])).toMatchObject({ ok: true, operation: 'push', actor: { id: ACTOR_ID } }); + }); + + it('zips a repository over the multi-file limit into the OS temp directory and pushes it as a tarball', async () => { + await write(`${ACTOR_PATH}/big.bin`, 'x'.repeat(MAX_MULTIFILE_BYTES + 1)); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(keyValueStoreGetOrCreate).toHaveBeenCalledWith(`actor-${ACTOR_ID}-source`); + expect(setRecord).toHaveBeenCalledTimes(1); + const record = setRecord.mock.calls[0][0] as { key: string; value: Buffer; contentType: string }; + expect(record.key).toBe('version-0.0.zip'); + expect(record.contentType).toBe('application/zip'); + expect(Buffer.isBuffer(record.value)).toBe(true); + expect(record.value.length).toBeGreaterThan(0); + + const modifier = versionPayload(versionUpdate); + expect(modifier.sourceType).toBe(ACTOR_SOURCE_TYPES.TARBALL); + expect(modifier.sourceFiles).toBeUndefined(); + expect(modifier.actorPath).toBe(ACTOR_PATH); + const tarballUrl = new URL(modifier.tarballUrl!); + expect(tarballUrl.pathname).toBe('/v2/key-value-stores/store1/records/version-0.0.zip'); + expect(tarballUrl.searchParams.get('disableRedirect')).toBe('true'); + expect(tarballUrl.searchParams.get('signature')).toBeTruthy(); + + expect(existsSync(join(process.cwd(), 'temp_file.zip'))).toBe(false); + expect(existsSync(joinPath('temp_file.zip'))).toBe(false); + }); + + it('warns about uncommitted changes, which the upload carries as they are', async () => { + await write(`${ACTOR_PATH}/src/index.ts`, 'console.log(2);\n'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(versionUpdate).toHaveBeenCalledTimes(1); + const warningLine = logMessages.error.find((line) => line.includes('uncommitted changes')); + expect(warningLine).toBeDefined(); + expect(warningLine!.split('\n')).toHaveLength(1); + }); + + describe('on a platform without the actorPath field', () => { + it('turns the 400 into the refusal with the tracking issue', async () => { + versionUpdate.mockRejectedValueOnce(actorPathRejected()); + + await testRunCommand(ActorsPushCommand, {}); + + expectRefusal(); + expect(actorDelete).not.toHaveBeenCalled(); + }); + + it('keeps stdout clean with --json', async () => { + versionUpdate.mockRejectedValueOnce(actorPathRejected()); + + await testRunCommand(ActorsPushCommand, { flags_json: true }); + + expectRefusal(); + expect(logMessages.log).toEqual([]); + }); + + it('deletes the Actor it has just created', async () => { + actorGet.mockResolvedValueOnce(null); + versionUpdate.mockRejectedValueOnce(actorPathRejected()); + + await testRunCommand(ActorsPushCommand, {}); + + expectRefusal(); + expect(versionPayload(actorsCreate)).toMatchObject({ versions: [{ actorPath: ACTOR_PATH }] }); + expect(actorDelete).toHaveBeenCalledTimes(1); + }); + + it('still reports the refusal when deleting the just created Actor fails', async () => { + actorGet.mockResolvedValueOnce(null); + versionUpdate.mockRejectedValueOnce(actorPathRejected()); + actorDelete.mockRejectedValueOnce(new Error('delete failed')); + + await testRunCommand(ActorsPushCommand, {}); + + expectRefusal(); + expect(actorDelete).toHaveBeenCalledTimes(1); + expect(logMessages.error.join('\n')).toContain('delete failed'); + }); + + it('refuses when the Actor create itself rejects actorPath, leaving nothing to delete', async () => { + actorGet.mockResolvedValueOnce(null); + actorsCreate.mockRejectedValueOnce(actorPathRejected()); + + await testRunCommand(ActorsPushCommand, {}); + + expectRefusal(); + expect(versionUpdate).not.toHaveBeenCalled(); + expect(actorDelete).not.toHaveBeenCalled(); + }); + + it('passes other API errors through', async () => { + versionUpdate.mockRejectedValueOnce(Object.assign(new Error('Version is locked'), { statusCode: 400 })); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(process.exitCode).toBe(1); + expect(logMessages.error.join('\n')).toContain('Version is locked'); + expect(logMessages.error.join('\n')).not.toContain(MONOREPO_PLATFORM_SUPPORT_ISSUE_URL); + }); + }); + + it('uploads the Docker context with .actorignore applied outside Git', async () => { + await rm(joinPath('.git'), { recursive: true, force: true }); + await write('.actorignore', 'packages/\n'); + gitState.gitAvailable = false; + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + const modifier = versionPayload(versionUpdate); + expect(modifier.sourceType).toBe(ACTOR_SOURCE_TYPES.SOURCE_FILES); + expect(modifier.actorPath).toBe(ACTOR_PATH); + const names = modifier.sourceFiles!.map((file) => file.name); + expect(names).toEqual( + expect.arrayContaining([`${ACTOR_PATH}/.actor/actor.json`, 'package.json', 'shared/TypeScript_Dockerfile']), + ); + expect(names.some((name) => name.startsWith('packages/'))).toBe(false); + expect(names.some((name) => name.startsWith('node_modules'))).toBe(false); + }); + + it('rejects a dockerfile outside the Docker context before touching the platform', async () => { + await writeActorConfig({ dockerContextDir: '../..', dockerfile: '../../../shared/TypeScript_Dockerfile' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.error.join('\n')).toContain('lies outside the Docker context'); + expect(actorGet).not.toHaveBeenCalled(); expect(versionUpdate).not.toHaveBeenCalled(); + expect(keyValueStoreGetOrCreate).not.toHaveBeenCalled(); }); it('rejects a dockerContextDir outside the Git repository, which the platform could not build', async () => { @@ -158,10 +359,10 @@ describe('apify push of a monorepo Actor', () => { await testRunCommand(ActorsPushCommand, {}); expect(commandProcess.exitCode).toBeFalsy(); - expect(versionUpdate).toHaveBeenCalledTimes(1); - const modifier = versionUpdate.mock.calls[0][0] as { sourceType: string; sourceFiles: { name: string }[] }; + const modifier = versionPayload(versionUpdate); expect(modifier.sourceType).toBe(ACTOR_SOURCE_TYPES.SOURCE_FILES); - expect(modifier.sourceFiles.map((file) => file.name).sort()).toEqual(['.actor/actor.json', 'src/index.ts']); + expect(modifier.sourceFiles!.map((file) => file.name).sort()).toEqual(['.actor/actor.json', 'src/index.ts']); + expect(modifier).not.toHaveProperty('actorPath'); expect(logMessages.log.join('\n')).toContain('Apify push result: SUCCEEDED'); }); }); From e330a0f2c6feaac21070dd5287f10cf85eb9648e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 10:18:55 +0200 Subject: [PATCH 08/42] fix: upload a monorepo Actor's directory symlinks as the files under them --- src/commands/actors/push.ts | 26 ++++++++++++++++++++--- test/local/commands/push-monorepo.test.ts | 17 ++++++++++++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index b6ba318ee..cd5d800a4 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -1,7 +1,7 @@ import { randomUUID } from 'node:crypto'; -import { readFileSync, statSync, unlinkSync } from 'node:fs'; +import { readFileSync, realpathSync, statSync, unlinkSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join, resolve } from 'node:path'; +import { join, relative, resolve } from 'node:path'; import process from 'node:process'; import type { Actor, ActorCollectionCreateOptions, ActorDefaultRunOptions, ApifyApiError } from 'apify-client'; @@ -24,6 +24,7 @@ import { getConsoleUrl } from '../../lib/console-url.js'; import { CommandExitCodes, DEPRECATED_LOCAL_CONFIG_NAME, LOCAL_CONFIG_PATH } from '../../lib/consts.js'; import { getContextFilePaths, + isOutside, monorepoUnsupportedMessage, readGitProvenance, resolveDockerContext, @@ -168,6 +169,25 @@ const rejectsActorPath = (err: unknown) => { return statusCode === 400 && typeof message === 'string' && message.includes('actorPath'); }; +/** + * The platform's multi-file and zip sources carry files only: a link to a file goes as that file, a link to a + * directory inside the upload as copies of the uploaded files under its target, and a link to nothing is left out. + */ +const resolveLinksForUpload = (root: string, files: string[]) => { + const stats = new Map(files.map((file) => [file, statSync(join(root, file), { throwIfNoEntry: false })])); + return files.flatMap((file) => { + const fileStats = stats.get(file); + if (fileStats?.isFile()) return [file]; + if (!fileStats?.isDirectory()) return []; + const target = relative(realpathSync(root), realpathSync(join(root, file))); + if (isOutside(target)) return []; + const prefix = `${toPosix(target)}/`; + return files + .filter((other) => other.startsWith(prefix) && stats.get(other)?.isFile()) + .map((other) => `${file}/${other.slice(prefix.length)}`); + }); +}; + export class ActorsPushCommand extends ApifyCommand { static override name = 'push' as const; @@ -316,7 +336,7 @@ export class ActorsPushCommand extends ApifyCommand { let actorPath: string | undefined; if (dockerContext.kind === 'context') { uploadRoot = dockerContext.sourceRoot; - uploadedFiles = (await getContextFilePaths(uploadRoot)).map(toPosix); + uploadedFiles = resolveLinksForUpload(uploadRoot, (await getContextFilePaths(uploadRoot)).map(toPosix)); actorPath = dockerContext.actorPath; } diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index b92a59100..701b528aa 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process'; -import { existsSync } from 'node:fs'; +import { existsSync, symlinkSync } from 'node:fs'; import { mkdir, rm, writeFile } from 'node:fs/promises'; import { dirname, join } from 'node:path'; @@ -234,6 +234,21 @@ describe('apify push of a monorepo Actor', () => { expect(existsSync(joinPath('temp_file.zip'))).toBe(false); }); + it('uploads the files under a directory symlink as copies and leaves a dangling link out', async () => { + symlinkSync('../../shared', joinPath(`${ACTOR_PATH}/shared`)); + symlinkSync('missing', joinPath('dangling')); + git('add', '.'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + const names = versionPayload(versionUpdate).sourceFiles!.map((file) => file.name); + expect(names).toContain(`${ACTOR_PATH}/shared/TypeScript_Dockerfile`); + expect(names).toContain('shared/TypeScript_Dockerfile'); + expect(names).not.toContain(`${ACTOR_PATH}/shared`); + expect(names).not.toContain('dangling'); + }); + it('warns about uncommitted changes, which the upload carries as they are', async () => { await write(`${ACTOR_PATH}/src/index.ts`, 'console.log(2);\n'); From 128e96abbde8b37f929a53f814f979a5abb25d1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 16 Oct 2026 10:18:56 +0200 Subject: [PATCH 09/42] test: restore the global exit code after each monorepo push case --- test/local/commands/push-monorepo.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index 701b528aa..2772298b3 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -138,9 +138,11 @@ function expectRefusal() { } let originalExitCode: typeof process.exitCode; +let originalGlobalExitCode: typeof process.exitCode; beforeEach(async () => { originalExitCode = commandProcess.exitCode; + originalGlobalExitCode = process.exitCode; gitState.gitAvailable = true; actorGet.mockClear(); actorDelete.mockClear(); @@ -169,6 +171,7 @@ beforeEach(async () => { afterEach(async () => { commandProcess.exitCode = originalExitCode; + process.exitCode = originalGlobalExitCode; await afterAllCalls(); }); From 8312f663c7c9a5833ed991072a9a229b1bdfca29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 12:27:04 +0100 Subject: [PATCH 10/42] fix: resolve the Actor directory's real path and require a directory as the Docker context --- src/lib/docker-context.ts | 40 +++++++--- test/local/lib/docker-context.test.ts | 103 ++++++++++++++++++-------- 2 files changed, 102 insertions(+), 41 deletions(-) diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts index eb7650219..2dcf0e8c6 100644 --- a/src/lib/docker-context.ts +++ b/src/lib/docker-context.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process'; -import { existsSync, lstatSync, realpathSync } from 'node:fs'; +import { lstatSync, realpathSync, statSync } from 'node:fs'; import { isAbsolute, join, relative, resolve, sep } from 'node:path'; import { ACTOR_SPECIFICATION_FOLDER } from './consts.js'; @@ -30,45 +30,61 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown return { kind: 'invalid', message: `"dockerContextDir" in .actor/actor.json must be a string.` }; } - const contextRoot = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, dockerContextDir); - const contextPathInActor = relative(actorDir, contextRoot); + // The platform resolves the context from the Actor folder's real path, so a symlinked `--dir` must too. + let realActorDir: string; + try { + realActorDir = realpathSync(actorDir); + } catch { + return { kind: 'invalid', message: `The Actor folder ${actorDir} does not exist.` }; + } + + const contextRoot = resolve(realActorDir, ACTOR_SPECIFICATION_FOLDER, dockerContextDir); + const contextPathInActor = relative(realActorDir, contextRoot); if (contextPathInActor === '' || !isOutside(contextPathInActor)) return { kind: 'none' }; - if (!existsSync(contextRoot)) { + const contextStats = statSync(contextRoot, { throwIfNoEntry: false }); + if (!contextStats) { return { kind: 'invalid', message: `"dockerContextDir" in .actor/actor.json points to ${contextRoot}, which does not exist.`, }; } + if (!contextStats.isDirectory()) { + return { + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${contextRoot}, which is not a directory.`, + }; + } + const realContextRoot = realpathSync(contextRoot); // The platform's clone holds nothing above the repository root. - const repoRoot = git(actorDir, ['rev-parse', '--show-toplevel']); + const repoRoot = git(realActorDir, ['rev-parse', '--show-toplevel']); if (repoRoot) { const sourceRoot = realpathSync(repoRoot); - if (isOutside(relative(sourceRoot, realpathSync(contextRoot)))) { + if (isOutside(relative(sourceRoot, realContextRoot))) { return { kind: 'invalid', message: - `"dockerContextDir" in .actor/actor.json points to ${contextRoot}, outside the Git repository ${repoRoot}. ` + + `"dockerContextDir" in .actor/actor.json points to ${realContextRoot}, outside the Git repository ${sourceRoot}. ` + 'The Apify platform builds from a clone of the repository, so it could not build this Actor.', }; } return { kind: 'context', - contextRoot, + contextRoot: realContextRoot, sourceRoot, - actorPath: toPosix(relative(sourceRoot, realpathSync(actorDir))), + actorPath: toPosix(relative(sourceRoot, realActorDir)), }; } - const actorPath = relative(contextRoot, actorDir); + const actorPath = relative(realContextRoot, realActorDir); if (isOutside(actorPath)) { return { kind: 'invalid', - message: `"dockerContextDir" in .actor/actor.json points to ${contextRoot}, which does not contain the Actor's folder.`, + message: `"dockerContextDir" in .actor/actor.json points to ${realContextRoot}, which does not contain the Actor's folder.`, }; } - return { kind: 'context', contextRoot, sourceRoot: contextRoot, actorPath: toPosix(actorPath) }; + return { kind: 'context', contextRoot: realContextRoot, sourceRoot: realContextRoot, actorPath: toPosix(actorPath) }; } function isOutside(relativePath: string): boolean { diff --git a/test/local/lib/docker-context.test.ts b/test/local/lib/docker-context.test.ts index 4b1477b3e..180487b84 100644 --- a/test/local/lib/docker-context.test.ts +++ b/test/local/lib/docker-context.test.ts @@ -1,11 +1,27 @@ import { execFileSync } from 'node:child_process'; -import { mkdirSync, mkdtempSync, realpathSync, rmSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join, resolve } from 'node:path'; +import { mkdirSync, realpathSync, symlinkSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; import { resolveDockerContext } from '../../../src/lib/docker-context.js'; import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; +// vi.mock is hoisted before imports, so the resolver gets the mocked version. With `gitAvailable` off, every git +// call fails as it does without git, which sends the resolver down its non-Git branch. +const gitState = vi.hoisted(() => ({ gitAvailable: true })); +vi.mock('node:child_process', async (importOriginal) => { + const original = await importOriginal(); + const unavailable = () => { + throw new Error('not a git repository'); + }; + return { + ...original, + execSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execSync(...args) : unavailable()) as typeof original.execSync, + execFileSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execFileSync(...args) : unavailable()) as typeof original.execFileSync, + }; +}); + const ACTOR_PATH = 'actors/x'; describe('resolveDockerContext', () => { @@ -17,6 +33,8 @@ describe('resolveDockerContext', () => { cwdParent: false, }); + // The resolver works from real paths, so the expectations do too. + let root: string; const actorDir = () => joinPath(ACTOR_PATH); beforeAll(async () => { @@ -24,7 +42,10 @@ describe('resolveDockerContext', () => { mkdirSync(joinPath(ACTOR_PATH, '.actor'), { recursive: true }); mkdirSync(joinPath(ACTOR_PATH, 'src'), { recursive: true }); mkdirSync(joinPath('shared'), { recursive: true }); + writeFileSync(joinPath('Dockerfile'), 'FROM apify/actor-node:20\n'); + symlinkSync(ACTOR_PATH, joinPath('linked-actor')); execFileSync('git', ['init', '--quiet'], { cwd: tmpPath, stdio: 'ignore' }); + root = realpathSync(tmpPath); }); afterAll(async () => { @@ -42,8 +63,8 @@ describe('resolveDockerContext', () => { it('resolves a context above the Actor folder to the repository root and the Actor path in it', () => { expect(resolveDockerContext(actorDir(), '../../..')).toEqual({ kind: 'context', - contextRoot: tmpPath, - sourceRoot: realpathSync(tmpPath), + contextRoot: root, + sourceRoot: root, actorPath: ACTOR_PATH, }); }); @@ -51,22 +72,26 @@ describe('resolveDockerContext', () => { it('keeps the repository root as the source for a context elsewhere in the repository', () => { expect(resolveDockerContext(actorDir(), '../../../shared')).toEqual({ kind: 'context', - contextRoot: joinPath('shared'), - sourceRoot: realpathSync(tmpPath), + contextRoot: join(root, 'shared'), + sourceRoot: root, actorPath: ACTOR_PATH, }); }); - it('rejects a context outside the Git repository', () => { - const repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { - cwd: actorDir(), - encoding: 'utf-8', - }).trim(); + it('resolves a symlinked Actor folder from its real path, as the platform does', () => { + expect(resolveDockerContext(joinPath('linked-actor'), '../../..')).toEqual({ + kind: 'context', + contextRoot: root, + sourceRoot: root, + actorPath: ACTOR_PATH, + }); + }); + it('rejects a context outside the Git repository', () => { expect(resolveDockerContext(actorDir(), '../../../..')).toEqual({ kind: 'invalid', message: - `"dockerContextDir" in .actor/actor.json points to ${resolve(tmpPath, '..')}, outside the Git repository ${repoRoot}. ` + + `"dockerContextDir" in .actor/actor.json points to ${dirname(root)}, outside the Git repository ${root}. ` + 'The Apify platform builds from a clone of the repository, so it could not build this Actor.', }); }); @@ -74,7 +99,21 @@ describe('resolveDockerContext', () => { it('rejects a context that does not exist', () => { expect(resolveDockerContext(actorDir(), '../../../missing')).toEqual({ kind: 'invalid', - message: `"dockerContextDir" in .actor/actor.json points to ${joinPath('missing')}, which does not exist.`, + message: `"dockerContextDir" in .actor/actor.json points to ${join(root, 'missing')}, which does not exist.`, + }); + }); + + it('rejects a context that is not a directory', () => { + expect(resolveDockerContext(actorDir(), '../../../Dockerfile')).toEqual({ + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${join(root, 'Dockerfile')}, which is not a directory.`, + }); + }); + + it('rejects an Actor folder that does not exist', () => { + expect(resolveDockerContext(joinPath('missing-actor'), '../../..')).toEqual({ + kind: 'invalid', + message: `The Actor folder ${joinPath('missing-actor')} does not exist.`, }); }); @@ -87,22 +126,31 @@ describe('resolveDockerContext', () => { }); describe('outside a Git repository', () => { + const { tmpPath, joinPath, beforeAllCalls, afterAllCalls } = useTempPath('docker-context-no-git-dir', { + create: true, + remove: true, + cwd: false, + cwdParent: false, + }); + let root: string; - let actorDir: string; + const actorDir = () => joinPath(ACTOR_PATH); - beforeAll(() => { - root = mkdtempSync(join(tmpdir(), 'apify-cli-docker-context-')); - actorDir = join(root, ACTOR_PATH); - mkdirSync(join(actorDir, '.actor'), { recursive: true }); - mkdirSync(join(root, 'other'), { recursive: true }); + beforeAll(async () => { + await beforeAllCalls(); + mkdirSync(joinPath(ACTOR_PATH, '.actor'), { recursive: true }); + mkdirSync(joinPath('other'), { recursive: true }); + root = realpathSync(tmpPath); + gitState.gitAvailable = false; }); - afterAll(() => { - rmSync(root, { recursive: true, force: true }); + afterAll(async () => { + gitState.gitAvailable = true; + await afterAllCalls(); }); it('makes the context the source when it contains the Actor folder', () => { - expect(resolveDockerContext(actorDir, '../../..')).toEqual({ + expect(resolveDockerContext(actorDir(), '../../..')).toEqual({ kind: 'context', contextRoot: root, sourceRoot: root, @@ -111,19 +159,16 @@ describe('resolveDockerContext', () => { }); it('rejects a sibling directory, which does not contain the Actor folder', () => { - expect(resolveDockerContext(actorDir, '../../../other')).toEqual({ + expect(resolveDockerContext(actorDir(), '../../../other')).toEqual({ kind: 'invalid', message: `"dockerContextDir" in .actor/actor.json points to ${join(root, 'other')}, which does not contain the Actor's folder.`, }); }); it('rejects a context that does not exist', () => { - // `resolve`, not `join`: on Windows the resolved context carries the drive (`D:\repo`), as the command resolves it. - const repoDir = resolve('/repo'); - - expect(resolveDockerContext(join(repoDir, 'actors', 'a'), '../../..')).toEqual({ + expect(resolveDockerContext(actorDir(), '../../../missing')).toEqual({ kind: 'invalid', - message: `"dockerContextDir" in .actor/actor.json points to ${repoDir}, which does not exist.`, + message: `"dockerContextDir" in .actor/actor.json points to ${join(root, 'missing')}, which does not exist.`, }); }); }); From feb2cec125102254941fcdb2ac0d4abf87f3bf1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 12:27:18 +0100 Subject: [PATCH 11/42] fix: return forward-slash paths from the Docker context file collector --- src/lib/docker-context.ts | 12 +++++-- test/local/lib/docker-context-files.test.ts | 35 +++++++++------------ 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts index 2dcf0e8c6..9a7176897 100644 --- a/src/lib/docker-context.ts +++ b/src/lib/docker-context.ts @@ -1,3 +1,6 @@ +// The Docker context of an Actor: where `dockerContextDir` in `.actor/actor.json` points and what a push of it +// would carry. `readGitProvenance` and `getContextFilePaths` have no caller in `apify push` yet; they serve the +// stage 2 upload of the context and the `actor-runtime` branch. import { execFileSync } from 'node:child_process'; import { lstatSync, realpathSync, statSync } from 'node:fs'; import { isAbsolute, join, relative, resolve, sep } from 'node:path'; @@ -146,8 +149,11 @@ function gitFileList(root: string, args: string[]): string[] | undefined { return output?.split('\0').filter(Boolean); } -/** What a Git clone of `root` has, as on disk now, plus untracked files Git does not ignore; `.actorignore` - * does not apply, as on the platform. Outside Git, what an ordinary push of `root` sends. */ +/** + * What a Git clone of `root` has, as on disk now, plus untracked files Git does not ignore; `.actorignore` does not + * apply, as on the platform. Outside Git, what an ordinary push of `root` sends. The paths are relative to `root` + * with forward slashes on every platform, as `getActorLocalFilePaths` returns them. + */ export async function getContextFilePaths(root: string): Promise { const tracked = gitFileList(root, ['--cached']); const untracked = gitFileList(root, ['--others', '--exclude-standard']); @@ -168,7 +174,7 @@ export async function getContextFilePaths(root: string): Promise { (filePath) => isPushable(filePath) && !filePath.split('/').some((segment) => UNTRACKED_IGNORED_SEGMENTS.has(segment)), ), - ].map((filePath) => filePath.split('/').join(sep)); + ]; } export function monorepoUnsupportedMessage(actorName: string): string { diff --git a/test/local/lib/docker-context-files.test.ts b/test/local/lib/docker-context-files.test.ts index 35260cf0b..0632678e2 100644 --- a/test/local/lib/docker-context-files.test.ts +++ b/test/local/lib/docker-context-files.test.ts @@ -1,6 +1,6 @@ import { execFileSync } from 'node:child_process'; import { mkdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; +import { dirname } from 'node:path'; import { getContextFilePaths } from '../../../src/lib/docker-context.js'; import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; @@ -67,45 +67,38 @@ describe('getContextFilePaths', () => { await afterAllCalls(); }); - it('lists the tracked files and the untracked files Git does not ignore', async () => { + it('lists the tracked files and the untracked files Git does not ignore, with forward slashes', async () => { const paths = await getContextFilePaths(tmpPath); expect(paths).toEqual( expect.arrayContaining([ 'package.json', '.gitignore', - join('shared', 'Dockerfile'), - join(ACTOR_PATH, '.actor', 'actor.json'), - join(ACTOR_PATH, 'src', 'index.ts'), - join('shared', 'start.sh'), + 'shared/Dockerfile', + `${ACTOR_PATH}/.actor/actor.json`, + // Tracked, so in the clone, although .actorignore excludes it. + `${ACTOR_PATH}/src/index.ts`, + 'shared/start.sh', ]), ); - expect(paths).not.toContain(join(ACTOR_PATH, 'dist', 'main.js')); + // Git-ignored, so not in the clone, whatever .actorignore force-includes. + expect(paths).not.toContain(`${ACTOR_PATH}/dist/main.js`); }); it('skips a tracked file that is deleted locally', async () => { - expect(await getContextFilePaths(tmpPath)).not.toContain(join('packages', 'utils', 'src', 'index.ts')); + expect(await getContextFilePaths(tmpPath)).not.toContain('packages/utils/src/index.ts'); }); it('drops node_modules and storage folders from the untracked files only', async () => { const paths = await getContextFilePaths(tmpPath); - expect(paths).toContain(join('node_modules', 'tracked', 'index.js')); - expect(paths).not.toContain(join('node_modules', 'untracked', 'index.js')); - expect(paths).not.toContain(join('storage', 'key_value_stores', 'default', 'INPUT.json')); - }); - - it('does not apply .actorignore, as the platform clone has none of its effects', async () => { - const paths = await getContextFilePaths(tmpPath); - - // Tracked, so in the clone, although .actorignore excludes it. - expect(paths).toContain(join(ACTOR_PATH, 'src', 'index.ts')); - // Git-ignored, so not in the clone, whatever .actorignore force-includes. - expect(paths).not.toContain(join(ACTOR_PATH, 'dist', 'main.js')); + expect(paths).toContain('node_modules/tracked/index.js'); + expect(paths).not.toContain('node_modules/untracked/index.js'); + expect(paths).not.toContain('storage/key_value_stores/default/INPUT.json'); }); it('keeps a symlink, as a Git clone does', async () => { - expect(await getContextFilePaths(tmpPath)).toContain(join(ACTOR_PATH, 'shared')); + expect(await getContextFilePaths(tmpPath)).toContain(`${ACTOR_PATH}/shared`); }); }); From b2ee9514a0846b97e1d5cd1b046922db15693c93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 12:27:25 +0100 Subject: [PATCH 12/42] fix: name the Actor folder in the monorepo refusal when actor.json has no name --- src/commands/actors/push.ts | 5 +++-- test/local/commands/push-monorepo.test.ts | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 2489eacc4..c1c0e848d 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -1,5 +1,5 @@ import { readFileSync, statSync, unlinkSync } from 'node:fs'; -import { join, resolve } from 'node:path'; +import { basename, join, resolve } from 'node:path'; import process from 'node:process'; import type { Actor, ActorCollectionCreateOptions, ActorDefaultRunOptions } from 'apify-client'; @@ -297,7 +297,8 @@ export class ActorsPushCommand extends ApifyCommand { return; } if (dockerContext.kind === 'context') { - error({ message: monorepoUnsupportedMessage(actorConfig!.name as string) }); + // Pushing by ID, actor.json may have no name; the folder then names the Actor. + error({ message: monorepoUnsupportedMessage((actorConfig?.name as string | undefined) ?? basename(cwd)) }); process.exitCode = CommandExitCodes.NotImplemented; return; } diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index 6d3eab22d..c405eaa19 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -141,6 +141,16 @@ describe('apify push of a monorepo Actor', () => { expect(versionUpdate).not.toHaveBeenCalled(); }); + it('names the Actor folder when actor.json has no name', async () => { + await writeActorConfig({ name: undefined, dockerContextDir: '../../..' }); + + await testRunCommand(ActorsPushCommand, { args_actorId: ACTOR_ID }); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.NotImplemented); + expect(logMessages.error.join('\n')).toContain('Actor typescript-actor sets "dockerContextDir"'); + expect(actorGet).not.toHaveBeenCalled(); + }); + it('rejects a dockerContextDir outside the Git repository, which the platform could not build', async () => { await writeActorConfig({ dockerContextDir: '../../../..' }); From d1fec8dcc7632065ec973b59069b9535b8baddac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 12:27:26 +0100 Subject: [PATCH 13/42] test: cover readGitProvenance and tidy the monorepo push test --- test/local/commands/push-monorepo.test.ts | 18 +++-- .../lib/docker-context-provenance.test.ts | 75 +++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 test/local/lib/docker-context-provenance.test.ts diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index c405eaa19..6c98db6cf 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -83,10 +83,13 @@ function git(...args: string[]) { execFileSync('git', args, { cwd: tmpPath, stdio: 'ignore' }); } +// The command sets its exit code on the mocked copy, but the framework records a thrown error on the real `process`. let originalExitCode: typeof process.exitCode; +let originalRealExitCode: typeof process.exitCode; beforeEach(async () => { originalExitCode = commandProcess.exitCode; + originalRealExitCode = process.exitCode; actorGet.mockClear(); versionUpdate.mockClear(); versionCreate.mockClear(); @@ -99,10 +102,9 @@ beforeEach(async () => { await write('.gitignore', 'node_modules\n'); await write('shared/TypeScript_Dockerfile', 'FROM apify/actor-node:20\n'); await write('packages/typescript-utils/src/index.ts', 'export const x = 1;\n'); - await write('node_modules/ignored/index.js', ''); await writeActorConfig({ dockerContextDir: '../../..', dockerfile: '../../../shared/TypeScript_Dockerfile' }); await write(`${ACTOR_PATH}/src/index.ts`, 'console.log(1);\n'); - git('init', '--quiet', '--initial-branch=main'); + git('init', '--quiet'); git('add', '.'); git('-c', 'user.name=Test', '-c', 'user.email=test@example.com', 'commit', '--quiet', '-m', 'init'); @@ -111,6 +113,7 @@ beforeEach(async () => { afterEach(async () => { commandProcess.exitCode = originalExitCode; + process.exitCode = originalRealExitCode; await afterAllCalls(); }); @@ -151,13 +154,17 @@ describe('apify push of a monorepo Actor', () => { expect(actorGet).not.toHaveBeenCalled(); }); - it('rejects a dockerContextDir outside the Git repository, which the platform could not build', async () => { - await writeActorConfig({ dockerContextDir: '../../../..' }); + it.each([ + ['outside the Git repository, which the platform could not build', '../../../..', 'outside the Git repository'], + ['that does not exist', '../../../missing', 'which does not exist'], + ['that is not a string', 42, 'must be a string'], + ])('rejects a dockerContextDir %s as an invalid actor.json', async (_, dockerContextDir, message) => { + await writeActorConfig({ dockerContextDir }); await testRunCommand(ActorsPushCommand, {}); expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); - expect(logMessages.error.join('\n')).toContain('outside the Git repository'); + expect(logMessages.error.join('\n')).toContain(message); expect(actorGet).not.toHaveBeenCalled(); expect(versionUpdate).not.toHaveBeenCalled(); }); @@ -168,6 +175,7 @@ describe('apify push of a monorepo Actor', () => { await testRunCommand(ActorsPushCommand, {}); expect(commandProcess.exitCode).toBeFalsy(); + expect(process.exitCode).toBeFalsy(); expect(versionUpdate).toHaveBeenCalledTimes(1); const modifier = versionUpdate.mock.calls[0][0] as { sourceType: string; sourceFiles: { name: string }[] }; expect(modifier.sourceType).toBe(ACTOR_SOURCE_TYPES.SOURCE_FILES); diff --git a/test/local/lib/docker-context-provenance.test.ts b/test/local/lib/docker-context-provenance.test.ts new file mode 100644 index 000000000..359f9d8f1 --- /dev/null +++ b/test/local/lib/docker-context-provenance.test.ts @@ -0,0 +1,75 @@ +import { execFileSync } from 'node:child_process'; +import { writeFileSync } from 'node:fs'; + +import { readGitProvenance } from '../../../src/lib/docker-context.js'; +import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; + +// vi.mock is hoisted before imports, so the reader gets the mocked version. With `gitAvailable` off, every git +// call fails as it does without git. +const gitState = vi.hoisted(() => ({ gitAvailable: true })); +vi.mock('node:child_process', async (importOriginal) => { + const original = await importOriginal(); + const unavailable = () => { + throw new Error('not a git repository'); + }; + return { + ...original, + execSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execSync(...args) : unavailable()) as typeof original.execSync, + execFileSync: ((...args: Parameters) => + gitState.gitAvailable ? original.execFileSync(...args) : unavailable()) as typeof original.execFileSync, + }; +}); + +describe('readGitProvenance', () => { + const { tmpPath, joinPath, beforeAllCalls, afterAllCalls } = useTempPath('docker-context-provenance-test-dir', { + create: true, + remove: true, + cwd: false, + cwdParent: false, + }); + + const git = (...args: string[]) => + execFileSync('git', args, { cwd: tmpPath, encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + + beforeAll(async () => { + await beforeAllCalls(); + writeFileSync(joinPath('README.md'), 'content'); + git('init', '--quiet', '--initial-branch=main'); + git('add', '.'); + git('-c', 'user.name=Test', '-c', 'user.email=test@example.com', 'commit', '--quiet', '-m', 'init'); + }); + + afterAll(async () => { + gitState.gitAvailable = true; + await afterAllCalls(); + }); + + it('reads the commit and the branch of a clean checkout with no remote', () => { + expect(readGitProvenance(tmpPath)).toEqual({ commit: git('rev-parse', 'HEAD'), branch: 'main', dirty: false }); + }); + + it('strips credentials from the origin URL and leaves an scp-like one alone', () => { + git('remote', 'add', 'origin', 'https://user:token@github.com/apify/actor-monorepo-example.git'); + expect(readGitProvenance(tmpPath).remoteUrl).toBe('https://github.com/apify/actor-monorepo-example.git'); + + git('remote', 'set-url', 'origin', 'git@github.com:apify/actor-monorepo-example.git'); + expect(readGitProvenance(tmpPath).remoteUrl).toBe('git@github.com:apify/actor-monorepo-example.git'); + }); + + it('flags an untracked file as dirty', () => { + writeFileSync(joinPath('notes.txt'), 'content'); + expect(readGitProvenance(tmpPath).dirty).toBe(true); + }); + + it('reports no branch on a detached HEAD', () => { + git('checkout', '--quiet', '--detach'); + expect(readGitProvenance(tmpPath)).not.toHaveProperty('branch'); + expect(readGitProvenance(tmpPath).commit).toBe(git('rev-parse', 'HEAD')); + }); + + it('reads nothing without git', () => { + gitState.gitAvailable = false; + expect(readGitProvenance(tmpPath)).toEqual({}); + }); +}); From 6d2bdfa91ffc22da5b686926b8bcacaa81085e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 12:40:42 +0100 Subject: [PATCH 14/42] fix: check the actor.json paths of a monorepo Actor from its real folder --- src/commands/actors/push.ts | 5 ++++- test/local/commands/push-monorepo.test.ts | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index aa27ce398..424d0e1f6 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -331,6 +331,7 @@ export class ActorsPushCommand extends ApifyCommand { process.exitCode = CommandExitCodes.InvalidActorJson; return; } + let actorDir = cwd; let uploadRoot = cwd; let uploadedFiles = filePathsToPush; let actorPath: string | undefined; @@ -338,11 +339,13 @@ export class ActorsPushCommand extends ApifyCommand { uploadRoot = dockerContext.sourceRoot; uploadedFiles = resolveLinksForUpload(uploadRoot, await getContextFilePaths(uploadRoot)); actorPath = dockerContext.actorPath; + // The source root is a real path, so the Actor folder has to be one too for its paths to fall inside it. + actorDir = join(uploadRoot, actorPath); } // The platform resolves these paths after the upload; a wrong one builds the wrong image or fails the build. const pathProblems = checkActorPaths({ - actorDir: cwd, + actorDir, uploadRoot, uploadedFiles, config: actorConfig, diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index 067d73870..a0949cd48 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -370,6 +370,18 @@ describe('apify push of a monorepo Actor', () => { expect(logMessages.error.join('\n')).toContain('Actor typescript-actor sets "dockerContextDir"'); }); + it('checks the actor.json paths from the real Actor folder when --dir goes through a symlink', async () => { + symlinkSync(ACTOR_PATH, joinPath('linked-actor')); + forceNewCwd('linked-actor'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + const modifier = versionPayload(versionUpdate); + expect(modifier.actorPath).toBe(ACTOR_PATH); + expect(modifier.sourceFiles!.map((file) => file.name)).toContain(`${ACTOR_PATH}/.actor/actor.json`); + }); + it.each([ ['outside the Git repository, which the platform could not build', '../../../..', 'outside the Git repository'], ['that does not exist', '../../../missing', 'which does not exist'], From db2cb8f528198dad80b746e633039fc6675185d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 13:20:32 +0100 Subject: [PATCH 15/42] refactor: split the Docker context resolution into none, within, outside, and invalid --- src/commands/actors/push.ts | 2 +- src/lib/docker-context.ts | 26 ++++++++++++++++++-------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index c1c0e848d..5c328bb6f 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -296,7 +296,7 @@ export class ActorsPushCommand extends ApifyCommand { process.exitCode = CommandExitCodes.InvalidActorJson; return; } - if (dockerContext.kind === 'context') { + if (dockerContext.kind === 'outside') { // Pushing by ID, actor.json may have no name; the folder then names the Actor. error({ message: monorepoUnsupportedMessage((actorConfig?.name as string | undefined) ?? basename(cwd)) }); process.exitCode = CommandExitCodes.NotImplemented; diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts index 9a7176897..3898c42a1 100644 --- a/src/lib/docker-context.ts +++ b/src/lib/docker-context.ts @@ -12,20 +12,28 @@ import { getActorLocalFilePaths } from './utils.js'; export const MONOREPO_PLATFORM_SUPPORT_ISSUE_URL = 'https://github.com/apify/apify-core/issues/28685'; export type DockerContextResolution = + /** `dockerContextDir` is absent: undefined, null, or an empty string. */ | { kind: 'none' } + /** The context is the Actor folder itself or a directory inside it, so a push of the folder carries it. */ + | { kind: 'within'; contextRoot: string } + /** The context lies beyond the Actor folder (a monorepo Actor): above it, or elsewhere in the Git repository. */ | { - kind: 'context'; + kind: 'outside'; contextRoot: string; /** The Git repository's root, else the context. */ sourceRoot: string; /** Relative to `sourceRoot`. */ actorPath: string; } + /** The field is not a string, or the directory it names is missing, not a directory, or out of reach. */ | { kind: 'invalid'; message: string }; /** - * `dockerContextDir` is relative to `.actor/`, as on the platform. A context at or below the Actor folder is an - * ordinary push, which the platform builds as it is; one above or beside it is a monorepo Actor. + * `dockerContextDir` is relative to `.actor/`, as on the platform. An absent field is `none`, with no path work. A + * context at or below the Actor folder is `within`: an ordinary push, which the platform builds as it is. A context + * above or beside the Actor folder is `outside`: a monorepo Actor, with the source root a push would have to carry. + * A context that cannot be used is `invalid`, with the message to show. Paths come back real, as the platform sees + * them. */ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown): DockerContextResolution { if (dockerContextDir === undefined || dockerContextDir === null || dockerContextDir === '') return { kind: 'none' }; @@ -42,9 +50,6 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown } const contextRoot = resolve(realActorDir, ACTOR_SPECIFICATION_FOLDER, dockerContextDir); - const contextPathInActor = relative(realActorDir, contextRoot); - if (contextPathInActor === '' || !isOutside(contextPathInActor)) return { kind: 'none' }; - const contextStats = statSync(contextRoot, { throwIfNoEntry: false }); if (!contextStats) { return { @@ -60,6 +65,11 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown } const realContextRoot = realpathSync(contextRoot); + const contextPathInActor = relative(realActorDir, contextRoot); + if (contextPathInActor === '' || !isOutside(contextPathInActor)) { + return { kind: 'within', contextRoot: realContextRoot }; + } + // The platform's clone holds nothing above the repository root. const repoRoot = git(realActorDir, ['rev-parse', '--show-toplevel']); if (repoRoot) { @@ -73,7 +83,7 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown }; } return { - kind: 'context', + kind: 'outside', contextRoot: realContextRoot, sourceRoot, actorPath: toPosix(relative(sourceRoot, realActorDir)), @@ -87,7 +97,7 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown message: `"dockerContextDir" in .actor/actor.json points to ${realContextRoot}, which does not contain the Actor's folder.`, }; } - return { kind: 'context', contextRoot: realContextRoot, sourceRoot: realContextRoot, actorPath: toPosix(actorPath) }; + return { kind: 'outside', contextRoot: realContextRoot, sourceRoot: realContextRoot, actorPath: toPosix(actorPath) }; } function isOutside(relativePath: string): boolean { From f7288865ab107f80a3411cc016b56a269b83dc56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 13:20:33 +0100 Subject: [PATCH 16/42] test: cover the none and within Docker context kinds --- test/local/commands/push-monorepo.test.ts | 7 ++-- test/local/lib/docker-context.test.ts | 39 ++++++++++++++++++----- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index 6c98db6cf..d1813d373 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -169,8 +169,11 @@ describe('apify push of a monorepo Actor', () => { expect(versionUpdate).not.toHaveBeenCalled(); }); - it('pushes the Actor folder as usual when the Docker context lies inside it', async () => { - await writeActorConfig({ dockerContextDir: '../src' }); + it.each([ + ['lies inside it', { dockerContextDir: '../src' }], + ['is not set', {}], + ])('pushes the Actor folder as usual when the Docker context %s', async (_, fields) => { + await writeActorConfig(fields); await testRunCommand(ActorsPushCommand, {}); diff --git a/test/local/lib/docker-context.test.ts b/test/local/lib/docker-context.test.ts index 180487b84..e5826da47 100644 --- a/test/local/lib/docker-context.test.ts +++ b/test/local/lib/docker-context.test.ts @@ -43,6 +43,7 @@ describe('resolveDockerContext', () => { mkdirSync(joinPath(ACTOR_PATH, 'src'), { recursive: true }); mkdirSync(joinPath('shared'), { recursive: true }); writeFileSync(joinPath('Dockerfile'), 'FROM apify/actor-node:20\n'); + writeFileSync(joinPath(ACTOR_PATH, 'Dockerfile'), 'FROM apify/actor-node:20\n'); symlinkSync(ACTOR_PATH, joinPath('linked-actor')); execFileSync('git', ['init', '--quiet'], { cwd: tmpPath, stdio: 'ignore' }); root = realpathSync(tmpPath); @@ -52,17 +53,39 @@ describe('resolveDockerContext', () => { await afterAllCalls(); }); - it('treats a missing field, or a context at or below the Actor folder, as an ordinary push', () => { + it('reports a missing field as none', () => { expect(resolveDockerContext(actorDir(), undefined)).toEqual({ kind: 'none' }); + expect(resolveDockerContext(actorDir(), null)).toEqual({ kind: 'none' }); expect(resolveDockerContext(actorDir(), '')).toEqual({ kind: 'none' }); - expect(resolveDockerContext(actorDir(), '..')).toEqual({ kind: 'none' }); - expect(resolveDockerContext(actorDir(), '.')).toEqual({ kind: 'none' }); - expect(resolveDockerContext(actorDir(), '../src')).toEqual({ kind: 'none' }); + }); + + it('reports a context at or below the Actor folder as within, with its real path', () => { + const actorRoot = join(root, ACTOR_PATH); + expect(resolveDockerContext(actorDir(), '..')).toEqual({ kind: 'within', contextRoot: actorRoot }); + expect(resolveDockerContext(actorDir(), '.')).toEqual({ kind: 'within', contextRoot: join(actorRoot, '.actor') }); + expect(resolveDockerContext(actorDir(), '../src')).toEqual({ + kind: 'within', + contextRoot: join(actorRoot, 'src'), + }); + }); + + it('rejects a context inside the Actor folder that does not exist', () => { + expect(resolveDockerContext(actorDir(), '../missing')).toEqual({ + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${join(root, ACTOR_PATH, 'missing')}, which does not exist.`, + }); + }); + + it('rejects a context inside the Actor folder that is not a directory', () => { + expect(resolveDockerContext(actorDir(), '../Dockerfile')).toEqual({ + kind: 'invalid', + message: `"dockerContextDir" in .actor/actor.json points to ${join(root, ACTOR_PATH, 'Dockerfile')}, which is not a directory.`, + }); }); it('resolves a context above the Actor folder to the repository root and the Actor path in it', () => { expect(resolveDockerContext(actorDir(), '../../..')).toEqual({ - kind: 'context', + kind: 'outside', contextRoot: root, sourceRoot: root, actorPath: ACTOR_PATH, @@ -71,7 +94,7 @@ describe('resolveDockerContext', () => { it('keeps the repository root as the source for a context elsewhere in the repository', () => { expect(resolveDockerContext(actorDir(), '../../../shared')).toEqual({ - kind: 'context', + kind: 'outside', contextRoot: join(root, 'shared'), sourceRoot: root, actorPath: ACTOR_PATH, @@ -80,7 +103,7 @@ describe('resolveDockerContext', () => { it('resolves a symlinked Actor folder from its real path, as the platform does', () => { expect(resolveDockerContext(joinPath('linked-actor'), '../../..')).toEqual({ - kind: 'context', + kind: 'outside', contextRoot: root, sourceRoot: root, actorPath: ACTOR_PATH, @@ -151,7 +174,7 @@ describe('resolveDockerContext', () => { it('makes the context the source when it contains the Actor folder', () => { expect(resolveDockerContext(actorDir(), '../../..')).toEqual({ - kind: 'context', + kind: 'outside', contextRoot: root, sourceRoot: root, actorPath: ACTOR_PATH, From 9300a0117063f016dd9d764b0db2480704c59497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 13:26:01 +0100 Subject: [PATCH 17/42] fix: require the Dockerfile inside a Docker context within the Actor folder --- src/commands/actors/push.ts | 1 + test/local/commands/push-actor-paths.test.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 01f9dda5e..a845fad89 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -310,6 +310,7 @@ export class ActorsPushCommand extends ApifyCommand { uploadRoot: cwd, uploadedFiles: filePathsToPush, config: actorConfig, + dockerContextRoot: dockerContext.kind === 'within' ? dockerContext.contextRoot : undefined, }); if (pathProblems.length > 0) { error({ message: formatActorPathProblems(pathProblems) }); diff --git a/test/local/commands/push-actor-paths.test.ts b/test/local/commands/push-actor-paths.test.ts index f55be9cef..800704c4d 100644 --- a/test/local/commands/push-actor-paths.test.ts +++ b/test/local/commands/push-actor-paths.test.ts @@ -131,6 +131,19 @@ describe('apify push with file paths in actor.json', () => { expect(versionUpdate).not.toHaveBeenCalled(); }); + it('requires the Dockerfile inside a Docker context within the Actor folder', async () => { + await write('docker/Dockerfile', 'FROM apify/actor-node:20\n'); + await writeActorConfig({ dockerContextDir: '../docker', dockerfile: '../Dockerfile' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.error.join('\n')).toContain( + `"dockerfile": "../Dockerfile" (${joinPath('Dockerfile')}) lies outside the Docker context ${joinPath('docker')}.`, + ); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + it('pushes as usual when every path points to an uploaded file', async () => { await writeActorConfig({ dockerfile: '../dockerfile', input: './INPUT_SCHEMA.json' }); From a9f2d19c5ee7a37dafcb62cba598cc195180e8db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 13:43:15 +0100 Subject: [PATCH 18/42] refactor: rename readGitProvenance to readGitState --- src/lib/docker-context.ts | 16 +++++++-------- ...e.test.ts => docker-context-state.test.ts} | 20 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) rename test/local/lib/{docker-context-provenance.test.ts => docker-context-state.test.ts} (75%) diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts index 3898c42a1..313ec90a4 100644 --- a/src/lib/docker-context.ts +++ b/src/lib/docker-context.ts @@ -1,5 +1,5 @@ // The Docker context of an Actor: where `dockerContextDir` in `.actor/actor.json` points and what a push of it -// would carry. `readGitProvenance` and `getContextFilePaths` have no caller in `apify push` yet; they serve the +// would carry. `readGitState` and `getContextFilePaths` have no caller in `apify push` yet; they serve the // stage 2 upload of the context and the `actor-runtime` branch. import { execFileSync } from 'node:child_process'; import { lstatSync, realpathSync, statSync } from 'node:fs'; @@ -108,7 +108,7 @@ function toPosix(filePath: string): string { return filePath.split(sep).join('/'); } -export interface GitProvenance { +export interface GitState { remoteUrl?: string; branch?: string; commit?: string; @@ -137,18 +137,18 @@ function withoutCredentials(remoteUrl: string): string { } } -export function readGitProvenance(dir: string): GitProvenance { +export function readGitState(dir: string): GitState { const commit = git(dir, ['rev-parse', 'HEAD']); if (!commit) return {}; - const provenance: GitProvenance = { commit }; + const state: GitState = { commit }; const branch = git(dir, ['rev-parse', '--abbrev-ref', 'HEAD']); - if (branch && branch !== 'HEAD') provenance.branch = branch; + if (branch && branch !== 'HEAD') state.branch = branch; const remoteUrl = git(dir, ['remote', 'get-url', 'origin']); - if (remoteUrl) provenance.remoteUrl = withoutCredentials(remoteUrl); + if (remoteUrl) state.remoteUrl = withoutCredentials(remoteUrl); const status = git(dir, ['status', '--porcelain', '--', '.']); - if (status !== undefined) provenance.dirty = status.length > 0; - return provenance; + if (status !== undefined) state.dirty = status.length > 0; + return state; } /** Untracked, these are never on the platform and too big to push by accident. */ diff --git a/test/local/lib/docker-context-provenance.test.ts b/test/local/lib/docker-context-state.test.ts similarity index 75% rename from test/local/lib/docker-context-provenance.test.ts rename to test/local/lib/docker-context-state.test.ts index 359f9d8f1..322f3ac68 100644 --- a/test/local/lib/docker-context-provenance.test.ts +++ b/test/local/lib/docker-context-state.test.ts @@ -1,7 +1,7 @@ import { execFileSync } from 'node:child_process'; import { writeFileSync } from 'node:fs'; -import { readGitProvenance } from '../../../src/lib/docker-context.js'; +import { readGitState } from '../../../src/lib/docker-context.js'; import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; // vi.mock is hoisted before imports, so the reader gets the mocked version. With `gitAvailable` off, every git @@ -21,8 +21,8 @@ vi.mock('node:child_process', async (importOriginal) => { }; }); -describe('readGitProvenance', () => { - const { tmpPath, joinPath, beforeAllCalls, afterAllCalls } = useTempPath('docker-context-provenance-test-dir', { +describe('readGitState', () => { + const { tmpPath, joinPath, beforeAllCalls, afterAllCalls } = useTempPath('docker-context-state-test-dir', { create: true, remove: true, cwd: false, @@ -46,30 +46,30 @@ describe('readGitProvenance', () => { }); it('reads the commit and the branch of a clean checkout with no remote', () => { - expect(readGitProvenance(tmpPath)).toEqual({ commit: git('rev-parse', 'HEAD'), branch: 'main', dirty: false }); + expect(readGitState(tmpPath)).toEqual({ commit: git('rev-parse', 'HEAD'), branch: 'main', dirty: false }); }); it('strips credentials from the origin URL and leaves an scp-like one alone', () => { git('remote', 'add', 'origin', 'https://user:token@github.com/apify/actor-monorepo-example.git'); - expect(readGitProvenance(tmpPath).remoteUrl).toBe('https://github.com/apify/actor-monorepo-example.git'); + expect(readGitState(tmpPath).remoteUrl).toBe('https://github.com/apify/actor-monorepo-example.git'); git('remote', 'set-url', 'origin', 'git@github.com:apify/actor-monorepo-example.git'); - expect(readGitProvenance(tmpPath).remoteUrl).toBe('git@github.com:apify/actor-monorepo-example.git'); + expect(readGitState(tmpPath).remoteUrl).toBe('git@github.com:apify/actor-monorepo-example.git'); }); it('flags an untracked file as dirty', () => { writeFileSync(joinPath('notes.txt'), 'content'); - expect(readGitProvenance(tmpPath).dirty).toBe(true); + expect(readGitState(tmpPath).dirty).toBe(true); }); it('reports no branch on a detached HEAD', () => { git('checkout', '--quiet', '--detach'); - expect(readGitProvenance(tmpPath)).not.toHaveProperty('branch'); - expect(readGitProvenance(tmpPath).commit).toBe(git('rev-parse', 'HEAD')); + expect(readGitState(tmpPath)).not.toHaveProperty('branch'); + expect(readGitState(tmpPath).commit).toBe(git('rev-parse', 'HEAD')); }); it('reads nothing without git', () => { gitState.gitAvailable = false; - expect(readGitProvenance(tmpPath)).toEqual({}); + expect(readGitState(tmpPath)).toEqual({}); }); }); From a31364e447b9a2d150f7764f501fb000b1ea4b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 15:03:24 +0100 Subject: [PATCH 19/42] fix: judge actor.json paths lexically, as push uploads them push reads through a symlink and uploads the content at the link's own path, so a link to a file outside the upload root builds on the platform. The realpath escape rule refused it; the uploaded files rule already rejects anything push does not send. --- src/lib/actor-paths.ts | 18 +++++++++--------- test/local/lib/actor-paths.test.ts | 11 ++++++++--- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index b7597df7a..0bcec06c6 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -1,4 +1,4 @@ -import { readdirSync, realpathSync, statSync } from 'node:fs'; +import { readdirSync, type Stats, statSync } from 'node:fs'; import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { ACTOR_SPECIFICATION_FOLDER } from './consts.js'; @@ -89,18 +89,17 @@ function checkPathField( ); } + // push reads through a symlink and uploads the content at this path, so the path is judged as it is, not by its + // target; a dangling symlink has no content to upload. const filePath = join(dir, name); - let realPath: string; + let stats: Stats; try { - realPath = realpathSync(filePath); + stats = statSync(filePath); } catch { return problem(resolvedPath, 'does not exist'); } - if (isOutside(relative(realpathSync(root), realPath))) { - return problem(resolvedPath, `lies outside the uploaded folder ${root} (through a symlink)`); - } - if (statSync(realPath).isDirectory()) return problem(resolvedPath, 'is a directory, not a file'); - if (dockerContextRoot && field === 'dockerfile' && isOutside(relative(realpathSync(dockerContextRoot), realPath))) { + if (stats.isDirectory()) return problem(resolvedPath, 'is a directory, not a file'); + if (dockerContextRoot && field === 'dockerfile' && isOutside(relative(resolve(dockerContextRoot), filePath))) { return problem(resolvedPath, `lies outside the Docker context ${dockerContextRoot}`); } @@ -120,7 +119,8 @@ function checkPathField( /** * Applies the platform's rules to every file path in `actor.json` before anything is uploaded: relative to - * `.actor/`, inside the upload root (symlinks resolved), an existing file, and one that push sends. + * `.actor/`, inside the upload root, an existing file, and one that push sends. A symlink counts by its own path, + * as push uploads its content there. */ export function checkActorPaths(options: ActorPathsOptions): ActorPathProblem[] { return collectPathFields(options.config).flatMap((pathField) => checkPathField(pathField, options) ?? []); diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index e7dcc87a6..317807ac4 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -114,13 +114,18 @@ describe('checkActorPaths', () => { ]); }); - it('rejects a symlink to a file outside the upload root', () => { - expect(check({ input: './linked_schema.json' })).toEqual([ + it('judges a symlink by its own path, as push uploads its content there', () => { + // The link points outside the upload root; push reads through it and uploads the content at the link's path. + expect(uploadedFiles).toContain('.actor/linked_schema.json'); + expect(check({ input: './linked_schema.json' })).toEqual([]); + + const withoutLink = uploadedFiles.filter((file) => file !== '.actor/linked_schema.json'); + expect(check({ input: './linked_schema.json' }, { uploadedFiles: withoutLink })).toEqual([ { field: 'input', value: './linked_schema.json', resolvedPath: join(actorDir, '.actor', 'linked_schema.json'), - reason: `lies outside the uploaded folder ${actorDir} (through a symlink)`, + reason: 'is not among the files apify push uploads (excluded by .gitignore or .actorignore?)', }, ]); }); From 0b495cfa27f5a48b90b340ddc9395f6e30fcc5ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 15:03:45 +0100 Subject: [PATCH 20/42] fix: check the inputSchema and outputSchema aliases The worker copies inputSchema over input and outputSchema over output before resolving, so a schema path under the alias was never checked. --- src/lib/actor-paths.ts | 12 ++++++++++-- test/local/lib/actor-paths.test.ts | 11 +++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 0bcec06c6..2be37a220 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -24,7 +24,12 @@ export interface ActorPathsOptions { /** The platform finds these by a directory listing, so their names match case-insensitively. */ const LISTED_FILE_FIELDS = ['dockerfile', 'readme', 'changelog']; -const SCHEMA_FIELDS = ['input', 'output', 'webServerSchema']; +/** The worker copies a set alias over its base key before resolving, so the alias wins and the base key is ignored. */ +const SCHEMA_FIELDS: { field: string; alias?: string }[] = [ + { field: 'input', alias: 'inputSchema' }, + { field: 'output', alias: 'outputSchema' }, + { field: 'webServerSchema' }, +]; const STORAGE_SCHEMA_FIELDS = ['keyValueStore', 'dataset']; interface PathField { @@ -45,7 +50,10 @@ function collectPathFields(config: Record): PathField[] { }; for (const field of LISTED_FILE_FIELDS) add(field, config[field], true); - for (const field of SCHEMA_FIELDS) add(field, config[field]); + for (const { field, alias } of SCHEMA_FIELDS) { + if (alias && config[alias]) add(alias, config[alias]); + else add(field, config[field]); + } if (isRecord(config.storages)) { for (const field of STORAGE_SCHEMA_FIELDS) add(`storages.${field}`, config.storages[field]); if (isRecord(config.storages.datasets)) { diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index 317807ac4..fc6a2dc33 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -152,6 +152,17 @@ describe('checkActorPaths', () => { ).toEqual(['output', 'webServerSchema', 'storages.keyValueStore', 'storages.dataset', 'storages.datasets.main']); }); + it('checks inputSchema and outputSchema, which win over input and output', () => { + expect(check({ inputSchema: './in.json', outputSchema: './out.json' }).map((problem) => problem.field)).toEqual([ + 'inputSchema', + 'outputSchema', + ]); + // The worker copies a set alias over its base key, so the base key is never resolved. + expect(check({ input: './missing.json', inputSchema: './INPUT_SCHEMA.json' })).toEqual([]); + expect(check({ output: './missing.json', outputSchema: { type: 'object' } })).toEqual([]); + expect(check({ input: './missing.json', inputSchema: '' }).map((problem) => problem.field)).toEqual(['input']); + }); + it('requires dockerfile inside the Docker context when one is given', () => { const monorepoFiles = [`${ACTOR_PATH}/Dockerfile`, `${ACTOR_PATH}/.actor/actor.json`, 'shared/Dockerfile']; const monorepo = { uploadRoot: root, uploadedFiles: monorepoFiles, dockerContextRoot: join(root, 'shared') }; From 3e14b8b5f566525ad737a2f37ecbdc234d7838f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 15:04:10 +0100 Subject: [PATCH 21/42] fix: skip empty path fields and reject backslashes The worker treats an empty string as an unset field, so an empty value was wrongly reported as the .actor folder being a directory. A backslash passes path.resolve on Windows but is a filename character on the Linux worker, so it is refused before any resolution. --- src/lib/actor-paths.ts | 6 +++++- test/local/lib/actor-paths.test.ts | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 2be37a220..4115580c3 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -46,7 +46,8 @@ function isRecord(value: unknown): value is Record { function collectPathFields(config: Record): PathField[] { const fields: PathField[] = []; const add = (field: string, value: unknown, caseInsensitive = false) => { - if (typeof value === 'string') fields.push({ field, value, caseInsensitive }); + // The worker treats an empty string as an unset field. + if (typeof value === 'string' && value !== '') fields.push({ field, value, caseInsensitive }); }; for (const field of LISTED_FILE_FIELDS) add(field, config[field], true); @@ -78,6 +79,9 @@ function checkPathField( { actorDir, uploadRoot, uploadedFiles, dockerContextRoot }: ActorPathsOptions, ): ActorPathProblem | undefined { const problem = (resolvedPath: string, reason: string) => ({ field, value, resolvedPath, reason }); + if (value.includes('\\')) { + return problem(value, 'uses a backslash; the platform resolves paths with forward slashes'); + } if (isAbsolute(value)) return problem(value, `is absolute; the path is relative to ${ACTOR_SPECIFICATION_FOLDER}/`); const resolvedPath = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, value); diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index fc6a2dc33..9106cc758 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -73,6 +73,21 @@ describe('checkActorPaths', () => { ]); }); + it('skips an empty string, which the platform treats as an unset field', () => { + expect(check({ dockerfile: '', input: '', storages: { dataset: '', datasets: { main: '' } } })).toEqual([]); + }); + + it('rejects a backslash before resolving anything', () => { + expect(check({ dockerfile: '..\\Dockerfile' })).toEqual([ + { + field: 'dockerfile', + value: '..\\Dockerfile', + resolvedPath: '..\\Dockerfile', + reason: 'uses a backslash; the platform resolves paths with forward slashes', + }, + ]); + }); + it('rejects an absolute path', () => { const value = resolve('/x'); From 775db15a836e753cc80f93a1d4b881b4dd14d60d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 15:05:30 +0100 Subject: [PATCH 22/42] test: cover the case mismatch reason and use the temp path hook The lib test now builds its fixture through useTempPath, as the other lib tests do, adds a changelog file so the case-insensitive match is checked for all three listed fields, and covers the reason given when the uploaded file differs only in case. The command test restores the real process.exitCode next to the mocked copy. --- test/local/commands/push-actor-paths.test.ts | 4 +++ test/local/lib/actor-paths.test.ts | 34 ++++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/test/local/commands/push-actor-paths.test.ts b/test/local/commands/push-actor-paths.test.ts index 800704c4d..0a243c141 100644 --- a/test/local/commands/push-actor-paths.test.ts +++ b/test/local/commands/push-actor-paths.test.ts @@ -73,10 +73,13 @@ async function writeActorConfig(fields: Record) { ); } +// The mocked copy may share the exit code with the real process, so both are restored. let originalExitCode: typeof process.exitCode; +let originalGlobalExitCode: typeof process.exitCode; beforeEach(async () => { originalExitCode = commandProcess.exitCode; + originalGlobalExitCode = process.exitCode; actorGet.mockClear(); versionUpdate.mockClear(); versionCreate.mockClear(); @@ -92,6 +95,7 @@ beforeEach(async () => { afterEach(async () => { commandProcess.exitCode = originalExitCode; + process.exitCode = originalGlobalExitCode; await afterAllCalls(); }); diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index 9106cc758..3e2a0a64f 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -1,13 +1,21 @@ -import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; +import { mkdirSync, realpathSync, symlinkSync, writeFileSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { checkActorPaths, formatActorPathProblems } from '../../../src/lib/actor-paths.js'; import { getActorLocalFilePaths } from '../../../src/lib/utils.js'; +import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; const ACTOR_PATH = 'actors/x'; describe('checkActorPaths', () => { + const { tmpPath, beforeAllCalls, afterAllCalls } = useTempPath('actor-paths-test-dir', { + create: true, + remove: true, + cwd: false, + cwdParent: false, + }); + + // The checker works from real paths, so the expectations do too. let root: string; let actorDir: string; let uploadedFiles: string[]; @@ -24,11 +32,13 @@ describe('checkActorPaths', () => { ) => checkActorPaths({ actorDir, uploadRoot: actorDir, uploadedFiles, config, ...options }); beforeAll(async () => { - root = mkdtempSync(join(tmpdir(), 'apify-cli-actor-paths-')); + await beforeAllCalls(); + root = realpathSync(tmpPath); actorDir = join(root, ACTOR_PATH); write(`${ACTOR_PATH}/.actor/actor.json`, '{}'); write(`${ACTOR_PATH}/.actor/INPUT_SCHEMA.json`, '{}'); write(`${ACTOR_PATH}/.actor/README.md`); + write(`${ACTOR_PATH}/.actor/CHANGELOG.md`); write(`${ACTOR_PATH}/.actorignore`, 'Dockerfile.ci\n'); write(`${ACTOR_PATH}/Dockerfile`, 'FROM apify/actor-node:20\n'); write(`${ACTOR_PATH}/Dockerfile.ci`, 'FROM apify/actor-node:20\n'); @@ -39,8 +49,8 @@ describe('checkActorPaths', () => { uploadedFiles = await getActorLocalFilePaths(actorDir); }); - afterAll(() => { - rmSync(root, { recursive: true, force: true }); + afterAll(async () => { + await afterAllCalls(); }); it('passes files inside the upload root, next to actor.json or above it', () => { @@ -129,6 +139,18 @@ describe('checkActorPaths', () => { ]); }); + it('rejects a file whose case differs from the uploaded one', () => { + // A case-insensitive file system finds README.md, but the Git index lists it as readme.md. + expect(check({ readme: './README.md' }, { uploadedFiles: ['.actor/readme.md'] })).toEqual([ + { + field: 'readme', + value: './README.md', + resolvedPath: join(actorDir, '.actor', 'README.md'), + reason: 'does not match the case of the uploaded file .actor/readme.md', + }, + ]); + }); + it('judges a symlink by its own path, as push uploads its content there', () => { // The link points outside the upload root; push reads through it and uploads the content at the link's path. expect(uploadedFiles).toContain('.actor/linked_schema.json'); @@ -146,7 +168,7 @@ describe('checkActorPaths', () => { }); it('matches the name of dockerfile, readme and changelog case-insensitively, and a schema exactly', () => { - expect(check({ dockerfile: '../dockerfile', readme: './readme.MD' })).toEqual([]); + expect(check({ dockerfile: '../dockerfile', readme: './readme.MD', changelog: './changelog.md' })).toEqual([]); expect(check({ input: './input_schema.json' })).toEqual([ { field: 'input', From 703159b40b665d07b78af07a0cee68b955bcb863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 15:06:03 +0100 Subject: [PATCH 23/42] refactor: compute the path check invariants once checkActorPaths resolves the roots and builds the posix set of uploaded files once and hands them to every field, instead of each field recomputing them. --- src/lib/actor-paths.ts | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 4115580c3..b59b42e8e 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -1,4 +1,4 @@ -import { readdirSync, type Stats, statSync } from 'node:fs'; +import { readdirSync, realpathSync, type Stats, statSync } from 'node:fs'; import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { ACTOR_SPECIFICATION_FOLDER } from './consts.js'; @@ -74,9 +74,17 @@ function listDirectory(dir: string): string[] { } } +/** The options as every field sees them: real roots and the uploaded files as posix paths. */ +interface PathCheckScope { + actorDir: string; + uploadRoot: string; + uploadedFiles: Set; + dockerContextRoot?: string; +} + function checkPathField( { field, value, caseInsensitive }: PathField, - { actorDir, uploadRoot, uploadedFiles, dockerContextRoot }: ActorPathsOptions, + { actorDir, uploadRoot, uploadedFiles, dockerContextRoot }: PathCheckScope, ): ActorPathProblem | undefined { const problem = (resolvedPath: string, reason: string) => ({ field, value, resolvedPath, reason }); if (value.includes('\\')) { @@ -85,8 +93,9 @@ function checkPathField( if (isAbsolute(value)) return problem(value, `is absolute; the path is relative to ${ACTOR_SPECIFICATION_FOLDER}/`); const resolvedPath = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, value); - const root = resolve(uploadRoot); - if (isOutside(relative(root, resolvedPath))) return problem(resolvedPath, `lies outside the uploaded folder ${root}`); + if (isOutside(relative(uploadRoot, resolvedPath))) { + return problem(resolvedPath, `lies outside the uploaded folder ${uploadRoot}`); + } const dir = dirname(resolvedPath); const wanted = basename(resolvedPath); @@ -111,14 +120,13 @@ function checkPathField( return problem(resolvedPath, 'does not exist'); } if (stats.isDirectory()) return problem(resolvedPath, 'is a directory, not a file'); - if (dockerContextRoot && field === 'dockerfile' && isOutside(relative(resolve(dockerContextRoot), filePath))) { + if (dockerContextRoot && field === 'dockerfile' && isOutside(relative(dockerContextRoot, filePath))) { return problem(resolvedPath, `lies outside the Docker context ${dockerContextRoot}`); } - const uploadedPath = toPosix(relative(root, filePath)); - const uploaded = uploadedFiles.map(toPosix); - if (!uploaded.includes(uploadedPath)) { - const differentCase = uploaded.find((uploadedFile) => uploadedFile.toLowerCase() === uploadedPath.toLowerCase()); + const uploadedPath = toPosix(relative(uploadRoot, filePath)); + if (!uploadedFiles.has(uploadedPath)) { + const differentCase = [...uploadedFiles].find((file) => file.toLowerCase() === uploadedPath.toLowerCase()); return problem( resolvedPath, differentCase @@ -135,7 +143,15 @@ function checkPathField( * as push uploads its content there. */ export function checkActorPaths(options: ActorPathsOptions): ActorPathProblem[] { - return collectPathFields(options.config).flatMap((pathField) => checkPathField(pathField, options) ?? []); + // The roots are real paths, as the Docker context resolver returns them, so a symlinked Actor folder does not + // count as an escape; a referenced path is judged lexically below the real Actor folder. + const scope: PathCheckScope = { + actorDir: realpathSync(options.actorDir), + uploadRoot: realpathSync(options.uploadRoot), + uploadedFiles: new Set(options.uploadedFiles.map(toPosix)), + dockerContextRoot: options.dockerContextRoot === undefined ? undefined : realpathSync(options.dockerContextRoot), + }; + return collectPathFields(options.config).flatMap((pathField) => checkPathField(pathField, scope) ?? []); } export function formatActorPathProblems(problems: ActorPathProblem[]): string { From cc9e1a8bc652ef862e2e4685a948293f9baa95b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 15:06:17 +0100 Subject: [PATCH 24/42] docs: state the BuildKit basis of the Docker context rule --- src/lib/actor-paths.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index b59b42e8e..43db6a79e 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -18,7 +18,11 @@ export interface ActorPathsOptions { /** Relative to `uploadRoot`, as the file collectors return them. */ uploadedFiles: string[]; config: Record; - /** When given, `dockerfile` must lie inside it, as BuildKit reads the Dockerfile from the context. */ + /** + * When given, `dockerfile` must lie inside it. The worker runs BuildKit with `--local dockerfile=` + * and `filename=`, so a Dockerfile outside the context cannot be read by the + * build. The same holds for a context within the Actor folder, which today's push already sends. + */ dockerContextRoot?: string; } From 39be1cbc43743b0fc4ff6e27378f5f5f66fa7a4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 16:02:47 +0100 Subject: [PATCH 25/42] docs: list the rules checkPathField applies, in order --- src/lib/actor-paths.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 43db6a79e..70bc1db45 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -86,6 +86,16 @@ interface PathCheckScope { dockerContextRoot?: string; } +/** + * Applies the platform's rules to one path field of `actor.json`, in order; the first failing rule wins. + * 1. No backslashes and not absolute: the platform resolves the value relative to `.actor/` on Linux. + * 2. Resolved from `.actor/`, it lies inside the upload root; the platform never sees anything else. + * 3. The file exists in its directory: by any case for `dockerfile`, `readme` and `changelog`, which the + * platform finds by listing the directory, and by the exact name for schema references. + * 4. It is a file, not a directory. + * 5. `dockerfile` lies inside the Docker context, as BuildKit reads it from there. + * 6. It is among the files push uploads: a file `.gitignore` or `.actorignore` drops never reaches the platform. + */ function checkPathField( { field, value, caseInsensitive }: PathField, { actorDir, uploadRoot, uploadedFiles, dockerContextRoot }: PathCheckScope, From f42dd5b1dd7fea4e87192f73678a54a32085e588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 16:04:56 +0100 Subject: [PATCH 26/42] docs: name the dangling symlink and case rules in the checkPathField list --- src/lib/actor-paths.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 70bc1db45..1747a8107 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -91,10 +91,12 @@ interface PathCheckScope { * 1. No backslashes and not absolute: the platform resolves the value relative to `.actor/` on Linux. * 2. Resolved from `.actor/`, it lies inside the upload root; the platform never sees anything else. * 3. The file exists in its directory: by any case for `dockerfile`, `readme` and `changelog`, which the - * platform finds by listing the directory, and by the exact name for schema references. + * platform finds by listing the directory, and by the exact name for schema references. A dangling symlink + * does not exist. * 4. It is a file, not a directory. * 5. `dockerfile` lies inside the Docker context, as BuildKit reads it from there. - * 6. It is among the files push uploads: a file `.gitignore` or `.actorignore` drops never reaches the platform. + * 6. It is among the files push uploads, in the same case: a file `.gitignore` or `.actorignore` drops never + * reaches the platform. */ function checkPathField( { field, value, caseInsensitive }: PathField, From 40a603cce4a789199f55bd1790a814d127a4a3d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 16:04:56 +0100 Subject: [PATCH 27/42] test: cover dangling and symlinked Dockerfile paths and a symlinked Actor folder --- test/local/lib/actor-paths.test.ts | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index 3e2a0a64f..3e06a65f7 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -2,6 +2,7 @@ import { mkdirSync, realpathSync, symlinkSync, writeFileSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { checkActorPaths, formatActorPathProblems } from '../../../src/lib/actor-paths.js'; +import { resolveDockerContext } from '../../../src/lib/docker-context.js'; import { getActorLocalFilePaths } from '../../../src/lib/utils.js'; import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; @@ -46,7 +47,10 @@ describe('checkActorPaths', () => { write('shared/Dockerfile', 'FROM apify/actor-node:20\n'); write('shared/input_schema.json', '{}'); symlinkSync(join(root, 'shared', 'input_schema.json'), join(actorDir, '.actor', 'linked_schema.json')); + symlinkSync(join(root, 'shared', 'Dockerfile'), join(actorDir, 'Dockerfile.shared')); uploadedFiles = await getActorLocalFilePaths(actorDir); + symlinkSync(join(root, 'shared', 'missing.json'), join(actorDir, '.actor', 'dangling.json')); + symlinkSync(actorDir, join(root, 'linked-actor')); }); afterAll(async () => { @@ -167,6 +171,39 @@ describe('checkActorPaths', () => { ]); }); + it('rejects a dangling symlink as missing', () => { + expect(check({ input: './dangling.json' }, { uploadedFiles: [...uploadedFiles, '.actor/dangling.json'] })).toEqual([ + { + field: 'input', + value: './dangling.json', + resolvedPath: join(actorDir, '.actor', 'dangling.json'), + reason: 'does not exist', + }, + ]); + }); + + it('judges a symlinked Dockerfile against the Docker context by its own path', () => { + // The link's target lies outside the context, but push uploads its content at the link's path inside it. + expect(uploadedFiles).toContain('Dockerfile.shared'); + expect(check({ dockerfile: '../Dockerfile.shared' }, { dockerContextRoot: actorDir })).toEqual([]); + }); + + it('accepts a Dockerfile in a context within a symlinked Actor folder', () => { + const linkedActorDir = join(root, 'linked-actor'); + const dockerContext = resolveDockerContext(linkedActorDir, '..'); + expect(dockerContext).toEqual({ kind: 'within', contextRoot: actorDir }); + + expect( + checkActorPaths({ + actorDir: linkedActorDir, + uploadRoot: linkedActorDir, + uploadedFiles, + config: { dockerfile: '../Dockerfile' }, + dockerContextRoot: actorDir, + }), + ).toEqual([]); + }); + it('matches the name of dockerfile, readme and changelog case-insensitively, and a schema exactly', () => { expect(check({ dockerfile: '../dockerfile', readme: './readme.MD', changelog: './changelog.md' })).toEqual([]); expect(check({ input: './input_schema.json' })).toEqual([ From 9a1a53dde812462588b385743b49650fe04b55b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 16:07:40 +0100 Subject: [PATCH 28/42] fix: leave links pointing outside the repository out of the monorepo upload --- src/commands/actors/push.ts | 38 ++++++++++++++++------- test/local/commands/push-monorepo.test.ts | 10 +++++- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 22d19a229..d1db4b0c0 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -1,5 +1,5 @@ import { randomUUID } from 'node:crypto'; -import { readFileSync, realpathSync, statSync, unlinkSync } from 'node:fs'; +import { lstatSync, readFileSync, realpathSync, type Stats, statSync, unlinkSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { basename, join, relative, resolve } from 'node:path'; import process from 'node:process'; @@ -170,20 +170,36 @@ const rejectsActorPath = (err: unknown) => { }; /** - * The platform's multi-file and zip sources carry files only: a link to a file goes as that file, a link to a - * directory inside the upload as copies of the uploaded files under its target, and a link to nothing is left out. + * The platform's multi-file and zip sources carry files only: a link to a file inside the upload goes as that file + * at the link's path, a link to a directory inside the upload as copies of the uploaded files under its target, and + * a link to nothing or to a target outside the upload is left out, as a clone of the repository could not follow it + * either. The `actor.json` path check then reports a config that refers to a left-out link as not uploaded. */ const resolveLinksForUpload = (root: string, files: string[]) => { - const stats = new Map(files.map((file) => [file, statSync(join(root, file), { throwIfNoEntry: false })])); + const realRoot = realpathSync(root); + // Where an entry's content lies, relative to the root, and what it is; nothing for a link the upload drops. + const locate = (file: string): { target: string; stats: Stats } | undefined => { + const filePath = join(root, file); + const stats = lstatSync(filePath, { throwIfNoEntry: false }); + if (!stats) return undefined; + if (!stats.isSymbolicLink()) return { target: file, stats }; + try { + const realPath = realpathSync(filePath); + const target = relative(realRoot, realPath); + return isOutside(target) ? undefined : { target: toPosix(target), stats: statSync(realPath) }; + } catch { + return undefined; + } + }; + const located = new Map(files.map((file) => [file, locate(file)])); return files.flatMap((file) => { - const fileStats = stats.get(file); - if (fileStats?.isFile()) return [file]; - if (!fileStats?.isDirectory()) return []; - const target = relative(realpathSync(root), realpathSync(join(root, file))); - if (isOutside(target)) return []; - const prefix = `${toPosix(target)}/`; + const entry = located.get(file); + if (!entry) return []; + if (entry.stats.isFile()) return [file]; + if (!entry.stats.isDirectory()) return []; + const prefix = `${entry.target}/`; return files - .filter((other) => other.startsWith(prefix) && stats.get(other)?.isFile()) + .filter((other) => other.startsWith(prefix) && located.get(other)?.stats.isFile()) .map((other) => `${file}/${other.slice(prefix.length)}`); }); }; diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index d9677e2b9..a6e7f804f 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -102,6 +102,9 @@ const { beforeAllCalls, afterAllCalls, joinPath, tmpPath, forceNewCwd } = useTem cwdParent: false, }); +// A sibling folder of the repository: the target of links that leave it. +const outside = useTempPath('push-monorepo-outside', { create: true, remove: true, cwd: false, cwdParent: false }); + const { logMessages } = useConsoleSpy(); const { ActorsPushCommand } = await import('../../../src/commands/actors/push.js'); @@ -154,6 +157,7 @@ beforeEach(async () => { setRecord.mockClear(); cwdCache.clear(); await beforeAllCalls(); + await outside.beforeAllCalls(); // The layout of apify/actor-monorepo-example, as its own Git repository. await write('package.json', '{"workspaces":["actors/**","packages/**"]}'); @@ -173,6 +177,7 @@ afterEach(async () => { commandProcess.exitCode = originalExitCode; process.exitCode = originalRealExitCode; await afterAllCalls(); + await outside.afterAllCalls(); }); describe('apify push of a monorepo Actor', () => { @@ -237,9 +242,11 @@ describe('apify push of a monorepo Actor', () => { expect(existsSync(joinPath('temp_file.zip'))).toBe(false); }); - it('uploads the files under a directory symlink as copies and leaves a dangling link out', async () => { + it('uploads the files under a directory symlink as copies and leaves dangling and outside links out', async () => { symlinkSync('../../shared', joinPath(`${ACTOR_PATH}/shared`)); symlinkSync('missing', joinPath('dangling')); + await writeFile(outside.joinPath('outside.txt'), 'outside\n'); + symlinkSync(outside.joinPath('outside.txt'), joinPath('outside.txt')); git('add', '.'); await testRunCommand(ActorsPushCommand, {}); @@ -250,6 +257,7 @@ describe('apify push of a monorepo Actor', () => { expect(names).toContain('shared/TypeScript_Dockerfile'); expect(names).not.toContain(`${ACTOR_PATH}/shared`); expect(names).not.toContain('dangling'); + expect(names).not.toContain('outside.txt'); }); it('warns about uncommitted changes, which the upload carries as they are', async () => { From 00755a39126e9ded15c0af62d7fad97508f182d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 16:07:40 +0100 Subject: [PATCH 29/42] test: cover schemas reached through symlinks in a monorepo push --- test/local/commands/push-monorepo.test.ts | 40 ++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index a6e7f804f..87e367c95 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -28,7 +28,7 @@ const currentVersion = { versionNumber: '0.0', sourceType: ACTOR_SOURCE_TYPES.SO interface VersionPayload { sourceType: string; - sourceFiles?: { name: string }[]; + sourceFiles?: { name: string; content?: string }[]; tarballUrl?: string; actorPath?: string; } @@ -260,6 +260,44 @@ describe('apify push of a monorepo Actor', () => { expect(names).not.toContain('outside.txt'); }); + it('accepts a schema reached through a symlink inside the repository and uploads it at the link path', async () => { + await write('shared/input_schema.json', '{"title":"Input","type":"object","properties":{}}'); + symlinkSync('../../../shared/input_schema.json', joinPath(`${ACTOR_PATH}/.actor/input_schema.json`)); + await writeActorConfig({ + dockerContextDir: '../../..', + dockerfile: '../../../shared/TypeScript_Dockerfile', + input: './input_schema.json', + }); + git('add', '.'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + const files = versionPayload(versionUpdate).sourceFiles!; + const linked = files.find((file) => file.name === `${ACTOR_PATH}/.actor/input_schema.json`); + expect(linked?.content).toContain('"title":"Input"'); + }); + + it('rejects a schema reached through a symlink leaving the repository, which the upload leaves out', async () => { + await writeFile(outside.joinPath('input_schema.json'), '{}'); + symlinkSync(outside.joinPath('input_schema.json'), joinPath(`${ACTOR_PATH}/.actor/input_schema.json`)); + await writeActorConfig({ + dockerContextDir: '../../..', + dockerfile: '../../../shared/TypeScript_Dockerfile', + input: './input_schema.json', + }); + git('add', '.'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + const stderr = logMessages.error.join('\n'); + expect(stderr).toContain('"input": "./input_schema.json"'); + expect(stderr).toContain('is not among the files apify push uploads'); + expect(actorGet).not.toHaveBeenCalled(); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + it('warns about uncommitted changes, which the upload carries as they are', async () => { await write(`${ACTOR_PATH}/src/index.ts`, 'console.log(2);\n'); From e3edc7960be8ba36f15f00c0040ac0c762b13aa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Mon, 26 Oct 2026 16:18:03 +0100 Subject: [PATCH 30/42] test: use a forward-slash absolute path so the check reads the same on Windows --- test/local/lib/actor-paths.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index 3e06a65f7..4da15336b 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -1,5 +1,5 @@ import { mkdirSync, realpathSync, symlinkSync, writeFileSync } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; +import { dirname, join } from 'node:path'; import { checkActorPaths, formatActorPathProblems } from '../../../src/lib/actor-paths.js'; import { resolveDockerContext } from '../../../src/lib/docker-context.js'; @@ -103,7 +103,7 @@ describe('checkActorPaths', () => { }); it('rejects an absolute path', () => { - const value = resolve('/x'); + const value = '/x'; expect(check({ input: value })).toEqual([ { field: 'input', value, resolvedPath: value, reason: 'is absolute; the path is relative to .actor/' }, From 50fb1b8462035599b0e096dc14f68a44c73067c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Tue, 27 Oct 2026 15:08:38 +0100 Subject: [PATCH 31/42] docs: explain why the Docker context is judged by its path as written --- src/lib/docker-context.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts index 313ec90a4..e0db40895 100644 --- a/src/lib/docker-context.ts +++ b/src/lib/docker-context.ts @@ -65,6 +65,9 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown } const realContextRoot = realpathSync(contextRoot); + // Inside or outside the Actor folder is judged by the path as written, not by where a symlink on it leads. Push + // follows a symlink and uploads its files under the link's own name: with `"dockerContextDir": "../sym"` and `sym` + // linking to a folder outside, the platform receives `sym` as a normal folder inside the Actor and builds from it. const contextPathInActor = relative(realActorDir, contextRoot); if (contextPathInActor === '' || !isOutside(contextPathInActor)) { return { kind: 'within', contextRoot: realContextRoot }; From 1af9abf48b82cc617127316af7ff8c59ee583b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Tue, 27 Oct 2026 15:08:39 +0100 Subject: [PATCH 32/42] test: pin the Docker context classification through symlinks --- test/local/lib/docker-context.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/local/lib/docker-context.test.ts b/test/local/lib/docker-context.test.ts index e5826da47..6e4a7c72f 100644 --- a/test/local/lib/docker-context.test.ts +++ b/test/local/lib/docker-context.test.ts @@ -45,6 +45,7 @@ describe('resolveDockerContext', () => { writeFileSync(joinPath('Dockerfile'), 'FROM apify/actor-node:20\n'); writeFileSync(joinPath(ACTOR_PATH, 'Dockerfile'), 'FROM apify/actor-node:20\n'); symlinkSync(ACTOR_PATH, joinPath('linked-actor')); + symlinkSync('../../shared', joinPath(ACTOR_PATH, 'sym')); execFileSync('git', ['init', '--quiet'], { cwd: tmpPath, stdio: 'ignore' }); root = realpathSync(tmpPath); }); @@ -110,6 +111,17 @@ describe('resolveDockerContext', () => { }); }); + it('judges the context by its path as written, as push uploads a symlink under its own name', () => { + // `sym` inside the Actor folder links to the shared folder outside it; push uploads it as a folder inside. + expect(resolveDockerContext(actorDir(), '../sym')).toMatchObject({ kind: 'within' }); + // This path leaves the Actor folder through a link that leads back in; the upload has no such link. + expect(resolveDockerContext(actorDir(), '../../../linked-actor/src')).toMatchObject({ + kind: 'outside', + sourceRoot: root, + actorPath: ACTOR_PATH, + }); + }); + it('rejects a context outside the Git repository', () => { expect(resolveDockerContext(actorDir(), '../../../..')).toEqual({ kind: 'invalid', From 944b6ae3be3acb4581dedf7ff3d1832c5d2f35e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Tue, 27 Oct 2026 15:37:22 +0100 Subject: [PATCH 33/42] fix: return the Docker context path as the upload lays it out --- src/lib/docker-context.ts | 24 +++++++++++++++--------- test/local/lib/docker-context.test.ts | 10 +++++++--- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/lib/docker-context.ts b/src/lib/docker-context.ts index e0db40895..1555cc48d 100644 --- a/src/lib/docker-context.ts +++ b/src/lib/docker-context.ts @@ -15,10 +15,15 @@ export type DockerContextResolution = /** `dockerContextDir` is absent: undefined, null, or an empty string. */ | { kind: 'none' } /** The context is the Actor folder itself or a directory inside it, so a push of the folder carries it. */ - | { kind: 'within'; contextRoot: string } + | { + kind: 'within'; + /** As the upload lays it out: the path as written, from the Actor folder's real path. */ + contextRoot: string; + } /** The context lies beyond the Actor folder (a monorepo Actor): above it, or elsewhere in the Git repository. */ | { kind: 'outside'; + /** As the upload lays it out: the path as written, from the Actor folder's real path. */ contextRoot: string; /** The Git repository's root, else the context. */ sourceRoot: string; @@ -32,8 +37,8 @@ export type DockerContextResolution = * `dockerContextDir` is relative to `.actor/`, as on the platform. An absent field is `none`, with no path work. A * context at or below the Actor folder is `within`: an ordinary push, which the platform builds as it is. A context * above or beside the Actor folder is `outside`: a monorepo Actor, with the source root a push would have to carry. - * A context that cannot be used is `invalid`, with the message to show. Paths come back real, as the platform sees - * them. + * A context that cannot be used is `invalid`, with the message to show. The context path comes back as the upload + * lays it out, the source root as a real path. */ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown): DockerContextResolution { if (dockerContextDir === undefined || dockerContextDir === null || dockerContextDir === '') return { kind: 'none' }; @@ -65,12 +70,13 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown } const realContextRoot = realpathSync(contextRoot); - // Inside or outside the Actor folder is judged by the path as written, not by where a symlink on it leads. Push - // follows a symlink and uploads its files under the link's own name: with `"dockerContextDir": "../sym"` and `sym` - // linking to a folder outside, the platform receives `sym` as a normal folder inside the Actor and builds from it. + // Inside or outside the Actor folder is judged, and returned, by the path as written, not by where a symlink on it + // leads. Push follows a symlink and uploads its files under the link's own name: with `"dockerContextDir": "../sym"` + // and `sym` linking to a folder outside, the platform receives `sym` as a normal folder inside the Actor and builds + // from it. const contextPathInActor = relative(realActorDir, contextRoot); if (contextPathInActor === '' || !isOutside(contextPathInActor)) { - return { kind: 'within', contextRoot: realContextRoot }; + return { kind: 'within', contextRoot }; } // The platform's clone holds nothing above the repository root. @@ -87,7 +93,7 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown } return { kind: 'outside', - contextRoot: realContextRoot, + contextRoot, sourceRoot, actorPath: toPosix(relative(sourceRoot, realActorDir)), }; @@ -100,7 +106,7 @@ export function resolveDockerContext(actorDir: string, dockerContextDir: unknown message: `"dockerContextDir" in .actor/actor.json points to ${realContextRoot}, which does not contain the Actor's folder.`, }; } - return { kind: 'outside', contextRoot: realContextRoot, sourceRoot: realContextRoot, actorPath: toPosix(actorPath) }; + return { kind: 'outside', contextRoot, sourceRoot: realContextRoot, actorPath: toPosix(actorPath) }; } function isOutside(relativePath: string): boolean { diff --git a/test/local/lib/docker-context.test.ts b/test/local/lib/docker-context.test.ts index 6e4a7c72f..4e92bbde7 100644 --- a/test/local/lib/docker-context.test.ts +++ b/test/local/lib/docker-context.test.ts @@ -60,7 +60,7 @@ describe('resolveDockerContext', () => { expect(resolveDockerContext(actorDir(), '')).toEqual({ kind: 'none' }); }); - it('reports a context at or below the Actor folder as within, with its real path', () => { + it('reports a context at or below the Actor folder as within, with its path as the upload lays it out', () => { const actorRoot = join(root, ACTOR_PATH); expect(resolveDockerContext(actorDir(), '..')).toEqual({ kind: 'within', contextRoot: actorRoot }); expect(resolveDockerContext(actorDir(), '.')).toEqual({ kind: 'within', contextRoot: join(actorRoot, '.actor') }); @@ -113,10 +113,14 @@ describe('resolveDockerContext', () => { it('judges the context by its path as written, as push uploads a symlink under its own name', () => { // `sym` inside the Actor folder links to the shared folder outside it; push uploads it as a folder inside. - expect(resolveDockerContext(actorDir(), '../sym')).toMatchObject({ kind: 'within' }); + expect(resolveDockerContext(actorDir(), '../sym')).toEqual({ + kind: 'within', + contextRoot: join(root, ACTOR_PATH, 'sym'), + }); // This path leaves the Actor folder through a link that leads back in; the upload has no such link. - expect(resolveDockerContext(actorDir(), '../../../linked-actor/src')).toMatchObject({ + expect(resolveDockerContext(actorDir(), '../../../linked-actor/src')).toEqual({ kind: 'outside', + contextRoot: join(root, 'linked-actor', 'src'), sourceRoot: root, actorPath: ACTOR_PATH, }); From ee0eda85f888b8a028ce48440304ec954a128274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Tue, 27 Oct 2026 15:38:39 +0100 Subject: [PATCH 34/42] fix: compare the Dockerfile with the Docker context as the upload lays it out --- src/lib/actor-paths.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 1747a8107..594295769 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -21,7 +21,8 @@ export interface ActorPathsOptions { /** * When given, `dockerfile` must lie inside it. The worker runs BuildKit with `--local dockerfile=` * and `filename=`, so a Dockerfile outside the context cannot be read by the - * build. The same holds for a context within the Actor folder, which today's push already sends. + * build. The same holds for a context within the Actor folder, which today's push already sends. Pass it as + * `resolveDockerContext` returns it: the path as written, from the real Actor folder. */ dockerContextRoot?: string; } @@ -159,13 +160,14 @@ function checkPathField( * as push uploads its content there. */ export function checkActorPaths(options: ActorPathsOptions): ActorPathProblem[] { - // The roots are real paths, as the Docker context resolver returns them, so a symlinked Actor folder does not - // count as an escape; a referenced path is judged lexically below the real Actor folder. + // The Actor folder and the upload root are real paths, as the Docker context resolver works from them, so a + // symlinked Actor folder does not count as an escape. A referenced path, like the Docker context the resolver + // returns, is judged as written below the real Actor folder, the way push lays out the upload. const scope: PathCheckScope = { actorDir: realpathSync(options.actorDir), uploadRoot: realpathSync(options.uploadRoot), uploadedFiles: new Set(options.uploadedFiles.map(toPosix)), - dockerContextRoot: options.dockerContextRoot === undefined ? undefined : realpathSync(options.dockerContextRoot), + dockerContextRoot: options.dockerContextRoot, }; return collectPathFields(options.config).flatMap((pathField) => checkPathField(pathField, scope) ?? []); } From d56fdf7b268a299f266ad3b1985fd16f7af385c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Tue, 27 Oct 2026 15:38:39 +0100 Subject: [PATCH 35/42] test: accept a Dockerfile in a Docker context reached through a symlink --- test/local/lib/actor-paths.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index 4da15336b..b03bb878f 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -51,6 +51,7 @@ describe('checkActorPaths', () => { uploadedFiles = await getActorLocalFilePaths(actorDir); symlinkSync(join(root, 'shared', 'missing.json'), join(actorDir, '.actor', 'dangling.json')); symlinkSync(actorDir, join(root, 'linked-actor')); + symlinkSync(join(root, 'shared'), join(actorDir, 'sym')); }); afterAll(async () => { @@ -204,6 +205,22 @@ describe('checkActorPaths', () => { ).toEqual([]); }); + it('accepts a Dockerfile in a Docker context reached through a symlink inside the Actor folder', async () => { + // `sym` links to the shared folder outside the Actor; push uploads it as a normal folder `sym` inside. + const dockerContext = resolveDockerContext(actorDir, '../sym'); + expect(dockerContext).toEqual({ kind: 'within', contextRoot: join(actorDir, 'sym') }); + + expect( + checkActorPaths({ + actorDir, + uploadRoot: actorDir, + uploadedFiles: await getActorLocalFilePaths(actorDir), + config: { dockerContextDir: '../sym', dockerfile: '../sym/Dockerfile' }, + dockerContextRoot: join(actorDir, 'sym'), + }), + ).toEqual([]); + }); + it('matches the name of dockerfile, readme and changelog case-insensitively, and a schema exactly', () => { expect(check({ dockerfile: '../dockerfile', readme: './readme.MD', changelog: './changelog.md' })).toEqual([]); expect(check({ input: './input_schema.json' })).toEqual([ From 04fb9654b31ef09072446a9a6db3479b8d5b14e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Tue, 27 Oct 2026 15:39:53 +0100 Subject: [PATCH 36/42] test: accept a Dockerfile in a monorepo Docker context reached through a symlink --- test/local/commands/push-monorepo.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index 87e367c95..ac9646411 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -278,6 +278,22 @@ describe('apify push of a monorepo Actor', () => { expect(linked?.content).toContain('"title":"Input"'); }); + it('accepts a Dockerfile in a Docker context reached through a symlink inside the repository', async () => { + // `context-link` links to the shared folder; the upload carries it as a normal folder under the link's name. + symlinkSync('shared', joinPath('context-link')); + await writeActorConfig({ + dockerContextDir: '../../../context-link', + dockerfile: '../../../context-link/TypeScript_Dockerfile', + }); + git('add', '.'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + const names = versionPayload(versionUpdate).sourceFiles!.map((file) => file.name); + expect(names).toContain('context-link/TypeScript_Dockerfile'); + }); + it('rejects a schema reached through a symlink leaving the repository, which the upload leaves out', async () => { await writeFile(outside.joinPath('input_schema.json'), '{}'); symlinkSync(outside.joinPath('input_schema.json'), joinPath(`${ACTOR_PATH}/.actor/input_schema.json`)); From cd7d2b562a8c4b32f5872232a9993db5ae8b37cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 30 Oct 2026 00:57:50 +0100 Subject: [PATCH 37/42] fix: check the default Dockerfile against the Docker context --- src/lib/actor-paths.ts | 78 +++++++++++++++++++++---- test/local/lib/actor-paths.test.ts | 94 ++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 12 deletions(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 594295769..3e19ed71e 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -6,6 +6,7 @@ import { isOutside, toPosix } from './docker-context.js'; export interface ActorPathProblem { field: string; + /** Empty when the field is not set and the problem lies with what the platform uses by default. */ value: string; resolvedPath: string; reason: string; @@ -21,7 +22,8 @@ export interface ActorPathsOptions { /** * When given, `dockerfile` must lie inside it. The worker runs BuildKit with `--local dockerfile=` * and `filename=`, so a Dockerfile outside the context cannot be read by the - * build. The same holds for a context within the Actor folder, which today's push already sends. Pass it as + * build. The same holds for a context within the Actor folder, which today's push already sends. Without + * `dockerfile`, the Dockerfile the platform would find by default is held to the same rules. Pass it as * `resolveDockerContext` returns it: the path as written, from the real Actor folder. */ dockerContextRoot?: string; @@ -36,6 +38,11 @@ const SCHEMA_FIELDS: { field: string; alias?: string }[] = [ { field: 'webServerSchema' }, ]; const STORAGE_SCHEMA_FIELDS = ['keyValueStore', 'dataset']; +/** + * Without `dockerfile`, the worker takes the first of these that exists, relative to `.actor/`; with none, it copies + * its built-in Node.js Dockerfile into the Docker context. + */ +const DEFAULT_DOCKERFILE_PATHS = ['./Dockerfile', '../Dockerfile']; interface PathField { field: string; @@ -79,6 +86,16 @@ function listDirectory(dir: string): string[] { } } +/** The entry for `filePath` in a listing of its directory, by the exact name and else by any case. */ +function findEntry(filePath: string): { sameCase?: string; anyCase?: string } { + const wanted = basename(filePath); + const entries = listDirectory(dirname(filePath)); + return { + sameCase: entries.find((entry) => entry === wanted), + anyCase: entries.find((entry) => entry.toLowerCase() === wanted.toLowerCase()), + }; +} + /** The options as every field sees them: real roots and the uploaded files as posix paths. */ interface PathCheckScope { actorDir: string; @@ -95,7 +112,7 @@ interface PathCheckScope { * platform finds by listing the directory, and by the exact name for schema references. A dangling symlink * does not exist. * 4. It is a file, not a directory. - * 5. `dockerfile` lies inside the Docker context, as BuildKit reads it from there. + * 5. `dockerfile`, set or found by default, lies inside the Docker context, as BuildKit reads it from there. * 6. It is among the files push uploads, in the same case: a file `.gitignore` or `.actorignore` drops never * reaches the platform. */ @@ -114,11 +131,7 @@ function checkPathField( return problem(resolvedPath, `lies outside the uploaded folder ${uploadRoot}`); } - const dir = dirname(resolvedPath); - const wanted = basename(resolvedPath); - const entries = listDirectory(dir); - const sameCase = entries.find((entry) => entry === wanted); - const anyCase = entries.find((entry) => entry.toLowerCase() === wanted.toLowerCase()); + const { sameCase, anyCase } = findEntry(resolvedPath); const name = sameCase ?? (caseInsensitive ? anyCase : undefined); if (!name) { return problem( @@ -129,7 +142,7 @@ function checkPathField( // push reads through a symlink and uploads the content at this path, so the path is judged as it is, not by its // target; a dangling symlink has no content to upload. - const filePath = join(dir, name); + const filePath = join(dirname(resolvedPath), name); let stats: Stats; try { stats = statSync(filePath); @@ -154,10 +167,43 @@ function checkPathField( return undefined; } +/** + * Without `dockerfile`, the platform takes the first default Dockerfile that exists, else copies its built-in one into + * the Docker context. The found default must pass the same rules as a set `dockerfile`. The built-in one is the + * documented default for a context that is the Actor folder itself; in any other context it would build files the + * built-in Dockerfile was never meant for, and silently ignore a Dockerfile the context may hold. + */ +function checkDefaultDockerfile(scope: PathCheckScope & { dockerContextRoot: string }): ActorPathProblem | undefined { + const { actorDir, dockerContextRoot } = scope; + const value = DEFAULT_DOCKERFILE_PATHS.find( + (candidate) => findEntry(resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, candidate)).anyCase, + ); + if (value) { + const problem = checkPathField({ field: 'dockerfile', value, caseInsensitive: true }, scope); + return ( + problem && { + ...problem, + value: '', + reason: `is not set, so the platform would use the default ${problem.resolvedPath}, which ${problem.reason}`, + } + ); + } + if (relative(actorDir, dockerContextRoot) === '') return undefined; + return { + field: 'dockerfile', + value: '', + resolvedPath: join(dockerContextRoot, 'Dockerfile'), + reason: + `is not set and there is no ${ACTOR_SPECIFICATION_FOLDER}/Dockerfile or Dockerfile in the Actor folder, ` + + `so the platform would build the Docker context ${dockerContextRoot} with its built-in Node.js Dockerfile; ` + + 'add a Dockerfile, or set "dockerfile" to one inside the Docker context', + }; +} + /** * Applies the platform's rules to every file path in `actor.json` before anything is uploaded: relative to * `.actor/`, inside the upload root, an existing file, and one that push sends. A symlink counts by its own path, - * as push uploads its content there. + * as push uploads its content there. With a Docker context and no `dockerfile`, the default Dockerfile is checked too. */ export function checkActorPaths(options: ActorPathsOptions): ActorPathProblem[] { // The Actor folder and the upload root are real paths, as the Docker context resolver works from them, so a @@ -169,14 +215,22 @@ export function checkActorPaths(options: ActorPathsOptions): ActorPathProblem[] uploadedFiles: new Set(options.uploadedFiles.map(toPosix)), dockerContextRoot: options.dockerContextRoot, }; - return collectPathFields(options.config).flatMap((pathField) => checkPathField(pathField, scope) ?? []); + const pathFields = collectPathFields(options.config); + const problems = pathFields.flatMap((pathField) => checkPathField(pathField, scope) ?? []); + + const { dockerContextRoot } = scope; + if (dockerContextRoot && !pathFields.some(({ field }) => field === 'dockerfile')) { + const problem = checkDefaultDockerfile({ ...scope, dockerContextRoot }); + if (problem) problems.push(problem); + } + return problems; } export function formatActorPathProblems(problems: ActorPathProblem[]): string { return [ '.actor/actor.json refers to files the Apify platform could not use:', - ...problems.map( - ({ field, value, resolvedPath, reason }) => ` "${field}": "${value}" (${resolvedPath}) ${reason}.`, + ...problems.map(({ field, value, resolvedPath, reason }) => + value ? ` "${field}": "${value}" (${resolvedPath}) ${reason}.` : ` "${field}" ${reason}.`, ), `Paths are relative to ${ACTOR_SPECIFICATION_FOLDER}/ and must point to files inside the uploaded folder.`, ].join('\n'); diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index b03bb878f..0e30c681b 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -271,6 +271,100 @@ describe('checkActorPaths', () => { }); }); +describe('checkActorPaths without dockerfile', () => { + const { tmpPath, beforeAllCalls, afterAllCalls } = useTempPath('actor-paths-default-dockerfile-test-dir', { + create: true, + remove: true, + cwd: false, + cwdParent: false, + }); + + let root: string; + + beforeAll(async () => { + await beforeAllCalls(); + root = realpathSync(tmpPath); + }); + + afterAll(async () => { + await afterAllCalls(); + }); + + // A fresh Actor folder with the given files, checked as push checks it: the folder is the upload root. + const checkActor = async ( + name: string, + files: string[], + dockerContextDir: string | undefined, + config: Record = {}, + ) => { + const actorDir = join(root, name); + for (const file of ['.actor/actor.json', ...files]) { + mkdirSync(dirname(join(actorDir, file)), { recursive: true }); + writeFileSync(join(actorDir, file), 'FROM apify/actor-node:20\n'); + } + const dockerContext = resolveDockerContext(actorDir, dockerContextDir); + const problems = checkActorPaths({ + actorDir, + uploadRoot: actorDir, + uploadedFiles: await getActorLocalFilePaths(actorDir), + config: { dockerContextDir, ...config }, + dockerContextRoot: dockerContext.kind === 'within' ? dockerContext.contextRoot : undefined, + }); + return { actorDir, problems }; + }; + + it('rejects a default Dockerfile outside the Docker context', async () => { + const { actorDir, problems } = await checkActor('outside', ['Dockerfile', 'docker/Dockerfile'], '../docker'); + + expect(problems).toEqual([ + { + field: 'dockerfile', + value: '', + resolvedPath: join(actorDir, 'Dockerfile'), + reason: `is not set, so the platform would use the default ${join(actorDir, 'Dockerfile')}, which lies outside the Docker context ${join(actorDir, 'docker')}`, + }, + ]); + }); + + it('rejects the built-in Dockerfile in a Docker context other than the Actor folder', async () => { + const { actorDir, problems } = await checkActor('built-in', ['docker/Dockerfile'], '../docker'); + + expect(problems).toEqual([ + { + field: 'dockerfile', + value: '', + resolvedPath: join(actorDir, 'docker', 'Dockerfile'), + reason: expect.stringContaining( + `so the platform would build the Docker context ${join(actorDir, 'docker')} with its built-in Node.js Dockerfile`, + ), + }, + ]); + }); + + it('accepts the built-in Dockerfile when the Docker context is the Actor folder', async () => { + expect((await checkActor('actor-context', ['src/main.js'], '..')).problems).toEqual([]); + }); + + it('does nothing without a Docker context', async () => { + expect((await checkActor('no-context', ['src/main.js'], undefined)).problems).toEqual([]); + expect((await checkActor('no-context-outside', ['Dockerfile', 'docker/Dockerfile'], undefined)).problems).toEqual( + [], + ); + }); + + it('accepts a default .actor/Dockerfile inside the Docker context, found by any case and before Dockerfile', async () => { + expect((await checkActor('in-actor-folder', ['.actor/dockerfile', 'Dockerfile'], '.')).problems).toEqual([]); + }); + + it('leaves a set dockerfile to the usual rules', async () => { + const { problems } = await checkActor('set', ['Dockerfile', 'docker/Dockerfile'], '../docker', { + dockerfile: '../docker/Dockerfile', + }); + + expect(problems).toEqual([]); + }); +}); + describe('formatActorPathProblems', () => { it('lists every problem with its field, value, resolved path and reason', () => { const message = formatActorPathProblems([ From 9b8ef37b3237f35791397e9db5eb98e321f19b7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 30 Oct 2026 00:59:27 +0100 Subject: [PATCH 38/42] fix: warn instead of failing when the platform falls back to its default README or CHANGELOG --- src/commands/actors/push.ts | 8 +- src/lib/actor-paths.ts | 48 +++++++++-- test/local/lib/actor-paths.test.ts | 132 ++++++++++++++++++++++++++--- 3 files changed, 166 insertions(+), 22 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index f096b50ec..25d9ff8ad 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -308,8 +308,12 @@ export class ActorsPushCommand extends ApifyCommand { config: actorConfig, dockerContextRoot: dockerContext.kind === 'within' ? dockerContext.contextRoot : undefined, }); - if (pathProblems.length > 0) { - error({ message: formatActorPathProblems(pathProblems) }); + // A readme or changelog the platform cannot find only falls back to its default, so push goes on. + const pathWarnings = formatActorPathProblems(pathProblems, 'warning'); + if (pathWarnings) warning({ message: pathWarnings }); + const pathErrors = formatActorPathProblems(pathProblems, 'error'); + if (pathErrors) { + error({ message: pathErrors }); process.exitCode = CommandExitCodes.InvalidActorJson; return; } diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 3e19ed71e..38ba6c18e 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -4,12 +4,19 @@ import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:pat import { ACTOR_SPECIFICATION_FOLDER } from './consts.js'; import { isOutside, toPosix } from './docker-context.js'; +/** + * An `error` would fail the build or build the wrong thing, so push stops. A `warning` is a path the platform cannot + * find but recovers from with its default lookup, so the build succeeds and push continues. + */ +export type ActorPathSeverity = 'error' | 'warning'; + export interface ActorPathProblem { field: string; /** Empty when the field is not set and the problem lies with what the platform uses by default. */ value: string; resolvedPath: string; reason: string; + severity: ActorPathSeverity; } export interface ActorPathsOptions { @@ -38,6 +45,12 @@ const SCHEMA_FIELDS: { field: string; alias?: string }[] = [ { field: 'webServerSchema' }, ]; const STORAGE_SCHEMA_FIELDS = ['keyValueStore', 'dataset']; +/** + * When the worker cannot find the file these name, it logs a warning and falls back to its default lookup + * (`.actor/ACTOR.md`, `.actor/README.md`, then `README.md` or `README` in the Actor folder; likewise for the + * changelog), so the build succeeds. A missing `dockerfile` falls back too, but to a Dockerfile the user did not choose. + */ +const FALLBACK_FIELDS = new Set(['readme', 'changelog']); /** * Without `dockerfile`, the worker takes the first of these that exists, relative to `.actor/`; with none, it copies * its built-in Node.js Dockerfile into the Docker context. @@ -115,14 +128,24 @@ interface PathCheckScope { * 5. `dockerfile`, set or found by default, lies inside the Docker context, as BuildKit reads it from there. * 6. It is among the files push uploads, in the same case: a file `.gitignore` or `.actorignore` drops never * reaches the platform. + * + * Every problem is an error, except for `readme` and `changelog` failing rules 1 (a backslash), 3, or 6: the worker + * does not find such a file and falls back to its default, so these are warnings. An absolute path, a path outside + * the upload root, or a directory fails the build for those fields too. */ function checkPathField( { field, value, caseInsensitive }: PathField, { actorDir, uploadRoot, uploadedFiles, dockerContextRoot }: PathCheckScope, ): ActorPathProblem | undefined { - const problem = (resolvedPath: string, reason: string) => ({ field, value, resolvedPath, reason }); + const problem = (resolvedPath: string, reason: string, platformFallsBack = false): ActorPathProblem => ({ + field, + value, + resolvedPath, + reason, + severity: platformFallsBack && FALLBACK_FIELDS.has(field) ? 'warning' : 'error', + }); if (value.includes('\\')) { - return problem(value, 'uses a backslash; the platform resolves paths with forward slashes'); + return problem(value, 'uses a backslash; the platform resolves paths with forward slashes', true); } if (isAbsolute(value)) return problem(value, `is absolute; the path is relative to ${ACTOR_SPECIFICATION_FOLDER}/`); @@ -137,6 +160,7 @@ function checkPathField( return problem( resolvedPath, anyCase ? `does not exist (the name must match ${anyCase} exactly)` : 'does not exist', + true, ); } @@ -147,7 +171,7 @@ function checkPathField( try { stats = statSync(filePath); } catch { - return problem(resolvedPath, 'does not exist'); + return problem(resolvedPath, 'does not exist', true); } if (stats.isDirectory()) return problem(resolvedPath, 'is a directory, not a file'); if (dockerContextRoot && field === 'dockerfile' && isOutside(relative(dockerContextRoot, filePath))) { @@ -162,6 +186,7 @@ function checkPathField( differentCase ? `does not match the case of the uploaded file ${differentCase}` : 'is not among the files apify push uploads (excluded by .gitignore or .actorignore?)', + true, ); } return undefined; @@ -197,6 +222,7 @@ function checkDefaultDockerfile(scope: PathCheckScope & { dockerContextRoot: str `is not set and there is no ${ACTOR_SPECIFICATION_FOLDER}/Dockerfile or Dockerfile in the Actor folder, ` + `so the platform would build the Docker context ${dockerContextRoot} with its built-in Node.js Dockerfile; ` + 'add a Dockerfile, or set "dockerfile" to one inside the Docker context', + severity: 'error', }; } @@ -226,10 +252,20 @@ export function checkActorPaths(options: ActorPathsOptions): ActorPathProblem[] return problems; } -export function formatActorPathProblems(problems: ActorPathProblem[]): string { +const PROBLEM_HEADERS: Record = { + error: '.actor/actor.json refers to files the Apify platform could not use:', + warning: + '.actor/actor.json refers to files the Apify platform will not find. ' + + 'The push continues, and the platform falls back to its default README or CHANGELOG:', +}; + +/** The message for the problems of one severity, with a header that says what happens; empty when there are none. */ +export function formatActorPathProblems(problems: ActorPathProblem[], severity: ActorPathSeverity): string { + const selected = problems.filter((problem) => problem.severity === severity); + if (selected.length === 0) return ''; return [ - '.actor/actor.json refers to files the Apify platform could not use:', - ...problems.map(({ field, value, resolvedPath, reason }) => + PROBLEM_HEADERS[severity], + ...selected.map(({ field, value, resolvedPath, reason }) => value ? ` "${field}": "${value}" (${resolvedPath}) ${reason}.` : ` "${field}" ${reason}.`, ), `Paths are relative to ${ACTOR_SPECIFICATION_FOLDER}/ and must point to files inside the uploaded folder.`, diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index 0e30c681b..49cc2fce4 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -1,7 +1,7 @@ import { mkdirSync, realpathSync, symlinkSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; -import { checkActorPaths, formatActorPathProblems } from '../../../src/lib/actor-paths.js'; +import { type ActorPathProblem, checkActorPaths, formatActorPathProblems } from '../../../src/lib/actor-paths.js'; import { resolveDockerContext } from '../../../src/lib/docker-context.js'; import { getActorLocalFilePaths } from '../../../src/lib/utils.js'; import { useTempPath } from '../../__setup__/hooks/useTempPath.js'; @@ -84,6 +84,7 @@ describe('checkActorPaths', () => { value: '../../../shared/Dockerfile', resolvedPath: join(root, 'shared', 'Dockerfile'), reason: `lies outside the uploaded folder ${actorDir}`, + severity: 'error', }, ]); }); @@ -99,6 +100,7 @@ describe('checkActorPaths', () => { value: '..\\Dockerfile', resolvedPath: '..\\Dockerfile', reason: 'uses a backslash; the platform resolves paths with forward slashes', + severity: 'error', }, ]); }); @@ -107,7 +109,13 @@ describe('checkActorPaths', () => { const value = '/x'; expect(check({ input: value })).toEqual([ - { field: 'input', value, resolvedPath: value, reason: 'is absolute; the path is relative to .actor/' }, + { + field: 'input', + value, + resolvedPath: value, + reason: 'is absolute; the path is relative to .actor/', + severity: 'error', + }, ]); }); @@ -118,6 +126,7 @@ describe('checkActorPaths', () => { value: './missing.json', resolvedPath: join(actorDir, '.actor', 'missing.json'), reason: 'does not exist', + severity: 'error', }, ]); }); @@ -129,6 +138,7 @@ describe('checkActorPaths', () => { value: '../src', resolvedPath: join(actorDir, 'src'), reason: 'is a directory, not a file', + severity: 'error', }, ]); }); @@ -140,6 +150,7 @@ describe('checkActorPaths', () => { value: '../Dockerfile.ci', resolvedPath: join(actorDir, 'Dockerfile.ci'), reason: 'is not among the files apify push uploads (excluded by .gitignore or .actorignore?)', + severity: 'error', }, ]); }); @@ -152,6 +163,7 @@ describe('checkActorPaths', () => { value: './README.md', resolvedPath: join(actorDir, '.actor', 'README.md'), reason: 'does not match the case of the uploaded file .actor/readme.md', + severity: 'warning', }, ]); }); @@ -168,6 +180,7 @@ describe('checkActorPaths', () => { value: './linked_schema.json', resolvedPath: join(actorDir, '.actor', 'linked_schema.json'), reason: 'is not among the files apify push uploads (excluded by .gitignore or .actorignore?)', + severity: 'error', }, ]); }); @@ -179,6 +192,7 @@ describe('checkActorPaths', () => { value: './dangling.json', resolvedPath: join(actorDir, '.actor', 'dangling.json'), reason: 'does not exist', + severity: 'error', }, ]); }); @@ -229,6 +243,7 @@ describe('checkActorPaths', () => { value: './input_schema.json', resolvedPath: join(actorDir, '.actor', 'input_schema.json'), reason: 'does not exist (the name must match INPUT_SCHEMA.json exactly)', + severity: 'error', }, ]); }); @@ -254,6 +269,62 @@ describe('checkActorPaths', () => { expect(check({ input: './missing.json', inputSchema: '' }).map((problem) => problem.field)).toEqual(['input']); }); + it('warns about a readme or changelog the platform falls back from', () => { + const severities = (config: Record) => + check(config, { uploadedFiles: [...uploadedFiles, '.actor/dangling.json'] }).map( + ({ field, reason, severity }) => ({ field, reason, severity }), + ); + + expect(severities({ readme: '../README.md', changelog: '../CHANGELOG.md' })).toEqual([ + { field: 'readme', reason: 'does not exist', severity: 'warning' }, + { field: 'changelog', reason: 'does not exist', severity: 'warning' }, + ]); + expect(severities({ readme: './dangling.json' })).toEqual([ + { field: 'readme', reason: 'does not exist', severity: 'warning' }, + ]); + expect(severities({ changelog: '../Dockerfile.ci' })).toEqual([ + { + field: 'changelog', + reason: 'is not among the files apify push uploads (excluded by .gitignore or .actorignore?)', + severity: 'warning', + }, + ]); + expect(severities({ readme: '.\\README.md' })).toEqual([ + { + field: 'readme', + reason: 'uses a backslash; the platform resolves paths with forward slashes', + severity: 'warning', + }, + ]); + }); + + it('rejects a readme or changelog that fails the build', () => { + expect( + check({ readme: '/README.md', changelog: '../../../shared/Dockerfile' }).map(({ field, severity }) => ({ + field, + severity, + })), + ).toEqual([ + { field: 'readme', severity: 'error' }, + { field: 'changelog', severity: 'error' }, + ]); + expect(check({ readme: '../src' }).map(({ reason, severity }) => ({ reason, severity }))).toEqual([ + { reason: 'is a directory, not a file', severity: 'error' }, + ]); + }); + + it('rejects a missing dockerfile or schema, which the platform does not recover from', () => { + expect( + check({ dockerfile: '../Dockerfile.prod', input: './missing.json' }).map(({ field, severity }) => ({ + field, + severity, + })), + ).toEqual([ + { field: 'dockerfile', severity: 'error' }, + { field: 'input', severity: 'error' }, + ]); + }); + it('requires dockerfile inside the Docker context when one is given', () => { const monorepoFiles = [`${ACTOR_PATH}/Dockerfile`, `${ACTOR_PATH}/.actor/actor.json`, 'shared/Dockerfile']; const monorepo = { uploadRoot: root, uploadedFiles: monorepoFiles, dockerContextRoot: join(root, 'shared') }; @@ -265,6 +336,7 @@ describe('checkActorPaths', () => { value: '../Dockerfile', resolvedPath: join(actorDir, 'Dockerfile'), reason: `lies outside the Docker context ${join(root, 'shared')}`, + severity: 'error', }, ]); expect(check({ dockerfile: '../Dockerfile' }, { ...monorepo, dockerContextRoot: root })).toEqual([]); @@ -322,6 +394,7 @@ describe('checkActorPaths without dockerfile', () => { value: '', resolvedPath: join(actorDir, 'Dockerfile'), reason: `is not set, so the platform would use the default ${join(actorDir, 'Dockerfile')}, which lies outside the Docker context ${join(actorDir, 'docker')}`, + severity: 'error', }, ]); }); @@ -337,6 +410,7 @@ describe('checkActorPaths without dockerfile', () => { reason: expect.stringContaining( `so the platform would build the Docker context ${join(actorDir, 'docker')} with its built-in Node.js Dockerfile`, ), + severity: 'error', }, ]); }); @@ -366,24 +440,54 @@ describe('checkActorPaths without dockerfile', () => { }); describe('formatActorPathProblems', () => { - it('lists every problem with its field, value, resolved path and reason', () => { - const message = formatActorPathProblems([ - { - field: 'dockerfile', - value: '../Dockerfile.prod', - resolvedPath: '/actor/Dockerfile.prod', - reason: 'does not exist', - }, - { field: 'input', value: '/schema.json', resolvedPath: '/schema.json', reason: 'is absolute' }, - ]); - - expect(message).toBe( + const problems: ActorPathProblem[] = [ + { + field: 'dockerfile', + value: '../Dockerfile.prod', + resolvedPath: '/actor/Dockerfile.prod', + reason: 'does not exist', + severity: 'error', + }, + { + field: 'readme', + value: './README.md', + resolvedPath: '/actor/.actor/README.md', + reason: 'does not exist', + severity: 'warning', + }, + { field: 'input', value: '/schema.json', resolvedPath: '/schema.json', reason: 'is absolute', severity: 'error' }, + { + field: 'dockerfile', + value: '', + resolvedPath: '/actor/Dockerfile', + reason: 'is not set, so the platform would use the default /actor/Dockerfile', + severity: 'error', + }, + ]; + + it('lists the errors with their field, value, resolved path and reason', () => { + expect(formatActorPathProblems(problems, 'error')).toBe( [ '.actor/actor.json refers to files the Apify platform could not use:', ' "dockerfile": "../Dockerfile.prod" (/actor/Dockerfile.prod) does not exist.', ' "input": "/schema.json" (/schema.json) is absolute.', + ' "dockerfile" is not set, so the platform would use the default /actor/Dockerfile.', + 'Paths are relative to .actor/ and must point to files inside the uploaded folder.', + ].join('\n'), + ); + }); + + it('lists the warnings under a header that says the push continues', () => { + expect(formatActorPathProblems(problems, 'warning')).toBe( + [ + '.actor/actor.json refers to files the Apify platform will not find. The push continues, and the platform falls back to its default README or CHANGELOG:', + ' "readme": "./README.md" (/actor/.actor/README.md) does not exist.', 'Paths are relative to .actor/ and must point to files inside the uploaded folder.', ].join('\n'), ); }); + + it('returns an empty string when no problem has the severity', () => { + expect(formatActorPathProblems(problems.slice(0, 1), 'warning')).toBe(''); + }); }); From 7f0a49c1c75d246649bb42522e1495cf282e74ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 30 Oct 2026 01:00:10 +0100 Subject: [PATCH 39/42] test: cover the default Dockerfile and the README and CHANGELOG warnings in apify push --- test/local/commands/push-actor-paths.test.ts | 117 ++++++++++++++++++- test/local/commands/push-monorepo.test.ts | 16 ++- 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/test/local/commands/push-actor-paths.test.ts b/test/local/commands/push-actor-paths.test.ts index 0a243c141..e6631810e 100644 --- a/test/local/commands/push-actor-paths.test.ts +++ b/test/local/commands/push-actor-paths.test.ts @@ -1,4 +1,4 @@ -import { mkdir, writeFile } from 'node:fs/promises'; +import { mkdir, rm, writeFile } from 'node:fs/promises'; import { dirname } from 'node:path'; import { ACTOR_SOURCE_TYPES } from '@apify/consts'; @@ -113,6 +113,8 @@ describe('apify push with file paths in actor.json', () => { const stderr = logMessages.error.join('\n'); expect(stderr).toContain('.actor/actor.json refers to files the Apify platform could not use:'); expect(stderr).toContain(`"dockerfile": "../Dockerfile.prod" (${joinPath('Dockerfile.prod')}) does not exist.`); + // The platform falls back from a readme it cannot find, so that one is only a warning. + expect(stderr).toContain('The push continues, and the platform falls back to its default README or CHANGELOG:'); expect(stderr).toContain( `"readme": "../Dockerfile.ci" (${joinPath('Dockerfile.ci')}) is not among the files apify push uploads (excluded by .gitignore or .actorignore?).`, ); @@ -148,6 +150,119 @@ describe('apify push with file paths in actor.json', () => { expect(versionUpdate).not.toHaveBeenCalled(); }); + it.each([ + ['dockerfile', '../Dockerfile.prod', 'Dockerfile.prod'], + ['input', './missing.json', '.actor/missing.json'], + ])('stops on a missing %s file', async (field, value, resolvedPath) => { + await writeActorConfig({ [field]: value }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.error.join('\n')).toContain( + `"${field}": "${value}" (${joinPath(resolvedPath)}) does not exist.`, + ); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it.each([ + ['absolute', '/README.md', 'is absolute'], + ['outside the uploaded folder', '../../README.md', 'lies outside the uploaded folder'], + ])('stops on an %s readme, which fails the build', async (_, readme, reason) => { + await writeActorConfig({ readme }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + const stderr = logMessages.error.join('\n'); + expect(stderr).toContain('.actor/actor.json refers to files the Apify platform could not use:'); + expect(stderr).toContain(`"readme": "${readme}"`); + expect(stderr).toContain(reason); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it('warns about a readme only in the Actor folder and pushes, keeping stdout clean with --json', async () => { + // The shape of the js-cypress template: the platform finds no .actor/README.md and falls back to README.md. + await write('README.md', '# Actor\n'); + await writeActorConfig({ readme: './README.md', dockerfile: '../Dockerfile' }); + + await testRunCommand(ActorsPushCommand, { flags_json: true }); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(versionUpdate).toHaveBeenCalledTimes(1); + const stderr = logMessages.error.join('\n'); + expect(stderr).toContain('The push continues, and the platform falls back to its default README or CHANGELOG:'); + expect(stderr).toContain(`"readme": "./README.md" (${joinPath('.actor', 'README.md')}) does not exist.`); + expect(stderr).not.toContain('could not use'); + const stdout = logMessages.log.join('\n'); + expect(stdout).not.toContain('README'); + expect(() => JSON.parse(stdout)).not.toThrow(); + }); + + it('warns about a missing changelog and pushes', async () => { + await writeActorConfig({ changelog: '../CHANGELOG.md' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(versionUpdate).toHaveBeenCalledTimes(1); + expect(logMessages.error.join('\n')).toContain( + `"changelog": "../CHANGELOG.md" (${joinPath('CHANGELOG.md')}) does not exist.`, + ); + expect(logMessages.log.join('\n')).toContain('Apify push result: SUCCEEDED'); + }); + + it('stops when the default Dockerfile lies outside the Docker context', async () => { + await write('docker/Dockerfile', 'FROM apify/actor-node:20\n'); + await writeActorConfig({ dockerContextDir: '../docker' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.error.join('\n')).toContain( + `"dockerfile" is not set, so the platform would use the default ${joinPath('Dockerfile')}, which lies outside the Docker context ${joinPath('docker')}.`, + ); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it('stops when the platform would build a Docker context with its built-in Dockerfile', async () => { + await rm(joinPath('Dockerfile')); + await write('docker/Dockerfile', 'FROM apify/actor-node:20\n'); + await writeActorConfig({ dockerContextDir: '../docker' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.error.join('\n')).toContain( + `"dockerfile" is not set and there is no .actor/Dockerfile or Dockerfile in the Actor folder, so the platform would build the Docker context ${joinPath('docker')} with its built-in Node.js Dockerfile`, + ); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + + it.each([ + ['the Actor folder as the Docker context', { dockerContextDir: '..' }], + ['no Docker context', {}], + ])('pushes without any Dockerfile and with %s', async (_, fields) => { + await rm(joinPath('Dockerfile')); + await writeActorConfig(fields); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(logMessages.error.join('\n')).not.toContain('actor.json refers to files'); + expect(versionUpdate).toHaveBeenCalledTimes(1); + }); + + it('pushes with a default .actor/Dockerfile inside the Docker context', async () => { + await write('.actor/Dockerfile', 'FROM apify/actor-node:20\n'); + await writeActorConfig({ dockerContextDir: '.' }); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + expect(versionUpdate).toHaveBeenCalledTimes(1); + }); + it('pushes as usual when every path points to an uploaded file', async () => { await writeActorConfig({ dockerfile: '../dockerfile', input: './INPUT_SCHEMA.json' }); diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index d1813d373..d2148be35 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -170,9 +170,15 @@ describe('apify push of a monorepo Actor', () => { }); it.each([ - ['lies inside it', { dockerContextDir: '../src' }], - ['is not set', {}], - ])('pushes the Actor folder as usual when the Docker context %s', async (_, fields) => { + ['is the Actor folder itself', { dockerContextDir: '..' }, []], + [ + 'lies inside it, with a Dockerfile there', + { dockerContextDir: '../src', dockerfile: '../src/Dockerfile' }, + ['src/Dockerfile'], + ], + ['is not set', {}, []], + ])('pushes the Actor folder as usual when the Docker context %s', async (_, fields, extraFiles) => { + for (const file of extraFiles) await write(`${ACTOR_PATH}/${file}`, 'FROM apify/actor-node:20\n'); await writeActorConfig(fields); await testRunCommand(ActorsPushCommand, {}); @@ -182,7 +188,9 @@ describe('apify push of a monorepo Actor', () => { expect(versionUpdate).toHaveBeenCalledTimes(1); const modifier = versionUpdate.mock.calls[0][0] as { sourceType: string; sourceFiles: { name: string }[] }; expect(modifier.sourceType).toBe(ACTOR_SOURCE_TYPES.SOURCE_FILES); - expect(modifier.sourceFiles.map((file) => file.name).sort()).toEqual(['.actor/actor.json', 'src/index.ts']); + expect(modifier.sourceFiles.map((file) => file.name).sort()).toEqual( + ['.actor/actor.json', 'src/index.ts', ...extraFiles].sort(), + ); expect(logMessages.log.join('\n')).toContain('Apify push result: SUCCEEDED'); }); }); From 89b781149b2c1422ff5c0740ff6886ca3ea987ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 30 Oct 2026 01:07:00 +0100 Subject: [PATCH 40/42] fix: read actor.json paths and the default Dockerfile as the platform does An absolute or escaping readme or changelog fails the build even with a backslash in it, so it stays an error. A default Dockerfile that push does not upload is skipped, as the platform never sees it. --- src/lib/actor-paths.ts | 58 +++++++++++++------- test/local/commands/push-actor-paths.test.ts | 2 +- test/local/lib/actor-paths.test.ts | 35 ++++++++++++ 3 files changed, 73 insertions(+), 22 deletions(-) diff --git a/src/lib/actor-paths.ts b/src/lib/actor-paths.ts index 38ba6c18e..721a405ae 100644 --- a/src/lib/actor-paths.ts +++ b/src/lib/actor-paths.ts @@ -1,5 +1,5 @@ import { readdirSync, realpathSync, type Stats, statSync } from 'node:fs'; -import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'; +import { basename, dirname, isAbsolute, join, posix, relative, resolve } from 'node:path'; import { ACTOR_SPECIFICATION_FOLDER } from './consts.js'; import { isOutside, toPosix } from './docker-context.js'; @@ -119,8 +119,9 @@ interface PathCheckScope { /** * Applies the platform's rules to one path field of `actor.json`, in order; the first failing rule wins. - * 1. No backslashes and not absolute: the platform resolves the value relative to `.actor/` on Linux. - * 2. Resolved from `.actor/`, it lies inside the upload root; the platform never sees anything else. + * 1. Not absolute, and resolved from `.actor/`, inside the upload root; the platform never sees anything else. The + * platform resolves the value on Linux, where a backslash is part of a name, so this rule reads it that way first. + * 2. No backslashes: on Linux, such a name matches no file a push from another system uploads. * 3. The file exists in its directory: by any case for `dockerfile`, `readme` and `changelog`, which the * platform finds by listing the directory, and by the exact name for schema references. A dangling symlink * does not exist. @@ -129,7 +130,7 @@ interface PathCheckScope { * 6. It is among the files push uploads, in the same case: a file `.gitignore` or `.actorignore` drops never * reaches the platform. * - * Every problem is an error, except for `readme` and `changelog` failing rules 1 (a backslash), 3, or 6: the worker + * Every problem is an error, except for `readme` and `changelog` failing rules 2 (a backslash), 3, or 6: the worker * does not find such a file and falls back to its default, so these are warnings. An absolute path, a path outside * the upload root, or a directory fails the build for those fields too. */ @@ -144,15 +145,20 @@ function checkPathField( reason, severity: platformFallsBack && FALLBACK_FIELDS.has(field) ? 'warning' : 'error', }); + const absolute = () => problem(value, `is absolute; the path is relative to ${ACTOR_SPECIFICATION_FOLDER}/`); + const resolvedPath = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, value); + const outside = () => problem(resolvedPath, `lies outside the uploaded folder ${uploadRoot}`); + + // As the platform reads the value: an absolute or escaping path fails the build, even with a backslash in it. + if (value.startsWith('/')) return absolute(); + const uploadedPath = posix.join(toPosix(relative(uploadRoot, actorDir)), ACTOR_SPECIFICATION_FOLDER, value); + if (uploadedPath === '..' || uploadedPath.startsWith('../')) return outside(); if (value.includes('\\')) { return problem(value, 'uses a backslash; the platform resolves paths with forward slashes', true); } - if (isAbsolute(value)) return problem(value, `is absolute; the path is relative to ${ACTOR_SPECIFICATION_FOLDER}/`); - - const resolvedPath = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, value); - if (isOutside(relative(uploadRoot, resolvedPath))) { - return problem(resolvedPath, `lies outside the uploaded folder ${uploadRoot}`); - } + // As this system reads it, for a Windows drive path. + if (isAbsolute(value)) return absolute(); + if (isOutside(relative(uploadRoot, resolvedPath))) return outside(); const { sameCase, anyCase } = findEntry(resolvedPath); const name = sameCase ?? (caseInsensitive ? anyCase : undefined); @@ -178,9 +184,9 @@ function checkPathField( return problem(resolvedPath, `lies outside the Docker context ${dockerContextRoot}`); } - const uploadedPath = toPosix(relative(uploadRoot, filePath)); - if (!uploadedFiles.has(uploadedPath)) { - const differentCase = [...uploadedFiles].find((file) => file.toLowerCase() === uploadedPath.toLowerCase()); + const uploadedFilePath = toPosix(relative(uploadRoot, filePath)); + if (!uploadedFiles.has(uploadedFilePath)) { + const differentCase = [...uploadedFiles].find((file) => file.toLowerCase() === uploadedFilePath.toLowerCase()); return problem( resolvedPath, differentCase @@ -192,17 +198,27 @@ function checkPathField( return undefined; } +/** Whether push sends this file, or files inside this directory, so the platform finds it among the uploaded files. */ +function isUploaded(filePath: string, { uploadRoot, uploadedFiles }: PathCheckScope): boolean { + const uploadedPath = toPosix(relative(uploadRoot, filePath)); + return uploadedFiles.has(uploadedPath) || [...uploadedFiles].some((file) => file.startsWith(`${uploadedPath}/`)); +} + /** - * Without `dockerfile`, the platform takes the first default Dockerfile that exists, else copies its built-in one into - * the Docker context. The found default must pass the same rules as a set `dockerfile`. The built-in one is the - * documented default for a context that is the Actor folder itself; in any other context it would build files the - * built-in Dockerfile was never meant for, and silently ignore a Dockerfile the context may hold. + * Without `dockerfile`, the platform takes the first default Dockerfile among the uploaded files, else copies its + * built-in one into the Docker context. A default that `.gitignore` or `.actorignore` drops never reaches the + * platform, so the lookup moves on to the next one. The found default must pass the same rules as a set `dockerfile`. + * The built-in one is the documented default for a context that is the Actor folder itself; in any other context it + * would build files the built-in Dockerfile was never meant for, and silently ignore a Dockerfile the context may hold. */ function checkDefaultDockerfile(scope: PathCheckScope & { dockerContextRoot: string }): ActorPathProblem | undefined { const { actorDir, dockerContextRoot } = scope; - const value = DEFAULT_DOCKERFILE_PATHS.find( - (candidate) => findEntry(resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, candidate)).anyCase, - ); + const value = DEFAULT_DOCKERFILE_PATHS.find((candidate) => { + const candidatePath = resolve(actorDir, ACTOR_SPECIFICATION_FOLDER, candidate); + const { sameCase, anyCase } = findEntry(candidatePath); + const name = sameCase ?? anyCase; + return name !== undefined && isUploaded(join(dirname(candidatePath), name), scope); + }); if (value) { const problem = checkPathField({ field: 'dockerfile', value, caseInsensitive: true }, scope); return ( @@ -219,7 +235,7 @@ function checkDefaultDockerfile(scope: PathCheckScope & { dockerContextRoot: str value: '', resolvedPath: join(dockerContextRoot, 'Dockerfile'), reason: - `is not set and there is no ${ACTOR_SPECIFICATION_FOLDER}/Dockerfile or Dockerfile in the Actor folder, ` + + `is not set and push uploads no ${ACTOR_SPECIFICATION_FOLDER}/Dockerfile or Dockerfile in the Actor folder, ` + `so the platform would build the Docker context ${dockerContextRoot} with its built-in Node.js Dockerfile; ` + 'add a Dockerfile, or set "dockerfile" to one inside the Docker context', severity: 'error', diff --git a/test/local/commands/push-actor-paths.test.ts b/test/local/commands/push-actor-paths.test.ts index e6631810e..c34341d81 100644 --- a/test/local/commands/push-actor-paths.test.ts +++ b/test/local/commands/push-actor-paths.test.ts @@ -234,7 +234,7 @@ describe('apify push with file paths in actor.json', () => { expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); expect(logMessages.error.join('\n')).toContain( - `"dockerfile" is not set and there is no .actor/Dockerfile or Dockerfile in the Actor folder, so the platform would build the Docker context ${joinPath('docker')} with its built-in Node.js Dockerfile`, + `"dockerfile" is not set and push uploads no .actor/Dockerfile or Dockerfile in the Actor folder, so the platform would build the Docker context ${joinPath('docker')} with its built-in Node.js Dockerfile`, ); expect(versionUpdate).not.toHaveBeenCalled(); }); diff --git a/test/local/lib/actor-paths.test.ts b/test/local/lib/actor-paths.test.ts index 49cc2fce4..209d67bda 100644 --- a/test/local/lib/actor-paths.test.ts +++ b/test/local/lib/actor-paths.test.ts @@ -313,6 +313,17 @@ describe('checkActorPaths', () => { ]); }); + it('reads a backslash as the platform does, so an absolute or escaping readme still fails the build', () => { + expect( + check({ readme: '/docs\\README.md', changelog: '../../../docs\\CHANGELOG.md' }).map( + ({ field, reason, severity }) => ({ field, reason, severity }), + ), + ).toEqual([ + { field: 'readme', reason: 'is absolute; the path is relative to .actor/', severity: 'error' }, + { field: 'changelog', reason: `lies outside the uploaded folder ${actorDir}`, severity: 'error' }, + ]); + }); + it('rejects a missing dockerfile or schema, which the platform does not recover from', () => { expect( check({ dockerfile: '../Dockerfile.prod', input: './missing.json' }).map(({ field, severity }) => ({ @@ -430,6 +441,30 @@ describe('checkActorPaths without dockerfile', () => { expect((await checkActor('in-actor-folder', ['.actor/dockerfile', 'Dockerfile'], '.')).problems).toEqual([]); }); + it('skips a default Dockerfile push does not upload, as the platform never sees it', async () => { + // `.actor/` is always uploaded, so only the Dockerfile in the Actor folder can be left out. + for (const name of ['ignored-actor-context', 'ignored-docker-context']) { + mkdirSync(join(root, name), { recursive: true }); + writeFileSync(join(root, name, '.actorignore'), 'Dockerfile\n'); + } + + expect((await checkActor('ignored-actor-context', ['Dockerfile'], '..')).problems).toEqual([]); + const { actorDir, problems } = await checkActor( + 'ignored-docker-context', + ['Dockerfile', 'docker/main.js'], + '../docker', + ); + expect(problems).toEqual([ + { + field: 'dockerfile', + value: '', + resolvedPath: join(actorDir, 'docker', 'Dockerfile'), + reason: expect.stringContaining('is not set and push uploads no .actor/Dockerfile or Dockerfile'), + severity: 'error', + }, + ]); + }); + it('leaves a set dockerfile to the usual rules', async () => { const { problems } = await checkActor('set', ['Dockerfile', 'docker/Dockerfile'], '../docker', { dockerfile: '../docker/Dockerfile', From baadeb13905614b457f24097e3a427c8d8c31451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 30 Oct 2026 01:07:01 +0100 Subject: [PATCH 41/42] fix: check the paths in actor.json before asking for a login A broken Actor now fails with the same error and exit code with or without a login. --- src/commands/actors/push.ts | 4 ++-- test/local/commands/push-actor-paths.test.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/commands/actors/push.ts b/src/commands/actors/push.ts index 25d9ff8ad..02e8d5889 100644 --- a/src/commands/actors/push.ts +++ b/src/commands/actors/push.ts @@ -274,8 +274,7 @@ export class ActorsPushCommand extends ApifyCommand { return; } - const apifyClient = await getLoggedClientOrThrow(); - + // Everything up to the path checks is local, so a broken Actor fails the same way with or without a login. const actorConfigResult = await useActorConfig({ cwd }); if (actorConfigResult.isErr()) { @@ -318,6 +317,7 @@ export class ActorsPushCommand extends ApifyCommand { return; } + const apifyClient = await getLoggedClientOrThrow(); const userInfo = await getCurrentUserInfo(); const isOrganizationLoggedIn = !!userInfo.organizationOwnerUserId; const redirectUrlPart = isOrganizationLoggedIn ? `/organization/${userInfo.id}` : ''; diff --git a/test/local/commands/push-actor-paths.test.ts b/test/local/commands/push-actor-paths.test.ts index c34341d81..962dafba3 100644 --- a/test/local/commands/push-actor-paths.test.ts +++ b/test/local/commands/push-actor-paths.test.ts @@ -58,6 +58,7 @@ const { beforeAllCalls, afterAllCalls, joinPath, tmpPath } = useTempPath(actName const { logMessages } = useConsoleSpy(); const { ActorsPushCommand } = await import('../../../src/commands/actors/push.js'); +const { getLoggedClientOrThrow } = await import('../../../src/lib/utils.js'); // The command sets the exit code on this mocked copy of `node:process`, not on the global one. const { default: commandProcess } = await import('node:process'); @@ -83,6 +84,7 @@ beforeEach(async () => { actorGet.mockClear(); versionUpdate.mockClear(); versionCreate.mockClear(); + vitest.mocked(getLoggedClientOrThrow).mockClear(); cwdCache.clear(); await beforeAllCalls(); @@ -100,7 +102,7 @@ afterEach(async () => { }); describe('apify push with file paths in actor.json', () => { - it('stops before reading or writing the Actor, listing every wrong path', async () => { + it('stops before logging in or reading or writing the Actor, listing every wrong path', async () => { await writeActorConfig({ dockerfile: '../Dockerfile.prod', readme: '../Dockerfile.ci', @@ -121,6 +123,7 @@ describe('apify push with file paths in actor.json', () => { expect(stderr).toContain( `"input": "../../INPUT_SCHEMA.json" (${joinPath('..', 'INPUT_SCHEMA.json')}) lies outside the uploaded folder ${tmpPath}.`, ); + expect(getLoggedClientOrThrow).not.toHaveBeenCalled(); expect(actorGet).not.toHaveBeenCalled(); expect(versionCreate).not.toHaveBeenCalled(); expect(versionUpdate).not.toHaveBeenCalled(); From 77a4cb46cb3d1ab5c60afb961ef99ee4125dd069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Hanu=C5=A1?= Date: Fri, 30 Oct 2026 01:10:26 +0100 Subject: [PATCH 42/42] test: cover the default Dockerfile for a monorepo Actor with the Docker context at the repository root --- test/local/commands/push-monorepo.test.ts | 46 ++++++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/test/local/commands/push-monorepo.test.ts b/test/local/commands/push-monorepo.test.ts index a9ed8622a..d4b4c590d 100644 --- a/test/local/commands/push-monorepo.test.ts +++ b/test/local/commands/push-monorepo.test.ts @@ -148,11 +148,11 @@ beforeEach(async () => { originalExitCode = commandProcess.exitCode; originalRealExitCode = process.exitCode; gitState.gitAvailable = true; - actorGet.mockClear(); - actorDelete.mockClear(); - actorsCreate.mockClear(); - versionUpdate.mockClear(); - versionCreate.mockClear(); + actorGet.mockReset(); + actorDelete.mockReset(); + actorsCreate.mockReset(); + versionUpdate.mockReset(); + versionCreate.mockReset(); keyValueStoreGetOrCreate.mockClear(); setRecord.mockClear(); cwdCache.clear(); @@ -422,8 +422,42 @@ describe('apify push of a monorepo Actor', () => { expect(keyValueStoreGetOrCreate).not.toHaveBeenCalled(); }); + describe('with the Docker context at the repository root and no dockerfile field', () => { + beforeEach(async () => { + await writeActorConfig({ dockerContextDir: '../../..' }); + }); + + it('pushes with the default Dockerfile in the Actor folder, which lies inside the context', async () => { + await write(`${ACTOR_PATH}/Dockerfile`, 'FROM apify/actor-node:20\n'); + git('add', '.'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBeFalsy(); + const modifier = versionPayload(versionUpdate); + expect(modifier.actorPath).toBe(ACTOR_PATH); + expect(modifier.sourceFiles!.map((file) => file.name)).toContain(`${ACTOR_PATH}/Dockerfile`); + expect(logMessages.log.join('\n')).toContain('Apify push result: SUCCEEDED'); + }); + + it('exits as an invalid actor.json without a Dockerfile at the default locations', async () => { + git('add', '.'); + + await testRunCommand(ActorsPushCommand, {}); + + expect(commandProcess.exitCode).toBe(CommandExitCodes.InvalidActorJson); + expect(logMessages.error.join('\n')).toContain('"dockerfile" is not set'); + expect(actorGet).not.toHaveBeenCalled(); + expect(versionUpdate).not.toHaveBeenCalled(); + }); + }); + it('names the Actor folder in the refusal when actor.json has no name', async () => { - await writeActorConfig({ name: undefined, dockerContextDir: '../../..' }); + await writeActorConfig({ + name: undefined, + dockerContextDir: '../../..', + dockerfile: '../../../shared/TypeScript_Dockerfile', + }); versionUpdate.mockRejectedValueOnce(actorPathRejected()); await testRunCommand(ActorsPushCommand, { args_actorId: ACTOR_ID });