fix(groovy): default the Docker runner to root for Commands tasks

[jymaire]

Closes #411

Problem

On the Docker task runner, the working directory can end up owned by a user other than the container image's default user. On a non-root image such as groovy:jdk21 (USER groovy, uid 1000), a groovy.Commands task cannot write into its own working directory, so outputFiles fails with FileNotFoundException: out.txt (Permission denied) and captures 0 files.

groovy.Script already forces user("root") on its Docker runner and is therefore unaffected — Commands had no equivalent, which is the inconsistency reported in the issue.

Fix

Default the Docker runner's user to root when the user has not configured one explicitly, on both config paths:

• the modern taskRunner: path, in Commands.run();
• the deprecated docker: property path, in Commands.injectDefaults() — necessary because CommandsWrapper.getTaskRunner() rebuilds the runner from dockerOptions on every call and discards any withTaskRunner(...) override.

Unlike Script (which replaces the task runner wholesale and drops any custom Docker options), this preserves every other option the user configured — volumes, memory, cpu, pull policy, credentials — and never overrides an explicitly set user.

Scope

Checked the other modules with a non-root-looking default image; both oven/bun and denoland/deno create a non-root account but never USER-switch to it, so they still run as root (docker run --rm oven/bun id → uid=0(root), same for denoland/deno). Groovy is the only affected module.

Tests

plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/CommandsTest.java:

• outputFilesOnNonRootImage() — modern taskRunner: path
• outputFilesOnNonRootImageLegacyDockerProperty() — deprecated docker: path

Both build a throwaway image FROM groovy:jdk21 with a user at uid 5000, so the reproduction does not depend on the host uid coincidentally matching the image's default uid (with groovy:jdk21's own uid 1000 the bug reproduces only on some hosts/engines). Verified TDD-style: without the fix both fail with uid=5000 and Permission denied; with it both log uid=0(root) and capture out.txt.

./gradlew :plugin-script-groovy:test — BUILD SUCCESSFUL, 3 tests, 0 failures.

Note for reviewers

Running containers as root by default is a real behavior change for anyone relying on the image's non-root user. It matches what groovy.Script has always done, and setting taskRunner.user explicitly opts out. Happy to instead scope it behind something more explicit if you'd prefer.

🤖 Generated with Claude Code

[github-actions]

📦 Artifacts

Name	Size	Updated	Expiration
	56.82 MB	Sep 2, 26, 2:44:09 PM UTC	Sep 9, 26, 2:44:06 PM UTC

🧪 Java Unit Tests

	Tests	Passed ✅	Skipped	Failed	Time ⏱
Java Tests Report	246 ran	246 ✅	0 ⚠️	0 ❌	23m 11s 29ms

🔁 Unreleased Commits

7 commits since v1.9.8

