Review comments

.github/workflows/native-dependencies.yml

@@ -12,16 +12,17 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v3
 
+      # Update the node version here after every Electron upgrade
       - name: Use Node.js 18.17.0
         uses: actions/setup-node@v3
         with:
           node-version: '18.17.0'
           registry-url: 'https://registry.npmjs.org'
 
-      - name: Use Python 3.x
+      - name: Use Python 3.11
         uses: actions/setup-python@v4
         with:
-          python-version: '3.x'
+          python-version: '3.11'
 
       - name: Install and Build
         shell: bash

packages/core/package.json

@@ -23,7 +23,6 @@
     "@types/dompurify": "^2.2.2",
     "@types/express": "^4.16.0",
     "@types/fs-extra": "^4.0.2",
-    "@types/glob": "^8.1.0",
     "@types/lodash.debounce": "4.0.3",
     "@types/lodash.throttle": "^4.1.3",
     "@types/markdown-it": "^12.2.3",
@@ -47,7 +46,6 @@
     "font-awesome": "^4.7.0",
     "fs-extra": "^4.0.2",
     "fuzzy": "^0.1.3",
-    "glob": "^8.1.0",
     "http-proxy-agent": "^5.0.0",
     "https-proxy-agent": "^5.0.0",
     "iconv-lite": "^0.6.0",
