Review of "Tailscale QuickSettings Toggle" version v6 (9)

Details Page Preview

Add Tailscale to GNOME quick settings. Fork of joaophi/tailscale-gnome-qs with continued maintenance and improvements. Make sure you set your user as tailscale operator: sudo tailscale set --operator=$USER

Extension Homepage
https://github.com/tailscale-qs/tailscale-gnome-qs

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

Shexli (experimental) warning 2 manual_review 1

Shexli found 3 issues that may need reviewer attention.

EGO-A-005 manual_review

extensions should not access the clipboard directly

Direct clipboard access via `St.Clipboard.get_default()` requires reviewer scrutiny.

Review Guidelines

  • extension.js:312
    St.Clipboard.get_default()
  • extension.js:313
    St.Clipboard.get_default()

EGO-L-003 warning

signals connected by extension should be disconnected in disable()

Signals assigned in `enable()` are missing matching disconnect calls in `disable()` or its helper methods.

Disconnect all signals

  • extension.js:76
    tailscale.connect('notify::exit-node', () => setVisible())
  • extension.js:77
    tailscale.connect('notify::running', () => setVisible())
  • extension.js:360
    tailscale.connect('notify::accept-dns', obj => dns.setToggleState(obj.accept_dns))
  • extension.js:353
    tailscale.connect('notify::accept-routes', obj => routes.setToggleState(obj.accept_routes))
  • extension.js:367
    tailscale.connect('notify::allow-lan-access', obj => lan.setToggleState(obj.allow_lan_access))
  • extension.js:279
              tailscale.connect('notify::exit-node-name', () => {
                  this.subtitle = tailscale.exit_node_name;
                  this.menu.setHeader(icon, this.title, this.subtitle);
                  disableExitNodeButton.reactive = tailscale.exit_node !== '';
              })
  • extension.js:341
    tailscale.connect('notify::nodes', obj => updateNodes(obj))
  • extension.js:403
    tailscale.connect('notify::profiles', obj => updateProfiles(obj))
  • extension.js:374
    tailscale.connect('notify::shields-up', obj => shields.setToggleState(obj.shields_up))
  • extension.js:381
    tailscale.connect('notify::ssh', obj => ssh.setToggleState(obj.ssh))

EGO-L-008 warning

Soup.Session instances should be aborted during cleanup

Soup.Session instances should be aborted during cleanup.

Soup.Session.abort

  • tailscale.js:11
            this.session = new Soup.Session({
                'remote-connectable': address,
                'timeout': 0,
                'idle-timeout': 0,
            })

All Versions

Version Status
v6 (9) Active
v6 (8) Rejected
v6 (7) Rejected
v6 (6) Rejected
5 Active
4 Inactive
3 Inactive
2 Rejected
1 Rejected

Previous Reviews on this Version

alexjfinch posted a review
Hi, Just wanted to flag this review in case it slipped through — #69660. It's a fairly large diff but the bulk of it is a ELINT-driven code refactor to bring the extension more in line with GNOME coding standards, rather than any functional change. The three Shexli flags are noted — the clipboard access and signal disconnect issues predate the refactor and were present in the previously approved version, so we don't believe they represent regressions. We're planning to address the signal lifecycle properly in the next iteration once the refactor lands. Happy to answer any questions or provide context on specific changes if that'd help move things along. Thanks
fmuellner active