[FIX] vpn : juger un profil monté sur son interface, et le marquer

Le verdict se fondait sur le seul fichier d'état, que RIEN n'efface quand
un tunnel meurt sans passer par « down » — machine redémarrée, processus
tué, session expirée. Un état laissé derrière était annoncé monté sur
l'écran même qui déclarait le processus mort et l'interface absente.

L'interface arbitre désormais : son nom vient de l'état s'il y en a un,
sinon du pilote pour ceux qui la nomment d'avance, si bien qu'un tunnel
monté hors de l'outil est vu aussi. Sshuttle n'en crée aucune, là le
processus reste seul juge. La liste des profils le marque, connecter un
profil déjà monté demande confirmation, et déconnecter un profil déjà
tombé le dit sans l'empêcher : « down » nettoie l'état laissé.

--- EN ---

The verdict rested on the state file alone, which NOTHING clears when a
tunnel dies without going through "down" — machine rebooted, process
killed, session expired. A leftover state was reported as mounted on the
very screen declaring the process gone and the interface absent.

The interface now arbitrates: its name comes from the state when there is
one, otherwise from drivers that name it in advance, so a tunnel brought
up outside the tool is seen too. Sshuttle creates none, and there the
process stays the only judge. The profile list marks it, connecting an
already-mounted profile asks first, and disconnecting an already-down one
says so without refusing: "down" clears the leftover state.

Assisted-by: Claude Opus 5
This commit is contained in:
Mathieu Benoit 2026-09-07 23:27:35 -04:00
parent aa14942709
commit ea07c71e04
4 changed files with 231 additions and 10 deletions

View file

@ -11791,6 +11791,25 @@ TRANSLATIONS = {
"fr": "L'installer aussi ? (o/N)",
"en": "Install it as well? (y/N)",
},
"This profile is already connected.": {
"fr": "Ce profil est déjà connecté.",
"en": "This profile is already connected.",
},
"Connect it again? (y/N)": {
"fr": "Le reconnecter quand même ? (o/N)",
"en": "Connect it again? (y/N)",
},
"This profile is not connected. Disconnecting still clears the state"
" a dead tunnel left behind.": {
"fr": (
"Ce profil n'est pas connecté. Le déconnecter efface tout de"
" même l'état qu'un tunnel mort a laissé."
),
"en": (
"This profile is not connected. Disconnecting still clears the"
" state a dead tunnel left behind."
),
},
"SSO helper": {
"fr": "greffon SSO",
"en": "SSO helper",

View file

@ -109,6 +109,14 @@ SSO_HELPER_NOTE = (
" optional, and its upstream is no longer maintained."
)
# Ce qu'on dit avant de descendre un profil qui n'est pas monté. Le geste
# garde un sens — il nettoie ce qu'un tunnel mort a laissé dans /run — et
# le taire ferait croire à une erreur de choix.
NOT_CONNECTED_NOTE = (
"This profile is not connected. Disconnecting still clears the state"
" a dead tunnel left behind."
)
MASTER_PASSWORD_WARNING = (
"The vault MASTER password is stored in the configuration in clear"
" text. Remove it and type it on demand."
@ -271,6 +279,13 @@ class VpnMenuMixin:
name = self._vpn_select_profile()
if not name:
return
# Remonter un tunnel qui tient rejoue toute l'authentification —
# jusqu'à un formulaire web et un second facteur — pour aboutir à
# une interface qui existait déjà.
if self._vpn_is_up(profiles.load(name)):
print(f"\n! {t('This profile is already connected.')}")
if not self._is_yes(input(f"{t('Connect it again? (y/N)')} : ")):
return
# Lu UNE fois pour les deux exécutions qui suivent.
secrets_env = self._vpn_secrets_env(name)
# Le plan d'abord, l'exécution ensuite : monter un tunnel réécrit
@ -283,8 +298,14 @@ class VpnMenuMixin:
def _vpn_disconnect(self):
name = self._vpn_select_profile()
if name:
self._vpn_cli(f"down --profile {name}")
if not name:
return
# « down » reste utile sur un profil déjà tombé : c'est lui qui
# efface l'état laissé dans /run par un tunnel mort sans lui. On le
# dit, on ne l'empêche pas.
if not self._vpn_is_up(profiles.load(name)):
print(f"\n! {t(NOT_CONNECTED_NOTE)}")
self._vpn_cli(f"down --profile {name}")
def _vpn_diagnose(self):
name = self._vpn_select_profile()
@ -320,8 +341,22 @@ class VpnMenuMixin:
# ------------------------------------------------------------------
# Profils
# ------------------------------------------------------------------
@staticmethod
def _vpn_is_up(profile):
"""Ce profil porte-t-il un tunnel vivant ? Faux si on ne peut pas
savoir — un pilote retiré de la configuration ne doit pas empêcher
de lister les profils."""
driver_cls = get_driver(profile.get("driver"))
return bool(driver_cls) and driver_cls(profile).is_up()
def _vpn_select_profile(self):
"""Nom du profil choisi, "" si l'utilisateur renonce."""
"""Nom du profil choisi, "" si l'utilisateur renonce.
L'état de chaque profil est affiché, parce que la liste sert autant
à connecter qu'à déconnecter : sans lui, on descend un tunnel déjà
mort ou on remonte celui qui tient, et la sortie du CLI est la
première chose qui le dit — trop tard.
"""
all_profiles = [profiles.with_defaults(p) for p in profiles.load_all()]
if not all_profiles:
print(t("No VPN profile yet: create one first."))
@ -332,8 +367,12 @@ class VpnMenuMixin:
if profile["default_route"]
else ", ".join(profile["routes"])
)
# Deux colonnes de large dans les deux cas : un emoji en occupe
# deux, et sans cela les lignes non connectées décaleraient tout
# ce qui suit.
marque = "🟢" if self._vpn_is_up(profile) else " "
print(
f"[{index}] {profile['name']:<20}"
f"[{index}] {marque} {profile['name']:<20}"
f" {profile['server']:<26} {target}"
)
answer = input(f"{t('Profile number (0 to go back)')} : ").strip()

View file

@ -783,13 +783,47 @@ class VpnDriver:
"présents" if not missing else f"absents : {', '.join(missing)}",
)
def is_up(self) -> bool:
"""Ce profil porte-t-il un tunnel VIVANT ?
Le fichier d'état ne suffit pas à répondre : il est écrit au
montage et RIEN ne l'efface quand le tunnel meurt sans passer par
« down » — machine redémarrée, processus tué, session expirée. Un
état laissé derrière déclarerait monté un profil dont l'interface a
disparu, et deux lignes plus bas le même écran dirait que le
processus est mort.
L'interface est donc l'arbitre. Son nom vient de l'état quand il y
en a un, sinon du pilote pour ceux qui la NOMMENT d'avance — ainsi
un tunnel monté hors de l'outil est vu lui aussi. Reste sshuttle,
qui détourne par le pare-feu sans créer d'interface : là, le
processus est le seul juge possible.
"""
iface = self.recorded_iface() or getattr(self, "iface", "")
if iface:
return interface_exists(iface)
if self.iface_kind:
return False
return self.pid_alive() is True
def check_mounted(self):
iface = self.recorded_iface()
return (
"profil monté (état /run)",
bool(iface),
f"interface {iface}" if iface else "aucun état : non connecté",
)
"""Verdict du montage, et la CAUSE quand il est faux.
« aucun état » et « état périmé » demandent deux gestes
différents : monter dans le premier cas, jouer « down » dans le
second pour effacer ce que le tunnel mort a laissé.
"""
iface = self.recorded_iface() or getattr(self, "iface", "")
if self.is_up():
return ("profil monté", True, f"interface {iface}")
if self.recorded_iface():
return (
"profil monté",
False,
f"état laissé pour {iface}, interface disparue :"
" jouer « down » pour le nettoyer",
)
return ("profil monté", False, "aucun état : non connecté")
def check_daemon(self, label="démon"):
alive = self.pid_alive()

View file

@ -19,6 +19,7 @@ import json
import os
import sys
import tempfile
import unicodedata
import unittest
from contextlib import redirect_stdout
from unittest.mock import patch
@ -37,6 +38,9 @@ from script.todo.vpn_menu import ( # noqa: E402
)
from script.vpn import profiles # noqa: E402
from script.vpn.drivers import DRIVERS # noqa: E402
from script.vpn.drivers.openconnect import ( # noqa: E402
OpenconnectDriver,
)
WG_PUBLIC = base64.b64encode(bytes(range(32, 64))).decode()
@ -655,6 +659,131 @@ class SsoHelperOffer(MenuBase):
self.assertIn("entretenu", printed)
def largeur_affichee(texte):
"""Largeur de `texte` en colonnes de terminal.
Les caractères que la norme Unicode classe « W » (wide) ou « F »
(fullwidth) — dont les emoji — en occupent deux pour un seul
caractère. Une colonne alignée à l'écran ne l'est donc pas dans
l'index de la chaîne, et l'inverse.
"""
return sum(
2 if unicodedata.east_asian_width(c) in "WF" else 1 for c in texte
)
class ShowingWhatIsConnected(MenuBase):
"""L'état de chaque profil, dans la liste qui sert à choisir.
La même liste sert à connecter et à déconnecter : sans l'état, on
descend un tunnel déjà mort ou on remonte celui qui tient, et la
sortie du CLI est la première chose qui le dit — trop tard.
"""
def setUp(self):
super().setUp()
profiles.save(
{
"name": "vivant",
"driver": "openconnect",
"server": "ssl.vpn.example-campus.net",
"oc_user": "someone",
}
)
profiles.save(
{
"name": "mort",
"driver": "openconnect",
"server": "ssl.vpn.example-campus.net",
"oc_user": "someone",
}
)
def listing(self, up):
"""La liste, avec `up` disant quels profils sont montés."""
with patch.object(
OpenconnectDriver,
"is_up",
lambda self: self.profile["name"] in up,
):
with self.answering("0"):
out = io.StringIO()
with redirect_stdout(out):
self.todo._vpn_select_profile()
return out.getvalue()
def test_the_connected_profile_is_marked(self):
printed = self.listing({"vivant"})
vivant = [l for l in printed.splitlines() if "vivant" in l][0]
mort = [l for l in printed.splitlines() if "mort" in l][0]
self.assertIn("🟢", vivant)
self.assertNotIn("🟢", mort)
def test_the_columns_stay_aligned(self):
"""Un emoji occupe deux COLONNES pour un seul caractère : la ligne
non marquée en réserve deux, sinon tout ce qui suit se décale.
La mesure porte donc sur les colonnes affichées et non sur
`str.index`, qui compte des caractères — l'écart d'un caractère
entre les deux lignes est précisément ce qui les aligne à l'écran.
"""
printed = self.listing({"vivant"})
lignes = [l for l in printed.splitlines() if "example-campus" in l]
self.assertEqual(len(lignes), 2, printed)
colonnes = {
largeur_affichee(l[: l.index("ssl.vpn.example-campus.net")])
for l in lignes
}
self.assertEqual(len(colonnes), 1, lignes)
def test_an_unknown_driver_does_not_break_the_listing(self):
"""Un pilote retiré de la configuration ne doit pas empêcher de
lister les profils, ni de supprimer celui qui le nomme."""
self.assertFalse(
self.todo._vpn_is_up({"name": "x", "driver": "disparu"})
)
def test_connecting_what_is_already_up_asks_first(self):
"""Remonter un tunnel qui tient rejoue toute l'authentification —
jusqu'à un formulaire web — pour aboutir à une interface qui
existait déjà."""
launched = []
with patch.object(OpenconnectDriver, "is_up", lambda self: True):
with patch.object(
self.todo, "_vpn_select_profile", return_value="vivant"
):
with patch.object(
self.todo,
"_vpn_cli",
lambda arguments, env=None: launched.append(arguments),
):
with self.answering("n"):
out = io.StringIO()
with redirect_stdout(out):
self.todo._vpn_connect()
self.assertIn("déjà connecté", out.getvalue())
self.assertEqual(launched, [], "rien ne devait être lancé")
def test_disconnecting_what_is_down_says_so_but_proceeds(self):
"""« down » reste utile : c'est lui qui efface l'état laissé dans
/run par un tunnel mort sans lui."""
launched = []
with patch.object(OpenconnectDriver, "is_up", lambda self: False):
with patch.object(
self.todo, "_vpn_select_profile", return_value="mort"
):
with patch.object(
self.todo,
"_vpn_cli",
lambda arguments, env=None: launched.append(arguments),
):
out = io.StringIO()
with redirect_stdout(out):
self.todo._vpn_disconnect()
self.assertIn("n'est pas connecté", out.getvalue())
self.assertEqual(launched, ["down --profile mort"])
class ChoosingTheXmlProfile(MenuBase):
"""Le choix du fichier `.xml` : parcours d'abord, saisie ensuite.