Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 36 additions & 11 deletions dotnet/src/Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,22 @@ async Task<Connection> StartCoreAsync(CancellationToken ct)
{
// In-process FFI hosting: load the Rust cdylib and let it spawn
// the CLI worker, instead of the SDK launching a CLI child process.
var ffiHost = FfiRuntimeHost.Create(ResolveCliPathForFfi(), GetNapiPrebuildsFolderOrThrow(), _options.Environment, _logger);
// The worker reads its configuration (telemetry export, etc.) from
// the environment passed here, so apply the same telemetry-derived
// vars the child-process path sets on its startInfo.Environment.
var ffiEnvironment = new Dictionary<string, string?>();
if (_options.Environment is not null)
{
foreach (var kvp in _options.Environment)
{
ffiEnvironment[kvp.Key] = kvp.Value;
}
}
ApplyTelemetryEnvironment(ffiEnvironment, _options.Telemetry);
var resolvedFfiEnvironment = ffiEnvironment
.Where(kvp => kvp.Value is not null)
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value!);
var ffiHost = FfiRuntimeHost.Create(ResolveCliPathForFfi(), GetNapiPrebuildsFolderOrThrow(), resolvedFfiEnvironment, _logger);
_ffiHost = ffiHost;
await ffiHost.StartAsync(ct);
connection = await ConnectToServerAsync(null, null, null, null, ct, ffiHost);
Expand Down Expand Up @@ -1909,6 +1924,25 @@ private static bool IsUnsupportedConnectMethod(RemoteRpcException ex)
|| string.Equals(ex.Message, "Unhandled method connect", StringComparison.Ordinal);
}

// Applies the telemetry-derived environment variables the runtime reads to
// enable OTLP export. Shared by the stdio/tcp child-process path and the
// in-process FFI path so telemetry behaves identically across transports.
private static void ApplyTelemetryEnvironment(IDictionary<string, string?> environment, TelemetryConfig? telemetry)
{
if (telemetry is null)
{
return;
}

environment["COPILOT_OTEL_ENABLED"] = "true";
if (telemetry.OtlpEndpoint is not null) environment["OTEL_EXPORTER_OTLP_ENDPOINT"] = telemetry.OtlpEndpoint;
if (telemetry.OtlpProtocol is not null) environment["OTEL_EXPORTER_OTLP_PROTOCOL"] = telemetry.OtlpProtocol;
if (telemetry.FilePath is not null) environment["COPILOT_OTEL_FILE_EXPORTER_PATH"] = telemetry.FilePath;
if (telemetry.ExporterType is not null) environment["COPILOT_OTEL_EXPORTER_TYPE"] = telemetry.ExporterType;
if (telemetry.SourceName is not null) environment["COPILOT_OTEL_SOURCE_NAME"] = telemetry.SourceName;
if (telemetry.CaptureContent is { } capture) environment["OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT"] = capture ? "true" : "false";
}

private async Task<(Process Process, int? DetectedLocalhostTcpPort, ProcessStderrPump StderrPump)> StartCliServerAsync(CancellationToken cancellationToken)
{
var options = _options;
Expand Down Expand Up @@ -2023,16 +2057,7 @@ private static bool IsUnsupportedConnectMethod(RemoteRpcException ex)
}

// Set telemetry environment variables if configured
if (options.Telemetry is { } telemetry)
{
startInfo.Environment["COPILOT_OTEL_ENABLED"] = "true";
if (telemetry.OtlpEndpoint is not null) startInfo.Environment["OTEL_EXPORTER_OTLP_ENDPOINT"] = telemetry.OtlpEndpoint;
if (telemetry.OtlpProtocol is not null) startInfo.Environment["OTEL_EXPORTER_OTLP_PROTOCOL"] = telemetry.OtlpProtocol;
if (telemetry.FilePath is not null) startInfo.Environment["COPILOT_OTEL_FILE_EXPORTER_PATH"] = telemetry.FilePath;
if (telemetry.ExporterType is not null) startInfo.Environment["COPILOT_OTEL_EXPORTER_TYPE"] = telemetry.ExporterType;
if (telemetry.SourceName is not null) startInfo.Environment["COPILOT_OTEL_SOURCE_NAME"] = telemetry.SourceName;
if (telemetry.CaptureContent is { } capture) startInfo.Environment["OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT"] = capture ? "true" : "false";
}
ApplyTelemetryEnvironment(startInfo.Environment, options.Telemetry);

var cliProcess = new Process { StartInfo = startInfo };
try
Expand Down
167 changes: 161 additions & 6 deletions dotnet/test/Harness/E2ETestContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Extensions.Logging;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text.RegularExpressions;

