Improve macOS deployment script with proper error handling, logging, and relay config#599
Open
jordaneunson wants to merge 1 commit intorustdesk:masterfrom
Open
Conversation
…and relay config Replace the existing macOS Bash deployment script with a production-tested version that adds: - Timestamped logging to /tmp/rustdesk-install.log - Proper error handling with exit codes and diagnostics - Download validation (link found, file not empty) - Console user detection and user-context execution via launchctl - Config directory initialization wait loop (vs fixed sleep) - Direct relay server TOML configuration (no config string needed) - LaunchAgent fallback for DEP/MDM enrollments with no logged-in user - Server/client mode toggle - Clean DMG cleanup on completion
📝 WalkthroughWalkthroughThe macOS Bash deployment documentation was significantly expanded to detail a complete RustDesk installation and configuration lifecycle. The script now includes configurable relay server parameters, comprehensive logging, DMG validation, console user detection, and fallback LaunchAgent creation for headless deployment scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/self-host/client-deployment/_index.en.md`:
- Around line 334-337: The script currently aborts when CONSOLE_USER is missing
(the block checking CONSOLE_USER and writing to LOG), which prevents the
LaunchAgent fallback for DEP/MDM no-user installs; instead of calling exit 1 in
that conditional, log the error to LOG and set a marker (e.g., export
NO_CONSOLE=true or leave CONSOLE_USER empty) so execution continues into the
LaunchAgent fallback flow; update the conditional around CONSOLE_USER (the block
that echoes to LOG) to not exit, and ensure later code that configures the
RustDesk directory/LaunchAgent checks NO_CONSOLE or an empty CONSOLE_USER to
take the fallback path.
- Around line 246-248: The log file is created world-writable via touch "$LOG"
and chmod 666 "$LOG" (and repeated in the other occurrences), and secrets
(RustDesk password and relay key) are being written to it; change the log
creation to set restrictive permissions (e.g., owner-only 600 or 640) and ensure
the file owner is the intended service account, and stop writing sensitive
values to the log — redact or omit the RustDesk password and relay key from any
logging statements (or store them in a secure vault/secure file with limited
access) and update all instances that use touch "$LOG"/chmod 666 "$LOG" to use
the secure permission pattern and remove secret output.
- Around line 320-321: The script currently assumes the copy succeeded by
running cp -R "$mount_point/RustDesk.app" "/Applications/" &> /dev/null and
immediately echoing a success line to $LOG; change this to check cp's exit
status (or use an if cp ...; then ... else ...) so failures are detected: run cp
without silencing stderr (or redirect stderr to the $LOG), then test the command
result and on success write the timestamped success message to $LOG, otherwise
write a timestamped error with the cp exit code and stderr and exit non‑zero to
stop the installer; update the lines referencing cp -R
"$mount_point/RustDesk.app" "/Applications/" and the subsequent echo to
implement this check.
- Around line 286-290: The grep -Eo regexes used when setting rd_link and
dmg_file (symbols: rd_link and dmg_file) use \d which is not POSIX ERE; update
both patterns for aarch64 and x86_64 to use [0-9]+ with escaped dots (e.g.,
replace occurrences like "\d{1}\.\d{1,2}\.\d{1,2}" and the full filename
patterns using \d with corresponding [0-9]+ quantifiers) so grep -Eo reliably
extracts version numbers and the rustdesk .dmg filenames on macOS/BSD and other
environments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac6d5847-0fc1-4b6b-bc9f-db1317a043d6
📒 Files selected for processing (1)
content/self-host/client-deployment/_index.en.md
Comment on lines
+246
to
+248
| # Create log file with world-writable permissions so the user-context subscript can write to it | ||
| touch "$LOG" | ||
| chmod 666 "$LOG" |
There was a problem hiding this comment.
Log file permissions and secret logging expose credentials.
World-writable /tmp/rustdesk-install.log plus logging of RustDesk password and relay key leaks sensitive data and allows tampering.
🔐 Suggested hardening
-touch "$LOG"
-chmod 666 "$LOG"
+touch "$LOG"
+chmod 600 "$LOG"
...
-echo "$(date '+%Y-%m-%d %H:%M:%S') | RustDesk Password: $rustdesk_pw" | tee -a "$LOG"
+echo "$(date '+%Y-%m-%d %H:%M:%S') | RustDesk Password: [REDACTED]" | tee -a "$LOG"
...
-echo "Relay Key: \$RELAY_KEY"
+echo "Relay Key: [REDACTED]"Also applies to: 385-387, 466-468
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/self-host/client-deployment/_index.en.md` around lines 246 - 248, The
log file is created world-writable via touch "$LOG" and chmod 666 "$LOG" (and
repeated in the other occurrences), and secrets (RustDesk password and relay
key) are being written to it; change the log creation to set restrictive
permissions (e.g., owner-only 600 or 640) and ensure the file owner is the
intended service account, and stop writing sensitive values to the log — redact
or omit the RustDesk password and relay key from any logging statements (or
store them in a secure vault/secure file with limited access) and update all
instances that use touch "$LOG"/chmod 666 "$LOG" to use the secure permission
pattern and remove secret output.
Comment on lines
286
to
+290
| rd_link=$(curl -sL https://github.com/rustdesk/rustdesk/releases/latest | grep -Eo "(http|https)://[a-zA-Z0-9./?=_-]*/\d{1}.\d{1,2}.\d{1,2}/rustdesk.\d{1}.\d{1,2}.\d{1,2}.aarch64.dmg") | ||
| dmg_file=$(echo $rd_link | grep -Eo "rustdesk.\d{1}.\d{1,2}.\d{1,2}.aarch64.dmg") | ||
| curl -L "$rd_link" --output "$dmg_file" | ||
| dmg_file="/tmp/$(echo $rd_link | grep -Eo "rustdesk.\d{1}.\d{1,2}.\d{1,2}.aarch64.dmg")" | ||
| else | ||
| rd_link=$(curl -sL https://github.com/rustdesk/rustdesk/releases/latest | grep -Eo "(http|https)://[a-zA-Z0-9./?=_-]*/\d{1}.\d{1,2}.\d{1,2}/rustdesk.\d{1}.\d{1,2}.\d{1,2}.x86_64.dmg") | ||
| dmg_file=$(echo $rd_link | grep -Eo "rustdesk.\d{1}.\d{1,2}.\d{1,2}.x86_64.dmg") | ||
| curl -L "$rd_link" --output "$dmg_file" | ||
| dmg_file="/tmp/$(echo $rd_link | grep -Eo "rustdesk.\d{1}.\d{1,2}.\d{1,2}.x86_64.dmg")" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify regex behavior used in the script (read-only).
set -euo pipefail
sample='https://github.com/rustdesk/rustdesk/releases/download/1.4.0/rustdesk-1.4.0-aarch64.dmg'
echo "Test 1: current style with \\d and grep -E"
echo "$sample" | grep -Eo '\d{1}\.\d{1,2}\.\d{1,2}' || echo "NO_MATCH"
echo "Test 2: ERE-safe style"
echo "$sample" | grep -Eo '[0-9]+\.[0-9]+\.[0-9]+' || echo "NO_MATCH"Repository: rustdesk/doc.rustdesk.com
Length of output: 154
Replace \d with [0-9]+ for portable regex matching with grep -Eo.
The current pattern using \d is not valid POSIX ERE and fails to match version numbers on macOS/BSD (and some Linux configurations). The shell test confirms grep -Eo '\d{1}\.\d{1,2}\.\d{1,2}' returns NO_MATCH while grep -Eo '[0-9]+\.[0-9]+\.[0-9]+' correctly extracts version strings. This causes silent extraction failures and installation aborts.
Replace at lines 286–290 and 294–297:
Affected locations
Lines 286–290 (aarch64) and 294–297 (x86_64)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/self-host/client-deployment/_index.en.md` around lines 286 - 290, The
grep -Eo regexes used when setting rd_link and dmg_file (symbols: rd_link and
dmg_file) use \d which is not POSIX ERE; update both patterns for aarch64 and
x86_64 to use [0-9]+ with escaped dots (e.g., replace occurrences like
"\d{1}\.\d{1,2}\.\d{1,2}" and the full filename patterns using \d with
corresponding [0-9]+ quantifiers) so grep -Eo reliably extracts version numbers
and the rustdesk .dmg filenames on macOS/BSD and other environments.
Comment on lines
320
to
+321
| cp -R "$mount_point/RustDesk.app" "/Applications/" &> /dev/null | ||
| echo "$(date '+%Y-%m-%d %H:%M:%S') | RustDesk.app copied to /Applications/" | tee -a "$LOG" |
There was a problem hiding this comment.
Copy success is assumed without checking return code.
cp -R ... &> /dev/null can fail silently, but success is logged immediately; this can mask install failures and cause downstream errors.
💡 Suggested fix
- cp -R "$mount_point/RustDesk.app" "/Applications/" &> /dev/null
- echo "$(date '+%Y-%m-%d %H:%M:%S') | RustDesk.app copied to /Applications/" | tee -a "$LOG"
+ if cp -R "$mount_point/RustDesk.app" "/Applications/" &> /dev/null; then
+ echo "$(date '+%Y-%m-%d %H:%M:%S') | RustDesk.app copied to /Applications/" | tee -a "$LOG"
+ else
+ echo "$(date '+%Y-%m-%d %H:%M:%S') | ERROR: Failed to copy RustDesk.app to /Applications." | tee -a "$LOG"
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cp -R "$mount_point/RustDesk.app" "/Applications/" &> /dev/null | |
| echo "$(date '+%Y-%m-%d %H:%M:%S') | RustDesk.app copied to /Applications/" | tee -a "$LOG" | |
| if cp -R "$mount_point/RustDesk.app" "/Applications/" &> /dev/null; then | |
| echo "$(date '+%Y-%m-%d %H:%M:%S') | RustDesk.app copied to /Applications/" | tee -a "$LOG" | |
| else | |
| echo "$(date '+%Y-%m-%d %H:%M:%S') | ERROR: Failed to copy RustDesk.app to /Applications." | tee -a "$LOG" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/self-host/client-deployment/_index.en.md` around lines 320 - 321, The
script currently assumes the copy succeeded by running cp -R
"$mount_point/RustDesk.app" "/Applications/" &> /dev/null and immediately
echoing a success line to $LOG; change this to check cp's exit status (or use an
if cp ...; then ... else ...) so failures are detected: run cp without silencing
stderr (or redirect stderr to the $LOG), then test the command result and on
success write the timestamped success message to $LOG, otherwise write a
timestamped error with the cp exit code and stderr and exit non‑zero to stop the
installer; update the lines referencing cp -R "$mount_point/RustDesk.app"
"/Applications/" and the subsequent echo to implement this check.
Comment on lines
+334
to
+337
| if [[ -z "$CONSOLE_USER" || "$CONSOLE_USER" == "loginwindow" || "$CONSOLE_USER" == "_mbsetupuser" ]]; then | ||
| echo "$(date '+%Y-%m-%d %H:%M:%S') | ERROR: No valid console user found. Cannot initialize RustDesk config directory." | tee -a "$LOG" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fallback path is currently unreachable for no-user installs.
exit 1 on Line 336 aborts before the LaunchAgent fallback path, so DEP/MDM “no logged-in user” installs never reach the intended recovery flow.
💡 Suggested fix
-if [[ -z "$CONSOLE_USER" || "$CONSOLE_USER" == "loginwindow" || "$CONSOLE_USER" == "_mbsetupuser" ]]; then
- echo "$(date '+%Y-%m-%d %H:%M:%S') | ERROR: No valid console user found. Cannot initialize RustDesk config directory." | tee -a "$LOG"
- exit 1
-fi
+if [[ -z "$CONSOLE_USER" || "$CONSOLE_USER" == "loginwindow" || "$CONSOLE_USER" == "_mbsetupuser" ]]; then
+ echo "$(date '+%Y-%m-%d %H:%M:%S') | No valid console user found. Deferring user-scoped configuration to LaunchAgent." | tee -a "$LOG"
+else
+ # existing user-initialization + wait-for-config flow
+fiAlso applies to: 497-516
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/self-host/client-deployment/_index.en.md` around lines 334 - 337, The
script currently aborts when CONSOLE_USER is missing (the block checking
CONSOLE_USER and writing to LOG), which prevents the LaunchAgent fallback for
DEP/MDM no-user installs; instead of calling exit 1 in that conditional, log the
error to LOG and set a marker (e.g., export NO_CONSOLE=true or leave
CONSOLE_USER empty) so execution continues into the LaunchAgent fallback flow;
update the conditional around CONSOLE_USER (the block that echoes to LOG) to not
exit, and ensure later code that configures the RustDesk directory/LaunchAgent
checks NO_CONSOLE or an empty CONSOLE_USER to take the fallback path.
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
2 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.
Summary
Replaces the existing macOS Bash deployment script in the client deployment docs with a production-tested version used across managed Mac fleets.
What changed
The previous script had minimal error handling, used fixed sleeps, downloaded to the current working directory, and relied on a config string. This version addresses all of those issues:
Improvements
/tmp/rustdesk-install.logfor troubleshootinglaunchctl asuserto run user-context operations correctlysleepRustDesk2.tomldirectly with relay server, key, and options (no config string needed)Configuration
Users set three variables at the top of the script:
RELAY_SERVER— their RustDesk relay server addressRELAY_KEY— their relay keyMSP— organization name (used for LaunchAgent labeling)This approach is more explicit and less error-prone than the config string method, especially for MSPs managing multiple relay servers.
Summary by CodeRabbit
Documentation