[Feat] Networks: private networking via WireGuard, custom, and provider-synced VPCs#1209
Open
RichardAnderson wants to merge 32 commits into
Open
[Feat] Networks: private networking via WireGuard, custom, and provider-synced VPCs#1209RichardAnderson wants to merge 32 commits into
RichardAnderson wants to merge 32 commits into
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughAdds a complete network management platform covering WireGuard and provider networks, CIDR allocation, firewall rule materialisation, queued synchronisation, peer management, HTTP/Inertia interfaces, reconciliation scheduling, provider discovery, tests, and documentation. ChangesNetwork lifecycle and synchronisation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class, project-scoped “Networks” to Vito, supporting Vito-managed WireGuard tunnels (with peers), custom private-IP networks, and provider-synced VPCs, plus network-scoped firewall rules applied consistently across members.
Changes:
- Introduces Network/NetworkServer/NetworkPeer/NetworkFirewallRule domain models, migrations, factories, policies, controllers, jobs, and Inertia pages.
- Adds provider capability to discover/sync private networks (AWS/DO/Hetzner/Linode/Vultr) and reconciles Vito state to match.
- Integrates network-managed firewall rules into per-server UFW application and surfaces network rules read-only on the server firewall page.
Reviewed changes
Copilot reviewed 149 out of 155 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Actions/Service/UninstallTest.php | Adds WireGuard uninstall guard coverage for network members. |
| tests/Feature/UniqueQueueTest.php | Adds test coverage for transient DB lock retry behavior in UniqueQueue. |
| resources/views/wireguard/peer-conf.blade.php | WireGuard peer config template for “peer device” downloads. |
| resources/views/ssh/wireguard/uninstall.blade.php | SSH script to uninstall WireGuard packages. |
| resources/views/ssh/wireguard/remove-network.blade.php | SSH script to stop/disable a per-network wg-quick interface and remove config. |
| resources/views/ssh/wireguard/latest-handshakes.blade.php | SSH script to read WireGuard latest handshakes. |
| resources/views/ssh/wireguard/install.blade.php | SSH script to install WireGuard packages and validate availability. |
| resources/views/ssh/wireguard/configure.blade.php | SSH script to install config, enable unit, and validate interface start. |
| resources/views/ssh/wireguard/conf.blade.php | WireGuard server interface config template. |
| resources/views/ssh/services/firewall/ufw/apply-rules.blade.php | Allows UFW rules without port/proto for “allow all” network rules. |
| resources/js/types/server-log.d.ts | Adds optional server_name and network_id to ServerLog type for network log UIs. |
| resources/js/types/network.d.ts | Adds frontend type definitions for networks, members, peers, rules, and stats. |
| resources/js/pages/site-tooling/index.tsx | Updates docs link. |
| resources/js/pages/services/components/install.tsx | Hides VPN services from the generic “install service” UI flow. |
| resources/js/pages/server-network/index.tsx | Updates docs link. |
| resources/js/pages/redirects/components/redirect-form-fields.tsx | Formatting-only change to Checkbox usage. |
| resources/js/pages/redirects/components/edit-redirect.tsx | Formatting-only prop type inline change. |
| resources/js/pages/networks/show.tsx | Adds network overview page (stats + recent activity). |
| resources/js/pages/networks/servers.tsx | Adds network servers listing + actions (sync/add/edit/remove) via VitoTable + dialogs. |
| resources/js/pages/networks/peers.tsx | Adds WireGuard peer device listing + actions via VitoTable + dialogs. |
| resources/js/pages/networks/logs.tsx | Adds dedicated network logs page. |
| resources/js/pages/networks/index.tsx | Adds top-level networks index listing + create/sync actions. |
| resources/js/pages/networks/firewall.tsx | Adds network firewall rules listing + create/edit/delete actions. |
| resources/js/pages/networks/components/private-ip-select.tsx | Adds reusable private-IP picker with refresh-on-demand. |
| resources/js/pages/networks/components/log-columns.tsx | Adds network-specific server column for logs (with truncation helper). |
| resources/js/pages/networks/components/firewall-rule-form.tsx | Adds network firewall rule dialog form (registry-compatible). |
| resources/js/pages/networks/components/edit-network-server.tsx | Adds custom-network member IP edit dialog. |
| resources/js/pages/networks/components/add-server.tsx | Adds “add server to network” dialog (custom network supports picking private IP). |
| resources/js/pages/networks/components/add-peer.tsx | Adds “add peer device” dialog for WireGuard networks. |
| resources/js/pages/firewall/index.tsx | Surfaces network-managed rules read-only on server firewall page. |
| resources/js/pages/backups/index.tsx | Formatting-only change to VitoTable props layout. |
| resources/js/pages/backups/files.tsx | Formatting-only change to ternary layout. |
| resources/js/pages/backups/components/restore-backup.tsx | Minor text punctuation change. |
| resources/js/lib/utils.ts | Adds currentPath() and truncate() helpers for nav active-state and UI truncation. |
| resources/js/layouts/network/layout.tsx | Adds dedicated network layout + secondary nav tabs. |
| resources/js/layouts/database/layout.tsx | Uses currentPath() to avoid query/hash affecting active nav state. |
| resources/js/components/ui/combobox.tsx | Extends Combobox items with optional search keywords. |
| resources/js/components/dialogs/registry.ts | Registers new network dialogs in the centralized dialog registry. |
| resources/js/components/app-sidebar.tsx | Adds “Networks” entry to main sidebar + improves active detection. |
| resources/js/components/app-sidebar-nested.tsx | Improves active detection via currentPath(). |
| package-lock.json | Updates lockfile contents; project name changed (needs correction). |
| docs/4.x/settings/server-providers.md | Documents provider connections being used for provider network discovery. |
| docs/4.x/servers/network.md | Clarifies server network page vs project networks feature. |
| docs/4.x/servers/firewall.md | Documents network rules shown on server firewall page. |
| docs/4.x/README.md | Adds Networks section and cross-links. |
| docs/4.x/networks/servers.md | New documentation for network servers tab. |
| docs/4.x/networks/provider-networks.md | New documentation for provider network discovery/sync semantics. |
| docs/4.x/networks/overview.md | New documentation for network concept, types, statuses, and tabs. |
| docs/4.x/networks/firewall.md | New documentation for network firewall rules and application behavior. |
| docs/4.x/networks/create.md | New documentation for creating WireGuard/custom networks. |
| docs/4.x/automation.md | Documents scheduled network reconciliation and notes provider sync is manual. |
| database/migrations/2026_07_24_195725_add_network_id_to_server_logs.php | Adds network_id foreign key to server_logs for network-scoped activity. |
| database/migrations/2026_07_24_195724_create_network_peers_table.php | Creates network_peers table for peer devices. |
| database/migrations/2026_07_24_195723_create_server_network_rules_table.php | Creates server_network_rules materialized rules table for network-driven firewall. |
| database/migrations/2026_07_24_195722_create_network_firewall_rules_table.php | Creates network_firewall_rules table. |
| database/migrations/2026_07_24_195721_create_network_servers_table.php | Creates network_servers membership table. |
| database/migrations/2026_07_24_195720_create_networks_table.php | Creates networks table with provider sync fields. |
| database/factories/NetworkServerFactory.php | Factory for network server membership. |
| database/factories/NetworkPeerFactory.php | Factory for network peer devices. |
| database/factories/NetworkFirewallRuleFactory.php | Factory for network firewall rules. |
| database/factories/NetworkFactory.php | Factory for networks. |
| app/ValidationRules/WireGuardPublicKeyRule.php | Validates WireGuard public keys and uniqueness within a network. |
| app/Traits/UniqueQueue.php | Adds transient DB error detection/retry logic (needs lock release cleanup). |
| app/Tables/Servers/ServerNetworkRuleTable.php | InertiaTable table for server network-managed rules listing. |
| app/Tables/NetworkTable.php | InertiaTable table for networks listing. |
| app/Tables/Networks/NetworkServerTable.php | InertiaTable table for network members listing. |
| app/Tables/Networks/NetworkPeerTable.php | InertiaTable table for network peers listing. |
| app/Tables/Networks/NetworkFirewallRuleTable.php | InertiaTable table for network firewall rules listing. |
| app/Support/Testing/SSHFake.php | Uses ServerLog::newLog() flow to support new network log context tagging. |
| app/Support/Cidr.php | Adds IPv4 CIDR utilities (canonicalization, overlap, host allocation). |
| app/Services/VPN/VPN.php | Defines VPN service interface for network configuration hooks. |
| app/Services/Firewall/Ufw.php | Applies both network and server firewall rules; finalizes server network rule statuses. |
| app/ServerProviders/Vultr.php | Adds ProvidesPrivateNetworks support for Vultr VPC discovery. |
| app/ServerProviders/ProvidesPrivateNetworks.php | Adds provider capability interface for private network discovery. |
| app/ServerProviders/Linode.php | Adds ProvidesPrivateNetworks support for Linode VPC discovery. |
| app/ServerProviders/Hetzner.php | Adds ProvidesPrivateNetworks support for Hetzner network discovery. |
| app/ServerProviders/DigitalOcean.php | Adds ProvidesPrivateNetworks support for DigitalOcean VPC discovery. |
| app/ServerProviders/AbstractProvider.php | Adds credential-safe PrivateNetworkSyncError wrapper helper. |
| app/Providers/ServiceTypeServiceProvider.php | Registers WireGuard as a service type (VPN). |
| app/Policies/NetworkPolicy.php | Adds authorization rules for network CRUD, incl. provider network delete semantics. |
| app/Models/ServerNetworkRule.php | Adds model for materialized per-server network firewall/handshake rules. |
| app/Models/ServerLog.php | Adds network context tagging to ServerLog creation + network relationship. |
| app/Models/ServerIpAddress.php | Triggers custom-network firewall re-apply when a referenced private IP is deleted. |
| app/Models/Server.php | Adds networkRules relationship + WireGuard sibling resync on server deletion. |
| app/Models/Project.php | Adds networks relationship. |
| app/Models/NetworkServer.php | Adds membership model for network servers (incl. encrypted private key). |
| app/Models/NetworkPeer.php | Adds peer device model (incl. concealment semantics). |
| app/Models/NetworkFirewallRule.php | Adds network firewall rule model. |
| app/Models/Network.php | Adds network model with relationships and enum casting. |
| app/Jobs/Network/SyncProviderNetworksJob.php | Adds background job for provider network reconciliation (with click de-dupe). |
| app/Jobs/Network/PollPeerHandshakesJob.php | Adds handshake polling job for WireGuard peers. |
| app/Jobs/Network/ApplyNetworkFirewallJob.php | Adds job to apply network firewall rules to a single member. |
| app/Http/Resources/ServerLogResource.php | Adds server_name and network_id to log resource. |
| app/Http/Resources/NetworkServerResource.php | Adds API resource for network members. |
| app/Http/Resources/NetworkResource.php | Adds API resource for networks. |
| app/Http/Resources/NetworkPeerResource.php | Adds API resource for network peers. |
| app/Http/Resources/NetworkFirewallRuleResource.php | Adds API resource for network firewall rules. |
| app/Http/Middleware/HandleInertiaRequests.php | Shares network context in Inertia props when routed under a network. |
| app/Http/Controllers/NetworkServerController.php | Adds endpoints for managing network server membership and sync. |
| app/Http/Controllers/NetworkPeerController.php | Adds endpoints for peer device lifecycle and config reveal. |
| app/Http/Controllers/NetworkFirewallRuleController.php | Adds endpoints for network firewall rules CRUD. |
| app/Http/Controllers/FirewallController.php | Adds networkRules table data to server firewall page. |
| app/Exceptions/PrivateNetworkSyncError.php | Adds safe exception type to avoid leaking credentials via failed_jobs. |
| app/Enums/ServerNetworkRuleKind.php | Adds enum for handshake vs rule materialization. |
| app/Enums/NetworkType.php | Adds enum for network type (provider/custom/wireguard). |
| app/Enums/NetworkStatus.php | Adds enum for network status (creating/syncing/active/failed/deleting). |
| app/Enums/NetworkServerStatus.php | Adds enum for member status (pending/updating/active/failed/leaving). |
| app/Enums/NetworkPeerStatus.php | Adds enum for peer status (pending/active/disabled). |
| app/Enums/NetworkAddressingPool.php | Adds enum for WireGuard address pool selection. |
| app/DTOs/PrivateNetworkMemberDTO.php | Adds DTO for provider-reported member + sanitizes IPv4 addresses. |
| app/DTOs/PrivateNetworkDTO.php | Adds DTO for provider networks + normalizes CIDRs to IPv4 canonical form. |
| app/Console/Kernel.php | Schedules network reconciliation command every 3 minutes. |
| app/Actions/Service/Install.php | Prevents installing VPN services via generic service install action. |
| app/Actions/Server/EditServer.php | Resyncs WireGuard peers when a server’s public IP changes. |
| app/Actions/Network/UpdateNetworkServerIp.php | Updates custom-network member IP selection and reapplies firewall. |
| app/Actions/Network/UpdateNetworkPeer.php | Updates peer enable/disable + triggers member resync and recompute. |
| app/Actions/Network/UpdateNetwork.php | Allows renaming a network with per-project uniqueness. |
| app/Actions/Network/SyncNetwork.php | Syncs a network (provider uses provider sync job; others resync members). |
| app/Actions/Network/RemoveServerFromNetwork.php | Removes member and triggers network-wide resync/firewall updates. |
| app/Actions/Network/RegenerateNetworkPeerKeys.php | Regenerates peer keys (BYO or generated), resyncs members, recomputes status. |
| app/Actions/Network/RecomputeNetworkStatus.php | Computes network status from member statuses + activates pending peers when stable. |
| app/Actions/Network/ManageNetworkFirewallRule.php | CRUD for network firewall rules + applies firewall + broadcasts. |
| app/Actions/Network/GetNetworkPeerConfig.php | Generates peer config and conditionally returns private key. |
| app/Actions/Network/GenerateWireGuardKeys.php | Generates Curve25519 WireGuard keypairs in wg-compatible format. |
| app/Actions/Network/FinalizeServerNetworkRules.php | Finalizes materialized server network rule statuses based on apply outcome. |
| app/Actions/Network/DispatchNetworkServerSync.php | Dispatches per-member sync/teardown to SSH queue based on readiness. |
| app/Actions/Network/DeleteNetworkPeer.php | Deletes peer device and triggers member resync + recompute. |
| app/Actions/Network/DeleteNetwork.php | Marks network deleting and tears down members. |
| app/Actions/Network/CreateNetworkPeer.php | Creates a peer device with allocated IP and keys (BYO optional) + resync. |
| app/Actions/Network/ConcealNetworkPeerKey.php | Removes stored peer private key after reveal. |
| app/Actions/Network/ApplyNetworkFirewall.php | Materializes and applies network firewall rules to members (defers unreachable). |
| app/Actions/Network/AllocateNetworkBlock.php | Allocates WireGuard address blocks avoiding conflicts and blocklisted ranges. |
| .prettierignore | Excludes wireguard view templates from Prettier formatting. |
![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: 51
🤖 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 `@app/Actions/Network/AddServersToNetwork.php`:
- Around line 127-131: The servers.* validation rules accept duplicate or
non-integer IDs. In app/Actions/Network/AddServersToNetwork.php lines 127-131
and app/Actions/Network/CreateNetwork.php lines 210-212, add integer and
distinct validation rules alongside the existing exists and unique rules,
preserving the current project and network constraints.
- Around line 67-97: The tunnel-address allocation logic is duplicated across
the network actions. Create a shared AllocateNetworkAddress action that locks
the network, collects member and peer IPs, allocates the next Cidr::nextHost
address, tracks allocations, and raises the existing exhaustion validation
error; update AddServersToNetwork, CreateNetworkPeer, and
CreateNetwork::buildWireGuard to use it instead of their inline or usedIps()
logic. Affected sites: app/Actions/Network/AddServersToNetwork.php lines 67-97,
replace the used-IP collection and per-server allocation;
app/Actions/Network/CreateNetworkPeer.php lines 83-90, replace usedIps() with
the shared action; app/Actions/Network/CreateNetwork.php lines 95-113, replace
the inline allocation loop with the shared action.
In `@app/Actions/Network/CreateNetwork.php`:
- Around line 125-144: Normalize the CIDR input once before the duplicate check
and creation in the network action, treating a missing or empty `cidr` as null.
Reuse that normalized value for the duplicate lookup and both `cidr` and
`cidr_canonical` fields in the `Network::create` call, ensuring empty input
cannot be passed to `Cidr::canonical` or persisted as a bogus CIDR.
In `@app/Actions/Network/MaterializeServerNetworkRules.php`:
- Around line 235-242: Extract the duplicated server-network-rule.updated
dispatch from MaterializeServerNetworkRules::broadcast into a shared injectable
broadcaster or trait, preserving the project_id, event type, and server_id
payload. Update FinalizeServerNetworkRules to use the same shared helper instead
of maintaining its own broadcast implementation; apply the change at
app/Actions/Network/MaterializeServerNetworkRules.php#L235-L242 and
app/Actions/Network/FinalizeServerNetworkRules.php#L51-L58.
- Around line 40-103: Reduce the complexity of forServer() by extracting the
self-contained reconciliation branches into private helpers, such as
createMissing(), updateChanged(), and pruneStale(), passing the existing,
desired, and applied state they require. Keep the transaction boundary, changed
tracking, status transitions, deletion behavior, and broadcast($server) behavior
unchanged.
In `@app/Actions/Network/RecomputeNetworkStatus.php`:
- Around line 24-38: Update the DELETING branch in RecomputeNetworkStatus so
that when servers remain, it dispatches a network.updated SocketEvent before
returning, using the network’s project and identifier data; preserve the
existing network.deleted event and deletion behavior when no servers remain.
In `@app/Actions/Network/SyncProviderNetworks.php`:
- Around line 50-62: Update SyncProviderNetworks::reconcile/prune flow so every
provider-reported network is excluded from pruning when reconciliation fails,
including failures caused by transient QueryException; track the discovered
external ID independently of reconcile’s return value, while preserving pruning
for persisted networks not reported by the provider.
- Around line 191-226: Replace the isset() check in the existing-member
departure loop with array_key_exists() so desired members remain attached even
when their desired IP value is null. Keep the surrounding LEAVING handling and
removal flow unchanged.
In `@app/Actions/Network/UpdateNetworkPeer.php`:
- Around line 43-56: Make the dependent network mutations atomic: in
app/Actions/Network/UpdateNetworkPeer.php, wrap the peer save, conditional
resyncMembers(), and recompute->handle() calls in DB::transaction(), dispatch
the socket event after the transaction commits, and move
RecomputeNetworkStatus::activatePendingPeers() socket dispatches to the same
commit boundary; in app/Actions/Network/UpdateNetworkServerIp.php, wrap
member->update() and firewall rule materialisation in DB::transaction(), then
dispatch the SSH firewall job only after commit.
In `@app/Console/Commands/ReconcileNetworksCommand.php`:
- Around line 142-162: Update forceConverge() to wrap the membership deletion
and RecomputeNetworkStatus::handle() call in a DB::transaction(), then dispatch
the deletion event only after the transaction commits. After commit, invoke the
existing RemoveServerFromNetwork, DispatchNetworkServerSync::resyncMembers(),
and ApplyNetworkFirewall::handle() convergence flow so remaining members and
firewall rules are synchronized.
In `@app/Console/Kernel.php`:
- Line 24: Update the networks:reconcile scheduling chain to pass a short
expiration, such as 10 minutes, to withoutOverlapping(), matching the command’s
three-minute cadence and preventing stale locks from suppressing reconciliation
for 24 hours.
In `@app/DTOs/PrivateNetworkDTO.php`:
- Around line 32-55: The IPv4 CIDR validation logic is duplicated across two
locations and should use the shared helper. In app/DTOs/PrivateNetworkDTO.php
lines 32-55, simplify normalizeCidr() to validate with
Cidr::isValidIpv4Cidr($cidr) before canonicalizing, removing the manual parsing
and unreachable prefix check; in app/Actions/Network/CreateNetwork.php lines
241-256, update the closure to call the same helper instead of re-parsing the
CIDR.
In `@app/Enums/ServerNetworkRuleKind.php`:
- Around line 5-9: Update ServerNetworkRuleKind to implement
App\Contracts\VitoEnum and add getColor(): string and getText(): string methods,
providing the required color and display text for both HANDSHAKE and RULE cases.
In `@app/Http/Controllers/NetworkPeerController.php`:
- Around line 53-60: Update NetworkPeerController::config to wrap the
GetNetworkPeerConfig::config result in the appropriate JsonResource and return
that resource directly, replacing response()->json() while preserving the
existing authorization and network-membership checks.
In `@app/Http/Middleware/HandleInertiaRequests.php`:
- Around line 86-92: Update the project-switching branch in
HandleInertiaRequests so that after assigning and saving the new
current_project_id, the shared currentProject value is refreshed from the
updated user/project relationship before building the response data. Ensure the
first network response exposes auth.currentProject from the new project while
preserving the existing NetworkResource behavior.
In `@app/Models/NetworkFirewallRule.php`:
- Around line 7-19: Document the NetworkFirewallRule model’s serverRules
relationship in its PHPDoc with a Collection<int, ServerNetworkRule> property
annotation, and import Illuminate\Database\Eloquent\Collection alongside the
existing Eloquent imports.
In `@app/Models/NetworkServer.php`:
- Around line 41-48: Update the private_key entry in NetworkServer::$casts from
the plain encrypted cast to the required encrypted:json cast, and verify the
related factory/action values remain JSON-compatible with this cast.
In `@app/Models/Server.php`:
- Around line 130-168: Replace the static server-ID-keyed
$networkSiblingsToResync array with a WeakMap keyed by Server instances, and
update the deleting/deleted listeners in boot() to store, retrieve, and unset
entries using the $server object. Preserve the existing sibling collection and
resync behavior while ensuring aborted deletions cannot retain stale state.
In `@app/ServerProviders/AWS.php`:
- Around line 45-57: Update privateNetworks() to consume every page returned by
both describeInstances() and describeVpcs(), using the AWS SDK paginator or
equivalent NextToken iteration. Aggregate all reservations before passing them
to vpcIdsFrom(), and aggregate all Vpcs before building the network result;
preserve the existing empty-result behavior.
- Around line 123-148: Update placementOf to select the NetworkInterfaces entry
whose Attachment.DeviceIndex is 0, using its VpcId and PrivateIpAddress; if no
primary interface exists, fall back to the first entry before using top-level
instance fields.
In `@app/ServerProviders/Vultr.php`:
- Around line 40-49: Replace the inline comments in the PrivateNetworkSyncError
catch block with a DocBlock placed immediately before the 404 handling,
preserving the rationale that stale upstream server IDs should be skipped so one
404 does not abort synchronization or suppress pruning for other networks. Keep
the existing continue and rethrow behavior unchanged.
In `@app/Services/VPN/WireGuard.php`:
- Around line 62-72: Update the WireGuard SSH script references used by
install(), the indicated related methods, and their corresponding view lookups
from ssh.wireguard.* to ssh.services.wireguard.*; relocate the templates into
resources/views/ssh/services/wireguard/ while preserving the existing script
names and execution behavior.
- Around line 207-218: Update WireGuard::uploadConf to ensure the temporary
configuration file is created with 0600 permissions before SSH upload, rather
than relying on Storage::disk('local')->put() defaults; preserve the existing
upload flow and finally cleanup.
In `@app/Support/Cidr.php`:
- Around line 7-36: Harden Cidr::split() and Cidr::mask() for invalid prefixes:
make missing, empty, or non-digit suffixes default to 32, and clamp numeric
prefixes above 32 to 32 before bit shifting. Preserve valid prefix values and
the existing zero-prefix behavior so canonicalization and allocation remain
correct.
In `@app/Traits/UniqueQueue.php`:
- Around line 49-61: Update UniqueQueue::isTransientDatabaseError to use
Laravel’s structured concurrency detection via
Illuminate\Database\Concerns\DetectsConcurrencyErrors, or inspect QueryException
SQLSTATE/driver error codes, instead of matching exception message fragments.
Preserve the boolean result so only recognized transient database concurrency
failures are retried.
In `@app/ValidationRules/WireGuardPublicKeyRule.php`:
- Around line 13-19: Update validate in WireGuardPublicKeyRule to require
canonical Base64 encoding by decoding the string and rejecting it when
base64_encode($decoded) does not exactly equal the original value, in addition
to the existing 32-byte decoded-length check. Perform this validation before any
peer-key storage or collision checks.
In `@database/factories/NetworkServerFactory.php`:
- Line 24: Update the IP generation in NetworkServerFactory to derive unique
host addresses from the 100.64.0.0/24 network instead of using the fixed
two-digit numerify pattern. Exclude the subnet address .0, preserve valid IPv4
formatting, and support creating more than 100 records without exhausting the
generator.
- Around line 25-26: Update the NetworkServerFactory key generation to create a
valid WireGuard key pair: generate one private key, then derive its matching
public key through the same production key-generation path, or use an
established valid fixture pair. Ensure the factory stores the derived public key
alongside the private key instead of generating both independently.
In `@database/migrations/2026_07_24_195723_create_server_network_rules_table.php`:
- Around line 15-18: Update the migration defining the server network rules
table to enforce consistency between server_id, network_id, network_server_id,
and network_firewall_rule_id. Prefer storing only network_server_id and deriving
the parent IDs; otherwise replace the independent foreign keys with
database-level composite constraints that require the referenced network server
and firewall rule to belong to the specified server and network.
In `@docs/4.x/networks/servers.md`:
- Line 33: Update the networking server retry description to remove the fixed
three-minute recovery guarantee or qualify it as the reconcile interval rather
than a completion deadline. Ensure the wording reflects that processing depends
on retries of stalled leaf/teardown attempts and may take longer than one
interval.
In `@resources/js/layouts/network/layout.tsx`:
- Around line 10-11: Update the network layout’s realtime update flow so the
live record from useRealtimeRecord is propagated to every child page as the
network value, rather than leaving pages to read page.props.network directly.
Ensure downstream Head titles, badges, and isCustom/isWireGuard/isManaged checks
consume the updated network while preserving the existing layout behavior.
In `@resources/js/pages/firewall/index.tsx`:
- Line 26: Update the hasNetworkRules logic used by the firewall page to keep
VitoTable mounted when a search query is active, even if networkRules.data is
empty; base visibility on persistent rule availability or the active query state
so users can clear searches with no matches.
In `@resources/js/pages/networks/components/add-peer.tsx`:
- Line 1: Remove the redundant post-submit resets from the onSuccess handlers in
the add-peer, add-server, and create-network dialog components: retain only
onOpenChange(false), deleting form.reset() everywhere and setByo(false) in
add-peer. Rely on the DialogHost instanceId remount behavior and existing
mount-time initialization for fresh form state on each open.
In `@resources/js/pages/networks/firewall.tsx`:
- Line 21: Update the network reference used by the page metadata and type-based
gating to consume the realtime network record rather than static
page.props.network. Locate the existing realtime network source used by
NetworkLayout and reuse that symbol in the page’s Head title and conditional
logic, while preserving the current behavior for other page content.
- Line 34: Update the external documentation anchor in the firewall page to
include rel="noreferrer" alongside target="_blank", preventing the opened page
from accessing window.opener while preserving the existing link destination and
behavior.
In `@resources/js/pages/networks/index.tsx`:
- Line 28: Add rel="noopener noreferrer" to the Docs anchors in
resources/js/pages/networks/index.tsx lines 28-28 and
resources/js/pages/networks/show.tsx lines 69-69, preserving their existing
target="_blank" behavior.
- Around line 61-65: Update the network row action around the Link and Button
components to use Button with asChild, making the Link its rendered interactive
element instead of nesting controls. Add a descriptive aria-label to the
icon-only Link while preserving its existing route, styling, and EyeIcon.
In `@resources/js/pages/networks/logs.tsx`:
- Line 17: Update the network reference in the logs page to use the realtime
network record, matching the approach used by the network layout, instead of
reading the static page.props.network value. Locate the existing realtime record
source and use it wherever the logs page currently relies on network.
- Line 27: Update the external documentation anchor in the networks logs page to
include rel="noreferrer" alongside target="_blank", preserving the existing href
and link behavior.
In `@resources/js/pages/networks/peers.tsx`:
- Line 21: Update the network data reference in the peers page to use the
realtime network record provided by the network layout, rather than the static
page.props.network value. Follow the existing pattern in the network layout and
other network pages, preserving the current consumers while ensuring they
receive live network updates.
- Line 31: Add rel="noreferrer" to the external documentation anchor in the
peers page, alongside its existing target="_blank" attribute.
In `@resources/js/pages/networks/servers.tsx`:
- Around line 23-26: Update the network type checks near isCustom, isWireGuard,
and isManaged to derive from the realtime network record used by the network
layout, rather than the initial page.props.network value. Ensure row-action
gating reacts when realtime network data changes, while preserving the existing
type comparisons.
- Line 36: Update the external documentation anchor in the servers page to
include rel="noreferrer" alongside target="_blank", preserving the existing href
and link behavior.
In `@resources/js/pages/networks/settings.tsx`:
- Line 20: Update the network source in the settings page from static page props
to the realtime network record used by the network layout, and ensure the Type
and Status Badge values derive from that realtime record as well. Locate the
change around the network declaration and preserve the existing rendering logic.
- Line 51: Update the external documentation anchor in the networks settings
page to include rel="noreferrer" alongside target="_blank", preserving the
existing href and link behavior.
- Around line 26-107: Render the validation message from form.errors.name
alongside the editable name field, using the existing InputError component. Keep
it tied to the name input so failed updates display the field-level error
without changing the current submit or focus behavior.
In `@resources/js/pages/server-network/index.tsx`:
- Line 59: Update the external anchor in the server network page to include
rel="noopener noreferrer" alongside target="_blank", preserving the existing
href and new-tab behavior.
In `@resources/js/pages/site-tooling/index.tsx`:
- Line 66: Add rel="noreferrer" to the external target="_blank" Docs links in
resources/js/pages/site-tooling/index.tsx:66-66 and
resources/js/pages/backups/index.tsx:36-36, preserving their existing href and
target attributes.
In `@resources/views/ssh/wireguard/configure.blade.php`:
- Around line 7-13: Update the WireGuard setup script around `systemctl restart`
to use `wg syncconf` for live configuration changes, preserving existing tunnels
when peers are added. Retain `systemctl restart` for cold starts and membership
IP changes, since `wg syncconf` does not apply `Address` or `MTU` updates; keep
the existing interface validation unchanged.
In `@tests/Feature/NetworkProviderSyncTest.php`:
- Around line 514-529: Strengthen test_sync_ignores_networks_of_another_project
by recording the other network’s relevant attributes before invoking
SyncProviderNetworks::forProject, then assert those attributes remain unchanged
afterward in addition to asserting the record still exists. Keep the test
focused on preventing writes across the project boundary.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4a59d81b-f557-4b64-824f-3861dcaa60ab
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (154)
.prettierignoreapp/Actions/Network/AddServersToNetwork.phpapp/Actions/Network/AllocateNetworkBlock.phpapp/Actions/Network/ApplyNetworkFirewall.phpapp/Actions/Network/ConcealNetworkPeerKey.phpapp/Actions/Network/CreateNetwork.phpapp/Actions/Network/CreateNetworkPeer.phpapp/Actions/Network/DeleteNetwork.phpapp/Actions/Network/DeleteNetworkPeer.phpapp/Actions/Network/DispatchNetworkServerSync.phpapp/Actions/Network/FinalizeServerNetworkRules.phpapp/Actions/Network/GenerateWireGuardKeys.phpapp/Actions/Network/GetNetworkPeerConfig.phpapp/Actions/Network/ManageNetworkFirewallRule.phpapp/Actions/Network/MaterializeServerNetworkRules.phpapp/Actions/Network/RecomputeNetworkStatus.phpapp/Actions/Network/RegenerateNetworkPeerKeys.phpapp/Actions/Network/RemoveServerFromNetwork.phpapp/Actions/Network/SyncNetwork.phpapp/Actions/Network/SyncProviderNetworks.phpapp/Actions/Network/UpdateNetwork.phpapp/Actions/Network/UpdateNetworkPeer.phpapp/Actions/Network/UpdateNetworkServerIp.phpapp/Actions/Server/EditServer.phpapp/Actions/Service/Install.phpapp/Console/Commands/ReconcileNetworksCommand.phpapp/Console/Kernel.phpapp/DTOs/PrivateNetworkDTO.phpapp/DTOs/PrivateNetworkMemberDTO.phpapp/Enums/NetworkAddressingPool.phpapp/Enums/NetworkPeerStatus.phpapp/Enums/NetworkServerStatus.phpapp/Enums/NetworkStatus.phpapp/Enums/NetworkType.phpapp/Enums/ServerNetworkRuleKind.phpapp/Exceptions/PrivateNetworkSyncError.phpapp/Http/Controllers/FirewallController.phpapp/Http/Controllers/NetworkController.phpapp/Http/Controllers/NetworkFirewallRuleController.phpapp/Http/Controllers/NetworkPeerController.phpapp/Http/Controllers/NetworkServerController.phpapp/Http/Middleware/HandleInertiaRequests.phpapp/Http/Resources/NetworkFirewallRuleResource.phpapp/Http/Resources/NetworkPeerResource.phpapp/Http/Resources/NetworkResource.phpapp/Http/Resources/NetworkServerResource.phpapp/Http/Resources/ServerLogResource.phpapp/Jobs/Network/ApplyNetworkFirewallJob.phpapp/Jobs/Network/PollPeerHandshakesJob.phpapp/Jobs/Network/SyncNetworkServerJob.phpapp/Jobs/Network/SyncProviderNetworksJob.phpapp/Models/Network.phpapp/Models/NetworkFirewallRule.phpapp/Models/NetworkPeer.phpapp/Models/NetworkServer.phpapp/Models/Project.phpapp/Models/Server.phpapp/Models/ServerIpAddress.phpapp/Models/ServerLog.phpapp/Models/ServerNetworkRule.phpapp/Policies/NetworkPolicy.phpapp/Providers/ServiceTypeServiceProvider.phpapp/ServerProviders/AWS.phpapp/ServerProviders/AbstractProvider.phpapp/ServerProviders/DigitalOcean.phpapp/ServerProviders/Hetzner.phpapp/ServerProviders/Linode.phpapp/ServerProviders/ProvidesPrivateNetworks.phpapp/ServerProviders/Vultr.phpapp/Services/Firewall/Ufw.phpapp/Services/VPN/VPN.phpapp/Services/VPN/WireGuard.phpapp/Support/Cidr.phpapp/Support/Testing/SSHFake.phpapp/Tables/NetworkTable.phpapp/Tables/Networks/NetworkFirewallRuleTable.phpapp/Tables/Networks/NetworkPeerTable.phpapp/Tables/Networks/NetworkServerTable.phpapp/Tables/Servers/ServerNetworkRuleTable.phpapp/Traits/UniqueQueue.phpapp/ValidationRules/WireGuardPublicKeyRule.phpdatabase/factories/NetworkFactory.phpdatabase/factories/NetworkFirewallRuleFactory.phpdatabase/factories/NetworkPeerFactory.phpdatabase/factories/NetworkServerFactory.phpdatabase/migrations/2026_07_24_195720_create_networks_table.phpdatabase/migrations/2026_07_24_195721_create_network_servers_table.phpdatabase/migrations/2026_07_24_195722_create_network_firewall_rules_table.phpdatabase/migrations/2026_07_24_195723_create_server_network_rules_table.phpdatabase/migrations/2026_07_24_195724_create_network_peers_table.phpdatabase/migrations/2026_07_24_195725_add_network_id_to_server_logs.phpdocs/4.x/README.mddocs/4.x/automation.mddocs/4.x/networks/create.mddocs/4.x/networks/firewall.mddocs/4.x/networks/overview.mddocs/4.x/networks/peers.mddocs/4.x/networks/provider-networks.mddocs/4.x/networks/servers.mddocs/4.x/servers/firewall.mddocs/4.x/servers/network.mddocs/4.x/settings/server-providers.mdresources/js/components/app-sidebar-nested.tsxresources/js/components/app-sidebar.tsxresources/js/components/dialogs/registry.tsresources/js/components/ui/combobox.tsxresources/js/layouts/database/layout.tsxresources/js/layouts/network/layout.tsxresources/js/lib/utils.tsresources/js/pages/backups/components/restore-backup.tsxresources/js/pages/backups/files.tsxresources/js/pages/backups/index.tsxresources/js/pages/firewall/index.tsxresources/js/pages/networks/components/add-peer.tsxresources/js/pages/networks/components/add-server.tsxresources/js/pages/networks/components/create-network.tsxresources/js/pages/networks/components/edit-network-server.tsxresources/js/pages/networks/components/firewall-rule-form.tsxresources/js/pages/networks/components/log-columns.tsxresources/js/pages/networks/components/peer-config.tsxresources/js/pages/networks/components/private-ip-select.tsxresources/js/pages/networks/firewall.tsxresources/js/pages/networks/index.tsxresources/js/pages/networks/logs.tsxresources/js/pages/networks/peers.tsxresources/js/pages/networks/servers.tsxresources/js/pages/networks/settings.tsxresources/js/pages/networks/show.tsxresources/js/pages/redirects/components/edit-redirect.tsxresources/js/pages/redirects/components/redirect-form-fields.tsxresources/js/pages/server-network/index.tsxresources/js/pages/services/components/install.tsxresources/js/pages/site-tooling/index.tsxresources/js/types/network.d.tsresources/js/types/server-log.d.tsresources/views/ssh/services/firewall/ufw/apply-rules.blade.phpresources/views/ssh/wireguard/conf.blade.phpresources/views/ssh/wireguard/configure.blade.phpresources/views/ssh/wireguard/install.blade.phpresources/views/ssh/wireguard/latest-handshakes.blade.phpresources/views/ssh/wireguard/remove-network.blade.phpresources/views/ssh/wireguard/uninstall.blade.phpresources/views/wireguard/peer-conf.blade.phpstorage/database.sqlite.bak-20260724-195719storage/database.sqlite.bak-20260724-203839storage/database.sqlite.bak-20260724-204013-prefkfixtests/Feature/NetworkFirewallTest.phptests/Feature/NetworkPeerTest.phptests/Feature/NetworkProviderSyncTest.phptests/Feature/NetworkSyncTest.phptests/Feature/NetworkTest.phptests/Feature/ProviderPrivateNetworkTest.phptests/Feature/UniqueQueueTest.phptests/Unit/Actions/Service/UninstallTest.php
RichardAnderson
force-pushed
the
feat/networks
branch
from
July 24, 2026 21:23
344abd1 to
b36ca70
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/js/components/app-sidebar.tsx (1)
202-212: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCompare
currentPath()againstchildItem.href, notchildItem.onlyActivePath.
onlyActivePathis used as the selection flag; elsewhere in this component the exact active route is compared with.href. Comparing the current path to the flag preventsgroupActivefrom becoming true for children such as the server-sideservers.show,databases, orsiteslinks, so the parent collapsible can fail to open when those child routes are active.🐛 Proposed fix
childItem.hidden ? false : childItem.onlyActivePath - ? currentPath() === childItem.onlyActivePath + ? currentPath() === childItem.href : window.location.href.startsWith(childItem.href),🤖 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 `@resources/js/components/app-sidebar.tsx` around lines 202 - 212, Update the groupActive calculation in the childItem.some callback to compare currentPath() with childItem.href when childItem.onlyActivePath is set, rather than comparing against the onlyActivePath flag. Preserve the existing hidden-child exclusion and startsWith behavior for other children.
♻️ Duplicate comments (4)
app/Services/VPN/WireGuard.php (1)
207-219: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winTemp config file is still briefly world-readable before
chmod.
Storage::disk('local')->put()creates the file with the disk's default permissions;@chmod(..., 0600)only tightens it afterwards, leaving a race window where the WireGuard private key is exposed. The@also silently swallows anychmodfailure. Create the file restricted before writing content instead of chmod-after-write.🔒 Proposed fix
private function uploadConf(SSH|SSHFake $ssh, string $remote, string $content): void { $tmpName = 'wg-'.Str::random(20); $disk = Storage::disk('local'); - $disk->put($tmpName, $content); - `@chmod`($disk->path($tmpName), 0600); + $path = $disk->path($tmpName); + touch($path); + chmod($path, 0600); + $disk->put($tmpName, $content); try { $ssh->upload($disk->path($tmpName), $remote, 'root'); } finally { $disk->delete($tmpName); } }🤖 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 `@app/Services/VPN/WireGuard.php` around lines 207 - 219, Update uploadConf so the temporary configuration file is created with 0600 permissions before writing content, rather than calling Storage::put followed by chmod. Remove the suppressed chmod call and preserve the existing SSH upload and finally-block cleanup behavior.app/Console/Commands/ReconcileNetworksCommand.php (1)
143-165: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftStill unresolved:
forceConverge()isn't transactional and skips peer-config convergence.Deleting the membership, broadcasting, and applying the firewall/status recompute happen outside a
DB::transaction(), and remaining members' WireGuard peer configs are never resynced (noDispatchNetworkServerSync::resyncMembers()call) after this forced removal — a prior review already flagged this exact gap on these lines.🤖 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 `@app/Console/Commands/ReconcileNetworksCommand.php` around lines 143 - 165, Update forceConverge() to perform the membership deletion and related network state updates within a DB::transaction(), and invoke DispatchNetworkServerSync::resyncMembers() for the remaining members after the forced removal. Preserve the existing logging, socket event, firewall application, and network status recomputation while ensuring peer configurations are resynchronized.app/Models/NetworkServer.php (1)
41-48: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
private_keystill uses the plainencryptedcast, notencrypted:json.Same concern as flagged on a previous commit — this WireGuard credential should follow the repo-wide
encrypted:jsonconvention (as used forServer::$authentication).🔒 Proposed fix
- 'private_key' => 'encrypted', + 'private_key' => 'encrypted:json',🤖 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 `@app/Models/NetworkServer.php` around lines 41 - 48, Update the private_key entry in NetworkServer’s casts to use the repository-standard encrypted:json cast, matching Server::$authentication, while leaving the other casts unchanged.Source: Coding guidelines
database/factories/NetworkServerFactory.php (1)
25-26: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPublic/private keys still aren't a matching WireGuard pair.
Same concern as before: generating both independently with
random_bytesmeans they don't represent one identity, which will break any test relying on real key correspondence.🔑 Proposed fix
- 'public_key' => base64_encode(random_bytes(32)), - 'private_key' => base64_encode(random_bytes(32)), + 'private_key' => $keys = app(\App\Actions\Network\GenerateWireGuardKeys::class)->generate(),Or reuse the same generator used in production (
GenerateWireGuardKeys::generate()), storing bothpublic_key/private_keyfrom its return value instead of generating them independently.🤖 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 `@database/factories/NetworkServerFactory.php` around lines 25 - 26, Update the network server factory key generation to use the production WireGuard key-pair generator, GenerateWireGuardKeys::generate(), and assign its corresponding private and public values to private_key and public_key instead of generating each with random_bytes.
🤖 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 `@app/Actions/Network/AllocateNetworkBlock.php`:
- Around line 122-131: Update memberSubnets() to avoid calling
$server->ipAddresses()->get() for each member server; fetch all ServerIpAddress
records for the member server IDs in one query, then apply the existing IPv4
filtering, CIDR canonicalization, uniqueness, and value normalization. Add the
ServerIpAddress import and preserve the current returned array behavior.
In `@app/Actions/Network/CreateNetwork.php`:
- Line 1: Extract the duplicated per-server WireGuard member creation sequence
from CreateNetwork::buildWireGuard() into a shared private method or
collaborator that allocates the next host IP, generates keys, and creates the
network_servers row. Update both CreateNetwork::buildWireGuard() and
AddServersToNetwork::addWireGuard() to call this shared implementation,
preserving their existing inputs and behavior.
In `@app/Actions/Network/FinalizeServerNetworkRules.php`:
- Around line 20-34: Wrap the two database mutations in
ServerNetworkRule::success—updating emitted rules and deleting deletingIds—in a
DB::transaction() callback, while preserving the existing conditional queries
and broadcasting behavior after the transaction completes.
In `@app/Actions/Network/MaterializeServerNetworkRules.php`:
- Around line 108-176: Refactor desiredFor() and its handshakes()/sources() flow
to memoise each network’s peer collection by network_id for the duration of the
call, reusing it for handshake generation, source generation, and the
devices-exist check instead of querying peers per membership. Also eager-load or
cache network firewallRules once per network and reuse that collection,
preserving the existing non-deleting filter and ordering while eliminating
repeated membership queries during forNetwork().
- Around line 56-83: Update the existing-rule handling in the materialization
loop so a matching row with status FirewallRuleStatus::DELETING is revived to
the desired lifecycle status, using CREATING when applied and READY otherwise.
Ensure this status-only transition marks changed and is persisted, while
retaining the existing field-change update behavior for other statuses.
- Around line 183-212: Guard nullable relationship dereferences in peers() and
sources(). In the peers mapping, safely handle missing peer->server before
reading ip or name, and in sources() safely handle a missing
peer->serverIpAddress when server_ip_address_id is set; skip entries without a
valid IP while preserving the existing output shape and reconciliation flow.
In `@app/Actions/Network/RegenerateNetworkPeerKeys.php`:
- Around line 41-52: Make topology side effects commit-consistent across
app/Actions/Network/RegenerateNetworkPeerKeys.php lines 41-52,
app/Actions/Network/ApplyNetworkFirewall.php lines 25-50, and
app/Actions/Network/RecomputeNetworkStatus.php lines 30-34, 59-63, and 74-78:
defer resync/recompute work and SocketEvent dispatches until the surrounding
database transaction commits, using the existing commit-callback mechanism where
available, so events expose only final durable state.
In `@app/Actions/Network/RemoveServerFromNetwork.php`:
- Around line 23-33: Centralize WireGuard member re-sync traversal in
DispatchNetworkServerSync::resyncMembers() by adding an optional exclusion
parameter and applying the existing ACTIVE/UPDATING filtering there. In
app/Actions/Network/RemoveServerFromNetwork.php lines 23-33, replace the manual
loop with resyncMembers($network) and remove the unused NetworkServerStatus
import. In app/Actions/Server/EditServer.php lines 67-82, replace the nested
query and loop with resyncMembers() for each WireGuard membership, passing the
edited server’s membership ID as the exclusion.
In `@app/Actions/Network/SyncProviderNetworks.php`:
- Around line 179-231: Update reconcileMembers to track whether any existing
member transitions to LEAVING during this pass, and skip the final removal loop
when such a transition occurs. Preserve normal pruning when no member was newly
marked LEAVING, while continuing to reactivate desired LEAVING members.
In `@app/Actions/Network/UpdateNetworkServerIp.php`:
- Around line 34-40: Scope the unique validation rule for server_ip_address_id
in the UpdateNetworkServerIp validation to the current network by adding the
appropriate network_id constraint, while retaining the existing member ignore
behavior and private-IP/server existence constraints.
In `@app/Http/Controllers/NetworkController.php`:
- Around line 109-126: Extract the shared private IP mapping from
memberIpsPayload() and serversPayload() into a private helper, such as
mapPrivateIps(), preserving the existing id, ip, and is_primary fields and
values() behavior. Replace both inline mappings with calls to the helper so the
two payload methods use one implementation.
In `@app/Http/Controllers/NetworkPeerController.php`:
- Around line 53-60: Update the config method in NetworkPeerController to add
the Cache-Control: no-store header to the JSON response returned by
GetNetworkPeerConfig, while preserving the existing authorization, ownership
validation, and response payload.
In `@app/Jobs/Network/ApplyNetworkFirewallJob.php`:
- Around line 41-44: Update the status filter in the network member query within
ApplyNetworkFirewallJob to target ACTIVE, PENDING, and UPDATING members, while
continuing to exclude members being removed. Keep the existing bulk update
behavior and mark all eligible synchronising members as FAILED.
In `@app/Jobs/Network/SyncNetworkServerJob.php`:
- Around line 46-64: Update failed() in
app/Jobs/Network/SyncNetworkServerJob.php (lines 46-64) and
app/Jobs/Network/PollPeerHandshakesJob.php (lines 49-58) to accept \Throwable
instead of Exception, preserving the existing logging and status-update behavior
for all throwable failures.
In `@app/Models/Network.php`:
- Around line 31-33: Update the Network model’s relationship property
annotations to document the remaining collections: add Collection<int,
ServerNetworkRule> $serverRules and Collection<int, ServerLog> $serverLogs
alongside the existing $servers, $firewallRules, and $peers annotations.
In `@app/Models/NetworkServer.php`:
- Around line 30-48: Add `private_key` to the NetworkServer model’s hidden
attributes via its `$hidden` configuration, matching the existing protection
pattern used by `Server::$authentication`; leave the current casts and fillable
attributes unchanged.
In `@app/Models/Server.php`:
- Around line 130-167: Update the Server deletion callbacks and the
networkSiblingsToResync flow so sibling re-sync dispatch is deferred until the
surrounding database transaction commits, using the framework’s after-commit
queue mechanism rather than immediate toPresent execution. Preserve the
collected sibling IDs and cleanup behavior, and ensure
DispatchNetworkServerSync/toPresent cannot queue SyncNetworkServerJob before the
Server deletion is committed.
In `@app/Models/ServerIpAddress.php`:
- Around line 50-72: Update the ServerIpAddress deletion flow in booted,
specifically the deleted callback, to remove each affected CUSTOM network
membership after the IP is deleted instead of reapplying firewall rules for the
incomplete membership. Reuse RemoveServerFromNetwork or the established
membership-removal mechanism, using reapplyNetworkIds to identify the affected
networks, and eliminate the direct ApplyNetworkFirewall call for these orphaned
memberships.
In `@app/ServerProviders/DigitalOcean.php`:
- Around line 102-134: Bound pagination in fetchAll for
app/ServerProviders/DigitalOcean.php (lines 102-134) by adding a maximum page
count to the links.pages.next continuation check. In
app/ServerProviders/Hetzner.php (lines 107-139), only advance when
meta.pagination.next_page is strictly greater than the current page. In
app/ServerProviders/Vultr.php (lines 116-148), stop pagination when
meta.links.next repeats the previous cursor.
In `@app/Services/Firewall/Ufw.php`:
- Around line 60-89: Update the serverRules query in the Ufw rule-application
flow to apply a deterministic ordering, matching the ordering used for
networkRules or the intended firewall precedence, before concatenating and
applying the collections. Keep the existing status filtering and finalize
success/failure handling unchanged.
In `@app/Tables/Servers/ServerNetworkRuleTable.php`:
- Around line 18-27: Update the query() method to replace the duplicated
orderByRaw/orderBy ordering chain with the ServerNetworkRule::scopeOrdered()
scope. Preserve the existing filtering, eager loading, and pagination behavior
while reusing the centralized ordering definition.
In `@database/factories/NetworkFactory.php`:
- Line 21: Update the network factory’s block generation so a single unique
block number provides enough range for both middle CIDR octets, rather than
limiting uniqueness to 0–255. Preserve the existing Faker uniqueness and ensure
the derived octets remain valid network-address values.
In `@resources/js/pages/networks/components/add-server.tsx`:
- Line 1: Extract the duplicated server-selection and private-IP refresh
behavior from AddServer and CreateNetwork into a shared useServerWithPrivateIp
hook. Move each component’s servers state, selection handler
(selectServer/selectPrimaryServer), applyRefreshedIps callback, and dynamic
ip_addresses.<id> error lookup into the hook, then update both dialogs to
consume its API while preserving their existing form behavior.
In `@resources/js/pages/networks/settings.tsx`:
- Around line 28-30: Update the form synchronization near the useForm
initialization so form.data.name follows realtime network.name changes when the
form is not dirty, while preserving any locally edited name. Ensure the non-edit
display and edit prefill use the latest record name without overwriting a dirty
local edit.
In `@tests/Feature/NetworkPeerTest.php`:
- Around line 157-158: Replace the explanatory inline // comments near the
WireGuard configuration assertions in the relevant test methods with PHPDoc
block comments, including the additional comment around the second referenced
location; preserve their wording and keep the comments outside executable
statements as required by the repository’s PHP style.
In `@tests/Feature/NetworkProviderSyncTest.php`:
- Around line 46-48: Replace the inline rationale comments with PHPDoc blocks:
in tests/Feature/NetworkProviderSyncTest.php#L46-L48, place the Http::fake()
merge-semantics explanation above setUp(); at `#L232`, document
test_already_leaving_member_is_not_torn_down_again; and in
tests/Feature/NetworkTest.php#L307-L308, document
test_empty_cidr_is_stored_as_null_not_canonicalised. Remove the corresponding
inline comments while preserving their explanations.
In `@tests/Feature/ProviderPrivateNetworkTest.php`:
- Around line 411-447: Extend the injection test coverage from member addresses
to WireGuard public keys by adding a matching untrusted-public-key data provider
and test in tests/Feature/ProviderPrivateNetworkTest.php. Exercise shell
metacharacters, embedded whitespace, and other invalid values through the
private-network member public_key path, then assert the resulting public key is
dropped. Tighten WireGuardPublicKeyRule so embedded whitespace and these unsafe
inputs are rejected before interpolation into the WireGuard template.
---
Outside diff comments:
In `@resources/js/components/app-sidebar.tsx`:
- Around line 202-212: Update the groupActive calculation in the childItem.some
callback to compare currentPath() with childItem.href when
childItem.onlyActivePath is set, rather than comparing against the
onlyActivePath flag. Preserve the existing hidden-child exclusion and startsWith
behavior for other children.
---
Duplicate comments:
In `@app/Console/Commands/ReconcileNetworksCommand.php`:
- Around line 143-165: Update forceConverge() to perform the membership deletion
and related network state updates within a DB::transaction(), and invoke
DispatchNetworkServerSync::resyncMembers() for the remaining members after the
forced removal. Preserve the existing logging, socket event, firewall
application, and network status recomputation while ensuring peer configurations
are resynchronized.
In `@app/Models/NetworkServer.php`:
- Around line 41-48: Update the private_key entry in NetworkServer’s casts to
use the repository-standard encrypted:json cast, matching
Server::$authentication, while leaving the other casts unchanged.
In `@app/Services/VPN/WireGuard.php`:
- Around line 207-219: Update uploadConf so the temporary configuration file is
created with 0600 permissions before writing content, rather than calling
Storage::put followed by chmod. Remove the suppressed chmod call and preserve
the existing SSH upload and finally-block cleanup behavior.
In `@database/factories/NetworkServerFactory.php`:
- Around line 25-26: Update the network server factory key generation to use the
production WireGuard key-pair generator, GenerateWireGuardKeys::generate(), and
assign its corresponding private and public values to private_key and public_key
instead of generating each with random_bytes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6cbb0338-b6c0-4cdb-8d2a-d18d999d1362
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (153)
.gitignore.prettierignoreapp/Actions/Network/AddServersToNetwork.phpapp/Actions/Network/AllocateNetworkBlock.phpapp/Actions/Network/ApplyNetworkFirewall.phpapp/Actions/Network/ConcealNetworkPeerKey.phpapp/Actions/Network/CreateNetwork.phpapp/Actions/Network/CreateNetworkPeer.phpapp/Actions/Network/DeleteNetwork.phpapp/Actions/Network/DeleteNetworkPeer.phpapp/Actions/Network/DispatchNetworkServerSync.phpapp/Actions/Network/FinalizeServerNetworkRules.phpapp/Actions/Network/GenerateWireGuardKeys.phpapp/Actions/Network/GetNetworkPeerConfig.phpapp/Actions/Network/ManageNetworkFirewallRule.phpapp/Actions/Network/MaterializeServerNetworkRules.phpapp/Actions/Network/RecomputeNetworkStatus.phpapp/Actions/Network/RegenerateNetworkPeerKeys.phpapp/Actions/Network/RemoveServerFromNetwork.phpapp/Actions/Network/SyncNetwork.phpapp/Actions/Network/SyncProviderNetworks.phpapp/Actions/Network/UpdateNetwork.phpapp/Actions/Network/UpdateNetworkPeer.phpapp/Actions/Network/UpdateNetworkServerIp.phpapp/Actions/Server/EditServer.phpapp/Actions/Service/Install.phpapp/Console/Commands/ReconcileNetworksCommand.phpapp/Console/Kernel.phpapp/DTOs/PrivateNetworkDTO.phpapp/DTOs/PrivateNetworkMemberDTO.phpapp/Enums/NetworkAddressingPool.phpapp/Enums/NetworkPeerStatus.phpapp/Enums/NetworkServerStatus.phpapp/Enums/NetworkStatus.phpapp/Enums/NetworkType.phpapp/Enums/ServerNetworkRuleKind.phpapp/Exceptions/PrivateNetworkSyncError.phpapp/Http/Controllers/FirewallController.phpapp/Http/Controllers/NetworkController.phpapp/Http/Controllers/NetworkFirewallRuleController.phpapp/Http/Controllers/NetworkPeerController.phpapp/Http/Controllers/NetworkServerController.phpapp/Http/Middleware/HandleInertiaRequests.phpapp/Http/Resources/NetworkFirewallRuleResource.phpapp/Http/Resources/NetworkPeerResource.phpapp/Http/Resources/NetworkResource.phpapp/Http/Resources/NetworkServerResource.phpapp/Http/Resources/ServerLogResource.phpapp/Jobs/Network/ApplyNetworkFirewallJob.phpapp/Jobs/Network/PollPeerHandshakesJob.phpapp/Jobs/Network/SyncNetworkServerJob.phpapp/Jobs/Network/SyncProviderNetworksJob.phpapp/Models/Network.phpapp/Models/NetworkFirewallRule.phpapp/Models/NetworkPeer.phpapp/Models/NetworkServer.phpapp/Models/Project.phpapp/Models/Server.phpapp/Models/ServerIpAddress.phpapp/Models/ServerLog.phpapp/Models/ServerNetworkRule.phpapp/Policies/NetworkPolicy.phpapp/Providers/ServiceTypeServiceProvider.phpapp/ServerProviders/AWS.phpapp/ServerProviders/AbstractProvider.phpapp/ServerProviders/DigitalOcean.phpapp/ServerProviders/Hetzner.phpapp/ServerProviders/Linode.phpapp/ServerProviders/ProvidesPrivateNetworks.phpapp/ServerProviders/Vultr.phpapp/Services/Firewall/Ufw.phpapp/Services/VPN/VPN.phpapp/Services/VPN/WireGuard.phpapp/Support/Cidr.phpapp/Support/Testing/SSHFake.phpapp/Tables/NetworkTable.phpapp/Tables/Networks/NetworkFirewallRuleTable.phpapp/Tables/Networks/NetworkPeerTable.phpapp/Tables/Networks/NetworkServerTable.phpapp/Tables/Servers/ServerNetworkRuleTable.phpapp/Traits/UniqueQueue.phpapp/ValidationRules/WireGuardPublicKeyRule.phpdatabase/factories/NetworkFactory.phpdatabase/factories/NetworkFirewallRuleFactory.phpdatabase/factories/NetworkPeerFactory.phpdatabase/factories/NetworkServerFactory.phpdatabase/migrations/2026_07_24_195720_create_networks_table.phpdatabase/migrations/2026_07_24_195721_create_network_servers_table.phpdatabase/migrations/2026_07_24_195722_create_network_firewall_rules_table.phpdatabase/migrations/2026_07_24_195723_create_server_network_rules_table.phpdatabase/migrations/2026_07_24_195724_create_network_peers_table.phpdatabase/migrations/2026_07_24_195725_add_network_id_to_server_logs.phpdocs/4.x/README.mddocs/4.x/automation.mddocs/4.x/networks/create.mddocs/4.x/networks/firewall.mddocs/4.x/networks/overview.mddocs/4.x/networks/peers.mddocs/4.x/networks/provider-networks.mddocs/4.x/networks/servers.mddocs/4.x/servers/firewall.mddocs/4.x/servers/network.mddocs/4.x/settings/server-providers.mdeslint.config.jsresources/js/components/app-sidebar-nested.tsxresources/js/components/app-sidebar.tsxresources/js/components/dialogs/registry.tsresources/js/components/ui/combobox.tsxresources/js/layouts/database/layout.tsxresources/js/layouts/network/layout.tsxresources/js/lib/utils.tsresources/js/pages/backups/components/restore-backup.tsxresources/js/pages/backups/files.tsxresources/js/pages/backups/index.tsxresources/js/pages/firewall/index.tsxresources/js/pages/networks/components/add-peer.tsxresources/js/pages/networks/components/add-server.tsxresources/js/pages/networks/components/create-network.tsxresources/js/pages/networks/components/edit-network-server.tsxresources/js/pages/networks/components/firewall-rule-form.tsxresources/js/pages/networks/components/log-columns.tsxresources/js/pages/networks/components/peer-config.tsxresources/js/pages/networks/components/private-ip-select.tsxresources/js/pages/networks/firewall.tsxresources/js/pages/networks/index.tsxresources/js/pages/networks/logs.tsxresources/js/pages/networks/peers.tsxresources/js/pages/networks/servers.tsxresources/js/pages/networks/settings.tsxresources/js/pages/networks/show.tsxresources/js/pages/redirects/components/edit-redirect.tsxresources/js/pages/redirects/components/redirect-form-fields.tsxresources/js/pages/server-network/index.tsxresources/js/pages/services/components/install.tsxresources/js/pages/site-tooling/index.tsxresources/js/types/network.d.tsresources/js/types/server-log.d.tsresources/views/ssh/services/firewall/ufw/apply-rules.blade.phpresources/views/ssh/wireguard/conf.blade.phpresources/views/ssh/wireguard/configure.blade.phpresources/views/ssh/wireguard/install.blade.phpresources/views/ssh/wireguard/latest-handshakes.blade.phpresources/views/ssh/wireguard/remove-network.blade.phpresources/views/ssh/wireguard/uninstall.blade.phpresources/views/wireguard/peer-conf.blade.phptests/Feature/NetworkFirewallTest.phptests/Feature/NetworkPeerTest.phptests/Feature/NetworkProviderSyncTest.phptests/Feature/NetworkSyncTest.phptests/Feature/NetworkTest.phptests/Feature/ProviderPrivateNetworkTest.phptests/Feature/UniqueQueueTest.phptests/Unit/Actions/Service/UninstallTest.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Jobs/Network/SyncNetworkServerJob.php (1)
46-63: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the failure transition against concurrent removal.
A removal can set the member to
LEAVINGwhile this sync is in flight;$this->member->save()then overwrites it asFAILED, preventing the teardown path from proceeding. Update only synchronising states and broadcast only if that conditional update succeeds.Proposed fix
- $this->member->status = NetworkServerStatus::FAILED; - $this->member->save(); - $this->broadcastMember(); + $updated = NetworkServer::query() + ->whereKey($this->member->id) + ->whereIn('status', [ + NetworkServerStatus::PENDING, + NetworkServerStatus::UPDATING, + ]) + ->update(['status' => NetworkServerStatus::FAILED]); + + if ($updated === 1) { + $this->broadcastMember(); + }As per path instructions, the required
UniqueQueuepattern does not protect this database state transition from concurrent removal requests.🤖 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 `@app/Jobs/Network/SyncNetworkServerJob.php` around lines 46 - 63, Update the failure transition in SyncNetworkServerJob::failed so it conditionally changes the member from the synchronising state to FAILED at the database level, preventing an existing LEAVING state from being overwritten. Broadcast the member only when that conditional update succeeds; preserve the teardown branch and status recomputation behavior.Source: Path instructions
🤖 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 `@app/Models/Server.php`:
- Around line 164-172: Update the after-commit query in the callback around
DispatchNetworkServer::toPresent() to retain a predicate excluding NetworkServer
records whose status is LEAVING when reloading the IDs. Keep the existing ID
filtering, relationships, and synchronization behavior unchanged for non-LEAVING
siblings.
In `@app/Models/ServerIpAddress.php`:
- Around line 71-75: Move the custom network membership cleanup from the IP
address before-commit/deleted handling into a DB::afterCommit callback scheduled
after the DELETING status save completes. Keep the existing NetworkServer query
and RemoveServerFromNetwork invocation unchanged within that deferred callback,
ensuring cleanup runs only after the transaction commits.
In `@app/ServerProviders/DigitalOcean.php`:
- Around line 131-132: Update the pagination flow around $hasNext and $page so
that a response with a next page when self::MAX_PAGES is reached throws a sync
error instead of ending successfully. Preserve normal page advancement below the
cap, and ensure the exception propagates through the sync path so pruning is
skipped.
---
Outside diff comments:
In `@app/Jobs/Network/SyncNetworkServerJob.php`:
- Around line 46-63: Update the failure transition in
SyncNetworkServerJob::failed so it conditionally changes the member from the
synchronising state to FAILED at the database level, preventing an existing
LEAVING state from being overwritten. Broadcast the member only when that
conditional update succeeds; preserve the teardown branch and status
recomputation behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3b4ec84a-8a1c-4b41-bca4-4760dea33228
📒 Files selected for processing (41)
app/Actions/Network/AddServersToNetwork.phpapp/Actions/Network/AllocateNetworkBlock.phpapp/Actions/Network/CreateNetwork.phpapp/Actions/Network/CreateWireGuardMembers.phpapp/Actions/Network/DispatchNetworkServerSync.phpapp/Actions/Network/FinalizeServerNetworkRules.phpapp/Actions/Network/MaterializeServerNetworkRules.phpapp/Actions/Network/RemoveServerFromNetwork.phpapp/Actions/Server/EditServer.phpapp/Console/Commands/ReconcileNetworksCommand.phpapp/Http/Controllers/NetworkController.phpapp/Http/Controllers/NetworkPeerController.phpapp/Jobs/Network/ApplyNetworkFirewallJob.phpapp/Jobs/Network/PollPeerHandshakesJob.phpapp/Jobs/Network/SyncNetworkServerJob.phpapp/Models/Network.phpapp/Models/NetworkPeer.phpapp/Models/NetworkServer.phpapp/Models/Server.phpapp/Models/ServerIpAddress.phpapp/Models/ServerNetworkRule.phpapp/ServerProviders/DigitalOcean.phpapp/ServerProviders/Hetzner.phpapp/ServerProviders/Vultr.phpapp/Services/Firewall/Ufw.phpapp/Services/VPN/WireGuard.phpapp/Tables/Servers/ServerNetworkRuleTable.phpapp/ValidationRules/WireGuardPublicKeyRule.phpdatabase/factories/NetworkFactory.phpdatabase/factories/NetworkServerFactory.phpresources/js/components/app-sidebar.tsxresources/js/hooks/use-server-with-private-ip.tsresources/js/pages/firewall/index.tsxresources/js/pages/networks/components/add-server.tsxresources/js/pages/networks/components/create-network.tsxresources/js/pages/networks/components/peer-config.tsxresources/js/pages/networks/settings.tsxresources/js/types/network.d.tstests/Feature/NetworkPeerTest.phptests/Feature/NetworkProviderSyncTest.phptests/Feature/NetworkTest.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
app/Services/VPN/WireGuard.php (1)
207-220: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winClean up the temporary secret on every failure path.
The
tryblock starts after the secondput(). If writing,chmod(), or the upload preparation fails, the temporary file containing the WireGuard private key is not deleted. Move the complete create/write/chmod/upload sequence insidetry/finallyand fail closed when permission or write operations fail.Suggested fix
$tmpName = 'wg-'.Str::random(20); $disk = Storage::disk('local'); - $disk->put($tmpName, ''); - $path = $disk->path($tmpName); - chmod($path, 0600); - $disk->put($tmpName, $content); - try { + $disk->put($tmpName, ''); + $path = $disk->path($tmpName); + if (! chmod($path, 0600)) { + throw new \RuntimeException('Unable to restrict WireGuard temporary file permissions.'); + } + $disk->put($tmpName, $content); $ssh->upload($path, $remote, 'root'); } finally { $disk->delete($tmpName);🤖 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 `@app/Services/VPN/WireGuard.php` around lines 207 - 220, Update uploadConf so the entire temporary-file creation, write, permission-setting, and SSH upload sequence is inside the existing try/finally, ensuring the generated temporary secret is deleted on any failure. Validate the initial and content writes plus chmod succeed, and fail closed by throwing or propagating an error when any operation fails before cleanup.app/Jobs/Network/PollPeerHandshakesJob.php (1)
31-45: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSelect a member with a ready WireGuard service.
reachableMember()only filters membership and server status. If the first ACTIVE member has no READY WireGuard service,handle()returns without trying another eligible member, even when one is available. This silently disables handshake polling for the entire network.Select or iterate through reachable members until a READY WireGuard service is found, and cover this fallback case with a feature test.
Also applies to: 87-94
🤖 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 `@app/Jobs/Network/PollPeerHandshakesJob.php` around lines 31 - 45, Update PollPeerHandshakesJob::handle and its member-selection flow so it iterates through reachable members until finding one whose WireGuard service is a READY Service, rather than returning after the first ineligible member; then apply latestHandshakes using that handler and add a feature test covering fallback to a later eligible member.app/Jobs/Network/SyncNetworkServerJob.php (1)
31-43: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftGuard queued jobs against stale membership state.
The job performs synchronisation without reloading or atomically claiming the current
NetworkServerstatus. A queued sync can therefore run after removal changes the member toLEAVING, whilefailed()can later overwrite a newer state withFAILED.Refresh and atomically claim the expected state before SSH work (
UPDATINGfor sync andLEAVINGfor teardown), and only update/broadcast failure when that transition still succeeds.Also applies to: 46-63
🤖 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 `@app/Jobs/Network/SyncNetworkServerJob.php` around lines 31 - 43, Update SyncNetworkServerJob::handle and its teardown/synchronization flow to refresh the current NetworkServer and atomically claim the expected status before any SSH work: UPDATING for sync and LEAVING for teardown. Abort when the claim fails because membership state is stale or already changed. In failed(), only transition to FAILED and broadcast the failure when the atomic state transition still succeeds, preventing stale jobs from overwriting newer membership state.app/Actions/Network/AddServersToNetwork.php (1)
37-41: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSerialise the WireGuard port-conflict check.
The conflict query runs before any shared lock. Concurrent adds—or an add racing network creation—can both pass validation and assign the same UDP port to one server, leaving one interface unable to bind. Lock the project row and run
validateNoPortConflict()inside this transaction.Proposed fix
- if ($network->type === NetworkType::WIREGUARD) { - $this->validateNoPortConflict($network, $input['servers'] ?? []); - }$newMemberIds = DB::transaction(function () use ($network, $input): array { + if ($network->type === NetworkType::WIREGUARD) { + Project::query()->whereKey($network->project_id)->lockForUpdate()->firstOrFail(); + $this->validateNoPortConflict($network, $input['servers']); + } + return $network->type === NetworkType::WIREGUARD ? $this->addWireGuard($network, $input) : $this->addCustom($network, $input); });🤖 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 `@app/Actions/Network/AddServersToNetwork.php` around lines 37 - 41, Update the transaction in AddServersToNetwork so it first acquires the project row’s shared lock, then runs validateNoPortConflict() and the WireGuard add flow while that lock is held. Ensure network creation and concurrent server additions serialize through the same project lock, while leaving the addCustom path unchanged.app/Actions/Network/RemoveServerFromNetwork.php (1)
20-28: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the removal lifecycle atomic before dispatching remote work.
teardown()persistsLEAVINGand queues a job before sibling synchronisation/firewall handling and status recomputation complete. A later failure leaves partial database state and already-dispatched work. Transactionally persist the lifecycle changes, then defer queue/firewall side effects until commit.As per coding guidelines and path instructions, “Wrap multi-step data mutations in database transactions to prevent partial state.”
🤖 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 `@app/Actions/Network/RemoveServerFromNetwork.php` around lines 20 - 28, Update the removal flow around RemoveServerFromNetwork and its teardown, resyncMembers, firewall handle, and recompute handle calls to execute database lifecycle mutations within a transaction, ensuring all persistence and status recomputation succeeds atomically. Defer teardown’s queued job and the subsequent synchronization/firewall side effects until the transaction commits, so failures roll back state without dispatching remote work.Sources: Coding guidelines, Path instructions
🤖 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 `@app/ServerProviders/DigitalOcean.php`:
- Around line 131-132: Make pagination failures abort synchronization instead of
returning partial provider state: in app/ServerProviders/DigitalOcean.php lines
131-132, throw PrivateNetworkSyncError when a next link exists at the MAX_PAGES
limit; in app/ServerProviders/Hetzner.php lines 134-135, throw when next_page is
not greater than the current page; and in app/ServerProviders/Vultr.php lines
143-144, throw when the pagination cursor repeats. Preserve normal advancement
and completion behavior for valid pagination.
In `@database/factories/NetworkServerFactory.php`:
- Around line 24-27: Update NetworkServerFactory so the value assigned by
hostAddress() is generated within the CIDR produced for the related
Network::factory() record. Coordinate network and member IP generation, ensuring
factory-created memberships always use an address inside the generated network’s
/24 rather than independently selecting from the broader range.
In `@resources/js/pages/networks/settings.tsx`:
- Around line 34-40: Update the successful save flow around form.put so it
resets the form baseline and clears its dirty state after the request succeeds.
Ensure the existing isDirty guard in the useEffect can accept subsequent
realtime network.name updates and the Save button is hidden after a successful
save, without changing unsaved-edit behavior.
---
Outside diff comments:
In `@app/Actions/Network/AddServersToNetwork.php`:
- Around line 37-41: Update the transaction in AddServersToNetwork so it first
acquires the project row’s shared lock, then runs validateNoPortConflict() and
the WireGuard add flow while that lock is held. Ensure network creation and
concurrent server additions serialize through the same project lock, while
leaving the addCustom path unchanged.
In `@app/Actions/Network/RemoveServerFromNetwork.php`:
- Around line 20-28: Update the removal flow around RemoveServerFromNetwork and
its teardown, resyncMembers, firewall handle, and recompute handle calls to
execute database lifecycle mutations within a transaction, ensuring all
persistence and status recomputation succeeds atomically. Defer teardown’s
queued job and the subsequent synchronization/firewall side effects until the
transaction commits, so failures roll back state without dispatching remote
work.
In `@app/Jobs/Network/PollPeerHandshakesJob.php`:
- Around line 31-45: Update PollPeerHandshakesJob::handle and its
member-selection flow so it iterates through reachable members until finding one
whose WireGuard service is a READY Service, rather than returning after the
first ineligible member; then apply latestHandshakes using that handler and add
a feature test covering fallback to a later eligible member.
In `@app/Jobs/Network/SyncNetworkServerJob.php`:
- Around line 31-43: Update SyncNetworkServerJob::handle and its
teardown/synchronization flow to refresh the current NetworkServer and
atomically claim the expected status before any SSH work: UPDATING for sync and
LEAVING for teardown. Abort when the claim fails because membership state is
stale or already changed. In failed(), only transition to FAILED and broadcast
the failure when the atomic state transition still succeeds, preventing stale
jobs from overwriting newer membership state.
In `@app/Services/VPN/WireGuard.php`:
- Around line 207-220: Update uploadConf so the entire temporary-file creation,
write, permission-setting, and SSH upload sequence is inside the existing
try/finally, ensuring the generated temporary secret is deleted on any failure.
Validate the initial and content writes plus chmod succeed, and fail closed by
throwing or propagating an error when any operation fails before cleanup.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3b4ec84a-8a1c-4b41-bca4-4760dea33228
📒 Files selected for processing (41)
app/Actions/Network/AddServersToNetwork.phpapp/Actions/Network/AllocateNetworkBlock.phpapp/Actions/Network/CreateNetwork.phpapp/Actions/Network/CreateWireGuardMembers.phpapp/Actions/Network/DispatchNetworkServerSync.phpapp/Actions/Network/FinalizeServerNetworkRules.phpapp/Actions/Network/MaterializeServerNetworkRules.phpapp/Actions/Network/RemoveServerFromNetwork.phpapp/Actions/Server/EditServer.phpapp/Console/Commands/ReconcileNetworksCommand.phpapp/Http/Controllers/NetworkController.phpapp/Http/Controllers/NetworkPeerController.phpapp/Jobs/Network/ApplyNetworkFirewallJob.phpapp/Jobs/Network/PollPeerHandshakesJob.phpapp/Jobs/Network/SyncNetworkServerJob.phpapp/Models/Network.phpapp/Models/NetworkPeer.phpapp/Models/NetworkServer.phpapp/Models/Server.phpapp/Models/ServerIpAddress.phpapp/Models/ServerNetworkRule.phpapp/ServerProviders/DigitalOcean.phpapp/ServerProviders/Hetzner.phpapp/ServerProviders/Vultr.phpapp/Services/Firewall/Ufw.phpapp/Services/VPN/WireGuard.phpapp/Tables/Servers/ServerNetworkRuleTable.phpapp/ValidationRules/WireGuardPublicKeyRule.phpdatabase/factories/NetworkFactory.phpdatabase/factories/NetworkServerFactory.phpresources/js/components/app-sidebar.tsxresources/js/hooks/use-server-with-private-ip.tsresources/js/pages/firewall/index.tsxresources/js/pages/networks/components/add-server.tsxresources/js/pages/networks/components/create-network.tsxresources/js/pages/networks/components/peer-config.tsxresources/js/pages/networks/settings.tsxresources/js/types/network.d.tstests/Feature/NetworkPeerTest.phptests/Feature/NetworkProviderSyncTest.phptests/Feature/NetworkTest.php
RichardAnderson
marked this pull request as ready for review
July 25, 2026 10:09
Contributor
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 (2)
app/Models/Server.php (1)
130-175: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winStatic sibling-resync accumulator can leak entries if deletion fails.
$networkSiblingsToResyncis a class-level static array populated in the firstdeletinghook, and only cleared indeleted. The seconddeletinghook (unchanged, below) wraps the rest of the teardown in a transaction and rethrows on failure, which aborts the delete pipeline beforedeletedever fires — leaving that server's entry in the static array permanently. In a long-running queue worker process this grows unbounded across failed deletions.
ServerIpAddress::$reapplyNetworkIds(same PR) uses a non-static, per-instance property for the exact same producer/consumer pattern acrossdeleting→deleted— the same instance is passed through both hooks, so there's no need for a static keyed array here either.🐛 Proposed fix
- /** `@var` array<int, array<int, int>> */ - protected static array $networkSiblingsToResync = []; + /** `@var` array<int, int> */ + public array $networkSiblingsToResync = []; public static function boot(): void { parent::boot(); static::deleting(function (Server $server): void { $siblings = []; ... - static::$networkSiblingsToResync[$server->id] = array_keys($siblings); + $server->networkSiblingsToResync = array_keys($siblings); }); static::deleted(function (Server $server): void { - $ids = static::$networkSiblingsToResync[$server->id] ?? []; - unset(static::$networkSiblingsToResync[$server->id]); + $ids = $server->networkSiblingsToResync; ...🤖 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 `@app/Models/Server.php` around lines 130 - 175, Replace the static keyed $networkSiblingsToResync accumulator with a non-static per-instance property on Server, and update the deleting and deleted hooks to write and consume that property directly. Remove the static declaration and cleanup logic, while preserving the existing sibling collection and after-commit resynchronization behavior.app/Actions/Network/AddServersToNetwork.php (1)
63-82: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard WireGuard port conflicts with a port-level lock.
validateNoPortConflict()reads other WireGuard networks by port, but this action only locks the currentProjectandNetworkrows. Concurrent adds to different WireGuard networks with the same port can both pass the check before either commits, so the second config creation can fail later on the host with a duplicate UDP port bind. Wrap the conflict check and membership creation inside a named lock keyed by the port.🤖 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 `@app/Actions/Network/AddServersToNetwork.php` around lines 63 - 82, The addWireGuard method must serialize WireGuard operations sharing the same port. Acquire a named lock keyed by the configured port before calling validateNoPortConflict, and keep both that validation and members->create within the lock’s callback/scope so concurrent networks cannot pass the check simultaneously.
🤖 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 `@app/Actions/Network/AddServersToNetwork.php`:
- Around line 63-82: The addWireGuard method must serialize WireGuard operations
sharing the same port. Acquire a named lock keyed by the configured port before
calling validateNoPortConflict, and keep both that validation and
members->create within the lock’s callback/scope so concurrent networks cannot
pass the check simultaneously.
In `@app/Models/Server.php`:
- Around line 130-175: Replace the static keyed $networkSiblingsToResync
accumulator with a non-static per-instance property on Server, and update the
deleting and deleted hooks to write and consume that property directly. Remove
the static declaration and cleanup logic, while preserving the existing sibling
collection and after-commit resynchronization behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fc3a2f69-a3f8-47f5-a521-78bb43eb6d47
📒 Files selected for processing (11)
app/Actions/Network/AddServersToNetwork.phpapp/Jobs/Network/PollPeerHandshakesJob.phpapp/Jobs/Network/SyncNetworkServerJob.phpapp/Models/Server.phpapp/Models/ServerIpAddress.phpapp/ServerProviders/DigitalOcean.phpapp/ServerProviders/Hetzner.phpapp/ServerProviders/Vultr.phpapp/Services/VPN/WireGuard.phpresources/js/pages/networks/settings.tsxtests/Feature/NetworkPeerTest.php
Contributor
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)
app/Models/Server.php (1)
137-173: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftMove network resynchronisation into an Action.
Lines 137-173 place network-specific discovery and lifecycle orchestration inside
Server::boot(). Delegate this work to an Action while preserving the existing after-commit callback andLEAVINGsafeguards.🤖 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 `@app/Models/Server.php` around lines 137 - 173, Extract the network sibling discovery and post-deletion resynchronisation logic from Server::boot() into a dedicated Action, and invoke that Action from the deleting/deleted lifecycle flow. Preserve the existing WireGuard filtering, sibling deduplication, LEAVING exclusions, and DB::afterCommit dispatch behavior.Source: Path instructions
🤖 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 `@app/Models/Server.php`:
- Around line 137-173: Extract the network sibling discovery and post-deletion
resynchronisation logic from Server::boot() into a dedicated Action, and invoke
that Action from the deleting/deleted lifecycle flow. Preserve the existing
WireGuard filtering, sibling deduplication, LEAVING exclusions, and
DB::afterCommit dispatch behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6c40cb08-b6f9-42cd-9d91-51e1882dea31
📒 Files selected for processing (1)
app/Models/Server.php
Member
|
• ## Review result: request changes Specs
Standards and security
|
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
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.
Adds project-scoped private networks that link servers either over Vito-managed WireGuard tunnels (with peer devices for laptops and CI runners), over private IPs you already have, or by discovering and syncing existing VPCs from AWS, DigitalOcean, Hetzner, Linode, and Vultr, each network carrying its own firewall rules that Vito keeps applied across every member.
View networks
View network details
Summary by CodeRabbit