feat(sandbox): allow non-absolute commands - #1705
Conversation
🦋 Changeset detectedLatest commit: 1e066ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
LGTM
The change is minimal and correct. The !parsed.command[0] check properly catches both empty strings and undefined (empty array), and the remaining NUL/Unicode/size validations are preserved. The riskiest aspect—removing the absolute-path requirement—is an intentional product decision to expose execvp-style semantics from the backend.
CI Failures: All three failing jobs (Runtime tests (24), Test examples (bun), Test examples (framework-sveltekit)) failed due to a transient HTTP 503 from GitHub when downloading Bun via oven-sh/setup-bun. Not caused by this PR; re-running should resolve it.
What this PR does
Relaxes sandbox command validation from requiring absolute executable paths to only requiring a non-empty first argument, enabling bare executable names (e.g., npm) and relative paths (e.g., ./script). Updates tests accordingly.
Tag @mendral-app with feedback or questions. View session
Linell
left a comment
There was a problem hiding this comment.
Approved, just a heads up: it looks like https://github.com/inngest/monorepo/pull/8209 needs to land before this is safe to release? Otherwise bare commands pass validation here and then 400 on the API.
This exposes the execvp-style semantics implemented by inngest/simcity#45 through the cloud API change in inngest/monorepo#8209.