Skip to content

Commit 4c4d717

Browse files
nick-y-snyknick-y-snyk
authored
feat(IDE-1701): settings page auth flow — bridge persist and forward apiUrl (#453)
* feat: inject __ideExecuteCommand__ bridge in settings page and bump protocol version to 25 [IDE-1701] Replace __ideLogin__/__ideLogout__ COM methods with a generic __ideExecuteCommand__ bridge that dispatches any LS command with callback support via window.__ideCallbacks__. Bump ProtocolVersion to 25. * refactor: extract ExecuteCommandBridge to shared class for webview reuse [IDE-1701] Move BuildClientScript() and DispatchAsync() into a standalone ExecuteCommandBridge class. HtmlSettingsScriptingBridge delegates dispatch and HtmlSettingsWindow uses BuildClientScript() for injection, enabling any future WebBrowser panel (e.g. tree view) to reuse the same bridge. * feat(IDE-1701): save login args from settings page and remove persist flag - Remove persist flag from OnHasAuthenticated — always saves endpoint + token - Keep Save(options, false) to avoid DidChangeConfigurationAsync loop - Keep isNewLogin guard — only triggers HandleAuthenticationSuccess on first login - Keep UpdateAuthToken(token, apiUrl) — settings page webview always updated - Add bridge persist in HtmlSettingsScriptingBridge.__ideExecuteCommand__: when snyk.login called with 3+ args, save authMethod/endpoint/ignoreUnknownCA to options properties directly (no Save() call → no SettingsChanged event → no DidChangeConfigurationAsync to LS) - Remove OnHasAuthenticated_NoPersist test; rename persist-prefixed tests - Add HtmlSettingsScriptingBridgeTest with 6 tests for login args persist * feat(IDE-1701): forward apiUrl in UpdateAuthToken for settings page webview Pass apiUrl alongside token when calling window.setAuthToken via InvokeSetAuthToken so the settings page can update both the token and apiUrl fields after auth. * fix(IDE-1701): apply auth webview bridge security patterns - Add callbackId XSS allowlist guard (regex ^(__cb_\d+)?$) in ExecuteCommandBridge.IsValidCallbackId, used in DispatchAsync and InvokeCommandCallback - Fix JS string escaping order in InvokeSetAuthToken: escape backslash before single-quote - Add volatile keyword to HtmlSettingsWindow singleton instance for cross-thread visibility - Clear stored token when auth method changes in ParseAndSaveConfigAsync to prevent stale token from one method being used with another - Add tests for all four fixes * fix: add ExecuteCommandBridge.cs to csproj compile items [IDE-1701] * fix: use ForContext(typeof) for static class logger [IDE-1701] * fix: expose LogManager.ForContext(Type) as public for static class callers [IDE-1701] * fix: add missing Language using in HtmlSettingsScriptingBridgeTest [IDE-1701] * fix(IDE-1701): inject bridge script into HTML before NavigateToString The LS HTML page checks window.__ideExecuteCommand__ during its own initialization scripts. Injecting only in LoadCompleted (after the page has already parsed and run its scripts) means the auth button is never wired up, causing it to do nothing when clicked. Now inject bridge functions directly into the HTML string before NavigateToString so they are defined before any LS page scripts run. The LoadCompleted injection is kept as a secondary safety net. * security: restrict webview executeCommand bridge to snyk.* namespace Prevents XSS-to-arbitrary-command escalation by rejecting any command not prefixed with "snyk." before it reaches the Language Server. * fix: address PR review feedback for executeCommand bridge - Extract <head> tag string to constant in InjectBridgeScriptIntoHtml - Move callbackId validation outside ThreadHelper.Run in InvokeCommandCallback - Extract JS string escaping into ExecuteCommandBridge.EscapeForJsString utility - Add _cb_8 and whitespace test cases for IsValidCallbackId - Add test for default OAuth fallback on invalid auth method string
1 parent 83032e5 commit 4c4d717

19 files changed

Lines changed: 612 additions & 124 deletions

‎Snyk.VisualStudio.Extension.2022/Language/ILanguageClientManager.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public interface ILanguageClientManager
2121
Task<SastSettings> InvokeGetSastEnabled(CancellationToken cancellationToken);
2222
Task<string> InvokeLogin(CancellationToken cancellationToken);
2323
Task<object> InvokeLogout(CancellationToken cancellationToken);
24+
Task<object> InvokeExecuteCommandAsync(string command, object[] args, CancellationToken cancellationToken);
2425
Task<object> DidChangeConfigurationAsync(CancellationToken cancellationToken);
2526
Task<string> InvokeCopyLinkAsync(CancellationToken cancellationToken);
2627
Task<string> InvokeGenerateIssueDescriptionAsync(string issueId, CancellationToken cancellationToken);

‎Snyk.VisualStudio.Extension.2022/Language/LsConstants.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
public static class LsConstants
44
{
5-
public const string ProtocolVersion = "23";
5+
public const string ProtocolVersion = "24";
66

77
// Notifications
88
public const string SnykHasAuthenticated = "$/snyk.hasAuthenticated";

‎Snyk.VisualStudio.Extension.2022/Language/SnykLanguageClient.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,16 @@ public async Task<object> InvokeLogout(CancellationToken cancellationToken)
362362
return isEnabled;
363363
}
364364

365+
public async Task<object> InvokeExecuteCommandAsync(string command, object[] args, CancellationToken cancellationToken)
366+
{
367+
var param = new LSP.ExecuteCommandParams
368+
{
369+
Command = command,
370+
Arguments = args
371+
};
372+
return await InvokeWithParametersAsync<object>(LsConstants.WorkspaceExecuteCommand, param, cancellationToken);
373+
}
374+
365375
public async Task<string> InvokeCopyLinkAsync(CancellationToken cancellationToken)
366376
{
367377
var param = new LSP.ExecuteCommandParams

‎Snyk.VisualStudio.Extension.2022/Language/SnykLanguageClientCustomTarget.cs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,20 +268,30 @@ public async Task OnHasAuthenticated(JToken arg)
268268
}
269269

270270
var token = arg["token"].ToString();
271-
272271
var apiUrl = arg["apiUrl"]?.ToString();
272+
273+
var oldToken = serviceProvider.Options.ApiToken?.ToString() ?? string.Empty;
274+
275+
// Always notify the HTML settings window so the token field updates immediately.
276+
HtmlSettingsWindow.Instance?.UpdateAuthToken(token, apiUrl);
277+
273278
if (!string.IsNullOrEmpty(apiUrl))
274279
{
275280
serviceProvider.Options.CustomEndpoint = apiUrl;
276281
}
277282

278283
serviceProvider.Options.ApiToken = new AuthenticationToken(serviceProvider.Options.AuthenticationMethod, token);
279-
serviceProvider.SnykOptionsManager.Save(serviceProvider.Options);
280284

281-
await serviceProvider.GeneralOptionsDialogPage.HandleAuthenticationSuccess(token, apiUrl);
285+
// Persist without triggering SettingsChanged → DidChangeConfigurationAsync back to the LS.
286+
serviceProvider.SnykOptionsManager.Save(serviceProvider.Options, false);
287+
288+
// Scan only when this is a new login (old token was blank).
289+
// Token refresh also has old token non-blank, so no scan.
290+
var isNewLogin = string.IsNullOrEmpty(oldToken) && !string.IsNullOrEmpty(token);
291+
if (!isNewLogin)
292+
return;
282293

283-
// Notify HTML settings window of auth token change
284-
HtmlSettingsWindow.Instance?.UpdateAuthToken(token);
294+
await serviceProvider.GeneralOptionsDialogPage.HandleAuthenticationSuccess(token, apiUrl);
285295

286296
if (!serviceProvider.Options.ApiToken.IsValid())
287297
return;

‎Snyk.VisualStudio.Extension.2022/LogManager.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public static class LogManager
4646
shared: true)
4747
.CreateLogger();
4848

