Skip to content

feat: validate actor.json paths before apify push uploads anything - #1503

Draft
DaveHanns wants to merge 3 commits into
feat/push-monorepo-preflightfrom
feat/push-validate-actor-json-paths
Draft

DaveHanns wants to merge 3 commits into
feat/push-monorepo-preflightfrom
feat/push-validate-actor-json-paths

Conversation

@DaveHanns

@DaveHanns DaveHanns commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

apify push reads .actor/actor.json with a plain JSON.parse and checks none of its file-path fields. The platform resolves dockerfile, readme, changelog, and every string-valued schema reference relative to .actor/ after the upload: a missing schema file fails the build, and a missing dockerfile, readme, or changelog only logs a warning on the worker side and falls back to the default, so the build goes green with the wrong image or README.

This PR adds a pre-flight check that applies the worker's path rules in the CLI, after the config is read and before the Actor is looked up or written. It needs no platform change.

Closes #1500. Part of apify/apify-core#28685.

Stacked on the preflight PR (#1502, branch feat/push-monorepo-preflight): the check runs right after the dockerContextDir resolver, so a monorepo Actor still gets the stage 1 refusal first.

Changes

  • New src/lib/actor-paths.ts: checkActorPaths({ actorDir, uploadRoot, uploadedFiles, config, dockerContextRoot? }) returns one problem (field, value, resolved path, reason) per failing field, and formatActorPathProblems turns the list into the error message.
  • Fields checked: dockerfile, readme, changelog, input, output, webServerSchema, storages.keyValueStore, storages.dataset, and storages.datasets.*. Object values (inline schemas) are skipped, and dockerContextDir stays with the resolver.
  • Rules per field: relative to .actor/, not absolute, inside the upload root (lexically and after symlink resolution), an existing file rather than a directory, and among the files push uploads (which catches a Dockerfile excluded by .gitignore or .actorignore). The name matches case-insensitively for dockerfile, readme, and changelog, as the worker finds those by a directory listing, and exactly otherwise.
  • src/commands/actors/push.ts: calls the check with the Actor folder as the upload root and the file list push already collected. A failure prints every problem and exits with CommandExitCodes.InvalidActorJson (5).
  • src/lib/docker-context.ts: exports the existing isOutside and toPosix helpers for reuse.
  • Monorepo push (Make apify push work for monorepo Actors that set dockerContextDir #595, stage 2) can pass the Git repository root as uploadRoot and the context as dockerContextRoot; the check then also requires dockerfile inside the Docker context.

Tests

  • test/local/lib/actor-paths.test.ts (unit): ./x and ../x inside the root pass; a path above the root, an absolute path, a missing file, a directory, a file excluded by .actorignore, and a symlink to a file outside the root fail; a wrong-case dockerfile and readme pass and a wrong-case input fails; every string-valued schema reference is checked; inline schema objects and a config without path fields are no-ops; dockerfile outside an explicit dockerContextRoot fails.
  • test/local/commands/push-actor-paths.test.ts (mocked client): wrong dockerfile, readme, and input stop push with exit code 5, every problem listed, and no Actor read or write; stdout stays clean with --json; valid paths push as usual.
  • Ran pnpm run lint, pnpm run format, pnpm run build, and pnpm run test:local: 765 passed, 2 failed. The two failures are the [python] template tests (python_support.test.ts and works-with-spaces-in-path-to-actor.test.ts), which cannot create a .venv with the Python 3.9 on this machine; they do not touch push.
  • pnpm run update-docs produced no change: no flag, argument, or description changed.
  • Not run: test:api (needs a platform token). No e2e test was added, because the login check precedes the config read, so a token-free e2e run exits with code 1 first.
  • Ran with Node.js 26.4.0 (the repository pins Node.js 24.21.0 through Volta, which is not installed here; engines asks for 22 or newer). The review pass repeated lint, format, build, update-docs, and test:local on Node.js 24.18.0 through nvm with the same result.

Notes for reviewers

  • The helper returns a list instead of throwing, so the command decides how to print and exit, and a later caller can pass a different upload root. The exit code and message stay out of the helper.
  • Names are matched by a directory listing in both modes, not by statSync, so a wrong-case input fails on macOS and Windows as it would on the Linux worker.
  • A valid dockerContextDir above the upload root keeps exit code 255 and the stage 1 message, since the resolver runs first.
  • A wrong-case dockerfile, readme, or changelog passes here because the worker accepts it; a reviewer may prefer a warning for it, which this PR does not add.
  • No new dependencies, only Node.js built-ins.

🤖 Generated with Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants