fix: comment out exec options when importing OpenVPN configs (#197) #200

Merged

mysticalsoap merged 1 commit from sanitize-imports into trunk

2026-08-28 18:49:24 -04:00

mysticalsoap commented

2026-08-28 18:46:42 -04:00

Owner

Problem

The root service runs imported OpenVPN configs verbatim, and a config can carry its own script-security 2 plus up/down/route-up/plugin/tls-verify/… — an import channel into root command execution, the same class #82 closed for the GUI (#197). WireGuard is already covered: wireguard.py rejects wg-quick extension keys, so PostUp never executes.

Fix

Sanitize-on-import, per the issue (matches the NetworkManager precedent of ignoring PostUp/PreDown on wg import). The import already commented out up /down lines — this completes the set as an EXEC_OPTIONS tuple (every option that names a command, plus script-security itself) and matches the line's first token instead of startswith, which also catches indented directives. Each disarmed line is logged and commented out rather than dropped, so the imported copy still shows what it arrived with.

Ordering matters: the exec check runs before the auth-user-pass rewrite, whose startswith match would otherwise claim auth-user-pass-verify — rewriting an exec option into the auth-file line and demanding credentials the config doesn't need.

Deliberately import-time only: configs imported before this change are not re-scanned at connect time, consistent with keeping legacy-data handling out while the fork has no users (same call as #199).

Verification

Full suite passes (491): new tests cover the directive set (including an indented line and a quoted argument), that remote survives untouched, that the import still succeeds, and the auth-user-pass-verify ordering trap (a cert-only config carrying it imports without credentials).
ruff check --select E9,F63,F7,F82 . clean.

🤖 Generated with Claude Code

## Problem The root service runs imported OpenVPN configs verbatim, and a config can carry its own `script-security 2` plus `up`/`down`/`route-up`/`plugin`/`tls-verify`/… — an import channel into root command execution, the same class #82 closed for the GUI (#197). WireGuard is already covered: wireguard.py rejects wg-quick extension keys, so `PostUp` never executes. ## Fix Sanitize-on-import, per the issue (matches the NetworkManager precedent of ignoring `PostUp`/`PreDown` on wg import). The import already commented out `up `/`down ` lines — this completes the set as an `EXEC_OPTIONS` tuple (every option that names a command, plus `script-security` itself) and matches the line's first token instead of `startswith`, which also catches indented directives. Each disarmed line is logged and commented out rather than dropped, so the imported copy still shows what it arrived with. Ordering matters: the exec check runs before the `auth-user-pass` rewrite, whose `startswith` match would otherwise claim `auth-user-pass-verify` — rewriting an exec option into the auth-file line and demanding credentials the config doesn't need. Deliberately import-time only: configs imported before this change are not re-scanned at connect time, consistent with keeping legacy-data handling out while the fork has no users (same call as #199). ## Verification - Full suite passes (491): new tests cover the directive set (including an indented line and a quoted argument), that `remote` survives untouched, that the import still succeeds, and the `auth-user-pass-verify` ordering trap (a cert-only config carrying it imports without credentials). - `ruff check --select E9,F63,F7,F82 .` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

mysticalsoap added 1 commit

2026-08-28 18:46:42 -04:00

fix: comment out exec options when importing OpenVPN configs (#197 )

ci / test (pull_request) Successful in 24s

Details

ci / test (push) Successful in 24s

Details

e9a5dd37ad

The root service runs imported configs verbatim, and a config can raise
its own script-security and name commands in up/down/route-up/plugin
and friends -- an import channel into root execution, the same class
the custom-scripts removal (#82) closed for the GUI. NetworkManager
ignores wg-quick's PostUp/PreDown on import for the same reason; the
WireGuard side here is already covered because wireguard.py rejects
extension keys.

The import already commented out up/down; this completes the list and
matches on the line's first token instead of startswith, which also
catches indented lines. The exec check must run before the
auth-user-pass rewrite: startswith("auth-user-pass") also claims
auth-user-pass-verify, turning an exec option into the auth-file line
and demanding credentials the config does not need.

Commented out rather than dropped so the imported copy still shows
what it arrived with, and logged so the import reports what it
disarmed.

Closes #197

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mysticalsoap merged commit e9a5dd37ad into trunk

2026-08-28 18:49:24 -04:00

mysticalsoap deleted branch sanitize-imports

2026-08-28 18:49:24 -04:00