Skip to content

Add managed permission settings to session startup - #2139

Draft
joshspicer wants to merge 4 commits into
github:mainfrom
joshspicer:joshspicer-sdk-managed-settings-permissions
Draft

Add managed permission settings to session startup#2139
joshspicer wants to merge 4 commits into
github:mainfrom
joshspicer:joshspicer-sdk-managed-settings-permissions

Conversation

@joshspicer

@joshspicer joshspicer commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Why

SDK hosts need a typed, cross-language way to inject enterprise permission policy at session startup, independent of the runtime's server/device managed-settings fetch path.

Public API

Create and resume configuration in Node, Python, Go, .NET, Rust, and Java now expose the same permissions-only object:

managedSettings?: {
  permissions?: {
    disableBypassPermissionsMode?: "disable";
    deny?: string[];
    ask?: string[];
    allow?: string[];
  };
}

Each SDK uses native language naming while serializing the same camelCase JSON. Generated RPC mirrors are updated for Node, Python, Go, Rust, and Java; .NET's generated RPC model does not mirror SessionOpenOptions, so its high-level wire type remains handwritten.

How it works

  • The field is forwarded on both create and resume.
  • enableManagedSettings remains independent and may be combined with direct injection.
  • Supplying managedSettings marks the session managed in every SDK, so permissive built-in handlers such as approveAll cannot bypass enterprise restrictions even when self-fetch is disabled.
  • The layer is startup-only and not persisted. Hosts must re-supply it on resume; omission clears the runtime's prior client layer.
  • Explicit empty arrays are preserved. This matters especially for allow: [], which means no operation is admitted; it is not equivalent to an absent allowlist.
  • Unset fields are omitted rather than serialized as null.
  • The SDK protocol version is unchanged because this is an optional additive field.

Compatibility and rollout

Older runtimes may ignore the additive field. Hosts must not rely on injected policy until they ship a runtime whose schema and enforcement include managedSettings.

This PR should publish only after github/copilot-agent-runtime#14000 is released. Downstream hosts such as VS Code must then bump to the published SDK/runtime pair before removing temporary type shims or enabling enforcement.

Validation

  • Node format/lint/typecheck and focused tests
  • Python Ruff and focused tests
  • Go focused tests and go vet
  • .NET format and focused tests
  • Rust format and focused tests
  • Java Spotless, generated RPC uptake, and focused tests

Copilot AI balanced review requested due to automatic review settings July 29, 2026 19:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (2)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zrpc_encoding.go: Generated file
  • Files reviewed: 25/30 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread go/types.go Outdated
Comment on lines +1470 to +1472
// Allow lists operations permitted without prompting. Every declared allow
// list across managed layers must admit an operation for it to be allowed.
Allow []string `json:"allow,omitempty"`
Copilot AI review requested due to automatic review settings July 31, 2026 22:41
@joshspicer
joshspicer force-pushed the joshspicer-sdk-managed-settings-permissions branch from 135b25a to 3676a65 Compare July 31, 2026 22:41
Comment thread python/copilot/generated/rpc.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (2)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zrpc_encoding.go: Generated file
Suppressed comments (5)

go/types.go:1485

  • An explicitly empty allow list is not equivalent to omitting allow: by this field's own contract, every present allow-list must admit an operation, so [] is a deny-all constraint. omitempty drops that value and therefore removes the host's restriction, weakening the managed policy. Preserve the nil-versus-empty distinction (for example with a pointer slice or custom marshaling) and update the test that currently asserts omission.
	Allow []string `json:"allow,omitempty"`

rust/src/types.rs:1763

  • This public field documents a single legal literal but accepts and serializes any string. That defeats the typed contract and lets invalid policy reach the runtime. Use a public enum for disable (the generated protocol already defines DisableBypassPermissionsMode) rather than String.
    pub disable_bypass_permissions_mode: Option<String>,

go/types.go:1478

  • The contract permits only the "disable" literal, but *string accepts and forwards arbitrary values. Expose a dedicated typed value/constant (the generated RPC package already has rpc.DisableBypassPermissionsMode) so callers cannot accidentally construct an invalid managed policy.
	DisableBypassPermissionsMode *string `json:"disableBypassPermissionsMode,omitempty"`

