Skip to content

Commit

Permalink
Support environment variables in the settings (#15082)
Browse files Browse the repository at this point in the history
Existing environment variables can be referenced by enclosing the name
in percent characters (e.g. `%PATH%`).

Resurrects #9287 by @christapley.

Tests added and manually tested.

Closes #2785
Closes #9233

Co-authored-by: Chris Tapley <chris.tapley.81@gmail.com>
  • Loading branch information
ianjoneill and christapley committed Apr 11, 2023
1 parent 508adbb commit 56d451d
Show file tree
Hide file tree
Showing 25 changed files with 322 additions and 262 deletions.
7 changes: 7 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2574,6 +2574,13 @@
"default": false,
"description": "When true, this profile should always open in an elevated context. If the window isn't running as an Administrator, then a new elevated window will be created."
},
"environment": {
"description": "Key-value pairs representing environment variables to set. Environment variable names are not case sensitive. You can reference existing environment variable names by enclosing them in literal percent characters (e.g. %PATH%).",
"type": "object",
"additionalProperties": {
"type": "string"
}
},
"experimental.autoMarkPrompts": {
"default": false,
"description": "When set to true, prompts will automatically be marked.",
Expand Down
47 changes: 47 additions & 0 deletions src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ namespace SettingsModelLocalTests
TEST_METHOD(LayerProfileProperties);
TEST_METHOD(LayerProfileIcon);
TEST_METHOD(LayerProfilesOnArray);
TEST_METHOD(ProfileWithEnvVars);
TEST_METHOD(ProfileWithEnvVarsSameNameDifferentCases);

TEST_METHOD(DuplicateProfileTest);
TEST_METHOD(TestGenGuidsForProfiles);
TEST_METHOD(TestCorrectOldDefaultShellPaths);
Expand Down Expand Up @@ -349,6 +352,50 @@ namespace SettingsModelLocalTests
VERIFY_ARE_NOT_EQUAL(settings->AllProfiles().GetAt(0).Guid(), settings->AllProfiles().GetAt(1).Guid());
}

void ProfileTests::ProfileWithEnvVars()
{
const std::string profileString{ R"({
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"environment": {
"VAR_1": "value1",
"VAR_2": "value2",
"VAR_3": "%VAR_3%;value3"
}
})" };
const auto profile = implementation::Profile::FromJson(VerifyParseSucceeded(profileString));
std::vector<IEnvironmentVariableMap> envVarMaps{};
envVarMaps.emplace_back(profile->EnvironmentVariables());
for (auto& envMap : envVarMaps)
{
VERIFY_ARE_EQUAL(static_cast<uint32_t>(3), envMap.Size());
VERIFY_ARE_EQUAL(L"value1", envMap.Lookup(L"VAR_1"));
VERIFY_ARE_EQUAL(L"value2", envMap.Lookup(L"VAR_2"));
VERIFY_ARE_EQUAL(L"%VAR_3%;value3", envMap.Lookup(L"VAR_3"));
}
}

void ProfileTests::ProfileWithEnvVarsSameNameDifferentCases()
{
const std::string userSettings{ R"({
"profiles": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"environment": {
"FOO": "VALUE",
"Foo": "Value"
}
}
]
})" };
const auto settings = winrt::make_self<implementation::CascadiaSettings>(userSettings);
const auto warnings = settings->Warnings();
VERIFY_ARE_EQUAL(static_cast<uint32_t>(2), warnings.Size());
uint32_t index;
VERIFY_IS_TRUE(warnings.IndexOf(SettingsLoadWarnings::InvalidProfileEnvironmentVariables, index));
}

void ProfileTests::TestCorrectOldDefaultShellPaths()
{
static constexpr std::string_view inboxProfiles{ R"({
Expand Down
8 changes: 7 additions & 1 deletion src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,13 @@ namespace SettingsModelLocalTests
"historySize": 9001,
"closeOnExit": "graceful",
"experimental.retroTerminalEffect": false
"experimental.retroTerminalEffect": false,
"environment":
{
"KEY_1": "VALUE_1",
"KEY_2": "%KEY_1%",
"KEY_3": "%PATH%"
}
})" };

