Repository navigation
Conversation
…to feat/push-validate-actor-json-paths
…to feat/push-validate-actor-json-paths
…to feat/push-validate-actor-json-paths
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.
The worker copies inputSchema over input and outputSchema over output before resolving, so a schema path under the alias was never checked.
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.
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.
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.
apify-factory
left a comment
Contributor
There was a problem hiding this comment.
Looks good, nothing blocking.
🏭 Factory review — ℹ️ not auto-approved
This change was reviewed but cannot be auto-approved: a gate failed, so the review cannot end in an approval.
| check | result |
|---|---|
| Static gates | ❌ 14/16 — base-branch, max-lines |
| Review findings | ✅ none |
| Reviewer escalation | ❌ The new check makes apify push fail on paths that used to push fine, based on platform worker rules this repo can't confirm (for example, a missing readme or dockerfile used to fall back to the default). |
Juror 1 — claude-sonnet-5-5, adversarial |
⏭️ skipped — a gate failed |
Why it was not auto-approved
Failed gates:
base-branch: base is feat/push-monorepo-preflight, required mastermax-lines: 646 changed lines (+644/-2), limit 200
The comments above are the review itself; this section only explains the approval decision. Push a change and request @apify-factory again to re-run it, or ask a human to review.
Reviewed e3edc7960 · workflow run
apify-factory
left a comment
Contributor
There was a problem hiding this comment.
Looks good, nothing blocking.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apify pushchecks none of the file-path fields in.actor/actor.json. The platform resolves them after the upload: a missing schema fails the build, and a missingdockerfile,readme, orchangelogonly logs a warning and falls back to the default, so the build goes green with the wrong image or README. This PR applies the worker's rules in the CLI before the Actor is looked up or written.Closes #1500. Part of apify/apify-core#28685. Stacked on #1502: the check runs after the
dockerContextDirresolver.Changes
src/lib/actor-paths.ts:checkActorPathsreturns one problem (field, value, resolved path, reason) per failing field;formatActorPathProblemsbuilds the message.push.tsprints it and exits with code 5.dockerfile,readme,changelog,input,output,webServerSchema, thestoragesschemas, plusinputSchemaandoutputSchema. The worker copies a set alias overinputoroutput, so the alias wins and the base key is skipped. Inline objects and empty strings (unset on the worker) are skipped..actor/, inside the upload root, an existing file, and among the files push uploads.dockerfile,readme, andchangelogmatch case-insensitively, as the worker finds them by a directory listing; schemas match exactly.dockerContextDirpoints inside the Actor folder,dockerfilemust lie inside that context: the worker runs BuildKit with--local dockerfile=<context dir>andfilename=<Dockerfile relative to the context>, so a Dockerfile outside it cannot be read by the build. This goes beyond the issue's checklist, because the same constraint applies to today's push.Tests
test/local/lib/actor-paths.test.ts: every rule, the aliases, empty strings, backslashes, symlinks (a dangling one and a symlinked Actor folder included), and the case mismatch reason.test/local/commands/push-actor-paths.test.ts: wrong paths stop push with exit code 5 before any Actor read or write,--jsonkeeps stdout clean, and valid paths push as usual.update-docs(no change), andtest:localpass, except the two[python]fixture tests that need a newer Python.Notes for reviewers
uploadRoot.--diris not mistaken for an escape.🤖 Generated with Claude Code