java/src/main/java/com/github/copilot/rpc/SessionConfig.java:108

  • The Java generated RPC surface was not regenerated for this schema addition. CopilotClient.getRpc().sessions.open(...) publicly consumes generated SessionOpenOptions, but that record still has no managedSettings field or generated managed-settings types, so this feature is unavailable through Java's typed RPC API while the other generated mirrors include it. Regenerate Java RPC sources from the updated schema rather than hand-editing them.
    private ManagedSettings managedSettings;

dotnet/src/Types.cs:3012

  • This property claims a single legal "disable" value but is an unrestricted string, so the new typed API accepts invalid policy and only fails later at runtime. Model it as a serialized enum/value type, consistent with other closed string-valued options in Types.cs.
    [JsonPropertyName("disableBypassPermissionsMode")]
    public string? DisableBypassPermissionsMode { get; set; }
  • Files reviewed: 25/30 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread go/rpc/zrpc.go
type SessionManagedPermissions struct {
// Permission rules that allow matching operations unless another managed source, deny, or
// ask rule restricts them.
Allow []string `json:"allow,omitzero"`
Comment thread go/client_test.go Outdated
Comment on lines +3439 to +3441
t.Run("omits empty permission arrays (omitempty idiom)", func(t *testing.T) {
// Go's `omitempty` drops both nil and empty slices; an empty rule list
// is semantically equivalent to no rules for that key.
Copilot AI review requested due to automatic review settings August 3, 2026 15:16
@joshspicer
joshspicer force-pushed the joshspicer-sdk-managed-settings-permissions branch from 3676a65 to 5411c88 Compare August 3, 2026 15:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (2)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zrpc_encoding.go: Generated file
Suppressed comments (7)

rust/src/wire.rs:331

  • This optional field is serialized as "managedSettings": null on every resume when unset because it is missing the neighboring skip_serializing_if attribute. That violates the startup option's omission contract and may cause schema-validating runtimes to reject normal resumes.
    pub managed_settings: Option<crate::types::ManagedSettings>,

go/types.go:1489

  • omitempty drops a non-nil empty Allow slice, but these states are not equivalent: an explicitly present empty allow list admits no operation under the documented intersection semantics, while an omitted list imposes no constraint. This can silently broaden an injected enterprise policy. Preserve non-nil empty slices (for example with Go 1.24's omitzero, as the generated RPC type does) and update the serialization test accordingly.
	Allow []string `json:"allow,omitempty"`

go/client_test.go:3507

  • This test codifies an unsafe equivalence for allow: an explicit empty allow list admits nothing, whereas omitting allow contributes no restriction. Update the test to require "allow": [] after changing serialization to preserve non-nil empty slices.
		// Go's `omitempty` drops both nil and empty slices; an empty rule list
		// is semantically equivalent to no rules for that key.

java/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java:69

  • @return is currently parsed as part of the rules parameter text rather than as a Javadoc block tag. Move it to a separate line.
     *            ask rules; @return this policy

java/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java:83

  • @return is embedded in the parameter description, leaving the fluent setter's return value undocumented in generated Javadoc. Use a separate block tag.
     *            allow rules; @return this policy

java/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java:55

  • @return is embedded in the @param description, so generated Javadoc does not document the method's return value. Put it on its own block-tag line.

This issue also appears in the following locations of the same file:

  • line 69
  • line 83
     *            deny rules; @return this policy

java/src/main/java/com/github/copilot/rpc/ManagedSettings.java:27

  • The inline @return text is part of the parameter description, not a Javadoc return tag. Split it onto its own block-tag line so the public fluent API is documented correctly.
     *            managed permission policy; @return this settings object
  • Files reviewed: 26/31 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread rust/src/wire.rs
pub enable_managed_settings: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub is_experimental_mode: Option<bool>,
pub managed_settings: Option<crate::types::ManagedSettings>,
Copilot AI review requested due to automatic review settings August 4, 2026 16:24
@joshspicer
joshspicer force-pushed the joshspicer-sdk-managed-settings-permissions branch from 5411c88 to d599be9 Compare August 4, 2026 16:24
joshspicer and others added 4 commits August 4, 2026 09:25
…/resume

Add an optional per-session `managedSettings` field (permissions-only
contract) across all six language SDKs, alongside the existing
`enableManagedSettings` boolean. Hosts can inject enterprise permission
policy at session startup via:

  managedSettings.permissions = {
    disableBypassPermissionsMode?: "disable",
    deny?: string[],
    ask?: string[],
    allow?: string[],
  }

Semantics: startup-only (not persisted), must be re-supplied on resume,
composes restrictively with runtime-managed settings, and older runtimes
fail closed. Wired through hand-written wire types at both create and
resume in Node, Python, Go, .NET, Rust, and Java, plus tests, docs, and
a CHANGELOG entry. Generated RPC mirror types regenerated from the
runtime schema (TS/Python/Go/Rust; C# unaffected as it does not mirror
SessionOpenOptions). No SDK protocol bump.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Treat direct managedSettings injection as a managed session in every language SDK and document the compatible-runtime requirement.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d5d4d699-33e2-4a55-9d48-57d2e483dd3d
Restore the managed-settings RPC definitions after rebasing onto the latest generated schema.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d5d4d699-33e2-4a55-9d48-57d2e483dd3d
Keep explicit empty Go rule arrays, omit unset Rust settings, and expose managed settings through the generated Java RPC surface.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d5d4d699-33e2-4a55-9d48-57d2e483dd3d
@joshspicer
joshspicer force-pushed the joshspicer-sdk-managed-settings-permissions branch from d599be9 to a078848 Compare August 4, 2026 16:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (2)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zrpc_encoding.go: Generated file
Suppressed comments (7)

rust/src/types.rs:1786

  • ManagedSettings explicitly documents that it “currently” contains only permissions, so new managed-setting sections are expected. Without #[non_exhaustive], adding the next section will break downstream Rust struct literals; this also differs from the extensible public config structs elsewhere in this file (for example GitHubMcpToolConfig and SessionConfig).
    rust/src/types.rs:1758
  • This new public policy struct is intended to evolve, but unlike the repository's other extensible Rust configuration types (for example Tool, GitHubMcpToolConfig, and SessionConfig), it is exhaustive. Adding another permission field later would therefore be a source-breaking change for downstream struct literals. Mark it #[non_exhaustive] before publishing the type.

This issue also appears on line 1786 of the same file.
rust/src/types.rs:1763

  • The wire contract permits only the literal "disable", but this public field accepts any string, so invalid policy values compile and fail only when starting a session. Model this as a single-variant serialized enum (as the generated DisableBypassPermissionsMode type does) so the SDK API cannot construct unsupported values.
    go/types.go:1485
  • The contract accepts only "disable", but *string allows callers to send arbitrary values and discover the error only at session startup. Use a named string type with a DisableBypassPermissionsModeDisable constant, consistent with SectionOverrideAction and ToolDefer, so the public API exposes the supported value explicitly.
	DisableBypassPermissionsMode *string `json:"disableBypassPermissionsMode,omitempty"`

dotnet/src/Types.cs:3012

  • This is a single-literal protocol field, but exposing it as string permits unsupported values that fail only when the runtime validates session startup. Use a JSON string enum containing Disable, as done for other constrained SDK options such as CopilotToolDefer, to keep invalid policy values out of the public API.
    [JsonPropertyName("disableBypassPermissionsMode")]
    public string? DisableBypassPermissionsMode { get; set; }

dotnet/test/Unit/ClientSessionLifetimeTests.cs:511

  • This test reaches into a private field by reflection, coupling it to an implementation detail rather than the SDK's public behavior. Exercise a permission request through the configured handler and assert the public PermissionInvocation.ManagedSettingsEnabled value instead, so the test remains valid if session internals are refactored.
        var managedField = typeof(CopilotSession).GetField("_managedSettingsEnabled", BindingFlags.Instance | BindingFlags.NonPublic)
            ?? throw new InvalidOperationException("Managed settings field was not found.");
        Assert.True((bool)managedField.GetValue(session)!);

java/src/test/java/com/github/copilot/ManagedSettingsTest.java:54

  • This test bypasses the public API with getDeclaredField/setAccessible, making it depend on the private field name. Verify the managed flag through the public permission-handler invocation context instead; that tests the externally observable safeguard and avoids reflection-based access to session internals.
        var field = CopilotSession.class.getDeclaredField("managedSettingsEnabled");
        field.setAccessible(true);
        assertEquals(true, field.getBoolean(session));
  • Files reviewed: 26/35 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI review requested due to automatic review settings August 4, 2026 16:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (2)
  • go/rpc/zrpc.go: Generated file
  • go/rpc/zrpc_encoding.go: Generated file
Suppressed comments (14)

java/src/test/java/com/github/copilot/ManagedSettingsTest.java:54

  • This test reaches package-private configuration code and then reflects into a private session field. Verify the safeguard through the public permission-handler behavior instead; otherwise internal refactors can break the test without changing the API contract.
        SessionRequestBuilder.configureSession(session, new SessionConfig().setManagedSettings(settings));

        var field = CopilotSession.class.getDeclaredField("managedSettingsEnabled");
        field.setAccessible(true);
        assertEquals(true, field.getBoolean(session));

rust/src/types.rs:1758

  • This public struct is exhaustive even though the permission contract is explicitly additive. Adding another permission field later would break downstream struct literals and exhaustive patterns; mark it #[non_exhaustive] now while the type is new.
    rust/src/types.rs:1763
  • The public contract restricts this value to the literal "disable", but Option<String> accepts and serializes any value. Use a dedicated enum/newtype so invalid policies are rejected by the Rust type system rather than only by the runtime.
    dotnet/src/Types.cs:3012
  • The property accepts any string although the contract has a single "disable" value. Model it as a nullable JSON string enum, consistent with the enum-backed mode properties elsewhere in this file, so invalid policy values cannot be serialized.
    public string? DisableBypassPermissionsMode { get; set; }

rust/src/types.rs:1786

  • This public top-level settings struct is exhaustive although its documentation says it currently carries only permissions. Future managed-settings siblings would therefore require a breaking Rust release; mark the new type #[non_exhaustive].
    go/types.go:1485
  • This exposes an unrestricted string even though the wire contract permits only "disable". Define a public named string type and constant (as this file does for ToolDefer and SectionOverrideAction) so callers cannot accidentally pass an ordinary string and the API reflects the permissions schema.
	DisableBypassPermissionsMode *string `json:"disableBypassPermissionsMode,omitempty"`

java/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java:40

  • Although this setter validates at runtime, the public API still exposes the schema's literal as an arbitrary String. Use an enum value (the same pattern as AgentMode) so unsupported policy values are not representable and callers get compile-time guidance.
    public ManagedSettingsPermissions setDisableBypassPermissionsMode(String value) {

java/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java:55

  • @return is embedded in the parameter description, so Javadoc does not recognize a return tag for this fluent public method. Put it on its own block-tag line.
     *            deny rules; @return this policy

java/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java:69

  • @return is embedded in the parameter description, so the generated Javadoc loses the return contract. Put it on a separate block-tag line.
     *            ask rules; @return this policy

java/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java:83

  • @return is embedded in the parameter description, so the generated Javadoc loses the return contract. Put it on a separate block-tag line.
     *            allow rules; @return this policy

java/src/main/java/com/github/copilot/rpc/ManagedSettings.java:27

  • @return is part of the @param prose here rather than a Javadoc block tag. Split it onto its own line so the fluent return value appears correctly in generated API documentation.
     *            managed permission policy; @return this settings object

java/src/test/java/com/github/copilot/ManagedSettingsTest.java:29

  • This verifies serialization through the package-private request builder instead of the public client API. Exercise createSession/resumeSession against the Java test server so the test also covers the public forwarding path and does not depend on internals.

This issue also appears on line 50 of the same file.

        var create = SessionRequestBuilder.buildCreateRequest(
                new SessionConfig().setEnableManagedSettings(true).setManagedSettings(managedSettings),
                "managed-create");
        var resume = SessionRequestBuilder.buildResumeRequest("managed-resume",
                new ResumeSessionConfig().setEnableManagedSettings(true).setManagedSettings(managedSettings));

java/src/test/java/com/github/copilot/ManagedSettingsTest.java:22

  • The PR calls out explicit empty arrays—especially allow: []—as security-relevant, but the Java test only serializes non-empty lists. Add a public-path assertion that an empty allow list remains [] rather than being omitted.
                .setAllow(List.of("Read(**)"));

dotnet/test/Unit/ClientSessionLifetimeTests.cs:511

  • This test reflects into _managedSettingsEnabled, so it does not verify the behavior through the public .NET API and is coupled to a private field name. Trigger a permission request through the fake server and assert the public ApproveAll behavior instead.
        var managedField = typeof(CopilotSession).GetField("_managedSettingsEnabled", BindingFlags.Instance | BindingFlags.NonPublic)
            ?? throw new InvalidOperationException("Managed settings field was not found.");
        Assert.True((bool)managedField.GetValue(session)!);
  • Files reviewed: 26/35 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants