fix: handle ssh-copy-id failure and support non-root initial accounts by edouard-mangel · Pull Request #3 · mariusdjen/vpskit

edouard-mangel · 2026-03-28T12:46:02Z

Problems fixed

1. Silent crash when ssh-copy-id fails

When the password is wrong or password auth is disabled on the server, ssh-copy-id exits non-zero. With set -euo pipefail, this aborts the script silently — the only output is tmp_script: unbound variable from the EXIT trap in vpskit.sh.

2. `tmp_script: unbound variable` in EXIT trap

In vpskit.sh, the EXIT trap inside run_script() referenced the local variable $tmp_script bare. When a child script exits via set -e, the local var may be out of scope when the trap fires. Fixed with ${tmp_script:-}.

3. Hardcoded `root` account

The new-VPS flow assumed root as the initial admin account everywhere. Some providers use ubuntu, admin, ec2-user, etc. Added a prompt after the IP step (defaults to root) so the correct account is used throughout.

Changes

setup.sh: wrap ssh-copy-id and manual pipe fallback in if ! with clear error + retry command; add initial user prompt; replace all root@$VPS_IP with $INITIAL_USER@$VPS_IP
vpskit.sh: use ${tmp_script:-} in EXIT trap
lang/en.sh + lang/fr.sh: add MSG_SETUP_NEW_STEP3_COPY_FAILED, MSG_SETUP_NEW_STEP3_COPY_FAILED_HINT, MSG_SETUP_NEW_INITIAL_USER_PROMPT; update step 3 desc to say "server password" instead of "root password"

Copilot

Pull request overview

This PR improves the interactive “new VPS” setup flow by (1) making SSH key installation failures non-silent/diagnosable and (2) allowing the initial SSH login account to be something other than root, with corresponding i18n updates.

Changes:

Add an “initial admin account” prompt for new VPS setup and use it for ssh-copy-id and connection tests.
Handle ssh-copy-id/manual key install failures with a clear error + retry hint instead of a silent exit.
Harden vpskit.sh cleanup trap to avoid unbound-variable crashes; add new language strings.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

File	Description
`vpskit.sh`	Adjusts EXIT trap cleanup to avoid `tmp_script` unbound variable crashes.
`setup.sh`	Prompts for initial SSH user; improves error handling around SSH key installation; uses initial user for connection test and SSH session selection.
`lang/en.sh`	Adds/updates English messages for initial user prompt and key-copy failure handling.
`lang/fr.sh`	Adds/updates French messages for initial user prompt and key-copy failure handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T12:50:12Z


-    SSH_USER="root"
+    SSH_USER="$INITIAL_USER"
    USE_SUDO=false


In MODE=new, SSH_USER is set to $INITIAL_USER but USE_SUDO is always set to false. If the initial account is a non-root user (e.g. ubuntu/ec2-user), the remote bootstrap script is executed with plain bash (no sudo) even though it writes to /root and runs privileged operations (apt/dnf, user creation, root disable), so the setup will fail. Consider setting USE_SUDO=true when INITIAL_USER is not root (or probing id -u / sudo availability) and keep USE_SUDO=false only for root logins.

Suggested change

USE_SUDO=false

if [ "$INITIAL_USER" = "root" ]; then

USE_SUDO=false

else

USE_SUDO=true

fi

Copilot · 2026-03-28T12:50:13Z


    tmp_script=$(mktemp)
-    trap 'rm -f "$tmp_script" "${_LANG_TMP:-}"' EXIT
+    trap 'rm -f "${tmp_script:-}" "${_LANG_TMP:-}"' EXIT


This trap ... EXIT inside run_script() overwrites the earlier EXIT trap that cleans up $_LANG_TMP (set when lang.sh is downloaded). Because run_script() later executes trap - EXIT, the original cleanup trap is never restored, and $_LANG_TMP can be left behind on normal runs. Also, when tmp_script/_LANG_TMP are unset, rm -f "${var:-}" may end up being called with an empty-string operand. Consider using a cleanup function that checks for non-empty paths, and restore the previous EXIT trap (or explicitly rm -f "${_LANG_TMP:-}" before clearing/restoring traps).

Copilot · 2026-03-28T12:50:13Z

+    echo ""
+    read -p "  $MSG_SETUP_NEW_INITIAL_USER_PROMPT" INITIAL_USER
+    INITIAL_USER=${INITIAL_USER:-root}
+


INITIAL_USER is prompted for the new-VPS flow but it isn't persisted to $LOCAL_STATE. If the setup is interrupted before the $USERNAME account is created (or before key-based login for $USERNAME works), a later resume will go through MODE=update and only tries ${USERNAME}@${VPS_IP} then root@${VPS_IP}—so VPSes with a non-root initial account and root login disabled can become non-resumable. Consider saving INITIAL_USER in the local state and including it as an additional SSH fallback in the update-mode connection test when $USERNAME is not yet available.

Suggested change

# Persister l'utilisateur initial dans l'état local pour permettre la reprise

if [[ -n "${LOCAL_STATE:-}" ]]; then

mkdir -p "$(dirname "$LOCAL_STATE")"

if [[ -f "$LOCAL_STATE" ]] && grep -q '^INITIAL_USER=' "$LOCAL_STATE"; then

tmp_state_file="$(mktemp)"

grep -v '^INITIAL_USER=' "$LOCAL_STATE" > "$tmp_state_file"

printf 'INITIAL_USER=%q\n' "$INITIAL_USER" >> "$tmp_state_file"

mv "$tmp_state_file" "$LOCAL_STATE"

else

printf 'INITIAL_USER=%q\n' "$INITIAL_USER" >> "$LOCAL_STATE"

fi

fi

- setup.sh: prompt for initial admin account (default: root) to support providers that use ubuntu, ec2-user, admin, etc. instead of root - setup.sh: wrap ssh-copy-id and manual pipe fallback in error handlers; show clear error message and retry command on failure instead of silent crash via set -e - setup.sh: set USE_SUDO=true when initial account is not root - setup.sh: persist INITIAL_USER in local state; restore on resume; use as third connection fallback in update mode (after username and root) - setup.sh: fix silent crash when reading INITIAL_USER from old state files that predate this field (grep miss + pipefail = silent exit) - security.sh: fall back to root when deploy user cannot connect; keeps USERNAME as the audit target while SSH_USER handles the connection - vpskit.sh: fix unbound variable crash in EXIT trap when a child script exits via set -e (use ${tmp_script:-}); restore _LANG_TMP cleanup trap after run_script() instead of unconditionally clearing it - lang/en.sh + lang/fr.sh: add messages for initial user prompt, key-copy failure hint, and security audit root fallback

On providers like AWS/Ubuntu where the initial account is not root, the remote setup script was creating an empty authorized_keys for the new deploy user because /root/.ssh/authorized_keys doesn't exist. Now falls back to copying from /home/__SSH_USER__/.ssh/authorized_keys (the account used to connect) when root's file is absent or empty. Inject SSH_USER as a placeholder alongside USERNAME.

mariusdjen · 2026-03-28T14:02:04Z

Thanks for this PR this is a real problem I'd been looking at too. The code is clean and well-structured.
A few things before I merge:

Have you been able to test this on a non-root VPS (OVH, AWS, etc.)? I'd like to validate before merging. I'll be testing on my side too.
The new SSH_USER placeholder in the remote script can you confirm it goes through sed_escape() before injection? (security)
The EXIT trap fix in vpskit.sh is a good start but as Copilot noted, there's still an edge case where _LANG_TMP cleanup can be lost. Not a blocker we can improve it after merge.

Great contribution, appreciate it!

edouard-mangel · 2026-03-28T14:21:44Z

Thanks for the review!

Yes, tested on OVH with an ubuntu initial account — the prompt, key copy, and connection test all worked as expected.
__SSH_USER__ does go through sed_escape() before injection. From setup.sh:
```
SAFE_SSH_USER=$(sed_escape "$SSH_USER")
sed -i "s|__SSH_USER__|$SAFE_SSH_USER|g" "$TMPSCRIPT"
```
Same pattern as SAFE_USERNAME / __USERNAME__ already used in the script.
Understood — happy to follow up on the _LANG_TMP trap cleanup in a separate PR.

mariusdjen

LGTM. Code is clean, answers are satisfactory. Will follow up on the EXIT trap cleanup in a separate PR.

Copilot AI review requested due to automatic review settings March 28, 2026 12:46

edouard-mangel requested a review from mariusdjen as a code owner March 28, 2026 12:46

Copilot started reviewing on behalf of edouard-mangel March 28, 2026 12:46 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

edouard-mangel force-pushed the fix/ssh-copy-id-error-handling branch from a3d275e to 0e6d559 Compare March 28, 2026 13:06

mariusdjen approved these changes Mar 28, 2026

View reviewed changes

mariusdjen merged commit b52af94 into mariusdjen:main Mar 28, 2026
1 check passed

mariusdjen mentioned this pull request Mar 28, 2026

Support custom SSH username (ubuntu, root, etc.) for VPS connection #2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: handle ssh-copy-id failure and support non-root initial accounts#3

fix: handle ssh-copy-id failure and support non-root initial accounts#3
mariusdjen merged 2 commits into
mariusdjen:mainfrom
edouard-mangel:fix/ssh-copy-id-error-handling

edouard-mangel commented Mar 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

mariusdjen commented Mar 28, 2026 •

edited

Loading

Uh oh!

edouard-mangel commented Mar 28, 2026

Uh oh!

mariusdjen left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

+    # Persister l'utilisateur initial dans l'état local pour permettre la reprise
+    if [[ -n "${LOCAL_STATE:-}" ]]; then
+        mkdir -p "$(dirname "$LOCAL_STATE")"
+        if [[ -f "$LOCAL_STATE" ]] && grep -q '^INITIAL_USER=' "$LOCAL_STATE"; then
+            tmp_state_file="$(mktemp)"
+            grep -v '^INITIAL_USER=' "$LOCAL_STATE" > "$tmp_state_file"
+            printf 'INITIAL_USER=%q\n' "$INITIAL_USER" >> "$tmp_state_file"
+            mv "$tmp_state_file" "$LOCAL_STATE"
+        else
+            printf 'INITIAL_USER=%q\n' "$INITIAL_USER" >> "$LOCAL_STATE"
+        fi
+    fi

Conversation

edouard-mangel commented Mar 28, 2026

Problems fixed

1. Silent crash when ssh-copy-id fails

2. tmp_script: unbound variable in EXIT trap

3. Hardcoded root account

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

mariusdjen commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edouard-mangel commented Mar 28, 2026

Uh oh!

mariusdjen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

2. `tmp_script: unbound variable` in EXIT trap

3. Hardcoded `root` account

mariusdjen commented Mar 28, 2026 •

edited

Loading