fix: improve initial setup and dashboard status#27
Conversation
📝 WalkthroughWalkthroughChangesThe plugin now defaults to disabled, preserves enablement during upgrades, gates startup and settings validation, improves dashboard states, reloads Nginx after NetBird enablement lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant Settings
participant Apply
participant NetBird
participant Nginx
Admin->>Settings: enable or disable NetBird
Settings->>Apply: submit configuration
Apply->>NetBird: apply settings
NetBird-->>Apply: wt0 becomes ready
Apply->>Nginx: reload WebGUI
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
🧹 Nitpick comments (1)
scripts/build.sh (1)
65-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDefend against filenames starting with a dash.
When expanding globs into command arguments, it is good practice to use
--to prevent filenames starting with a dash from being interpreted as options by the command.♻️ Proposed refactor
- tar --owner=0 --group=0 -cJf "${DIST_DIR}/${PKG_FILE}" "${package_entries[@]}" + tar --owner=0 --group=0 -cJf "${DIST_DIR}/${PKG_FILE}" -- "${package_entries[@]}"🤖 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 `@scripts/build.sh` at line 65, Update the tar invocation in the package archive step to include the option terminator before the expanded package_entries arguments, ensuring filenames beginning with a dash are treated as archive entries rather than command options.
🤖 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 `@src/usr/local/emhttp/plugins/netbird/include/action.php`:
- Around line 358-367: Reorder the configuration writes in the surrounding
action handler so Netbird\writeProfileCfg($name, $creds) completes successfully
before Netbird\writeGlobalCfg($globalCfg) persists ENABLE_NETBIRD=1. Preserve
the existing HTTP 500 responses and error messages, and ensure a profile-write
failure exits before the global configuration is written.
In `@src/usr/local/emhttp/plugins/netbird/scripts/apply.sh`:
- Around line 50-64: Update reload_nginx_when_wt0_ready so its polling condition
requires wt0 to have an assigned IPv4 address, rather than only verifying that
the interface exists. Keep the existing retry loop and reload behavior unchanged
once an address is detected.
---
Nitpick comments:
In `@scripts/build.sh`:
- Line 65: Update the tar invocation in the package archive step to include the
option terminator before the expanded package_entries arguments, ensuring
filenames beginning with a dash are treated as archive entries rather than
command options.
🪄 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: 9a104ee3-d512-42b2-95e3-d72afd82ace0
📒 Files selected for processing (11)
README.mdplugin/ca_template.xmlplugin/netbird.plgscripts/build.shsrc/install/doinst.shsrc/usr/local/emhttp/plugins/netbird/Netbird-1-Settings.pagesrc/usr/local/emhttp/plugins/netbird/Netbird_dashboard.pagesrc/usr/local/emhttp/plugins/netbird/default.cfgsrc/usr/local/emhttp/plugins/netbird/include/action.phpsrc/usr/local/emhttp/plugins/netbird/scripts/apply.shsrc/usr/local/etc/rc.d/rc.netbird
| if (!Netbird\writeGlobalCfg($globalCfg)) { | ||
| http_response_code(500); | ||
| echo json_encode(['type' => 'error', 'message' => 'Could not write the NetBird global configuration.']); | ||
| break; | ||
| } | ||
|
|
||
| if (!Netbird\writeProfileCfg($name, $creds)) { | ||
| if ($enable === '1' && !Netbird\writeProfileCfg($name, $creds)) { | ||
| http_response_code(500); | ||
| echo json_encode(['type' => 'error', 'message' => "Could not write profile '$name'."]); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist the profile before enabling NetBird globally.
If writeProfileCfg() fails, ENABLE_NETBIRD=1 remains saved despite the reported failure. A later array start can therefore launch NetBird with missing or stale credentials.
Proposed fix
- if (!Netbird\writeGlobalCfg($globalCfg)) {
- http_response_code(500);
- echo json_encode(['type' => 'error', 'message' => 'Could not write the NetBird global configuration.']);
- break;
- }
if ($enable === '1' && !Netbird\writeProfileCfg($name, $creds)) {
http_response_code(500);
echo json_encode(['type' => 'error', 'message' => "Could not write profile '$name'."]);
break;
}
+ if (!Netbird\writeGlobalCfg($globalCfg)) {
+ http_response_code(500);
+ echo json_encode(['type' => 'error', 'message' => 'Could not write the NetBird global configuration.']);
+ break;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!Netbird\writeGlobalCfg($globalCfg)) { | |
| http_response_code(500); | |
| echo json_encode(['type' => 'error', 'message' => 'Could not write the NetBird global configuration.']); | |
| break; | |
| } | |
| if (!Netbird\writeProfileCfg($name, $creds)) { | |
| if ($enable === '1' && !Netbird\writeProfileCfg($name, $creds)) { | |
| http_response_code(500); | |
| echo json_encode(['type' => 'error', 'message' => "Could not write profile '$name'."]); | |
| break; | |
| } | |
| if ($enable === '1' && !Netbird\writeProfileCfg($name, $creds)) { | |
| http_response_code(500); | |
| echo json_encode(['type' => 'error', 'message' => "Could not write profile '$name'."]); | |
| break; | |
| } | |
| if (!Netbird\writeGlobalCfg($globalCfg)) { | |
| http_response_code(500); | |
| echo json_encode(['type' => 'error', 'message' => 'Could not write the NetBird global configuration.']); | |
| break; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 359-359: Avoid side effects in a file that defines symbols
Context: echo json_encode(['type' => 'error', 'message' => 'Could not write the NetBird global configuration.']);
Note: [CWE-710] Improper Adherence to Coding Standards.
(no-side-effect)
[warning] 364-364: Avoid side effects in a file that defines symbols
Context: echo json_encode(['type' => 'error', 'message' => "Could not write profile '$name'."]);
Note: [CWE-710] Improper Adherence to Coding Standards.
(no-side-effect)
🤖 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 `@src/usr/local/emhttp/plugins/netbird/include/action.php` around lines 358 -
367, Reorder the configuration writes in the surrounding action handler so
Netbird\writeProfileCfg($name, $creds) completes successfully before
Netbird\writeGlobalCfg($globalCfg) persists ENABLE_NETBIRD=1. Preserve the
existing HTTP 500 responses and error messages, and ensure a profile-write
failure exits before the global configuration is written.
| # Once a first-time connection creates wt0, reload nginx so the Unraid WebGUI | ||
| # binds to the NetBird address added to network-extra.cfg by the installer. The | ||
| # watcher closes the apply lock descriptor so it cannot delay another operation. | ||
| reload_nginx_when_wt0_ready() { | ||
| ( | ||
| exec 9>&- | ||
| for _ in $(seq 1 15); do | ||
| if ip -4 addr show wt0 >/dev/null 2>&1; then | ||
| /etc/rc.d/rc.nginx reload >/dev/null 2>&1 | ||
| exit 0 | ||
| fi | ||
| sleep 2 | ||
| done | ||
| ) >/dev/null 2>&1 </dev/null & | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wait for an IPv4 address, not merely the wt0 device.
ip -4 addr show wt0 succeeds when the interface exists even if address assignment is still pending. The watcher can reload Nginx too early and then exit without retrying.
Proposed fix
- if ip -4 addr show wt0 >/dev/null 2>&1; then
+ if ip -4 -o addr show dev wt0 | grep -q ' inet '; then
/etc/rc.d/rc.nginx reload >/dev/null 2>&1
exit 0
fiAlso applies to: 230-230
🤖 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 `@src/usr/local/emhttp/plugins/netbird/scripts/apply.sh` around lines 50 - 64,
Update reload_nginx_when_wt0_ready so its polling condition requires wt0 to have
an assigned IPv4 address, rather than only verifying that the interface exists.
Keep the existing retry loop and reload behavior unchanged once an address is
detected.
1 participant
Description
Fixes #25.
Problem
A fresh install starts NetBird before a profile has been configured. Without a setup key the daemon cannot connect, but the Settings page also blocks saving the disabled state because it validates the setup key first.
The dashboard then shows the expected authentication failure as a large error, which makes an unconfigured install look broken instead of simply waiting for setup.
Changes
netbird.cfgkeep their previous enabled behavior.DisabledorAwaiting configuration, hides the expected pre-setup authentication error, and wraps unexpected errors so they cannot stretch the tile.wt0first appears so the WebGUI becomes available on the NetBird address after initial setup.While testing the upgrade, I also found that fallback-built packages stored files with a
./prefix. Unraid did not recognize./install/doinst.shduring upgrades, so the event hooks disappeared and/installwas left behind. The fallback builder now uses proper Slackware paths and rejects packages with a misplaced install script or prefixed payload paths.Testing
Tested on a live Unraid 7.3.2 server with NetBird 0.74.4:
2026.07.14.1016; NetBird restarted, reconnected with the same identity and address, and preserved the config and profile hashes.rc.netbird startstays stopped while disabled and the dashboard showsDisabled.doinst.sh, recreated the event hooks, stored the install script in pkgtools, and left no/installdirectory behind.git diff --check, and package-layout validation.Summary by CodeRabbit
New Features
Bug Fixes
Documentation