fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info#8
Conversation
…to repair/health-check/view-info
Follow-up to an Elytra-wording inventory: fixes user-facing bugs where
Wings-only install flows still displayed or sent "Elytra", plus three UI
tools that never gained Wings support after the Wings/Wings-RS migration.
- Fixed select_release_version() and its Wings/combined-installer call
sites, which caused "Elytra is installed from binary releases." to
display during Wings and Wings-RS installs.
- create_node_via_api() now accepts a daemon_type parameter and sends the
correct value ("wings" vs "elytra") to the panel API instead of always
hardcoding "elytra"; node name/description labels follow suit.
- installers/both.sh's install_wings_daemon() (Wings-only) no longer
prints "Elytra" in its config/SSL/permission/completion output;
ELYTRA_DIR renamed to WINGS_INSTALL_DIR for clarity.
- ui/both.sh's Wings/Wings-RS configuration flow (variables, function
names, welcome banner) renamed off of ELYTRA_*/configure_elytra_settings.
- Certbot renewal hook now restarts wings in addition to nginx/elytra.
- Added save_wings_install_info/load_wings_install_info/
wings_install_info_exists/display_wings_install_info/
show_wings_completion/auto_fix_wings_issues to lib.sh.
installers/wings.sh was calling save_wings_install_info/
show_wings_completion, which didn't exist anywhere in the repo and
crashed the installer right after a successful Wings install;
installers/both.sh was also misusing the Elytra-only auto-fix/save-info
functions on its Wings install.
- installers/panel.sh's post-install API key messaging no longer assumes
Elytra; mentions both wings.sh and legacy elytra.sh.
- install.sh's ELYTRA_INSTALLED/ELYTRA_VERSION/run_elytra_update (which
actually tracked Wings, not Elytra) renamed to WINGS_INSTALLED/
WINGS_VERSION/run_wings_update.
- ui/repair.sh, ui/health-check.sh, ui/view-info.sh: added Wings
detection, a "Fix Wings Permissions" repair action (reusing the new
auto_fix_wings_issues), a "Check Wings Health" option (reusing the
existing check_wings_health), and Wings installation-info display -
these three tools previously only knew about legacy Elytra.
- installers/auto-update-wings.sh's INSTALL_DIR defaulted to /etc/wings
and several data-directory checks were hardcoded to /var/lib/wings,
neither of which Wings actually uses (the real path is
/etc/pterodactyl and /var/lib/pterodactyl/*); its health check and
auto-fix logic were checking/repairing the wrong directories.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ead code) Follow-up to the previous Elytra->Wings wording pass, covering the items deferred to phase 2 in the original inventory report. - Removed the dead --skip-wings-setup/SKIP_WINGS_SETUP flag from installers/elytra.sh (a never-read copy-paste leftover from wings.sh; wings.sh's own copy is left alone, out of this fix's scope). - ui/uninstall.sh's auto-updater detection only checked for hydrodactyl-elytra-auto-update.timer, so a Wings-only install with its own hydrodactyl-wings-auto-update.timer was invisible to the uninstaller UI. Renamed the misleading ELYTRA_UPDATER_INSTALLED to DAEMON_UPDATER_INSTALLED and check both timer names. - installers/both.sh ignored WINGS_RELEASE_VERSION entirely and always installed the latest Wings release, even though ui/both.sh lets the user pick a specific version. Wired it through to get_latest_release/ download_release_asset, mirroring installers/wings.sh's existing logic. - Added Wings/Wings-RS options to .github/ISSUE_TEMPLATE/bug-report.yml's installation-type dropdown and config.yml's contact links, which previously only offered Panel/Elytra. - Fixed a real bug in manual-guides/both-same-machine.md: Step 5 creates data directories under /var/lib/hydrodactyl/... but the very next commands chown/chmod /var/lib/elytra/..., a path that was never created - the actual data directories never got their ownership/permissions set. Also fixed a garbled sentence about the database password. - Added manual-guides/wings-manual.md, a full manual installation guide for Wings/wings-rs (previously the only daemon manual was Elytra's, despite Wings being the current recommended daemon with no manual walkthrough at all). Wired it into manual-guides/README.md's guide table, "which guide" section, prerequisites, learning path, and quick links; added deprecation-aware notices to elytra-manual.md and both-same-machine.md pointing at it; updated hydrodactyl-panel-manual.md's "Next Steps" to recommend Wings with Elytra noted as the legacy path. - README.md: filled in Wings/Wings-RS coverage that was missing across Features, the Repair Tool and Health Check sections (also fixing their option numbers, which had drifted to 7/8 instead of the current 6/7), SSL restart hooks, default/custom repositories, uninstall options, directory structure, troubleshooting, and logs. Added Wings and wings-rs to Credits and relabeled the architecture diagram. Clarified that "Auto-updaters only" removes on-demand update scripts and any leftover timers from older installs, not a currently-scheduled feature (the scheduled auto-updater/systemd-timer layer was removed in PR itzzjustmateo#3); re-scanned the rest of the file for the same stale wording and found no other occurrences. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughFeature: Wings and wings-rs support now spans installation, provisioning, health checks, repair, updates, uninstall, documentation, and issue routing. Fix: Wings paths, permissions, SSL handling, updater detection, and daemon-specific API payloads are updated. Refactor: Shared daemon detection, configuration, metadata, and repair handling are consolidated while legacy Elytra remains supported and labeled.
ChangesWings integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@installers/auto-update-wings.sh`:
- Around line 683-685: Complete the permission-check fix in auto-update Wings
logic: within the config.yml branch following the info call in
auto_fix_wings_issues, add the same sed replacement used by the canonical
lib/lib.sh and installers/both.sh implementations to change
check_permissions_on_boot: true to false.
In `@lib/lib.sh`:
- Around line 4588-4661: Update auto_fix_wings_issues to resolve the
contradictory permission strategy: either replace blanket chmod 777 with
shared-group ownership and setgid/ACL permissions, or retain world-writable
access and remove redundant chown operations. If retaining ACL inheritance,
change the default ACL entries for volumes to grant rwx (including write) for
the intended users/groups and apply them consistently to all required data
directories, updating the comments to match the chosen security model.
- Around line 3883-3926: Update save_wings_install_info so $info_file is created
with mode 600 before the grouped echo block writes any secrets. Pre-create it
using install -m 600 (or an equivalent tightly scoped umask), then write to the
existing path and retain the final chmod as a safety check.
In `@manual-guides/both-same-machine.md`:
- Around line 405-410: Align the guide’s ownership and permission commands with
the legacy Elytra data paths used by configs/elytra-config.yml and the README:
update the chown and chmod targets from /var/lib/hydrodactyl to /var/lib/elytra,
including volumes, archives, and backups.
In `@manual-guides/README.md`:
- Around line 67-70: Update the supported OS requirements in the manual guide
index to match the versions documented in manual-guides/wings-manual.md and the
root README, including Debian 11, Rocky 8, and AlmaLinux 8. Keep the OS matrix
consistent across all referenced documentation.
- Around line 81-83: Update the daemon prerequisites list in the relevant README
section to explicitly name “Wings/Wings-RS and Elytra” instead of only “Wings
and Elytra,” while leaving the Docker Engine and swap accounting requirements
unchanged.
In `@manual-guides/wings-manual.md`:
- Around line 83-86: Separate the distro-specific commands in the installation
instructions: place the Ubuntu/Debian apt commands and Rocky/AlmaLinux dnf
commands in distinct fenced blocks, or add OS detection so only the appropriate
command runs.
- Around line 92-101: Separate the combined “Ubuntu/Debian” installation section
into distinct Ubuntu and Debian instructions. Keep the Ubuntu commands using
download.docker.com/linux/ubuntu, and add Debian-specific repository commands
using download.docker.com/linux/debian with the appropriate Debian release
codename, while retaining the package installation steps for both.
- Around line 320-324: Update the API verification examples and the additional
affected command to use the configured endpoint scheme: HTTP when TLS is
disabled and HTTPS only when TLS is enabled, rather than hardcoding https://.
Keep the curl checks and expected response guidance consistent for both modes.
- Around line 166-171: Update the “Record the Installed Version” section to
instruct users to replace <version-you-installed> with the actual GitHub release
tag, matching the value written by installers/wings.sh and installers/both.sh
and consumed by installers/auto-update-wings.sh; show an explicit real-tag
example rather than implying the placeholder should be written verbatim.
- Around line 121-130: Update the Wings guide text around the pterodactyl
account setup to state that this account is used only for Docker group
membership and file/container ownership, while wings.service continues running
the daemon as root. Avoid wording that implies Wings drops privileges to
pterodactyl.
- Around line 385-399: The “Updating Wings” section only downloads the Go daemon
and can overwrite wings-rs installations with the wrong binary. Split the
guidance into explicit Go and wings-rs variants, documenting the correct
variant-specific download/update commands or linking wings-rs users to its
matching update procedure; clearly warn users not to apply the Go flow to
wings-rs nodes.
In `@README.md`:
- Around line 95-105: Update the README service references in the “Restart All
Services” and “Panel Health” sections to use the actual systemd unit name
`pyroq.service` (or `pyroq`) instead of `hydroq`, matching the service defined
and operated in the referenced manual guides.
- Line 28: Update the Docker compatibility requirement bullets in README.md to
explicitly include Wings-RS alongside Wings and Elytra, including the
corresponding occurrence noted by the reviewer, so all supported Docker-backed
integrations are listed consistently.
- Line 92: Align the documented Wings UID/GID across README.md and
manual-guides/wings-manual.md by choosing one canonical value, then update the
repair-tool description and manual installation commands to use that same value
consistently.
In `@ui/view-info.sh`:
- Around line 107-119: Update both Wings failure-log checks in the health
detection and display logic of view-info.sh to use
/etc/pterodactyl/update-health-check-failure.log instead of
/etc/wings/update-health-check-failure.log, keeping detection and user-facing
output consistent with the INSTALL_DIR used by the Wings installers.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d13f2a73-4470-4fc9-b14b-f9af06a0a991
📒 Files selected for processing (21)
.github/ISSUE_TEMPLATE/bug-report.yml.github/ISSUE_TEMPLATE/config.ymlREADME.mdinstall.shinstallers/auto-update-wings.shinstallers/both.shinstallers/elytra.shinstallers/panel.shinstallers/wings.shlib/lib.shmanual-guides/README.mdmanual-guides/both-same-machine.mdmanual-guides/elytra-manual.mdmanual-guides/hydrodactyl-panel-manual.mdmanual-guides/wings-manual.mdui/both.shui/health-check.shui/repair.shui/uninstall.shui/view-info.shui/wings.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
⚠️ CI failures not shown inline (8)
GitHub Actions: Markdown Lint / Lint Markdown: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 353 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: Markdown Lint / 0_Lint Markdown.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 353 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: CI / 1_Bash Syntax Check.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / ShellCheck: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / 2_ShellCheck.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / Executable Permissions: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / Bash Syntax Check: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / 0_Executable Permissions.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
🧰 Additional context used
🪛 ast-grep (0.44.1)
installers/auto-update-wings.sh
[warning] 675-675: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod 777 /var/lib/pterodactyl/volumes
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 676-676: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 /var/lib/pterodactyl/volumes/*
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 677-677: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod 777 /var/lib/pterodactyl/archives
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 678-678: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 /var/lib/pterodactyl/archives/*
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 679-679: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod 777 /var/lib/pterodactyl/backups
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 680-680: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 /var/lib/pterodactyl/backups/*
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
ui/both.sh
[warning] 398-398: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: token_valid=false
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-password-assignment-bash)
lib/lib.sh
[warning] 4620-4620: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod 777 /var/lib/pterodactyl/volumes
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 4621-4621: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 /var/lib/pterodactyl/volumes/*
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 4622-4622: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod 777 /var/lib/pterodactyl/archives
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 4623-4623: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 /var/lib/pterodactyl/archives/*
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 4624-4624: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod 777 /var/lib/pterodactyl/backups
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 4625-4625: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 /var/lib/pterodactyl/backups/*
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
🪛 LanguageTool
manual-guides/wings-manual.md
[grammar] ~92-~92: Ensure spelling is correct
Context: ... ## Step 2: Install Docker ### Ubuntu/Debian bash apt install -y apt-transport-https ca-certificates curl gnupg lsb-release mkdir -p /etc/apt/keyrings curl -fsSL https://download.docker.com/linux/ubuntu/gpg | gpg --dearmor -o /etc/apt/keyrings/docker.gpg echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.gpg] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable" | tee /etc/apt/sources.list.d/docker.list > /dev/null apt update apt install -y docker-ce docker-ce-cli containerd.io docker-compose-plugin ### Rocky Linux/AlmaLinux ```bash dnf instal...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~282-~282: Ensure spelling is correct
Context: ...Firewall Configuration ### UFW (Ubuntu/Debian) bash ufw allow 22/tcp # SSH ufw allow 80/tcp # HTTP (certbot renewal) ufw allow 443/tcp # HTTPS ufw allow 8080/tcp # Wings API ufw allow 2022/tcp # SFTP # Game server ports (adjust as needed) ufw allow 25565:25665/tcp # Minecraft ufw allow 25565:25665/udp ufw allow 27015:27150/tcp # Source Engine ufw allow 27015:27150/udp ufw enable ### FirewallD (Rocky/AlmaLinux) ```bash fire...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
README.md
[style] ~89-~89: Consider using a different verb for a more formal wording.
Context: ...Option 6) The built-in repair tool can fix common issues: - **Fix Panel Permissio...
(FIX_RESOLVE)
🔇 Additional comments (31)
.github/ISSUE_TEMPLATE/bug-report.yml (1)
49-52: LGTM!.github/ISSUE_TEMPLATE/config.yml (1)
9-17: LGTM!README.md (1)
11-27: LGTM!Also applies to: 29-30, 91-91, 122-122, 150-152, 167-195, 223-247, 269-269, 333-336, 388-389, 423-425
manual-guides/README.md (1)
3-36: LGTM!Also applies to: 93-95, 112-112, 130-132
manual-guides/both-same-machine.md (2)
3-4: LGTM!Also applies to: 244-244, 849-851
407-410: 🔒 Security & PrivacyThe 777 permissions are intentional here.
This guide follows the same Elytra permission model used elsewhere in the repo; it’s not a stray backup/archive policy change in this snippet.
> Likely an incorrect or invalid review comment.manual-guides/elytra-manual.md (1)
3-5: LGTM!manual-guides/hydrodactyl-panel-manual.md (1)
641-643: LGTM!manual-guides/wings-manual.md (1)
1-66: LGTM!Also applies to: 70-82, 104-115, 135-164, 176-221, 223-249, 265-318, 331-358, 365-379, 401-416
lib/lib.sh (5)
520-520: LGTM!Also applies to: 986-986
1065-1131: Elytra's ghost properly exorcised from the version-selector.The non-panel branch now speaks in
daemon_labelinstead of hardcoding "Elytra" everywhere. Clean generalization, no notes.
2611-2632: LGTM!
3951-3977: Load/exists/display trio, and the token stays hidden.
load_wings_install_info,wings_install_info_exists,display_wings_install_info, andshow_wings_completionare clean, mirror the existing Elytra pattern, anddisplay_wings_install_infocorrectly masksNODE_TOKENas(hidden)rather than dumping it to the terminal. Good instinct.Also applies to: 4058-4092, 4152-4180
3550-3646: 🗄️ Data Integrity & Integration
daemon_typeneeds a panel-side source of truth. This tree doesn’t define whether the API accepts"wings"here, so that value is still an external contract gamble.installers/elytra.sh (2)
421-421: LGTM!
108-108: 📐 Maintainability & Code QualityWrong file:
--skip-wings-setupstill lives ininstallers/wings.sh, notinstallers/elytra.sh. This comment doesn’t apply to the Elytra installer; the parser/help/export path for that flag is still in the Wings installer, so the note belongs there instead.> Likely an incorrect or invalid review comment.installers/wings.sh (1)
412-412: LGTM!ui/wings.sh (1)
166-166: A genuine bug slain, not just a refactor.Swapping the component identifier to
"wings"means the Wings installer will finally print "Wings is installed from binary releases" instead of quietly gaslighting the operator with Elytra branding mid-Wings-install. This is the kind of copy-paste residue that survives code review for months because nobody reads their own CLI output closely enough. Good catch.ui/health-check.sh (3)
9-9: LGTM!Also applies to: 98-98
158-171:detect_wings_binary— a faithful twin ofdetect_elytra_binary.Straightforward, matches the existing detection pattern for the same two well-known binary paths. No complaints.
185-278: Menu renumbering checks out end-to-end.Traced every branch:
0=Panel,1=Wings (new),2=Elytra,3=All,4=System Resources,5=Back — the prompt range, thecasearms, and the "invalid option" message all agree on0-5. The "Check All" branch correctly tracks three independent presence flags and only errors when all three come up empty. This is the boring, correct kind of diff. Rare specimen, treasure it.ui/uninstall.sh (2)
115-115: LGTM!Also applies to: 169-169, 285-285
54-86: 🩺 Stability & AvailabilityNo cleanup precision bug
REMOVE_AUTO_UPDATERSonly gates the action;remove_auto_updater_elytra()already stops, disables, and removes bothhydrodactyl-elytra-auto-update.timerandhydrodactyl-wings-auto-update.timer, so the merged detection flag doesn’t leave a stray timer behind.> Likely an incorrect or invalid review comment.installers/panel.sh (1)
622-622: LGTM!Also applies to: 632-632, 659-666
installers/both.sh (2)
677-677: 🗄️ Data Integrity & IntegrationThe panel API contract for
daemon_type: "wings"is still an unconfirmed prophecy.
create_node_via_apinow ships"wings"as the 9th arg regardless of whether the user picked Go Wings or Wings-RS, and per the PR's own notes this value's acceptance by the panel API is still unverified. If the panel's node model doesn't recognize"wings"as a validdaemon_typeenum, every automated node-creation call silently falls back to the manual artisan/MySQL path — a much worse UX than a clean failure, and one that won't surface until someone actually runs this in anger.Confirm the panel's node schema/enum actually accepts
"wings"(and whether Wings-RS needs a distinct value) before trusting this path in production.
7-9: LGTM!Also applies to: 81-81, 104-105, 743-743, 752-752, 764-764, 791-813, 822-832, 841-878, 905-910, 923-936, 950-950, 1156-1157
ui/both.sh (1)
7-7: LGTM!Also applies to: 58-63, 345-363, 372-404, 413-452, 524-525, 586-591, 606-610
installers/auto-update-wings.sh (1)
32-32: LGTM!Also applies to: 549-549, 618-618, 645-682, 686-692
install.sh (1)
39-39: LGTM!Also applies to: 254-256, 268-270, 298-299, 344-344, 392-392, 420-426, 472-481, 495-499, 616-616
ui/repair.sh (1)
9-9: LGTM!Also applies to: 95-119, 207-224, 277-283, 453-455, 495-506, 517-558
ui/view-info.sh (1)
9-9: LGTM!Also applies to: 43-82, 93-98, 128-128, 176-183
Verified each finding against the actual current code before fixing, per review. Three findings from the same review pass were confirmed invalid and skipped (both-same-machine.md's intentional 777 permissions, --skip-wings-setup misattributed to installers/elytra.sh instead of installers/wings.sh, and a ui/uninstall.sh cleanup-precision comment) - not included here. Real bugs: - installers/auto-update-wings.sh's auto_fix_wings_issues() printed "Disabling permission checks in Wings config..." but never actually ran the sed to flip check_permissions_on_boot - added the missing sed, matching lib.sh's and installers/both.sh's implementations. Confirmed this was a pre-existing bug (present before this branch's changes), not something introduced by the earlier Wings-wording commits. - ui/view-info.sh checked/displayed a Wings health-check-failure log at /etc/wings/... - fixed to /etc/pterodactyl/..., matching WINGS_INSTALL_DIR everywhere else. - manual-guides/both-same-machine.md's legacy Elytra section created data directories under /var/lib/elytra/... but a few remaining references (config.yml body, disk-usage check) still pointed at /var/lib/hydrodactyl/... - a second instance of the same class of bug fixed in the previous commit. Unified everything in that section on /var/lib/elytra/..., matching configs/elytra-config.yml and the root README. - lib.sh's save_wings_install_info() wrote the info file (containing GITHUB_TOKEN/PANEL_API_KEY/NODE_TOKEN) and chmod 600'd it afterward, leaving a brief window where the file exists under the process umask (usually world/group-readable). Pre-create it with `install -m 600 /dev/null` before writing instead. - lib.sh's auto_fix_wings_issues() set default ACLs with a comment claiming they make new files inherit 777, but the actual flags (d:o:rx/d:g:rx) omit write. Changed to d:o:rwx/d:g:rwx to match the stated intent (containers running as arbitrary UIDs need write access on files other containers create later, same reasoning as the explicit chmod 777 immediately above it). The already-committed chmod 777 itself is unchanged - that's the documented, accepted Wings/Pterodactyl pattern, not a bug. - Reconciled Wings' UID: installers/wings.sh and configs/wings.service create/reference a `pterodactyl` user at UID/GID 9999, but lib.sh's and installers/auto-update-wings.sh's auto_fix_wings_issues() were chowning Wings directories to 8888 (copied from the Elytra/hydrodactyl UID when the Wings version was added) and README.md's Repair Tool bullet said 8888 too. Standardized all three on 9999. Note: installers/both.sh (the combined Panel+Wings installer) separately creates and chows to a `hydrodactyl` user at 8888 for the same directories - that's a deeper, pre-existing inconsistency between the two installers that goes beyond this fix's scope; flagging it rather than changing installer user- creation logic here. Documentation bugs (would fail if followed literally): - manual-guides/wings-manual.md's Docker install mixed an apt command and a dnf command in the same certbot code block; split into separate per-OS blocks. Also split the "Ubuntu/Debian" Docker-repo section into Ubuntu and Debian subsections, since it only ever pointed at the Ubuntu repo URL - added a Debian-specific block using download.docker.com/linux/debian. - wings-manual.md's "Updating Wings" section only showed the Go-variant download command, which would silently overwrite a wings-rs install with the wrong binary - split into separate Go/wings-rs blocks with a note to use the one matching the installed variant. Minor accuracy/consistency fixes: - manual-guides/README.md's prerequisites said Debian 12+/Rocky 9+/ AlmaLinux 9+, understating what lib.sh's detect_os() actually supports (Debian 11+, Rocky/AlmaLinux 8+) and disagreeing with the root README and wings-manual.md/elytra-manual.md - aligned to the same baseline. - manual-guides/README.md's "Wings and Elytra additionally require" changed to "Wings/Wings-RS and Elytra". - wings-manual.md's version-recording step showed a literal <version-you-installed> placeholder as if it should be typed verbatim - replaced with a real command that captures the actual release tag from the GitHub API, matching installers/wings.sh's own logic. - wings-manual.md's pterodactyl-user step could read as if Wings drops privileges to that account - clarified that wings.service runs as root (per configs/wings.service) and the account exists only for docker group membership and file/volume ownership. - wings-manual.md's API verification/troubleshooting commands hardcoded https even though SSL (Step 7) is optional and off by default - now show both http and https variants with a note on which applies. - README.md's Docker Compatibility bullets said "Wings and Elytra"; added Wings-RS explicitly (three daemon variants, not two). - manual-guides/both-same-machine.md and hydrodactyl-panel-manual.md's queue worker systemd unit was still named pyroq.service, from before the project's pyroq->hydroq rename. lib.sh's install_hydroq() and configs/hydroq.service confirm hydroq is the current real service name (root README.md already had this right) - renamed all pyroq references in both manual guides to hydroq. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Three additional nitpick-level notes from this review (not among the 16 actionable inline threads, and each self-flagged by CodeRabbit as "Likely an incorrect or invalid review comment") were checked and skipped:
|
Follow-up to investigation confirming pterodactyl:9999 as the canonical Wings identity (matches installers/wings.sh, lib.sh's shared auto_fix_wings_issues(), installers/auto-update-wings.sh, and the docs; verified against two independent third-party Hydrodactyl installers and upstream Pterodactyl's own Wings docs, which create no dedicated user at all). installers/both.sh's install_wings_daemon() was still creating/using a "hydrodactyl" user at UID/GID 8888 - a leftover from when this exact code block created the Elytra service account, before this installer's daemon backend was switched from Elytra to Wings (git blame traces both to the same rename commit, deda822, which only did a straight pyrodactyl-> hydrodactyl string substitution and didn't reconsider the UID once the block started serving Wings instead of Elytra). Changed the groupadd/useradd/usermod block and the data-directory chown to pterodactyl:9999, mirroring installers/wings.sh's user-creation logic exactly. Left every unrelated "hydrodactyl" reference in the file untouched (DB_USER MySQL username, /etc/hydrodactyl panel-config paths, /root/.config/hydrodactyl credentials dir, /tmp/hydrodactyl-lib.sh, PANEL_REPO) - none of those are the Wings system user. Confirmed no config/*.service unit references this user (wings.service runs as root/root) and installers/panel.sh has no same-named user that could be confused with it. Note for follow-up: installers/uninstall.sh's remove_wings() has a comment explaining it intentionally never deletes a system user, because "installers/both.sh instead reuses the hydrodactyl user shared with the panel" - that reasoning is now stale, since both.sh creates the same pterodactyl user as the standalone installer. Left as-is since it wasn't part of this fix's requested scope; flagging for a follow-up decision on whether remove_wings() should now clean up the pterodactyl user the way remove_elytra() cleans up its hydrodactyl user. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ce9c3 remove_wings() previously skipped deleting a system user, reasoning that installers/both.sh (combined) reused a "hydrodactyl" user shared with the panel while installers/wings.sh (standalone) used a separate "pterodactyl" user - so an uninstall script couldn't safely know which one applied. Commit 1ace9c3 unified both.sh onto the same "pterodactyl" user installers/wings.sh already used, which invalidates that premise: both install paths now create the identical dedicated user, and it was never used by the panel to begin with (confirmed installers/panel.sh has no "pterodactyl" reference at all - the panel's own process user is $WEBUSER, a completely separate identity). remove_wings() now deletes the pterodactyl user/group the same way remove_elytra() already deletes its hydrodactyl user/group, safe regardless of which installer created it or whether a panel is also present on the system. Updated the now-stale comments in both remove_elytra() and remove_daemon_common() that referenced the old "can't safely delete, might be shared" reasoning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
manual-guides/wings-manual.md (1)
151-153: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the Wings data ownership and permissions here
mkdir -palone leaves the data tree in the wrong state for the documentedpterodactylowner. Mirror the installer flow here: chown/var/lib/pterodactyl/{volumes,archives,backups}, make/var/lib/pterodactyltraversable, and keep/etc/pterodactylrestricted to config credentials.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@manual-guides/wings-manual.md` around lines 151 - 153, Update the setup commands near the directory creation instructions to mirror the installer’s ownership and permission flow: recursively chown `/var/lib/pterodactyl/volumes`, `/var/lib/pterodactyl/archives`, and `/var/lib/pterodactyl/backups` to the `pterodactyl` user and group, make `/var/lib/pterodactyl` traversable, and apply restricted permissions to `/etc/pterodactyl` for configuration credentials.ui/view-info.sh (1)
76-83: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
IFS=', 'betrays you — only the comma survives, the space is banished.For
"${found_parts[*]}", bash joins with the first character ofIFSonly. SoIFS=', 'yieldsPanel,Wings,Elytra, not the friendlyPanel, Wings, Elytrayou clearly intended. Trivial to fix, but it's shipping malformed user-facing text on every multi-daemon box.🩹 Proposed fix — join with a real ", "
- local found_list - found_list=$(IFS=', '; echo "${found_parts[*]}") + local found_list + found_list=$(printf '%s, ' "${found_parts[@]}") + found_list=${found_list%, } output "Installation information found for: ${found_list}."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/view-info.sh` around lines 76 - 83, Fix the list formatting in the found_list construction after found_parts is populated: "${found_parts[*]}" uses only the first IFS character, producing comma-only separators. Update the joining logic to explicitly produce ", " between entries while preserving the existing output message and behavior for Panel, Wings, and Elytra.installers/auto-update-wings.sh (1)
645-708: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThree copies of the same spellbook, and they already drifted apart once.
auto_fix_wings_issuesnow lives in triplicate —lib/lib.sh,installers/both.sh, and here — and this very copy is the one that silently lost itscheck_permissions_on_bootsedfor a while (freshly re-sealed). This copy also lacks thesetfacldefault-ACL block the other two carry, so the three are already diverging again. If the standalone auto-updater genuinely cannot sourcelib/lib.sh, this duplication is a necessary evil; if it can, fold them into one shared function before the next divergence bug crawls out of the crypt.Please confirm whether the auto-update scripts are designed to run fully standalone (no
lib/lib.shsourcing); if they can source it, consolidate the three copies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installers/auto-update-wings.sh` around lines 645 - 708, Confirm whether the auto-update scripts, including installers/auto-update-wings.sh, are required to run without sourcing lib/lib.sh; if not, remove the duplicated auto_fix_wings_issues implementations from installers/both.sh and installers/auto-update-wings.sh and source the shared function from lib/lib.sh. If standalone execution is required, keep the copies synchronized by adding the missing setfacl default-ACL logic and ensuring the check_permissions_on_boot update matches the canonical implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@installers/both.sh`:
- Around line 764-770: Ensure every fallback branch in the pterodactyl user
creation logic uses primary group ID 9999. Update both fallback useradd
invocations in the pterodactyl setup block to include --gid 9999, matching the
existing first branch and the group created by groupadd.
---
Outside diff comments:
In `@installers/auto-update-wings.sh`:
- Around line 645-708: Confirm whether the auto-update scripts, including
installers/auto-update-wings.sh, are required to run without sourcing
lib/lib.sh; if not, remove the duplicated auto_fix_wings_issues implementations
from installers/both.sh and installers/auto-update-wings.sh and source the
shared function from lib/lib.sh. If standalone execution is required, keep the
copies synchronized by adding the missing setfacl default-ACL logic and ensuring
the check_permissions_on_boot update matches the canonical implementation.
In `@manual-guides/wings-manual.md`:
- Around line 151-153: Update the setup commands near the directory creation
instructions to mirror the installer’s ownership and permission flow:
recursively chown `/var/lib/pterodactyl/volumes`,
`/var/lib/pterodactyl/archives`, and `/var/lib/pterodactyl/backups` to the
`pterodactyl` user and group, make `/var/lib/pterodactyl` traversable, and apply
restricted permissions to `/etc/pterodactyl` for configuration credentials.
In `@ui/view-info.sh`:
- Around line 76-83: Fix the list formatting in the found_list construction
after found_parts is populated: "${found_parts[*]}" uses only the first IFS
character, producing comma-only separators. Update the joining logic to
explicitly produce ", " between entries while preserving the existing output
message and behavior for Panel, Wings, and Elytra.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 046a60ac-39c3-497f-a42c-ce7264f45d58
📒 Files selected for processing (10)
README.mdinstallers/auto-update-wings.shinstallers/both.shinstallers/uninstall.shlib/lib.shmanual-guides/README.mdmanual-guides/both-same-machine.mdmanual-guides/hydrodactyl-panel-manual.mdmanual-guides/wings-manual.mdui/view-info.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
⚠️ CI failures not shown inline (8)
GitHub Actions: Markdown Lint / 0_Lint Markdown.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 362 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: Markdown Lint / Lint Markdown: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 362 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: CI / Executable Permissions: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / Bash Syntax Check: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / 2_Bash Syntax Check.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / 0_Executable Permissions.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / ShellCheck: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / 1_ShellCheck.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
🧰 Additional context used
🪛 LanguageTool
manual-guides/wings-manual.md
[grammar] ~225-~225: Ensure spelling is correct
Context: ... a Let's Encrypt certificate: Ubuntu/Debian: bash apt install -y certbot Rocky Linux/AlmaLinux: ```bash dnf ins...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (21)
README.md (1)
20-30: LGTM!Also applies to: 75-75, 87-105, 122-122, 150-152, 171-177, 333-336, 423-425
manual-guides/README.md (1)
3-5: LGTM!Also applies to: 11-14, 18-36, 63-70, 81-83, 91-95, 110-116, 128-132
manual-guides/both-same-machine.md (1)
244-244: LGTM!Also applies to: 351-374, 390-390, 405-413, 432-434, 643-652, 848-851
manual-guides/hydrodactyl-panel-manual.md (1)
413-413: LGTM!Also applies to: 458-464, 526-526, 589-595, 641-643
manual-guides/wings-manual.md (1)
1-66: LGTM!Also applies to: 70-143, 155-188, 192-260, 264-300, 304-390, 401-445, 449-458
ui/view-info.sh (3)
112-114: Wings failure-log path now correctly points at/etc/pterodactylin both detection and display — matchingINSTALL_DIR. Already flagged and resolved in a prior review.Also applies to: 139-145
93-97: LGTM!
176-182: LGTM!lib/lib.sh (6)
3550-3570: LGTM!
3626-3646: 🩺 Stability & Availability | ⚡ Quick winConfirm the panel actually accepts
daemon_typebefore this incantation reaches the altar.You're feeding
daemon_typeinto a Pterodactyl-compatible node API. Vanilla Pterodactyl's node endpoint has no such field — if this panel forwards to a strict backend, an unexpected key can either be silently dropped (harmless) or rejected (node creation dies). Your own PR summary lists "confirmation of the panel API's expecteddaemon_type: \"wings\"value" as outstanding, so this contract is currently faith-based. Thejqand printf branches agree on the payload, at least — no argument-order curse there.Please confirm against the panel's node schema that
daemon_typeis a recognized field and that"wings"/"elytra"are accepted values.
3884-3930: LGTM!
3956-3981: LGTM!
4063-4096: LGTM!
4595-4668: LGTM!installers/both.sh (4)
677-677: LGTM!
791-816: LGTM!
905-936: LGTM!
1157-1157: LGTM!installers/auto-update-wings.sh (1)
683-686: LGTM!installers/uninstall.sh (2)
111-125: LGTM!
127-145: 🩺 Stability & AvailabilityDrop the
pterodactyl-user warning. This repo creates that identity only for Wings; the panel uses$WEBUSER/www-data, so uninstalling Wings’ own user/group is just cleaning up the daemon’s leftovers, not torching panel ownership.> Likely an incorrect or invalid review comment.
…mmit 570a0de) - installers/both.sh: the pterodactyl useradd fallback chain specified --gid 9999 only on the first attempt; the two fallback attempts omitted it, so if those ever fired the new user would land in whatever default group useradd picks instead of the pterodactyl group created just before. Added --gid 9999 to all three attempts. installers/wings.sh (the pattern's source - both.sh's block was copied from it) had the identical bug; fixed there too for consistency. Note: installers/elytra.sh's hydrodactyl useradd chain has the same missing-gid shape, but that's unrelated legacy Elytra code outside this PR's Wings-focused scope - left untouched, flagging for a follow-up. - manual-guides/wings-manual.md: fixed the same missing --gid 9999 in its copy of the useradd block, and added a permissions note after the "Create Directories" step. Checked installers/wings.sh's actual install_wings() first - it does NOT chown/chmod the data directories itself (unlike installers/both.sh and lib.sh's auto_fix_wings_issues()); it leaves check_permissions_on_boot untouched (default true), relying on Wings fixing its own directory permissions on first boot. The guide now explains that, plus the explicit chown/chmod commands for anyone who disables that setting. - ui/view-info.sh: "${found_parts[*]}" with IFS=', ' only uses the first IFS character as separator in bash, so the summary line read "Panel,Wings,Elytra" instead of "Panel, Wings, Elytra". Replaced with printf '%s, ' + trailing-separator strip, verified against a standalone test. - auto_fix_wings_issues() duplication: confirmed only 2 real function definitions exist (lib.sh, installers/auto-update-wings.sh) - installers/both.sh calls the shared lib.sh one, it doesn't define its own. Consolidating the two remaining copies isn't possible: installers/auto-update-wings.sh is invoked via get_script()'s `bash <(curl ...)` path (a fresh subshell with no inherited functions) both from the interactive "Update Wings Daemon" menu and standalone via cron/manual runs, and defines its own complete parallel set of output/logging helpers (setup_colors, log, output, success, error, warning, info, debug) - self-containment is deliberate, not an oversight, and lib.sh's functions are never actually available in either invocation path. Brought the two copies to full parity instead: added the entirely-missing setfacl default-ACL block (containers running as arbitrary UIDs need write access on files other containers create later, same reasoning as the explicit chmod 777 beside it) and matched the "Attempting to auto-fix Wings issues..." message and a missing comment line. Confirmed via a normalized diff that both implementations are now identical aside from lib.sh hardcoding /etc/pterodactyl where auto-update-wings.sh uses its own $INSTALL_DIR variable (intentional - that variable is user-configurable in this script, throughout the rest of the file). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Follow-up on this review's findings that weren't posted as individually-resolvable inline threads (outside the diff range / bundled in "Additional comments"): Fixed in 02e1b6a
Not changed — likely invalid (self-flagged)
Still open
|
Bugs (1-6): - ui/repair.sh: guard detect_wings_binary/detect_elytra_binary calls in restart_services() with `|| true`, since set -e otherwise aborts the whole script (and the "Restart All Services" menu option, which calls restart_services unguarded) whenever one daemon isn't installed. - lib.sh: save_wings_install_info() now falls back to GITHUB_TOKEN_WINGS before GITHUB_TOKEN (matching save_wings_update_config()'s existing pattern), so a private-repo token entered via the combined installer is no longer silently dropped from wings-info. - ui/repair.sh + lib.sh: fix_wings_permissions() now calls auto_fix_wings_issues with a new skip_restart option, so "Run All Fixes" no longer restarts Wings twice (once via fix_wings_permissions, once via restart_services) - mirrors fix_elytra_permissions(), which never restarted the service itself either. - lib.sh + installers/auto-update-wings.sh: the setfacl default-ACL step in auto_fix_*_issues() now actually covers archives/backups, not just volumes, matching what its own comment already claimed. - installers/elytra.sh: add the missing --gid 8888 to the 2nd/3rd useradd fallback branches for the hydrodactyl user, the same bug already fixed for the pterodactyl user elsewhere in this branch. - ui/both.sh: unify the WINGS_VARIANT Go/Rust display wording with lib.sh/ui/wings.sh's "Pterodactyl Wings (Go)"/"wings-rs (Rust)" (the more widely used form), so the same install doesn't show different variant names on the pre-install summary vs. the view-info tool. Cleanup (7-9): - lib.sh: consolidate auto_fix_elytra_issues()/auto_fix_wings_issues() (~70 duplicated lines each) into a shared _auto_fix_daemon_issues() helper; also reconciles the setfacl ACL flags that had drifted (elytra d:o:rx/d:g:rx vs wings d:o:rwx/d:g:rwx) to rwx for both, since both daemons' containers write as arbitrary UIDs for the same reason. - lib.sh: move detect_wings_binary()/detect_elytra_binary() out of ui/health-check.sh and ui/repair.sh (duplicated in both) into lib.sh, which both files already source. - lib.sh: extract the repeated elytra/wings -> display-label case statement (select_release_version(), create_node_via_api()) into a shared daemon_display_name() helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
ui/repair.sh (2)
181-201: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDetected
$wings_binaryis displayed but never used for the actual fix.
fix_wings_permissionsdetects the binary path (line 185), prints "Found Wings binary at: $wings_binary" (line 190), then callsauto_fix_wings_issues skip_restart(line 197) — which hardcodes/usr/local/bin/wingsvia lib.sh:4647-4649. The detected path is cosmetic only. If Wings lives at/usr/bin/wings, the user sees "Found Wings binary at: /usr/bin/wings" followed by "Wings permissions fixed", but_auto_fix_daemon_issuesnever found the binary, sochmod +xwas silently skipped.Compare with
fix_elytra_permissions(line 147):chmod +x "$elytra_binary"— it uses the detected path directly. The Wings equivalent delegates to a wrapper that ignores it.Quickest localized fix: apply
chmod +xon the detected path before calling the shared routine, matching the Elytra function's approach.🔧 Localized fix — use detected path for binary chmod
output "Found Wings binary at: $wings_binary" + # Fix binary permissions using the detected path — auto_fix_wings_issues + # hardcodes /usr/local/bin/wings and may miss /usr/bin/wings installations + chmod +x "$wings_binary" + # Reuse the shared fix routine from lib.sh (binary/data-dir/config # permissions) instead of duplicating it here. Skip its restart step - # restart_services() below is the single place Wings gets restarted, # matching fix_elytra_permissions()'s design (permission fixes don't # restart the service; "Restart All Services"/"Run All Fixes" does). auto_fix_wings_issues skip_restart🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/repair.sh` around lines 181 - 201, fix_wings_permissions currently only displays the detected binary path while auto_fix_wings_issues may ignore /usr/bin/wings. Add an explicit chmod +x "$wings_binary" using the detected path, matching fix_elytra_permissions, before invoking auto_fix_wings_issues; preserve the existing shared routine and skip_restart behavior.
130-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
fix_elytra_permissionsinlines ~50 lines thatfix_wings_permissionscorrectly delegates.
fix_wings_permissions(lines 181-201) is a clean 21-line function that detects the binary and delegates toauto_fix_wings_issues skip_restart. Meanwhile,fix_elytra_permissions(lines 130-179) hand-rolls the same sequence: binary chmod, data-dir mkdir, chown, chmod 777, config sed, permission setting — all of which_auto_fix_daemon_issuesalready handles generically.If an
auto_fix_elytra_issueswrapper exists in lib.sh (matching theauto_fix_wings_issuespattern),fix_elytra_permissionscould collapse to the same shape. If it doesn't exist yet, creating it is trivial.♻️ Proposed refactor — collapse fix_elytra_permissions to match fix_wings_permissions
fix_elytra_permissions() { print_flame "Fixing Elytra Permissions" local elytra_binary - local elytra_dir - elytra_binary=$(detect_elytra_binary) || { error "Elytra binary not found at /usr/local/bin/elytra or /usr/bin/elytra" return 1 } - - elytra_dir=$(detect_elytra_config_dir) - output "Found Elytra binary at: $elytra_binary" - - output "Setting binary permissions..." - chmod +x "$elytra_binary" - - output "Creating Elytra data directories if needed..." - mkdir -p /var/lib/elytra/volumes /var/lib/elytra/archives /var/lib/elytra/backups - - output "Setting ownership on Elytra data directories..." - chown -R 8888:8888 /var/lib/elytra/volumes 2>/dev/null || true - chown -R 8888:8888 /var/lib/elytra/archives 2>/dev/null || true - chown -R 8888:8888 /var/lib/elytra/backups 2>/dev/null || true - chown -R 8888:8888 "$elytra_dir" 2>/dev/null || true - - output "Setting permissions on Elytra data directories..." - # Note: 777 is required for containerized game servers to access these directories - # Ensure parent /var/lib/elytra is accessible - chmod 755 /var/lib/elytra 2>/dev/null || true - # Ensure the volumes directory itself and all contents have 777 - chmod 777 /var/lib/elytra/volumes 2>/dev/null || true - chmod -R 777 /var/lib/elytra/volumes/* 2>/dev/null || true - chmod 777 /var/lib/elytra/archives 2>/dev/null || true - chmod -R 777 /var/lib/elytra/archives/* 2>/dev/null || true - chmod 777 /var/lib/elytra/backups 2>/dev/null || true - chmod -R 777 /var/lib/elytra/backups/* 2>/dev/null || true - chmod -R 755 "$elytra_dir" 2>/dev/null || true - - # Disable check_permissions_on_boot to prevent Elytra from resetting permissions - if [ -f "$elytra_dir/config.yml" ]; then - output "Disabling permission checks in Elytra config..." - sed -i 's/check_permissions_on_boot: true/check_permissions_on_boot: false/' "$elytra_dir/config.yml" 2>/dev/null || true - fi - + # Reuse the shared fix routine from lib.sh — same pattern as fix_wings_permissions + auto_fix_elytra_issues skip_restart success "Elytra permissions fixed" return 0 }This requires
auto_fix_elytra_issuesto exist in lib.sh. Verify first:#!/bin/bash # Check if auto_fix_elytra_issues wrapper exists in lib.sh rg -n 'auto_fix_elytra_issues' lib/ --type sh🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/repair.sh` around lines 130 - 201, Refactor fix_elytra_permissions to delegate to an auto_fix_elytra_issues wrapper, matching fix_wings_permissions. Verify whether auto_fix_elytra_issues exists in lib.sh; if absent, add it using the existing generic daemon-fix helper with skip_restart, then reduce fix_elytra_permissions to binary detection, delegation, success reporting, and return handling.ui/health-check.sh (1)
208-233: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
detect_wings_binarychecks two paths;check_wings_healthonly knows one.
detect_wings_binary()(lib.sh:348-360) checks both/usr/local/bin/wingsand/usr/bin/wings. Butcheck_wings_health()(lib.sh:4469-4550) hardcodes/usr/local/bin/wingsthroughout — its binary existence check, executable check, and version check all reference that single path. When Wings lives at/usr/bin/wings, this "Check All Installed Components" flow will detect Wings (line 217:has_wings=true), callcheck_wings_health(line 231), and produce a false "Wings binary not found at /usr/local/bin/wings" error withhas_errors=true.The same contract mismatch poisons
fix_wings_permissionsinui/repair.sh—auto_fix_wings_issues(lib.sh:4647-4649) hardcodes/usr/local/bin/wingsas the binary path into_auto_fix_daemon_issues, so thechmod +xstep silently skips the actual binary.Root cause is in lib.sh (not in this review scope), but the downstream impact originates from these new call sites. Fix
check_wings_healthandauto_fix_wings_issuesto accept a binary path parameter, or have them internally calldetect_wings_binaryinstead of hardcoding.🔧 Suggested fix for check_wings_health (lib.sh, outside review scope)
check_wings_health() { local has_errors=false + local wings_bin + wings_bin=$(detect_wings_binary 2>/dev/null) || { + error "Wings binary not found" + has_errors=true + } echo "" output "${COLOR_ORANGE}Wings Health Check${COLOR_NC}" echo "" # Check binary exists - if [ -f "/usr/local/bin/wings" ]; then - output "✓ Wings binary exists at /usr/local/bin/wings" + if [ -n "$wings_bin" ] && [ -f "$wings_bin" ]; then + output "✓ Wings binary exists at $wings_bin" # Check binary is executable - if [ -x "/usr/local/bin/wings" ]; then + if [ -x "$wings_bin" ]; then output "✓ Wings binary is executable" else warning "Wings binary is not executable" has_errors=true fi # Check binary version local version - version=$(/usr/local/bin/wings --version 2>/dev/null | head -1) + version=$("$wings_bin" --version 2>/dev/null | head -1) if [ -n "$version" ]; then output "✓ Wings version: $version" fi else - error "Wings binary not found at /usr/local/bin/wings" + error "Wings binary not found" has_errors=true fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/health-check.sh` around lines 208 - 233, Update the Wings health and repair call sites to use the binary path returned by detect_wings_binary instead of assuming /usr/local/bin/wings. Pass the detected path from the component-check flow into check_wings_health, and ensure auto_fix_wings_issues forwards the detected path to _auto_fix_daemon_issues so existence, executable, version, and permission checks target /usr/bin/wings when applicable.installers/auto-update-wings.sh (1)
656-702: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffA cursed doppelgänger of
lib.sh's_auto_fix_daemon_issues, doomed to drift.This body is a hand-copied twin of the freshly-refactored shared helper in
lib/lib.sh— same intent, subtly different flesh (here it'schmod 777 dir+chmod -R 777 dir/*globs; there it's the cleanerchmod -R 777 "$data_root/volumes"). Since this auto-updater is standalone and defines its ownoutput(), it can't cheaply sourcelib.sh, so I'm not demanding you slay it. But two copies of permission-repair logic will inevitably rot out of sync — the last regression here (the missingsed) proves it. Consider extracting a tiny sourced snippet or at least leave a breadcrumb comment pointing at the canonicallib.shversion so the next poor soul knows to update both graves.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installers/auto-update-wings.sh` around lines 656 - 702, Add a clear breadcrumb comment above the duplicated permission-repair block in the auto-update script identifying lib.sh’s _auto_fix_daemon_issues as the canonical implementation and instructing maintainers to keep both copies synchronized; retain the standalone behavior while aligning the commands with the shared helper where practical.lib/lib.sh (1)
3658-3678: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDrop the made-up
daemon_typepayload key.
daemon_typeis fine as a local label, but the node API doesn’t define it; sending it over the wire is either ignored fluff or a request rejection waiting to happen. Keep the JSON to documented node fields only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/lib.sh` around lines 3658 - 3678, Remove the daemon_type field from the JSON payload in both the jq construction and printf fallback within the node creation logic. Retain daemon_type for local labeling if needed, but do not pass it to either JSON serializer or send it to the node API; update the corresponding jq --arg and printf argument usage to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@installers/auto-update-wings.sh`:
- Around line 656-702: Add a clear breadcrumb comment above the duplicated
permission-repair block in the auto-update script identifying lib.sh’s
_auto_fix_daemon_issues as the canonical implementation and instructing
maintainers to keep both copies synchronized; retain the standalone behavior
while aligning the commands with the shared helper where practical.
In `@lib/lib.sh`:
- Around line 3658-3678: Remove the daemon_type field from the JSON payload in
both the jq construction and printf fallback within the node creation logic.
Retain daemon_type for local labeling if needed, but do not pass it to either
JSON serializer or send it to the node API; update the corresponding jq --arg
and printf argument usage to match.
In `@ui/health-check.sh`:
- Around line 208-233: Update the Wings health and repair call sites to use the
binary path returned by detect_wings_binary instead of assuming
/usr/local/bin/wings. Pass the detected path from the component-check flow into
check_wings_health, and ensure auto_fix_wings_issues forwards the detected path
to _auto_fix_daemon_issues so existence, executable, version, and permission
checks target /usr/bin/wings when applicable.
In `@ui/repair.sh`:
- Around line 181-201: fix_wings_permissions currently only displays the
detected binary path while auto_fix_wings_issues may ignore /usr/bin/wings. Add
an explicit chmod +x "$wings_binary" using the detected path, matching
fix_elytra_permissions, before invoking auto_fix_wings_issues; preserve the
existing shared routine and skip_restart behavior.
- Around line 130-201: Refactor fix_elytra_permissions to delegate to an
auto_fix_elytra_issues wrapper, matching fix_wings_permissions. Verify whether
auto_fix_elytra_issues exists in lib.sh; if absent, add it using the existing
generic daemon-fix helper with skip_restart, then reduce fix_elytra_permissions
to binary detection, delegation, success reporting, and return handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 339d8ce1-42ba-400f-83fe-9854aaeac166
📒 Files selected for processing (6)
installers/auto-update-wings.shinstallers/elytra.shlib/lib.shui/both.shui/health-check.shui/repair.sh
📜 Review details
⚠️ CI failures not shown inline (8)
GitHub Actions: Markdown Lint / Lint Markdown: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 364 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: Markdown Lint / 0_Lint Markdown.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 364 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: CI / 1_Executable Permissions.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / Bash Syntax Check: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / ShellCheck: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / Executable Permissions: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / 2_ShellCheck.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / 0_Bash Syntax Check.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
🧰 Additional context used
🪛 ast-grep (0.44.1)
lib/lib.sh
[warning] 4596-4596: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 "$data_root/volumes"
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 4597-4597: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 "$data_root/archives"
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
[warning] 4598-4598: Granting world-writable permissions (e.g. chmod 777, chmod -R 777, chmod a+w, chmod o+w) lets any local user modify the file or directory, enabling tampering and privilege escalation. Grant the least privilege needed instead, for example chmod 755 for executables/directories or chmod 644 for regular files, and use group ownership (chmod g+w with a dedicated group) when shared write access is required.
Context: chmod -R 777 "$data_root/backups"
Note: [CWE-732] Incorrect Permission Assignment for Critical Resource.
(chmod-world-writable-bash)
🔇 Additional comments (16)
ui/health-check.sh (2)
144-144: LGTM!
208-233: LGTM!ui/repair.sh (3)
254-266: LGTM!
76-76: LGTM!
494-535: 🎯 Functional CorrectnessNo change needed — option
0)is already handled. The0-8prompt matches the dispatch; this isn’t a bug.> Likely an incorrect or invalid review comment.lib/lib.sh (5)
4596-4599: The world-writable curse returns, but it was already sanctified.Static analysis once again wails about
chmod -R 777on the data dirs (CWE-732). This was already exorcised in prior review — the maintainer confirmed 777 is deliberate for arbitrary-UID game containers, and the ACL comment/incantation now honestly readsrwx. Noting it only so the linter's ghost doesn't get mistaken for a fresh haunting.
332-361: LGTM!
1014-1022: LGTM!Also applies to: 1105-1110
3916-3966: LGTM!
4647-4648: 🩺 Stability & AvailabilityUID 9999 is already Wings’ ownership. The
pterodactyluser/group is created with UID/GID9999in bothinstallers/wings.shandinstallers/both.sh, so thechown -R 9999:9999matches the install flow.> Likely an incorrect or invalid review comment.installers/auto-update-wings.sh (1)
676-681: World-writable again — same accepted pact aslib.sh.
chmod 777on the data dirs trips CWE-732, but this mirrors the maintainer-confirmed, deliberate choice for arbitrary-UID containers already litigated in prior review. No new sin here.installers/elytra.sh (3)
38-131: LGTM!Also applies to: 133-176
292-295: The three-headeduseraddfallback finally agrees on a GID.All three shell-fallback branches now carry
--gid 8888, so a user summoned via the/bin/falsepath no longer lands in a mismatched primary group. The prior ritual would've birthed an inconsistent user depending on which shell existed. Clean.
421-421: LGTM!ui/both.sh (2)
343-351: LGTM!
520-530: LGTM!
- ui/repair.sh: fix_wings_permissions() now chmod +x's the actually detected $wings_binary before calling auto_fix_wings_issues, instead of relying solely on auto_fix_wings_issues' own (previously hardcoded) binary path - matches fix_elytra_permissions()'s existing pattern. - lib.sh: check_wings_health() now calls detect_wings_binary() instead of hardcoding /usr/local/bin/wings, so Wings installs at /usr/bin/wings are correctly detected instead of reporting a false "binary not found". - lib.sh: auto_fix_wings_issues()/auto_fix_elytra_issues() now resolve the actual binary path via detect_wings_binary()/detect_elytra_binary() before calling the shared _auto_fix_daemon_issues() helper, falling back to the historical hardcoded path only if detection fails (so a chmod +x during repair/install no longer silently no-ops against a path the binary isn't actually at). No caller passes a binary_path argument directly, so this is a drop-in change for every call site (installers/both.sh, ui/repair.sh). - installers/auto-update-wings.sh: left its own hardcoded /usr/local/bin/wings as-is (that script always installs and updates Wings at that exact path by design, so there's nothing to detect) and added a comment marking lib.sh's auto_fix_wings_issues() as the canonical implementation this standalone copy must be kept in sync with, since it can't source lib.sh. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for the third pass — responding to the 4 "Outside diff range" findings from this review (no individually resolvable threads exist for these, same as the previous two rounds). Fixed in
|
Root cause (found via real-machine testing): create_node_via_api() always sent scheme: "https" in the node-creation JSON payload, regardless of whether the install actually configured SSL. On a Panel+Wings install with "SSL: None", `wings configure` fetches this node from the panel and defaults its ssl.enabled to true with a Let's Encrypt-style cert path that was never created, so Wings crashes on boot trying to load a certificate that doesn't exist. - lib.sh: create_node_via_api() gains a 10th node_scheme argument (default "https" for backward compatibility) and now sends it as the JSON "scheme" field instead of the hardcoded literal, in both the jq and printf-fallback code paths. - lib.sh: added daemon_scheme(), alongside the existing panel_scheme(), both now backed by a shared _scheme_from_ssl_vars() helper. They're separate named functions (not just one) because the same variable names (CONFIGURE_LETSENCRYPT/ASSUME_SSL/SSL_CERT_PATH/SSL_KEY_PATH) mean different things depending on which script reads them: the panel's own SSL in installers/both.sh/panel.sh, vs. a standalone daemon's own SSL in installers/wings.sh/elytra.sh. - installers/both.sh: create_node_in_panel() now passes panel_scheme() - a combined install has no separate daemon SSL flow, so Wings should match whatever the panel itself is using. - installers/wings.sh + installers/elytra.sh: auto_configure_wings()/ auto_configure_elytra() now pass daemon_scheme(), which reads each installer's own CONFIGURE_LETSENCRYPT/ASSUME_SSL/SSL_CERT_PATH/ SSL_KEY_PATH variables (already used for daemon-local SSL setup) rather than the panel's. - installers/wings.sh + installers/elytra.sh: as defense in depth, the post-'configure' SSL patching now also forces ssl.enabled: false in config.yml when no usable certificate was found, instead of silently leaving whatever 'wings configure'/'elytra configure' defaulted to from the node's scheme. Previously only the "cert found" branch touched ssl.enabled; the "no cert" branch left it untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
installers/auto-update-wings.sh (1)
653-657: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStop hardcoding
/usr/local/bin/wingsin the auto-fix path.
lib/lib.shalready resolves Wings withdetect_wings_binary()(/usr/local/bin/wingsor/usr/bin/wings), but this copy onlychmod +x’s/usr/local/bin/wings. If Wings lives in/usr/bin/wings, the “fix” misses it and blithely claims success anyway.
File Fix installers/auto-update-wings.sh:653-657resolve the binary path first, then chmod +xthat path🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installers/auto-update-wings.sh` around lines 653 - 657, Update the “Fix binary permissions” block in auto-update-wings.sh to call detect_wings_binary() and use its resolved path instead of hardcoding /usr/local/bin/wings. Check that the returned path exists before running chmod, and only report the permission fix when chmod targets the detected Wings binary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@installers/auto-update-wings.sh`:
- Around line 653-657: Update the “Fix binary permissions” block in
auto-update-wings.sh to call detect_wings_binary() and use its resolved path
instead of hardcoding /usr/local/bin/wings. Check that the returned path exists
before running chmod, and only report the permission fix when chmod targets the
detected Wings binary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdc811eb-251e-4333-80cd-b6a6b37409c9
📒 Files selected for processing (6)
installers/auto-update-wings.shinstallers/both.shinstallers/elytra.shinstallers/wings.shlib/lib.shui/repair.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
⚠️ CI failures not shown inline (8)
GitHub Actions: Markdown Lint / 0_Lint Markdown.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 364 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: Markdown Lint / Lint Markdown: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 364 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: CI / Executable Permissions: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / Bash Syntax Check: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / 1_Bash Syntax Check.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / 0_ShellCheck.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / 2_Executable Permissions.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / ShellCheck: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
🔇 Additional comments (10)
installers/elytra.sh (2)
421-421: LGTM!
647-651: LGTM!installers/wings.sh (2)
412-412: LGTM!
582-600: LGTM!ui/repair.sh (1)
181-204: LGTM!lib/lib.sh (4)
812-845: LGTM!
3613-3709: LGTM!
4508-4529: LGTM!
4673-4685: LGTM!installers/both.sh (1)
677-677: LGTM!
|
Hold on a sec—I'm running some detailed tests right now, |
|
ok |
|
btw. in the future you can also create the branches directly on this repo—but I think you cannot directly commit to main |
…gs auto-updater
Both found via real-machine testing of installers/auto-update-wings.sh.
1. INSTALL_DIR variable name collision
lib.sh exports INSTALL_DIR="/var/www/hydrodactyl" (the panel's dir).
installers/auto-update-wings.sh used the same name with a
${INSTALL_DIR:-/etc/pterodactyl} default - since install.sh's "Update
Wings Daemon" menu spawns this script via get_script() as a child
process (bash <(curl ...)), it inherits that exported panel path,
silently overriding the intended default. Every use of the variable
(health-check-failure log, config.yml checks, the chown/find calls in
auto_fix_wings_issues()) then operated on the panel's directory instead
of /etc/pterodactyl.
Renamed to WINGS_INSTALL_DIR throughout the file - matches the naming
already used for the same purpose in installers/both.sh/wings.sh.
Checked the other installer scripts for the same pattern:
- installers/wings.sh, installers/both.sh: their WINGS_INSTALL_DIR is a
plain literal assignment (no ${VAR:-default}), so no collision risk.
- installers/elytra.sh: already uses ELYTRA_INSTALL_DIR (with a comment
explicitly noting the same collision reasoning) - already safe.
- installers/auto-update-panel.sh: uses the same INSTALL_DIR name and
${INSTALL_DIR:-/var/www/hydrodactyl} pattern, but since this script's
own purpose is updating the panel, the inherited value happens to
match its own default - not an actual bug.
- installers/auto-update-elytra.sh: had the identical
INSTALL_DIR="${INSTALL_DIR:-/etc/elytra}" pattern as the Wings script.
Renamed to ELYTRA_INSTALL_DIR to match. Unlike Wings, install.sh has
no "Update Elytra Daemon" menu entry that spawns this script from an
already lib.sh-sourced shell, so this specific collision isn't
reachable via any call path currently in the repo - fixed anyway for
consistency with installers/elytra.sh's own established naming and
as a defensive fix against future call sites.
2. Rollback "Text file busy"
Both rollback branches (Wings failed to start after update; health
check still failing after auto-fix) called `cp` directly onto
/usr/local/bin/wings without first stopping the service. In the
health-check rollback specifically, start_wings had just succeeded, so
Wings (running the new binary) was definitely still active when cp
tried to overwrite it in place, guaranteeing ETXTBSY.
- stop_wings() now always issues `systemctl stop wings` instead of only
when systemctl already reports the unit active - wings.service has
Restart=on-failure (configs/wings.service), so a crash-looping daemon
can sit in a transient state that `is-active --quiet` doesn't count
as active, which previously made stop_wings() skip the stop
entirely. It now also warns if the unit is still active after the
stop instead of silently proceeding.
- Extracted the duplicated rollback logic into restore_wings_backup(),
which calls stop_wings() first, then restores via a temp file + mv
(atomic replace) instead of cp, mirroring the same safe-replace
pattern perform_update() already uses for installing the new binary.
Both rollback call sites now just call this helper.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cd9d07d
|
I've made the correction. |
…reen check_installations() now also reads WINGS_VARIANT from /etc/hydrodactyl/auto-update-wings.env (chosen over install-info/wings-info: install.sh's own run_wings_update() self-heals this file whenever it's missing, and it doesn't carry GITHUB_TOKEN like wings-info does, so there's nothing sensitive to avoid pulling into this shell). show_welcome() uses it to show "Wings-RS installed (...)" instead of "Wings installed (...)" when WINGS_VARIANT is "rs", and falls back to the existing "Wings" label with no error when the variant is unknown (older installs predating variant tracking, or a missing env file). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
install.sh (1)
268-283: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix
install.sh:268-283to usedetect_wings_binaryA valid
/usr/bin/wingsinstall is treated as absent here, so options 4/5 and the Wings health-check branch disappear even though the shared detector already supports that path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@install.sh` around lines 268 - 283, Update the Wings installation detection around WINGS_INSTALLED to call the existing detect_wings_binary helper instead of checking only /usr/local/bin/wings. Preserve the subsequent version and variant loading behavior, and ensure valid /usr/bin/wings installations set WINGS_INSTALLED so options 4/5 and the health-check branch remain available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@installers/wings.sh`:
- Around line 582-600: Scope the SSL enabled-state edits in the config.yml
handling block to api.ssl.enabled instead of replacing any bare enabled:
true/false entry. Update both branches around the existing certificate and key
substitutions so unrelated settings such as throttles.enabled remain unchanged
while the SSL state is correctly forced on or off.
---
Outside diff comments:
In `@install.sh`:
- Around line 268-283: Update the Wings installation detection around
WINGS_INSTALLED to call the existing detect_wings_binary helper instead of
checking only /usr/local/bin/wings. Preserve the subsequent version and variant
loading behavior, and ensure valid /usr/bin/wings installations set
WINGS_INSTALLED so options 4/5 and the health-check branch remain available.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1d7ac2af-e819-4559-ac53-b608c6de798a
📒 Files selected for processing (8)
install.shinstallers/auto-update-elytra.shinstallers/auto-update-wings.shinstallers/both.shinstallers/elytra.shinstallers/wings.shlib/lib.shui/repair.sh
📜 Review details
⚠️ CI failures not shown inline (8)
GitHub Actions: Markdown Lint / 0_Lint Markdown.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 364 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: Markdown Lint / Lint Markdown: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run DavidAnson/markdownlint-cli2-action@v19
with:
globs: *.md
manual-guides/**/*.md
separator:
##[endgroup]
markdownlint-cli2 v0.17.2 (markdownlint v0.37.4)
Finding: *.md manual-guides/**/*.md
Linting: 10 file(s)
Summary: 364 error(s)
##[error]CODE_OF_CONDUCT.md:116:1 MD034/no-bare-urls Bare URL used [Context: "https://www.contributor-covena..."] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md034.md
GitHub Actions: CI / Bash Syntax Check: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / 1_ShellCheck.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / 0_Bash Syntax Check.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! bash -n "$file"; then�[0m
�[36;1m echo "::error file=$file::Syntax error in $file"�[0m
GitHub Actions: CI / 2_Executable Permissions.txt: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / Executable Permissions: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run failed=0
�[36;1mfailed=0�[0m
�[36;1mwhile IFS= read -r -d '' file; do�[0m
�[36;1m if ! test -x "$file"; then�[0m
�[36;1m echo "::error file=$file::$file is not executable"�[0m
GitHub Actions: CI / ShellCheck: fix: clean up remaining Elytra wording and add Wings parity to repair/health-check/view-info
Conclusion: failure
##[group]Run exit 1
�[36;1mexit 1�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
##[error]Process completed with exit code 1.
🔇 Additional comments (16)
lib/lib.sh (4)
812-846: LGTM!
3613-3709: LGTM!
4673-4685: LGTM!
4526-4527: 📐 Maintainability & Code QualityNo stale path message here.
detect_wings_binary()only checks/usr/local/bin/wingsand/usr/bin/wings, so the error string matches the detector; the same wording inui/repair.shis consistent too.> Likely an incorrect or invalid review comment.installers/elytra.sh (2)
421-421: LGTM!
647-651: LGTM!installers/wings.sh (1)
412-412: LGTM!installers/both.sh (1)
677-677: LGTM!installers/auto-update-elytra.sh (2)
31-36: LGTM!
512-551: LGTM!installers/auto-update-wings.sh (4)
32-37: LGTM!
299-316: LGTM!
346-377: LGTM!
674-736: LGTM!install.sh (1)
257-257: LGTM!Also applies to: 309-312
ui/repair.sh (1)
181-204: LGTM!
- install.sh: check_installations() now calls detect_wings_binary() (checks both /usr/local/bin/wings and /usr/bin/wings) instead of hardcoding the first path, so WINGS_INSTALLED is set correctly on /usr/bin/wings installs too - this flag gates the "Update Wings Daemon"/"Update both" menu options and the Wings health-check branches, all of which now work correctly for those installs. - installers/wings.sh: the "enabled: true/false" sed toggles for config.yml's api.ssl block were unscoped bare patterns, so they could also match unrelated "enabled:" keys elsewhere in the file (e.g. throttles.enabled). Scoped both to the /ssl:/,/certificate:/ address range, matching the api.ssl block's actual boundaries (verified against the config.yml structure documented in manual-guides/both-same-machine.md and elytra-manual.md). Tested against a realistic sample config.yml with throttles.enabled set to both true and false in both the enable and disable directions - confirmed only api.ssl.enabled ever changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@itzzjustmateo |
|
If CodeRabbit says it is fine, then you can merge it yourself |
2 participants
Summary
Follow-up to PR #3's Wings/Wings-RS work: an inventory of the whole repo for leftover "Elytra" wording turned up several places where Wings-only install flows still displayed/sent "Elytra" (real bugs, not just cosmetic), three maintenance tools with no Wings support at all, and a handful of documentation gaps. This PR addresses all of them across two commits.
Priority 1 — real-harm bugs (user-visible or functional)
select_release_version()and its Wings/combined-installer call sites hardcoded "Elytra" wording, so choosing Wings or Wings-RS displayed"Elytra is installed from binary releases."create_node_via_api()always sentdaemon_type: "elytra"to the panel API regardless of which daemon was actually being installed; it now accepts adaemon_typeparameter and sends"wings"for Wings/Wings-RS installs,"elytra"for legacy installs, with node name/description labels following suit.installers/both.sh'sinstall_wings_daemon()(Wings-only) printed "Elytra" throughout its config/SSL/permission/completion output, most visiblysuccess "Elytra installed and started"at the very end of a Wings-only install.ELYTRA_DIRrenamed toWINGS_INSTALL_DIRfor clarity.ui/both.sh's Wings/Wings-RS configuration flow (variables, function names, welcome banner) was never renamed offELYTRA_*/configure_elytra_settingswhen the Wings/Wings-RS variant picker was added.installers/wings.shcalledsave_wings_install_info/show_wings_completion, which didn't exist anywhere in the repo — this crashed the installer right after a successful Wings install. Added the missing functions (save/load/exists/display_wings_install_info,show_wings_completion,auto_fix_wings_issues) tolib.sh;installers/both.shwas also misusing the Elytra-only versions on its Wings install.installers/panel.sh's post-install API key messaging assumed Elytra and told the user to runelytra.sh, even for Wings setups.install.sh'sELYTRA_INSTALLED/ELYTRA_VERSION/run_elytra_updateactually tracked Wings (checked/usr/local/bin/wings), not Elytra — renamed toWINGS_INSTALLED/WINGS_VERSION/run_wings_updatefor clarity (displayed text was already correct, only the internal names were backwards).installers/auto-update-wings.sh'sINSTALL_DIRdefaulted to/etc/wingsand several data-directory checks were hardcoded to/var/lib/wings, neither of which Wings actually uses (the real path is/etc/pterodactyl//var/lib/pterodactyl/*) — its health check and auto-fix logic were checking/repairing the wrong directories. Confirmedauto-update-elytra.shhas no equivalent issue.installers/both.shignoredWINGS_RELEASE_VERSIONentirely and always installed latest, even thoughui/both.shlets the user pick a specific version — wired it through, mirroringinstallers/wings.sh's existing logic.Priority 2 — Wings parity for maintenance tools
ui/repair.sh,ui/health-check.sh, andui/view-info.shonly ever knew about legacy Elytra (no Wings detection, no "Fix Wings Permissions", no "Check Wings Health", no Wings install-info display), despite Wings being the current daemon with full support elsewhere in the codebase. Added Wings/Wings-RS detection and actions to all three, reusing existing shared helpers (check_wings_health(), the newauto_fix_wings_issues()) rather than duplicating logic.Phase 2 — documentation and remaining cleanup
both-same-machine.mdwhere Step 5 creates data directories under/var/lib/hydrodactyl/...but the next commandschown/chmod/var/lib/elytra/...— a path that was never created, so the actual data directories never got their ownership/permissions set. Addedmanual-guides/wings-manual.md(previously there was no manual Wings walkthrough at all, only Elytra's), and wired deprecation-aware Wings signposting into the other three guides..github/ISSUE_TEMPLATE/: added Wings/Wings-RS options to the bug report's installation-type dropdown and contact links, which previously only offered Panel/Elytra.--skip-wings-setup/SKIP_WINGS_SETUPflag frominstallers/elytra.sh(never read anywhere, a copy-paste leftover fromwings.sh).ui/uninstall.sh's auto-updater detection only checked forhydrodactyl-elytra-auto-update.timer, missing a Wings-only install'shydrodactyl-wings-auto-update.timer; renamed the misleadingELYTRA_UPDATER_INSTALLEDtoDAEMON_UPDATER_INSTALLEDand check both.Note on
daemon_typecreate_node_via_api()now sendsdaemon_type: "wings"for Wings/Wings-RS nodes (previously always"elytra"). This value is inferred from this codebase's own internal component-identifier convention ("panel"/"elytra"/"wings"used throughout) since the panel's source isn't in this repo — the exact string the panel's API schema expects for Wings has not been independently verified against panel source. Please confirm against the actual Hydrodactyl/Pterodactyl paneldaemon_typeenum before merging, or point me at the relevant panel code/docs and I'll adjust.Test plan
bash -non every modified.shfile (all pass)shellcheckwas not available in this environment — recommend running it in CI/locally before mergeui/wings.sh→installers/wings.sh) to confirm thesave_wings_install_info/show_wings_completionfix resolves the post-install crashui/repair.sh,ui/health-check.sh,ui/view-info.shagainst a real Wings installdaemon_type: "wings"against the actual panel API schema (see note above)Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com