feat: add Docker README validation workflow#28
Conversation
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-28
|
🚅 Deployed to the applirank-pr-28 environment in applirank
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA GitHub Actions workflow is added to validate Docker container setup and service readiness. The workflow checks out the repository, provisions environment variables, starts Docker services, installs dependencies, starts a Nuxt development server, polls for HTTP readiness, and performs cleanup with comprehensive logging. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/docker-readme-validation.yml (1)
83-84: Prefernpm ciovernpm installin CI.
npm installcan silently updatepackage-lock.jsonon a stale lockfile, making the validation non-deterministic.npm ciinstalls exactly what the lockfile declares and fails fast on any mismatch — which is what a validation workflow should do regardless of README wording.♻️ Proposed change
- - name: Install dependencies (README parity) - run: npm install + - name: Install dependencies + run: npm ci🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-readme-validation.yml around lines 83 - 84, Replace the CI dependency step named "Install dependencies (README parity)" to use deterministic installs: change the command from "npm install" to "npm ci" so the workflow installs exactly what's in package-lock.json and fails on lockfile mismatches; update the run value for that step (the step with the name Install dependencies (README parity)) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docker-readme-validation.yml:
- Around line 93-96: The curl in the polling loop (the command invoking curl
-fsS http://127.0.0.1:3000) can hang if a TCP connection is accepted but the
HTTP stack stalls; modify that curl invocation to include a per-attempt timeout
(e.g., add --max-time 5) so each poll attempt is bounded and the loop can
retry/exit reliably.
- Around line 59-65: The docker inspect assignment for current_state (the line
that sets current_state="$(docker inspect --format=... "$container_id")") can
return non-zero if the container dies between ps -q and inspect and will trigger
set -e; modify that assignment so inspect failures do not cause the script to
exit (e.g. capture or ignore inspect errors and return a safe default) — ensure
the container_id check remains, but change the current_state assignment to
tolerate docker inspect failing (use a shell-safe fallback such as appending a
no-op failure handler or redirecting stderr and || true) so the timeout/log dump
path still executes.
- Around line 22-29: Replace mutable action tags with immutable commit SHAs:
change uses: actions/checkout@v4 to uses:
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 (v4 SHA) and update
uses: actions/setup-node@v4 to the current stable release commit SHA from the
actions/setup-node releases page; update the workflow step names "Checkout" and
"Setup Node.js" accordingly and add a comment referencing Dependabot for
automated SHA updates.
---
Nitpick comments:
In @.github/workflows/docker-readme-validation.yml:
- Around line 83-84: Replace the CI dependency step named "Install dependencies
(README parity)" to use deterministic installs: change the command from "npm
install" to "npm ci" so the workflow installs exactly what's in
package-lock.json and fails on lockfile mismatches; update the run value for
that step (the step with the name Install dependencies (README parity))
accordingly.
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| cache: npm |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
actions/checkout v4 current commit SHA GitHub Actions
💡 Result:
actions/checkout@v4 currently resolves to commit SHA:
34e114876b0b11c390a56381ad16ebd13914f8d5 (tag v4 points to commit 34e1148…). [1][2]
Sources:
[1] https://github.com/actions/checkout/tags
[2] actions/checkout@34e1148
🌐 Web query:
GitHub Actions security best practices pinning commit SHA vs version tags
💡 Result:
Pinning uses: in GitHub Actions: commit SHA vs version tags
What GitHub recommends (most secure)
- Pin third-party actions to a full-length commit SHA (immutable reference). This is GitHub’s primary security hardening recommendation because tags/branches can be moved if an action repo (or maintainer account) is compromised. [1]
Example:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7Best practice is to use the SHA that corresponds to a release tag and keep the human-friendly version in a comment. [2]
When tags are acceptable (more convenient, less secure)
- Tags (including
v1,v2,v3and evenv4.1.7) are mutable unless you have strong guarantees; they can be moved or deleted, so you’re trusting the publisher and their account security. GitHub notes tags are used widely for convenience, but are riskier than SHAs. [1]
Operational tradeoff (how to stay secure and up-to-date)
- SHA pinning prevents surprise changes, but you must update intentionally.
- Use Dependabot version updates to propose PRs that bump pinned SHAs (ideally when your SHA is tied to a release tag). [2]
Enforcing at scale (org/repo controls)
- GitHub supports policies to require SHA pinning so workflows fail if an action isn’t pinned to a full commit SHA. [1]
- GitHub also added admin policy support to enforce SHA pinning (and to block specific actions/versions) via the “allowed actions and reusable workflows” policy. [3]
One more gotcha: “pinned” can still be effectively mutable
Even if you pin the action itself to a SHA, review whether it pulls mutable dependencies (e.g., unpinned container images, downloads scripts at runtime, or calls other actions by floating tags). [2]
Bottom line: For security best practice, pin to full commit SHAs (preferably the SHA for a tagged release) and automate updates with Dependabot; use tags only when you knowingly accept the added supply-chain risk. [1][2]
Pin third-party actions to full commit SHAs instead of mutable version tags.
Lines 22-29 use actions/checkout@v4 and actions/setup-node@v4. Version tags can be moved or compromised; pin to immutable commit SHAs per GitHub security guidance:
actions/checkout@v4→actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4actions/setup-node@v4→ look up current stable SHA at https://github.com/actions/setup-node/releases
Use Dependabot to automate updates to these pinned SHAs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-readme-validation.yml around lines 22 - 29, Replace
mutable action tags with immutable commit SHAs: change uses: actions/checkout@v4
to uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 (v4 SHA) and
update uses: actions/setup-node@v4 to the current stable release commit SHA from
the actions/setup-node releases page; update the workflow step names "Checkout"
and "Setup Node.js" accordingly and add a comment referencing Dependabot for
automated SHA updates.
| if [ -n "$container_id" ]; then | ||
| current_state="$(docker inspect --format='{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}' "$container_id")" | ||
| echo "$service state: $current_state" | ||
| if [ "$current_state" = "$expected_state" ]; then | ||
| break | ||
| fi | ||
| fi |
There was a problem hiding this comment.
docker inspect may trip set -e if container exits between the ps -q and inspect calls.
If the container dies in that narrow window, the assignment on line 60 exits non-zero, set -euo pipefail terminates the step immediately, and the useful timeout/log dump (lines 71–72) is never reached.
🛡️ Proposed fix
container_id="$(docker compose ps -q "$service")"
if [ -n "$container_id" ]; then
- current_state="$(docker inspect --format='{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}' "$container_id")"
+ current_state="$(docker inspect --format='{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}' "$container_id" 2>/dev/null)" || true
echo "$service state: $current_state"
if [ "$current_state" = "$expected_state" ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-readme-validation.yml around lines 59 - 65, The
docker inspect assignment for current_state (the line that sets
current_state="$(docker inspect --format=... "$container_id")") can return
non-zero if the container dies between ps -q and inspect and will trigger set
-e; modify that assignment so inspect failures do not cause the script to exit
(e.g. capture or ignore inspect errors and return a safe default) — ensure the
container_id check remains, but change the current_state assignment to tolerate
docker inspect failing (use a shell-safe fallback such as appending a no-op
failure handler or redirecting stderr and || true) so the timeout/log dump path
still executes.
| for _ in $(seq 1 90); do | ||
| if curl -fsS http://127.0.0.1:3000 > /dev/null; then | ||
| echo "Nuxt dev server is reachable on http://127.0.0.1:3000" | ||
| exit 0 |
There was a problem hiding this comment.
Add --max-time to curl to bound each poll attempt.
Without --max-time, a connection that is accepted but stalls (Nuxt dev server accepting TCP before the HTTP stack is ready) will cause curl to hang indefinitely, silently consuming the entire job timeout rather than retrying.
🛡️ Proposed fix
- if curl -fsS http://127.0.0.1:3000 > /dev/null; then
+ if curl -fsS --max-time 5 http://127.0.0.1:3000 > /dev/null; then📝 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.
| for _ in $(seq 1 90); do | |
| if curl -fsS http://127.0.0.1:3000 > /dev/null; then | |
| echo "Nuxt dev server is reachable on http://127.0.0.1:3000" | |
| exit 0 | |
| for _ in $(seq 1 90); do | |
| if curl -fsS --max-time 5 http://127.0.0.1:3000 > /dev/null; then | |
| echo "Nuxt dev server is reachable on http://127.0.0.1:3000" | |
| exit 0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-readme-validation.yml around lines 93 - 96, The
curl in the polling loop (the command invoking curl -fsS http://127.0.0.1:3000)
can hang if a TCP connection is accepted but the HTTP stack stalls; modify that
curl invocation to include a per-attempt timeout (e.g., add --max-time 5) so
each poll attempt is bounded and the loop can retry/exit reliably.
…rove readiness check
railway-app
Bot
temporarily deployed
to
applirank / applirank-pr-28
1 participant
Summary
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit