Folder import aborts on unrelated files instead of just not copying them

mysticalsoap commented

2026-08-19 20:02:25 -04:00

Owner

Importing a config folder aborts with "seems to contain many unrelated files" when the selected folder has >=10 files outside a small extension whitelist. Hit live 2026-08-19 trying to import a single .ovpn from ~/downloads.

The check (update.py:sanity_check, used by add_folder) is a heuristic band-aid over the real problem: after it passes, add_folder runs shutil.copytree(self.folderpath, ...) -- the root service copies the entire selected tree into its temp dir, so the guard tries to guess whether the user meant it. Its defects compound:

Recursive count, top-level import. conf_files comes from os.listdir (top level only), but sanity_check walks recursively -- files in subdirectories count toward the abort threshold yet are never imported.
Arbitrary threshold. 9 unrelated files ride along into a root-owned temp dir; 10 abort the import.
The whitelist omits .crt (['.ovpn', '.conf', '.key', '.cert', '.pem']) -- the extension this codebase itself uses everywhere (stunnel.crt, AzireVPN .crt). A legitimate folder of 10+ configs with paired .crt certs self-aborts.
cert_files is dead code -- assigned from a comprehension copy-pasted from conf_files (filters .ovpn/.conf, so it could never find certs), never referenced afterward.

Fix direction: invert the design. Copy exactly the files the import consumes -- the top-level configs already enumerated, plus cert/key files they reference -- and drop copytree and sanity_check entirely. Any source folder then works (downloads included), and the root service never touches files it wasn't asked to import.

Importing a config folder aborts with "seems to contain many unrelated files" when the selected folder has >=10 files outside a small extension whitelist. Hit live 2026-08-19 trying to import a single .ovpn from ~/downloads. The check (`update.py:sanity_check`, used by `add_folder`) is a heuristic band-aid over the real problem: after it passes, `add_folder` runs `shutil.copytree(self.folderpath, ...)` -- the root service copies the **entire selected tree** into its temp dir, so the guard tries to guess whether the user meant it. Its defects compound: - **Recursive count, top-level import.** `conf_files` comes from `os.listdir` (top level only), but `sanity_check` walks recursively -- files in subdirectories count toward the abort threshold yet are never imported. - **Arbitrary threshold.** 9 unrelated files ride along into a root-owned temp dir; 10 abort the import. - **The whitelist omits `.crt`** (`['.ovpn', '.conf', '.key', '.cert', '.pem']`) -- the extension this codebase itself uses everywhere (`stunnel.crt`, AzireVPN `.crt`). A legitimate folder of 10+ configs with paired .crt certs self-aborts. - **`cert_files` is dead code** -- assigned from a comprehension copy-pasted from `conf_files` (filters .ovpn/.conf, so it could never find certs), never referenced afterward. Fix direction: invert the design. Copy exactly the files the import consumes -- the top-level configs already enumerated, plus cert/key files they reference -- and drop `copytree` and `sanity_check` entirely. Any source folder then works (downloads included), and the root service never touches files it wasn't asked to import.

mysticalsoap commented

2026-08-19 20:11:43 -04:00

Author

Owner

It gets worse: the GUI side (aqomui_gui.py import flow) is a file picker (getOpenFileName, filtered to *.ovpn), captioned "Choose Folder", launched from a dropdown item called "Manually add config file folder" -- and then calls .absolutePath() on the chosen file, discarding the filename and importing the file's parent directory. Picking a single .ovpn in ~/downloads is doing it right, and it still detonates the sanity check because the importer substitutes the whole folder.

So the redesign has an obvious UI shape: getOpenFileNames (multi-select), import exactly the chosen files plus the cert/key files they reference, no directory semantics anywhere.

It gets worse: the GUI side (`aqomui_gui.py` import flow) is a **file** picker (`getOpenFileName`, filtered to *.ovpn), captioned "Choose Folder", launched from a dropdown item called "Manually add config file folder" -- and then calls `.absolutePath()` on the chosen file, discarding the filename and importing the file's **parent directory**. Picking a single .ovpn in ~/downloads is doing it right, and it still detonates the sanity check because the importer substitutes the whole folder. So the redesign has an obvious UI shape: `getOpenFileNames` (multi-select), import exactly the chosen files plus the cert/key files they reference, no directory semantics anywhere.

mysticalsoap referenced this issue

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

Remove the per-provider custom scripts feature #82

mysticalsoap referenced this issue

2026-08-20 18:55:14 -04:00

Give the six provider implementations shared structure #23

mysticalsoap referenced this issue from a commit

2026-08-20 19:02:52 -04:00

change: import the config files the user picks, not their whole directory

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

2026-08-20 19:03:05 -04:00

change: import the config files the user picks, not their whole directory #101