URI validation for winget cli commands #4707
Conversation
| auto isAllowed = configurationPolicies->at(zone); | ||
| if(!isAllowed) | ||
| { | ||
| context.Reporter.Error() << "Configuration is disabled for Zone: " << zone << std::endl; |
There was a problem hiding this comment.
TODO:
- Finalizing strings
- Move to resource file
| @@ -122,6 +122,10 @@ If you disable this setting, users will not be able to use the Windows Package M | |||
| If you disable or do not configure this setting, no proxy will be used by default. | |||
|
|
|||
| If you enable this setting, the specified proxy will be used by default.</string> | |||
| <string id="EnableDSCAllowedZones">Enable App Installer Allowed Zones for DSC</string> | |||
| <string id="EnableDSCAllowedZonesExplanation"></string> | |||
There was a problem hiding this comment.
Yes, but also, we're still thinking about the scope that this policy should cover (DSC only, or more ... )
| switch (response.Decision()) | ||
| { | ||
| case AppInstaller::UriValidation::UriValidationDecision::Block: | ||
| context.Reporter.Error() << std::endl << "Blocked by smart screen" << std::endl << "Feedback: " << response.Feedback() << std::endl; |
There was a problem hiding this comment.
If Feedback isn't localized, it should probably only be sent to the log and not stdout.
There was a problem hiding this comment.
All strings are not finalized/localized yet, there's an email thread for deciding on a couple things including localization
| // The decision made based on the Uri validation. | ||
| enum class UriValidationDecision | ||
| { | ||
| Allow, |
There was a problem hiding this comment.
Isn't there also a Warn response that SmartScreen can return?
https://learn.microsoft.com/en-us/defender-endpoint/web-protection-overview#order-of-precedence
There was a problem hiding this comment.
You're right, there's a PUA response as well. However, PUA/Warn will also be treated as Blocked, but the details will be communicated in the logs instead and the logic will be in the injected code. I can potentially reduce this enum to a bool. I will revisit this later once things are a little bit more finalized.
| // Uri to give feedback to smart screen about the decision. | ||
| std::string m_feedback; | ||
| public: | ||
| UriValidationResult(UriValidationDecision decision) : m_decision(decision), m_feedback(std::string()) {} |
There was a problem hiding this comment.
Should there be a default (enpty) constructor?
| <None Include="packages.config" Condition="Exists('$(MSBuildProjectDirectory)\packages.config')" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Windows.CppWinRT" Version="2.0.230706.1" TargetFramework="native" /> |
There was a problem hiding this comment.
This doesnt seem to match the version of cppwinrt in NOTICE.txt. Does the version need to be bumped across all the solution files, or is there something I’m missing?
Line 769 in 2306d08
AmelBawa-msft
changed the title
Description
flowchart TD %%{init:{'flowchart':{"defaultRenderer": "elk"}}}%% A[winget cli] --> B B{is configuration?} --> |yes| C[Get URI Zone] B --> |no| D{Package catalog trusted?} D --> |no| C D --> |yes| E(Resume operation) C --> F{Zone blocked by policy?} F -->|yes| G(APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY) F --> |no| H{Zone Internet or Untrusted?} H --> |yes| I{SmartScreen blocked by policy?} H --> |no| E I --> |no| J{Blocked by SmartScreen?} I --> |yes| E J --> |yes| K(APPINSTALLER_CLI_ERROR_BLOCKED_BY_REPUTATION_SERVICE) J --> |no| E style G fill:red,stroke:darkred,color:white style K fill:red,stroke:darkred,color:white style E fill:green,stroke:darkgreen,color:whiteTODO
Follow ups
Microsoft Reviewers: Open in CodeFlow