Add deploy script and workflow (#109)#110
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a Deploy GitHub Actions workflow and a remote Bash deploy script, expands CI push triggers to run on Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant CI as CI Job
participant DeployWF as Deploy Workflow
participant SSH as SSH Action
participant Server as Deployment Server
participant Script as deploy.sh
participant Git as Git Repo
participant Make as Make/Docker
GHA->>CI: Run CI on push (develop/main)
CI-->>GHA: Report success
GHA->>DeployWF: Trigger deploy workflow
DeployWF->>SSH: Fetch & run remote script
SSH->>Server: Connect and execute script
Server->>Script: Start deploy.sh
Script->>Script: Validate env, prerequisites
Script->>Git: Clone or update DEPLOY_DIR
Git-->>Script: Repo ready
Script->>Script: Check .env exists
Script->>Make: make down (best-effort)
Make-->>Script: teardown result
Script->>Make: make build
Make-->>Script: build result
Script->>Make: make up
Make-->>Script: services started
Script-->>Server: exit status
Server-->>SSH: return status
SSH-->>DeployWF: report outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
📝 Coding Plan
Comment |
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/deploy-script/deploy.sh (1)
24-33: Consider cleaning untracked files after reset.When updating an existing repository,
git reset --hardonly resets tracked files. Untracked files or directories (e.g., leftover build artifacts) may persist and cause unexpected behavior.💡 Proposed improvement
if [[ -d "$DEPLOY_DIR/.git" ]]; then log "Pulling latest in $DEPLOY_DIR..." git -C "$DEPLOY_DIR" fetch origin git -C "$DEPLOY_DIR" checkout "$BRANCH" git -C "$DEPLOY_DIR" reset --hard "origin/$BRANCH" + git -C "$DEPLOY_DIR" clean -fdx --exclude='.env' elseThe
--exclude='.env'preserves the manually managed environment file while removing other untracked files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-script/deploy.sh around lines 24 - 33, The update sequence in the deploy script (inside the branch where DEPLOY_DIR already exists) uses git reset --hard but does not remove untracked files; after git -C "$DEPLOY_DIR" reset --hard "origin/$BRANCH" add a git clean invocation to remove untracked files and directories while preserving manually managed files (e.g., exclude '.env') so leftover build artifacts don't persist; target the commands that reference DEPLOY_DIR and BRANCH in the existing if-branch (the git -C "$DEPLOY_DIR" fetch/checkout/reset block) and place the clean step immediately after the reset..github/workflows/deploy.yml (1)
41-48: Consider adding error handling for script download failure.While
curl -fwill fail on HTTP errors, the script execution continues sequentially. If the download fails,bash /tmp/deploy.shwill fail because the file doesn't exist, but the error message won't clearly indicate a download failure.💡 Proposed improvement for clearer error handling
script: | if [ -n "$DEPLOY_SCRIPT_TOKEN" ]; then - curl -sSL -f -H "Authorization: token $DEPLOY_SCRIPT_TOKEN" "$SCRIPT_URL" -o /tmp/deploy.sh + curl -sSL -f -H "Authorization: token $DEPLOY_SCRIPT_TOKEN" "$SCRIPT_URL" -o /tmp/deploy.sh || { echo "Failed to download deploy script"; exit 1; } else - curl -sSL -f "$SCRIPT_URL" -o /tmp/deploy.sh + curl -sSL -f "$SCRIPT_URL" -o /tmp/deploy.sh || { echo "Failed to download deploy script"; exit 1; } fi bash /tmp/deploy.sh rm -f /tmp/deploy.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy.yml around lines 41 - 48, The deployment job's script may continue even if curl fails to download SCRIPT_URL, so update the script block that uses DEPLOY_SCRIPT_TOKEN and /tmp/deploy.sh to explicitly verify the download succeeded before executing: after the curl command in both branches check the exit status or test that /tmp/deploy.sh exists and is non-empty, and if the check fails emit a clear error mentioning SCRIPT_URL and exit non‑zero to abort the job instead of running bash /tmp/deploy.sh; ensure the check and error message are placed immediately before the bash invocation in the same script block.
🤖 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/deploy-script/deploy.sh:
- Around line 35-42: The current fallback that copies "$DEPLOY_DIR/.env.example"
into "$DEPLOY_DIR/.env" is unsafe for production; change the conditional so the
copy only happens when an explicit opt-in variable (e.g., ALLOW_ENV_FALLBACK set
to "true") is present and non-empty. Concretely, update the if/elif block to
check ALLOW_ENV_FALLBACK (or a named flag) before running cp/chmod and keep the
existing log/error/exit flows (use log, cp, chmod, exit, and DEPLOY_DIR
identifiers) so that by default in production the script fails with the current
error message unless the operator explicitly enables the fallback.
---
Nitpick comments:
In @.github/workflows/deploy-script/deploy.sh:
- Around line 24-33: The update sequence in the deploy script (inside the branch
where DEPLOY_DIR already exists) uses git reset --hard but does not remove
untracked files; after git -C "$DEPLOY_DIR" reset --hard "origin/$BRANCH" add a
git clean invocation to remove untracked files and directories while preserving
manually managed files (e.g., exclude '.env') so leftover build artifacts don't
persist; target the commands that reference DEPLOY_DIR and BRANCH in the
existing if-branch (the git -C "$DEPLOY_DIR" fetch/checkout/reset block) and
place the clean step immediately after the reset.
In @.github/workflows/deploy.yml:
- Around line 41-48: The deployment job's script may continue even if curl fails
to download SCRIPT_URL, so update the script block that uses DEPLOY_SCRIPT_TOKEN
and /tmp/deploy.sh to explicitly verify the download succeeded before executing:
after the curl command in both branches check the exit status or test that
/tmp/deploy.sh exists and is non-empty, and if the check fails emit a clear
error mentioning SCRIPT_URL and exit non‑zero to abort the job instead of
running bash /tmp/deploy.sh; ensure the check and error message are placed
immediately before the bash invocation in the same script block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1c0d2049-54f3-422b-8b4a-27b546f5d608
📒 Files selected for processing (6)
.github/workflows/actions.yml.github/workflows/deploy-script/deploy.sh.github/workflows/deploy.ymlREADME.mddocs/Deployment.mddocs/README.md
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/actions.yml (1)
9-10: Consider filtering non-deployable pushes here too.
deploy.ymlis chained off successfulCIruns onpush, so this will now rebuild and restart staging/production even for docs-only or README-only merges todevelop/main. Mirroring the existingpull_request.paths-ignorerules here keeps deployments tied to deployable changes instead of every branch update.♻️ Suggested change
push: branches: [develop, main] + paths-ignore: + - "**.md" + - "docs/**" + - ".gitignore"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/actions.yml around lines 9 - 10, Update the push trigger in the .github/workflows/actions.yml 'push' block to mirror the existing pull_request.paths-ignore rules so CI isn't run for non-deployable changes (e.g., docs-only or README-only commits); specifically, add the same paths or paths-ignore patterns used for 'pull_request' to the 'push' trigger to prevent deploy.yml from being invoked on irrelevant pushes.
🤖 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/deploy-script/deploy.sh:
- Around line 14-15: The script currently assumes DEPLOY_DIR
(DEPLOY_DIR="${DEPLOY_DIR:-/opt/boost-data-collector}") is absent/empty which
breaks first-deploy for non-root users; update deploy.sh to handle an existing
deploy directory by detecting if "$DEPLOY_DIR" exists and is non-empty and then
either (a) perform a git clone into a temporary directory and rsync/move files
into DEPLOY_DIR, or (b) initialize/pull an existing repo with git -C
"$DEPLOY_DIR" fetch && git -C "$DEPLOY_DIR" checkout -f <branch> — and ensure
ownership/permissions are fixed (use chown/chmod) so the SSH deploy user can
write; implement this logic around the DEPLOY_DIR handling in deploy.sh and add
clear logging for each branch (created, reused, permission fixed).
In @.github/workflows/deploy.yml:
- Around line 27-33: The workflow step "Set SSH vars (production)" is writing
multiline secrets (PROD_SSH_PRIVATE_KEY / DEV_SSH_PRIVATE_KEY) into $GITHUB_ENV
using a single-line echo which corrupts the multiline key; change the write to
use a multiline heredoc into $GITHUB_ENV (e.g., use cat <<'EOF' >> $GITHUB_ENV
... EOF) so SSH_PRIVATE_KEY preserves newlines, keep the same variable names
(SSH_PRIVATE_KEY, SSH_HOST, SSH_USER, SSH_PORT) and ensure the
appleboy/ssh-action reads SSH_PRIVATE_KEY from the environment.
---
Nitpick comments:
In @.github/workflows/actions.yml:
- Around line 9-10: Update the push trigger in the .github/workflows/actions.yml
'push' block to mirror the existing pull_request.paths-ignore rules so CI isn't
run for non-deployable changes (e.g., docs-only or README-only commits);
specifically, add the same paths or paths-ignore patterns used for
'pull_request' to the 'push' trigger to prevent deploy.yml from being invoked on
irrelevant pushes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 79d0f392-dbf6-442e-83d0-43caf00c7bb5
📒 Files selected for processing (6)
.github/workflows/actions.yml.github/workflows/deploy-script/deploy.sh.github/workflows/deploy.ymlREADME.mddocs/Deployment.mddocs/README.md
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/deploy.yml (1)
21-21: Empty environment fallback may cause unexpected behavior.If
head_branchis neithermainnordevelop, the environment resolves to an empty string''. This could lead to confusing errors or unintended behavior if the workflow runs for an unexpected branch.Consider failing explicitly or using a more defensive approach:
environment: ${{ github.event.workflow_run.head_branch == 'main' && 'production' || github.event.workflow_run.head_branch == 'develop' && 'staging' }}However, since the
branches:filter on line 6 restricts triggers tomainanddeveloponly, this scenario should not occur in practice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy.yml at line 21, The ternary-like expression for environment currently falls back to an empty string (''), which can cause unexpected behavior if head_branch is neither 'main' nor 'develop'; update the expression used to compute environment (the environment: line) to remove the empty-string fallback and instead only evaluate to 'production' or 'staging' (e.g., environment: ${{ github.event.workflow_run.head_branch == 'main' && 'production' || github.event.workflow_run.head_branch == 'develop' && 'staging' }}) or alternatively add an explicit guard that errors or fails when head_branch is unexpected so the workflow cannot proceed with a blank environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/Deployment.md`:
- Around line 77-82: Docs currently instruct creating
/opt/boost-data-collector/.env before first deploy which conflicts with
deploy.sh's git clone (clone fails if target dir exists and is non-empty);
either update Deployment.md to instruct creating the .env after the initial
clone (two-step: run deploy.sh to clone, then create
/opt/boost-data-collector/.env with chmod 600) or modify deploy.sh to handle
pre-existing directories by checking for an existing git repo (e.g., if .git
exists do git pull, otherwise clone into a temp dir and move or remove non-repo
files before cloning); reference the .env path and deploy.sh in your change so
reviewers can find the exact places to update.
In `@README.md`:
- Line 188: Update the README line that lists secrets to match Deployment.md and
the workflow: replace the references to PROD_SSH_* / DEV_SSH_* with the
environment-scoped secret names used by the workflow (e.g. SSH_HOST, SSH_USER,
SSH_KEY, etc.) and note that secrets are set per-environment rather than as
PROD_/DEV_-prefixed repository secrets; ensure any example or bullet that
previously referenced PROD_SSH_* / DEV_SSH_* now references the
environment-scoped names (SSH_HOST, SSH_USER) and clarifies they live in the
environment settings.
---
Nitpick comments:
In @.github/workflows/deploy.yml:
- Line 21: The ternary-like expression for environment currently falls back to
an empty string (''), which can cause unexpected behavior if head_branch is
neither 'main' nor 'develop'; update the expression used to compute environment
(the environment: line) to remove the empty-string fallback and instead only
evaluate to 'production' or 'staging' (e.g., environment: ${{
github.event.workflow_run.head_branch == 'main' && 'production' ||
github.event.workflow_run.head_branch == 'develop' && 'staging' }}) or
alternatively add an explicit guard that errors or fails when head_branch is
unexpected so the workflow cannot proceed with a blank environment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1a439273-2b1a-46db-95c5-e14fef1c50f0
📒 Files selected for processing (6)
.github/workflows/actions.yml.github/workflows/deploy-script/deploy.sh.github/workflows/deploy.ymlREADME.mddocs/Deployment.mddocs/README.md
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
docs/Deployment.md (1)
7-15: Minor: Consider adding a language identifier to the fenced code block.The ASCII diagram could use
textorplaintextas a language identifier to satisfy linting rules, though this is purely cosmetic.📝 Optional fix
-``` +```text Push to main/develop ↓🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/Deployment.md` around lines 7 - 15, The fenced ASCII diagram lacks a language identifier which triggers linting; update the triple-backtick fence for the diagram (the block containing "Push to main/develop ↓ CI workflow (lint + tests) ↓ on success Deploy workflow ↓ SSH into server → pull latest code → restart containers") to include a simple identifier such as text or plaintext (e.g., change ``` to ```text) so the markdown linter is satisfied while keeping the diagram content unchanged..github/workflows/deploy-script/deploy.sh (2)
21-22: Consider adding a Docker prerequisite check.The script validates
gitandmakebut notdocker(ordocker compose), which the Makefile targets likely require. Adding a check would provide clearer error messaging if Docker is missing.💡 Optional: Add Docker check
command -v git >/dev/null 2>&1 || { log "ERROR: git is not installed."; exit 1; } command -v make >/dev/null 2>&1 || { log "ERROR: make is not installed."; exit 1; } +command -v docker >/dev/null 2>&1 || { log "ERROR: docker is not installed."; exit 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-script/deploy.sh around lines 21 - 22, The script currently checks for git and make using "command -v git" and "command -v make" but misses verifying Docker, causing unclear failures when Makefile targets require it; add checks that verify docker is installed (e.g., "command -v docker") and optionally validate docker compose availability (either "command -v docker-compose" or "docker compose version" depending on your compose usage) and use the same log/exit pattern (log "ERROR: docker is not installed." and exit 1) to provide a clear error message; place these checks alongside the existing command -v git/make checks in the deploy.sh initialization section.
24-28:git checkoutmay fail if the local branch doesn't exist yet.When the repo exists but the branch was never checked out locally,
git checkout "$BRANCH"will fail before reachingreset --hard. Consider usingcheckout -Borswitch -Cto create/reset the branch in one step.♻️ Proposed fix
if [[ -d "$DEPLOY_DIR/.git" ]]; then log "Pulling latest in $DEPLOY_DIR..." git -C "$DEPLOY_DIR" fetch origin - git -C "$DEPLOY_DIR" checkout "$BRANCH" - git -C "$DEPLOY_DIR" reset --hard "origin/$BRANCH" + git -C "$DEPLOY_DIR" checkout -B "$BRANCH" "origin/$BRANCH" else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-script/deploy.sh around lines 24 - 28, The git checkout can fail if the branch doesn't exist locally; update the deploy logic that operates on $DEPLOY_DIR and $BRANCH to create-or-reset the branch in one step (e.g., replace the failing git -C "$DEPLOY_DIR" checkout "$BRANCH" with a create-or-reset variant such as checkout -B "$BRANCH" or switch -C "$BRANCH") so the subsequent git -C "$DEPLOY_DIR" reset --hard "origin/$BRANCH" always runs against a valid local branch..github/workflows/deploy.yml (1)
41-48: Consider validating the downloaded script before execution.The script is downloaded via
curland immediately executed withbash. While-fensures curl fails on HTTP errors, there's no integrity check (e.g., checksum verification) to guard against a corrupted or tampered download.For sensitive deployment scripts, consider adding a checksum verification step or pinning to a specific commit SHA in
SCRIPT_URL(which the default already does viahead_sha).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy.yml around lines 41 - 48, The workflow currently downloads "$SCRIPT_URL" to /tmp/deploy.sh with curl and immediately runs it via bash (using DEPLOY_SCRIPT_TOKEN for auth if present); add an integrity check before executing by either pinning SCRIPT_URL to a specific commit SHA or by verifying a cryptographic checksum/signature: fetch the trusted checksum or signature alongside /tmp/deploy.sh (or embed known SHA256), compute/verify it (e.g., using sha256sum -c or gpg --verify) and only run bash /tmp/deploy.sh if the verification passes, otherwise fail the job and remove /tmp/deploy.sh.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/deploy-script/deploy.sh:
- Around line 21-22: The script currently checks for git and make using "command
-v git" and "command -v make" but misses verifying Docker, causing unclear
failures when Makefile targets require it; add checks that verify docker is
installed (e.g., "command -v docker") and optionally validate docker compose
availability (either "command -v docker-compose" or "docker compose version"
depending on your compose usage) and use the same log/exit pattern (log "ERROR:
docker is not installed." and exit 1) to provide a clear error message; place
these checks alongside the existing command -v git/make checks in the deploy.sh
initialization section.
- Around line 24-28: The git checkout can fail if the branch doesn't exist
locally; update the deploy logic that operates on $DEPLOY_DIR and $BRANCH to
create-or-reset the branch in one step (e.g., replace the failing git -C
"$DEPLOY_DIR" checkout "$BRANCH" with a create-or-reset variant such as checkout
-B "$BRANCH" or switch -C "$BRANCH") so the subsequent git -C "$DEPLOY_DIR"
reset --hard "origin/$BRANCH" always runs against a valid local branch.
In @.github/workflows/deploy.yml:
- Around line 41-48: The workflow currently downloads "$SCRIPT_URL" to
/tmp/deploy.sh with curl and immediately runs it via bash (using
DEPLOY_SCRIPT_TOKEN for auth if present); add an integrity check before
executing by either pinning SCRIPT_URL to a specific commit SHA or by verifying
a cryptographic checksum/signature: fetch the trusted checksum or signature
alongside /tmp/deploy.sh (or embed known SHA256), compute/verify it (e.g., using
sha256sum -c or gpg --verify) and only run bash /tmp/deploy.sh if the
verification passes, otherwise fail the job and remove /tmp/deploy.sh.
In `@docs/Deployment.md`:
- Around line 7-15: The fenced ASCII diagram lacks a language identifier which
triggers linting; update the triple-backtick fence for the diagram (the block
containing "Push to main/develop ↓ CI workflow (lint + tests) ↓ on success
Deploy workflow ↓ SSH into server → pull latest code → restart containers") to
include a simple identifier such as text or plaintext (e.g., change ``` to
```text) so the markdown linter is satisfied while keeping the diagram content
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7618f80f-cf8e-4ddc-a4c1-26ccc7b0fb05
📒 Files selected for processing (6)
.github/workflows/actions.yml.github/workflows/deploy-script/deploy.sh.github/workflows/deploy.ymlREADME.mddocs/Deployment.mddocs/README.md
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/Deployment.md (1)
83-87:⚠️ Potential issue | 🟠 MajorAvoid root-owned
.envunless the deploy SSH user is root.Using
sudohere can create a600file unreadable by the deploy user running the workflow on subsequent deploys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/Deployment.md` around lines 83 - 87, The docs show using "sudo nano /opt/boost-data-collector/.env" and "sudo chmod 600 /opt/boost-data-collector/.env", which can make the .env owned by root and unreadable by the deploy SSH user; update the Deployment.md instructions to avoid creating a root-owned .env by either editing/creating the file as the deploy user (remove sudo) or, if elevated permissions are required, ensure you chown the file back to the deploy user and set appropriate permissions (e.g., use chown <deploy-user> /opt/boost-data-collector/.env then chmod 600) and mention the deploy SSH user explicitly so subsequent workflows can read the file.
🤖 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/deploy.yml:
- Around line 41-50: Add strict error handling and safe tempfile usage before
downloading and executing the remote script: enable shell failsafe via set -euo
pipefail, replace the hard-coded /tmp/deploy.sh with a secure mktemp-created
filename, install a trap to rm the temp file on EXIT, and ensure the downloaded
file is only executed if curl succeeds (or rely on set -e) and is
present/executable before running bash; keep references to DEPLOY_SCRIPT_TOKEN,
SCRIPT_URL, curl, bash, and the exit_code capture logic and preserve removing
the temp file in the trap rather than after execution.
- Line 31: Replace the mutable tag in the GitHub Actions usage line by pinning
appleboy/ssh-action to its full-length commit SHA: locate the line containing
"uses: appleboy/ssh-action@v1.2.5" and update the reference to the exact commit
SHA (from the appleboy/ssh-action repo) so it reads "uses:
appleboy/ssh-action@<full-commit-sha>"; ensure you use the full 40-character
commit hash for immutability and reproducible deployments.
In `@docs/Deployment.md`:
- Line 75: Update the incorrect relative link in docs/Deployment.md so it points
to the actual deploy script location (.github/workflows/deploy-script/deploy.sh)
instead of docs/.github/..., by editing the markdown link target for deploy.sh
to the correct relative path; ensure the link text still references deploy.sh
and verify it resolves to the repository root path where
.github/workflows/deploy-script/deploy.sh lives.
In `@README.md`:
- Line 188: Update the README line that lists the required environment secrets
so SSH_PORT is marked optional: change the wording that currently lists
`SSH_HOST`, `SSH_USER`, `SSH_PRIVATE_KEY`, `SSH_PORT` as all required to
indicate `SSH_PORT` is optional (defaults to 22 in the workflow), while keeping
`SSH_HOST`, `SSH_USER`, and `SSH_PRIVATE_KEY` as required; reference the exact
secret names `SSH_PORT`, `SSH_HOST`, `SSH_USER`, and `SSH_PRIVATE_KEY` when
editing.
---
Duplicate comments:
In `@docs/Deployment.md`:
- Around line 83-87: The docs show using "sudo nano
/opt/boost-data-collector/.env" and "sudo chmod 600
/opt/boost-data-collector/.env", which can make the .env owned by root and
unreadable by the deploy SSH user; update the Deployment.md instructions to
avoid creating a root-owned .env by either editing/creating the file as the
deploy user (remove sudo) or, if elevated permissions are required, ensure you
chown the file back to the deploy user and set appropriate permissions (e.g.,
use chown <deploy-user> /opt/boost-data-collector/.env then chmod 600) and
mention the deploy SSH user explicitly so subsequent workflows can read the
file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bfeb6242-8b45-45d3-ad93-c99129661c37
📒 Files selected for processing (6)
.github/workflows/actions.yml.github/workflows/deploy-script/deploy.sh.github/workflows/deploy.ymlREADME.mddocs/Deployment.mddocs/README.md
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/deploy-script/deploy.sh (1)
14-32:⚠️ Potential issue | 🟠 Major
DEPLOY_DIRbootstrap remains brittle for common non-root SSH users.Line 14 defaults to
/opt/...and Lines 30-32 attempt clone without an explicit writability/non-git-directory precheck. First deploy can fail with permission errors or opaque clone failures when the directory already exists.Suggested hardening
DEPLOY_DIR="${DEPLOY_DIR:-/opt/boost-data-collector}" + +ensure_deploy_dir() { + mkdir -p "$DEPLOY_DIR" || { + log "ERROR: Cannot create DEPLOY_DIR: $DEPLOY_DIR" + exit 1 + } + + if [[ ! -w "$DEPLOY_DIR" ]]; then + log "ERROR: DEPLOY_DIR is not writable by deploy user: $DEPLOY_DIR" + log " Fix ownership/permissions or set DEPLOY_DIR to a writable path." + exit 1 + fi + + if [[ ! -d "$DEPLOY_DIR/.git" ]] && find "$DEPLOY_DIR" -mindepth 1 -maxdepth 1 ! -name '.env' -print -quit | grep -q .; then + log "ERROR: $DEPLOY_DIR exists, is not a git repo, and contains unexpected files." + exit 1 + fi +} if [[ -z "${REPO_URL:-}" || -z "${BRANCH:-}" ]]; then log "ERROR: REPO_URL and BRANCH must be set." exit 1 fi @@ +ensure_deploy_dir if [[ -d "$DEPLOY_DIR/.git" ]]; then log "Pulling latest in $DEPLOY_DIR..." git -C "$DEPLOY_DIR" fetch origin @@ else log "Cloning $REPO_URL (branch: $BRANCH) into $DEPLOY_DIR..." - mkdir -p "$(dirname "$DEPLOY_DIR")" git clone --branch "$BRANCH" "$REPO_URL" "$DEPLOY_DIR" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-script/deploy.sh around lines 14 - 32, The script currently defaults DEPLOY_DIR to /opt/... and blindly tries to mkdir/clone which breaks for non-root SSH users; change the DEPLOY_DIR default to a per-user location (e.g., ${DEPLOY_DIR:-"$HOME/.local/boost-data-collector"}) and add prechecks before cloning: verify parent directory is writable (test -w "$(dirname "$DEPLOY_DIR")" or attempt to create a temp file), and if the path exists but is not a git repo (the if [[ -d "$DEPLOY_DIR/.git" ]] check), fail with a clear error instructing the user to remove or fix the directory (or optionally clone into a tmp dir and move), and ensure mkdir -p "$(dirname "$DEPLOY_DIR")" only runs after writability is confirmed so git clone has permission to create the target.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/Deployment.md`:
- Around line 7-15: The fenced flow block in Deployment.md is missing a language
tag (MD040); update the opening fence for the code block containing "Push to
main/develop ... SSH into server → pull latest code → restart containers" to
include a language identifier (e.g., change the opening "```" to "```text") so
the block is tagged as plain text and satisfies the linter.
---
Duplicate comments:
In @.github/workflows/deploy-script/deploy.sh:
- Around line 14-32: The script currently defaults DEPLOY_DIR to /opt/... and
blindly tries to mkdir/clone which breaks for non-root SSH users; change the
DEPLOY_DIR default to a per-user location (e.g.,
${DEPLOY_DIR:-"$HOME/.local/boost-data-collector"}) and add prechecks before
cloning: verify parent directory is writable (test -w "$(dirname "$DEPLOY_DIR")"
or attempt to create a temp file), and if the path exists but is not a git repo
(the if [[ -d "$DEPLOY_DIR/.git" ]] check), fail with a clear error instructing
the user to remove or fix the directory (or optionally clone into a tmp dir and
move), and ensure mkdir -p "$(dirname "$DEPLOY_DIR")" only runs after
writability is confirmed so git clone has permission to create the target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aa6912f8-1ad7-4870-be44-f8852bc2b262
📒 Files selected for processing (6)
.github/workflows/actions.yml.github/workflows/deploy-script/deploy.sh.github/workflows/deploy.ymlREADME.mddocs/Deployment.mddocs/README.md
jonathanMLDev
approved these changes
Mar 19, 2026
jonathanMLDev
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
deploy.ymlanddeploy.shfile.actions.yml.Summary by CodeRabbit
New Features
Documentation
Chores