Skip to content

🐛 docker-build: quote the bake metadata heredoc - #7449

Merged
GeigerJ2 merged 1 commit into
aiidateam:mainfrom
GeigerJ2:fix/docker-bake-metadata-heredoc
Jul 16, 2026
Merged

🐛 docker-build: quote the bake metadata heredoc#7449
GeigerJ2 merged 1 commit into
aiidateam:mainfrom
GeigerJ2:fix/docker-bake-metadata-heredoc

Conversation

@GeigerJ2

@GeigerJ2 GeigerJ2 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

The Write metadata output to a json file step pipes the bake metadata through an unquoted heredoc:

cat << EOF > bake_metadata.json
${{ steps.build.outputs.metadata }}
EOF

GitHub substitutes the metadata textually, and the shell then expands what it finds. The metadata's buildx.build.provenance embeds the push event payload, commit messages included, so every backtick in a commit message is treated as command substitution: its contents run on the runner and vanish from the JSON.

in:   {"message":"Rename from `zmq_broker_service` to `broker_service`."}
out:  {"message":"Rename from  to ."}

This is not hypothetical: our own commit convention is <emoji> `Scope`: description, so conventional commit messages are exactly the ones this mangles. It had been corrupting the recorded metadata silently for a while. #7444 is the example above, and it still produced parseable JSON, so nothing went red and nobody noticed. #7254's message carries 26 backticks, enough for the substitution to run past a string terminator and leave the pretty-printed JSON's real newlines inside a string literal, so the following github-script step died on

SyntaxError: Bad control character in string literal in JSON

and Docker Images has been red on main since. (The earlier red run on #7444 was an unrelated arm64 ports.ubuntu.com timeout.)

Quote the delimiter so the shell leaves the payload alone. ${{ }} is GitHub-side templating and still expands, so only the shell expansion is suppressed. Incidentally this also closes a script-injection path, since commit messages reaching bash is exactly that, though a fork PR can't reach this workflow and anyone who can get a commit onto main can already run code in CI, so the practical gain is mostly that the metadata is no longer silently corrupted.

Reproducing the mangling outside CI

The payload has to be inlined textually the way GitHub does it, before bash sees the script (passing it via a shell variable hides the bug, since that is the expansion the fix suppresses):

METADATA='{"message": "Rename from `zmq_broker_service` to `broker_service`."}'
printf 'cat << EOF > old.json\n%s\nEOF\n' "$METADATA" > old_step.sh
bash old_step.sh
old_step.sh: line 1: zmq_broker_service: command not found
old_step.sh: line 1: broker_service: command not found
$ cat old.json
{"message": "Rename from  to ."}

Swapping in << 'EOF' preserves the message byte for byte.

The `Write metadata output to a json file` step pipes the bake metadata
through an unquoted heredoc:

    cat << EOF > bake_metadata.json
    ${{ steps.build.outputs.metadata }}
    EOF

GitHub substitutes the metadata textually, and the shell then expands
what it finds. That metadata embeds the git commit message, so every
backtick in a commit message is treated as command substitution and its
contents are eaten:

    in:   {"message":"Rename from `zmq_broker_service` to
    `broker_service`."} out:  {"message":"Rename from  to ."}

With enough backticks the substitution runs past a string terminator and
the pretty-printed JSON's real newlines end up inside a string literal,
so the following `github-script` step dies on

    SyntaxError: Bad control character in string literal in JSON

This is not hypothetical: the repository's own commit convention is
``<emoji> `Scope`: description``, so conventional commit messages are
exactly the ones that break the build. `Docker Images` has been red on
`main` since aiidateam#7444, and again for aiidateam#7254.

Quote the delimiter so the shell leaves the payload alone. `${{ }}` is
GitHub-side templating and still expands, so only the shell expansion is
suppressed.
@GeigerJ2
GeigerJ2 requested a review from agoscinski July 16, 2026 11:25
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Docker build workflow now uses a single-quoted heredoc delimiter when writing bake_metadata.json, changing how the heredoc body is interpreted.

Changes

Docker metadata generation

Layer / File(s) Summary
Quote metadata heredoc delimiter
.github/workflows/docker-build.yml
The metadata-writing step changes cat << EOF to cat << 'EOF' when generating bake_metadata.json.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: quoting the bake metadata heredoc in docker-build.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.60%. Comparing base (d5bb1b5) to head (afd6b54).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7449      +/-   ##
==========================================
- Coverage   80.61%   80.60%   -0.00%     
==========================================
  Files         580      580              
  Lines       46722    46722              
==========================================
- Hits        37658    37657       -1     
- Misses       9064     9065       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- name: Write metadata output to a json file
run: |
cat << EOF > bake_metadata.json
cat << 'EOF' > bake_metadata.json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh, TIL, had no idea that such quoting works

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither, agent proposed 😉

@GeigerJ2
GeigerJ2 merged commit c04d915 into aiidateam:main Jul 16, 2026
26 of 27 checks passed
@GeigerJ2
GeigerJ2 deleted the fix/docker-bake-metadata-heredoc branch July 16, 2026 13:36
danielhollas added a commit to aiidalab/aiidalab-docker-stack that referenced this pull request Jul 22, 2026
Same change as was done in aiidateam/aiida-core#7449,
see that PR for details.

Notably, the metadata's buildx.build.provenance embeds the push event payload,
commit messages included, so every backtick in a commit message
is treated as command substitution!
agoscinski pushed a commit that referenced this pull request Jul 25, 2026
agoscinski pushed a commit that referenced this pull request Jul 25, 2026
agoscinski pushed a commit that referenced this pull request Jul 25, 2026
agoscinski pushed a commit to agoscinski/aiida-core that referenced this pull request Jul 31, 2026
agoscinski pushed a commit to agoscinski/aiida-core that referenced this pull request Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants