Skip to content

infra: rent server using scaleway#599

Open
JuArce wants to merge 22 commits into
mainfrom
infra_rent_server
Open

infra: rent server using scaleway#599
JuArce wants to merge 22 commits into
mainfrom
infra_rent_server

Conversation

@JuArce
Copy link
Copy Markdown
Contributor

@JuArce JuArce commented May 20, 2026

No description provided.

@JuArce JuArce self-assigned this May 20, 2026
@github-actions
Copy link
Copy Markdown

Codex Code Review

Found security issues in the PR diff:

  • High: first-seen SSH host keys are trusted before sending secrets and root commands
    infra/provision_server.sh uses StrictHostKeyChecking=accept-new, and infra/rent_baremetal.sh deletes any prior host key before handing off. The next step scps the GitHub deploy private key and runs provisioning as root/admin. A MITM on first connection can receive the deploy key and provisioning script. Require an expected host key/fingerprint from Scaleway or an operator prompt before copying secrets/executing commands.

  • High: root extracts an unauthenticated tarball into /opt
    infra/provision.sh downloads lambda-vm-sysroot-rv64im.tar.gz and line 119 extracts it as root without checksum/signature verification or path validation. A compromised artifact can overwrite arbitrary paths via absolute paths or ../ entries. Pin and verify a checksum/signature, and extract after validating archive paths.

  • Medium: GitHub host key is accepted via unauthenticated ssh-keyscan
    infra/provision.sh appends whatever key the network returns for github.com, then line 162 clones with the deploy key. A MITM can poison known_hosts and serve a malicious repository. Pin GitHub’s published SSH host keys instead of trusting ssh-keyscan output directly.

  • Low: deploy private key can be left behind in /tmp on re-provision
    infra/provision_server.sh always uploads the local deploy key when present, but infra/provision.sh only removes /tmp/lambda_vm_read_only_key when the target app key does not already exist. Re-running provisioning after the key is installed leaves an extra private key staged in /tmp. Always remove the staged key after handling it, ideally with a trap, and use a unique root-owned temp path.

@claude
Copy link
Copy Markdown

claude Bot commented May 20, 2026

Code Review – infra: rent server using Scaleway

This PR adds shell scripts to rent, provision, and configure a Scaleway Elastic Metal server. The overall structure is solid: idempotent design, proper set -euo pipefail, safety checks before hardening sshd, and good documentation. A few security issues worth addressing before merging.


Security

High – MITM on first connection exposes deploy key
provision_server.sh uses -o StrictHostKeyChecking=accept-new for all SSH/SCP calls, including the one that copies the GitHub deploy key to the server. If an attacker intercepts the initial connection (spoofed IP, ARP poisoning, DNS hijack), they receive the deploy key. Consider doing a manual ssh-keyscan from a trusted location and pinning the fingerprint before the scp step, or at minimum move the deploy key transfer to a second connection that happens after the host key is established.

Medium – Deploy key lands in world-readable /tmp with a race window
scp places the key at /tmp/lambda_vm_read_only_key (mode 644 at creation time); a separate ssh … chmod 600 call tightens it afterwards. Any process on the server between those two commands can read the key. Prefer piping the key over the existing SSH session via a heredoc into install -m 0600 -o app, bypassing the /tmp staging entirely.

Medium – No integrity check on downloaded binaries
The sysroot tarball and ethrex_hoodi.bin are fetched with plain curl -L and extracted/used immediately without SHA-256 verification. A compromised CDN or intercepted connection silently delivers a malicious binary to every provisioned server. Add expected checksums and verify with sha256sum -c before use.

Medium – curl-pipe-bash for Rust and Claude Code installers
Both use curl … | sh / curl … | bash. This is standard practice for these tools but means a compromised upstream executes arbitrary code. Consider downloading first and verifying against a pinned hash before executing.

Low – Hardcoded Scaleway project UUID in a public repo
SCW_PROJECT_ID is hardcoded as a default in rent_baremetal.sh. The project ID is now publicly visible. While not secret on its own, it can assist enumeration or targeted API calls. Require it to be set via env var with no default, or move it to a local config file.


Bugs

  • Section numbering in provision.sh skips step 5 (goes 4 → 6). Minor but confusing.
  • apt-get upgrade is commented out (line 23). The server is provisioned with whatever package versions shipped with the Debian 12 image, including unpatched CVEs. Uncomment it or add a note explaining why it is intentionally skipped.
  • ROOT_DIR is set in rent_baremetal.sh but never used. Dead variable.

Comment thread infra/provision_server.sh
Comment thread infra/provision_server.sh Outdated
Comment thread infra/provision.sh
Comment thread infra/provision.sh Outdated
Comment thread infra/rent_baremetal.sh
@JuArce JuArce marked this pull request as ready for review May 20, 2026 20:27
@github-actions
Copy link
Copy Markdown

Codex Code Review

Findings

  • High - Hardcoded SSH keys get passwordless root-equivalent access: infra/provision.sh grants admin NOPASSWD:ALL, then infra/provision.sh hardcodes team keys and copies them into both admin and app authorized keys. Anyone holding one of those private keys gets sudo-root on every provisioned machine. Prefer deriving admin access from the Scaleway project keys or an explicit operator-provided key file, not a committed static access list.

  • Medium - GitHub host key is trusted from the network at provisioning time: infra/provision.sh uses ssh-keyscan github.com directly before cloning. A MITM during first provisioning can poison known_hosts and make the subsequent clone pull attacker-controlled code. Pin GitHub’s published host keys or ship a checked-in known_hosts entry instead of trusting live keyscan output.

No Rust/VM-specific issues in this diff; the PR only adds infra shell scripts and docs.

Comment thread infra/provision.sh
Comment thread infra/provision.sh
@claude
Copy link
Copy Markdown

claude Bot commented May 20, 2026

Review: infra/rent+provision scripts

Overall the scripts are well-structured, idempotent, and handle the important edge cases (IP recycling, key cleanup, profile guard). Two real issues and one consistency note, all low severity.

Issues

[Low] LC_ALL=C defeats UTF-8 locale setupprovision.sh:188-191
LC_ALL takes runtime precedence over every other LC_* variable, so LC_CTYPE=en_US.UTF-8 is a no-op. Any program that checks LC_ALL first sees C (ASCII). Drop LC_ALL and rely on LANG=en_US.UTF-8 as the fallback. (Inline suggestion posted.)

[Low] LC_TYPE typoprovision.sh:190
LC_TYPE is not a POSIX locale category; the correct name is LC_CTYPE (already on the next line). The spurious variable is silently ignored. (Covered by same inline suggestion.)

[Low] Inconsistent TLS hardening across curl | bash installsprovision.sh:106
The rustup install uses --proto '=https' --tlsv1.2; the Claude Code install uses plain -fsSL without those flags. Both are curl-to-bash and should be consistent. (Inline suggestion posted.)

No issues found

  • Deploy key staging (/tmp/keydir) is safe: fresh server, transferred atomically via stdin, cleaned up unconditionally.
  • StrictHostKeyChecking=accept-new is appropriate for automated provisioning and already documented in the README.
  • SSH hardening config (99-hardening.conf) looks solid.
  • Profile guard preventing accidental non-vm profile runs is a nice safety net.

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