Fix: Disallow reading settings from share containing apostrophe

[Squidly271]

Summary by CodeRabbit

• New Features

  • Added a warning in SMB settings when a share’s security is set to Public, updating automatically on load and when changed.

• Bug Fixes

  • NFS and SMB share selectors now exclude shares with apostrophes in their names to avoid display and selection issues.
  • Share editing filters aligned to prevent listing shares with apostrophes, ensuring consistent behavior across read/write and security options.

[coderabbitai]

Walkthrough

Adds filtering to exclude shares with apostrophes in their names across NFS, SMB, and Share Edit pages. In SMB, introduces a client-side checkPublicSelection function to toggle a public-security warning on load and on selection change. No public APIs changed.

Changes

Cohort / File(s)	Summary
NFS share filtering emhttp/plugins/dynamix/SecurityNFS.page	Adds guard to skip shares whose names contain a single quote (') in read/write NFS option iterations.
SMB UI warning and filtering emhttp/plugins/dynamix/SecuritySMB.page	Adds checkPublicSelection(select) to show/hide a warning when SMB security is set to public; invoked on DOMContentLoaded and on select change. Filters out shares with apostrophes across SMB option lists.
Share selection filtering emhttp/plugins/dynamix/ShareEdit.page	Tightens share filtering to exclude names containing a single quote (') during share selection logic.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant DOM as DOM
  participant SMBPage as SecuritySMB.page
  participant UI as Warning Banner

  User->>DOM: Load SMB Security page
  DOM->>SMBPage: DOMContentLoaded
  SMBPage->>SMBPage: checkPublicSelection(shareSecurity)
  alt Security = Public
    SMBPage->>UI: Show warning
  else Security ≠ Public
    SMBPage->>UI: Hide warning
  end

  User->>SMBPage: Change shareSecurity
  SMBPage->>SMBPage: checkPublicSelection(shareSecurity)
  alt Security = Public
    SMBPage->>UI: Show warning
  else
    SMBPage->>UI: Hide warning
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through shares, apostrophes trimmed,
Tidying lists where quirks had brimmed.
A gentle squeak: “Public? Beware!”
A banner pops—proceed with care.
With paws on keys and whiskers straight,
I ship this patch—secure and great! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly describes the central change: preventing reading settings from shares whose names contain an apostrophe. The diffs show added filters in SecurityNFS.page, SecuritySMB.page, and ShareEdit.page to exclude names with a single quote, so the title accurately reflects the primary intent and is concise.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/shares

---

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

[github-actions]

🔧 PR Test Plugin Available

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

Version: 2025.09.23.2246
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-2397/webgui-pr-2397.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

📝 Modified Files:

Click to expand file list

emhttp/plugins/dynamix/SecurityNFS.page
emhttp/plugins/dynamix/SecuritySMB.page
emhttp/plugins/dynamix/ShareEdit.page

🔄 To Remove:

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

plugin remove webgui-pr-2025.09.23.2246

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

emhttp/plugins/dynamix/SecuritySMB.page (1)

384-403: checkPublicSelection: add a small null-guard for robustness (optional).

If the selector ever fails (edge templates), avoid a potential NPE.

Apply this minimal diff:

 document.addEventListener("DOMContentLoaded", function() {
-  let smbSecuritySelect = document.querySelector('[name="shareSecurity"]');
-  checkPublicSelection(smbSecuritySelect);
+  const smbSecuritySelect = document.querySelector('[name="shareSecurity"]');
+  if (smbSecuritySelect) checkPublicSelection(smbSecuritySelect);
 });

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7caf24 and ae0d70c.

📒 Files selected for processing (3)

• emhttp/plugins/dynamix/SecurityNFS.page (2 hunks)
• emhttp/plugins/dynamix/SecuritySMB.page (6 hunks)
• emhttp/plugins/dynamix/ShareEdit.page (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (3)

emhttp/plugins/dynamix/ShareEdit.page (1)

329-331: Apostrophe-filter in clone lists looks correct; strict === false avoids 0-index trap. Confirm scope (write targets) is intended.

This filter feeds both the “Read settings from” and “Write settings to” lists. If the goal was only to block reading from apostrophe-named shares, hiding them from write targets is a broader behavior change. Intentional?

emhttp/plugins/dynamix/SecurityNFS.page (1)

34-35: Consistent apostrophe-filter in NFS read/write lists — LGTM.

Filtering via strpos($list['name'],"'") === false is safe and does what we want.

If the original requirement is only “disallow reading,” confirm that excluding apostrophe-named shares from write targets is also desired.

Also applies to: 51-52

emhttp/plugins/dynamix/SecuritySMB.page (1)

37-38: Apostrophe-filter applied across SMB read/write and user-access lists — LGTM.

Matches NFS and ShareEdit behavior. As with other pages, please confirm the broader scope (blocking write targets too) is intentional.

Also applies to: 54-55, 157-158, 174-175, 220-221, 237-238

[github-actions]

🧹 PR Test Plugin Cleaned Up

The test plugin and associated files for this PR have been removed from the preview environment.

---

_{🤖 This comment is automatically generated when a PR is closed.}