@@ -168,9 +166,6 @@
     {
       "backend": "lib/node/request/backend-request-module",
       "backendElectron": "lib/electron-node/request/electron-backend-request-module"
-    },
-    {
-      "backend": "lib/node/remote/backend-remote-module"
     }
   ],
   "keywords": [

packages/core/src/browser/credentials-service.ts

@@ -22,7 +22,7 @@
 
 import { inject, injectable } from 'inversify';
 import { Emitter, Event } from '../common/event';
-import { KeytarService } from '../common/keytar-protocol';
+import { KeyStoreService } from '../common/key-store';
 
 export interface CredentialsProvider {
     getPassword(service: string, account: string): Promise<string | undefined>;
@@ -50,7 +50,7 @@ export class CredentialsServiceImpl implements CredentialsService {
 
     private credentialsProvider: CredentialsProvider;
 
-    constructor(@inject(KeytarService) private readonly keytarService: KeytarService) {
+    constructor(@inject(KeyStoreService) private readonly keytarService: KeyStoreService) {
         this.credentialsProvider = new KeytarCredentialsProvider(this.keytarService);
     }
 
@@ -82,7 +82,7 @@ export class CredentialsServiceImpl implements CredentialsService {
 
 class KeytarCredentialsProvider implements CredentialsProvider {
 
-    constructor(private readonly keytarService: KeytarService) { }
+    constructor(private readonly keytarService: KeyStoreService) { }
 
     deletePassword(service: string, account: string): Promise<boolean> {
         return this.keytarService.deletePassword(service, account);

packages/core/src/browser/frontend-application-module.ts

@@ -96,7 +96,7 @@ import { EncodingRegistry } from './encoding-registry';
 import { EncodingService } from '../common/encoding-service';
 import { AuthenticationService, AuthenticationServiceImpl } from '../browser/authentication-service';
 import { DecorationsService, DecorationsServiceImpl } from './decorations-service';
-import { keytarServicePath, KeytarService } from '../common/keytar-protocol';
+import { keyStoreServicePath, KeyStoreService } from '../common/key-store';
 import { CredentialsService, CredentialsServiceImpl } from './credentials-service';
 import { ContributionFilterRegistry, ContributionFilterRegistryImpl } from '../common/contribution-filter';
 import { QuickCommandFrontendContribution } from './quick-input/quick-command-frontend-contribution';
@@ -136,7 +136,6 @@ import { MarkdownRenderer, MarkdownRendererFactory, MarkdownRendererImpl } from
 import { StylingParticipant, StylingService } from './styling-service';
 import { bindCommonStylingParticipants } from './common-styling-participants';
 import { HoverService } from './hover-service';
-import { NullRemoteService, RemoteService } from './remote-service';
 import { AdditionalViewsMenuWidget, AdditionalViewsMenuWidgetFactory } from './shell/additional-views-menu-widget';
 
 export { bindResourceProvider, bindMessageService, bindPreferenceService };
@@ -399,9 +398,9 @@ export const frontendApplicationModule = new ContainerModule((bind, _unbind, _is
     bind(AuthenticationService).to(AuthenticationServiceImpl).inSingletonScope();
     bind(DecorationsService).to(DecorationsServiceImpl).inSingletonScope();
 
-    bind(KeytarService).toDynamicValue(ctx => {
+    bind(KeyStoreService).toDynamicValue(ctx => {
         const connection = ctx.container.get(WebSocketConnectionProvider);
-        return connection.createProxy<KeytarService>(keytarServicePath);
+        return connection.createProxy<KeyStoreService>(keyStoreServicePath);
     }).inSingletonScope();
 
     bind(CredentialsService).to(CredentialsServiceImpl);
@@ -449,8 +448,5 @@ export const frontendApplicationModule = new ContainerModule((bind, _unbind, _is
     bindContributionProvider(bind, StylingParticipant);
     bind(FrontendApplicationContribution).toService(StylingService);
 
-    bind(NullRemoteService).toSelf().inSingletonScope();
-    bind(RemoteService).toService(NullRemoteService);
-
     bind(SecondaryWindowHandler).toSelf().inSingletonScope();
 });

packages/core/src/browser/index.ts

@@ -44,4 +44,3 @@ export * from './tooltip-service';
 export * from './decoration-style';
 export * from './styling-service';
 export * from './hover-service';
-export * from './remote-service';

packages/core/src/browser/messaging/ws-connection-provider.ts

@@ -14,7 +14,7 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import { injectable, interfaces, decorate, unmanaged, postConstruct } from 'inversify';
+import { injectable, interfaces, decorate, unmanaged } from 'inversify';
 import { RpcProxyFactory, RpcProxy, Emitter, Event, Channel } from '../../common';
 import { Endpoint } from '../endpoint';
 import { AbstractConnectionProvider } from '../../common/messaging/abstract-connection-provider';
@@ -63,15 +63,23 @@ export class WebSocketConnectionProvider extends AbstractConnectionProvider<WebS
         }
     }
 
-    protected socket: Socket;
+    protected readonly socket: Socket;
 
     constructor() {
         super();
-    }
-
-    @postConstruct()
-    protected init(): void {
-        this.connect();
+        const url = this.createWebSocketUrl(WebSocketChannel.wsPath);
+        this.socket = this.createWebSocket(url);
+        this.socket.on('connect', () => {
+            this.initializeMultiplexer();
+            if (this.reconnectChannelOpeners.length > 0) {
+                this.reconnectChannelOpeners.forEach(opener => opener());
+                this.reconnectChannelOpeners = [];
+            }
+            this.socket.on('disconnect', () => this.fireSocketDidClose());
+            this.socket.on('message', () => this.onIncomingMessageActivityEmitter.fire(undefined));
+            this.fireSocketDidOpen();
+        });
+        this.socket.connect();
     }
 
     protected createMainChannel(): Channel {
@@ -122,22 +130,6 @@ export class WebSocketConnectionProvider extends AbstractConnectionProvider<WebS
         return new Endpoint({ path });
     }
 
-    protected connect(path: string = WebSocketChannel.wsPath): void {
-        const url = this.createWebSocketUrl(path);
-        this.socket = this.createWebSocket(url);
-        this.socket.on('connect', () => {
-            this.initializeMultiplexer();
-            if (this.reconnectChannelOpeners.length > 0) {
-                this.reconnectChannelOpeners.forEach(opener => opener());
-                this.reconnectChannelOpeners = [];
-            }
-            this.socket.on('disconnect', () => this.fireSocketDidClose());
-            this.socket.on('message', () => this.onIncomingMessageActivityEmitter.fire(undefined));
-            this.fireSocketDidOpen();
-        });
-        this.socket.connect();
-    }
-
     /**
      * Creates a web socket for the given url
      */

packages/core/src/common/keytar-protocol.ts → packages/core/src/common/key-store.ts

@@ -14,10 +14,10 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
 // *****************************************************************************
 
-export const keytarServicePath = '/services/keytar';
+export const keyStoreServicePath = '/services/keyStore';
 
-export const KeytarService = Symbol('KeytarService');
-export interface KeytarService {
+export const KeyStoreService = Symbol('KeyStoreService');
+export interface KeyStoreService {
     setPassword(service: string, account: string, password: string): Promise<void>;
     getPassword(service: string, account: string): Promise<string | undefined>;
     deletePassword(service: string, account: string): Promise<boolean>;

packages/core/src/electron-node/hosting/electron-ws-origin-validator.ts

@@ -16,7 +16,7 @@
 
 import * as http from 'http';
 import { inject, injectable } from 'inversify';
-import { BackendRemoteService } from '../../node/remote/backend-remote-service';
+import { BackendRemoteService } from '../../node/backend-remote-service';
 import { WsRequestValidatorContribution } from '../../node/ws-request-validators';
 
 @injectable()

packages/core/src/electron-node/token/electron-token-validator.ts

@@ -84,6 +84,8 @@ export class ElectronTokenValidator implements WsRequestValidatorContribution {
         if (token) {
             return JSON.parse(token);
         } else {
+            // No token has been passed to the backend server
+            // That indicates we're running without a local frontend
             return undefined;
         }
     }

packages/core/src/node/backend-application-module.ts

@@ -31,8 +31,8 @@ import { EnvVariablesServerImpl } from './env-variables';
 import { ConnectionContainerModule } from './messaging/connection-container-module';
 import { QuickInputService, quickInputServicePath, QuickPickService, quickPickServicePath } from '../common/quick-pick-service';
 import { WsRequestValidator, WsRequestValidatorContribution } from './ws-request-validators';
-import { KeytarService, keytarServicePath } from '../common/keytar-protocol';
-import { KeytarServiceImpl } from './keytar-server';
+import { KeyStoreService, keyStoreServicePath } from '../common/key-store';
+import { KeyStoreServiceImpl } from './key-store-server';
 import { ContributionFilterRegistry, ContributionFilterRegistryImpl } from '../common/contribution-filter';
 import { EnvironmentUtils } from './environment-utils';
 import { ProcessUtils } from './process-utils';
@@ -41,6 +41,7 @@ import { bindNodeStopwatch, bindBackendStopwatchServer } from './performance';
 import { OSBackendProviderImpl } from './os-backend-provider';
 import { BackendRequestFacade } from './request/backend-request-facade';
 import { FileSystemLocking, FileSystemLockingImpl } from './filesystem-locking';
+import { BackendRemoteService } from './backend-remote-service';
 
 decorate(injectable(), ApplicationPackage);
 
@@ -107,9 +108,9 @@ export const backendApplicationModule = new ContainerModule(bind => {
 
     bind(WsRequestValidator).toSelf().inSingletonScope();
     bindContributionProvider(bind, WsRequestValidatorContribution);
-    bind(KeytarService).to(KeytarServiceImpl).inSingletonScope();
+    bind(KeyStoreService).to(KeyStoreServiceImpl).inSingletonScope();
     bind(ConnectionHandler).toDynamicValue(ctx =>
-        new RpcConnectionHandler(keytarServicePath, () => ctx.container.get<KeytarService>(KeytarService))
+        new RpcConnectionHandler(keyStoreServicePath, () => ctx.container.get<KeyStoreService>(KeyStoreService))
     ).inSingletonScope();
 
     bind(ContributionFilterRegistry).to(ContributionFilterRegistryImpl).inSingletonScope();
@@ -126,6 +127,7 @@ export const backendApplicationModule = new ContainerModule(bind => {
     bind(ProxyCliContribution).toSelf().inSingletonScope();
     bind(CliContribution).toService(ProxyCliContribution);
 
+    bind(BackendRemoteService).toSelf().inSingletonScope();
     bind(BackendRequestFacade).toSelf().inSingletonScope();
     bind(ConnectionHandler).toDynamicValue(
         ctx => new RpcConnectionHandler(REQUEST_SERVICE_PATH, () => ctx.container.get(BackendRequestFacade))

packages/core/src/node/keytar-server.ts → packages/core/src/node/key-store-server.ts

@@ -20,31 +20,27 @@
  *--------------------------------------------------------------------------------------------*/
 // code copied and modified from https://github.com/microsoft/vscode/blob/1.55.2/src/vs/platform/native/electron-main/nativeHostMainService.ts#L679-L771
 
-import { KeytarService } from '../common/keytar-protocol';
-import { inject, injectable } from 'inversify';
+import { KeyStoreService } from '../common/key-store';
+import { injectable } from 'inversify';
 import { isWindows } from '../common';
-import { BackendRemoteService } from './remote/backend-remote-service';
 
 @injectable()
-export class KeytarServiceImpl implements KeytarService {
-
-    @inject(BackendRemoteService)
-    protected readonly backendRemoteService: BackendRemoteService;
+export class KeyStoreServiceImpl implements KeyStoreService {
 
     private static readonly MAX_PASSWORD_LENGTH = 2500;
-    private static readonly PASSWORD_CHUNK_SIZE = KeytarServiceImpl.MAX_PASSWORD_LENGTH - 100;
+    private static readonly PASSWORD_CHUNK_SIZE = KeyStoreServiceImpl.MAX_PASSWORD_LENGTH - 100;
 
-    protected cache?: typeof import('keytar');
+    protected keytarImplementation?: typeof import('keytar');
 
     async setPassword(service: string, account: string, password: string): Promise<void> {
         const keytar = await this.getKeytar();
-        if (isWindows && password.length > KeytarServiceImpl.MAX_PASSWORD_LENGTH) {
+        if (isWindows && password.length > KeyStoreServiceImpl.MAX_PASSWORD_LENGTH) {
             let index = 0;
             let chunk = 0;
             let hasNextChunk = true;
             while (hasNextChunk) {
-                const passwordChunk = password.substring(index, index + KeytarServiceImpl.PASSWORD_CHUNK_SIZE);
-                index += KeytarServiceImpl.PASSWORD_CHUNK_SIZE;
+                const passwordChunk = password.substring(index, index + KeyStoreServiceImpl.PASSWORD_CHUNK_SIZE);
+                index += KeyStoreServiceImpl.PASSWORD_CHUNK_SIZE;
                 hasNextChunk = password.length - index > 0;
 
                 const content: ChunkedPassword = {
@@ -94,9 +90,7 @@ export class KeytarServiceImpl implements KeytarService {
     async findPassword(service: string): Promise<string | undefined> {
         const keytar = await this.getKeytar();
         const password = await keytar.findPassword(service);
-        if (password) {
-            return password;
-        }
+        return password ?? undefined;
     }
 
     async findCredentials(service: string): Promise<Array<{ account: string, password: string }>> {
@@ -105,20 +99,18 @@ export class KeytarServiceImpl implements KeytarService {
     }
 
     protected async getKeytar(): Promise<typeof import('keytar')> {
-        if (this.cache) {
-            return this.cache;
+        if (this.keytarImplementation) {
+            return this.keytarImplementation;
         }
         try {
-            return (this.cache = await import('keytar'));
+            this.keytarImplementation = await import('keytar');
+            // Try using keytar to see if it throws or not.
+            await this.keytarImplementation.findCredentials('test-keytar-loads');
         } catch (err) {
-            if (this.backendRemoteService.isRemoteServer()) {
-                // When running as a remote server, the necessary prerequisites might not be installed on the system
-                // Just use an in-memory cache for credentials
-                return (this.cache = new InMemoryCredentialsProvider());
-            } else {
-                throw err;
-            }
+            this.keytarImplementation = new InMemoryCredentialsProvider();
+            console.warn('OS level credential store could not be accessed. Using in-memory credentials provider', err);
         }
+        return this.keytarImplementation;
     }
 }
 

packages/core/src/node/messaging/messaging-contribution.ts

@@ -60,11 +60,11 @@ export class MessagingContribution implements BackendApplicationContribution, Me
     }
 
     wsChannel(spec: string, callback: (params: MessagingService.PathParams, channel: Channel) => void): void {
-        return this.channelHandlers.push(spec, (params, channel) => callback(params, channel));
+        this.channelHandlers.push(spec, (params, channel) => callback(params, channel));
     }
 
     ws(spec: string, callback: (params: MessagingService.PathParams, socket: Socket) => void): void {
-        return this.wsHandlers.push(spec, callback);
+        this.wsHandlers.push(spec, callback);
     }
 
     protected checkAliveTimeout = 30000; // 30 seconds