Skip to content

Add toggle to enable/disable insecure settings#3723

Merged
Martí Climent (marticliment) merged 12 commits intomainfrom
Add-secure-settings-toggle
Jun 13, 2025
Merged

Add toggle to enable/disable insecure settings#3723
Martí Climent (marticliment) merged 12 commits intomainfrom
Add-secure-settings-toggle

Conversation

@marticliment
Copy link
Collaborator

@marticliment Martí Climent (marticliment) commented Jun 7, 2025

Certain features, such as custom command-line arguments, and future ones such as pre-install and post-install scripts, pose an important security risk. They should be disabled by default, and in a way that a remote, unauthorized agent cannot manually enable this features.

  • Implement secure settings (can only be changed with administrator permissions, yet they should be user-based)
  • Create a settings page to handle these secure settings. Create a UI warning to enable and disable them.

@marticliment Martí Climent (marticliment) marked this pull request as ready for review June 13, 2025 22:21
@marticliment Martí Climent (marticliment) merged commit 9d279be into main Jun 13, 2025
2 checks passed
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a toggle for enabling/disabling insecure settings to reduce security risks. Key changes include updating UI text and controls for administrator-related settings, integrating new secure settings CLI commands, and refactoring installation option handling to work with secure settings.

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/UniGetUI/Pages/SettingsPages/GeneralPages/Updates.xaml Updated border and text for settings cards to reflect new secure settings messaging.
src/UniGetUI/Pages/SettingsPages/GeneralPages/SettingsHomepage.xaml(.cs) Renamed and reordered settings buttons, and added a new shortcut for manager preferences.
src/UniGetUI/Pages/SettingsPages/GeneralPages/Operations.xaml Updated text to match the revised administrator settings terminology.
src/UniGetUI/Pages/SettingsPages/GeneralPages/Experimental.xaml Minor UI spacing adjustments with no functional change.
src/UniGetUI/Pages/SettingsPages/GeneralPages/Administrator.xaml(.cs) Introduced a warning title bar and reworded texts to alert users about potential security risks.
src/UniGetUI/Pages/DialogPages/InstallOptions_Package.xaml.cs Commented out a condition related to overriding settings – behavior change should be verified.
src/UniGetUI/EntryPoint.cs Added new CLI argument branches to enable/disable secure settings.
src/UniGetUI/Controls/SettingsWidgets/{SecureCheckboxCard.cs, CheckboxCard.cs} Added support for warning texts and inversion logic for secure settings toggles.
src/UniGetUI/CLIHandler.cs Updated trimming logic for CLI arguments and added secure settings commands.
src/UniGetUI.PackageEngine.Serializable/InstallOptions.cs & InstallOptionsFactory.cs Removed in-method sanitation and introduced secure options handling.
src/UniGetUI.Core.SecureSettings/* New project to manage secure settings with elevated permissions.
cli-arguments.md Updated documentation to reflect new CLI commands for secure settings.
Comments suppressed due to low confidence (1)

src/UniGetUI/Pages/SettingsPages/GeneralPages/Administrator.xaml:77

  • There is a spelling mistake in the warning text ('thay' should be 'they').
<widgets:TranslatedTextBlock Text="Pre and post install commands can do very nasty things to your device, if designed to do so. Be aware thay they may break things unless used carefully" />

if (options.CustomParameters.Count > 0)
Logger.Warn($"Custom CLI parameters [{string.Join(' ', options.CustomParameters)}] will be discarded");

options.CustomParameters = [];
Copy link

Copilot AI Jun 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assignment 'options.CustomParameters = [];' may cause issues if CustomParameters is not of an array type; consider using an appropriate collection initializer (e.g., new List()) to match its declared type.

Copilot uses AI. Check for mistakes.
Comment on lines +318 to +321
//if (!options.OverridesNextLevelOpts)
//{
options = await InstallOptionsFactory.LoadApplicableAsync(this.Package, overridePackageOptions: options);
//}
Copy link

Copilot AI Jun 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting out the condition that checks 'OverridesNextLevelOpts' changes the logic of updating options; please confirm that this behavioral change is intentional.

Copilot uses AI. Check for mistakes.
CoreTools.Translate("Enable the settings below if and only if you fully understand what they do, and the implications they may have.") + "\n\n" +
CoreTools.Translate("The settings will list, in their descriptions, the potential security issues they may have.") + " ";

// The following settings may pose a security risk, hence they are disabled by default. Enable them ONLY if you undertsand what you are doing. Some of those settings will show a UAC prompt before being enabled."
Copy link

Copilot AI Jun 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a spelling mistake in the comment ('undertsand' should be 'understand').

Suggested change
// The following settings may pose a security risk, hence they are disabled by default. Enable them ONLY if you undertsand what you are doing. Some of those settings will show a UAC prompt before being enabled."
// The following settings may pose a security risk, hence they are disabled by default. Enable them ONLY if you understand what you are doing. Some of those settings will show a UAC prompt before being enabled."

Copilot uses AI. Check for mistakes.
@marticliment Martí Climent (marticliment) deleted the Add-secure-settings-toggle branch July 18, 2025 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants