Initial work for first-boot setup wizard for an OEM style installation #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Restructured monolithic codebase into setup_station package with separate modules for each configuration screen (language, keyboard, timezone, network, user management). Implements utility class pattern with centralized data flow through SetupData. Added assets and CSS styling. Work in progress.
## Security Fixes (CRITICAL)
- Fix password exposure in set_admin_user() - use stdin instead of shell
- Fix command injection vulnerabilities - replace shell=True with list args
- Add comprehensive input validation:
- Username: format, length (max 32), alphanumeric validation
- Shell: validate against /etc/shells
- Home directory: path traversal protection
- Hostname: RFC 1123 format validation
- Timezone: path traversal + format validation
## Error Handling & Type Hints
- Add error handling throughout system_calls.py with check=True
- Add try-except blocks with proper exception chaining (from e)
- Add type hints to all functions in system_calls.py and common.py
- Add PEP 257 compliant docstrings with parameters, returns, exceptions
## Code Quality Improvements
- Remove 70 lines of duplicate/dead code:
- Remove duplicate constants from common.py (now in data.py only)
- Remove unused save_selection() and save_language_data() from language.py
- Centralize all hardcoded paths in data.py:
- Update network_setup.py, setup_system.py, add_users.py to import paths
- Add docstring to deprecated() decorator in common.py
- Remove unused /tmp/.setup-station code from interface_controller.py
## Files Modified
- system_calls.py: Complete security overhaul with validation
- common.py: Remove duplicates, add type hints
- language.py: Remove dead code, add error handling docs
- keyboard.py, timezone.py, add_admin.py: Add error handling docs
- network_setup.py, setup_system.py, add_users.py: Use centralized paths
- interface_controller.py: Remove unused temporary directory code
- Refactor setup.py to match install-station structure:
- Add module docstring and improve documentation
- Rename datafilelist() to data_file_list() for consistency
- Add custom translation commands (UpdateTranslationsCommand, CreateTranslationCommand)
- Define proper data_files with CSS, images, and desktop file
- Add packages=['setup_station'] specification
- Enable i18n support with DistUtilsExtra cmdclass
- Initialize translation infrastructure:
- Create po/ directory with 36 language files matching install-station
- Generate setup-station.pot template file
- Add automatic UTF-8 charset conversion after xgettext runs
- Support locales: ar_SA, bg_BG, ca_ES, cs_CZ, da_DK, de_DE, el_GR,
en_GB, en_US, es_ES, et_EE, fi_FI, fr_CA, fr_FR, hr_HR, hu_HU,
it_IT, ja_JP, ko_KR, lt_LT, lv_LV, nl_NL, no_NO, pl_PL, pt_BR,
pt_PT, ro_RO, ru_RU, sk_SK, sl_SI, sv_SE, th_TH, tr_TR, vi_VN,
zh_CN, zh_TW
- Translation workflow commands now available:
- python setup.py update_translations
- python setup.py create_translation --locale=xx_XX
- python setup.py build_i18n
- Fix module import in system_calls.py:
- Change 'from data import' to 'from setup_station.data import'
- Resolves ModuleNotFoundError when running setup-station-init
- Fix data paths in data.py:
- Update all paths from 'setup-station-init' to 'setup-station'
- Ensures CSS and image files are found at correct installation location
- Add missing Next button enable/disable logic in add_admin.py:
- Import Button from interface_controller
- Enable Next button when passwords match and are valid
- Disable Next button when passwords don't match or are invalid
- Fixes issue where Next button remained inactive on admin user screen
Add get_text() translations to 6 modules that were missing i18n support,
covering 52 new user-facing strings across the setup-station application.
Changes:
- setup_system.py: Progress messages and setup completion text
- network_setup.py: Network detection, WiFi auth dialog strings
- common.py: Password strength validation messages
- keyboard.py: Keyboard layout and model labels
- timezone.py: Continent and city column headers
- add_users.py: Legacy user setup interface strings
Translation implementation:
- Use string concatenation for dynamic content (SSID names) to prevent
translator errors with format placeholders
- Update add_admin.py password validation to work with translated strings
- Generate updated .pot template and merge into all 36 .po files
- Provide complete French translations for fr_FR and fr_CA
All new strings follow the existing translation infrastructure using
get_text() from setup_station.data module.
Contributor
Reviewer's GuideImplements the initial GhostBSD "Setup Station" first-boot wizard with a modular GTK3 UI, centralized state and i18n, secure system-call helpers, and modernized packaging/build + translation tooling. Sequence diagram for Setup Station navigation and system setupsequenceDiagram
actor User
participant Init as setup_station_init
participant Win as Window
participant Iface as Interface
participant Lang as Language
participant Kbd as Keyboard
participant TZ as TimeZone
participant Net as NetworkSetup
participant Admin as AddAdminUser
participant SW as SetupWindow
participant SP as SetupProgress
participant SS as setup_system_module
participant SC as system_calls
User->>Init: execute first boot wizard
Init->>Win: get_window() and configure
Init->>Iface: get_interface()
Iface->>Lang: initialize()
Iface->>Win: add(language_root_widget)
Win-->>User: show language screen
User->>Iface: click Next on Language
Iface->>Lang: get_language() and save_language()
Lang->>SC: localize_system(language_code)
Iface->>Kbd: get_model() and insert page
Iface->>Win: show_all()
Win-->>User: show keyboard screen
User->>Iface: click Next on Keyboard
Iface->>Kbd: save_keyboard()
Kbd->>SC: set_keyboard(layout, variant, model)
Iface->>TZ: get_model() and insert page
Win-->>User: show timezone screen
User->>Iface: click Next on Timezone
Iface->>TZ: apply_timezone()
TZ->>SC: set_timezone(timezone)
Iface->>Net: get_model() and insert page
Win-->>User: show network screen
User->>Iface: configure WiFi and click Next
Iface->>Net: (optional) wifi_setup and connect
Iface->>Admin: get_model() and insert page
Win-->>User: show admin user screen
User->>Admin: enter user and password
Admin->>common_password: password_strength(password)
Admin-->>Button: enable next_button
User->>Iface: click Next on Admin
Iface->>Admin: save_admin_user()
Admin->>SC: set_admin_user(username, name, password, shell, homedir, hostname)
Iface->>SW: create SetupWindow
Iface->>SP: create SetupProgress
SP->>SS: start setup_system(progress_bar) thread
SS->>Lang: save_language()
Lang->>SC: localize_system()
SS->>Kbd: save_keyboard()
Kbd->>SC: set_keyboard()
SS->>TZ: apply_timezone()
TZ->>SC: set_timezone()
SS->>Admin: save_admin_user()
Admin->>SC: set_admin_user()
SS-->>User: progress_bar shows "Setup complete"
Class diagram for Setup Station first-boot wizard architectureclassDiagram
class SetupData {
<<datastore>>
+string language
+string language_code
+string keyboard_layout
+string keyboard_layout_code
+string keyboard_variant
+string keyboard_model
+string keyboard_model_code
+string timezone
+dict network_config
+string username
+string user_fullname
+string user_password
+string user_shell
+string user_home_directory
+string hostname
+string root_password
+reset() void
}
class Window {
<<singleton>>
-Gtk_Window window
+connect(signal, callback) int
+set_border_width(width) void
+set_default_size(width, height) void
+set_size_request(width, height) void
+set_title(title) void
+set_icon_from_file(filename) void
+add(widget) void
+show_all() void
+get_window() Gtk_Window
}
class Button {
<<utility>>
+Gtk_Button back_button
+Gtk_Button next_button
-Gtk_Box _box
+update_button_labels() void
+hide_all() void
+show_initial() void
+show_back() void
+hide_back() void
+box() Gtk_Box
}
class Interface {
<<controller>>
+language
+keyboard
+timezone
+network_setup
+add_admin
+Gtk_Notebook page
+Gtk_Notebook nbButton
+get_interface() Gtk_Box
+delete(_widget, _event) void
+next_page(_widget) void
+back_page(_widget) void
}
class Language {
<<screen>>
+Gtk_Box vbox1
+string language
+Gtk_TreeView treeview
+Gtk_Label welcome_text
+Gtk_Label language_column_header
+language_selection(tree_selection) void
+update_ui_text() void
+setup_language_columns(treeview) void
+initialize() void
+get_model() Gtk_Box
+get_language() string
+save_language() void
}
class Keyboard {
<<screen>>
+Gtk_Box vbox1
+string kb_layout
+string kb_variant
+string kb_model
+Gtk_TreeView treeView
+Gtk_TreeStore model_store
+Gtk_TreeSelection tree_selection
+layout_columns(treeview) void
+variant_columns(treeview) void
+layout_selection(tree_selection) void
+model_selection(tree_selection) void
+get_model() Gtk_Box
+save_keyboard_data() void
+save_keyboard() void
}
class PlaceHolderEntry {
<<widget>>
+string placeholder
+bool _default
+get_text() string
-_focus_in_event(_widget, _event) void
-_focus_out_event(_widget, _event) void
}
class TimeZone {
<<screen>>
+Gtk_Box vbox1
+string continent
+string city
+Gtk_TreeView continenttreeView
+Gtk_TreeView citytreeView
+Gtk_TreeStore city_store
+Gtk_TreeSelection continenttree_selection
+continent_columns(treeView) void
+city_columns(treeView) void
+continent_selection(tree_selection) void
+city_selection(tree_selection) void
+apply_timezone() void
+get_model() Gtk_Box
}
class NetworkSetup {
<<screen>>
+Gtk_Box vbox1
+dict network_info
+Gtk_Label wire_connection_label
+Gtk_Image wire_connection_image
+Gtk_Label wifi_connection_label
+Gtk_Image wifi_connection_image
+Gtk_Box connection_box
+Gtk_ListStore store
+Gtk_Window window
+Gtk_Entry password
+get_model() Gtk_Box
+wifi_stat(bar) string
+update_network_detection() void
+initialize() void
+wifi_setup(tree_selection, wifi_card) void
+add_to_wpa_supplicant(_widget, ssid_info, card) void
+try_to_connect_to_ssid(ssid, ssid_info, card) void
+restart_authentication(ssid_info, card) void
+on_check(widget) void
+authentication(ssid_info, card, failed) string
+close(_widget) void
+setup_wpa_supplicant(ssid, ssid_info, pwd) void
+open_wpa_supplicant(ssid) void
}
class AddAdminUser {
<<screen>>
+Gtk_Box vbox1
+Gtk_Entry user
+Gtk_Entry name
+Gtk_Entry password
+Gtk_Entry repassword
+Gtk_Label label3
+Gtk_Image img
+Gtk_Entry host
+string shell
+save_user_data() void
+get_user_information() dict
+save_admin_user() void
+user_and_host(_widget) void
+get_model() Gtk_Box
+password_verification(_widget) void
}
class SetupWindow {
<<screen>>
+Gtk_Box vBox
+get_model() Gtk_Box
}
class SetupProgress {
<<logic>>
+Gtk_ProgressBar pbar
+get_progressbar() Gtk_ProgressBar
}
class system_calls {
<<module>>
+replace_pattern(current, new, file) void
+language_dictionary() dict
+localize_system(locale) void
+keyboard_dictionary() dict
+keyboard_models() dict
+change_keyboard(kb_layout, kb_variant, kb_model) void
+set_keyboard(kb_layout, kb_variant, kb_model) void
+timezone_dictionary() dict
+set_timezone(timezone) void
+set_admin_user(username, name, password, shell, homedir, hostname) void
}
class common_password {
<<module>>
+lower_case(text) bool
+upper_case(text) bool
+lower_and_number(text) bool
+upper_and_number(text) bool
+lower_upper(text) bool
+lower_upper_number(text) bool
+all_character(text) bool
+password_strength(password) string
+deprecated(version, reason)
}
class setup_system_module {
<<module>>
+update_progress(progress_bar, text) void
+setup_system(progress_bar) void
}
%% Relationships
Interface --> Button
Interface --> Window
Interface --> Language
Interface --> Keyboard
Interface --> TimeZone
Interface --> NetworkSetup
Interface --> AddAdminUser
Interface --> SetupWindow
Interface --> SetupProgress
Language --> SetupData
Language --> Window
Language --> system_calls : uses language_dictionary
Keyboard --> SetupData
Keyboard --> system_calls : uses keyboard_dictionary
TimeZone --> SetupData
TimeZone --> system_calls : uses timezone_dictionary
NetworkSetup --> SetupData
AddAdminUser --> SetupData
AddAdminUser --> common_password : uses password_strength
AddAdminUser --> system_calls : uses set_admin_user
AddAdminUser --> Button
SetupWindow --> SetupProgress
setup_system_module --> Language : save_language
setup_system_module --> Keyboard : save_keyboard
setup_system_module --> TimeZone : apply_timezone
setup_system_module --> AddAdminUser : save_admin_user
PlaceHolderEntry --|> Gtk_Entry
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
General comments:
- In
interface_controller.Interface, the attributeslanguage,keyboard,timezone,network_setup, andadd_adminare never assigned to the actual screen classes before use (e.g.,cls.language.initialize()), so the interface will currently fail at runtime; wire these up toLanguage,Keyboard,TimeZone,NetworkSetup, andAddAdminUserbefore calling their methods. - In
NetworkSetup.setup_wpa_supplicant/open_wpa_supplicant, the SSID is written into/etc/wpa_supplicant.confwithout consistently quoting it (e.g.,ssid={ssid}inopen_wpa_supplicant), which will break for SSIDs with spaces or special characters; normalize all SSID writes to use quoted values and use context managers when opening the file. - The custom i18n commands in
setup.py(e.g.,UpdateTranslationsCommand.runandCreateTranslationCommand.run) useos.systemwith shell-constructed strings and only print errors; consider switching tosubprocess.runwith argument lists and explicit error handling to avoid shell quirks and to propagate failures more reliably.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `interface_controller.Interface`, the attributes `language`, `keyboard`, `timezone`, `network_setup`, and `add_admin` are never assigned to the actual screen classes before use (e.g., `cls.language.initialize()`), so the interface will currently fail at runtime; wire these up to `Language`, `Keyboard`, `TimeZone`, `NetworkSetup`, and `AddAdminUser` before calling their methods.
- In `NetworkSetup.setup_wpa_supplicant` / `open_wpa_supplicant`, the SSID is written into `/etc/wpa_supplicant.conf` without consistently quoting it (e.g., `ssid={ssid}` in `open_wpa_supplicant`), which will break for SSIDs with spaces or special characters; normalize all SSID writes to use quoted values and use context managers when opening the file.
- The custom i18n commands in `setup.py` (e.g., `UpdateTranslationsCommand.run` and `CreateTranslationCommand.run`) use `os.system` with shell-constructed strings and only print errors; consider switching to `subprocess.run` with argument lists and explicit error handling to avoid shell quirks and to propagate failures more reliably.
## Individual Comments
### Comment 1
<location> `setup.py:158` </location>
<code_context>
+ description="Setup Station - GhostBSD post-installation setup utility",
license='BSD',
author='Eric Turgeon',
url='https://github/GhostBSD/setup-station/',
package_dir={'': '.'},
install_requires=['setuptools'],
</code_context>
<issue_to_address>
**issue (bug_risk):** Project URL is missing the domain part and likely should be a full GitHub URL.
The current value `https://github/GhostBSD/setup-station/` is invalid. Please update it to the correct GitHub URL (e.g. `https://github.com/GhostBSD/setup-station/`) so package metadata correctly links to the project homepage.
</issue_to_address>
### Comment 2
<location> `setup_station/network_setup.py:404-413` </location>
<code_context>
+ ws += f'\n psk="{pwd}"\n'
+ ws += '}\n'
+ else:
+ ws = '\nnetwork={'
+ ws += f'\n ssid="{ssid}"'
+ ws += '\n key_mgmt=NONE'
+ ws += '\n wep_tx_keyidx=0'
+ ws += f'\n wep_key0={pwd}\n'
+ ws += '}\n'
+ wsf = open("/etc/wpa_supplicant.conf", 'a')
+ wsf.writelines(ws)
+ wsf.close()
+
+ @staticmethod
+ def open_wpa_supplicant(ssid: str) -> None:
+ """
+ Add open (no password) WiFi network to wpa_supplicant configuration.
+
+ Args:
+ ssid: SSID of the network
+ """
+ ws = '\nnetwork={'
+ ws += f'\n ssid={ssid}'
+ ws += '\n key_mgmt=NONE\n}\n'
+ with open("/etc/wpa_supplicant.conf", 'a') as wsf:
</code_context>
<issue_to_address>
**issue:** Open-network wpa_supplicant entry does not quote SSID, which will break SSIDs with spaces or special characters.
In `open_wpa_supplicant`, the SSID is written as `ssid={ssid}` without quotes, unlike `setup_wpa_supplicant` which uses `ssid="{ssid}"`. This will break networks whose SSIDs contain spaces or special characters because it produces an invalid `wpa_supplicant.conf`. Please quote the SSID here as well, e.g.:
```python
ws += f'\n ssid="{ssid}"'
```
</issue_to_address>
### Comment 3
<location> `setup_station/network_setup.py:284-292` </location>
<code_context>
+ ssid_info: Tuple containing SSID information
+ card: Name of the wireless network interface
+ """
+ if connectToSsid(ssid, card) is False:
+ delete_ssid_wpa_supplicant_config(ssid)
+ GLib.idle_add(cls.restart_authentication, ssid_info, card)
+ else:
+ for _ in list(range(30)):
+ if nic_status(card) == 'associated':
+ cls.network_info = networkdictionary()
+ print(cls.network_info)
+ cls.update_network_detection()
+ break
+ sleep(1)
</code_context>
<issue_to_address>
**issue (bug_risk):** GTK UI is updated from a background thread in `try_to_connect_to_ssid`, which is not thread-safe.
Because `try_to_connect_to_ssid` runs in a background thread, calling `cls.update_network_detection()` (which touches GTK widgets like `cls.wire_connection_label` and `cls.wifi_connection_image`) from there is unsafe. GTK updates must be done on the main thread.
As you already do on failure with `GLib.idle_add(cls.restart_authentication, ...)`, please also marshal the success path back to the main loop: wrap the `networkdictionary()` refresh and `update_network_detection()` call in `GLib.idle_add(...)` so all widget access happens on the GTK main thread.
</issue_to_address>
### Comment 4
<location> `setup_station/add_users.py:134-135` </location>
<code_context>
+ return self.vbox1
+
+ def password_verification(self, _widget, button3):
+ password = self.password.get_text()
+ password_strength(password, self.label3)
+ repassword = self.repassword.get_text()
+ if password == repassword and password != "" and " " not in password:
</code_context>
<issue_to_address>
**issue (bug_risk):** `password_strength` is called with two arguments but now only accepts one, which will raise a TypeError.
`password_strength` in `setup_station/common.py` now takes only `password: str` and returns a strength message, but here it’s still called as `password_strength(password, self.label3)`, which will raise a `TypeError` at runtime.
Update this call to use the new API, e.g.:
```python
strength_message = password_strength(password)
self.label3.set_text(strength_message)
```
and keep the rest of the verification logic as-is.
</issue_to_address>
### Comment 5
<location> `setup_station/system_calls.py:435-437` </location>
<code_context>
+ check=True
+ )
+
+ # Set hostname
+ run(['sysrc', f'hostname={hostname}'], check=True)
+ run(['hostname', hostname], check=True)
</code_context>
<issue_to_address>
**issue (bug_risk):** `set_admin_user` allows an empty hostname but still writes it to sysrc and `hostname`, which likely yields an invalid hostname.
Right now the hostname is only validated when it’s truthy:
```python
if hostname and not re.match(...):
raise ValueError(...)
```
but you always run:
```python
run(['sysrc', f'hostname={hostname}'], check=True)
run(['hostname', hostname], check=True)
```
If `hostname` is an empty string, this will set an empty hostname at the system level. Please either require a non-empty hostname (and raise if empty) or skip the `sysrc`/`hostname` calls when no hostname is provided.
</issue_to_address>
### Comment 6
<location> `setup_station/system_calls.py:51-57` </location>
<code_context>
result = run(
f'{pc_sysinstall} query-langs',
shell=True,
capture_output=True,
text=True,
check=True
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 7
<location> `setup_station/system_calls.py:53` </location>
<code_context>
shell=True,
</code_context>
<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
```suggestion
shell=False,
```
*Source: opengrep*
</issue_to_address>
### Comment 8
<location> `setup_station/system_calls.py:120-126` </location>
<code_context>
result1 = run(
f'{pc_sysinstall} xkeyboard-layouts',
shell=True,
capture_output=True,
text=True,
check=True
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 9
<location> `setup_station/system_calls.py:122` </location>
<code_context>
shell=True,
</code_context>
<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
```suggestion
shell=False,
```
*Source: opengrep*
</issue_to_address>
### Comment 10
<location> `setup_station/system_calls.py:137-143` </location>
<code_context>
result2 = run(
f'{pc_sysinstall} xkeyboard-variants',
shell=True,
capture_output=True,
text=True,
check=True
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 11
<location> `setup_station/system_calls.py:139` </location>
<code_context>
shell=True,
</code_context>
<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
```suggestion
shell=False,
```
*Source: opengrep*
</issue_to_address>
### Comment 12
<location> `setup_station/system_calls.py:169-175` </location>
<code_context>
result = run(
f'{pc_sysinstall} xkeyboard-models',
shell=True,
capture_output=True,
text=True,
check=True
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 13
<location> `setup_station/system_calls.py:171` </location>
<code_context>
shell=True,
</code_context>
<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
```suggestion
shell=False,
```
*Source: opengrep*
</issue_to_address>
### Comment 14
<location> `setup_station/system_calls.py:301-306` </location>
<code_context>
result = run(
[pc_sysinstall, 'list-tzones'],
capture_output=True,
text=True,
check=True
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 15
<location> `setup.py:104` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Raise a specific error instead of the general `Exception` or `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>If a piece of code raises a specific exception type
rather than the generic
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
or [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception),
the calling code can:
- get more information about what type of error it is
- define specific exception handling for it
This way, callers of the code can handle the error appropriately.
How can you solve this?
- Use one of the [built-in exceptions](https://docs.python.org/3/library/exceptions.html) of the standard library.
- [Define your own error class](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions) that subclasses `Exception`.
So instead of having code raising `Exception` or `BaseException` like
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
you can have code raising a specific error like
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
or
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>
### Comment 16
<location> `setup_station/add_admin.py:81` </location>
<code_context>
SetupData.hostname = hostname if hostname else f'{username}-ghostbsd'
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if-expression with `or` ([`or-if-exp-identity`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/or-if-exp-identity))
```suggestion
SetupData.hostname = hostname or f'{username}-ghostbsd'
```
<br/><details><summary>Explanation</summary>Here we find ourselves setting a value if it evaluates to `True`, and otherwise
using a default.
The 'After' case is a bit easier to read and avoids the duplication of
`input_currency`.
It works because the left-hand side is evaluated first. If it evaluates to
true then `currency` will be set to this and the right-hand side will not be
evaluated. If it evaluates to false the right-hand side will be evaluated and
`currency` will be set to `DEFAULT_CURRENCY`.
</details>
</issue_to_address>
### Comment 17
<location> `setup_station/system_calls.py:227` </location>
<code_context>
kx_model = kb_model if kb_model else "pc104"
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if-expression with `or` ([`or-if-exp-identity`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/or-if-exp-identity))
```suggestion
kx_model = kb_model or "pc104"
```
<br/><details><summary>Explanation</summary>Here we find ourselves setting a value if it evaluates to `True`, and otherwise
using a default.
The 'After' case is a bit easier to read and avoids the duplication of
`input_currency`.
It works because the left-hand side is evaluated first. If it evaluates to
true then `currency` will be set to this and the right-hand side will not be
evaluated. If it evaluates to false the right-hand side will be evaluated and
`currency` will be set to `DEFAULT_CURRENCY`.
</details>
</issue_to_address>
### Comment 18
<location> `setup_station/system_calls.py:228` </location>
<code_context>
kx_layout = kb_layout if kb_layout else "us"
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if-expression with `or` ([`or-if-exp-identity`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/or-if-exp-identity))
```suggestion
kx_layout = kb_layout or "us"
```
<br/><details><summary>Explanation</summary>Here we find ourselves setting a value if it evaluates to `True`, and otherwise
using a default.
The 'After' case is a bit easier to read and avoids the duplication of
`input_currency`.
It works because the left-hand side is evaluated first. If it evaluates to
true then `currency` will be set to this and the right-hand side will not be
evaluated. If it evaluates to false the right-hand side will be evaluated and
`currency` will be set to `DEFAULT_CURRENCY`.
</details>
</issue_to_address>
### Comment 19
<location> `setup.py:34-38` </location>
<code_context>
def data_file_list(install_base, source_base):
"""
Generate list of data files for installation.
Args:
install_base: Base installation path
source_base: Source directory to scan
Returns:
List of (install_path, files) tuples for setuptools
"""
data = []
for root, subFolders, files in os.walk(source_base):
file_list = []
for f in files:
file_list.append(os.path.join(root, f))
# Only add directories that actually have files
if file_list:
data.append((root.replace(source_base, install_base), file_list))
return data
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Convert for loop into list comprehension ([`list-comprehension`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/list-comprehension/))
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if file_list := [os.path.join(root, f) for f in files]:
```
</issue_to_address>
### Comment 20
<location> `setup_station/add_users.py:26` </location>
<code_context>
def save_selection(self):
uname = self.user.get_text()
name = self.name.get_text()
up = self.password.get_text()
shell = self.sh
hf = '/home/%s' % self.user.get_text()
hst = self.host.get_text()
ul = [uname, name, up, shell, hf]
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace interpolated string formatting with f-string ([`replace-interpolation-with-fstring`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/replace-interpolation-with-fstring/))
```suggestion
hf = f'/home/{self.user.get_text()}'
```
</issue_to_address>
### Comment 21
<location> `setup_station/add_users.py:51` </location>
<code_context>
def user_and_host(self, _widget):
username = self.name.get_text().split()
self.host.set_text("%s-ghostbsd-pc" % username[0].lower())
self.user.set_text(username[0].lower())
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace interpolated string formatting with f-string ([`replace-interpolation-with-fstring`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/replace-interpolation-with-fstring/))
```suggestion
self.host.set_text(f"{username[0].lower()}-ghostbsd-pc")
```
</issue_to_address>
### Comment 22
<location> `setup_station/common.py:107` </location>
<code_context>
def password_strength(password: str) -> str:
"""
Evaluate password strength and return the message.
Args:
password: The password to evaluate
Returns:
str: Message describing password strength or validation error
"""
same_character_type = any(
[
lower_case(password),
upper_case(password),
password.isdigit()
]
)
mix_character = any(
[
lower_and_number(password),
upper_and_number(password),
lower_upper(password)
]
)
# Passwords that should not be allowed
not_allowed = {'password', 'Password', 'PASSWORD'}
# Check if a password is not allowed
if password in not_allowed:
return get_text("Password not allowed")
elif ' ' in password or '\t' in password:
return get_text("Space not allowed")
elif len(password) <= 4:
return get_text("Super Weak")
elif len(password) <= 8 and same_character_type:
return get_text("Super Weak")
elif len(password) <= 8 and mix_character:
return get_text("Very Weak")
elif len(password) <= 8 and lower_upper_number(password):
return get_text("Fairly Weak")
elif len(password) <= 8 and all_character(password):
return get_text("Weak")
elif len(password) <= 12 and same_character_type:
return get_text("Very Weak")
elif len(password) <= 12 and mix_character:
return get_text("Fairly Weak")
elif len(password) <= 12 and lower_upper_number(password):
return get_text("Weak")
elif len(password) <= 12 and all_character(password):
return get_text("Strong")
elif len(password) <= 16 and same_character_type:
return get_text("Fairly Weak")
elif len(password) <= 16 and mix_character:
return get_text("Weak")
elif len(password) <= 16 and lower_upper_number(password):
return get_text("Strong")
elif len(password) <= 16 and all_character(password):
return get_text("Fairly Strong")
elif len(password) <= 20 and same_character_type:
return get_text("Weak")
elif len(password) <= 20 and mix_character:
return get_text("Strong")
elif len(password) <= 20 and lower_upper_number(password):
return get_text("Fairly Strong")
elif len(password) <= 20 and all_character(password):
return get_text("Very Strong")
elif len(password) <= 24 and same_character_type:
return get_text("Strong")
elif len(password) <= 24 and mix_character:
return get_text("Fairly Strong")
elif len(password) <= 24 and lower_upper_number(password):
return get_text("Very Strong")
elif len(password) <= 24 and all_character(password):
return get_text("Super Strong")
elif same_character_type:
return get_text("Fairly Strong")
else:
return get_text("Super Strong")
</code_context>
<issue_to_address>
**issue (code-quality):** Low code quality found in password\_strength - 14% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))
<br/><details><summary>Explanation</summary>The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.</details>
</issue_to_address>
### Comment 23
<location> `setup_station/keyboard.py:63-65` </location>
<code_context>
def get_text(self):
if self._default:
return ''
return Gtk.Entry.get_text(self)
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return '' if self._default else Gtk.Entry.get_text(self)
```
</issue_to_address>
### Comment 24
<location> `setup_station/network_setup.py:129` </location>
<code_context>
@classmethod
def initialize(cls) -> None:
"""
Initialize the network setup UI.
Detects network interfaces and creates the interface for wired/wireless setup.
"""
cls.network_info = networkdictionary()
print(cls.network_info)
cls.vbox1 = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, homogeneous=False, spacing=0)
cls.vbox1.show()
cards = cls.network_info['cards']
card_list = list(cards.keys())
r = re.compile("wlan")
wlan_list = list(filter(r.match, card_list))
wire_list = list(set(card_list).difference(wlan_list))
cls.wire_connection_label = Gtk.Label()
cls.wire_connection_label.set_xalign(0.01)
cls.wire_connection_image = Gtk.Image()
cls.wifi_connection_label = Gtk.Label()
cls.wifi_connection_label.set_xalign(0.01)
cls.wifi_connection_image = Gtk.Image()
if wire_list:
for card in wire_list:
if cards[card]['state']['connection'] == 'Connected':
wire_text = get_text('Network card connected to the internet')
cls.wire_connection_image.set_from_stock(Gtk.STOCK_YES, 5)
print('Connected True')
break
else:
wire_text = get_text('Network card not connected to the internet')
cls.wire_connection_image.set_from_stock(Gtk.STOCK_NO, 5)
else:
wire_text = get_text('No network card detected')
cls.wire_connection_image.set_from_stock(Gtk.STOCK_NO, 5)
cls.wire_connection_label.set_label(wire_text)
wlan_card = ""
if wlan_list:
for wlan_card in wlan_list:
if cards[wlan_card]['state']['connection'] == 'Connected':
wifi_text = get_text('WiFi card detected and connected to an access point')
cls.wifi_connection_image.set_from_stock(Gtk.STOCK_YES, 5)
break
else:
wifi_text = get_text('WiFi card detected but not connected to an access point')
cls.wifi_connection_image.set_from_stock(Gtk.STOCK_NO, 5)
else:
wifi_text = get_text('WiFi card not detected or not supported')
cls.wifi_connection_image.set_from_stock(Gtk.STOCK_NO, 5)
cls.wifi_connection_label.set_label(wifi_text)
cls.connection_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, homogeneous=True, spacing=20)
if wlan_card:
# add a default card variable
sw = Gtk.ScrolledWindow()
sw.set_shadow_type(Gtk.ShadowType.ETCHED_IN)
sw.set_policy(Gtk.PolicyType.AUTOMATIC, Gtk.PolicyType.AUTOMATIC)
cls.store = Gtk.ListStore(GdkPixbuf.Pixbuf, str, str)
for ssid in cls.network_info['cards'][wlan_card]['info']:
ssid_info = cls.network_info['cards'][wlan_card]['info'][ssid]
bar = ssid_info[4]
stat = NetworkSetup.wifi_stat(bar)
pixbuf = Gtk.IconTheme.get_default().load_icon(stat, 32, 0)
cls.store.append([pixbuf, ssid, f'{ssid_info}'])
treeview = Gtk.TreeView()
treeview.set_model(cls.store)
treeview.set_rules_hint(True)
pixbuf_cell = Gtk.CellRendererPixbuf()
pixbuf_column = Gtk.TreeViewColumn(get_text('Stat'), pixbuf_cell)
pixbuf_column.add_attribute(pixbuf_cell, "pixbuf", 0)
pixbuf_column.set_resizable(True)
treeview.append_column(pixbuf_column)
cell = Gtk.CellRendererText()
column = Gtk.TreeViewColumn(get_text('SSID'), cell, text=1)
column.set_sort_column_id(1)
treeview.append_column(column)
tree_selection = treeview.get_selection()
tree_selection.set_mode(Gtk.SelectionMode.SINGLE)
tree_selection.connect("changed", cls.wifi_setup, wlan_card)
sw.add(treeview)
cls.connection_box.pack_start(sw, True, True, 50)
main_grid = Gtk.Grid()
main_grid.set_row_spacing(10)
main_grid.set_column_spacing(10)
main_grid.set_column_homogeneous(True)
main_grid.set_row_homogeneous(True)
cls.vbox1.pack_start(main_grid, True, True, 10)
main_grid.attach(cls.wire_connection_image, 2, 1, 1, 1)
main_grid.attach(cls.wire_connection_label, 3, 1, 8, 1)
main_grid.attach(cls.wifi_connection_image, 2, 2, 1, 1)
main_grid.attach(cls.wifi_connection_label, 3, 2, 8, 1)
main_grid.attach(cls.connection_box, 1, 4, 10, 5)
</code_context>
<issue_to_address>
**issue (code-quality):** Low code quality found in NetworkSetup.initialize - 24% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))
<br/><details><summary>Explanation</summary>The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.</details>
</issue_to_address>
### Comment 25
<location> `setup_station/network_setup.py:242-252` </location>
<code_context>
@classmethod
def wifi_setup(cls, tree_selection: Gtk.TreeSelection, wifi_card: str) -> None:
"""
Handle WiFi network selection from the list.
Args:
tree_selection: TreeSelection containing the selected SSID
wifi_card: Name of the wireless network interface
"""
model, treeiter = tree_selection.get_selected()
if treeiter is not None:
ssid = model[treeiter][1]
ssid_info = cls.network_info['cards'][wifi_card]['info'][ssid]
caps = ssid_info[6]
print(ssid) # added the code to authenticate.
print(ssid_info)
if caps == 'E' or caps == 'ES':
if f'"{ssid}"' in open("/etc/wpa_supplicant.conf").read():
cls.try_to_connect_to_ssid(ssid, ssid_info, wifi_card)
else:
NetworkSetup.open_wpa_supplicant(ssid)
cls.try_to_connect_to_ssid(ssid, ssid_info, wifi_card)
else:
if f'"{ssid}"' in open('/etc/wpa_supplicant.conf').read():
cls.try_to_connect_to_ssid(ssid, ssid_info, wifi_card)
else:
cls.authentication(ssid_info, wifi_card, False)
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Merge else clause's nested if statement into elif ([`merge-else-if-into-elif`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-else-if-into-elif/))
- Replace multiple comparisons of same variable with `in` operator ([`merge-comparisons`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-comparisons/))
- Hoist nested repeated code outside conditional statements ([`hoist-similar-statement-from-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/hoist-similar-statement-from-if/))
</issue_to_address>
### Comment 26
<location> `setup_station/network_setup.py:350` </location>
<code_context>
@classmethod
def authentication(cls, ssid_info: tuple, card: str, failed: bool) -> str:
"""
Display WiFi authentication dialog.
Args:
ssid_info: Tuple containing SSID information
card: Name of the wireless network interface
failed: Whether previous authentication attempt failed
Returns:
str: Status message
"""
cls.window = Gtk.Window()
cls.window.set_title(get_text("WiFi Network Authentication Required"))
cls.window.set_border_width(0)
cls.window.set_size_request(500, 200)
box1 = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, homogeneous=False, spacing=0)
cls.window.add(box1)
box1.show()
box2 = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, homogeneous=False, spacing=10)
box2.set_border_width(10)
box1.pack_start(box2, True, True, 0)
box2.show()
# Creating MBR or GPT drive
if failed:
title = ssid_info[0] + " " + get_text("Wi-Fi Network Authentication failed")
else:
title = get_text("Authentication required by") + " " + ssid_info[0] + " " + get_text("Wi-Fi Network")
label = Gtk.Label(label=f"<b><span size='large'>{title}</span></b>")
label.set_use_markup(True)
pwd_label = Gtk.Label(label=get_text("Password:"))
cls.password = Gtk.Entry()
cls.password.set_visibility(False)
check = Gtk.CheckButton(label=get_text("Show password"))
check.connect("toggled", cls.on_check)
table = Gtk.Table(1, 2, True)
table.attach(label, 0, 5, 0, 1)
table.attach(pwd_label, 1, 2, 2, 3)
table.attach(cls.password, 2, 4, 2, 3)
table.attach(check, 2, 4, 3, 4)
box2.pack_start(table, False, False, 0)
box2 = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, homogeneous=False, spacing=10)
box2.set_border_width(5)
box1.pack_start(box2, False, True, 0)
box2.show()
# Add create_scheme button
cancel = Gtk.Button(stock=Gtk.STOCK_CANCEL)
cancel.connect("clicked", cls.close)
connect = Gtk.Button(stock=Gtk.STOCK_CONNECT)
connect.connect("clicked", cls.add_to_wpa_supplicant, ssid_info, card)
table = Gtk.Table(1, 2, True)
table.set_col_spacings(10)
table.attach(connect, 4, 5, 0, 1)
table.attach(cancel, 3, 4, 0, 1)
box2.pack_end(table, True, True, 5)
cls.window.show_all()
return 'Done'
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use f-string instead of string concatenation ([`use-fstring-for-concatenation`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-fstring-for-concatenation/))
```suggestion
title = f"{ssid_info[0]} " + get_text("Wi-Fi Network Authentication failed")
```
</issue_to_address>
### Comment 27
<location> `setup_station/network_setup.py:404` </location>
<code_context>
@staticmethod
def setup_wpa_supplicant(ssid: str, ssid_info: tuple, pwd: str) -> None:
"""
Write WiFi credentials to wpa_supplicant configuration.
Args:
ssid: SSID of the network
ssid_info: Tuple containing SSID security information
pwd: Password for the network
"""
if 'RSN' in ssid_info[-1]:
ws = '\nnetwork={'
ws += f'\n ssid="{ssid}"'
ws += '\n key_mgmt=WPA-PSK'
ws += '\n proto=RSN'
ws += f'\n psk="{pwd}"\n'
ws += '}\n'
elif 'WPA' in ssid_info[-1]:
ws = '\nnetwork={'
ws += f'\n ssid="{ssid}"'
ws += '\n key_mgmt=WPA-PSK'
ws += '\n proto=WPA'
ws += f'\n psk="{pwd}"\n'
ws += '}\n'
else:
ws = '\nnetwork={'
ws += f'\n ssid="{ssid}"'
ws += '\n key_mgmt=NONE'
ws += '\n wep_tx_keyidx=0'
ws += f'\n wep_key0={pwd}\n'
ws += '}\n'
wsf = open("/etc/wpa_supplicant.conf", 'a')
wsf.writelines(ws)
wsf.close()
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Hoist repeated code outside conditional statement [×3] ([`hoist-statement-from-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/hoist-statement-from-if/))
- Use `with` when opening file to ensure closure ([`ensure-file-closed`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/ensure-file-closed/))
</issue_to_address>
### Comment 28
<location> `setup_station/setup_system.py:31` </location>
<code_context>
def update_progress(progress_bar: Gtk.ProgressBar, text: str) -> None:
"""
This function is used to update the progress bar.
:param progress_bar: The progress bar to update.
:param text: The text to display.
"""
new_val = progress_bar.get_fraction() + 0.000003
progress_bar.set_fraction(new_val)
progress_bar.set_text(text[0:80])
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace a[0:x] with a[:x] and a[x:len(a)] with a[x:] ([`remove-redundant-slice-index`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-slice-index/))
```suggestion
progress_bar.set_text(text[:80])
```
</issue_to_address>
### Comment 29
<location> `setup_station/system_calls.py:104` </location>
<code_context>
def localize_system(locale: str) -> None:
"""
Apply locale configuration to the system.
Args:
locale: Locale code (e.g., 'en_US', 'fr_FR')
Raises:
IOError: If file operations fail
ValueError: If locale is empty
"""
if not locale:
raise ValueError("Locale cannot be empty")
slick_greeter = "/usr/local/share/xgreeters/slick-greeter.desktop"
gtk_greeter = "/usr/local/share/xgreeters/lightdm-gtk-greeter.desktop"
try:
replace_pattern('lang=C', f'lang={locale}', '/etc/login.conf')
replace_pattern('en_US', locale, '/etc/profile')
replace_pattern('en_US', locale, '/usr/share/skel/dot.profile')
if os.path.exists(slick_greeter):
replace_pattern(
'Exec=slick-greeter',
f'Exec=env LANG={locale}.UTF-8 slick-greeter',
slick_greeter
)
elif os.path.exists(gtk_greeter):
replace_pattern(
'Exec=lightdm-gtk-greeter',
f'Exec=env LANG={locale}.UTF-8 lightdm-gtk-greeter',
gtk_greeter
)
except (IOError, OSError, FileNotFoundError) as e:
raise IOError(f"Failed to localize system with locale '{locale}': {e}") from e
</code_context>
<issue_to_address>
**suggestion (code-quality):** Remove redundant exceptions from an except clause ([`remove-redundant-exception`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-exception/))
```suggestion
except (IOError, OSError) as e:
```
</issue_to_address>
### Comment 30
<location> `setup_station/system_calls.py:204` </location>
<code_context>
def change_keyboard(kb_layout: str, kb_variant: str = None, kb_model: str = None) -> None:
"""
Change the current X session keyboard layout immediately.
Args:
kb_layout: Keyboard layout code (e.g., 'us', 'fr', 'de')
kb_variant: Optional keyboard variant
kb_model: Optional keyboard model
Raises:
RuntimeError: If setxkbmap command fails
"""
try:
if kb_variant is None and kb_model is not None:
run(['setxkbmap', '-layout', kb_layout, '-model', kb_model], check=True)
elif kb_variant is not None and kb_model is None:
run(['setxkbmap', '-layout', kb_layout, '-variant', kb_variant], check=True)
elif kb_variant is not None and kb_model is not None:
run(['setxkbmap', '-layout', kb_layout, '-variant', kb_variant, '-model', kb_model], check=True)
else:
run(['setxkbmap', '-layout', kb_layout], check=True)
except Exception as e:
raise RuntimeError(f"Failed to change keyboard layout: {e}") from e
</code_context>
<issue_to_address>
**suggestion (code-quality):** Remove redundant conditional ([`remove-redundant-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-if/))
```suggestion
elif kb_variant is not None:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
github-project-automation
bot
moved this from In Review
to In Progress
in Development Tracker
This commit addresses all critical, high, and low priority issues from the
Sourcery AI code review, significantly improving code security, maintainability,
and quality.
Security fixes:
- Fix shell injection vulnerabilities in system_calls.py by removing all
shell=True usage and using list-based subprocess arguments
- Fix thread safety in network_setup.py:292 by wrapping GTK widget updates
in GLib.idle_add() when called from background threads
- Add empty hostname validation in system_calls.py to prevent commands
executing with empty values
Bug fixes:
- Fix API signature mismatch in add_users.py:135 - password_strength() now
called with correct single argument, label updated separately
- Fix SSID quoting in network_setup.py:437 by adding quotes to open networks
- Fix GitHub URL in setup.py:158 (add missing .com)
Code quality improvements:
- Refactor password_strength() in common.py:
* Replace 85-line if/elif chain with clean match statement
* Simplify 6 helper functions to 4 using regex | operator
* Use industry-standard length ranges (≤8, 9-12, 13-15, 16+)
* Remove "Super Weak"/"Super Strong" levels (8 → 6 strength levels)
* Add _get_complexity_tier() helper for better organization
- Replace % string formatting with f-strings in add_users.py
- Update network_setup.py to use context managers for file operations
All blocking issues resolved. One false positive identified (uninitialized
screen classes - verified as correct utility class pattern usage).
github-project-automation
bot
moved this from In Progress
to Done
in Development Tracker
Member
Author
It is a hardcoded variable. That is a false positive. |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Initial work on setup-station to provide a first-boot setup experience for GhostBSD, enabling OEM style installation and custom
deployments.
What This PR Does
This PR delivers the initial implementation of setup-station - the setup wizard that runs after GhostBSD installation. It transforms a
fresh installation into a personalized, ready-to-use system, establishing the foundation for OEM style installation scenarios for
system integrators and distributors.
Major Changes
🏗️ Core Implementation
🔒 Security Improvements
🌍 Complete Translation Support
📦 Code Quality
Components Included
Translated modules:
Build system:
Future Work
Future enhancements will include:
Testing
✅ Manual testing on GhostBSD fresh installations
✅ Translation infrastructure verified with French translations
✅ Security patterns reviewed for common vulnerabilities
✅ Build system tested with setup.py update_translations
Summary
This PR establishes the foundation for a first-boot setup experience and enables GhostBSD for OEM style installation partnerships and
custom distributions.
Summary by Sourcery
Introduce the initial OEM-style first-boot "Setup Station" wizard for GhostBSD, including post-install configuration screens, supporting system utilities, and i18n-ready packaging.
New Features:
Enhancements:
Build:
Chores: