Skip to content

Scope deploy fetch to main#11

Merged
BeforeLights merged 2 commits into
devfrom
fix/deploy-verified-sha
May 7, 2026
Merged

Scope deploy fetch to main#11
BeforeLights merged 2 commits into
devfrom
fix/deploy-verified-sha

Conversation

@BeforeLights
Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6774d2f0-9ac8-4cc7-9f35-d30282d2961f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/deploy-verified-sha

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Verify commit reachability before deploying to Lightsail

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Scope git fetch to main branch only for deployment verification
• Add validation to ensure deployed commit is reachable from main
• Prevent deployment of unverified or unreachable commits
• Improve error messaging for deployment failures
Diagram
flowchart LR
  A["Deploy Workflow"] -->|"Fetch main branch"| B["git fetch origin main"]
  B -->|"Verify commit exists"| C["git cat-file check"]
  C -->|"Success"| D["Deploy code"]
  C -->|"Failure"| E["Exit with error message"]
Loading

Grey Divider

File Changes

1. .github/workflows/deploy-lightsail.yml 🐞 Bug fix +5/-2

Add commit verification before Lightsail deployment

• Changed git fetch to scope only main branch with +refs/heads/main:refs/remotes/origin/main
• Added conditional check to verify commit is reachable after fetch
• Added descriptive error message when commit verification fails
• Exit deployment with status code 1 on verification failure

.github/workflows/deploy-lightsail.yml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented May 7, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Non-main SHA can deploy ✓ Resolved 🐞 Bug ⛨ Security
Description
The deploy step claims to verify COMMIT_SHA is “reachable” from origin/main, but it only runs
git cat-file -e, which merely checks whether the commit object exists locally. Because the
workflow runs in a long-lived server repo (cd "$APP_DIR") and does not re-clone, an old/stale
commit object can remain present and pass this check even if it is not in origin/main, allowing
deployment of unverified code.
Code

.github/workflows/deploy-lightsail.yml[R75-79]

+            git fetch --prune origin +refs/heads/main:refs/remotes/origin/main
+            if ! git cat-file -e "$COMMIT_SHA^{commit}"; then
+              echo "Commit $COMMIT_SHA is not reachable after fetching origin/main; refusing to deploy unverified code. Re-run the workflow for the current main commit." >&2
+              exit 1
+            fi
Evidence
The deploy job runs over SSH inside an existing repository directory on the server and does not
perform a fresh clone, so prior git objects can persist across runs. The new guard uses `git
cat-file -e (object-exists check) rather than an ancestry test against origin/main`, so it does
not actually enforce “commit is on main” in the presence of previously-fetched objects.

.github/workflows/deploy-lightsail.yml[59-80]
.github/workflows/deploy-lightsail.yml[71-80]
Best Practice: Git documentation (cat-file, merge-base)

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The deploy workflow intends to refuse deploying a SHA that is not on `origin/main`, but it currently uses `git cat-file -e` which only checks local object existence (and can be satisfied by stale objects in a persistent server checkout).
## Issue Context
This deploy runs over SSH in an existing directory (`$APP_DIR`) and does not re-clone, so the `.git` object store can contain commits from previous fetches even if they are no longer in `origin/main`.
## Fix Focus Areas
- .github/workflows/deploy-lightsail.yml[75-80]
### Suggested change
After fetching `origin/main`, replace the `cat-file` check with an ancestry check:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Stale deploy allowed 🐞 Bug ⛨ Security
Description
The deploy gate uses git merge-base --is-ancestor $COMMIT_SHA origin/main, which passes for any
historical commit still reachable from main, so re-running an older workflow run can deploy outdated
code even though the message says to re-run for the current main commit.
Code

.github/workflows/deploy-lightsail.yml[R75-80]

+            git fetch --prune origin +refs/heads/main:refs/remotes/origin/main
+            if ! git merge-base --is-ancestor "$COMMIT_SHA" origin/main; then
+              echo "Commit $COMMIT_SHA is not on origin/main after fetch; refusing to deploy unverified code. Re-run the workflow for the current main commit." >&2
+              exit 1
+            fi
           git reset --hard "$COMMIT_SHA"
Evidence
COMMIT_SHA is taken from the workflow run (github.sha) and the deploy step only checks it is an
ancestor of origin/main before resetting to it. An ancestor check does not enforce equality with
the current tip of origin/main, so older SHAs on main remain deployable.

.github/workflows/deploy-lightsail.yml[61-66]
.github/workflows/deploy-lightsail.yml[75-81]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The deploy job currently verifies `COMMIT_SHA` only as an ancestor of `origin/main`, which still allows deploying older main commits (e.g., by re-running an old workflow run). This contradicts the error message that implies only the current main commit is allowed.
### Issue Context
`COMMIT_SHA` is set from `${{ github.sha }}` and then the remote-tracking `origin/main` is fetched and used for verification.
### Fix Focus Areas
- .github/workflows/deploy-lightsail.yml[61-81]
### Suggested change
After fetching `origin/main`, compute `MAIN_SHA=$(git rev-parse origin/main)` and require `COMMIT_SHA` to equal `MAIN_SHA` (or alternatively, ignore `COMMIT_SHA` and reset directly to `origin/main`). If you intentionally want to allow deploying older main commits, update the error message to reflect that policy (and consider adding an explicit allowlist mechanism).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread .github/workflows/deploy-lightsail.yml
@BeforeLights
Copy link
Copy Markdown
Contributor Author

/review -i

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented May 7, 2026

Persistent review updated to latest commit 81ba70a

@BeforeLights BeforeLights merged commit 738e8d1 into dev May 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant