Remove the per-provider custom scripts feature #82

New issue

Closed

opened 2026-08-20 00:48:12 -04:00 by mysticalsoap · 1 comment

mysticalsoap commented

2026-08-20 00:48:12 -04:00

Owner

Remove the per-provider custom-scripts feature: the Scripts tab, save_scripts/clear_scripts/update_scripts_enabled in the GUI, exe_custom_scripts and its six call sites in tunnel.py, and the {provider}_scripts config keys.

Why:

It is arbitrary root command execution configured from the GUI: script strings live in config.json (written via cfgupdate) and run inside the root service's tunnel threads. polkit gates the writer, but it's a wide, quiet root-exec channel kept alive for a feature with no known users — same reasoning that deleted the self-updater (#19).
The hooks get no context (no device, no role, no env), so they can't do anything a tunnel hook is actually for.
They're keyed per provider but fire per tunnel process, which is how #74's sibling bug #81 exists (pre/down firing for hop connects). This removal supersedes #81 — the buggy guards exist only to gate this feature; the removal PR should close both.

If demand ever shows up, the re-add should be the inverse shape: hooks run in the GUI process as the user, triggered by the status/state signals the GUI already receives (it now holds the typed tunnel state for context) — no root involvement. Same check-only-re-add pattern as #48.

Remove the per-provider custom-scripts feature: the Scripts tab, `save_scripts`/`clear_scripts`/`update_scripts_enabled` in the GUI, `exe_custom_scripts` and its six call sites in `tunnel.py`, and the `{provider}_scripts` config keys. Why: - It is arbitrary root command execution configured from the GUI: script strings live in config.json (written via `cfgupdate`) and run inside the root service's tunnel threads. polkit gates the writer, but it's a wide, quiet root-exec channel kept alive for a feature with no known users — same reasoning that deleted the self-updater (#19). - The hooks get no context (no device, no role, no env), so they can't do anything a tunnel hook is actually for. - They're keyed per provider but fire per tunnel process, which is how #74's sibling bug #81 exists (pre/down firing for hop connects). This removal supersedes #81 — the buggy guards exist only to gate this feature; the removal PR should close both. If demand ever shows up, the re-add should be the inverse shape: hooks run in the GUI process as the user, triggered by the status/state signals the GUI already receives (it now holds the typed tunnel state for context) — no root involvement. Same check-only-re-add pattern as #48.

mysticalsoap added the

cleanup

security

labels

2026-08-20 00:48:12 -04:00

mysticalsoap referenced this issue

2026-08-20 10:48:59 -04:00

Module decomposition (index) #83

mysticalsoap referenced this issue

2026-08-23 01:44:31 -04:00

Extract the Provider tab into gui/provider.py #129

mysticalsoap referenced this issue

2026-08-28 18:22:12 -04:00

Imported OpenVPN configs can carry script directives into root execution #197

mysticalsoap commented

2026-08-28 18:22:25 -04:00

Author

Owner

Upstream history and ecosystem research before starting the removal — it strengthens the no-known-users rationale into an evidence-backed one:

The feature was author-driven, not requested. corrad1nho added it in e8ba225 (2018-11-24) in a same-day sprint alongside connection profiles; it shipped as one changelog line in 0.8.0 and was never documented in the README. No issue or PR in upstream's lifetime asked for it — every 'script' hit in that tracker is install/packaging or PIA-cert related, and the feature-request roundup from a month before (upstream #40) asked for a map, kill switch and server rotation, no hooks.
It shipped broken and nobody noticed. The initial implementation ran the wrong variable; the author fixed it himself six weeks later (upstream PR #61). That fix is the only maintenance the feature ever received, and no user ever filed a bug against it — for an arbitrary-root-exec feature, strong evidence of zero users.
Ecosystem check: serious implementations are either privilege-symmetric and file-based — OpenVPN --up/--down behind --script-security, wg-quick PostUp in a root-owned config, NetworkManager dispatcher.d (refuses scripts not owned by root or writable by group/world), Tunnelblick .tblk scripts behind admin-secured configs — or run hooks unprivileged as the user (Viscosity, Eddie), which is exactly the re-add shape this issue proposes. Direct precedent for the removal: NetworkManager deliberately ignores PostUp/PreDown when importing wg-quick configs (https://blogs.gnome.org/thaller/2019/03/15/wireguard-in-networkmanager/).
Adjacent channel found during the research and split out: imported OpenVPN configs can carry script directives into root execution — #197.

Upstream history and ecosystem research before starting the removal — it strengthens the no-known-users rationale into an evidence-backed one: - **The feature was author-driven, not requested.** corrad1nho added it in e8ba225 (2018-11-24) in a same-day sprint alongside connection profiles; it shipped as one changelog line in 0.8.0 and was never documented in the README. No issue or PR in upstream's lifetime asked for it — every 'script' hit in that tracker is install/packaging or PIA-cert related, and the feature-request roundup from a month before (upstream #40) asked for a map, kill switch and server rotation, no hooks. - **It shipped broken and nobody noticed.** The initial implementation ran the wrong variable; the author fixed it himself six weeks later (upstream PR #61). That fix is the only maintenance the feature ever received, and no user ever filed a bug against it — for an arbitrary-root-exec feature, strong evidence of zero users. - **Ecosystem check:** serious implementations are either privilege-symmetric and file-based — OpenVPN --up/--down behind --script-security, wg-quick PostUp in a root-owned config, NetworkManager dispatcher.d (refuses scripts not owned by root or writable by group/world), Tunnelblick .tblk scripts behind admin-secured configs — or run hooks unprivileged as the user (Viscosity, Eddie), which is exactly the re-add shape this issue proposes. Direct precedent for the removal: NetworkManager deliberately ignores PostUp/PreDown when importing wg-quick configs (https://blogs.gnome.org/thaller/2019/03/15/wireguard-in-networkmanager/). - Adjacent channel found during the research and split out: imported OpenVPN configs can carry script directives into root execution — #197.

mysticalsoap referenced this issue from a commit

2026-08-28 18:31:45 -04:00

change: drop the per-provider custom scripts feature (#82)

mysticalsoap referenced this issue from a pull request that will close it,

2026-08-28 18:31:57 -04:00

change: drop the per-provider custom scripts feature (#82) #198

mysticalsoap closed this issue

2026-08-28 18:34:51 -04:00

mysticalsoap referenced this issue from a commit

2026-08-28 18:36:43 -04:00

change: drop the legacy scripts-key scrub from load_config

mysticalsoap referenced this issue from a commit