Skip to content

feat: add gcp deploy in orion worker, remove dev env scripts#1964

Merged
genedna merged 1 commit into
gitmono-dev:mainfrom
benjamin-747:main
Feb 26, 2026
Merged

feat: add gcp deploy in orion worker, remove dev env scripts#1964
genedna merged 1 commit into
gitmono-dev:mainfrom
benjamin-747:main

Conversation

@benjamin-747

Copy link
Copy Markdown
Collaborator

No description provided.

@benjamin-747 benjamin-747 requested a review from genedna February 26, 2026 07:35
@github-actions

This comment was marked as outdated.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c9dd619bb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .github/workflows/orion-client-deploy.yml
@github-actions

github-actions Bot commented Feb 26, 2026

Copy link
Copy Markdown

Claude finished @benjamin-747's task in 1m 19s —— View job


Code Review: feat: add gcp deploy in orion worker, remove dev env scripts

Tasks:

  • Gather context and analyze PR changes
  • Review workflow file changes
  • Review deleted files impact
  • Provide comprehensive feedback

Summary

This PR makes two distinct changes:

  1. Workflow refactor – Replaces SSH-based deploy of orion-client-deploy.yml with rsync + ssh-action steps targeting both the legacy orion VM and a new GCP VM.
  2. Dev-environment cleanup – Deletes all scripts under scripts/dev/, docs/build.md, plus their supporting nginx config and compose files (~1500 lines).

Overall the intention is good, but there are several issues worth addressing before merging.


Security Issues

[HIGH] Deploy job runs on workflow_dispatch from any branch

.github/workflows/orion-client-deploy.yml:64

deploy:
  if: ${{ github.repository == 'web3infra-foundation/mega' }}

The build job has no if guard at all, and the deploy job only checks the repository name. Since the workflow has an unrestricted workflow_dispatch trigger (line 14), any maintainer can manually dispatch this workflow from an arbitrary branch and push untested binaries to both production VMs. The previous condition (github.ref == 'refs/heads/main') protected against this.

Recommended fix:

deploy:
  if: >
    github.repository == 'web3infra-foundation/mega' &&
    (github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch')

Or more strictly, only allow production deploys from main:

deploy:
  if: >
    github.repository == 'web3infra-foundation/mega' &&
    github.ref == 'refs/heads/main'

Fix this →

[MEDIUM] SSH deploy to legacy VM uses root user

.github/workflows/orion-client-deploy.yml:91-92 and 97-98

The legacy orion_vm deployment still uses remote_user: root and username: root. While there is a comment acknowledging this technical debt (lines 76–83), deploying to root over SSH in a CI/CD pipeline is a significant security concern. An infrastructure migration to a least-privilege user should be tracked as a follow-up issue if not done now.

[LOW] No SSH host key verification for rsync step

The burnett01/rsync-deployments action does not verify host keys by default. This opens a potential for a man-in-the-middle attack during binary transfer. Consider adding known host verification or using rsync_options that include StrictHostKeyChecking=yes with a pre-populated known_hosts.


Code Quality Issues

Concurrency change: cancel-in-progress: true

.github/workflows/orion-client-deploy.yml:18

The concurrency setting was changed from cancel-in-progress: false to true. This means a new push to main can cancel an in-progress deploy mid-flight. If the rsync upload to orion_vm completes but the restart step is then cancelled, the service could be left in an inconsistent state with mismatched binaries. Consider keeping this false for deploy workflows or adding rollback logic.

Deploy resilience: two VMs deploy sequentially with no failure isolation

.github/workflows/orion-client-deploy.yml:84-122

The four deploy steps (upload + restart for each VM) run sequentially. If the orion_vm restart fails (line 101), the GCP VM will never be updated — or vice versa. The previous approach used a matrix strategy with fail-fast: false which provided partial fault isolation. Consider structuring the two VM deploys as separate jobs with needs: build and continue-on-error: true, or restoring the matrix pattern.

Minor: Typo in step name

.github/workflows/orion-client-deploy.yml:93

- name: Restart service  to orion_vm

There is a double space in the step name. This is cosmetic but worth fixing.

build job has no repository guard

Previously the build job had:

if: ${{ github.repository == 'web3infra-foundation/mega' || github.event_name == 'workflow_dispatch' }}

The new version has no if at all, which means the build job will also run on forks. While this is less critical than the deploy guard, it wastes CI minutes on external fork builds.


Deleted Dev Scripts — Impact Assessment

The deletion of scripts/dev/ (391-line Docker setup, docker-compose, .env.example, nginx config, README) is a significant removal of onboarding infrastructure for local development. A few concerns:

  • docs/build.md was the primary build documentation and is now deleted. docs/development.md exists but may not fully cover what build.md provided. Verify that docs/development.md (or another doc) now covers all previously documented setup steps.
  • The scripts/dev/README.md was 158 lines of contributor-facing setup instructions. Contributors who relied on it will have no replacement guidance.
  • If there is a planned replacement (e.g., a devcontainer, Nix flake, or Docker-based alternative), it should be referenced in the PR description or a README update.

Testing / Validation

  • Have both deploys (orion_vm and gcp_vm) been manually verified to succeed end-to-end?
  • Is orion-runner.service the correct service name for both VMs? The previous workflow restarted orion-worker.service.
  • Are ORION_GCP_VM_HOST and ORION_GCP_VM_SSH_KEY secrets already present in the repository?

Verdict

The GCP deploy addition is directionally correct. However, the security regression in the deploy guard (confirmed by Codex review above), the cancel-in-progress: true risk for a deploy job, and the removal of extensive dev documentation without a clear replacement are issues that should be addressed before merging.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: adf400b652

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .github/workflows/orion-client-deploy.yml
@genedna genedna merged commit e62844b into gitmono-dev:main Feb 26, 2026
11 checks 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.

2 participants