static constexpr std::string_view smallProfileString{ R"(
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@
<value>Failed to parse "startupActions".</value>
<comment>{Locked="\"startupActions\""}</comment>
</data>
<data name="InvalidProfileEnvironmentVariables" xml:space="preserve">
<value>Found multiple environment variables with the same name in different cases (lower/upper) - only one value will be used.</value>
</data>
<data name="CmdCommandArgDesc" xml:space="preserve">
<value>An optional command, with arguments, to be spawned in the new tab or pane</value>
</data>
Expand Down
17 changes: 8 additions & 9 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,8 @@ namespace winrt::TerminalApp::implementation
nullptr,
settings.InitialRows(),
settings.InitialCols(),
winrt::guid());
winrt::guid(),
profile.Guid());

if constexpr (Feature_VtPassthroughMode::IsEnabled())
{
Expand All @@ -1228,12 +1229,9 @@ namespace winrt::TerminalApp::implementation

else
{
// profile is guaranteed to exist here
auto guidWString = Utils::GuidToString(profile.Guid());

StringMap envMap{};
envMap.Insert(L"WT_PROFILE_ID", guidWString);
envMap.Insert(L"WSLENV", L"WT_PROFILE_ID");
const auto environment = settings.EnvironmentVariables() != nullptr ?
settings.EnvironmentVariables().GetView() :
nullptr;

// Update the path to be relative to whatever our CWD is.
//
Expand Down Expand Up @@ -1264,10 +1262,11 @@ namespace winrt::TerminalApp::implementation
auto valueSet = TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
newWorkingDirectory,
settings.StartingTitle(),
envMap.GetView(),
environment,
settings.InitialRows(),
settings.InitialCols(),
winrt::guid());
winrt::guid(),
profile.Guid());

valueSet.Insert(L"passthroughMode", Windows::Foundation::PropertyValue::CreateBoolean(settings.VtPassthrough()));
valueSet.Insert(L"reloadEnvironmentVariables",
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ static const std::array settingsLoadWarningsLabels{
USES_RESOURCE(L"InvalidColorSchemeInCmd"),
USES_RESOURCE(L"InvalidSplitSize"),
USES_RESOURCE(L"FailedToParseStartupActions"),
USES_RESOURCE(L"InvalidProfileEnvironmentVariables"),
USES_RESOURCE(L"FailedToParseSubCommands"),
USES_RESOURCE(L"UnknownTheme"),
USES_RESOURCE(L"DuplicateRemainingProfilesEntry"),
Expand Down
71 changes: 34 additions & 37 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
#include "ConptyConnection.h"

#include <conpty-static.h>
#include <til/string.h>
#include <til/env.h>
#include <winternl.h>

#include "CTerminalHandoff.h"
#include "LibraryResources.h"
#include "../../types/inc/Environment.hpp"
#include "../../types/inc/utils.hpp"

#include "ConptyConnection.g.cpp"
Expand Down Expand Up @@ -84,31 +84,19 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation

auto cmdline{ wil::ExpandEnvironmentStringsW<std::wstring>(_commandline.c_str()) }; // mutable copy -- required for CreateProcessW

Utils::EnvironmentVariableMapW environment;
til::env environment;
auto zeroEnvMap = wil::scope_exit([&]() noexcept {
// Can't zero the keys, but at least we can zero the values.
for (auto& [name, value] : environment)
{
::SecureZeroMemory(value.data(), value.size() * sizeof(decltype(value.begin())::value_type));
}

environment.clear();
});

// Populate the environment map with the current environment.
if (_reloadEnvironmentVariables)
{
til::env refreshedEnvironment;
refreshedEnvironment.regenerate();

for (auto& [key, value] : refreshedEnvironment.as_map())
{
environment.try_emplace(key, std::move(value));
}
environment.regenerate();
}
else
{
RETURN_IF_FAILED(Utils::UpdateEnvironmentMapW(environment));
environment = til::env::from_current_environment();
}

{
Expand All @@ -119,39 +107,45 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
const auto guidSubStr = std::wstring_view{ wsGuid }.substr(1);

// Ensure every connection has the unique identifier in the environment.
environment.insert_or_assign(L"WT_SESSION", guidSubStr.data());
environment.as_map().insert_or_assign(L"WT_SESSION", guidSubStr.data());

// The profile Guid does include the enclosing '{}'
const auto profileGuid{ Utils::GuidToString(_profileGuid) };
environment.as_map().insert_or_assign(L"WT_PROFILE_ID", profileGuid.data());

// WSLENV is a colon-delimited list of environment variables (+flags) that should appear inside WSL
// https://devblogs.microsoft.com/commandline/share-environment-vars-between-wsl-and-windows/
std::wstring wslEnv{ L"WT_SESSION:WT_PROFILE_ID:" };
if (_environment)
{
// add additional WT env vars like WT_SETTINGS, WT_DEFAULTS and WT_PROFILE_ID
for (auto item : _environment)
// Order the environment variable names so that resolution order is consistent
std::set<std::wstring, til::wstring_case_insensitive_compare> keys{};
for (const auto item : _environment)
{
keys.insert(item.Key().c_str());
}
// add additional env vars
for (const auto& key : keys)
{
try
{
auto key = item.Key();
// This will throw if the value isn't a string. If that
// happens, then just skip this entry.
auto value = winrt::unbox_value<hstring>(item.Value());

// avoid clobbering WSLENV
if (std::wstring_view{ key } == L"WSLENV")
{
auto current = environment[L"WSLENV"];
value = current + L":" + value;
}
const auto value = winrt::unbox_value<hstring>(_environment.Lookup(key));

environment.insert_or_assign(key.c_str(), value.c_str());
environment.set_user_environment_var(key.c_str(), value.c_str());
// For each environment variable added to the environment, also add it to WSLENV
wslEnv += key + L":";
}
CATCH_LOG();
}
}

// WSLENV is a colon-delimited list of environment variables (+flags) that should appear inside WSL
// https://devblogs.microsoft.com/commandline/share-environment-vars-between-wsl-and-windows/

auto wslEnv = environment[L"WSLENV"];
wslEnv = L"WT_SESSION:" + wslEnv; // prepend WT_SESSION to make sure it's visible inside WSL.
environment.insert_or_assign(L"WSLENV", wslEnv);
// We want to prepend new environment variables to WSLENV - that way if a variable already
// exists in WSLENV but with a flag, the flag will be respected.
// (This behaviour was empirically observed)
wslEnv += environment.as_map()[L"WSLENV"];
environment.as_map().insert_or_assign(L"WSLENV", wslEnv);
}

std::vector<wchar_t> newEnvVars;
Expand All @@ -160,7 +154,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
newEnvVars.size() * sizeof(decltype(newEnvVars.begin())::value_type));
});

RETURN_IF_FAILED(Utils::EnvironmentMapToEnvironmentStringsW(environment, newEnvVars));
RETURN_IF_FAILED(environment.to_environment_strings_w(newEnvVars));

auto lpEnvironment = newEnvVars.empty() ? nullptr : newEnvVars.data();

Expand Down Expand Up @@ -244,7 +238,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
const Windows::Foundation::Collections::IMapView<hstring, hstring>& environment,
uint32_t rows,
uint32_t columns,
const winrt::guid& guid)
const winrt::guid& guid,
const winrt::guid& profileGuid)
{
Windows::Foundation::Collections::ValueSet vs{};

Expand All @@ -254,6 +249,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
vs.Insert(L"initialRows", Windows::Foundation::PropertyValue::CreateUInt32(rows));
vs.Insert(L"initialCols", Windows::Foundation::PropertyValue::CreateUInt32(columns));
vs.Insert(L"guid", Windows::Foundation::PropertyValue::CreateGuid(guid));
vs.Insert(L"profileGuid", Windows::Foundation::PropertyValue::CreateGuid(profileGuid));

if (environment)
{
Expand Down Expand Up @@ -288,6 +284,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
}
_reloadEnvironmentVariables = winrt::unbox_value_or<bool>(settings.TryLookup(L"reloadEnvironmentVariables").try_as<Windows::Foundation::IPropertyValue>(),
_reloadEnvironmentVariables);
_profileGuid = winrt::unbox_value_or<winrt::guid>(settings.TryLookup(L"profileGuid").try_as<Windows::Foundation::IPropertyValue>(), _profileGuid);
}

