mirror of
https://github.com/woodpecker-ci/woodpecker.git
synced 2026-08-31 10:08:10 +00:00
Make generated command script execution robust #5
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Command steps hit a known failure mode where generated shell execution failed with errors like:
This matched upstream reports around command entrypoint/script handling. We had to work around it by overriding the entrypoint and writing
CI_SCRIPTto a file before executing it:Desired behavior
Woodpecker should execute generated command scripts in a way that is robust for larger or more complex scripts by default. For example:
Why this matters
Command execution is core CI functionality. Users should not need custom entrypoints across many workflows to avoid intermittent shell quoting/truncation issues.
@opencode-bot What do you think about this?
OpenCode Answer
Execution completed for
pleroma/woodpeckeratf87a97893225(job #53).Question
Answer
Agree with the direction: decoding CI_SCRIPT to a file and executing that file is the right default for POSIX command steps. The current wrapper at pipeline/backend/common/script.go:31 ("echo $CI_SCRIPT | base64 -d | /bin/sh -e"), used by the docker and kubernetes backends, pipes the decoded script through shell stdin, which is quoting/truncation-sensitive, loses file/line context in shell errors, and cannot be re-run or inspected. A file-based wrapper fixes that class of failure, and the project already endorsed this direction on Windows in #6969 ("iex is a poor mechanism..."), with #7065's partial revert showing that passing the payload as an entrypoint argument is equally fragile — env var + file execution avoids both. Two caveats: (1) it does not lift the real size ceiling, since CI_SCRIPT travels as one env var and exec fails E2BIG above ~128KB (MAX_ARG_STRLEN) — verified locally: 127,892B env works, 159,892B fails even for a trivial command, and the reporter's workaround shares this limit; scripts beyond ~96KB need the script mounted as a file, not env-passed. (2) I could not reproduce the exact intermittent 'unterminated quoted string' with dash up to the exec limit, so that root cause remains unconfirmed here and is likely runtime/image-specific — but a file-based wrapper removes it regardless.
Grounding: GenerateContainerConf (pipeline/backend/common/script.go) base64-encodes a generated script into env CI_SCRIPT and overrides the image entrypoint. POSIX: /bin/sh -c 'echo $CI_SCRIPT | base64 -d | /bin/sh -e' (line 31); Windows decodes via [Convert]::FromBase64String | iex (line 27). Docker uses it in convert.go:54-67 and kubernetes in pod.go:260-273; both let step.Entrypoint override it (the workaround users use today). The generated POSIX script (script_posix.go) writes .netrc, unsets CI_NETRC_* and CI_SCRIPT (lines 60-62), mkdir/cd workdir, then per-command 'echo + ' tracing — so error (-e) and tracing behavior live in the script and survive switching to 'sh -e '. Assessment: (1) File execution is a strict improvement — removes echo/pipe fragility, gives named-file error context, and the artifact is debuggable. Suggested wrapper: /bin/sh -c 'printf "%s" "$CI_SCRIPT" | base64 -d > /tmp/wp-script.sh && /bin/sh -e /tmp/wp-script.sh' (quote $CI_SCRIPT; use mktemp; keep -e and existing echo-tracing rather than adding -x, which would double-print with traceScript; note /tmp writability and the retained base64 dependency in the step image). Apply the analogous WriteAllText change for Windows. (2) Repo history supports it: #6969 (
9c018c7) moved Windows off iex citing escaping fragility; #7065 (a1de4fc) partially reverted because -encodedcommand passes the whole script as an argv entry, which hits command-line length limits — argv and stdin-pipe transports are both fragile; env + file is the consistent fix. (3) Hard limit my probe confirmed: with dash, both the current pipe wrapper and the file workaround work up to ~128KB of base64 (script ~96KB); above that exec fails E2BIG before any shell runs (single env string > MAX_ARG_STRLEN 131072), so the issue's 'robust for larger scripts' goal is only partly met — truly large scripts need a different transport (mounted volume/emptyDir). (4) The exact intermittent 'unterminated quoted string' did not reproduce with dash on Linux within the working size range, so that specific root cause is unconfirmed in this checkout; treat it as runtime/image-specific truncation. (5) Compatibility: docs document the wrapper and CI_SCRIPT (docs/docs/20-usage/20-workflow-syntax.md:176, 50-environment.md:155 — which already mislabels CI_SCRIPT as an 'internal script path'); keep CI_SCRIPT as base64 content and custom-entrypoint behavior unchanged, and update these docs alongside the change. The local backend is unaffected (it never uses CI_SCRIPT).