fix: caller metadata with a line break can no longer inject workflow commands into package publish logs - #3447
Merged
Patrick-Erichsen merged 1 commit intoAug 11, 2026
Conversation
The resolve step echoes the publish command with shlex.quote, which is shell quoting rather than output escaping: it wraps a value holding a line break in single quotes and leaves the break itself intact. A caller-supplied changelog, categories or topics value carrying a newline therefore opened a second line in the step log, and the runner parses each stdout line, so that second line reached it as a workflow command. Escape the parts that are not printable in the echo. The re-runnable .sh file keeps plain shell quoting, because there the quoting is what makes the script correct.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Contributor
|
@Yigtwxx is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Codex review: needs maintainer review before merge. Reviewed August 11, 2026, 10:22 AM ET / 14:22 UTC. ClawSweeper reviewWhat this changesThe PR escapes non-printable caller metadata in the reusable package-publish workflow’s command log so it cannot be parsed as a GitHub Actions workflow command. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep open: current main retains the affected log behavior, while this is a focused, proof-positive hardening patch with no blocking correctness findings. Priority: P2 Review scores
Verification
How this fits togetherClawHub’s reusable package-publish workflow turns caller inputs into CLI arguments, writes a rerunnable shell script, and logs the resolved command. GitHub Actions consumes that log while the package CLI consumes the original argument list. flowchart LR
Caller[Workflow caller metadata] --> Resolve[Resolve publish command]
Resolve --> Arguments[Package CLI arguments]
Arguments --> Script[Runnable shell script]
Arguments --> Log[Resolved command log]
Log --> Runner[GitHub Actions runner]
Arguments --> Publish[Package publish CLI]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the log-only escaping with its regression contract after normal merge-gate validation, keeping publisher arguments and the generated rerunnable script unchanged. Do we have a high-confidence way to reproduce the issue? Yes. The supplied real reusable-workflow dispatch reproduces an injected annotation from newline-bearing metadata on the PR base, and current main retains the same affected workflow lines. Is this the best way to solve the issue? Yes. Escaping only the diagnostic rendering prevents runner parsing without changing the CLI argument list or the rerunnable shell script; rejecting or rewriting metadata at execution time would be broader and less compatible. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d9157142e9c5. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
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
2 participants
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.
Related: #3414
What Problem This Solves
A repository that publishes packages through the reusable
package-publish.ymlworkflow can pass achangelog,categoriesortopicsvalue. If that value contains a line break, the workflow's ownrun log stops being one line and the GitHub Actions runner executes the remainder as a workflow
command: the caller can set an annotation, and the same channel reaches
::error,::warning,::add-maskand::stop-commands.The values are caller-controlled
workflow_callinputs, so anything that composes this workflow —a matrix job, a release automation, a downstream repository passing a changelog straight from a tag
message or a PR body — can carry a newline into the log without anyone intending it.
Why This Change Was Made
The resolve step builds a shell line and both writes it to a re-runnable
.shfile and echoes it:shlex.quoteis shell quoting, not output escaping. Given"a\n::notice::x"it returns'a\n::notice::x'— single quotes around a line break that is still a line break. That is correctfor the
.shfile, where a shell reads the quotes, and wrong for the echo, where the runner readseach stdout line on its own.
So the file keeps
shell_lineuntouched and only the echo changes, through aquote_for_loghelper that falls back to
json.dumpswhen the shell-quoted form is not printable:Two choices inside that:
str.isprintable(), not an explicit\r\ncheck. It is false for every C0/C1 controlcharacter and for the Unicode line and paragraph separators, so nothing has to enumerate the
characters that can split a line.
--changelog 'Adds 日本語 notes'is printable and stays on thereadable, copy-pasteable
shlex.quotepath; only a value that cannot be printed as one line isescaped.
json.dumps, not a blanketrepr.ensure_asciidefaults to true, so the fallback outputis ASCII-only and cannot smuggle a separator back in.
This is the same fix as #3414 on the skill workflow, where the finding was raised. I noticed the
package side while writing that one; it has the same shape because the two workflows forward the
same three caller-controlled inputs.
User Impact
Callers of the reusable package publish workflow can no longer influence the runner through
metadata inputs, intentionally or by accident. Nothing else changes: the executed command is a
subprocess.runargument list and is untouched, the generated.shfile is byte-identical, and ametadata value with no control characters logs exactly as it did before.
Evidence
Real behavior. Two jobs in one dispatch, same multi-line
changelog, differing only in thepinned ClawHub SHA —
82313c2b(currentmain) and25aec929(this branch's head). Both hardcodedry_run: trueand pass nosecrets:(run 31409524372,
workflow source):
before-fix
— the echo breaks in two and the runner consumes the second line:
after-fix
— one line, and the payload is inert text:
The check-run annotations are the unambiguous half, because a line the runner accepts as a command
is removed from the log rather than printed:
The injected annotation carries the tail of the real command, which is what the runner swallowed.
Both jobs end red, and that is expected. The step under test,
Resolve publish command, isgreen in both; the caller repository holds no ClawPack package, so both jobs then stop at
Run package publishwith the identicalexit code 1and the identical plugin-inspector warningabove. Those two annotations appear on both sides and cancel out; the
INJECTEDone does not.One thing worth a maintainer's eye, which the fix does not and cannot address: the raw second line
is also in both jobs' logs before the resolve step runs, in the runner's own
##[group] Inputsecho of the reusable-workflow inputs and in the
env:block printed above each step. Those are therunner writing its own log rather than step stdout, so they are never parsed — the after-fix job
having no
INJECTEDannotation is the proof of that. A multi-line input stays visible in logseither way; what this fix removes is the part where it becomes executable.
Tests.
src/__tests__/package-publish-workflow.test.tsgains a case that pins the helperitself, that the echo goes through it, and that
shell_linekeeps plain shell quoting for the.shfile:
Negative control: restoring only
.github/workflows/package-publish.ymlfrommaingives1 failed | 5 passed, so the test fails without the fix. The test pins the fallback rather thanonly the absence of the old call, so replacing
quote_for_logwith anything that leaves a controlcharacter intact fails it.
All seven embedded Python blocks in the workflow were extracted from the file and parsed with
ast.parse; all seven are syntactically valid, including the 243-line block this touches.Pre-existing CI failure, unrelated to this branch.
pr-gatescurrently fails atbun auditonmainitself and therefore on every open PR; #3446 fixes that separately. It is not caused by thisdiff.
Screenshots: N/A, workflow change with no UI surface.