feat(IDE-1701): settings page auth flow — bridge persist and forward apiUrl by nick-y-snyk · Pull Request #724 · snyk/vscode-extension

nick-y-snyk · 2026-03-02T19:03:34Z

What & Why

The settings page drives authentication via snyk.login with [authMethod, endpoint, insecure] args. Before forwarding to the LS, the IDE must persist these values locally so they survive a page close. Additionally, $/snyk.hasAuthenticated now carries apiUrl which is forwarded to the webview so the settings page can update both fields after auth.

Changes

ExecuteCommandBridge (new shared class)
Replaces __ideLogin__/__ideLogout__ JS bridge functions with a single window.__ideExecuteCommand__(cmd, args, callback) bridge. Callback results returned to JS via window.__ideCallbacks__.

MessageHandlerFactory — bridge persist
When type === 'executeCommand', command === 'snyk.login', and args.length >= 3, saves auth params to VS Code settings before forwarding to ExecuteCommandBridge:

args[0] mapped to VS Code auth method string ("oauth" → 'OAuth2 (Recommended)', "pat" → 'Personal Access Token', "token" → 'API Token (Legacy)') and written via new IConfiguration.setAuthenticationMethod()
args[1] (endpoint) written via configuration.setEndpoint() wrapped in the AuthenticationService.authFlowUpdatingEndpoint guard — prevents ConfigurationWatcher from clearing the token when the endpoint changes
args[2] (insecure) written via new IConfiguration.setInsecure() → stores !insecure to http.proxyStrictSSL
IConfiguration receives IConfiguration as an optional 4th constructor param; extension.ts passes it
SyncConfigurationFeature fires a redundant didChangeConfiguration for snyk.* writes — unavoidable without middleware, harmless since LS handles it idempotently; http.* does not trigger it

authenticationService.ts — endpoint guard
Adds authFlowUpdatingEndpoint static flag. Set to true while setEndpoint() runs during updateTokenAndEndpoint, cleared in finally. External callers (e.g. MessageHandlerFactory) can also set/clear it via setAuthFlowUpdatingEndpoint().

configurationWatcher.ts
Checks AuthenticationService.isAuthFlowUpdatingEndpoint() before clearing the token on endpoint change.

languageServer.ts
$/snyk.hasAuthenticated handler passes apiUrl alongside token to setAuthToken(token, apiUrl) on the webview provider.

htmlInjectionService.ts / workspaceConfigurationWebviewProvider.ts
setAuthToken(token, apiUrl) forwarding: webview provider posts { type: 'setAuthToken', token, apiUrl }; injection service listener calls window.setAuthToken(message.token, message.apiUrl).

commandController.ts
initiateLogin() passes getAuthenticationMethod(), snykApiEndpoint, getInsecure() as args to SNYK_LOGIN_COMMAND — ensures command-palette login also sends auth params to LS.

Tests

messageHandlerFactory.test.ts: 5 new login-args-persist tests (method mapping, endpoint guard, insecure, no-op on <3 args)
authenticationService.test.ts: 3 new guard-flag tests (true during call, false after, false after error)
executeCommandBridge.test.ts: new unit tests for the shared bridge class
htmlInjectionService.test.ts: __ideExecuteCommand__ injection present, deprecated functions absent
commandController.test.ts: auth params forwarded to login command

Test plan

Settings page: change endpoint + auth method → click Authenticate → IDE saves values → auth succeeds → token and apiUrl appear in settings page
Close/reopen settings page → values persist
Command palette login → auth succeeds → auth params forwarded to LS
No token cleared when endpoint changes during auth flow (guard works)
Run test suite: npm test

…nd bridge Consolidates __ideLogin__ and __ideLogout__ into a single __ideExecuteCommand__ bridge function, and passes auth configuration params to the login command.

snyk-io · 2026-03-02T19:03:46Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

- Use discriminated union for WebviewMessage (SaveConfigMessage | ExecuteCommandMessage) - Validate arguments as array in isWebviewMessage type guard - Use ErrorHandler.handle in catch block instead of template literal (preserves stack trace) - Remove unreachable default case from switch (unknown types rejected by type guard) - Add missing tests: empty command guard, empty config guard, non-array arguments, error path - Fix ordering assertion to use getCall() instead of indexOf() - Add test for injectIdeScripts with no closing body tag

Wire up window.__ideCallbacks__ registry and commandResult message round-trip so the webview can receive LS command results asynchronously.

…use [IDE-1701] Move the window.__ideExecuteCommand__ client-side script builder and extension-side command dispatch into a standalone ExecuteCommandBridge class. MessageHandlerFactory and HtmlInjectionService delegate to it, enabling any future HTML webview (e.g. tree view) to reuse the same bridge.

Adds a static guard flag on AuthenticationService that is set true while setEndpoint is running. ConfigurationWatcher skips clearToken when the flag is set, preventing the login flow from wiping the token it just received.

… flag - Revert persist flag from updateTokenAndEndpoint — always saves and triggers scan - Revert persist destructuring in hasAuthenticated notification handler - Add setAuthenticationMethod/setInsecure to IConfiguration/Configuration - Add bridge persist in MessageHandlerFactory: when snyk.login called with 3+ args from settings page, save authMethod/endpoint/insecure to IDE storage before forwarding to LS (no didChangeConfiguration for http.proxyStrictSSL; snyk.* changes handled idempotently by LS) - Pass configuration to MessageHandlerFactory in extension.ts - Add tests for login args persist and auth method mapping

Pass apiUrl alongside token when calling window.setAuthToken from the hasAuthenticated notification so the settings page can update both fields.

Resolved conflict in commandController.test.ts by keeping both the initiateLogin tests (from this branch) and the truncateForDisplay tests (from main), and aligning import style to use assert namespace.

- Validate callbackId against /^__cb_\d+$/ in ExecuteCommandBridge before echoing back to JS, preventing prototype-pollution via crafted keys - Guard window.setAuthToken call with typeof check in injected script so pages that have not yet defined setAuthToken do not throw on message receipt

…E-1701] The LS HTML settings page handles auth method, endpoint, and insecure persistence itself via auto-save, so the extension no longer needs to snapshot login args before forwarding snyk.login. Remove saveLoginArgs from MessageHandlerFactory and drop the configuration dependency. Remove the authFlowUpdatingEndpoint static guard from AuthenticationService and the conditional clearToken() skip in ConfigurationWatcher — the guard is no longer needed since the extension is not writing the endpoint during the auth flow. Also remove setAuthenticationMethod and setInsecure from IConfiguration as they have no remaining callers.

ShawkyZ · 2026-03-25T14:51:53Z

src/snyk/common/views/workspaceConfiguration/handlers/messageHandlerFactory.ts


 export interface IMessageHandlerFactory {
-  handleMessage(message: unknown): Promise<void>;
+  handleMessage(message: unknown): Promise<{ callbackId: string; result: unknown } | void>;


nitpick extract to a type

ShawkyZ · 2026-03-25T14:56:34Z

src/snyk/common/views/workspaceConfiguration/workspaceConfigurationWebviewProvider.ts

-      (msg: unknown) => this.messageHandlerFactory.handleMessage(msg),
+      async (msg: unknown) => {
+        const callbackResult = await this.messageHandlerFactory.handleMessage(msg);
+        if (callbackResult?.callbackId) {


when do we have a callbackId ? and what does it mean ?

added code comment

Prevents XSS-to-arbitrary-command escalation by rejecting any command not prefixed with "snyk." before it reaches vscode.commands.executeCommand.

snyk-pr-review-bot · 2026-03-26T10:42:11Z

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 Security concerns Insecure Command Execution Bridge: The generic bridge for `snyk.*` commands (src/snyk/common/views/executeCommandBridge.ts) lacks argument validation and a strict allowlist. This allows a potentially compromised webview to execute sensitive extension commands with arbitrary payloads, such as opening malicious URLs via the `snyk.open` command.
⚡ Recommended focus areas for review Missing Persistence Logic 🟠 [major] The PR description states that `MessageHandlerFactory` should persist `snyk.login` arguments to VS Code settings before forwarding to the bridge. However, the diff only shows a generic forwarding case for `executeCommand`. Without this logic, auth parameters passed from the settings webview will not be saved locally, violating the stated requirement that they must 'survive a page close'. case 'executeCommand': return await this.executeCommandBridge.handleMessage(message); Broad Attack Surface 🟠 [major] The `ExecuteCommandBridge` allows any webview to execute any command starting with `snyk.` and provides arguments directly to `commandExecutor.executeCommand`. This is insecure because it includes sensitive commands like `snyk.open` (which opens URLs/files) and provides no argument validation. A compromised webview could exploit this to perform actions outside the intended scope of the settings page. const result = await this.commandExecutor.executeCommand(msg.command, ...(msg.arguments ?? [])); Breaking Change 🟡 [minor] The removal of the 'login' and 'logout' cases in `MessageHandlerFactory` combined with the update to `isWebviewMessage` means that any webview still using the legacy message types will fail with a 'Received invalid message' warning. This breaks backward compatibility with older HTML templates or cached versions of the settings page. if (type === 'saveConfig') { return hasOptionalPropertyOfType(message, 'config', 'string'); } return type === 'executeCommand';
📚 Repository Context Analyzed This review considered 50 relevant code sections from 15 files (average relevance: 0.78) src/extension.ts (lines 1-18) src/snyk/common/tsUtil.ts (lines 1-52) src/snyk/common/views/webviewProvider.ts (lines 1-80) src/snyk/base/modules/baseSnykModule.ts (lines 1-90) src/snyk/common/views/nonce.ts (lines 1-8) src/snyk/common/views/workspaceConfiguration/services/htmlInjectionService.ts (lines 1-71) src/snyk/common/configuration/configuration.ts (lines 49-761) ... and 8 more file(s)

feat: replace login/logout webview commands with generic executeComma…

772f6f0

…nd bridge Consolidates __ideLogin__ and __ideLogout__ into a single __ideExecuteCommand__ bridge function, and passes auth configuration params to the login command.

nick-y-snyk requested review from a team as code owners March 2, 2026 19:03

nick-y-snyk added 6 commits March 3, 2026 10:18

feat: add callback support to __ideExecuteCommand__ bridge [IDE-1701]

5e1a5ed

Wire up window.__ideCallbacks__ registry and commandResult message round-trip so the webview can receive LS command results asynchronously.

feat(IDE-1701): forward apiUrl in setAuthToken for settings page webview

0fb2ad6

Pass apiUrl alongside token when calling window.setAuthToken from the hasAuthenticated notification so the settings page can update both fields.

nick-y-snyk changed the title ~~feat: replace login/logout webview commands with generic executeCommand bridge~~ feat(IDE-1701): settings page auth flow — bridge persist and remove persist flag Mar 5, 2026

nick-y-snyk changed the title ~~feat(IDE-1701): settings page auth flow — bridge persist and remove persist flag~~ feat(IDE-1701): settings page auth flow — bridge persist and forward apiUrl Mar 5, 2026

Merge branch 'main' into feat/generic-webview-execute-command-bridge

4b7723d