49-
private static ILogger ForContext(Type type) => Logger.Value.ForContext(type)
49+
public static ILogger ForContext(Type type) => Logger.Value.ForContext(type)
5050
.ForContext("ShortSourceContext", type.Name);
5151
}
5252

‎Snyk.VisualStudio.Extension.2022/Settings/HtmlSettingsWindow.xaml.cs‎

Lines changed: 76 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace Snyk.VisualStudio.Extension.Settings
2222
public partial class HtmlSettingsWindow : DialogWindow
2323
{
2424
protected static readonly ILogger Logger = LogManager.ForContext<HtmlSettingsWindow>();
25-
private static HtmlSettingsWindow instance;
25+
private static volatile HtmlSettingsWindow instance;
2626

2727
public static HtmlSettingsWindow Instance => instance;
2828

@@ -93,6 +93,7 @@ private async Task LoadHtmlSettingsAsync()
9393
SettingsBrowser.UpdateLayout();
9494

9595
html = HtmlResourceLoader.ApplyTheme(html);
96+
html = InjectBridgeScriptIntoHtml(html);
9697
SettingsBrowser.NavigateToString(html);
9798
}
9899
catch (Exception ex)
@@ -113,7 +114,8 @@ protected virtual void SetupScriptingBridge()
113114
scriptingBridge = new HtmlSettingsScriptingBridge(
114115
serviceProvider,
115116
onModified: () => IsDirty = true,
116-
onReset: () => IsDirty = false);
117+
onReset: () => IsDirty = false,
118+
onCommandResult: (callbackId, resultJson) => InvokeCommandCallback(callbackId, resultJson));
117119