if (_guid == guid{})
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalConnection/ConptyConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
const Windows::Foundation::Collections::IMapView<hstring, hstring>& environment,
uint32_t rows,
uint32_t columns,
const winrt::guid& guid);
const winrt::guid& guid,
const winrt::guid& profileGuid);

WINRT_CALLBACK(TerminalOutput, TerminalOutputHandler);

Expand Down Expand Up @@ -90,6 +91,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
std::array<char, 4096> _buffer{};
bool _passthroughMode{};
bool _reloadEnvironmentVariables{};
guid _profileGuid{};

struct StartupInfoFromDefTerm
{
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalConnection/ConptyConnection.idl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace Microsoft.Terminal.TerminalConnection
IMapView<String, String> environment,
UInt32 rows,
UInt32 columns,
Guid guid);
Guid guid,
Guid profileGuid);
};
}
1 change: 0 additions & 1 deletion src/cascadia/TerminalControl/IControlSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ namespace Microsoft.Terminal.Control

String Commandline { get; };
String StartingDirectory { get; };
String EnvironmentVariables { get; };

TextAntialiasingMode AntialiasingMode { get; };

Expand Down
26 changes: 26 additions & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <shellapi.h>
#include <shlwapi.h>
#include <til/latch.h>
#include <til/string.h>

using namespace winrt::Microsoft::Terminal;
using namespace winrt::Microsoft::Terminal::Settings;
Expand Down Expand Up @@ -417,6 +418,7 @@ void CascadiaSettings::_validateSettings()
_validateKeybindings();
_validateColorSchemesInCommands();
_validateThemeExists();
_validateProfileEnvironmentVariables();
}

// Method Description:
Expand Down Expand Up @@ -541,6 +543,30 @@ void CascadiaSettings::_validateMediaResources()
}
}

// Method Description:
// - Checks if the profiles contain multiple environment variables with the same name, but different
// cases
void CascadiaSettings::_validateProfileEnvironmentVariables()
{
for (const auto& profile : _allProfiles)
{
std::set<std::wstring, til::wstring_case_insensitive_compare> envVarNames{};
if (profile.EnvironmentVariables() == nullptr)
{
continue;
}
for (const auto [key, value] : profile.EnvironmentVariables())
{
const auto iterator = envVarNames.insert(key.c_str());
if (!iterator.second)
{
_warnings.Append(SettingsLoadWarnings::InvalidProfileEnvironmentVariables);
return;
}
}
}
}

// Method Description:
// - Helper to get the GUID of a profile, given an optional index and a possible
// "profile" value to override that.
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
void _validateSettings();
void _validateAllSchemesExist();
void _validateMediaResources();
void _validateProfileEnvironmentVariables();
void _validateKeybindings() const;
void _validateColorSchemesInCommands() const;
bool _hasInvalidColorScheme(const Model::Command& command) const;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/MTSMSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ Author(s):
X(CloseOnExitMode, CloseOnExit, "closeOnExit", CloseOnExitMode::Automatic) \
X(hstring, TabTitle, "tabTitle") \
X(Model::BellStyle, BellStyle, "bellStyle", BellStyle::Audible) \
X(IEnvironmentVariableMap, EnvironmentVariables, "environment", nullptr) \
X(bool, UseAtlasEngine, "useAtlasEngine", Feature_AtlasEngine::IsEnabled()) \
X(bool, RightClickContextMenu, "experimental.rightClickContextMenu", false) \
X(Windows::Foundation::Collections::IVector<winrt::hstring>, BellSound, "bellSound", nullptr) \
Expand Down
Loading

0 comments on commit 56d451d

Please sign in to comment.