Modernize Docker-Prep for VM-Setup pinned launches - #6
Conversation
Align with VM-Setup hardening: honor ephemeral env vars, verify Scripts checksums, add shell-security CI, pin Portainer to lts, and publish releases so VM-Setup can sync pins. Co-authored-by: Michael <Narehood@users.noreply.github.com>
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds ephemeral pinned-launch support, checksum verification, stricter shell validation, pinned Portainer deployment, tagged-release automation, VM-Setup synchronization, and documentation for the new workflows. ChangesDocker-Prep security and release integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub
participant ReleaseWorkflow
participant VMSetup
GitHub->>ReleaseWorkflow: Push v* tag
ReleaseWorkflow->>ReleaseWorkflow: Validate tag against install.sh VERSION
ReleaseWorkflow->>GitHub: Create GitHub Release
ReleaseWorkflow->>VMSetup: Dispatch docker-prep-release with tag
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (1)
tests/security-checks.sh (1)
16-19: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRequire manifest coverage for every script.
sha256sum --checkonly validates listed entries. Compare manifest filenames againstScripts/*.shso a newly added but unlisted menu script cannot pass CI and then fail integrity verification at runtime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/security-checks.sh` around lines 16 - 19, Update the checksum validation block in tests/security-checks.sh to compare filenames in .checksums.sha256 against all Scripts/*.sh files, and fail when any script is missing from the manifest. Preserve the existing sha256sum --check --strict validation for listed entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/shell-security.yml:
- Around line 14-16: Update the actions/checkout step in the shell-security
workflow to pin it to the specified full commit SHA
34e114876b0b11c390a56381ad16ebd13914f8d5 instead of the mutable v4 tag, while
preserving persist-credentials: false.
In `@Scripts/portainer-install.sh`:
- Around line 262-266: Update the installation flow around pin_portainer_image
to download and validate the compose content in a temporary file first, then
atomically replace portainer-compose.yaml only after validation succeeds. On any
download or validation failure, remove only the temporary file and preserve the
existing live compose file.
---
Nitpick comments:
In `@tests/security-checks.sh`:
- Around line 16-19: Update the checksum validation block in
tests/security-checks.sh to compare filenames in .checksums.sha256 against all
Scripts/*.sh files, and fail when any script is missing from the manifest.
Preserve the existing sha256sum --check --strict validation for listed entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34545e07-1d83-41c5-a03a-bd6b3a3aaead
📒 Files selected for processing (12)
.github/workflows/release.yml.github/workflows/shell-security.yml.gitignoreREADME.mdScripts/.checksums.sha256Scripts/DockerGroup.shScripts/UserCreation.shScripts/portainer-install.shScripts/serverSetup.shinstall.shtests/security-checks.shtools/generate-checksums.sh
| - uses: actions/checkout@v4 | ||
| with: | ||
| persist-credentials: false |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh api repos/actions/checkout/commits/v4 --jq '.sha'Repository: Narehood/Docker-Prep
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sha="$(gh api repos/actions/checkout/commits/v4 --jq '.sha')"
echo "resolved_v4_sha=$sha"
echo "commit_lookup:"
gh api "repos/actions/checkout/commits/$sha" --jq '.sha'Repository: Narehood/Docker-Prep
Length of output: 271
Pin actions/checkout to a full commit SHA. v4 is mutable and can move without a repository change, which weakens supply-chain reproducibility in this security-sensitive workflow. Pin it to 34e114876b0b11c390a56381ad16ebd13914f8d5 instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/shell-security.yml around lines 14 - 16, Update the
actions/checkout step in the shell-security workflow to pin it to the specified
full commit SHA 34e114876b0b11c390a56381ad16ebd13914f8d5 instead of the mutable
v4 tag, while preserving persist-credentials: false.
| if ! pin_portainer_image "$compose_file"; then | ||
| $SUDO rm -f "$compose_file" | ||
| return 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the existing compose file when validation fails.
The download overwrites the live compose path, then this failure path deletes it. Stage the download in a temporary file, pin and validate it, and only then atomically replace portainer-compose.yaml; otherwise a transient upstream change can remove the last known-good configuration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Scripts/portainer-install.sh` around lines 262 - 266, Update the installation
flow around pin_portainer_image to download and validate the compose content in
a temporary file first, then atomically replace portainer-compose.yaml only
after validation succeeds. On any download or validation failure, remove only
the temporary file and preserve the existing live compose file.
|
Superseded by #7 ( |
2 participants
Summary
DOCKER_PREP_EPHEMERAL/DOCKER_PREP_REVISIONso VM-Setup temp checkouts disable self-update and show the pinned revisionScripts/*.shagainstScripts/.checksums.sha256before execution, withtools/generate-checksums.shfor regenerationtests/security-checks.sh+ ShellCheck) and a tag-driven Release workflow that can notify VM-Setup viarepository_dispatchportainer/portainer-ce:lts, keep download-to-file Docker install, and document the consumer/release contract in the README2.4.0for the modernization release lineAlignment with VM-Setup
Matches the consumer contract from VM-Setup PR #48 (
cursor/harden-bash-app-b08a): pinned revision launch, ephemeral env vars, and GitHub Releases for pin sync.Validation
bash tests/security-checks.shshellcheck --severity=error install.sh Scripts/*.sh tests/*.sh tools/*.shSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests