chore: GitHub workflow to auto-deploy PRs to Cloudflare#880
chore: GitHub workflow to auto-deploy PRs to Cloudflare#880
Conversation
Add GitHub workflows to automatically deploy PRs to Cloudflare Containers after tests pass, with automatic cleanup after 24 hours. Components: - workers/preview/: Cloudflare Worker entry point that routes traffic to the Rails container - preview-deploy.yml: Deploys PRs after CI passes, comments preview URL on PR - preview-cleanup.yml: Cleans up previews on PR close or after 24 hours via scheduled job The container sleeps after 30 minutes of inactivity and wakes automatically on the next request. Required secrets: - CLOUDFLARE_API_TOKEN - CLOUDFLARE_ACCOUNT_ID - CLOUDFLARE_WORKERS_SUBDOMAIN https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
- Add preview-specific Dockerfile with PostgreSQL server included - Add docker-entrypoint.sh to start PostgreSQL and run migrations - Change RAILS_ENV from production to development - Auto-generate SECRET_KEY_BASE and DATABASE_URL for self-contained previews https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
- Install redis-server in the preview Dockerfile - Start Redis in the entrypoint before PostgreSQL - Auto-configure REDIS_URL for Sidekiq background jobs https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
jjmata
added
the
infra/tooling
github-project-automation
bot
moved this to Todo
in Next release (v0.6.8) - `alpha.*` builds
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-PR preview deployment and scheduled/manual cleanup: a deploy workflow that builds and deploys PR-specific Cloudflare preview Workers and records GitHub deployments, plus a cleanup workflow that deletes previews on PR close or by age/manual run. Also adds preview worker code, config, and a Dockerfile for Rails previews. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as GitHub PR
participant Actions as GitHub Actions
participant CF as Cloudflare (wrangler/API)
participant GHDeploy as GitHub Deployments
participant Worker as Preview Worker
PR->>Actions: open/synchronize/reopen PR
Actions->>CF: build & wrangler deploy sure-preview-{PR#}
Actions->>GHDeploy: create deployment (env: preview-pr-{PR#})
CF->>Worker: instantiate/update container instance
Actions->>GHDeploy: update deployment status with environment_url
Actions->>PR: post or update PR comment with preview URL
Actions->>Actions: upload artifact with cleanup metadata (24h)
sequenceDiagram
participant Trigger as Cleanup Trigger
participant Actions as GitHub Actions
participant CF as Cloudflare (API)
participant GHDeploy as GitHub Deployments
alt PR closed event
Trigger->>Actions: run cleanup-on-close for PR#{n}
Actions->>CF: wrangler delete sure-preview-{n}
Actions->>GHDeploy: mark preview-pr-{n} inactive
else Scheduled/manual run
Trigger->>Actions: run cleanup-expired
Actions->>CF: list workers (sure-preview-*)
CF-->>Actions: worker list + metadata
loop per worker older than 24h
Actions->>CF: delete worker
Actions->>GHDeploy: mark preview-pr-{derived} inactive
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 900391c9b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/workflows/preview-cleanup.yml:
- Around line 104-110: When called with a specific inputs.pr_number the script
only deletes the Cloudflare Worker (WORKER_NAME and wrangler delete) but doesn't
mark the related GitHub deployment as inactive; update the manual-PR branch of
the workflow to mirror the batch cleanup by calling the GitHub Deployments API
(using gh api) to find the deployment for that PR/environment and POST a
deployment_status with "state":"inactive" after wrangler delete (use
inputs.pr_number to construct the environment or lookup the deployment, then
call gh api repos/:owner/:repo/deployments/:deployment_id/statuses with
{"state":"inactive"}); ensure error handling mirrors the batch path so stale
deployments are deactivated.
In @.github/workflows/preview-deploy.yml:
- Around line 69-74: The workflow run is passing an unquoted shell expansion for
PR_NUMBER which triggers SC2086; update the npx wrangler deploy invocation to
quote the variable assignment (e.g., use --var 'PR_NUMBER=${{
github.event.pull_request.number }}') so the shell treats it as a single word,
and ensure any other uses like the PREVIEW_URL construction remain quoted;
modify the command in the run block where npx wrangler deploy and PREVIEW_URL
are defined (look for the line containing npx wrangler deploy --var
PR_NUMBER:${{ github.event.pull_request.number }} and the PREVIEW_URL
assignment) to apply the quoted form.
- Around line 137-158: The Schedule cleanup step sets CLEANUP_TIME but doesn't
persist it to the artifact, so the cleanup workflow can't read the timestamp;
modify the actions/github-script block (the "Schedule cleanup" step) to write a
small metadata file (e.g., cleanup-meta.json or cleanup-time.txt) containing the
cleanupTime/CLEANUP_TIME using fs.writeFileSync, and then include that file's
path alongside workers/preview/wrangler.toml in the "Store cleanup metadata"
upload-artifact step (preview-cleanup-pr-${{ github.event.pull_request.number
}}) so the cleanup workflow can read the timestamp from the artifact.
- Around line 29-34: The workflow references a non-existent package-lock file
via the actions/setup-node setting cache-dependency-path:
workers/preview/package-lock.json which prevents npm cache from working; either
generate that lockfile by running npm install in the workers/preview directory
(so package-lock.json exists) and commit it, or remove the cache-dependency-path
line from the Setup Node.js step (actions/setup-node@v4) to disable npm caching
for that job; update the YAML accordingly to ensure node-version: "20" and
cache: "npm" remain consistent with the chosen approach.
In `@workers/preview/Dockerfile`:
- Line 7: The FROM instruction uses the non-canonical registry hostname
"registry.docker.com"; update the Dockerfile's FROM line (the stage named
"base") to use the standard Docker Hub endpoint by replacing
"registry.docker.com/library/ruby:$RUBY_VERSION-slim" with either
"docker.io/library/ruby:$RUBY_VERSION-slim" or simply "ruby:$RUBY_VERSION-slim".
In `@workers/preview/src/index.ts`:
- Around line 7-25: The RailsContainer lifecycle hooks are not matching the
`@cloudflare/containers` API: update the onStop method signature in class
RailsContainer to accept stopParams (e.g., stopParams: { exitCode?: number;
reason?: string }) and log or use stopParams.reason instead of the no-arg
version, and update onError to both log the error and re-throw it (i.e.,
console.error("Rails container error:", error); throw error;) so the container
preserves the default fail-fast behavior; keep defaultPort and sleepAfter as-is.
🧹 Nitpick comments (6)
workers/preview/docker-entrypoint.sh (2)
10-16: Add failure handling when Redis doesn't start.The wait loop exits silently if Redis never becomes ready after 10 attempts. Consider adding explicit failure handling to avoid silent issues during container startup.
🛠️ Proposed fix
# Wait for Redis to be ready echo "Waiting for Redis to be ready..." +redis_ready=false for i in {1..10}; do if redis-cli ping > /dev/null 2>&1; then echo "Redis is ready" + redis_ready=true break fi sleep 1 done + +if [ "$redis_ready" = false ]; then + echo "ERROR: Redis failed to start within 10 seconds" + exit 1 +fi
24-30: Add failure handling when PostgreSQL doesn't start.Similar to the Redis loop, this wait loop exits silently if PostgreSQL never becomes ready. The static analysis warning about unused variable
i(SC2034) can also be addressed by using_instead.🛠️ Proposed fix
# Wait for PostgreSQL to be ready echo "Waiting for PostgreSQL to be ready..." -for i in {1..30}; do +pg_ready=false +for _ in {1..30}; do if pg_isready -h localhost -U postgres > /dev/null 2>&1; then echo "PostgreSQL is ready" + pg_ready=true break fi sleep 1 done + +if [ "$pg_ready" = false ]; then + echo "ERROR: PostgreSQL failed to start within 30 seconds" + exit 1 +fi.github/workflows/preview-deploy.yml (1)
76-89: Consider handling the 'cancelled' job status.The deployment status update only handles 'success' and assumes all other states are 'failure'. GitHub Actions also has a 'cancelled' state which might warrant different handling or messaging.
🛠️ Proposed enhancement
script: | - const state = '${{ job.status }}' === 'success' ? 'success' : 'failure'; + const jobStatus = '${{ job.status }}'; + let state; + if (jobStatus === 'success') { + state = 'success'; + } else if (jobStatus === 'cancelled') { + state = 'inactive'; + } else { + state = 'failure'; + } await github.rest.repos.createDeploymentStatus({.github/workflows/preview-cleanup.yml (3)
36-37: Consider pinning the Wrangler version for reproducibility.Installing
wranglerwithout a version specifier means the workflow may behave differently over time as new versions are released, potentially introducing breaking changes.♻️ Suggested fix
- name: Install Wrangler - run: npm install -g wrangler + run: npm install -g wrangler@3Alternatively, pin to a specific minor version (e.g.,
wrangler@3.99.0) for stricter control.
151-151: Use bash parameter expansion instead ofsed.Per static analysis (SC2001), this can be simplified using bash parameter expansion, which is more efficient and avoids spawning a subprocess.
♻️ Suggested fix
# Extract PR number and cleanup GitHub deployment - PR_NUM=$(echo "$WORKER" | sed 's/sure-preview-//') + PR_NUM=${WORKER#sure-preview-}
95-96: Same version pinning concern as the other job.Apply consistent version pinning here as well.
♻️ Suggested fix
- name: Install Wrangler - run: npm install -g wrangler + run: npm install -g wrangler@3
| - name: Schedule cleanup | ||
| if: success() | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| // Create a repository dispatch event to trigger cleanup after 24 hours | ||
| // The cleanup workflow will use the scheduled time to determine if cleanup is needed | ||
| const fs = require('fs'); | ||
| const cleanupTime = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(); | ||
|
|
||
| // Store cleanup schedule in artifact for the cleanup workflow | ||
| core.exportVariable('CLEANUP_TIME', cleanupTime); | ||
| console.log(`Preview scheduled for cleanup at: ${cleanupTime}`); | ||
|
|
||
| - name: Store cleanup metadata | ||
| if: success() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: preview-cleanup-pr-${{ github.event.pull_request.number }} | ||
| path: | | ||
| workers/preview/wrangler.toml | ||
| retention-days: 2 |
There was a problem hiding this comment.
Cleanup scheduling mechanism appears incomplete.
The Schedule cleanup step exports CLEANUP_TIME as an environment variable, but this value isn't persisted to the artifact. The subsequent Store cleanup metadata step only uploads wrangler.toml, not the cleanup timestamp. The cleanup workflow will need some way to determine when the preview should be deleted.
Consider creating a metadata file with the cleanup time and including it in the artifact:
🛠️ Proposed fix
- name: Schedule cleanup
if: success()
uses: actions/github-script@v7
with:
script: |
- // Create a repository dispatch event to trigger cleanup after 24 hours
- // The cleanup workflow will use the scheduled time to determine if cleanup is needed
const fs = require('fs');
const cleanupTime = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString();
-
- // Store cleanup schedule in artifact for the cleanup workflow
- core.exportVariable('CLEANUP_TIME', cleanupTime);
+ const metadata = {
+ pr_number: ${{ github.event.pull_request.number }},
+ cleanup_time: cleanupTime,
+ deployed_at: new Date().toISOString()
+ };
+ fs.writeFileSync('workers/preview/cleanup-metadata.json', JSON.stringify(metadata, null, 2));
console.log(`Preview scheduled for cleanup at: ${cleanupTime}`);
- name: Store cleanup metadata
if: success()
uses: actions/upload-artifact@v4
with:
name: preview-cleanup-pr-${{ github.event.pull_request.number }}
path: |
workers/preview/wrangler.toml
+ workers/preview/cleanup-metadata.json
retention-days: 2🤖 Prompt for AI Agents
In @.github/workflows/preview-deploy.yml around lines 137 - 158, The Schedule
cleanup step sets CLEANUP_TIME but doesn't persist it to the artifact, so the
cleanup workflow can't read the timestamp; modify the actions/github-script
block (the "Schedule cleanup" step) to write a small metadata file (e.g.,
cleanup-meta.json or cleanup-time.txt) containing the cleanupTime/CLEANUP_TIME
using fs.writeFileSync, and then include that file's path alongside
workers/preview/wrangler.toml in the "Store cleanup metadata" upload-artifact
step (preview-cleanup-pr-${{ github.event.pull_request.number }}) so the cleanup
workflow can read the timestamp from the artifact.
| # Includes PostgreSQL and Redis for self-contained development testing | ||
|
|
||
| ARG RUBY_VERSION=3.4.7 | ||
| FROM registry.docker.com/library/ruby:$RUBY_VERSION-slim AS base |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Docker Hub registry URL docker.io registry.docker.com official
💡 Result:
- Canonical “Docker Hub” registry hostname in image names:
docker.io(e.g.,docker.io/library/ubuntu). The legacy default domain wasindex.docker.io. [1] - Actual OCI/Docker Registry (v2) API endpoint used for pulls/pushes:
https://registry-1.docker.io(e.g.,GET https://registry-1.docker.io/v2/...). [2] - Auth/token service used in the v2 flow: the registry challenges with a token realm
https://auth.docker.io/tokenandservice="registry.docker.io". [3] registry.docker.com/registry.docker.io:registry.docker.ioappears as the service name in Docker’s auth challenge (not the registry data endpoint). The documented registry data endpoint isregistry-1.docker.io;registry.docker.comis not the standard/official Docker Hub registry API endpoint to target. [2][3]
Sources:
[1] https://git.taigrr.com/taigrr/wtf/src/commit/260ad3e968729d375e4b168decd284bb543e9aec/vendor/github.com/docker/distribution/reference/normalize.go
[2] https://github.com/docker-archive/docker-registry
[3] https://docs.docker.com/reference/api/registry/auth/
Change base image registry to the standard Docker Hub endpoint.
The base image uses registry.docker.com which is not an official Docker Hub registry endpoint. The canonical registry hostname is docker.io. Change to docker.io/library/ruby:$RUBY_VERSION-slim or simply ruby:$RUBY_VERSION-slim (registry prefix can be omitted to default to Docker Hub).
🤖 Prompt for AI Agents
In `@workers/preview/Dockerfile` at line 7, The FROM instruction uses the
non-canonical registry hostname "registry.docker.com"; update the Dockerfile's
FROM line (the stage named "base") to use the standard Docker Hub endpoint by
replacing "registry.docker.com/library/ruby:$RUBY_VERSION-slim" with either
"docker.io/library/ruby:$RUBY_VERSION-slim" or simply "ruby:$RUBY_VERSION-slim".
| export class RailsContainer extends Container { | ||
| // Rails runs on port 3000 | ||
| defaultPort = 3000; | ||
|
|
||
| // Sleep after 30 minutes of inactivity to save resources | ||
| sleepAfter = "30m"; | ||
|
|
||
| override onStart(): void { | ||
| console.log("Rails container starting..."); | ||
| } | ||
|
|
||
| override onStop(): void { | ||
| console.log("Rails container stopped"); | ||
| } | ||
|
|
||
| override onError(error: unknown): void { | ||
| console.error("Rails container error:", error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cloudflare Containers API sleepAfter format documentation
💡 Result:
In Cloudflare’s Containers API (@cloudflare/containers), sleepAfter controls how long a container stays alive with no activity.
Type / format
string | number- Number: interpreted as seconds
- String: a duration like
"30s","5m", or"1h"(seconds/minutes/hours) - Default:
"10m"(10 minutes) [1][2]
Examples:
sleepAfter = 300(5 minutes) [2]sleepAfter = "5m"[1][2]
Sources: Cloudflare Containers docs [1] and cloudflare/containers API reference [2].
🌐 Web query:
Cloudflare Containers lifecycle hooks onStart onStop onError
💡 Result:
In Cloudflare Containers (the @cloudflare/containers package), lifecycle hooks are methods you override on your Container subclass; they run automatically when the container’s state changes. [1][2]
Hooks
onStart(): runs when the container starts successfully (called on state transitions likestopped → runningandrunning → healthy). [1]onStop(stopParams): runs when the container shuts down;stopParamscan include fields likeexitCodeand areason(useful to distinguish graceful vs crash/kill/inactivity). [2]onError(error): runs when the container encounters an error; the default behavior is to log and re-throw unless you override it. [1]
Example
import { Container } from "@cloudflare/containers";
export class MyContainer extends Container {
defaultPort = 4000;
sleepAfter = "5m";
override onStart() {
console.log("started");
}
override onStop(stopParams) {
console.log("stopped", stopParams.exitCode, stopParams.reason);
}
override onError(error) {
console.log("error", error);
throw error; // optional: keep default “fail fast” behavior
}
}[2][1]
Sources: Cloudflare Containers “Status Hooks” example and @cloudflare/containers docs/README. [2][1]
Update onStop() and onError() lifecycle hooks to match the @cloudflare/containers API contract.
The sleepAfter = "30m" format is correct. However, two lifecycle hooks need fixes:
-
onStop()should acceptstopParamsparameter to accessexitCodeandreason:override onStop(stopParams: { exitCode?: number; reason?: string }): void { console.log("Rails container stopped", stopParams.reason); }
-
onError()should re-throw the error to maintain default "fail fast" behavior:override onError(error: unknown): void { console.error("Rails container error:", error); throw error; }
🤖 Prompt for AI Agents
In `@workers/preview/src/index.ts` around lines 7 - 25, The RailsContainer
lifecycle hooks are not matching the `@cloudflare/containers` API: update the
onStop method signature in class RailsContainer to accept stopParams (e.g.,
stopParams: { exitCode?: number; reason?: string }) and log or use
stopParams.reason instead of the no-arg version, and update onError to both log
the error and re-throw it (i.e., console.error("Rails container error:", error);
throw error;) so the container preserves the default fail-fast behavior; keep
defaultPort and sleepAfter as-is.
When using workflow_dispatch with a specific pr_number, the workflow now also marks the associated GitHub deployment as inactive, mirroring the behavior of the batch cleanup path. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/preview-cleanup.yml:
- Around line 1-26: The workflow lacks explicit permissions for deployment
status operations, causing REST API calls to fail; add a permissions block
immediately after the "on:" section that grants deployments: write so the
GITHUB_TOKEN can list and update deployment statuses used by the cleanup jobs
(e.g., jobs like cleanup-on-close and any job that calls the REST API for
deployment/status updates).
- Around line 99-121: The script currently inlines `${{ inputs.pr_number }}`
into bash which risks command injection; change to pass the workflow_dispatch
input via an env var (e.g., set PR_INPUT in the job env) and read it in the
script as a plain variable, then validate it is a positive integer before use
(reject or skip if not numeric). Update uses of PR_NUM and WORKER_NAME (where
PR_NUM is assigned from the input) to use the sanitized numeric variable, and
only call `wrangler delete` and `gh api` when validation succeeds; keep existing
fallback/echo behavior on invalid or empty input.
| env: | ||
| CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} | ||
| CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| # If specific PR number provided, only cleanup that one | ||
| if [ -n "${{ inputs.pr_number }}" ]; then | ||
| PR_NUM="${{ inputs.pr_number }}" | ||
| WORKER_NAME="sure-preview-$PR_NUM" | ||
| echo "Manually deleting Worker: $WORKER_NAME" | ||
| wrangler delete --name "$WORKER_NAME" --force || echo "Worker may not exist" | ||
|
|
||
| # Cleanup GitHub deployment for this PR | ||
| echo "Cleaning up GitHub deployment for PR #$PR_NUM" | ||
| gh api \ | ||
| -X GET "/repos/${{ github.repository }}/deployments?environment=preview-pr-$PR_NUM" \ | ||
| --jq '.[].id' 2>/dev/null | while read -r DEPLOY_ID; do | ||
| if [ -n "$DEPLOY_ID" ]; then | ||
| gh api \ | ||
| -X POST "/repos/${{ github.repository }}/deployments/$DEPLOY_ID/statuses" \ | ||
| -f state=inactive \ | ||
| -f description="Preview manually deleted" || true | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat .github/workflows/preview-cleanup.ymlRepository: we-promise/sure
Length of output: 6785
🌐 Web query:
GitHub Actions workflow_dispatch input security best practices command injection prevention
💡 Result:
Best practices for workflow_dispatch inputs (command-injection prevention)
-
Treat all inputs as untrusted (even if only maintainers can run the workflow). Avoid letting attacker-controlled strings reach an executable sink (shell, JS
exec, SQL, CLI args, API payloads). GitHub explicitly warns that attacker-controlled values in thegithubcontext can lead to script injection if interpreted as code. [1] -
Do not inline
${{ inputs.* }}insiderun:scripts.
Expressions are expanded before the runner executes the generated script, which can turn input text into shell syntax. Use the “env indirection” pattern instead: put the input into an environment variable and reference it as a normal shell variable. [2]
Preferred pattern:- name: Safe usage of workflow_dispatch input env: TARGET: ${{ inputs.target }} run: | set -euo pipefail echo "Target is: $TARGET"
-
Validate inputs with an allowlist (and fail closed).
For anything that will influence commands/paths/refs:- Prefer
type: choicewith fixedoptionswhere possible (reduces surprise values). - Otherwise enforce strict regex/allowlist checks in the first step, before using the value anywhere.
- Prefer
-
Always quote shell variables; never
eval.
Even with env indirection, the value is still untrusted—double-quoting prevents word-splitting/globbing surprises, and avoidingevalprevents re-interpretation as code. GitHub Security Lab notes quoting as an additional good practice when using the env indirection remediation. [2] -
Minimize blast radius with least privilege.
- Set
GITHUB_TOKENpermissions to the minimum required (ideally default read-only; elevate per-job only when needed). [3] - Consider splitting “input handling/validation” into a low-privilege job, and only proceed to privileged jobs if validation passes.
- Set
-
Protect high-risk operations with Environments + required reviewers (and prevent self-approval).
For manual dispatch workflows that deploy/rotate secrets/etc., gate the deployment job behind a protected Environment with Required reviewers, and (where available) enable prevent self-reviews so the person who triggered the run can’t approve it themselves. [4] [5] -
Don’t use
workflow_dispatchinputs for secrets; mask anything sensitive that isn’t a secret.- Use GitHub Secrets for sensitive values; redaction is not foolproof, so avoid plaintext in workflow files and follow secret-handling guidance. [3]
- If you must accept a sensitive-ish value (generally discouraged), mask it (e.g.,
::add-mask::...) to reduce log leakage. [3]
-
Add automated detection for unsafe patterns.
GitHub recommends enabling code scanning; GitHub Security Lab/CodeQL can catch unsafe interpolation patterns in workflows. [6] -
Pin third-party actions and avoid mutable dependencies.
Pin actions to a full commit SHA and review action code/dependencies to reduce supply-chain risk (important because an injection often becomes “game over” if a workflow is already privileged). [7]
Sources
[1] GitHub Docs – Script injections (security risks, untrusted contexts) (docs.github.com)
[2] GitHub Security Lab – Untrusted input (why ${{ }} in run: is risky; env indirection remediation) (securitylab.github.com)
[3] GitHub Docs – Secure use reference (least privilege, masking, secrets guidance) (docs.github.com)
[4] GitHub Docs – Deployments & environments (required reviewers; prevent self-reviews option described) (docs.github.com)
[5] GitHub Changelog – Prevent self-reviews for secure deployments across environments (github.blog)
[6] GitHub Blog – Workflow security tips (CodeQL/code scanning for workflow injection patterns) (github.blog)
[7] GitHub Well-Architected – Actions security (avoid workflow injection; avoid mutable dependencies) (wellarchitected.github.com)
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections?utm_source=openai
- 2: https://securitylab.github.com/resources/github-actions-untrusted-input/?utm_source=openai
- 3: https://docs.github.com/en/actions/how-tos/security-for-github-actions/security-guides/using-githubs-security-features-to-secure-your-use-of-github-actions?utm_source=openai
- 4: https://docs.github.com/en/enterprise-server%403.15/actions/reference/workflows-and-actions/deployments-and-environments?utm_source=openai
- 5: https://github.blog/changelog/2023-10-16-actions-prevent-self-reviews-for-secure-deployments-across-actions-environments?utm_source=openai
- 6: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/?utm_source=openai
- 7: https://wellarchitected.github.com/library/application-security/recommendations/actions-security/?utm_source=openai
Move workflow_dispatch input to environment variable and validate format.
Line 105–107 inlines ${{ inputs.pr_number }} directly into bash, allowing command injection. GitHub Actions expands ${{ }} expressions before script execution, so an input like $(malicious_command) becomes executable code. Pass the input via env and validate it as numeric.
Safer input handling
- name: Cleanup expired previews
env:
CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ PR_NUM: ${{ inputs.pr_number }}
run: |
# If specific PR number provided, only cleanup that one
- if [ -n "${{ inputs.pr_number }}" ]; then
- PR_NUM="${{ inputs.pr_number }}"
+ if [ -n "$PR_NUM" ]; then
+ if ! [[ "$PR_NUM" =~ ^[0-9]+$ ]]; then
+ echo "Invalid pr_number: $PR_NUM"
+ exit 1
+ fi
WORKER_NAME="sure-preview-$PR_NUM"
echo "Manually deleting Worker: $WORKER_NAME"
wrangler delete --name "$WORKER_NAME" --force || echo "Worker may not exist"🤖 Prompt for AI Agents
In @.github/workflows/preview-cleanup.yml around lines 99 - 121, The script
currently inlines `${{ inputs.pr_number }}` into bash which risks command
injection; change to pass the workflow_dispatch input via an env var (e.g., set
PR_INPUT in the job env) and read it in the script as a plain variable, then
validate it is a positive integer before use (reject or skip if not numeric).
Update uses of PR_NUM and WORKER_NAME (where PR_NUM is assigned from the input)
to use the sanitized numeric variable, and only call `wrangler delete` and `gh
api` when validation succeeds; keep existing fallback/echo behavior on invalid
or empty input.
The setup-node action's cache feature requires a package-lock.json which doesn't exist in workers/preview/. Remove the cache configuration to fix the workflow. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
Add condition to check steps.deployment.outputs.result exists before attempting to update deployment status. This prevents a JavaScript syntax error when the deployment step fails and no ID is available. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
Quote the --var argument and GITHUB_OUTPUT redirection to prevent word splitting issues. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
Add deployments: write permission to the cleanup workflow so the GITHUB_TOKEN can list and update deployment statuses. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
Use object syntax for image config to set build context to repository root, allowing the Dockerfile to reference files from both the root (Gemfile, .ruby-version) and workers/preview/ (docker-entrypoint.sh). https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
- Update workflow to run wrangler with --config flag from repo root - Update wrangler.toml paths (main, image) to be relative to repo root - Embed entrypoint script directly in Dockerfile using heredoc - Remove separate docker-entrypoint.sh file This ensures the Docker build context includes Gemfile, .ruby-version, and other files at the repo root. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/preview-deploy.yml:
- Around line 61-72: The Deploy to Cloudflare Containers step uses npx wrangler
from the repo root which can fetch a different version; modify that job step
(the step named "Deploy to Cloudflare Containers") to run in the workers/preview
working directory and call the local wrangler config (change --config to
wrangler.toml) so it uses the repo's installed wrangler in
workers/preview/node_modules; keep the same environment variables and update the
PREVIEW_URL construction if it relies on the working directory, ensuring the
command becomes npx wrangler deploy --config wrangler.toml --var "PR_NUMBER:${{
github.event.pull_request.number }}" executed with working-directory:
workers/preview.
In `@workers/preview/Dockerfile`:
- Around line 64-123: The Dockerfile uses a shell heredoc inside a RUN that
BuildKit may not parse; change the heredoc to Dockerfile heredoc syntax by
moving the redirection after RUN, e.g. replace the line "RUN cat >
/rails/bin/preview-entrypoint << 'ENTRYPOINT_EOF'" with "RUN <<'ENTRYPOINT_EOF'
cat > /rails/bin/preview-entrypoint" so the heredoc is attached to the RUN
instruction (keep the same body and the terminating ENTRYPOINT_EOF, then keep
the chmod/chown lines). This targets the preview entrypoint creation block that
writes /rails/bin/preview-entrypoint.
| # Create preview entrypoint script inline | ||
| RUN cat > /rails/bin/preview-entrypoint << 'ENTRYPOINT_EOF' | ||
| #!/bin/bash | ||
| set -e | ||
|
|
||
| # Start Redis | ||
| echo "Starting Redis..." | ||
| sudo redis-server --daemonize yes --bind 127.0.0.1 | ||
|
|
||
| # Wait for Redis to be ready | ||
| echo "Waiting for Redis to be ready..." | ||
| for i in {1..10}; do | ||
| if redis-cli ping > /dev/null 2>&1; then | ||
| echo "Redis is ready" | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
|
|
||
| # Start PostgreSQL | ||
| echo "Starting PostgreSQL..." | ||
| sudo pg_ctlcluster 15 main start || sudo pg_ctlcluster 16 main start || sudo pg_ctlcluster 17 main start | ||
|
|
||
| # Wait for PostgreSQL to be ready | ||
| echo "Waiting for PostgreSQL to be ready..." | ||
| for i in {1..30}; do | ||
| if pg_isready -h localhost -U postgres > /dev/null 2>&1; then | ||
| echo "PostgreSQL is ready" | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
|
|
||
| # Create database user and database if they don't exist | ||
| echo "Setting up database..." | ||
| psql -h localhost -U postgres -tc "SELECT 1 FROM pg_roles WHERE rolname='rails'" | grep -q 1 || \ | ||
| psql -h localhost -U postgres -c "CREATE USER rails WITH SUPERUSER PASSWORD 'rails';" | ||
|
|
||
| psql -h localhost -U postgres -tc "SELECT 1 FROM pg_database WHERE datname='sure_development'" | grep -q 1 || \ | ||
| psql -h localhost -U postgres -c "CREATE DATABASE sure_development OWNER rails;" | ||
|
|
||
| # Set DATABASE_URL if not already set | ||
| export DATABASE_URL="${DATABASE_URL:-postgres://rails:rails@localhost:5432/sure_development}" | ||
|
|
||
| # Set REDIS_URL if not already set | ||
| export REDIS_URL="${REDIS_URL:-redis://localhost:6379/0}" | ||
|
|
||
| # Generate SECRET_KEY_BASE if not set | ||
| export SECRET_KEY_BASE="${SECRET_KEY_BASE:-$(openssl rand -hex 64)}" | ||
|
|
||
| # Run database migrations | ||
| echo "Running database migrations..." | ||
| ./bin/rails db:prepare | ||
|
|
||
| # Execute the main command | ||
| echo "Starting Rails server..." | ||
| exec "$@" | ||
| ENTRYPOINT_EOF | ||
| RUN chmod 755 /rails/bin/preview-entrypoint && chown rails:rails /rails/bin/preview-entrypoint | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Dockerfile heredoc syntax RUN instruction BuildKit
💡 Result:
BuildKit adds here-document (heredoc) support so you can write multi-line scripts directly in a single RUN (and also COPY) instruction.
1) Enable a Dockerfile syntax that supports heredocs
Put a syntax directive on the first line (recommended):
# syntax=docker/dockerfile:1This tells BuildKit which Dockerfile frontend to use. Newer features may require a newer/labs frontend (for example, older guides used docker/dockerfile:1.3-labs). [1][2]
2) Basic RUN heredoc (uses the default shell)
RUN <<EOF
apt-get update
apt-get install -y curl
EOFEverything until the delimiter-only line (EOF) is treated as part of the same RUN. [1]
3) Specify the interpreter/command explicitly
Put the command after the delimiter token:
RUN <<EOT bash
set -ex
apt-get update
apt-get install -y vim
EOT[1]
4) Use a shebang inside the heredoc
RUN <<EOT
#!/usr/bin/env python
print("hello world")
EOT[1][2]
5) Notes: variable expansion and tab stripping
- Normal heredoc rules apply (expansion depends on quoting the delimiter; you can also use
<<-EOFto allow tab-stripping). Docker’s reference shows examples likeCOPY <<-"EOT"to prevent build-time expansion. [1]
Sources: Dockerfile reference (Here-documents, RUN examples) [1]; Docker blog introduction to Dockerfile heredocs [2].
🌐 Web query:
Hadolint DL1000 error Dockerfile
💡 Result:
Hadolint DL1000 isn’t a “rule” violation—it’s the code Hadolint uses to report Dockerfile parse errors (i.e., Hadolint couldn’t parse your Dockerfile, so it can’t run the normal DL3xxx checks). This shows up as ruleId/checkName = DL1000 in formats like SARIF and CodeClimate. [1][2]
How to fix it
- Look at the parser error text right next to DL1000 (it should point to a line/column and say what token/construct was unexpected). That message is the real clue. [1]
- Fix common parse causes:
- Bad line continuations: trailing
\with extra spaces/comments, or missing continuation. - Invalid instruction syntax (e.g., malformed
RUN,ENV,ARG,COPY, JSON-array forms, quoting). - Using newer Dockerfile syntax/features that your Hadolint version doesn’t understand yet → upgrade Hadolint.
- Bad line continuations: trailing
- Run locally with more context, e.g.
hadolint -V Dockerfile(or whatever your integration runs), and/or compare withdocker buildto confirm where the syntax breaks. (Hadolint CLI supports standard file/format options; parse errors are surfaced before rule checks.) [3]
If you paste the exact DL1000 message + the failing Dockerfile lines, I can point out the precise syntax issue.
Sources: [1] [2] [3]
🌐 Web query:
Dockerfile RUN cat heredoc << syntax vs <<EOF
💡 Result:
There are two different “heredoc” mechanisms that can look similar in a Dockerfile:
1) Shell heredoc (old, works everywhere): RUN cat <<EOF … EOF
This is your shell (usually /bin/sh -c) parsing a heredoc and feeding it to cat (or any command).
RUN cat <<'EOF' >/etc/my.conf
line 1: $NOT_EXPANDED
EOF- The Dockerfile instruction is just
RUN …; everything happens inside the shell. - Quoting the delimiter (
<<'EOF') prevents shell variable expansion (standard shell behavior). - You still often end up needing
&& \line continuations for long sequences of commands.
2) Dockerfile heredoc (BuildKit feature): RUN <<EOF … EOF
This is the Dockerfile frontend (BuildKit) treating the following lines as part of the same RUN instruction (and optionally choosing the interpreter).
# syntax=docker/dockerfile:1
FROM debian
RUN <<'EOF' bash
set -e
echo "multi-line, no && \\ needed"
EOF- The heredoc is handled at the Dockerfile level, not by “escaping newlines” in a single shell line.
- It’s explicitly documented as “Here-Documents” for
RUN/COPY, and supports things likeRUN <<EOT bashand quoted delimiters to control expansion. [2] - Requires BuildKit / modern Dockerfile syntax frontend (older builders will fail to parse it). [1][2]
Rule of thumb:
- If you want to write a file: use shell heredoc with
cat <<EOF >file(portable), or DockerfileCOPY <<EOF /path(BuildKit). [2] - If you want a clean multi-line script in one RUN: prefer
RUN <<EOF … EOF(BuildKit). [1][2]
Sources: [1] Docker blog on Dockerfile heredocs, [2] Dockerfile reference (“Here-Documents”).
🏁 Script executed:
# Check if the Dockerfile has a syntax directive
head -n 10 workers/preview/DockerfileRepository: we-promise/sure
Length of output: 314
🏁 Script executed:
# Get the exact lines from the Dockerfile to verify the syntax
sed -n '64,75p' workers/preview/DockerfileRepository: we-promise/sure
Length of output: 361
Use Dockerfile heredoc syntax so the image builds.
With the # syntax=docker/dockerfile:1 directive present, use Dockerfile heredoc syntax (BuildKit feature) instead of shell heredoc. Line 65 currently uses RUN cat > ... << 'ENTRYPOINT_EOF', which the Dockerfile parser may not handle correctly, potentially causing parse errors (DL1000).
Fix
-RUN cat > /rails/bin/preview-entrypoint << 'ENTRYPOINT_EOF'
+RUN <<'ENTRYPOINT_EOF' cat > /rails/bin/preview-entrypoint🧰 Tools
🪛 Checkov (3.2.334)
[medium] 106-107: Basic Auth Credentials
(CKV_SECRET_4)
🪛 Hadolint (2.14.0)
[error] 67-67: unexpected 's'
expecting '#', '', ADD, ARG, CMD, COPY, ENTRYPOINT, ENV, EXPOSE, FROM, HEALTHCHECK, LABEL, MAINTAINER, ONBUILD, RUN, SHELL, STOPSIGNAL, USER, VOLUME, WORKDIR, a pragma, at least one space, or end of input
(DL1000)
🤖 Prompt for AI Agents
In `@workers/preview/Dockerfile` around lines 64 - 123, The Dockerfile uses a
shell heredoc inside a RUN that BuildKit may not parse; change the heredoc to
Dockerfile heredoc syntax by moving the redirection after RUN, e.g. replace the
line "RUN cat > /rails/bin/preview-entrypoint << 'ENTRYPOINT_EOF'" with "RUN
<<'ENTRYPOINT_EOF' cat > /rails/bin/preview-entrypoint" so the heredoc is
attached to the RUN instruction (keep the same body and the terminating
ENTRYPOINT_EOF, then keep the chmod/chown lines). This targets the preview
entrypoint creation block that writes /rails/bin/preview-entrypoint.
Wrangler resolves paths relative to the config file, not the current directory. Moving Dockerfile.preview to repo root ensures: - Build context is the repo root (where Gemfile, .ruby-version are) - Path in wrangler.toml is ../../Dockerfile.preview (relative to config) - Worker runs from workers/preview/ directory again https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Dockerfile.preview`:
- Around line 75-81: The readiness loops for Redis (the for loop using
"redis-cli ping") and PostgreSQL (the similar loop at lines handling pg
readiness) must fail explicitly when the retry limit is reached; modify both
loops to detect when the loop finishes without a successful connection and then
print a clear error message and exit with a non-zero status. Specifically,
update the redis loop (the for i in {1..10}; do ... redis-cli ping ... done) to
echo "Redis did not become ready in time" and run exit 1 after the loop if no
success, and apply the same pattern to the PostgreSQL readiness loop (the loop
that checks pg_isready/psql) so it echoes a "Postgres did not become ready in
time" message and exits non-zero on timeout.
🧹 Nitpick comments (2)
Dockerfile.preview (1)
85-85: Detect PostgreSQL version dynamically instead of hardcoding.The fallback chain
pg_ctlcluster 15 main start || ... 16 ... || ... 17is fragile. If the base image updates to a different version, this silently fails. Detect the installed version dynamically.♻️ Proposed fix
-sudo pg_ctlcluster 15 main start || sudo pg_ctlcluster 16 main start || sudo pg_ctlcluster 17 main start +PG_VERSION=$(ls /etc/postgresql/ | head -1) +sudo pg_ctlcluster "$PG_VERSION" main start.github/workflows/preview-deploy.yml (1)
29-36: Consider adding npm caching for faster deployments.The Node.js setup doesn't use npm caching. While this works, adding a
package-lock.jsontoworkers/preview/and enabling cache would speed up subsequent runs.
| for i in {1..10}; do | ||
| if redis-cli ping > /dev/null 2>&1; then | ||
| echo "Redis is ready" | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done |
There was a problem hiding this comment.
Readiness loops should fail if services don't start in time.
Both Redis and PostgreSQL readiness loops silently continue even if the service fails to start within the timeout period. This could lead to confusing downstream errors during database operations.
🛠️ Proposed fix for Redis loop
for i in {1..10}; do
if redis-cli ping > /dev/null 2>&1; then
echo "Redis is ready"
break
fi
+ if [ "$i" -eq 10 ]; then
+ echo "ERROR: Redis failed to start within timeout" >&2
+ exit 1
+ fi
sleep 1
doneApply the same pattern to the PostgreSQL loop (lines 89-95).
📝 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 i in {1..10}; do | |
| if redis-cli ping > /dev/null 2>&1; then | |
| echo "Redis is ready" | |
| break | |
| fi | |
| sleep 1 | |
| done | |
| for i in {1..10}; do | |
| if redis-cli ping > /dev/null 2>&1; then | |
| echo "Redis is ready" | |
| break | |
| fi | |
| if [ "$i" -eq 10 ]; then | |
| echo "ERROR: Redis failed to start within timeout" >&2 | |
| exit 1 | |
| fi | |
| sleep 1 | |
| done |
🤖 Prompt for AI Agents
In `@Dockerfile.preview` around lines 75 - 81, The readiness loops for Redis (the
for loop using "redis-cli ping") and PostgreSQL (the similar loop at lines
handling pg readiness) must fail explicitly when the retry limit is reached;
modify both loops to detect when the loop finishes without a successful
connection and then print a clear error message and exit with a non-zero status.
Specifically, update the redis loop (the for i in {1..10}; do ... redis-cli ping
... done) to echo "Redis did not become ready in time" and run exit 1 after the
loop if no success, and apply the same pattern to the PostgreSQL readiness loop
(the loop that checks pg_isready/psql) so it echoes a "Postgres did not become
ready in time" message and exits non-zero on timeout.
Shell glob patterns don't work with redirection operators. Use find to locate the actual pg_hba.conf path before writing to it. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
🚀 Preview Deployment ReadyYour preview environment has been deployed to Cloudflare Containers. Preview URL: https://sure-preview-880.sure-finances.workers.dev
Deployed from commit 7b1a529 |
Add workers_dev = true to make the preview worker accessible via the workers.dev subdomain. https://claude.ai/code/session_013EZuzBxWPEEYp3TQptXWdP
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Dockerfile.preview`:
- Around line 46-48: The sudoers include created for the rails user lacks the
required strict permissions; after writing the entry for "/etc/sudoers.d/rails"
(created in the RUN step that calls groupadd and useradd and echoes the sudo
line), set the file mode to 0440 so sudo will accept it — e.g., ensure the
Dockerfile RUN sequence that creates /etc/sudoers.d/rails (the echo to
/etc/sudoers.d/rails) also calls chmod 0440 on that file (or uses a creation
method that sets mode 0440) so the rails sudo entry is not ignored.
| RUN groupadd --system --gid 1000 rails && \ | ||
| useradd rails --uid 1000 --gid 1000 --create-home --shell /bin/bash && \ | ||
| echo "rails ALL=(ALL) NOPASSWD: /usr/bin/pg_ctlcluster, /usr/bin/redis-server" >> /etc/sudoers.d/rails |
There was a problem hiding this comment.
Set strict perms on the sudoers include so sudo doesn’t ignore it.
Files under /etc/sudoers.d must be mode 0440; otherwise sudo can reject them and the entrypoint’s sudo calls will fail, preventing Redis/Postgres from starting.
Fix
RUN groupadd --system --gid 1000 rails && \
useradd rails --uid 1000 --gid 1000 --create-home --shell /bin/bash && \
- echo "rails ALL=(ALL) NOPASSWD: /usr/bin/pg_ctlcluster, /usr/bin/redis-server" >> /etc/sudoers.d/rails
+ echo "rails ALL=(ALL) NOPASSWD: /usr/bin/pg_ctlcluster, /usr/bin/redis-server" >> /etc/sudoers.d/rails && \
+ chmod 0440 /etc/sudoers.d/rails🤖 Prompt for AI Agents
In `@Dockerfile.preview` around lines 46 - 48, The sudoers include created for the
rails user lacks the required strict permissions; after writing the entry for
"/etc/sudoers.d/rails" (created in the RUN step that calls groupadd and useradd
and echoes the sudo line), set the file mode to 0440 so sudo will accept it —
e.g., ensure the Dockerfile RUN sequence that creates /etc/sudoers.d/rails (the
echo to /etc/sudoers.d/rails) also calls chmod 0440 on that file (or uses a
creation method that sets mode 0440) so the rails sudo entry is not ignored.
2 participants
Summary by CodeRabbit
New Features
Chores