From 2c666aa292f78befa2db4013860092c27c866ccb Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 21 Aug 2025 17:05:27 -0700 Subject: [PATCH] Clear Name, Source, and Commandline from Profiles.Defaults (#19225) The Name, Source, and Commandline profile settings should not be allowed to be set on the Profiles.Defaults object. This just enforces that by clearing them (as is done with Guid). These profile settings are omitted from the settings UI's profile defaults page. Closes #19202 --- .../CascadiaSettingsSerialization.cpp | 4 ++ .../DeserializationTests.cpp | 27 +++++--- .../MediaResourceTests.cpp | 34 +++++----- .../UnitTests_SettingsModel/ProfileTests.cpp | 62 +++++++++++++++++++ 4 files changed, 98 insertions(+), 29 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index 7b2a7f17c9..7d369a9828 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -839,7 +839,11 @@ void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source settings.baseLayerProfile = Profile::FromJson(json.profileDefaults); // Remove the `guid` member from the default settings. // That will hyper-explode, so just don't let them do that. + // Also remove name, source, and commandline; those are not valid for the profiles defaults object. settings.baseLayerProfile->ClearGuid(); + settings.baseLayerProfile->ClearName(); + settings.baseLayerProfile->ClearSource(); + settings.baseLayerProfile->ClearCommandline(); settings.baseLayerProfile->Origin(OriginTag::ProfilesDefaults); } diff --git a/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp b/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp index 72c6b80256..c253a17493 100644 --- a/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/DeserializationTests.cpp @@ -1788,16 +1788,18 @@ namespace SettingsModelUnitTests "profiles": { "defaults": { - "name": "PROFILE DEFAULTS" + "tabTitle": "PROFILE DEFAULTS TAB TITLE" }, "list": [ { "guid": "{61c54bbd-1111-5271-96e7-009a87ff44bf}", - "name": "CMD" + "name": "CMD", + "tabTitle": "CMD Tab Title" }, { "guid": "{61c54bbd-2222-5271-96e7-009a87ff44bf}", - "name": "PowerShell" + "name": "PowerShell", + "tabTitle": "PowerShell Tab Title" }, { "guid": "{61c54bbd-3333-5271-96e7-009a87ff44bf}" @@ -1816,25 +1818,30 @@ namespace SettingsModelUnitTests // test profiles VERIFY_ARE_EQUAL(settings->AllProfiles().Size(), copyImpl->AllProfiles().Size()); VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(0).Name(), copyImpl->AllProfiles().GetAt(0).Name()); + VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(0).TabTitle(), copyImpl->AllProfiles().GetAt(0).TabTitle()); VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(1).Name(), copyImpl->AllProfiles().GetAt(1).Name()); + VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(1).TabTitle(), copyImpl->AllProfiles().GetAt(1).TabTitle()); VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(2).Name(), copyImpl->AllProfiles().GetAt(2).Name()); - VERIFY_ARE_EQUAL(settings->ProfileDefaults().Name(), copyImpl->ProfileDefaults().Name()); + VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(2).TabTitle(), copyImpl->AllProfiles().GetAt(2).TabTitle()); + VERIFY_ARE_EQUAL(settings->ProfileDefaults().TabTitle(), copyImpl->ProfileDefaults().TabTitle()); // Modifying profile.defaults should... - VERIFY_ARE_EQUAL(settings->ProfileDefaults().HasName(), copyImpl->ProfileDefaults().HasName()); - copyImpl->ProfileDefaults().Name(L"changed value"); + VERIFY_ARE_EQUAL(settings->ProfileDefaults().HasTabTitle(), copyImpl->ProfileDefaults().HasTabTitle()); + copyImpl->ProfileDefaults().TabTitle(L"changed value"); - // ...keep the same name for the first two profiles + // ...keep the same name and tab title for the first two profiles VERIFY_ARE_EQUAL(settings->AllProfiles().Size(), copyImpl->AllProfiles().Size()); VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(0).Name(), copyImpl->AllProfiles().GetAt(0).Name()); + VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(0).TabTitle(), copyImpl->AllProfiles().GetAt(0).TabTitle()); VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(1).Name(), copyImpl->AllProfiles().GetAt(1).Name()); + VERIFY_ARE_EQUAL(settings->AllProfiles().GetAt(1).TabTitle(), copyImpl->AllProfiles().GetAt(1).TabTitle()); // ...but change the name for the one that inherited it from profile.defaults - VERIFY_ARE_NOT_EQUAL(settings->AllProfiles().GetAt(2).Name(), copyImpl->AllProfiles().GetAt(2).Name()); + VERIFY_ARE_NOT_EQUAL(settings->AllProfiles().GetAt(2).TabTitle(), copyImpl->AllProfiles().GetAt(2).TabTitle()); // profile.defaults should be different between the two graphs - VERIFY_ARE_EQUAL(settings->ProfileDefaults().HasName(), copyImpl->ProfileDefaults().HasName()); - VERIFY_ARE_NOT_EQUAL(settings->ProfileDefaults().Name(), copyImpl->ProfileDefaults().Name()); + VERIFY_ARE_EQUAL(settings->ProfileDefaults().HasTabTitle(), copyImpl->ProfileDefaults().HasTabTitle()); + VERIFY_ARE_NOT_EQUAL(settings->ProfileDefaults().TabTitle(), copyImpl->ProfileDefaults().TabTitle()); Log::Comment(L"Test empty profiles.defaults"); static constexpr std::string_view emptyPDJson{ R"( diff --git a/src/cascadia/UnitTests_SettingsModel/MediaResourceTests.cpp b/src/cascadia/UnitTests_SettingsModel/MediaResourceTests.cpp index 16165dad8e..d10971906e 100644 --- a/src/cascadia/UnitTests_SettingsModel/MediaResourceTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/MediaResourceTests.cpp @@ -103,8 +103,9 @@ namespace SettingsModelUnitTests TEST_METHOD(RealResolverSpecialKeywords); TEST_METHOD(RealResolverUrlCases); - static constexpr std::wstring_view defaultsCommandline{ LR"(C:\Windows\System32\PING.EXE)" }; // Normalized by Profile (this is the casing that Windows stores on disk) + static constexpr std::wstring_view pingCommandline{ LR"(C:\Windows\System32\PING.EXE)" }; // Normalized by Profile (this is the casing that Windows stores on disk) static constexpr std::wstring_view overrideCommandline{ LR"(C:\Windows\System32\cscript.exe)" }; + static constexpr std::wstring_view cmdCommandline{ LR"(C:\Windows\System32\cmd.exe)" }; // The default commandline for a profile static constexpr std::wstring_view fragmentBasePath1{ LR"(C:\Windows\Media)" }; private: @@ -809,7 +810,6 @@ namespace SettingsModelUnitTests "profiles": { "defaults": { "icon": "DoesNotMatter", - "commandline": "C:\\Windows\\System32\\ping.exe", } } })"); @@ -818,7 +818,7 @@ namespace SettingsModelUnitTests auto profile{ settings->GetProfileByName(L"Base") }; auto icon{ profile.Icon() }; VERIFY_IS_TRUE(icon.Ok()); // Profile with commandline always has an icon - VERIFY_ARE_EQUAL(defaultsCommandline, icon.Resolved()); + VERIFY_ARE_EQUAL(cmdCommandline, icon.Resolved()); } // The invalid resource came from the profile itself, which has its own commandline. @@ -856,7 +856,7 @@ namespace SettingsModelUnitTests VERIFY_ARE_EQUAL(overrideCommandline, icon.Resolved()); } - // The invalid resource came from the profile itself, which inherits a commandline from the parent (defaults, ping.exe) + // The invalid resource came from the profile itself, where the commandline is the default value (profile.commandline default value is CMD.exe) void MediaResourceTests::ProfileSpecifiesInvalidIconAndNoCommandline() { WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; @@ -871,8 +871,7 @@ namespace SettingsModelUnitTests "profiles": { "defaults": { "icon": "DoesNotMatter", - "commandline": "C:\\Windows\\System32\\ping.exe", - }, + }, "list": [ { "guid": "{af9dec6c-1337-4278-897d-69ca04920b27}", @@ -887,10 +886,10 @@ namespace SettingsModelUnitTests auto profile{ settings->GetProfileByName(L"ProfileSpecifiesInvalidIconAndNoCommandline") }; auto icon{ profile.Icon() }; VERIFY_IS_TRUE(icon.Ok()); - VERIFY_ARE_EQUAL(defaultsCommandline, icon.Resolved()); + VERIFY_ARE_EQUAL(cmdCommandline, icon.Resolved()); } - // The invalid resource came from the Defaults profile, which has the Defaults command line (PROFILE COMMANDLINE IGNORED) + // The invalid resource came from the Defaults profile, where the commandline falls back to the default value of CMD.exe (PROFILE COMMANDLINE IGNORED) void MediaResourceTests::ProfileInheritsInvalidIconAndHasCommandline() { WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; @@ -904,8 +903,7 @@ namespace SettingsModelUnitTests settings = createSettings(R"({ "profiles": { "defaults": { - "icon": "DoesNotMatter", - "commandline": "C:\\Windows\\System32\\ping.exe", + "icon": "DoesNotMatter" }, "list": [ { @@ -921,10 +919,10 @@ namespace SettingsModelUnitTests auto profile{ settings->GetProfileByName(L"ProfileInheritsInvalidIconAndHasCommandline") }; auto icon{ profile.Icon() }; VERIFY_IS_TRUE(icon.Ok()); - VERIFY_ARE_EQUAL(defaultsCommandline, icon.Resolved()); + VERIFY_ARE_EQUAL(cmdCommandline, icon.Resolved()); } - // The invalid resource came from the Defaults profile, which has the Defaults command line (PROFILE COMMANDLINE MISSING) + // The invalid resource came from the Defaults profile, which has the default command line of CMD.exe (PROFILE COMMANDLINE MISSING) void MediaResourceTests::ProfileInheritsInvalidIconAndHasNoCommandline() { WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; @@ -938,8 +936,7 @@ namespace SettingsModelUnitTests settings = createSettings(R"({ "profiles": { "defaults": { - "icon": "DoesNotMatter", - "commandline": "C:\\Windows\\System32\\ping.exe", + "icon": "DoesNotMatter" }, "list": [ { @@ -954,7 +951,7 @@ namespace SettingsModelUnitTests auto profile{ settings->GetProfileByName(L"ProfileInheritsInvalidIconAndHasNoCommandline") }; auto icon{ profile.Icon() }; VERIFY_IS_TRUE(icon.Ok()); - VERIFY_ARE_EQUAL(defaultsCommandline, icon.Resolved()); + VERIFY_ARE_EQUAL(cmdCommandline, icon.Resolved()); } // The invalid resource came from the profile itself, which has its own commandline. @@ -992,7 +989,7 @@ namespace SettingsModelUnitTests VERIFY_ARE_EQUAL(overrideCommandline, icon.Resolved()); } - // The invalid resource came from the profile itself, which inherits a commandline from the parent (defaults, ping.exe) + // The invalid resource came from the profile itself, where the commandline falls back to the default value of CMD.exe void MediaResourceTests::ProfileSpecifiesNullIconAndHasNoCommandline() { WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; @@ -1006,8 +1003,7 @@ namespace SettingsModelUnitTests settings = createSettings(R"({ "profiles": { "defaults": { - "icon": "DoesNotMatter", - "commandline": "C:\\Windows\\System32\\ping.exe", + "icon": "DoesNotMatter" }, "list": [ { @@ -1023,7 +1019,7 @@ namespace SettingsModelUnitTests auto profile{ settings->GetProfileByName(L"ProfileSpecifiesNullIconAndHasNoCommandline") }; auto icon{ profile.Icon() }; VERIFY_IS_TRUE(icon.Ok()); // Profile with commandline always has an icon - VERIFY_ARE_EQUAL(defaultsCommandline, icon.Resolved()); + VERIFY_ARE_EQUAL(cmdCommandline, icon.Resolved()); } #pragma endregion diff --git a/src/cascadia/UnitTests_SettingsModel/ProfileTests.cpp b/src/cascadia/UnitTests_SettingsModel/ProfileTests.cpp index 56816de6e7..7697715b8f 100644 --- a/src/cascadia/UnitTests_SettingsModel/ProfileTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/ProfileTests.cpp @@ -30,6 +30,7 @@ namespace SettingsModelUnitTests TEST_METHOD(DuplicateProfileTest); TEST_METHOD(TestGenGuidsForProfiles); TEST_METHOD(TestCorrectOldDefaultShellPaths); + TEST_METHOD(ProfileDefaultsProhibitedSettings); }; void ProfileTests::ProfileGeneratesGuid() @@ -470,4 +471,65 @@ namespace SettingsModelUnitTests VERIFY_ARE_EQUAL(L"%SystemRoot%\\System32\\cmd.exe", allProfiles.GetAt(2).Commandline()); VERIFY_ARE_EQUAL(L"cmd.exe", allProfiles.GetAt(3).Commandline()); } + + void ProfileTests::ProfileDefaultsProhibitedSettings() + { + static constexpr std::string_view userProfiles{ R"({ + "profiles": { + "defaults": + { + "guid": "{00000000-0000-0000-0000-000000000000}", + "name": "Default Profile Name", + "source": "Default Profile Source", + "commandline": "foo.exe" + }, + "list": + [ + { + "name" : "PowerShell", + "commandline": "powershell.exe", + "guid" : "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}" + }, + { + "name": "Profile with just a name" + }, + { + "guid": "{a0776706-1fa6-4439-b46c-287a65c084d5}", + } + ] + } + })" }; + + const auto settings = winrt::make_self(userProfiles); + + // Profile Defaults should not have a GUID, name, source, or commandline. + const auto profileDefaults = settings->ProfileDefaults(); + VERIFY_IS_FALSE(profileDefaults.HasGuid()); + VERIFY_IS_FALSE(profileDefaults.HasName()); + VERIFY_IS_FALSE(profileDefaults.HasSource()); + VERIFY_IS_FALSE(profileDefaults.HasCommandline()); + + const auto allProfiles = settings->AllProfiles(); + VERIFY_ARE_EQUAL(3u, allProfiles.Size()); + + // Profile settings should be set to the ones set at that layer + VERIFY_ARE_EQUAL(L"PowerShell", allProfiles.GetAt(0).Name()); + VERIFY_ARE_EQUAL(L"%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", allProfiles.GetAt(0).Commandline()); + VERIFY_ARE_EQUAL(Utils::GuidFromString(L"{61c54bbd-c2c6-5271-96e7-009a87ff44bf}"), static_cast(allProfiles.GetAt(0).Guid())); + VERIFY_IS_FALSE(allProfiles.GetAt(0).HasSource()); + + // Profile should not inherit the values attempted to be set on the Profiles Defaults layer + // This profile only has a name set + VERIFY_ARE_EQUAL(L"Profile with just a name", allProfiles.GetAt(1).Name()); + VERIFY_ARE_NOT_EQUAL(Utils::GuidFromString(L"{00000000-0000-0000-0000-000000000000}"), static_cast(allProfiles.GetAt(1).Guid())); + VERIFY_ARE_NOT_EQUAL(L"Default Profile Source", allProfiles.GetAt(1).Source()); + VERIFY_ARE_NOT_EQUAL(L"foo.exe", allProfiles.GetAt(1).Commandline()); + + // Profile should not inherit the values attempted to be set on the Profiles Defaults layer + // This profile only has a guid set + VERIFY_ARE_NOT_EQUAL(L"Default Profile Name", allProfiles.GetAt(2).Name()); + VERIFY_ARE_NOT_EQUAL(Utils::GuidFromString(L"{00000000-0000-0000-0000-000000000000}"), static_cast(allProfiles.GetAt(2).Guid())); + VERIFY_ARE_NOT_EQUAL(L"Default Profile Source", allProfiles.GetAt(2).Source()); + VERIFY_ARE_NOT_EQUAL(L"foo.exe", allProfiles.GetAt(2).Commandline()); + } }