118120
// Set the scripting bridge as the ObjectForScripting
119121
SettingsBrowser.ObjectForScripting = scriptingBridge;
@@ -169,8 +171,36 @@ protected virtual async Task<string> GetHtmlContentAsync()
169171
return HtmlResourceLoader.LoadFallbackHtml(serviceProvider.Options);
170172
}
171173

174+
private static string BuildIdeBridgeScript() =>
175+
@"window.__saveIdeConfig__ = function(jsonString) { window.external.__saveIdeConfig__(jsonString); };
176+
window.__onFormDirtyChange__ = function(isDirty) { window.external.__onFormDirtyChange__(isDirty); };
177+
window.__ideSaveAttemptFinished__ = function(status) { window.external.__ideSaveAttemptFinished__(status); };"
178+
+ ExecuteCommandBridge.BuildClientScript();
179+
180+
/// <summary>
181+
/// Injects IDE bridge functions into the HTML string before it is loaded by the browser.
182+
/// This ensures the functions are defined before the page's own scripts run, which is
183+
/// required for the LS HTML page to wire up its auth and command buttons correctly.
184+
/// </summary>
185+
protected virtual string InjectBridgeScriptIntoHtml(string html)
186+
{
187+
var script = BuildIdeBridgeScript();
188+
var scriptTag = $"<script type=\"text/javascript\">{script}</script>";
189+
190+
const string headTag = "<head>";
191+
var headIndex = html.IndexOf(headTag, StringComparison.OrdinalIgnoreCase);
192+
if (headIndex >= 0)
193+
{
194+
return html.Insert(headIndex + headTag.Length, scriptTag);
195+
}
196+
197+
// Fallback: prepend if no <head> found
198+
return scriptTag + html;
199+
}
200+
172201
/// <summary>
173-
/// Injects IDE bridge functions into the HTML window object for save/login/logout operations.
202+
/// Injects IDE bridge functions into the HTML window object for save and command operations.
203+
/// Called after LoadCompleted as a secondary injection to ensure functions are (re-)applied.
174204
/// </summary>
175205
protected virtual void InjectIdeBridgeFunctions()
176206
{
@@ -179,28 +209,9 @@ protected virtual void InjectIdeBridgeFunctions()
179209
dynamic doc = SettingsBrowser.Document;
180210
if (doc == null) return;
181211

182-
// Inject bridge functions that LS HTML expects
183-
var script = @"
184-
window.__saveIdeConfig__ = function(jsonString) {
185-
window.external.__saveIdeConfig__(jsonString);
186-
};
187-
window.__ideLogin__ = function() {
188-
window.external.__ideLogin__();
189-
};
190-
window.__ideLogout__ = function() {
191-
window.external.__ideLogout__();
192-
};
193-
window.__onFormDirtyChange__ = function(isDirty) {
194-
window.external.__onFormDirtyChange__(isDirty);
195-
};
196-
window.__ideSaveAttemptFinished__ = function(status) {
197-
window.external.__ideSaveAttemptFinished__(status);
198-
};
199-
";
200-
201212
var scriptElement = doc.CreateElement("script");
202213
scriptElement.SetAttribute("type", "text/javascript");
203-
scriptElement.InnerText = script;
214+
scriptElement.InnerText = BuildIdeBridgeScript();
204215
doc.GetElementsByTagName("head")[0].AppendChild(scriptElement);
205216
}
206217
catch (Exception ex)
@@ -259,7 +270,7 @@ private void TitleBar_MouseLeftButtonDown(object sender, MouseButtonEventArgs e)
259270
}
260271
}
261272

262-
public void UpdateAuthToken(string token)
273+
public void UpdateAuthToken(string token, string apiUrl = null)
263274
{
264275
try
265276
{
@@ -269,7 +280,7 @@ public void UpdateAuthToken(string token)
269280

270281
if (SettingsBrowser?.Document != null)
271282
{
272-
InvokeSetAuthToken(token);
283+
InvokeSetAuthToken(token, apiUrl);
273284
}
274285
});
275286
}
@@ -279,7 +290,42 @@ public void UpdateAuthToken(string token)
279290
}
280291
}
281292

