diff --git a/cli/src/tunnels/code_server.rs b/cli/src/tunnels/code_server.rs index 18466efe0277..cb9db9edaa14 100644 --- a/cli/src/tunnels/code_server.rs +++ b/cli/src/tunnels/code_server.rs @@ -43,6 +43,7 @@ static LISTENING_PORT_RE: LazyLock = LazyLock::new(|| Regex::new(r"Extension host agent listening on (.+)").unwrap()); static WEB_UI_RE: LazyLock = LazyLock::new(|| Regex::new(r"Web UI available at (.+)").unwrap()); +const AGENT_HOST_BRIDGE_CONNECTION_TOKEN_ENV: &str = "VSCODE_AGENT_HOST_BRIDGE_CONNECTION_TOKEN"; #[derive(Clone, Debug, Default)] pub struct CodeServerArgs { @@ -172,12 +173,18 @@ impl CodeServerArgs { if let Some(host) = &self.agent_host_bridge_host { args.push(format!("--agent-host-bridge-host={host}")); } - if let Some(token) = &self.agent_host_bridge_connection_token { - args.push(format!("--agent-host-bridge-connection-token={token}")); - } } args } + + fn apply_to_command(&self, command: &mut Command) { + command.args(self.command_arguments()); + if self.agent_host_bridge_port.is_some() { + if let Some(token) = &self.agent_host_bridge_connection_token { + command.env(AGENT_HOST_BRIDGE_CONNECTION_TOKEN_ENV, token); + } + } + } } /// Base server params that can be `resolve()`d to a `ResolvedServerParams`. @@ -630,7 +637,11 @@ impl<'a> ServerBuilder<'a> { async fn spawn_server_process(&self, mut cmd: Command) -> Result { info!(self.logger, "Starting server..."); - debug!(self.logger, "Starting server with command... {:?}", cmd); + debug!( + self.logger, + "Starting server process: {:?}", + cmd.as_std().get_program() + ); // On Windows spawning a code-server binary will run cmd.exe /c C:\path\to\code-server.cmd... // This spawns a cmd.exe window for the user, which if they close will kill the code-server process @@ -688,8 +699,10 @@ impl<'a> ServerBuilder<'a> { fn get_base_command(&self) -> Command { let mut cmd = new_script_command(&self.server_paths.executable); - cmd.stdin(std::process::Stdio::null()) - .args(self.server_params.code_server_args.command_arguments()); + cmd.stdin(std::process::Stdio::null()); + self.server_params + .code_server_args + .apply_to_command(&mut cmd); cmd } } @@ -959,3 +972,47 @@ async fn get_should_use_breakaway_from_job() -> bool { cmd.args(["/C", "echo ok"]).output().await.is_ok() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn agent_host_bridge_connection_token_is_only_in_command_environment() { + let args = CodeServerArgs { + agent_host_bridge_host: Some("127.0.0.1".to_string()), + agent_host_bridge_port: Some(9000), + agent_host_bridge_connection_token: Some("secret-token".to_string()), + ..Default::default() + }; + let mut command = Command::new("code-server"); + args.apply_to_command(&mut command); + let command = command.as_std(); + + assert_eq!( + ( + command + .get_args() + .map(|argument| argument.to_string_lossy().into_owned()) + .collect::>(), + command + .get_envs() + .map(|(name, value)| ( + name.to_string_lossy().into_owned(), + value.map(|value| value.to_string_lossy().into_owned()) + )) + .collect::>(), + ), + ( + vec![ + "--agent-host-bridge-port=9000".to_string(), + "--agent-host-bridge-host=127.0.0.1".to_string(), + ], + vec![( + AGENT_HOST_BRIDGE_CONNECTION_TOKEN_ENV.to_string(), + Some("secret-token".to_string()), + )], + ) + ); + } +} diff --git a/extensions/vscode-test-resolver/src/extension.ts b/extensions/vscode-test-resolver/src/extension.ts index 32a93fe36462..3cc55e003e83 100644 --- a/extensions/vscode-test-resolver/src/extension.ts +++ b/extensions/vscode-test-resolver/src/extension.ts @@ -23,6 +23,7 @@ const enum CharCode { let outputChannel: vscode.OutputChannel; const SLOWED_DOWN_CONNECTION_DELAY = 800; +const agentHostBridgeConnectionTokenEnvironmentVariable = 'VSCODE_AGENT_HOST_BRIDGE_CONNECTION_TOKEN'; function isExpectedSocketCloseError(error: NodeJS.ErrnoException): boolean { return error.code === 'ECONNRESET' || error.code === 'EPIPE' || error.code === 'ECONNABORTED'; @@ -190,7 +191,7 @@ export function activate(context: vscode.ExtensionContext) { } const agentHostBridgeToken = getConfiguration('agentHostBridgeConnectionToken'); if (typeof agentHostBridgeToken === 'string' && agentHostBridgeToken) { - commandArgs.push('--agent-host-bridge-connection-token', agentHostBridgeToken); + env[agentHostBridgeConnectionTokenEnvironmentVariable] = agentHostBridgeToken; } if (!commit) { // dev mode diff --git a/src/vs/server/node/agentHostChannel.ts b/src/vs/server/node/agentHostChannel.ts index 322ed923f35a..725475157a56 100644 --- a/src/vs/server/node/agentHostChannel.ts +++ b/src/vs/server/node/agentHostChannel.ts @@ -208,7 +208,6 @@ class WebSocketUpstreamConnection extends Disposable implements IUpstreamConnect const url = this._buildUrl(); const wsOptions = await this._buildWsOptions(); - this._logService.info(`[AgentHostChannel] Opening upstream to ${this._endpoint.socketPath ?? url}`); const socket = new ws.WebSocket(url, wsOptions); this._ws = socket; @@ -361,9 +360,7 @@ export class AgentHostChannel extends Disposable implements IServerCha private _getOrCreate(ctx: TContext): IUpstreamConnection { let conn = this._perCtx.get(ctx); if (!conn) { - conn = typeof this._endpoint === 'function' - ? new LazyUpstreamConnection(() => this._resolveEndpoint(), this._upstreamFactory, this._logService) - : this._upstreamFactory(this._endpoint); + conn = new LazyUpstreamConnection(() => this._resolveEndpoint(), endpoint => this._createUpstream(endpoint), this._logService); this._perCtx.set(ctx, conn); // If the upstream closes on its own (e.g. agent host restart or // connection drop), evict it from the cache so the next @@ -379,6 +376,12 @@ export class AgentHostChannel extends Disposable implements IServerCha return conn; } + private _createUpstream(endpoint: IAgentHostUpstreamEndpoint): IUpstreamConnection { + const logTarget = endpoint.socketPath ?? `${endpoint.host ?? 'localhost'}:${endpoint.port ?? '0'}`; + this._logService.info(`[AgentHostChannel] Opening upstream to ${logTarget}`); + return this._upstreamFactory(endpoint); + } + private async _resolveEndpoint(): Promise { const endpoint = this._endpoint; if (typeof endpoint !== 'function') { diff --git a/src/vs/server/node/remoteExtensionHostAgentCli.ts b/src/vs/server/node/remoteExtensionHostAgentCli.ts index 46238862457d..f7d10061ad87 100644 --- a/src/vs/server/node/remoteExtensionHostAgentCli.ts +++ b/src/vs/server/node/remoteExtensionHostAgentCli.ts @@ -25,7 +25,7 @@ import { DiskFileSystemProvider } from '../../platform/files/node/diskFileSystem import { Schemas } from '../../base/common/network.js'; import { IFileService } from '../../platform/files/common/files.js'; import { IProductService } from '../../platform/product/common/productService.js'; -import { IServerEnvironmentService, ServerEnvironmentService, ServerParsedArgs } from './serverEnvironmentService.js'; +import { getRedactedServerParsedArgs, IServerEnvironmentService, ServerEnvironmentService, ServerParsedArgs } from './serverEnvironmentService.js'; import { ExtensionManagementCLI } from '../../platform/extensionManagement/common/extensionManagementCLI.js'; import { ILanguagePackService } from '../../platform/languagePacks/common/languagePacks.js'; import { NativeLanguagePackService } from '../../platform/languagePacks/node/languagePacks.js'; @@ -107,7 +107,7 @@ class CliMain extends Disposable { const logService = new LogService(this._register(loggerService.createLogger('remoteCLI', { name: localize('remotecli', "Remote CLI") }))); services.set(ILogService, logService); logService.trace(`Remote configuration data at ${this.remoteDataFolder}`); - logService.trace('process arguments:', this.args); + logService.trace('process arguments:', getRedactedServerParsedArgs(this.args)); // Files const fileService = this._register(new FileService(logService)); diff --git a/src/vs/server/node/remoteExtensionHostAgentServer.ts b/src/vs/server/node/remoteExtensionHostAgentServer.ts index 2fc682eb1ccd..fcecdac09780 100644 --- a/src/vs/server/node/remoteExtensionHostAgentServer.ts +++ b/src/vs/server/node/remoteExtensionHostAgentServer.ts @@ -620,7 +620,7 @@ export interface IServerAPI { dispose(): void; } -export async function createServer(address: string | net.AddressInfo | null, args: ServerParsedArgs, REMOTE_DATA_FOLDER: string): Promise { +export async function createServer(address: string | net.AddressInfo | null, args: ServerParsedArgs, REMOTE_DATA_FOLDER: string, agentHostBridgeConnectionToken: string | undefined): Promise { const connectionToken = await determineServerConnectionToken(args); if (connectionToken instanceof ServerConnectionTokenParseError) { @@ -661,7 +661,7 @@ export async function createServer(address: string | net.AddressInfo | null, arg }); const disposables = new DisposableStore(); - const { socketServer, instantiationService } = await setupServerServices(connectionToken, args, REMOTE_DATA_FOLDER, disposables); + const { socketServer, instantiationService } = await setupServerServices(connectionToken, args, REMOTE_DATA_FOLDER, agentHostBridgeConnectionToken, disposables); // Set the unexpected error handler after the services have been initialized, to avoid having // the telemetry service overwrite our handler diff --git a/src/vs/server/node/server.main.ts b/src/vs/server/node/server.main.ts index c0ccc85d027a..fde803a0435e 100644 --- a/src/vs/server/node/server.main.ts +++ b/src/vs/server/node/server.main.ts @@ -12,7 +12,7 @@ import { createServer as doCreateServer, IServerAPI } from './remoteExtensionHos import { parseArgs, ErrorReporter } from '../../platform/environment/node/argv.js'; import { join, dirname } from '../../base/common/path.js'; import { performance } from 'perf_hooks'; -import { serverOptions } from './serverEnvironmentService.js'; +import { agentHostBridgeConnectionTokenEnvironmentVariable, serverOptions } from './serverEnvironmentService.js'; import product from '../../platform/product/common/product.js'; import * as perf from '../../base/common/performance.js'; @@ -35,6 +35,8 @@ const errorReporter: ErrorReporter = { }; const args = parseArgs(process.argv.slice(2), serverOptions, errorReporter); +const agentHostBridgeConnectionToken = process.env[agentHostBridgeConnectionTokenEnvironmentVariable]; +delete process.env[agentHostBridgeConnectionTokenEnvironmentVariable]; const REMOTE_DATA_FOLDER = args['server-data-dir'] || process.env['VSCODE_AGENT_FOLDER'] || join(os.homedir(), product.serverDataFolderName || '.vscode-remote'); const USER_DATA_PATH = join(REMOTE_DATA_FOLDER, 'data'); @@ -67,5 +69,5 @@ export function spawnCli() { * invoked by server-main.js */ export function createServer(address: string | net.AddressInfo | null): Promise { - return doCreateServer(address, args, REMOTE_DATA_FOLDER); + return doCreateServer(address, args, REMOTE_DATA_FOLDER, agentHostBridgeConnectionToken); } diff --git a/src/vs/server/node/serverEnvironmentService.ts b/src/vs/server/node/serverEnvironmentService.ts index 4407fcb60667..935468360027 100644 --- a/src/vs/server/node/serverEnvironmentService.ts +++ b/src/vs/server/node/serverEnvironmentService.ts @@ -15,6 +15,22 @@ import { joinPath } from '../../base/common/resources.js'; import { join } from '../../base/common/path.js'; import { ProtocolConstants } from '../../base/parts/ipc/common/ipc.net.js'; +export const agentHostBridgeConnectionTokenEnvironmentVariable = 'VSCODE_AGENT_HOST_BRIDGE_CONNECTION_TOKEN'; + +/** + * Returns server arguments with connection tokens redacted for logging. + */ +export function getRedactedServerParsedArgs(args: ServerParsedArgs): ServerParsedArgs { + const redactedArgs = { ...args }; + if (typeof redactedArgs['connection-token'] !== 'undefined') { + redactedArgs['connection-token'] = ''; + } + if (typeof redactedArgs['agent-host-bridge-connection-token'] !== 'undefined') { + redactedArgs['agent-host-bridge-connection-token'] = ''; + } + return redactedArgs; +} + export const serverOptions: OptionDescriptions> = { /* ----- server setup ----- */ diff --git a/src/vs/server/node/serverServices.ts b/src/vs/server/node/serverServices.ts index cb9e8cdfaa9b..98fba7905261 100644 --- a/src/vs/server/node/serverServices.ts +++ b/src/vs/server/node/serverServices.ts @@ -60,7 +60,7 @@ import { IServerTelemetryService, ServerNullTelemetryService, ServerTelemetrySer import { RemoteTerminalChannel } from './remoteTerminalChannel.js'; import { createURITransformer } from '../../base/common/uriTransformer.js'; import { ServerConnectionToken, ServerConnectionTokenType } from './serverConnectionToken.js'; -import { ServerEnvironmentService, ServerParsedArgs } from './serverEnvironmentService.js'; +import { getRedactedServerParsedArgs, ServerEnvironmentService, ServerParsedArgs } from './serverEnvironmentService.js'; import { REMOTE_TERMINAL_CHANNEL_NAME } from '../../workbench/contrib/terminal/common/remote/remoteTerminalChannel.js'; import { REMOTE_FILE_SYSTEM_CHANNEL_NAME } from '../../workbench/services/remote/common/remoteFileSystemProviderClient.js'; import { ExtensionHostStatusService, IExtensionHostStatusService } from './extensionHostStatusService.js'; @@ -109,7 +109,7 @@ import { SandboxHelperService } from '../../platform/sandbox/node/sandboxHelper. const eventPrefix = 'monacoworkbench'; -export async function setupServerServices(connectionToken: ServerConnectionToken, args: ServerParsedArgs, REMOTE_DATA_FOLDER: string, disposables: DisposableStore) { +export async function setupServerServices(connectionToken: ServerConnectionToken, args: ServerParsedArgs, REMOTE_DATA_FOLDER: string, agentHostBridgeConnectionToken: string | undefined, disposables: DisposableStore) { const services = new ServiceCollection(); const socketServer = new SocketServer(); @@ -131,7 +131,7 @@ export async function setupServerServices(connectionToken: ServerConnectionToken disposables.add(logService.onDidChangeLogLevel(logLevel => log(logService, logLevel, `Log level changed to ${LogLevelToString(logService.getLevel())}`))); logService.trace(`Remote configuration data at ${REMOTE_DATA_FOLDER}`); - logService.trace('process arguments:', environmentService.args); + logService.trace('process arguments:', getRedactedServerParsedArgs(environmentService.args)); if (Array.isArray(productService.serverGreeting)) { logService.info(`\n\n${productService.serverGreeting.join('\n')}\n\n`); } @@ -292,6 +292,7 @@ export async function setupServerServices(connectionToken: ServerConnectionToken const bridgePath = args['agent-host-bridge-path'] ?? spawnPath; const bridgeHost = args['agent-host-bridge-host'] ?? args.host ?? 'localhost'; const bridgeToken = args['agent-host-bridge-connection-token'] + ?? agentHostBridgeConnectionToken ?? ((bridgePort || bridgePath) && connectionToken.type === ServerConnectionTokenType.Mandatory ? connectionToken.value : undefined); @@ -312,11 +313,11 @@ export async function setupServerServices(connectionToken: ServerConnectionToken socketServer.registerChannel(AgentHostIpcChannels.RemoteProxy, new UnavailableAgentHostChannel()); logService.info(`[AgentHostChannel] Registered unavailable IPC channel '${AgentHostIpcChannels.RemoteProxy}': no --agent-host-bridge-port / --agent-host-bridge-path set.`); } - } else if (args['agent-host-bridge-port'] || args['agent-host-bridge-path'] || args['agent-host-bridge-host'] || args['agent-host-bridge-connection-token']) { + } else if (args['agent-host-bridge-port'] || args['agent-host-bridge-path'] || args['agent-host-bridge-host'] || args['agent-host-bridge-connection-token'] || agentHostBridgeConnectionToken) { const bridgePort = args['agent-host-bridge-port']; const bridgePath = args['agent-host-bridge-path']; const bridgeHost = args['agent-host-bridge-host'] ?? args.host ?? 'localhost'; - const bridgeToken = args['agent-host-bridge-connection-token']; + const bridgeToken = args['agent-host-bridge-connection-token'] ?? agentHostBridgeConnectionToken; if (bridgePort || bridgePath) { const agentHostBridge = disposables.add(new AgentHostChannel( socketServer, diff --git a/src/vs/server/test/node/agentHostChannel.test.ts b/src/vs/server/test/node/agentHostChannel.test.ts index 755f0f35e281..905ef1128609 100644 --- a/src/vs/server/test/node/agentHostChannel.test.ts +++ b/src/vs/server/test/node/agentHostChannel.test.ts @@ -11,6 +11,14 @@ import type { Client, IPCServer } from '../../../base/parts/ipc/common/ipc.js'; import { NullLogService } from '../../../platform/log/common/log.js'; import { AgentHostChannel, IAgentHostUpstreamEndpoint, IUpstreamConnection, UnavailableAgentHostChannel } from '../../node/agentHostChannel.js'; +class TestLogService extends NullLogService { + readonly infos: string[] = []; + + override info(message: string, ...args: unknown[]): void { + this.infos.push([message, ...args].join(' ')); + } +} + class FakeUpstream extends Disposable implements IUpstreamConnection { private readonly _onFrame = this._register(new Emitter()); readonly onFrame: Event = this._onFrame.event; @@ -148,6 +156,27 @@ suite('AgentHostChannel', () => { assert.strictEqual(resolveCount, 1); }); + test('does not log the upstream connection token', async () => { + const ipc = ds.add(new FakeIPCServer()); + const logService = new TestLogService(); + const channel = ds.add(new AgentHostChannel( + ipc as unknown as IPCServer, + { host: 'localhost', port: '12345', connectionToken: 'secret-token' }, + logService, + () => ds.add(new FakeUpstream()), + )); + + channel.listen('renderer', 'frame'); + assert.deepStrictEqual(logService.infos, []); + + await channel.call('renderer', 'connect'); + + assert.deepStrictEqual(logService.infos, [ + '[AgentHostChannel] Renderer ctx=renderer requested connect to upstream', + '[AgentHostChannel] Opening upstream to localhost:12345', + ]); + }); + test('shares deferred endpoint resolution between renderer contexts', async () => { const ipc = ds.add(new FakeIPCServer()); let resolveCount = 0; diff --git a/src/vs/server/test/node/serverConnectionToken.test.ts b/src/vs/server/test/node/serverConnectionToken.test.ts index e9affb736ead..50c199c2412e 100644 --- a/src/vs/server/test/node/serverConnectionToken.test.ts +++ b/src/vs/server/test/node/serverConnectionToken.test.ts @@ -11,7 +11,7 @@ import { connectionTokenCookieName, connectionTokenQueryName } from '../../../ba import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../base/test/common/utils.js'; import { getRandomTestPath } from '../../../base/test/node/testUtils.js'; import { MandatoryServerConnectionToken, parseServerConnectionToken, requestHasValidConnectionToken, ServerConnectionToken, ServerConnectionTokenParseError, ServerConnectionTokenType } from '../../node/serverConnectionToken.js'; -import { ServerParsedArgs } from '../../node/serverEnvironmentService.js'; +import { getRedactedServerParsedArgs, ServerParsedArgs } from '../../node/serverEnvironmentService.js'; suite('parseServerConnectionToken', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -95,3 +95,31 @@ suite('requestHasValidConnectionToken', () => { assert.strictEqual(requestHasValidConnectionToken(connectionToken, { headers }, new URLSearchParams()), true); }); }); + +suite('getRedactedServerParsedArgs', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('redacts connection tokens without changing the original arguments', () => { + const args = { + 'connection-token': 'server-token', + 'agent-host-bridge-connection-token': 'bridge-token', + 'agent-host-bridge-port': '9000', + } as ServerParsedArgs; + + assert.deepStrictEqual({ + redactedArgs: getRedactedServerParsedArgs(args), + args, + }, { + redactedArgs: { + 'connection-token': '', + 'agent-host-bridge-connection-token': '', + 'agent-host-bridge-port': '9000', + }, + args: { + 'connection-token': 'server-token', + 'agent-host-bridge-connection-token': 'bridge-token', + 'agent-host-bridge-port': '9000', + }, + }); + }); +});