Validate location address by moubctez · Pull Request #2388 · DefGuard/defguard

moubctez · 2026-03-17T12:51:22Z

…tion_address

Copilot

Pull request overview

Implements stricter validation for WireGuard location “gateway IP” addresses so that network/broadcast addresses (e.g. 192.168.44.0/29) are rejected, and refactors WireguardNetwork address handling to centralize validation.

Changes:

Introduces WireguardNetwork::set_address(...) validation and makes WireguardNetwork.address private with an address() accessor.
Updates core/setup/managers/tests to use the new address API (set_address / address()), and adjusts test data away from network addresses.
Standardizes many DB helpers to return sqlx::Result and moves some device→network lookup helpers onto WireguardNetwork.

Reviewed changes

Copilot reviewed 68 out of 69 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
crates/defguard_static_ip/src/lib.rs	Updates tests to use new address builder/accessor APIs.
crates/defguard_setup/tests/wizard_state.rs	Adjusts seeded WireGuard location creation to new address API.
crates/defguard_setup/tests/auto_adoption_wizard.rs	Updates auto-adoption test seeding for new `WireguardNetwork` APIs.
crates/defguard_setup/src/handlers/auto_wizard.rs	Uses `set_address` when applying VPN settings in wizard handler.
crates/defguard_setup/src/auto_adoption.rs	Refactors network creation to `set_address`; minor pattern cleanup.
crates/defguard_session_manager/tests/common/mod.rs	Updates location/device setup to use `set_address` and new helper.
crates/defguard_proxy_manager/src/servers/enrollment.rs	Uses `WireguardNetwork::find_network_device_networks` instead of `Device` method.
crates/defguard_gateway_manager/src/handler.rs	Switches to `address()` accessor when generating gateway config updates.
crates/defguard_core/tests/integration/api/wireguard_network_import.rs	Updates import tests for new address validation + accessor.
crates/defguard_core/tests/integration/api/wireguard_network_devices.rs	Uses new `WireguardNetwork::find_network_device_networks` helper.
crates/defguard_core/tests/integration/api/wireguard.rs	Updates integration tests to avoid network-address inputs.
crates/defguard_core/tests/integration/api/acl/rules.rs	Updates WireGuard network construction call sites for new signature.
crates/defguard_core/tests/integration/api/acl/mod.rs	Cleans imports after `WireguardNetwork::new` signature change.
crates/defguard_core/src/wg_config.rs	Uses `set_address`, sets mtu/fwmark post-construction, updates tests.
crates/defguard_core/src/location_management/tests.rs	Refactors test network address updates to use `set_address`.
crates/defguard_core/src/location_management/mod.rs	Uses `address()` accessor; refactors tests to new builder API.
crates/defguard_core/src/location_management/allowed_peers.rs	Simplifies sqlx result types and updates test setup to new address API.
crates/defguard_core/src/lib.rs	Refactors dev-env and init_vpn_location to use `set_address`.
crates/defguard_core/src/handlers/wireguard.rs	Uses `try_set_address` / `set_address` on create/modify paths.
crates/defguard_core/src/handlers/user.rs	Switches to `sqlx::Result` signatures.
crates/defguard_core/src/handlers/ssh_authorized_keys.rs	Switches to `sqlx::Result` signatures.
crates/defguard_core/src/handlers/network_devices.rs	Uses new `WireguardNetwork::find_network_device_networks` and `address()` accessor.
crates/defguard_core/src/handlers/mod.rs	Maps `IpNetworkError` into 400 responses via `WebError::IpNetwork`.
crates/defguard_core/src/error.rs	Adds `WebError::IpNetwork` for `ipnetwork::IpNetworkError`.
crates/defguard_core/src/enterprise/limits.rs	Switches to `sqlx::Result` signatures.
crates/defguard_core/src/enterprise/license.rs	Normalizes DB error typing to `sqlx::Error`.
crates/defguard_core/src/enterprise/ldap/model.rs	Switches to `sqlx::Result` signatures; minor formatting.
crates/defguard_core/src/enterprise/ldap/error.rs	Normalizes DB error typing to `sqlx::Error`.
crates/defguard_core/src/enterprise/firewall/tests/unapplied_rules.rs	Updates test locations to use `set_address`.
crates/defguard_core/src/enterprise/firewall/tests/mod.rs	Uses `address()` accessor and updates location setup to new API.
crates/defguard_core/src/enterprise/firewall/tests/gh1868.rs	Updates test locations to use `set_address` and `address()`.
crates/defguard_core/src/enterprise/firewall/tests/expired_rules.rs	Updates test locations to use `set_address` where needed.
crates/defguard_core/src/enterprise/firewall/tests/disabled_rules.rs	Updates test locations to use `set_address`.
crates/defguard_core/src/enterprise/firewall/tests/destination.rs	Updates test locations to use `set_address`.
crates/defguard_core/src/enterprise/firewall/tests/all_locations.rs	Updates test locations to use `set_address`.
crates/defguard_core/src/enterprise/firewall/mod.rs	Uses `address()` accessor; normalizes `sqlx::Result` return types.
crates/defguard_core/src/enterprise/directory_sync/tests.rs	Updates location creation to use `set_address`.
crates/defguard_core/src/enterprise/directory_sync/mod.rs	Normalizes DB error typing to `sqlx::Error`.
crates/defguard_core/src/enterprise/db/models/openid_provider.rs	Switches to `sqlx::Result` signatures.
crates/defguard_core/src/enterprise/db/models/api_tokens.rs	Switches to `sqlx::Result` signatures.
crates/defguard_core/src/enterprise/db/models/activity_log_stream.rs	Switches to `sqlx::Result` signatures.
crates/defguard_core/src/enterprise/db/models/acl/tests.rs	Updates imports and network creation to match new WireGuard API.
crates/defguard_core/src/enterprise/db/models/acl.rs	Normalizes DB error typing; uses `WireguardNetwork::all_for_rule`.
crates/defguard_core/src/db/models/webhook.rs	Switches to `sqlx::Result` signatures.
crates/defguard_core/src/db/models/enrollment.rs	Normalizes DB error typing to `sqlx::Error`.
crates/defguard_common/src/types/user_info.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/wireguard/tests.rs	Adds unit/integration test coverage for address validation behavior.
crates/defguard_common/src/db/models/wireguard.rs	Core refactor: private `address`, `address()` accessor, `set_address` validation, new constructor signature, new helper queries.
crates/defguard_common/src/db/models/webauthn.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/vpn_session_stats.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/vpn_client_session.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/user.rs	Normalizes DB error typing and switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/setup_auto_adoption.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/session.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/polling_token.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/oauth2token.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/oauth2client.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/oauth2authorizedapp.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/migration_wizard.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/mfa_info.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/initial_setup_wizard.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/group.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/gateway.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/device_login.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/device.rs	Switches to `sqlx::Result`; replaces device method with `WireguardNetwork` helper usage.
crates/defguard_common/src/db/models/biometric_auth.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/authentication_key.rs	Switches to `sqlx::Result` signatures.
crates/defguard_common/src/db/models/auth_code.rs	Switches to `sqlx::Result` signatures.
.sqlx/query-c58c7b4dc7463a93895b17d591e3e4a83ac3623590674e47bb1a1dbf9c25d77f.json	Removes obsolete sqlx prepared query metadata.

Files not reviewed (1)

.sqlx/query-c58c7b4dc7463a93895b17d591e3e4a83ac3623590674e47bb1a1dbf9c25d77f.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/defguard_common/src/db/models/wireguard.rs

+    /// Try to set `address` from comma-separated string of addresses.
+    pub fn try_set_address(mut self, address: &str) -> Result<Self, IpNetworkError> {
+        self.address = Vec::new();
+        for addr in address.split(',') {
+            self.address.push(addr.trim().parse()?);
+        }
        if address.is_empty() {
-            return Err(IpNetworkError::InvalidAddr("invalid address".into()));
+            Err(IpNetworkError::InvalidAddr("empty address".into()))
+        } else {
+            Ok(self)
+        }


crates/defguard_core/src/handlers/wireguard.rs

+    let mut network = WireguardNetwork::new(
        data.name,
-        parse_address_list(&data.address),
        data.port,
        data.endpoint,
        data.dns,
-        data.mtu,
-        data.fwmark,
        allowed_ips,
        data.allow_all_groups,
-        data.keepalive_interval,
-        data.peer_disconnect_threshold,
        data.acl_enabled,
        data.acl_default_allow,
        data.location_mfa_mode,
        data.service_location_mode,
-    );
+    )
+    .try_set_address(&data.address)?;
+    network.mtu = data.mtu;


crates/defguard_common/src/db/models/wireguard.rs

+    pub fn new<V>(
        name: String,
-        address: Vec<IpNetwork>,
        port: i32,
        endpoint: String,
        dns: Option<String>,
-        mtu: i32,
-        fwmark: i64,
-        allowed_ips: Vec<IpNetwork>,
+        allowed_ips: V,
        allow_all_groups: bool,
-        keepalive_interval: i32,
-        peer_disconnect_threshold: i32,
        acl_enabled: bool,
        acl_default_allow: bool,
        location_mfa_mode: LocationMfaMode,
        service_location_mode: ServiceLocationMode,
-    ) -> Self {
+    ) -> Self


crates/defguard_common/src/db/models/wireguard/tests.rs

+    .unwrap()]);
+    assert!(result.is_err());
+}
+


moubctez added 7 commits March 17, 2026 11:12

WireguardNetwork address getter/setter

b80daff

Reduce WireguardNetwork::new

ddfef85

Rewrite WireguardNetwork::try_set_address

4677c70

Fix tests

7b3b7f7

Merge branch 'dev' of github.com:DefGuard/defguard into validate_loca…

4cee0e9

…tion_address

Move wireguard tests to a separate file

870d792

Add test for set_address

3eca8ec

wojcik91 requested a review from Copilot March 17, 2026 21:04

Copilot started reviewing on behalf of wojcik91 March 17, 2026 21:04 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Validate location address#2388

Validate location address#2388
moubctez wants to merge 7 commits intodevfrom
validate_location_address

moubctez commented Mar 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

moubctez commented Mar 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants