update.php fixes

[elibosley]

Restrict update.php #file writes to the expected configuration roots (/boot/config, /etc/wireguard) so a write cannot land outside config storage via an absolute path or ...

• Resolve the target with realpath() (collapsing ../symlinks) and range-check it with a shared in_safe_path() helper.
• Promote in_safe_path() into Wrappers.php so update.php and FileUpload.php share one implementation (drops the duplicate copy in FileUpload.php).
• Add a regression test that drives the real update.php and asserts out-of-root targets are refused while legitimate /boot/config writes still succeed.

Relative #file paths are unaffected (still placed under /boot/config/plugins/).

Summary by CodeRabbit

• Bug Fixes
  • Strengthened validation for command execution requests so only paths inside the allowed document root are accepted.
  • Requests with invalid or out-of-scope command paths are now rejected and logged, reducing the risk of unintended file access.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 87cc3b49-8d08-4175-8540-05133860443b

📥 Commits

Reviewing files that changed from the base of the PR and between dd727fd and a0a37dc.

📒 Files selected for processing (1)

• emhttp/update.php

---

Walkthrough

In emhttp/update.php, the #command validation is tightened: the script now rejects the command not only when realpath() returns false, but also when the resolved command path falls outside the resolved $docroot subtree.

Changes

Command Path Confinement

Layer / File(s)	Summary
Docroot confinement guard for #command emhttp/update.php	Rejection condition for #command extended to also fail when the resolved command lies outside the resolved $docroot prefix, not just when realpath() returns false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

A bunny checked the path one day,
"Does this command go astray?"
Outside docroot? No no no!
Back in bounds is where you go.
🐇 Safe and snug, hooray! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is too generic and does not describe the specific update.php path-validation change.	Rename it to mention the main fix, such as restricting update.php #command handling to the document-root subtree.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch update-php-file-fixes

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2026.06.29.2026
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

1. Go to Plugins → Install Plugin
2. Copy and paste this URL:

https://preview.dl.unraid.net/pr-plugins/pr-2684/webgui-pr-2684.plg

3. Click Install

Alternative: Direct Download

• 📦 Download PLG
• 📦 Download TXZ

⚠️ Important Notes:

• Testing only: This plugin is for testing PR changes
• Backup included: Original files are automatically backed up
• Easy removal: Files are restored when plugin is removed
• Conflicts: Remove this plugin before installing production updates
• Post-merge behavior: This preview stays available after merge until preview storage expires or it is manually cleaned up

📝 Modified Files:

Click to expand file list

emhttp/update.php

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2684, or run:

plugin remove webgui-pr-2684

---

_{🤖 This comment is automatically generated and will be updated with each new push to this PR.}

[coderabbitai]

Actionable comments posted: 3

🤖 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 `@emhttp/plugins/dynamix/agents/tests/update_php_file_confinement_test.sh`:
- Around line 27-32: The confinement test is swallowing real `update.php`
execution failures by discarding all output and forcing success with `|| true`.
Update `update_php_file_confinement_test.sh` so the PHP probe around `include
"update.php"` still treats denied writes as passing, but surfaces non-zero PHP
exits when PHP is missing, `update.php` fatals, or the include setup breaks;
keep the `update.php` invocation path intact and remove the unconditional
success masking.
- Around line 35-50: The traversal assertion in assert_blocked is checking the
literal escaped path string instead of the normalized destination, so it can
miss files created after path resolution. Update assert_blocked to accept a
separate path to verify, and use that for the relative '..' traversal case while
keeping run_update pointed at the original traversal input. Make sure the check
in update_php_file_confinement_test.sh validates the actual resolved target
created by update.php, not the unnormalized literal.

In `@emhttp/update.php`:
- Line 70: The path resolution in update.php only falls back to the parent and
grandparent, so it can reject valid targets that are deeper under an allowed
root. Update the ancestor lookup around the $resolved_dir assignment to walk
upward from dirname($file) until the nearest existing directory is found, rather
than stopping after two checks. Keep the logic localized to the existing
resolution flow in update.php so the allowed-root validation continues to use
the resolved ancestor.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 473a14b1-dc08-4c48-b066-d95f59e65103

📥 Commits

Reviewing files that changed from the base of the PR and between bccf49e and 8b3ceda.

📒 Files selected for processing (4)

• emhttp/plugins/dynamix/agents/tests/update_php_file_confinement_test.sh
• emhttp/plugins/dynamix/include/FileUpload.php
• emhttp/plugins/dynamix/include/Wrappers.php
• emhttp/update.php

[elibosley]

Verifying this change (not committed)

One-line hardening of update.php's #command handler: the realpath-resolved command must stay inside the document root before it runs, so a ../-escape can't execute a binary outside the webgui.

Offline check from the repo root — should print blocked:

php -d short_open_tag=On -r '
  error_reporting(0);
  $_SERVER["DOCUMENT_ROOT"] = getcwd()."/emhttp";
  $_POST = ["#command" => "../../../../../../bin/echo", "#arg" => ["PWNED"]];
  chdir("emhttp"); include "update.php";
' | grep -q PWNED && echo "NOT contained" || echo "blocked"

On a running server: an authenticated POST with #command=../../../../../../bin/touch + #arg[]=/tmp/x must not create /tmp/x, and syslog logs Invalid #command: .... Legit #command scripts under /webGui/scripts and /plugins/*/scripts still run.