Pump and Publish using Docker #47
Conversation
WalkthroughA new Docker-based workflow was introduced for version bumping and publishing, replacing direct dependency installation on the runner. A dedicated script ( Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Docker Builder
participant Container (docker-pump-version.sh)
participant NPM Registry
GitHub Actions->>Docker Builder: Build cardano-csl-builder image
GitHub Actions->>Container (docker-pump-version.sh): Run docker-pump-version.sh with version arg
Container (docker-pump-version.sh)->>Container (docker-pump-version.sh): Update version in files
Container (docker-pump-version.sh)->>NPM Registry: Publish NodeJS and Browser packages
GitHub Actions->>scripts/pump-version.sh: Run version bump script (no publish)
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai i just force pushed, please full review |
|
Tip For best results, initiate chat on the files or code changes. ✅ Actions performedFull review triggered after force push. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docker-pump-version.sh (1)
14-16:.npmrcis written twice (workflow + here).Remove one of the writes to keep the pipeline DRY and avoid confusion about which config is in effect.
🧹 Nitpick comments (3)
.github/workflows/pump-version.yml (1)
24-28: Consider publishing a pre-built base image instead of building on every run.Building
cardano-csl-builderon the runner takes several minutes and offers an unnecessary attack surface repetition.
Push the image to GHCR (or Docker Hub) once anddocker pullit here; rebuild only when the Dockerfile changes.docker-pump-version.sh (2)
5-5: Fail fast if no version argument is supplied.
set -uwill already abort, but a friendlier message helps CI triage.-version=$1 +if [[ $# -ne 1 ]]; then + echo "Usage: $0 <new-version>" >&2 + exit 1 +fi +version=$1
19-20: Publishing steps run heavy builds twice per job.
npm install+wasm-packare invoked earlier and again inside the publish scripts.
Consider a single build artefact reused acrossjs:publish-*to cut ~3–4 min per run.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonrust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/pump-version.yml(1 hunks)docker-pump-version.sh(1 hunks)package.json(1 hunks)rust/Cargo.toml(1 hunks)scripts/pump-version.sh(0 hunks)
💤 Files with no reviewable changes (1)
- scripts/pump-version.sh
🧰 Additional context used
🧠 Learnings (2)
rust/Cargo.toml (2)
Learnt from: ljttl3q04t
PR: minswap/cardano-serialization-lib#45
File: rust/src/serialization.rs:1983-1985
Timestamp: 2025-07-01T03:57:39.056Z
Learning: In the minswap/cardano-serialization-lib repository, using panic! for unknown/unused CertificateEnum variants in serialization is acceptable as a simple solution when those certificate types are never used in the codebase.
Learnt from: ljttl3q04t
PR: minswap/cardano-serialization-lib#45
File: rust/src/lib.rs:1050-1074
Timestamp: 2025-07-01T03:53:00.482Z
Learning: In the minswap/cardano-serialization-lib repository, dummy/placeholder structs for certificate types (like CommitteeHotAuth, CommitteeColdResign, etc.) are acceptable when they are not currently used and are intended as placeholders for future implementation.
package.json (1)
Learnt from: ljttl3q04t
PR: minswap/cardano-serialization-lib#45
File: rust/src/serialization.rs:1983-1985
Timestamp: 2025-07-01T03:57:39.056Z
Learning: In the minswap/cardano-serialization-lib repository, using panic! for unknown/unused CertificateEnum variants in serialization is acceptable as a simple solution when those certificate types are never used in the codebase.
🔇 Additional comments (2)
rust/Cargo.toml (1)
3-3: Version bump looks fine.No functional impact beyond metadata. ✅
package.json (1)
3-3: Consistent version bump.Matches the Rust crate version – good.
| docker run --rm \ | ||
| -v ${{ github.workspace }}:/usr/src/app \ | ||
| -w /usr/src/app \ | ||
| -e NPM_TOKEN="${{ secrets.NPM_TOKEN }}" \ | ||
| -e GITHUB_TOKEN="${{ secrets.GITHUB_TOKEN }}" \ | ||
| cardano-csl-builder \ | ||
| /bin/bash -c " | ||
| # Install Node.js dependencies | ||
| npm install --ignore-scripts | ||
| npm install -g --force wasm-pack@0.10.1 | ||
| # Run the pump version script | ||
| ./docker-pump-version.sh ${{ env.NEXT_VERSION }} | ||
| " | ||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Secrets are mounted into a container built from repo code – review threat model.
Because the image is built from the PR’s Dockerfile, malicious code could be baked into the image and exfiltrate NPM_TOKEN / GITHUB_TOKEN when this step runs.
If untrusted contributors can modify the Dockerfile, restrict this job to protected branches or use a vetted image.
To confirm whether cardano-csl-builder is built from the PR’s Dockerfile (and thus can be tampered with) or pulled as a prebuilt image, let’s inspect the workflow for any build steps:
🏁 Script executed:
#!/bin/bash
set -e
echo "Looking for docker build steps in pump-version.yml:"
rg -n "docker build" -n .github/workflows/pump-version.yml || echo "→ No docker build found in workflow."
echo
echo "Listing references to cardano-csl-builder:"
rg -n "cardano-csl-builder" -n .github/workflows/pump-version.yml || echo "→ No references found."
echo
echo "Full workflow file preview:"
sed -n '1,200p' .github/workflows/pump-version.ymlLength of output: 2463
Harden secret usage in Docker workflow
The workflow builds cardano-csl-builder from PR-supplied Dockerfile and then mounts ${{ secrets.NPM_TOKEN }} and ${{ secrets.GITHUB_TOKEN }} into that image. A malicious PR could modify the Dockerfile or included scripts to exfiltrate these tokens.
Locations to address:
- .github/workflows/pump-version.yml
- Line 26:
docker build -t cardano-csl-builder . - Lines 41–55:
docker run … cardano-csl-builder …
- Line 26:
Recommendations:
- Use a pre-built, versioned image from a trusted registry instead of building from PR code.
- Or restrict this job to a protected branch (e.g.
if: github.ref == 'refs/heads/minswap-11'with branch protection enabled) so untrusted PRs cannot modify the image. - Alternatively, split “build” and “publish” into separate workflows: build on PRs, but only publish/run with secrets on merges to protected branches.
🤖 Prompt for AI Agents
In .github/workflows/pump-version.yml around lines 26 and 41-55, the workflow
builds the Docker image from PR-supplied code and runs it with secrets mounted,
risking secret exposure. To fix this, either use a pre-built, versioned Docker
image from a trusted registry instead of building from PR code, or restrict the
job to run only on protected branches by adding a condition like `if: github.ref
== 'refs/heads/minswap-11'`. Alternatively, separate the build and publish steps
into different workflows so that secrets are only used in workflows triggered by
merges to protected branches.
| echo "update version field" | ||
| sed -i -E 's/"version": ".+minswap.+"/"version": "'"$version"'"/' package.json | ||
| sed -i -E 's/"version": ".+minswap.+"/"version": "'"$version"'"/' package-lock.json | ||
|
|
||
| sed -i -E 's/version = ".+minswap.+"/version = "'"$version"'"/' rust/Cargo.toml | ||
| sed -i -E 's/version = ".+minswap.+"/version = "'"$version"'"/' rust/Cargo.lock | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
sed patterns are over-greedy and may touch unintended matches.
.+minswap.+spans across quotes and will replace every “version” field that containsminswap, even in nested dependencies ofpackage-lock.json.- A single
-irun rewrites all occurrences, so unsafe replacements have no easy rollback.
Prefer precise semver‐aware replacements via jq / cargo set-version, or at least tighten the regex:
-sed -i -E 's/"version": ".+minswap.+"/"version": "'"$version"'"/' package.json
+sed -i -E 's/"version": "[0-9]+\.[0-9]+\.[0-9]+-minswap\.[0-9]+"/"version": "'"$version"'"/' package.jsonRepeat for the other files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "update version field" | |
| sed -i -E 's/"version": ".+minswap.+"/"version": "'"$version"'"/' package.json | |
| sed -i -E 's/"version": ".+minswap.+"/"version": "'"$version"'"/' package-lock.json | |
| sed -i -E 's/version = ".+minswap.+"/version = "'"$version"'"/' rust/Cargo.toml | |
| sed -i -E 's/version = ".+minswap.+"/version = "'"$version"'"/' rust/Cargo.lock | |
| echo "update version field" | |
| sed -i -E 's/"version": "[0-9]+\.[0-9]+\.[0-9]+-minswap\.[0-9]+"/"version": "'"$version"'"/' package.json | |
| sed -i -E 's/"version": ".+minswap.+"/"version": "'"$version"'"/' package-lock.json | |
| sed -i -E 's/version = ".+minswap.+"/version = "'"$version"'"/' rust/Cargo.toml | |
| sed -i -E 's/version = ".+minswap.+"/version = "'"$version"'"/' rust/Cargo.lock |
🤖 Prompt for AI Agents
In docker-pump-version.sh around lines 7 to 13, the sed patterns are too broad
and replace all version fields containing "minswap," including nested
dependencies, which is unsafe. Replace these with more precise commands: use jq
to update the version field in package.json and package-lock.json, and use cargo
set-version to update the version in Cargo.toml and Cargo.lock. This approach
ensures only the intended version fields are updated safely without affecting
nested dependencies or requiring complex regex.
Summary by CodeRabbit
New Features
Chores