Skip to content

Commit

Permalink
Add settings to warn about large or multiline pastes (#6631)
Browse files Browse the repository at this point in the history
Before sending calling the `HandleClipboardData` member function on
the `PasteFromClipboardEventArgs` object when we receive a request
from the `TermControl` to send it the clipboard's text content, we
now display a warning to let the user choose whether to continue or
not if the text is larger than 5 KiB or contains the _new line_
character, which can be a security issue if the user is pasting the
text in a shell.

These warnings can be disabled with the `largePasteWarning` and
`multiLinePasteWarning` global settings respectively.

Closes #2349
  • Loading branch information
beviu authored Jul 1, 2020
1 parent 6b43ace commit 985f85d
Show file tree
Hide file tree
Showing 13 changed files with 186 additions and 35 deletions.
1 change: 1 addition & 0 deletions .github/actions/spell-check/dictionary/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ IBind
IClass
IComparable
ICustom
IDialog
IDirect
IExplorer
IMap
Expand Down
2 changes: 2 additions & 0 deletions doc/cascadia/SettingsSchema.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Properties listed below affect the entire window, regardless of the profile sett
| `alwaysShowTabs` | _Required_ | Boolean | `true` | When set to `true`, tabs are always displayed. When set to `false` and `showTabsInTitlebar` is set to `false`, tabs only appear after typing <kbd>Ctrl</kbd> + <kbd>T</kbd>. |
| `copyOnSelect` | Optional | Boolean | `false` | When set to `true`, a selection is immediately copied to your clipboard upon creation. When set to `false`, the selection persists and awaits further action. |
| `copyFormatting` | Optional | Boolean | `false` | When set to `true`, the color and font formatting of selected text is also copied to your clipboard. When set to `false`, only plain text is copied to your clipboard. |
| `largePasteWarning` | Optional | Boolean | `true` | When set to `true`, trying to paste text with more than 5 KiB of characters will display a warning asking you whether to continue or not with the paste. |
| `multiLinePasteWarning` | Optional | Boolean | `true` | When set to `true`, trying to paste text with a _new line_ character will display a warning asking you whether to continue or not with the paste. |
| `defaultProfile` | _Required_ | String | PowerShell guid | Sets the default profile. Opens by typing <kbd>Ctrl</kbd> + <kbd>T</kbd> or by clicking the '+' icon. The guid of the desired default profile is used as the value. |
| `initialCols` | _Required_ | Integer | `120` | The number of columns displayed in the window upon first load. |
| `initialPosition` | Optional | String | `","` | The position of the top left corner of the window upon first load. On a system with multiple displays, these coordinates are relative to the top left of the primary display. If `launchMode` is set to `"maximized"`, the window will be maximized on the monitor specified by those coordinates. |
Expand Down
10 changes: 10 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,16 @@
"description": "When set to `true`, the color and font formatting of selected text is also copied to your clipboard. When set to `false`, only plain text is copied to your clipboard.",
"type": "boolean"
},
"largePasteWarning": {
"default": true,
"description": "When set to true, trying to paste text with more than 5 KiB of characters will display a warning asking you whether to continue or not with the paste.",
"type": "boolean"
},
"multiLinePasteWarning": {
"default": true,
"description": "When set to true, trying to paste text with a \"new line\" character will display a warning asking you whether to continue or not with the paste.",
"type": "boolean"
},
"defaultProfile": {
"description": "Sets the default profile. Opens by clicking the \"+\" icon or typing the key binding assigned to \"newTab\".",
"type": "string"
Expand Down
22 changes: 12 additions & 10 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using namespace winrt::Windows::ApplicationModel;
using namespace winrt::Windows::ApplicationModel::DataTransfer;
using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Xaml::Controls;
using namespace winrt::Windows::UI::Core;
using namespace winrt::Windows::System;
using namespace winrt::Microsoft::Terminal;
Expand Down Expand Up @@ -231,7 +232,7 @@ namespace winrt::TerminalApp::implementation
// this as a MTA, before the app is Create()'d
WINRT_ASSERT(_loadedInitialSettings);

_root->ShowDialog({ this, &AppLogic::_ShowDialog });
_root->DialogPresenter(*this);

// In UWP mode, we cannot handle taking over the title bar for tabs,
// so this setting is overridden to false no matter what the preference is.
Expand Down Expand Up @@ -277,17 +278,18 @@ namespace winrt::TerminalApp::implementation
// - Only one dialog can be visible at a time. If another dialog is visible
// when this is called, nothing happens.
// Arguments:
// sender: unused
// dialog: the dialog object that is going to show up
fire_and_forget AppLogic::_ShowDialog(const winrt::Windows::Foundation::IInspectable& sender, winrt::Windows::UI::Xaml::Controls::ContentDialog dialog)
// - dialog: the dialog object that is going to show up
// Return value:
// - an IAsyncOperation with the dialog result
winrt::Windows::Foundation::IAsyncOperation<ContentDialogResult> AppLogic::ShowDialog(winrt::Windows::UI::Xaml::Controls::ContentDialog dialog)
{
// DON'T release this lock in a wil::scope_exit. The scope_exit will get
// called when we await, which is not what we want.
std::unique_lock lock{ _dialogLock, std::try_to_lock };
if (!lock)
{
// Another dialog is visible.
return;
co_return ContentDialogResult::None;
}

// IMPORTANT: This is necessary as documented in the ContentDialog MSDN docs.
Expand Down Expand Up @@ -321,7 +323,7 @@ namespace winrt::TerminalApp::implementation
auto loadedRevoker{ dialog.Loaded(winrt::auto_revoke, themingLambda) }; // if it's not yet in the tree

// Display the dialog.
co_await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup);
co_return co_await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup);

