Skip to content

Commit

Permalink
Block control codes and truncate longer configuration text blocks (#4436
Browse files Browse the repository at this point in the history
)

## Change
All control codes in the range [0x0, 0x20) and the DELETE control code
0x7F (with the exceptions of the tab, line feed, and carriage return
characters) will result in an error from the YAML parser. An alternative
solution is to convert them to their control pictures, but it was
decided that it was best to fail at this time.

The configuration output for each unit during the "show" portion (used
by almost all of the commands to display details about the file) will
limit the amount of output allowed for any field that comes from an
external source. Data from the file will present a warning that it was
truncated just below its output. If anything is truncated, an overall
"error" will be output as well.
  • Loading branch information
JohnMcPMS committed May 1, 2024
1 parent 2b185be commit 457f84b
Show file tree
Hide file tree
Showing 20 changed files with 815 additions and 410 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/allow.txt
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ und
UNICODESTRING
uninstalling
Unmarshal
unskipped
untimes
updatefile
updatemanifest
Expand Down
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ ECustom
EFGH
EFile
efileresource
EMalicious
endregion
ENDSESSION
EPester
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarning);
WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningPromptApply);
WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningPromptTest);
WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningSetViewTruncated);
WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningValueTruncated);
WINGET_DEFINE_RESOURCE_STRINGID(ConfigureCommandLongDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ConfigureCommandShortDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ConfigureShowCommandLongDescription);
Expand Down
505 changes: 295 additions & 210 deletions src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp

Large diffs are not rendered by default.

13 changes: 13 additions & 0 deletions src/AppInstallerCLIE2ETests/ConfigureShowCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,18 @@ public void ShowDetailsFromHttpsConfigurationFile()
Assert.AreEqual(0, result.ExitCode);
Assert.True(result.StdOut.Contains(Constants.TestRepoName));
}

/// <summary>
/// This test ensures that there is not significant overflow from large strings in the configuration file.
/// </summary>
[Test]
public void ShowTruncatedDetailsAndFileContent()
{
var result = TestCommon.RunAICLICommand("configure show", $"{TestCommon.GetTestDataFile("Configuration\\LargeContentStrings.yml")} --verbose");
Assert.AreEqual(0, result.ExitCode);
Assert.True(result.StdOut.Contains("<this value has been truncated; inspect the file contents for the complete text>"));
Assert.True(result.StdOut.Contains("Some of the data present in the configuration file was truncated for this output; inspect the file contents for the complete content."));
Assert.False(result.StdOut.Contains("Line5"));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
properties:
configurationVersion: 0.2
resources:
- resource: xE2EMalicious/E2EMalicious
id: firstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirst
directives:
repository: AppInstallerCLIE2ETestsRepo
description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6"
settings:
key: Foo
description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6"
- resource: Unknown
dependsOn:
- firstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirst
directives:
module: UnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknown
repository: AppInstallerCLIE2ETestsRepo
description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6"
settings:
Path: ${WinGetConfigRoot}\ConfigServerUnexpectedExit.txt
Content: Contents!
description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6"
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#
# Module manifest for module 'xE2ETestResource'
#

@{

RootModule = 'xE2EMalicious.psm1'
ModuleVersion = '0.0.0.1'
GUID = 'a0be43e8-ac22-4244-8efc-7263dfa50b92'
CompatiblePSEditions = 'Core'
Author = "WinGet Dev Team"
CompanyName = 'Microsoft Corporation'
Copyright = '(c) Microsoft Corporation. All rights reserved.'
Description = "PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests"
PowerShellVersion = '7.2'
FunctionsToExport = @()
CmdletsToExport = @()
DscResourcesToExport = @(
'E2EMalicious'
)
HelpInfoURI = 'https://www.contoso.com/help'

# Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell.
PrivateData = @{

PSData = @{
ProjectUri = 'https://github.com/microsoft/winget-cli'
IconUri = 'https://www.contoso.com/icons/icon.png'
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# E2E module with resources.

enum Ensure
{
Absent
Present
}

# This resource just checks if a file is there or not with and if its with the specified content.
[DscResource()]
class E2EMalicious
{
[DscProperty(Key)]
[string] $Path

[DscProperty()]
[Ensure] $Ensure = [Ensure]::Present

[DscProperty()]
[string] $Content = $null

[E2EFileResource] Get()
{
if ([string]::IsNullOrEmpty($this.Path))
{
throw
}

$fileContent = $null
if (Test-Path -Path $this.Path -PathType Leaf)
{
$fileContent = Get-Content $this.Path -Raw
}

$result = @{
Path = $this.Path
Content = $fileContent
}

return $result
}

[bool] Test()
{
$get = $this.Get()

if (Test-Path -Path $this.Path -PathType Leaf)
{
if ($this.Ensure -eq [Ensure]::Present)
{
return $this.Content -eq $get.Content
}
}
elseif ($this.Ensure -eq [Ensure]::Absent)
{
return $true
}

return $false
}

[void] Set()
{
if (-not $this.Test())
{
if (Test-Path -Path $this.Path -PathType Leaf)
{
if ($this.Ensure -eq [Ensure]::Present)
{
Set-Content $this.Path $this.Content -NoNewline
}
else
{
Remove-Item $this.Path
}
}
else
{
if ($this.Ensure -eq [Ensure]::Present)
{
Set-Content $this.Path $this.Content -NoNewline
}
}
}
}
}
7 changes: 7 additions & 0 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -2889,4 +2889,11 @@ Please specify one of them using the --source option to proceed.</value>
<data name="MSStoreDownloadPackageNotFound" xml:space="preserve">
<value>The MSStore package could not be found.</value>
</data>
<data name="ConfigurationWarningSetViewTruncated" xml:space="preserve">
<value>Some of the data present in the configuration file was truncated for this output; inspect the file contents for the complete content.</value>
</data>
<data name="ConfigurationWarningValueTruncated" xml:space="preserve">
<value>&lt;this value has been truncated; inspect the file contents for the complete text&gt;</value>
<comment>Keep some form of separator like the "&lt;&gt;" around the text so that it stands out from the preceding text.</comment>
</data>
</root>
31 changes: 13 additions & 18 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@
<ClCompile Include="TestCommon.cpp" />
<ClCompile Include="WorkflowCommon.cpp" />
<ClCompile Include="WorkflowGroupPolicy.cpp" />
<ClCompile Include="Yaml.cpp" />
<ClCompile Include="YamlManifest.cpp" />
</ItemGroup>
<ItemGroup>
Expand Down Expand Up @@ -929,40 +930,34 @@
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-DefaultLocale.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Locale.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Locale2.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Shadow.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Shadow2.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Installer.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Node-Merge.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Node-Merge2.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Node-Mapping.yaml">
<DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent>
<FileType>Document</FileType>
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\ContainsEscapeControlCode.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
</ItemGroup>
<ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,9 @@
<ClCompile Include="PackageVersionDataManifest.cpp">
<Filter>Source Files\Common</Filter>
</ClCompile>
<ClCompile Include="Yaml.cpp">
<Filter>Source Files\Common</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down Expand Up @@ -978,5 +981,8 @@
<CopyFileToFolders Include="TestData\Node-Mapping.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\ContainsEscapeControlCode.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
</ItemGroup>
</Project>
36 changes: 33 additions & 3 deletions src/AppInstallerCLITests/Strings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ TEST_CASE("Format", "[strings]")
REQUIRE("First {1}" == Format("{0} {1}", "First"));
}

TEST_CASE("SplitIntoLines", "[string]")
TEST_CASE("SplitIntoLines", "[strings]")
{
REQUIRE(SplitIntoLines("Boring test") == std::vector<std::string>{ "Boring test" });
REQUIRE(SplitIntoLines(
Expand All @@ -261,7 +261,7 @@ TEST_CASE("SplitIntoLines", "[string]")
== std::vector<std::string>{ "You want my treasure?", "You can have it!", "I left everything I gathered in one place!", "You just have to find it!" });
}

TEST_CASE("SplitWithSeparator", "[string]")
TEST_CASE("SplitWithSeparator", "[strings]")
{
std::vector<std::string> test1 = Split("first;second;third", ';');
REQUIRE(test1.size() == 3);
Expand All @@ -285,10 +285,40 @@ TEST_CASE("SplitWithSeparator", "[string]")
REQUIRE(test4[1] == "spaces");
}

TEST_CASE("ConvertGuid", "[string]")
TEST_CASE("ConvertGuid", "[strings]")
{
std::string validGuidString = "{4d1e55b2-f16f-11cf-88cb-001111000030}";
GUID guid = { 0x4d1e55b2, 0xf16f, 0x11cf, 0x88, 0xcb, 0x00, 0x11, 0x11, 0x00, 0x00, 0x30 };

REQUIRE(CaseInsensitiveEquals(ConvertGuidToString(guid), validGuidString));
}

TEST_CASE("FindControlCodeToConvert", "[strings]")
{
REQUIRE(FindControlCodeToConvert("No codes") == std::string::npos);
REQUIRE(FindControlCodeToConvert("Allowed codes: \t\r\n") == std::string::npos);
REQUIRE(FindControlCodeToConvert("\x1bSkipped code", 1) == std::string::npos);

REQUIRE(FindControlCodeToConvert("\x1bUnskipped code") == 0);
REQUIRE(FindControlCodeToConvert("Escape code: \x1b") == 13);

std::string_view allCodes{ "\x0\x1\x2\x3\x4\x5\x6\x7\x8\xb\xc\xe\xf\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x7f"sv };
for (size_t i = 0; i < allCodes.length(); ++i)
{
REQUIRE(FindControlCodeToConvert(allCodes, i) == i);
}
}

TEST_CASE("ConvertControlCodesToPictures", "[strings]")
{
REQUIRE(ConvertControlCodesToPictures("No codes") == "No codes");
REQUIRE(ConvertControlCodesToPictures("Allowed codes: \t\r\n") == "Allowed codes: \t\r\n");

REQUIRE(ConvertControlCodesToPictures("\x1b Code First") == ConvertToUTF8(L"\x241b Code First"));
REQUIRE(ConvertControlCodesToPictures("Escape code: \x1b") == ConvertToUTF8(L"Escape code: \x241b"));

std::string_view allCodes{ "\x0\x1\x2\x3\x4\x5\x6\x7\x8\xb\xc\xe\xf\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x7f"sv };
std::wstring_view allPictures{ L"\x2400\x2401\x2402\x2403\x2404\x2405\x2406\x2407\x2408\x240b\x240c\x240e\x240f\x2410\x2411\x2412\x2413\x2414\x2415\x2416\x2417\x2418\x2419\x241a\x241b\x241c\x241d\x241e\x241f\x2421"sv };

REQUIRE(ConvertControlCodesToPictures(allCodes) == ConvertToUTF8(allPictures));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
key: "This is the ESCAPE control code: \x1b"
Loading

0 comments on commit 457f84b

Please sign in to comment.