Skip to content

Conversation

@MatthiasLanter
Copy link

@MatthiasLanter MatthiasLanter commented Sep 4, 2025

Add basic WireGuard functions to connect and disconnect a tunnel based on WireGuard configurations.
The connection name is configured and read by the comment ‘# Name = ’ in the configuration file.

Summary by Sourcery

Add basic WireGuard integration by implementing a new API module for service status and tunnel control and integrating it into the tray icon menu with status display and connect/disconnect actions.

New Features:

  • Introduce wg_api module to retrieve WireGuard service state, list configurations, and manage tunnel connections
  • Extend the tray icon menu to include a WireGuard VPN section showing each tunnel’s status and offering connect/disconnect actions
  • Add connectWG and disconnectWG handlers to invoke wg_api functions and refresh the tray UI

@MatthiasLanter MatthiasLanter requested review from a team as code owners September 4, 2025 14:30
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Sep 4, 2025

Reviewer's Guide

Adds basic WireGuard VPN support by introducing a dedicated API module for configuration parsing, status checks, and control commands, and by extending the system tray icon interface with a dynamic WireGuard section that lists tunnels and offers connect/disconnect actions, with automatic state refresh.

Sequence diagram for WireGuard tunnel connect/disconnect from tray menu

sequenceDiagram
    actor User
    participant TrayIcon
    participant wg_api
    User->>TrayIcon: Select "Enable" or "Disable" for a WireGuard tunnel
    TrayIcon->>wg_api: enable_wg(wgconfig) or disable_wg(wgconfig)
    wg_api-->>TrayIcon: Execute command, update tunnel state
    TrayIcon->>TrayIcon: updateinfo()
    TrayIcon->>wg_api: wg_dictionary()
    wg_api-->>TrayIcon: Return updated tunnel states
    TrayIcon-->>User: Update tray menu with new tunnel status
Loading

File-Level Changes

Change Details Files
Introduce a WireGuard API module for config parsing and tunnel control
  • Define WG_CONFIG_PATH and import necessary modules
  • Implement wg_service_state(), wg_status(), and wg_dictionary() functions
  • Add enable_wg() and disable_wg() wrappers for wg-quick commands
NetworkMgr/wg_api.py
Extend tray icon menu to display WireGuard tunnels
  • Add a WireGuard VPN section in nm_menu() when configs are present
  • Iterate over wginfo['configs'] and append stateful menu items
  • Connect menu items to connectWG/disconnectWG handlers
NetworkMgr/trayicon.py
Add connect/disconnect handlers and state refresh integration
  • Implement connectWG() and disconnectWG() methods calling the API and updateinfo()
  • Call wg_dictionary() inside updateinfo() to refresh self.wginfo
NetworkMgr/trayicon.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

Blocking issues:

  • Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
  • Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
  • Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
  • Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)

General comments:

  • Use a with open(...) context manager when reading WireGuard config files to ensure file handles are closed properly.
  • Avoid shell=True with dynamically interpolated wgconfig in run() to reduce injection risk—pass commands as lists or sanitize inputs.
  • Run enableWG/disableWG calls off the main GTK thread (e.g., in a worker thread) to prevent blocking the UI during long-running subprocesses.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Use a with open(...) context manager when reading WireGuard config files to ensure file handles are closed properly.
- Avoid shell=True with dynamically interpolated wgconfig in run() to reduce injection risk—pass commands as lists or sanitize inputs.
- Run enableWG/disableWG calls off the main GTK thread (e.g., in a worker thread) to prevent blocking the UI during long-running subprocesses.

## Individual Comments

### Comment 1
<location> `NetworkMgr/wg_api.py:9` </location>
<code_context>
+prefix = '/usr/local' if system() == 'FreeBSD' else sys.prefix
+wgconfigpath = prefix + '/etc/wireguard/'
+
+def wg_service_state():
+    run('service wireguard status', shell=True)
+
+def wireguardDictionary():
</code_context>

<issue_to_address>
wg_service_state does not return the service status.

The function executes the command but does not return its output, resulting in None being stored. Please update the function to return the command result.
</issue_to_address>

### Comment 2
<location> `NetworkMgr/wg_api.py:23` </location>
<code_context>
+    if os.path.exists(wgconfigpath):
+        wgconfigs = sorted(os.listdir(wgconfigpath))
+        for wgconfig in wgconfigs:
+            content = open(wgconfigpath + wgconfig)
+            for line in content:
+                if "# Name = " in line:
+                    wgName = line.split('=')[1].strip()
</code_context>

<issue_to_address>
File handle 'content' is not closed after reading.

Use a 'with' statement to ensure the file is closed automatically after reading.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
        for wgconfig in wgconfigs:
            content = open(wgconfigpath + wgconfig)
            for line in content:
                if "# Name = " in line:
                    wgName = line.split('=')[1].strip()
            wgDevice = wgconfig.replace('.conf', '')
            wgState = wg_status(wgDevice)
            seconddictionary = { 'state': wgState, 'info': wgName }
            configs[wgDevice] = seconddictionary
=======
        for wgconfig in wgconfigs:
            with open(wgconfigpath + wgconfig) as content:
                for line in content:
                    if "# Name = " in line:
                        wgName = line.split('=')[1].strip()
            wgDevice = wgconfig.replace('.conf', '')
            wgState = wg_status(wgDevice)
            seconddictionary = { 'state': wgState, 'info': wgName }
            configs[wgDevice] = seconddictionary
>>>>>>> REPLACE

</suggested_fix>

### Comment 3
<location> `NetworkMgr/wg_api.py:25` </location>
<code_context>
+        for wgconfig in wgconfigs:
+            content = open(wgconfigpath + wgconfig)
+            for line in content:
+                if "# Name = " in line:
+                    wgName = line.split('=')[1].strip()
+            wgDevice = wgconfig.replace('.conf', '')
+            wgState = wg_status(wgDevice)
</code_context>

<issue_to_address>
Potential for 'wgName' to be undefined if no matching line is found.

Initialize 'wgName' with a default value before the loop to prevent errors if no matching line is found.
</issue_to_address>

### Comment 4
<location> `NetworkMgr/wg_api.py:53` </location>
<code_context>
+    run(f'wg-quick up {wgconfig}', shell=True)
+
+
+def wg_status(wgconfig):
+    # out = Popen(
+    #     f'wg show {wgconfig}',
+    #     shell=True, stdout=PIPE, stderr=PIPE,
+    #     universal_newlines=True
+    # )
+    result=run(['wg', 'show', wgconfig], stdout=PIPE, stderr=PIPE)
+    out = result.stdout.decode('utf-8')
+    error = result.stderr.decode('utf-8')
</code_context>

<issue_to_address>
No error handling for failed subprocess calls.

Check the subprocess return code or handle exceptions to ensure errors are managed appropriately.
</issue_to_address>

### Comment 5
<location> `NetworkMgr/trayicon.py:86` </location>
<code_context>
+                    wg_item = Gtk.MenuItem(_("%s Connected") % connection_info)
+                    wg_item.set_sensitive(False)
+                    self.menu.append(wg_item)
+                    disconnectwg_item = Gtk.ImageMenuItem(_(f"Disable {wgDev}"))
+                    disconnectwg_item.connect("activate", self.disconnectWG, wgDev)
+                    self.menu.append(disconnectwg_item)
</code_context>

<issue_to_address>
Use of Gtk.ImageMenuItem is deprecated in GTK 3+.

Switch to Gtk.MenuItem and add an icon manually to ensure compatibility with GTK 3+.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
                    disconnectwg_item = Gtk.ImageMenuItem(_(f"Disable {wgDev}"))
                    disconnectwg_item.connect("activate", self.disconnectWG, wgDev)
                    self.menu.append(disconnectwg_item)
=======
                    # Create a MenuItem with an icon manually (Gtk.ImageMenuItem is deprecated)
                    disconnectwg_item = Gtk.MenuItem()
                    box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=6)
                    icon = Gtk.Image.new_from_icon_name("network-offline-symbolic", Gtk.IconSize.MENU)
                    label = Gtk.Label(label=_(f"Disable {wgDev}"))
                    box.pack_start(icon, False, False, 0)
                    box.pack_start(label, False, False, 0)
                    box.show_all()
                    disconnectwg_item.add(box)
                    disconnectwg_item.connect("activate", self.disconnectWG, wgDev)
                    self.menu.append(disconnectwg_item)
>>>>>>> REPLACE

</suggested_fix>

## Security Issues

### Issue 1
<location> `NetworkMgr/wg_api.py:46` </location>

<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.

*Source: opengrep*
</issue_to_address>

### Issue 2
<location> `NetworkMgr/wg_api.py:46` </location>

<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.

```suggestion
    run(f'wg-quick down {wgconfig}', shell=False)
```

*Source: opengrep*
</issue_to_address>

### Issue 3
<location> `NetworkMgr/wg_api.py:50` </location>

<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.

*Source: opengrep*
</issue_to_address>

### Issue 4
<location> `NetworkMgr/wg_api.py:50` </location>

<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.

```suggestion
    run(f'wg-quick up {wgconfig}', shell=False)
```

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@github-project-automation github-project-automation bot moved this from In Review to In Progress in Development Tracker Sep 4, 2025
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Copy link
Member

@ericbsd ericbsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend using an editor with Pylint or a similar tool, such as PyCharm, to ensure the code meets a certain standard.

Wireguard menu entries only if configuration exists and Wireguard service is stopped
@ericbsd
Copy link
Member

ericbsd commented Sep 22, 2025

@sourcery-ai review

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
  • Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
  • Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
  • Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)

General comments:

  • In wg_api.py you’re importing os from subprocess and referencing sys.prefix without importing sys; update the imports to import os and sys directly to avoid runtime errors.
  • Use a context manager (with open(...)) when reading WireGuard config files in wg_dictionary to ensure file handles are properly closed.
  • The trayicon logic checks wginfo['service'] against '"NO"' (including quotes), which likely never matches—adjust the comparison to the actual output of wg_service_state so the menu shows correctly.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In wg_api.py you’re importing os from subprocess and referencing sys.prefix without importing sys; update the imports to import os and sys directly to avoid runtime errors.
- Use a context manager (with open(...)) when reading WireGuard config files in wg_dictionary to ensure file handles are properly closed.
- The trayicon logic checks wginfo['service'] against '"NO"' (including quotes), which likely never matches—adjust the comparison to the actual output of wg_service_state so the menu shows correctly.

## Individual Comments

### Comment 1
<location> `NetworkMgr/wg_api.py:33-44` </location>
<code_context>
+    if os.path.exists(WG_CONFIG_PATH):
+        wgconfigs = sorted(os.listdir(WG_CONFIG_PATH))
+        for wgconfig in wgconfigs:
+            content = open(WG_CONFIG_PATH + wgconfig, encoding="utf-8")
+            wg_device = wgconfig.replace('.conf', '')
+            wg_state = wg_status(wg_device)
+            wg_name = wg_device
+            for line in content:
+                if "# Name = " in line:
+                    wg_name = line.split('=')[1].strip()
</code_context>

<issue_to_address>
**suggestion (bug_risk):** File handle 'content' is not closed after reading.

Use a 'with' statement when opening files to ensure they are closed automatically, even if an error occurs.

```suggestion
        for wgconfig in wgconfigs:
            wg_device = wgconfig.replace('.conf', '')
            wg_state = wg_status(wg_device)
            wg_name = wg_device
            with open(WG_CONFIG_PATH + wgconfig, encoding="utf-8") as content:
                for line in content:
                    if "# Name = " in line:
                        wg_name = line.split('=')[1].strip()
                        break

            seconddictionary = { 'state': wg_state, 'info': wg_name }
            configs[wg_device] = seconddictionary
```
</issue_to_address>

### Comment 2
<location> `NetworkMgr/wg_api.py:52` </location>
<code_context>
+
+def disable_wg(wgconfig):
+    """Function disable the specified WireGuard configuration (device)."""
+    run(f'wg-quick down {wgconfig}', shell=True, check=False)
+
+def enable_wg(wgconfig):
</code_context>

<issue_to_address>
**🚨 issue (security):** Use of shell=True with string interpolation may introduce security risks.

If 'wgconfig' is not trusted, this approach can allow shell injection. Use a list of arguments with 'run' to avoid this risk.
</issue_to_address>

### Comment 3
<location> `NetworkMgr/wg_api.py:56` </location>
<code_context>
+
+def enable_wg(wgconfig):
+    """Function enable the specified WireGuard configuration (device)."""
+    run(f'wg-quick up {wgconfig}', shell=True, check=False)
+
+def wg_status(wgconfig):
</code_context>

<issue_to_address>
**🚨 issue (security):** Potential shell injection risk with shell=True and interpolated arguments.

Consider passing arguments as a list to avoid shell injection risks.
</issue_to_address>

### Comment 4
<location> `NetworkMgr/wg_api.py:52` </location>
<code_context>
    run(f'wg-quick down {wgconfig}', shell=True, check=False)
</code_context>

<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.

*Source: opengrep*
</issue_to_address>

### Comment 5
<location> `NetworkMgr/wg_api.py:52` </location>
<code_context>
    run(f'wg-quick down {wgconfig}', shell=True, check=False)
</code_context>

<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.

```suggestion
    run(f'wg-quick down {wgconfig}', shell=False, check=False)
```

*Source: opengrep*
</issue_to_address>

### Comment 6
<location> `NetworkMgr/wg_api.py:56` </location>
<code_context>
    run(f'wg-quick up {wgconfig}', shell=True, check=False)
</code_context>

<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.