namespace GitHub.Copilot.Test.Harness;
Expand Down Expand Up @@ -216,6 +217,16 @@

env["GITHUB_TOKEN"] = env["GH_TOKEN"] = DefaultGitHubToken;

// Disable HMAC auth for E2E runs. CI sets COPILOT_HMAC_KEY at the job
// level as an ambient credential, but the replay snapshots are captured
// against Bearer/OAuth (SDK-token) requests. In stdio the SDK token
// outranks HMAC so this is a no-op, but in-process auth resolution runs
// host-side in this process and would otherwise pick HMAC (which ranks
// above the GitHub token) and fail provider.getEndpoint. An empty value
// disables the method (runtime filters out empty HMAC keys).
env["COPILOT_HMAC_KEY"] = "";
env["CAPI_HMAC_KEY"] = "";

return env!;
}

Expand All @@ -226,6 +237,105 @@
: Environment.GetEnvironmentVariable("GITHUB_TOKEN");
}

[DllImport("libc", EntryPoint = "setenv", CharSet = CharSet.Ansi,
BestFitMapping = false, ThrowOnUnmappableChar = true)]
private static extern int NativeSetEnv(string name, string value, int overwrite);

Check notice

Code scanning / CodeQL

Unmanaged code Note test

Minimise the use of unmanaged code.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed

[DllImport("libc", EntryPoint = "unsetenv", CharSet = CharSet.Ansi,
BestFitMapping = false, ThrowOnUnmappableChar = true)]
private static extern int NativeUnsetEnv(string name);

Check notice

Code scanning / CodeQL

Unmanaged code Note test

Minimise the use of unmanaged code.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed

// Records the original process-env value of each variable the in-process
// mirror overwrites, so it can be restored after the test. A null entry
// means the variable was unset. Guarded by _clientsLock.
private readonly Dictionary<string, string?> _mirroredEnvBackup = new();

// Sets an environment variable on both the managed cache and (on Unix) the
// libc environment block, so native getenv/std::env::var readers in the loaded
// cdylib observe it. On Windows the managed setter already reaches native
// GetEnvironmentVariableW, so setenv is not needed.
private static void SetProcessEnvironmentVariable(string name, string value)
{
Environment.SetEnvironmentVariable(name, value);
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
_ = NativeSetEnv(name, value, 1);

Check notice

Code scanning / CodeQL

Calls to unmanaged code Note test

Replace this call with a call to managed code if possible.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
}
}

// Restores (or unsets) an environment variable on both the managed cache and
// (on Unix) the libc environment block.
private static void RestoreProcessEnvironmentVariable(string name, string? value)
{
Environment.SetEnvironmentVariable(name, value);
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
if (value is null)
{
_ = NativeUnsetEnv(name);

Check notice

Code scanning / CodeQL

Calls to unmanaged code Note test

Replace this call with a call to managed code if possible.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
}
else
{
_ = NativeSetEnv(name, value, 1);

Check notice

Code scanning / CodeQL

Calls to unmanaged code Note test

Replace this call with a call to managed code if possible.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
}

Check notice

Code scanning / CodeQL

Missed ternary opportunity Note test

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
}
}

// Mirrors a variable onto the shared process environment for the duration of
// the current test, backing up its original value on first mutation so
// RestoreMirroredEnvironment can undo it afterward. Without this, clearing
// HMAC / redirecting the GitHub API URL for an in-process test would leak into
// later tests (e.g. ClientE2ETests' direct-construction stdio/tcp cases that
// rely on the ambient CI HMAC credential), whose fixtures use a different,
// possibly already-disposed replay proxy.
private void MirrorProcessEnvironmentVariable(string name, string value)
{
lock (_clientsLock)
{
if (!_mirroredEnvBackup.ContainsKey(name))
{
_mirroredEnvBackup[name] = Environment.GetEnvironmentVariable(name);
}
}

SetProcessEnvironmentVariable(name, value);
}

// Restores every process-env variable mutated by the in-process mirror back to
// its pre-test value. Called after each test so cross-test/cross-class env
// pollution cannot occur.
private void RestoreMirroredEnvironment()
{
KeyValuePair<string, string?>[] backup;
lock (_clientsLock)
{
if (_mirroredEnvBackup.Count == 0)
{
return;
}

backup = [.. _mirroredEnvBackup];
_mirroredEnvBackup.Clear();
}

foreach (var (name, value) in backup)
{
RestoreProcessEnvironmentVariable(name, value);
}
}

// Mirrors CopilotClient's default-connection resolution: the no-Connection
// case honors COPILOT_SDK_DEFAULT_CONNECTION (from options.Environment, else
// the process env), defaulting to stdio.
private static bool IsDefaultConnectionInProcess(IReadOnlyDictionary<string, string>? environment)
{
var value = environment is not null
&& environment.TryGetValue("COPILOT_SDK_DEFAULT_CONNECTION", out var fromOptions)
? fromOptions
: Environment.GetEnvironmentVariable("COPILOT_SDK_DEFAULT_CONNECTION");
return string.Equals(value, "inprocess", StringComparison.OrdinalIgnoreCase);
}

public CopilotClient CreateClient(
bool? useStdio = null,
CopilotClientOptions? options = null,
Expand All @@ -238,9 +348,10 @@
options.Environment ??= GetEnvironment();
options.Logger ??= Logger;

// Build the connection. If the caller supplied one, just ensure the runtime path is set;
// otherwise default to Stdio with the bundled runtime (matches CopilotClient's own default).
// useStdio is a convenience shortcut for the no-Connection case; passing both is ambiguous.
// Build the connection. If the caller supplied one, just ensure the runtime path is set.
// When neither a Connection nor useStdio is specified, leave Connection null so
// CopilotClient honors COPILOT_SDK_DEFAULT_CONNECTION (defaulting to stdio); useStdio
// is a convenience shortcut to pin stdio/tcp. Passing both a Connection and useStdio is ambiguous.
if (useStdio is not null && options.Connection is not null)
{
throw new ArgumentException(
Expand All @@ -252,16 +363,57 @@
var cliPath = GetCliPath(_repoRoot);
switch (options.Connection)
{
case null when useStdio == true:
options.Connection = RuntimeConnection.ForStdio(path: cliPath);
break;
case null when useStdio == false:
options.Connection = RuntimeConnection.ForTcp(path: cliPath);
break;
case null:
options.Connection = useStdio == false
? RuntimeConnection.ForTcp(path: cliPath)
: RuntimeConnection.ForStdio(path: cliPath);
// useStdio is null: leave Connection unset so CopilotClient's
// ResolveDefaultConnection honors COPILOT_SDK_DEFAULT_CONNECTION
// (stdio by default, or in-process). The CLI path flows through
// options.Environment["COPILOT_CLI_PATH"] (GetEnvironment copies
// the process env, where CI's setup-copilot sets it).
break;
case ChildProcessRuntimeConnection child when child.Path is null:
child.Path = cliPath;
break;
}

// In-process hosting workaround (applies whenever the in-process FFI
// transport is the default for this run): several runtime code paths run
// host-side in this process (the loaded cdylib) and read the ambient
// process environment rather than the environment passed to
// copilot_runtime_host_start — e.g. native fetch_copilot_user reads
// COPILOT_DEBUG_GITHUB_API_URL via std::env::var, the gh-CLI fallback
// spawns `gh auth token` (inheriting this process's GH_TOKEN /
// GITHUB_TOKEN / GH_CONFIG_DIR), auth-method selection reads the HMAC
// keys, and session state/config reads COPILOT_HOME / XDG_*. So our
// per-test redirects, cleared tokens, cleared HMAC keys, and isolated
// home in options.Environment are invisible to them, and auth either
// escapes the replay proxy (-> 401) or wrongly selects HMAC over the
// GitHub token. Mirror the whole intended test environment onto this
// process's real environment block so every host-side read observes it;
// there is no benefit to a narrower allowlist since in-process mode is
// mutating the shared host env regardless, and RestoreMirroredEnvironment
// reverts every mutation after the test. Gated to the in-process default;
// we deliberately do not account for individual tests that pin a
// non-in-process transport, since those still resolve auth against this
// same mirrored env harmlessly.
// Note .NET's Environment.SetEnvironmentVariable does NOT reach libc
// getenv on Unix, so we also call setenv directly. Safe because E2E tests
// run serially (DisableTestParallelization) and in-process is
// single-runtime-per-process. Remove once the runtime stops relying on
// the ambient process environment for these host-side reads.
if (IsDefaultConnectionInProcess(options.Environment))
{
foreach (var (name, value) in options.Environment)
{
MirrorProcessEnvironmentVariable(name, value);
}
}
Comment on lines +292 to +298

// Auto-inject auth token unless connecting to an existing runtime via URI.
var isExistingRuntime = options.Connection is UriRuntimeConnection;
if (autoInjectGitHubToken
Expand Down Expand Up @@ -320,6 +472,9 @@
}
}

// Undo any in-process env mirroring so it cannot leak into the next test.
RestoreMirroredEnvironment();

if (errors.Count == 1)
{
throw errors[0];
Expand Down
Loading