// After the dialog is dismissed, the dialog lock (held by `lock`) will
// be released so another can be shown
Expand All @@ -333,7 +335,7 @@ namespace winrt::TerminalApp::implementation
// as the title and first content of the dialog, then also displays a
// message for whatever exception was found while validating the settings.
// - Only one dialog can be visible at a time. If another dialog is visible
// when this is called, nothing happens. See _ShowDialog for details
// when this is called, nothing happens. See ShowDialog for details
// Arguments:
// - titleKey: The key to use to lookup the title text from our resources.
// - contentKey: The key to use to lookup the content text from our resources.
Expand Down Expand Up @@ -378,15 +380,15 @@ namespace winrt::TerminalApp::implementation
dialog.CloseButtonText(buttonText);
dialog.DefaultButton(Controls::ContentDialogButton::Close);

_ShowDialog(nullptr, dialog);
ShowDialog(dialog);
}

// Method Description:
// - Displays a dialog for warnings found while loading or validating the
// settings. Displays messages for whatever warnings were found while
// validating the settings.
// - Only one dialog can be visible at a time. If another dialog is visible
// when this is called, nothing happens. See _ShowDialog for details
// when this is called, nothing happens. See ShowDialog for details
void AppLogic::_ShowLoadWarningsDialog()
{
auto title = RS_(L"SettingsValidateErrorTitle");
Expand Down Expand Up @@ -437,7 +439,7 @@ namespace winrt::TerminalApp::implementation
dialog.CloseButtonText(buttonText);
dialog.DefaultButton(Controls::ContentDialogButton::Close);

_ShowDialog(nullptr, dialog);
ShowDialog(dialog);
}

// Method Description:
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ namespace winrt::TerminalApp::implementation

void WindowCloseButtonClicked();

winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> ShowDialog(winrt::Windows::UI::Xaml::Controls::ContentDialog dialog);

// -------------------------------- WinRT Events ---------------------------------
DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(RequestedThemeChanged, _requestedThemeChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::ElementTheme);

Expand Down Expand Up @@ -79,7 +81,6 @@ namespace winrt::TerminalApp::implementation
::TerminalApp::AppCommandlineArgs _appArgs;
int _ParseArgs(winrt::array_view<const hstring>& args);

fire_and_forget _ShowDialog(const winrt::Windows::Foundation::IInspectable& sender, winrt::Windows::UI::Xaml::Controls::ContentDialog dialog);
void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey, HRESULT settingsLoadedResult);
void _ShowLoadWarningsDialog();

Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace TerminalApp
FullscreenMode,
};