SHA	Title	Author	Date
e37e4bd	chore(deps): bump gradle-wrapper from 9.7.0 to 9.7.1 (#409)	dependabot[bot]	Aug 27, 26, 8:24:16 AM UTC
7fff1ff	chore(deps): bump com.github.ben-manes.versions from 0.60.0 to 0.61.0 (#408)	dependabot[bot]	Aug 27, 26, 8:24:26 AM UTC
8169b93	chore(deps): bump org.apache.groovy:groovy-jsr223 from 5.0.7 to 5.1.0 (#407)	dependabot[bot]	Aug 27, 26, 8:24:37 AM UTC
7c41b6b	chore(deps): move from test-logger to kestra logger	Ludovic DEHON	Aug 28, 26, 4:23:17 PM UTC
4f8d706	chore(deps): bump org.apache.groovy:groovy-jsr223 from 5.1.0 to 5.1.1 (#413)	dependabot[bot]	Aug 31, 26, 8:26:37 AM UTC
a38e2d5	chore(deps): bump the kestra-gradle-plugins group with 4 updates (#412)	dependabot[bot]	Aug 31, 26, 8:26:48 AM UTC
4de3bc2	chore: use jvmargs for all plugins	Ludovic DEHON	Sep 2, 26, 12:36:14 PM UTC

[github-actions]

Tests report quick summary:

success ✅ > tests: 246, success: 246, skipped: 0, failed: 0

unfold for details

Project	Status	Success	Skipped	Failed
plugin-script-bun	success ✅	3	0	0
plugin-script-deno	success ✅	3	0	0
plugin-script-dotnet	success ✅	8	0	0
plugin-script-go	success ✅	18	0	0
plugin-script-groovy	success ✅	10	0	0
plugin-script-jbang	success ✅	2	0	0
plugin-script-julia	success ✅	2	0	0
plugin-script-jython	success ✅	5	0	0
plugin-script-lua	success ✅	3	0	0
plugin-script-nashorn	success ✅	6	0	0
plugin-script-node	success ✅	13	0	0
plugin-script-perl	success ✅	3	0	0
plugin-script-php	success ✅	3	0	0
plugin-script-powershell	success ✅	4	0	0
plugin-script-python	success ✅	75	0	0
plugin-script-r	success ✅	2	0	0
plugin-script-ruby	success ✅	39	0	0
plugin-script-shell	success ✅	47	0	0

[fdelbrayelle]

Kestra Plugin Code Review

Business Requirements — partial

Issue #411: Docker runner creates the working directory as root while Commands runs as the image's non-root user, breaking outputFiles (matches Script, which already forces root). The fix covers both the modern taskRunner: path (run()) and the deprecated docker: path (injectDefaults()), and includes two regression tests. Scope was verified against bun/deno (unaffected), which is good discipline.

However: the linked issue's reporter environment is Kestra 2.0.0-rc10. Decompiling script-2.0.0-SNAPSHOT.jar vs the currently-pinned script-1.3.19.jar (this repo's gradle.properties) confirms AbstractExecScript no longer declares a docker field / getDocker() in 2.0.0-SNAPSHOT — only the vestigial injectDefaults(DockerOptions) overload remains. That means the legacy-path half of this fix (injectDefaults() override, Commands.java lines 94-105) and its dedicated test outputFilesOnNonRootImageLegacyDockerProperty() (which calls .docker(DockerOptions...), CommandsTest.java line 125) will not compile once this plugin bumps kestraVersion toward the 2.0 line the issue was actually filed against. Worth confirming which core version this fix is meant to ship against before merge.

Kestra Guidelines — 3 findings

• 🟠 Commands.java line ~120: Undocumented behavior change — Docker now defaults to root for every containerImage/taskRunner combination used by Commands (not just groovy:jdk21), with no @Schema or plugin-doc (io.kestra.plugin.scripts.groovy.md) update explaining the new default.
• 🟡 CommandsTest.java line 151 (buildNonRootTestImage): test infra overengineering — a new test-only dependency (docker-java-api) plus building/pushing/removing a throwaway custom image just to get a non-root uid, when DockerOptions.user/Docker.user is a plain numeric-uid-capable String — .user("5000") on the stock groovy:jdk21 image would reproduce the same bug without any of this.
• 🟢 CommandsTest.java line ~75-79: the 5-line comment blocks above both new test methods exceed the "one short line max" comment guideline; trim to a single line + issue link.

Security (OWASP Top 10:2025 + KPS) — 1 finding

• 🟠 A02:2025 Security Misconfiguration (Commands.java line ~120): forcing every Docker-based Commands execution to run as root by default broadens the blast radius of the fully flow-author-controlled commands: property (arbitrary shell/groovy commands) for any custom image the user supplies, not just the reported one. This is the same trade-off already made in Script.java, but silently extending it to Commands.java without documentation removes the user's ability to know they need to opt out via user. The PR description itself invites this feedback ("Happy to instead scope it behind something more explicit") — recommend either documenting the new default prominently, or narrowing the fix to actually chown/chmod the working directory to the container's runtime user instead of running the whole command as root.

Performance — 1 finding

• 🟡 CommandsTest.java line 163-164: buildImageCmd(...).start().awaitImageId() has no timeout; a stalled Docker build/daemon would hang the test run indefinitely. Not a production code path, low severity, and avoidable entirely per the test-simplification suggestion above.

Verdict: REQUEST CHANGES

(Note: this review was submitted via the API in a degraded form — a diagnostic single-comment call was accidentally submitted as a review before the full multi-comment review could be posted; this body was then updated in place to carry the complete findings since GitHub does not allow converting an already-submitted review's event/state after the fact. Please treat the verdict above, not the "Commented" badge, as authoritative.)

[jymaire]

Thanks for the thorough review — pushed b0693ea addressing most of it. Point-by-point:

🟠 Root-by-default undocumented — fixed

Agreed, this was the weakest part of the PR. Now documented in two places:

• Commands' class-level @Schema description, so it shows up in the task's generated docs;
• a new Container user section in io.kestra.plugin.scripts.groovy.md explaining why the default exists and that taskRunner.user opts out.

I kept the root default rather than switching to a chown/chmod of the working directory. The chown approach needs a privileged step to run before the user's commands to be effective, so it either injects a root beforeCommands entry (same privilege, less visible) or needs core-side support in the Docker runner — both are a bigger change than this bug fix, and neither matches what Script already does. If we want to move away from root-by-default, it should be a deliberate change across both tasks rather than a divergence introduced here.

While in that doc I also corrected an unrelated inaccuracy it asserted: it claimed Eval and Script "both execute in-process on the Kestra worker", but Script runs on a task runner (Docker by default) — see Script.run(), which builds a CommandsWrapper and forces Docker.builder().user("root").

🟡 awaitImageId() has no timeout — fixed

Bounded at 5 minutes (IMAGE_BUILD_TIMEOUT_MINUTES).

🟢 Comment blocks too long — fixed

Trimmed the two blocks in CommandsTest and both in Commands.java to a single line each, with the issue link.

🟡 .user("5000") instead of building a throwaway image — respectfully, this doesn't work

The fix only defaults to root when user is unset (docker.getUser() == null / original.getUser() == null). Passing .user("5000") explicitly opts out of the defaulting, so such a test would fail with the fix applied — it can't act as a regression test for it. The bug specifically requires the image's own default user to be non-root while the task leaves user unset, which is why the test builds an image with USER kestratest at uid 5000. Keeping it as is.

The uid-5000 image (rather than stock groovy:jdk21 at uid 1000) is also deliberate: with uid 1000 the bug only reproduces on hosts whose own uid isn't 1000, so the test would silently pass on some machines and in some CI configurations.

🟠 Forward-compat with the 2.0 line — confirmed, and narrower than stated

Verified by decompiling the cached jars rather than trusting either of our recollections:

javap -p io.kestra.plugin.scripts.exec.AbstractExecScript

• script-1.3.19 (what gradle.properties pins): declares protected DockerOptions docker, getDocker(), and both injectDefaults(...) overloads.
• script-2.0.0-rc12: docker / getDocker() are gone; both injectDefaults(...) overloads still exist, and injectDefaults(RunContext, DockerOptions) is only reachable from the deprecated overload.

So the correct read is:

• the injectDefaults override in Commands.java still compiles on 2.0 — it just becomes unreachable, exactly like the pre-existing identical override in Script.java (line 90), which is in the same position and isn't a new problem introduced here;
• the only actual compile break on a 2.0 bump is the test's .docker(DockerOptions...) builder call.

This PR targets the pinned kestraVersion=1.3.19, where the legacy docker: path is live and the fix is required for it — CommandsWrapper.getTaskRunner() rebuilds the runner from dockerOptions on every call, so a withTaskRunner(...) override alone doesn't survive on that path. When the repo bumps to the 2.0 line, that half and its test get deleted along with the docker field, same as every other legacy-docker usage across these modules. Happy to be corrected if the intent is to ship this against 2.0 directly — in that case I'd drop the injectDefaults half and the legacy test from this PR.