Samba: Restrict enabled_shares schema to list() of share names#4718
Open
kingpanther13 wants to merge 4 commits into
Open
Samba: Restrict enabled_shares schema to list() of share names#4718kingpanther13 wants to merge 4 commits into
kingpanther13 wants to merge 4 commits into
Conversation
Values have always been validated case-insensitively and lower-cased at runtime before rendering smb.conf. Persisting the canonical lower-case form prepares for a future release that restricts the schema to the exact share names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The configuration UI offers the share names as a multi-select instead of an empty free-text suggestion list. Values must now be lower case; 12.7.2 migrated stored configs automatically. The startup migration and runtime lower-casing are removed as dead code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 11, 2026
With the list() schema the Supervisor rejects non-lower-case values before the add-on starts, so the startup migration from 12.8.1 can never run against a config that needs it.
kingpanther13
marked this pull request as ready for review
July 15, 2026 20:39
Contributor
📝 WalkthroughWalkthroughThe Samba add-on version advances to 13.0.0, ChangesEnabled shares validation
Estimated code review effort: 3 (Moderate) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@samba/CHANGELOG.md`:
- Around line 7-9: Update the migration version reference in the changelog entry
from 12.8.1 to 12.7.2, preserving the surrounding explanation that
configurations from that version onward migrate automatically.
🪄 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: CHILL
Plan: Pro
Run ID: 7ec7f75b-7aaf-4527-ae81-0c8bf03742d5
📒 Files selected for processing (4)
samba/CHANGELOG.mdsamba/config.yamlsamba/rootfs/etc/s6-overlay/s6-rc.d/init-smbd/runsamba/translations/en.yaml
💤 Files with no reviewable changes (1)
- samba/rootfs/etc/s6-overlay/s6-rc.d/init-smbd/run
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Step 2 of the plan from home-assistant/supervisor#7023: switch
enabled_sharesfrommatch(^(?i:(...))$)tolist(addons|addon_configs|backup|config|media|share|ssl). The configuration UI then offers the valid share names as a populated multi-select instead of an empty free-text suggestion list, resolving home-assistant/frontend#51510.BREAKING: values must be exactly lower case. Configs saved on 12.7.2+ (#4717) were migrated automatically; older configs with non-lower-case values fail validation with a clear message. The step-1 startup migration and the runtime lower-casing are removed as dead code.
Stacked on #4717 (the diff includes it until that PR merges). Opened as a draft so maintainers can schedule/bundle it with other breaking changes (e.g. the NetBIOS default change mentioned in the supervisor discussion).
Summary by CodeRabbit
Breaking Changes
Documentation
Update (2026-07-15): #4717 merged; the step-1 migration shipped as 12.8.1 (the 12.7.2 references above predate the renumbering). This PR is no longer stacked — the diff is step 2 only, rebased on current master. Configs saved on 12.8.1 or later were migrated automatically; older configs with non-lower-case values fail validation with a clear message. The 12.8.1 startup migration and the runtime lower-casing are removed as dead code: with the strict schema, validation rejects a non-migrated config before the add-on can start, so the migration could never run when it was needed.