*Source: opengrep*
</issue_to_address>

### Comment 7
<location> `NetworkMgr/wg_api.py:56` </location>
<code_context>
    run(f'wg-quick up {wgconfig}', shell=True, check=False)
</code_context>

<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.

```suggestion
    run(f'wg-quick up {wgconfig}', shell=False, check=False)
```

*Source: opengrep*
</issue_to_address>

### Comment 8
<location> `NetworkMgr/trayicon.py:71` </location>
<code_context>
    def nm_menu(self):
        self.menu = Gtk.Menu()
        if len(self.wginfo['configs'])> 0 and self.wginfo['service'] == '"NO"':
            wg_title = Gtk.MenuItem()
            wg_title.set_label(_("WireGuard VPN"))
            wg_title.set_sensitive(False)
            self.menu.append(wg_title)
            self.menu.append(Gtk.SeparatorMenuItem())
            wg_devices = self.wginfo['configs']
            for wg_dev in wg_devices:
                connection_state = wg_devices[wg_dev]['state']
                connection_info = wg_devices[wg_dev]['info']
                if connection_state == "Connected":
                    wg_item = Gtk.MenuItem(_("%s Connected") % connection_info)
                    wg_item.set_sensitive(False)
                    self.menu.append(wg_item)
                    disconnectwg_item = Gtk.ImageMenuItem(_(f"Disable {wg_dev}"))
                    disconnectwg_item.connect("activate", self.disconnectWG, wg_dev)
                    self.menu.append(disconnectwg_item)
                else:
                    notonlinewg = Gtk.MenuItem(_("%s Disconnected") % connection_info)
                    notonlinewg.set_sensitive(False)
                    self.menu.append(notonlinewg)
                    wiredwg_item = Gtk.MenuItem(_("Enable"))
                    wiredwg_item.connect("activate", self.connectWG, wg_dev)
                    self.menu.append(wiredwg_item)
            self.menu.append(Gtk.SeparatorMenuItem())

        e_title = Gtk.MenuItem()
        e_title.set_label(_("Ethernet Network"))
        e_title.set_sensitive(False)
        self.menu.append(e_title)
        self.menu.append(Gtk.SeparatorMenuItem())
        cardnum = 1
        wifinum = 1
        cards = self.cardinfo['cards']
        for netcard in cards:
            connection_state = cards[netcard]['state']["connection"]
            if "wlan" not in netcard:
                if connection_state == "Connected":
                    wired_item = Gtk.MenuItem(_("Wired %s Connected") % cardnum)
                    wired_item.set_sensitive(False)
                    self.menu.append(wired_item)
                    disconnect_item = Gtk.ImageMenuItem(_(f"Disable {netcard}"))
                    disconnect_item.connect("activate", self.disconnectcard,
                                            netcard)
                    self.menu.append(disconnect_item)
                    configure_item = Gtk.ImageMenuItem(f"Configure {netcard}")
                    configure_item.connect("activate", self.configuration_window_open, netcard)
                    self.menu.append(configure_item)
                elif connection_state == "Disconnected":
                    notonline = Gtk.MenuItem(_("Wired %s Disconnected") % cardnum)
                    notonline.set_sensitive(False)
                    self.menu.append(notonline)
                    wired_item = Gtk.MenuItem(_("Enable"))
                    wired_item.connect("activate", self.connectcard, netcard)
                    self.menu.append(wired_item)
                else:
                    disconnected = Gtk.MenuItem(_("Wired %s Unplug") % cardnum)
                    disconnected.set_sensitive(False)
                    self.menu.append(disconnected)
                cardnum += 1
                self.menu.append(Gtk.SeparatorMenuItem())
            elif "wlan" in netcard:
                if connection_state == "Disabled":
                    wd_title = Gtk.MenuItem()
                    wd_title.set_label(_("WiFi %s Disabled") % wifinum)
                    wd_title.set_sensitive(False)
                    self.menu.append(wd_title)
                    enawifi = Gtk.MenuItem(_("Enable Wifi %s") % wifinum)
                    enawifi.connect("activate", self.enable_Wifi, netcard)
                    self.menu.append(enawifi)
                elif connection_state == "Disconnected":
                    d_title = Gtk.MenuItem()
                    d_title.set_label(_("WiFi %s Disconnected") % wifinum)
                    d_title.set_sensitive(False)
                    self.menu.append(d_title)
                    self.wifiListMenu(netcard, None, False, cards)
                    diswifi = Gtk.MenuItem(_("Disable Wifi %s") % wifinum)
                    diswifi.connect("activate", self.disable_Wifi, netcard)
                    self.menu.append(diswifi)
                else:
                    ssid = cards[netcard]['state']["ssid"]
                    bar = cards[netcard]['info'][ssid][4]
                    wc_title = Gtk.MenuItem(_("WiFi %s Connected") % wifinum)
                    wc_title.set_sensitive(False)
                    self.menu.append(wc_title)
                    connection_item = Gtk.ImageMenuItem(ssid)
                    connection_item.set_image(self.wifi_signal_icon(bar))
                    connection_item.show()
                    disconnect_item = Gtk.MenuItem(_("Disconnect from %s") % ssid)
                    disconnect_item.connect("activate", self.disconnect_wifi,
                                            netcard)
                    self.menu.append(connection_item)
                    self.menu.append(disconnect_item)
                    self.wifiListMenu(netcard, ssid, True, cards)
                    diswifi = Gtk.MenuItem(_("Disable Wifi %s") % wifinum)
                    diswifi.connect("activate", self.disable_Wifi, netcard)
                    self.menu.append(diswifi)
                    configure_item = Gtk.ImageMenuItem(f"Configure {netcard}")
                    configure_item.connect("activate", self.configuration_window_open, netcard)
                    self.menu.append(configure_item)
                self.menu.append(Gtk.SeparatorMenuItem())
                wifinum += 1

        open_item = Gtk.MenuItem(_("Restart Networking"))
        open_item.connect("activate", restart_all_nics)
        self.menu.append(open_item)
        close_manager = Gtk.MenuItem(_("Close Network Manager"))
        close_manager.connect("activate", self.stop_manager)
        self.menu.append(close_manager)
        self.menu.show_all()
        return self.menu