[default_interface] runtimeclass AppLogic : IDirectKeyListener
[default_interface] runtimeclass AppLogic : IDirectKeyListener, IDialogPresenter
{
AppLogic();

Expand Down Expand Up @@ -50,6 +50,10 @@ namespace TerminalApp
void TitlebarClicked();
void WindowCloseButtonClicked();

// See IDialogPresenter and TerminalPage's DialogPresenter for more
// information.
Windows.Foundation.IAsyncOperation<Windows.UI.Xaml.Controls.ContentDialogResult> ShowDialog(Windows.UI.Xaml.Controls.ContentDialog dialog);

event Windows.Foundation.TypedEventHandler<Object, Windows.UI.Xaml.UIElement> SetTitleBarContent;
event Windows.Foundation.TypedEventHandler<Object, String> TitleChanged;
event Windows.Foundation.TypedEventHandler<Object, LastTabClosedEventArgs> LastTabClosed;
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalApp/GlobalAppSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ static constexpr std::string_view ShowTabsInTitlebarKey{ "showTabsInTitlebar" };
static constexpr std::string_view WordDelimitersKey{ "wordDelimiters" };
static constexpr std::string_view CopyOnSelectKey{ "copyOnSelect" };
static constexpr std::string_view CopyFormattingKey{ "copyFormatting" };
static constexpr std::string_view WarnAboutLargePasteKey{ "largePasteWarning" };
static constexpr std::string_view WarnAboutMultiLinePasteKey{ "multiLinePasteWarning" };
static constexpr std::string_view LaunchModeKey{ "launchMode" };
static constexpr std::string_view ConfirmCloseAllKey{ "confirmCloseAllTabs" };
static constexpr std::string_view SnapToGridOnResizeKey{ "snapToGridOnResize" };
Expand Down Expand Up @@ -190,6 +192,10 @@ void GlobalAppSettings::LayerJson(const Json::Value& json)

JsonUtils::GetBool(json, CopyFormattingKey, _CopyFormatting);

JsonUtils::GetBool(json, WarnAboutLargePasteKey, _WarnAboutLargePaste);

JsonUtils::GetBool(json, WarnAboutMultiLinePasteKey, _WarnAboutMultiLinePaste);

if (auto launchMode{ json[JsonKey(LaunchModeKey)] })
{
_LaunchMode = _ParseLaunchMode(GetWstringFromJson(launchMode));
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/GlobalAppSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ class TerminalApp::GlobalAppSettings final
GETSET_PROPERTY(std::wstring, WordDelimiters); // default value set in constructor
GETSET_PROPERTY(bool, CopyOnSelect, false);
GETSET_PROPERTY(bool, CopyFormatting, false);
GETSET_PROPERTY(bool, WarnAboutLargePaste, true);
GETSET_PROPERTY(bool, WarnAboutMultiLinePaste, true);
GETSET_PROPERTY(LaunchPosition, InitialPosition);
GETSET_PROPERTY(winrt::TerminalApp::LaunchMode, LaunchMode, winrt::TerminalApp::LaunchMode::DefaultMode);
GETSET_PROPERTY(bool, SnapToGridOnResize, true);
Expand Down
24 changes: 24 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,30 @@
<value>Command Prompt</value>
<comment>This is the name of "Command Prompt", as localized in Windows. The localization here should match the one in the Windows product for "Command Prompt"</comment>
</data>
<data name="LargePasteDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="LargePasteDialog.Content" xml:space="preserve">
<value>You are about to paste text that is longer than 5 KiB. Do you wish to continue?</value>
</data>
<data name="LargePasteDialog.PrimaryButtonText" xml:space="preserve">
<value>Paste anyway</value>
</data>
<data name="LargePasteDialog.Title" xml:space="preserve">
<value>Warning</value>
</data>
<data name="MultiLinePasteDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="MultiLinePasteDialog.Content" xml:space="preserve">
<value>You are about to paste text that contains multiple lines. If you paste this text into your shell, it may result in the unexpected execution of commands. Do you wish to continue?</value>
</data>
<data name="MultiLinePasteDialog.PrimaryButtonText" xml:space="preserve">
<value>Paste anyway</value>
</data>
<data name="MultiLinePasteDialog.Title" xml:space="preserve">
<value>Warning</value>
</data>
<data name="CommandPalette_SearchBox.PlaceholderText" xml:space="preserve">
<value>Type a command name...</value>
</data>
Expand Down
Loading

0 comments on commit 985f85d

Please sign in to comment.