282-
private void InvokeSetAuthToken(string token)
293+
private void InvokeCommandCallback(string callbackId, string resultJson)
294+
{
295+
if (!ExecuteCommandBridge.IsValidCallbackId(callbackId))
296+
{
297+
Logger.Warning("Rejected callbackId with unexpected format: {CallbackId}", callbackId);
298+
return;
299+
}
300+
301+
try
302+
{
303+
ThreadHelper.JoinableTaskFactory.Run(async () =>
304+
{
305+
await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
306+
307+
dynamic doc = SettingsBrowser.Document;
308+
if (doc == null) return;
309+
310+
var escapedCallbackId = ExecuteCommandBridge.EscapeForJsString(callbackId);
311+
var script = $"if(window.__ideCallbacks__&&window.__ideCallbacks__['{escapedCallbackId}']){{" +
312+
$"var cb=window.__ideCallbacks__['{escapedCallbackId}'];" +
313+
$"delete window.__ideCallbacks__['{escapedCallbackId}'];" +
314+
$"cb({resultJson});}}";
315+
316+
var scriptElement = doc.CreateElement("script");
317+
scriptElement.SetAttribute("type", "text/javascript");
318+
scriptElement.InnerText = script;
319+
doc.GetElementsByTagName("head")[0].AppendChild(scriptElement);
320+
});
321+
}
322+
catch (Exception ex)
323+
{
324+
Logger.Error(ex, "Error invoking command callback {CallbackId}", callbackId);
325+
}
326+
}
327+
328+
private void InvokeSetAuthToken(string token, string apiUrl)
283329
{
284330
try
285331
{
@@ -291,11 +337,12 @@ private void InvokeSetAuthToken(string token)
291337
return;
292338
}
293339

294-
var escapedToken = token.Replace("'", "\\'").Replace("\"", "\\\"");
340+
var escapedToken = ExecuteCommandBridge.EscapeForJsString(token);
341+
var escapedApiUrl = ExecuteCommandBridge.EscapeForJsString(apiUrl ?? string.Empty);
295342
var script = $@"
296343
(function() {{
297344
if (typeof window.setAuthToken === 'function') {{
298-
window.setAuthToken('{escapedToken}');
345+
window.setAuthToken('{escapedToken}', '{escapedApiUrl}');
299346
}} else {{
300347
console.warn('window.setAuthToken is not available');
301348
}}
@@ -307,7 +354,7 @@ private void InvokeSetAuthToken(string token)
307354
scriptElement.InnerText = script;
308355
doc.GetElementsByTagName("head")[0].AppendChild(scriptElement);
309356

310-
Logger.Information("Invoked window.setAuthToken with token");
357+
Logger.Information("Invoked window.setAuthToken with token and apiUrl");
311358
}
312359
catch (Exception ex)
313360
{

‎Snyk.VisualStudio.Extension.2022/Snyk.VisualStudio.Extension.2022.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@
251251
</Compile>
252252
<Compile Include="UI\Html\BaseHtmlProvider.cs" />
253253
<Compile Include="UI\Html\ColorExtension.cs" />
254+
<Compile Include="UI\Html\ExecuteCommandBridge.cs" />
254255
<Compile Include="UI\Html\HtmlSettingsScriptingBridge.cs" />
255256
<Compile Include="UI\Html\IdeConfigData.cs" />
256257
<Compile Include="UI\Html\HtmlResourceLoader.cs" />

‎Snyk.VisualStudio.Extension.2022/UI/Html/ExecuteCommandBridge.cs‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// ABOUTME: Shared bridge for the window.__ideExecuteCommand__ JS<->IDE contract.
2+
// ABOUTME: Reusable by any WebBrowser panel (settings, tree view, etc.).
3+
4+
using System;
5+
using System.Text.RegularExpressions;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Newtonsoft.Json;
9+
using Serilog;
10+
using Snyk.VisualStudio.Extension;
11+
using Snyk.VisualStudio.Extension.Language;
12+
13+
namespace Snyk.VisualStudio.Extension.UI.Html
14+
{
15+
/// <summary>
16+
/// Shared bridge for the <c>window.__ideExecuteCommand__</c> JS↔IDE contract.
17+
/// Reusable by any WebBrowser panel (settings window, tree view, etc.).
18+
///
19+
/// Responsibilities:
20+
/// <list type="bullet">
21+
/// <item>Provide the ES5-compatible JS that defines <c>window.__ideExecuteCommand__</c>.</item>
22+
/// <item>Dispatch incoming commands to the Language Server.</item>
23+
/// <item>Return results to the JS callback via <c>window.__ideCallbacks__</c>.</item>
24+
/// </list>
25+
/// </summary>
26+
public static class ExecuteCommandBridge
27+
{
28+
private static readonly ILogger Logger = LogManager.ForContext(typeof(ExecuteCommandBridge));
29+
30+
// Allowlist regex for callbackIds produced by BuildClientScript: "" or "__cb_<digits>"
31+
private static readonly Regex CallbackIdPattern = new Regex(@"^(__cb_\d+)?$", RegexOptions.Compiled);
32+
33+
/// <summary>
34+
/// Escapes a string for safe embedding inside a single-quoted JavaScript string literal.
35+
/// Handles backslashes first, then single quotes, to avoid double-escaping.
36+
/// </summary>
37+
public static string EscapeForJsString(string value) =>
38+
(value ?? string.Empty).Replace("\\", "\\\\").Replace("'", "\\'");
39+
40+
/// <summary>
41+
/// Returns true if <paramref name="callbackId"/> matches the expected format produced by
42+
/// <see cref="BuildClientScript"/>. Used as an XSS guard before injecting the id back into JS.
43+
/// </summary>
44+
public static bool IsValidCallbackId(string callbackId) =>
45+
CallbackIdPattern.IsMatch(callbackId ?? string.Empty);
46+
47+
/// <summary>
48+
/// Returns true if <paramref name="command"/> is in the <c>snyk.*</c> namespace and may be
49+
/// dispatched from a webview.
50+
/// </summary>
51+
public static bool IsAllowedCommand(string command) =>
52+
!string.IsNullOrEmpty(command) && command.StartsWith("snyk.");
53+
54+
/// <summary>
55+
/// Returns the ES5-compatible JavaScript that defines <c>window.__ideExecuteCommand__</c>.
56+
/// Assumes <c>window.external.__ideExecuteCommand__(command, argsJson, callbackId)</c>
57+
/// is provided by the COM bridge (<see cref="HtmlSettingsScriptingBridge"/>).
58+
/// </summary>
59+
public static string BuildClientScript()
60+
{
61+
return @"
62+
window.__ideCallbacks__ = {};
63+
var __cbCounter_ide = 0;
64+
window.__ideExecuteCommand__ = function(command, args, callback) {
65+
var callbackId = '';
66+
if (typeof callback === 'function') {
67+
callbackId = '__cb_' + (++__cbCounter_ide);
68+
window.__ideCallbacks__[callbackId] = callback;
69+
}
70+
var argsJson = JSON.stringify(args || []);
71+
window.external.__ideExecuteCommand__(command, argsJson, callbackId);
72+
};
73+
";
74+
}
75+
76+
/// <summary>
77+
/// Dispatches a command to the Language Server and invokes <paramref name="onCommandResult"/>
78+
/// with the serialized result when a <paramref name="callbackId"/> is provided.
79+
/// </summary>
80+
public static async Task DispatchAsync(
81+
ILanguageClientManager languageClientManager,
82+
string command,
83+
string argsJson,
84+
string callbackId,
85+
Action<string, string> onCommandResult,
86+
CancellationToken cancellationToken = default)
87+
{
88+
try
89+
{
90+
if (!IsAllowedCommand(command))
91+
{
92+
Logger.Warning("Webview attempted to execute disallowed command: {Command}", command);
93+
return;
94+
}
95+
96+
var args = string.IsNullOrEmpty(argsJson)
97+
? Array.Empty<object>()
98+
: JsonConvert.DeserializeObject<object[]>(argsJson);
99+
100+
var result = await languageClientManager.InvokeExecuteCommandAsync(
101+
command, args, cancellationToken);
102+
103+
if (!string.IsNullOrEmpty(callbackId))
104+
{
105+
if (!IsValidCallbackId(callbackId))
106+
{
107+
Logger.Warning("Rejected callbackId with unexpected format: {CallbackId}", callbackId);
108+
return;
109+
}
110+
111+
var resultJson = result != null ? JsonConvert.SerializeObject(result) : "null";
112+
onCommandResult?.Invoke(callbackId, resultJson);
113+
}
114+
}
115+
catch (Exception ex)
116+
{
117+
Logger.Error(ex, "Error executing command {Command} from HTML bridge", command);
118+
}
119+
}
120+
}
121+
}

0 commit comments

Comments
 (0)