Multitenancy hardening: Client Mode#1428
Conversation
This comment has been minimized.
This comment has been minimized.
7ee91d6 to
a41b260
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
159647a to
7a4f8b5
Compare
This comment has been minimized.
This comment has been minimized.
9bf763f to
d58befa
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| foreach (var entry in list) | ||
| { | ||
| if (entry == "*") | ||
| { | ||
| throw new ArgumentException( | ||
| $"Invalid {field} entry '*': there is no bare wildcard. " + | ||
| "Use `new ToolSet().AddBuiltIn(\"*\")`, `.AddMcp(\"*\")`, or " + | ||
| "`.AddCustom(\"*\")` to target a specific source.", | ||
| nameof(list)); | ||
| } | ||
| } |
This comment has been minimized.
This comment has been minimized.
| [Fact] | ||
| public void CopilotClient_Mode_Empty_Accepts_Base_Directory() | ||
| { | ||
| var dir = Path.Combine(Path.GetTempPath(), "copilot-empty-mode-test-" + Guid.NewGuid().ToString("N")); |
d11f417 to
f61b858
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “empty” client mode across SDKs to support multi-tenant / untrusted-prompt hosts by forcing explicit tool allowlisting, stripping environment context from system prompts by default, and applying additional safe session options after create/resume.
Changes:
- Introduces
mode="empty"/ClientMode::Empty/ModeEmpty/CopilotClientMode.Emptyand enforces extra validation (explicit storage + explicitavailableTools). - Adds
ToolSetbuilders and “isolated” built-in tool allowlists, plus tool-filter validation (reject bare"*"). - Adds E2E + unit coverage across Node/Python/Go/Rust/.NET and shared CAPI proxy snapshots; updates wire payloads to send
toolFilterPrecedence="excluded".
Show a summary per file
| File | Description |
|---|---|
| test/snapshots/mode_empty/empty_mode_system_message_replace_llm_follows_caller_content_verbatim.yaml | New empty-mode E2E cassette for systemMessage: replace. |
| test/snapshots/mode_empty/empty_mode_strips_environment_context_from_the_system_message_by_default.yaml | New empty-mode E2E cassette verifying env-context stripping. |
| test/snapshots/mode_empty/empty_mode_isolated_set_shell_tool_is_not_exposed.yaml | New empty-mode E2E cassette for isolated built-in tool set. |
| test/snapshots/mode_empty/empty_mode_excluded_tools_subtracts_from_available_tools.yaml | New empty-mode E2E cassette for allowlist+denylist composition. |
| test/snapshots/mode_empty/empty_mode_builtin_star_exposes_all_built_in_tools.yaml | New empty-mode E2E cassette for builtin:*. |
| test/snapshots/mode_empty/empty_mode_append_caller_instruction_takes_effect_and_env_context_stripped.yaml | New empty-mode E2E cassette for append→customize promotion + env stripping. |
| rust/tests/e2e/mode_empty.rs | Rust E2E coverage mirroring Node cassettes. |
| rust/tests/e2e.rs | Registers the new Rust E2E module. |
| rust/src/wire.rs | Adds toolFilterPrecedence to create/resume wire payloads. |
| rust/src/types.rs | Adds session option fields and ensures wire payload emits deny-wins precedence. |
| rust/src/session.rs | Enforces empty-mode requirements, strips env context, defaults telemetry off, applies post-create option patch. |
| rust/src/mode.rs | New Rust mode + ToolSet + isolated allowlist + validation helpers. |
| rust/src/lib.rs | Exposes mode module + re-exports ClientMode, ToolSet, BUILTIN_TOOLS_ISOLATED; adds client option mode. |
| rust/src/generated/api_types.rs | Schema/codegen updates (incl. options update enum + MCP auth config reshaping). |
| python/test_tool_set.py | Python unit tests for ToolSet + empty-mode helpers. |
| python/e2e/test_mode_empty_e2e.py | Python E2E tests sharing the same recorded snapshots. |
| python/copilot/client.py | Adds mode, ToolSet support for tool filters, empty-mode defaults, and post-create patching. |
| python/copilot/_mode.py | Implements Python ToolSet + isolated list + empty-mode validation/defaulting helpers. |
| python/copilot/init.py | Re-exports ToolSet / mode types / isolated list. |
| nodejs/test/toolSet.test.ts | Node unit tests for ToolSet, empty-mode validation/defaulting, and wire normalization. |
| nodejs/test/e2e/mode_empty.e2e.test.ts | Node E2E tests for empty-mode behavior + tool exposure + env stripping. |
| nodejs/src/types.ts | Adds CopilotClientMode, ToolSet-typed tool filters, and new session option flags. |
| nodejs/src/toolSet.ts | Adds ToolSet builder and BuiltInTools.Isolated curated set. |
| nodejs/src/index.ts | Exports ToolSet + BuiltInTools; exports CopilotClientMode type. |
| nodejs/src/generated/rpc.ts | Schema/codegen updates (incl. toolFilterPrecedence type + MCP auth config reshaping). |
| nodejs/src/client.ts | Implements empty-mode validation/defaulting, ToolSet normalization, toolFilterPrecedence, and post-create option patching. |
| nodejs/package.json | Bumps @github/copilot dependency. |
| nodejs/package-lock.json | Lockfile update for @github/copilot bump. |
| go/types.go | Adds ClientOptions.Mode and new session option flags; wires toolFilterPrecedence + related fields. |
| go/toolset.go | Adds Go ClientMode, ToolSet builder, and isolated built-in tool list. |
| go/toolset_test.go | Go unit tests for ToolSet + empty-mode helpers. |
| go/rpc/zrpc.go | Generated RPC updates for MCP auth config + toolFilterPrecedence enum. |
| go/rpc/zrpc_encoding.go | Encoding updates for MCP auth config union decoding. |
| go/mode_empty.go | Implements Go empty-mode validation/defaulting and post-create options patch. |
| go/internal/e2e/mode_empty_e2e_test.go | Go E2E tests sharing recorded snapshots. |
| go/client.go | Hooks empty-mode defaults/validation into create/resume + CLI env updates (disable keytar). |
| dotnet/test/Unit/ToolSetTests.cs | .NET unit coverage for ToolSet and empty-mode construction constraints. |
| dotnet/test/E2E/ModeEmptyE2ETests.cs | .NET E2E coverage mirroring other SDKs and recorded snapshots. |
| dotnet/src/Types.cs | Adds CopilotClientMode + new session option flags and docs. |
| dotnet/src/ToolSet.cs | Adds .NET ToolSet builder + BuiltInTools.Isolated. |
| dotnet/src/Generated/Rpc.cs | Generated RPC updates for toolFilterPrecedence option. |
| dotnet/src/Client.cs | Implements empty-mode validation/defaulting, tool filter validation, toolFilterPrecedence, and post-create option patching. |
Copilot's findings
Files not reviewed (3)
- go/rpc/zrpc.go: Language not supported
- go/rpc/zrpc_encoding.go: Language not supported
- nodejs/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)
dotnet/src/Client.cs:855
- If
UpdateSessionOptionsForModeAsyncthrows aftersession.createsucceeds, the catch block only callsRemoveFromClient()(unregisters locally) and does not sendsession.destroyto the runtime. In empty mode this can leave a running session with defaults the SDK failed to harden. Consider best-effortawait session.DisposeAsync()/session.destroyin the failure path once the session has been created/resumed.
session.WorkspacePath = response.WorkspacePath;
session.SetCapabilities(response.Capabilities);
session.SetOpenCanvases(response.OpenCanvases);
await UpdateSessionOptionsForModeAsync(session, config, cancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
session.RemoveFromClient();
if (ex is not OperationCanceledException)
{
LoggingHelpers.LogTiming(_logger, LogLevel.Warning, ex,
"CopilotClient.CreateSessionAsync failed. Elapsed={Elapsed}, SessionId={SessionId}",
totalTimestamp,
sessionId);
}
throw;
dotnet/src/Client.cs:1030
ResumeSessionAsynchas the same failure-mode asCreateSessionAsync: ifUpdateSessionOptionsForModeAsync(or any later step) throws after the resume RPC succeeds, the catch path only unregisters locally and does not sendsession.destroyto the runtime. Consider best-effort disposing/destroying the runtime session in this failure path, especially for empty mode hardening.
await UpdateSessionOptionsForModeAsync(session, config, cancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
session.RemoveFromClient();
if (ex is not OperationCanceledException)
{
LoggingHelpers.LogTiming(_logger, LogLevel.Warning, ex,
"CopilotClient.ResumeSessionAsync failed. Elapsed={Elapsed}, SessionId={SessionId}",
totalTimestamp,
sessionId);
}
throw;
- Files reviewed: 36/43 changed files
- Comments generated: 6
This comment has been minimized.
This comment has been minimized.
Adds Node SDK surface for the multitenancy hardening work in github/copilot-agent-runtime#7155 (runtime PR #8760). - New `mode: "empty" | "copilot-cli"` on CopilotClientOptions; empty mode requires baseDirectory or sessionFs and rejects sessions without explicit availableTools. - New ToolSet builder + BuiltInTools.Isolated constant for ergonomic, source-qualified tool patterns (builtin:*, mcp:*, custom:*). - availableTools / excludedTools now accept ToolSet or string[]; bare "*" is rejected with a clear error pointing at the source-qualified forms. - New toolFilterMode option ("allowPrecedence" | "denyPrecedence"); empty mode defaults to denyPrecedence so apps can compose include+exclude. - Unit tests (18) and e2e tests (3) including recorded CapiProxy snapshots. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The SDK no longer exposes 'toolFilterMode'. Every session.create / session.resume request now sends toolFilterMode: 'denyPrecedence' unconditionally, so SDK callers always get composable include+exclude semantics (a tool is enabled when it matches availableTools — or availableTools is unset — AND it does not match excludedTools). Allowlist-precedence remains available on the runtime side as a CLI-only concession to legacy behavior; SDK consumers don't need it and the toggle was just extra surface area. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…recedence -> excluded Mirrors the rename landed in the runtime PR. Also regenerates rpc.ts to pick up the new toolFilterPrecedence field on SessionUpdateOptionsParams, and renames the corresponding E2E capture snapshot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
Three deterministic tests using sendAndWait + element-name instructions: - default: env_context stripped (ARGON) - replace: caller content used verbatim (KRYPTON) - append: caller instruction applied and env_context still stripped (XENON) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…KEYTAR in empty mode - Add skipCustomInstructions, customAgentsLocalOnly, coauthorEnabled, manageScheduleEnabled to SessionConfigBase. App-supplied values now win over the empty-mode defaults that are otherwise forced via the post-create session.options.update patch. Also forwarded in copilot-cli mode when the app sets them. - Set COPILOT_DISABLE_KEYTAR=1 on the runtime spawn env when mode === 'empty', so the runtime skips the process-wide system keychain (unsafe for multi-tenant hosts) and falls back to file-based credentials scoped to COPILOT_HOME. - 31/31 unit tests passing (2 new covering the override and copilot-cli forwarding paths). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port of nodejs/src/client.ts mode=empty work (commits 78e7280..61f8694) to the .NET SDK. - Add CopilotClientMode enum (Empty, CopilotCli) and Mode field on CopilotClientOptions; validation in constructor requires BaseDirectory, SessionFs, or UriRuntimeConnection when Mode=Empty. - Add ToolSet builder and BuiltInTools.Isolated curated set in dotnet/src/ToolSet.cs. ToolSet inherits from List<string> so instances can be assigned directly to AvailableTools/ExcludedTools. - Tool-filter resolution always emits toolFilterPrecedence=excluded on the wire (CreateSessionRequest/ResumeSessionRequest). Bare "*" rejected. Empty mode requires AvailableTools. - Empty-mode safe defaults applied in Create/Resume: - environment_context stripped from system message - EnableSessionTelemetry=false (caller wins) - post-create session.options.update patch sets installedPlugins=[] plus 4 opt-back-in flags (SkipCustomInstructions=true, CustomAgentsLocalOnly=true, CoauthorEnabled=false, ManageScheduleEnabled=false), caller wins - COPILOT_DISABLE_KEYTAR=1 env in spawned CLI - Add SessionConfigBase fields for the 4 opt-back-in flags. - Add E2E tests at dotnet/test/E2E/ModeEmptyE2ETests.cs (6 tests) sharing recorded cassettes with the Node SDK under test/snapshots/mode_empty/. - Regenerate dotnet/src/Generated/Rpc.cs against runtime branch schema to expose OptionsUpdateToolFilterPrecedence. - Tidy: rename existing mode_empty cassettes to clean snake_case names shared across all language SDKs; update Node test titles to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mirrors the C#/Node Mode=Empty implementation: - ClientMode (ModeEmpty/ModeCopilotCli) + Mode field on ClientOptions - ToolSet builder with AddBuiltIn/AddMcp/AddCustom + BuiltInToolsIsolated - Tool-name charset validation (panics on /[^a-zA-Z0-9_-]/) - NewClient validation: empty mode requires BaseDirectory, SessionFs, or UriConnection - 4 opt-back-in fields on SessionConfig and ResumeSessionConfig (SkipCustomInstructions, CustomAgentsLocalOnly, CoauthorEnabled, ManageScheduleEnabled) - ToolFilterPrecedence on createSessionRequest/resumeSessionRequest - Mode helpers: resolveToolFilterOptions, systemMessageForMode (strips environment_context), applyConfigDefaultsForMode (telemetry off), updateSessionOptionsForMode (post-create patch) - COPILOT_DISABLE_KEYTAR=1 in spawned runtime env when Mode=ModeEmpty - toolset_test.go unit tests (16/16 passing) - mode_empty_e2e_test.go reuses the shared test/snapshots/mode_empty/ cassettes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…aults) Mirrors the Node implementation from the same PR: - CopilotClientMode = Literal["copilot-cli", "empty"], default "copilot-cli" - ToolSet builder with add_builtin / add_mcp / add_custom; entries are source-qualified strings (builtin:*/mcp:*/custom:*) — no bare wildcard - BUILTIN_TOOLS_ISOLATED — built-ins safe for single-session, no-host-state contexts (ask_user, task_complete, exit_plan_mode, subagent helpers, …) - _CopilotClientOptions.mode + CopilotClient(mode=...) kwarg - Empty mode validates base_directory or session_fs or URI connection at construction time - create_session / resume_session: validate available_tools is set in empty mode; reject bare "*"; normalize ToolSet -> list[str]; transform system_message to strip environment_context; default enable_session_telemetry to False; always emit toolFilterPrecedence "excluded" - 4 opt-back-in SessionConfig fields: skip_custom_instructions, custom_agents_local_only, coauthor_enabled, manage_schedule_enabled — applied via session.options.update after create/resume. installedPlugins is forced to [] in empty mode. Failure to apply the patch tears the session down so empty-mode callers never end up with a permissive session. - Runtime spawn env: COPILOT_DISABLE_KEYTAR=1 when mode="empty" - Exports new symbols from `copilot` package - 40 unit tests in python/test_tool_set.py covering the builder and helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ports the empty-mode SDK feature from Node (commits 78e7280..61f8694) to the Rust SDK: - Add `tool_filter_precedence: "excluded"` to SessionCreate/SessionResume wire payloads (always sent, matching Node). - `create_session` / `resume_session`: - Reject empty mode when `available_tools` is unset. - Validate `available_tools` / `excluded_tools` reject bare "*". - Apply `system_message_for_mode` to strip `environment_context` in empty mode unless the caller has already overridden it. - Default `enable_session_telemetry = false` in empty mode when unset. - After session creation succeeds, send a `session.options.update` patch with safe defaults (skipCustomInstructions=true, customAgentsLocalOnly=true, coauthorEnabled=false, manageScheduleEnabled=false, installedPlugins=[]). If the patch fails, disconnect the session and propagate the error. - In copilot-cli mode, the same patch is sent only for fields the caller explicitly provided on the SessionConfig. - Add `with_*` builders on SessionConfig and ResumeSessionConfig for the four opt-back-in fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CI runs `cargo +nightly-2026-04-14 fmt --all -- --config-path .rustfmt.nightly.toml` after codegen. Local codegen only invoked stable rustfmt, leaving these 3 files with unconsolidated imports that nightly rustfmt collapses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- go/types.go: gofmt re-aligns struct fields after adding ToolFilterPrecedence field with a longer type name. - python: ruff format/lint --fix on mode_empty test, _mode, client. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- nodejs/dotnet/go: best-effort disconnect/destroy session when the post-create options.update RPC fails so empty mode never leaves a runtime session alive with permissive defaults. Logic lives inside updateSessionOptionsForMode so call sites stay one line. - python: same cleanup already happens; harden it so a failing disconnect doesn't mask the original error. - python _normalize_tool_filter: reject bare str so passing 'builtin:bash' fails fast instead of silently splitting into chars. - nodejs client.ts: fix doc comment claiming append-mode is rejected in empty mode (we promote it to customize). - go toolset.go: drop misleading 'implicit conversion' wording. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| self._sessions.pop(session.session_id, None) | ||
| try: | ||
| await session.disconnect() | ||
| except BaseException: |
| catch | ||
| { | ||
| // Swallow: original error is what the caller needs. | ||
| } |
6fc1c7d to
aedf2c3
Compare
Cross-SDK Consistency Review ✅ (with one gap noted)The PR adds the Client Mode /
Java SDK gapThe Java SDK (
This is already tracked as a follow-up checkbox in the PR description, so no action is required before merging — just flagging for completeness. When the Java implementation lands, the equivalent API would be: // New: ClientMode enum
public enum ClientMode { COPILOT_CLI, EMPTY }
// New: CopilotClientOptions.setMode(ClientMode)
var client = new CopilotClient(new CopilotClientOptions()
.setMode(ClientMode.EMPTY)
.setBaseDirectory("/srv/agents"));
// New: ToolSet builder + BuiltinTools constants
var session = client.createSession(new SessionConfig()
.setOnPermissionRequest(PermissionHandler.APPROVE_ALL)
.setAvailableTools(new ToolSet()
.addBuiltIn(BuiltinTools.ISOLATED)
.addMcp("*"))
.setExcludedTools(List.of("mcp:github-delete_repository"))).get();No blocking issues for this PR — the five SDKs that are updated are consistent with each other.
|
Why
By default,
CopilotClientgives every session the full Copilot CLI experience: every built-in tool, every host-side capability, user files like AGENTS.md / co-author trailers / plugins / custom agents, and a system prompt full of environment context.That is great for an IDE plugin, but unsafe for a multi-tenant app where each session may belong to a different end user. There is no single switch to opt out of the host integration and start from a clean slate.
What
Add
CopilotClientMode.Empty(TSmode: "empty"). When set, the SDK flips the host-integration defaults to safe values and requires every session to declare its tool surface up front viaavailableTools. The runtime stays mode-agnostic; this is a thin SDK-side translation.Empty mode contract
availableToolsis required on every session; pass aToolSet(orstring[]).excludedToolscomposes with it via deny-precedence so callers can write "everything matching X except Y".environment_contextsection (working directory, OS, etc.) is stripped unless the caller explicitly opts in.baseDirectory(orsessionFs) must be set so session data doesn't leak into~/.copilotby mistakeTypeScript
C#
Coverage
TypeScript, Python, Go, C#, and Rust SDKs. Mirrored E2E tests across all five SDKs (shared cassettes under
test/snapshots/mode_empty/) verify the SDK translation reaches the runtime correctly: the tool list the LLM sees, the stripped system message, and end-to-end behavior on representative prompts.Remaining work
This PR establishes the pattern for setting empty mode, but doesn't implement all the runtime flags for disabling all the things we want to disable in that mode. The remaining parts are tracked in different issue that @MackinnonBuck is handling.