</code_context>

<issue_to_address>
**issue (code-quality):** We've found these issues:

- Extract duplicate code into method ([`extract-duplicate-method`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/extract-duplicate-method/))
- Simplify conditional into switch-like form [×2] ([`switch`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/switch/))
- Remove redundant conditional ([`remove-redundant-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-if/))
- Low code quality found in trayIcon.nm\_menu - 20% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))

<br/><details><summary>Explanation</summary>



The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.

How can you solve this?

It might be worth refactoring this function to make it shorter and more readable.

- Reduce the function length by extracting pieces of functionality out into
  their own functions. This is the most important thing you can do - ideally a
  function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
  sits together within the function rather than being scattered.</details>
</issue_to_address>

### Comment 9
<location> `NetworkMgr/wg_api.py:14-21` </location>
<code_context>
def wg_service_state():
    """Function returns the WireGuard service status."""
    result = run(['service', 'wireguard', 'rcvar'], stdout=PIPE, stderr=PIPE, check=False)
    out = result.stdout.decode('utf-8')

    state = 'Unknown'

    for line in out.splitlines():
        if "wireguard_enable=" in line:
            state = line.split('=')[1].strip()
            break

    return state

</code_context>

<issue_to_address>
**suggestion (code-quality):** We've found these issues:

- Use the built-in function `next` instead of a for-loop ([`use-next`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-next/))
- Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))

```suggestion
    return next(
        (
            line.split('=')[1].strip()
            for line in out.splitlines()
            if "wireguard_enable=" in line
        ),
        'Unknown',
    )
```
</issue_to_address>

### Comment 10
<location> `NetworkMgr/wg_api.py:64-69` </location>
<code_context>
def wg_status(wgconfig):
    """Function returning the WireGuard configuration (device) is connected or not."""
    result = run(['wg', 'show', wgconfig], stdout=PIPE, stderr=PIPE, check=False)
    out = result.stdout.decode('utf-8')
    error = result.stderr.decode('utf-8')

    if len(out) == 0 and 'Unable to access interface: Device not configured' in error:
        status = "Disconnected"
    else:
        status = "Connected"

    return status

</code_context>

<issue_to_address>
**suggestion (code-quality):** We've found these issues:

- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
- Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))

```suggestion
    return (
        "Disconnected"
        if len(out) == 0
        and 'Unable to access interface: Device not configured' in error
        else "Connected"
    )
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

…ing.

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@ericbsd ericbsd self-requested a review September 22, 2025 19:45
Copy link
Member

@ericbsd ericbsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I might make minor changes later.

@ericbsd ericbsd merged commit 33c50c0 into ghostbsd:master Sep 22, 2025
1 check failed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Development Tracker Sep 22, 2025
@ericbsd
Copy link
Member

ericbsd commented Sep 22, 2025

@MatthiasLanter Thanks for the PR.

@MatthiasLanter
Copy link
Author

Very happy to have contributed to this great project. Hopefully this extension will be helpful to many users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants