fix: imported configs must stay readable by the gui #50

Merged

mysticalsoap merged 1 commit from mysticalsoap/fix/import-file-modes into trunk

2026-08-19 01:04:02 -04:00

AGit

mysticalsoap commented

2026-08-19 00:45:46 -04:00

Owner

Problem

Pressing Modify on a custom-provider server crashed the whole GUI (core dump). Two bugs compounding, both surfaced while manually testing #49's modify-server flow:

copy_certs sets os.umask(0o077) for the credentials file but restores it only at the very end of the function — after every config copy. shutil.copyfile creates fresh destination files, so every custom-provider config imported since the beginning has landed in ROOTDIR as 0600, unreadable by the GUI.
modify_server had no guard for an unreadable config, and PyQt5 aborts the process when an exception escapes a slot — so the PermissionError from the dialog's config display became a core dump.

Fix

The umask is restored immediately after the auth file is written (the only thing needing it), with a comment on why the placement matters. It was also process-wide for the whole copy — the service's main thread could race it.
Modes are now explicit and content-aware instead of umask-dependent: configs (.ovpn/.conf) get 0644 unless they inline a personal secret (<key>, <pkcs12>, <secret>, <auth-user-pass>, <tls-crypt-v2>), which keeps them 0600 along with keys/certs — openvpn runs as root and reads them regardless. <tls-auth>/<tls-crypt> deliberately don't count: providers ship the same HMAC key in every bulk config, and treating it as a secret would lock the GUI's own modify dialog out of most imports. Source-file modes no longer decide what lands in ROOTDIR either. A config kept at 0600 can't be opened in the modify dialog — that's the graceful notification path below, not a crash.
The supported-provider openvpn.conf chmod loses its stray exec bits (0655 → 0644).
modify_server catches OSError and notifies instead of crashing.

Verification

New TestImportedFileModes in tests/test_update.py: a staged custom import asserts a tls-crypt-only config lands 0644, a config with an inlined <key> lands 0600, keys and the auth file land 0600 (with source modes deliberately set to mislead), and that the umask is restored after copy_certs. Logic hand-verified locally (pytest runs in CI).
The crash itself was reproduced live (journal traceback: PermissionError on a 0600 imported config, followed by the core dump).
Existing broken installs need the on-disk file fixed once by hand — the code change only affects future imports.

Assisted-by: claude-fable-5

## Problem Pressing Modify on a custom-provider server crashed the whole GUI (core dump). Two bugs compounding, both surfaced while manually testing #49's modify-server flow: 1. `copy_certs` sets `os.umask(0o077)` for the credentials file but restores it only at the very end of the function — after every config copy. `shutil.copyfile` creates fresh destination files, so every custom-provider config imported since the beginning has landed in `ROOTDIR` as `0600`, unreadable by the GUI. 2. `modify_server` had no guard for an unreadable config, and PyQt5 aborts the process when an exception escapes a slot — so the `PermissionError` from the dialog's config display became a core dump. ## Fix - The umask is restored immediately after the auth file is written (the only thing needing it), with a comment on why the placement matters. It was also process-wide for the whole copy — the service's main thread could race it. - Modes are now explicit and content-aware instead of umask-dependent: configs (`.ovpn`/`.conf`) get `0644` **unless they inline a personal secret** (`<key>`, `<pkcs12>`, `<secret>`, `<auth-user-pass>`, `<tls-crypt-v2>`), which keeps them `0600` along with keys/certs — openvpn runs as root and reads them regardless. `<tls-auth>`/`<tls-crypt>` deliberately don't count: providers ship the same HMAC key in every bulk config, and treating it as a secret would lock the GUI's own modify dialog out of most imports. Source-file modes no longer decide what lands in `ROOTDIR` either. A config kept at `0600` can't be opened in the modify dialog — that's the graceful notification path below, not a crash. - The supported-provider `openvpn.conf` chmod loses its stray exec bits (`0655` → `0644`). - `modify_server` catches `OSError` and notifies instead of crashing. ## Verification - New `TestImportedFileModes` in `tests/test_update.py`: a staged custom import asserts a tls-crypt-only config lands `0644`, a config with an inlined `<key>` lands `0600`, keys and the auth file land `0600` (with source modes deliberately set to mislead), and that the umask is restored after `copy_certs`. Logic hand-verified locally (pytest runs in CI). - The crash itself was reproduced live (journal traceback: `PermissionError` on a `0600` imported config, followed by the core dump). - Existing broken installs need the on-disk file fixed once by hand — the code change only affects future imports. Assisted-by: claude-fable-5

mysticalsoap added 1 commit

2026-08-19 00:45:46 -04:00

fix: imported configs must stay readable by the gui

ci / test (pull_request) Successful in 29s

Details

7bc0a5c874

copy_certs tightened the umask for the credentials file but only
restored it after every copy, so custom-provider configs landed 0600
and the modify-server dialog crashed the gui trying to read one back.
Modes are now explicit -- configs 0644, keys and certs 0600 -- and an
unreadable config gets an error notification instead of a core dump.

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

mysticalsoap force-pushed mysticalsoap/fix/import-file-modes from 7bc0a5c874

ci / test (pull_request) Successful in 29s

Details

to eb536fce2c

ci / test (pull_request) Successful in 27s

Details

2026-08-19 00:57:45 -04:00

Compare

mysticalsoap merged commit 1bdb7463d5 into trunk

2026-08-19 01:04:02 -04:00