Add managed permission settings to session startup - #2139
Draft
joshspicer wants to merge 2 commits into
Draft
Conversation
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"` |
joshspicer
force-pushed
the
joshspicer-sdk-managed-settings-permissions
branch
from
July 31, 2026 22:41
135b25a to
3676a65
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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
allowlist is not equivalent to omittingallow: by this field's own contract, every present allow-list must admit an operation, so[]is a deny-all constraint.omitemptydrops 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 definesDisableBypassPermissionsMode) rather thanString.
pub disable_bypass_permissions_mode: Option<String>,
go/types.go:1478
- The contract permits only the
"disable"literal, but*stringaccepts and forwards arbitrary values. Expose a dedicated typed value/constant (the generated RPC package already hasrpc.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 generatedSessionOpenOptions, but that record still has nomanagedSettingsfield 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 inTypes.cs.
[JsonPropertyName("disableBypassPermissionsMode")]
public string? DisableBypassPermissionsMode { get; set; }
- Files reviewed: 25/30 changed files
- Comments generated: 2
- Review effort level: Balanced
| 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 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. |
…/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
joshspicer
force-pushed
the
joshspicer-sdk-managed-settings-permissions
branch
from
August 3, 2026 15:16
3676a65 to
5411c88
Compare
Contributor
There was a problem hiding this comment.
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": nullon every resume when unset because it is missing the neighboringskip_serializing_ifattribute. 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
omitemptydrops a non-nil emptyAllowslice, 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'somitzero, 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 omittingallowcontributes 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
@returnis currently parsed as part of therulesparameter 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
@returnis 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
@returnis embedded in the@paramdescription, 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
@returntext 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
| 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>, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
SDK hosts need a typed way to inject enterprise permission policy at session startup, independent of the runtime server/device managed-settings fetch path.
What
Adds optional
managedSettings.permissionssupport alongsideenableManagedSettingsfor create and resume across Node, Python, Go, .NET, Rust, and Java. The public types expose bypass disable plus deny/ask/allow rule arrays, and each client forwards the same camelCase JSON shape. Direct injection activates each SDK's managed-session safeguards even when runtime self-fetch is disabled.The setting is startup-only and must be re-supplied on resume. It composes restrictively with runtime-fetched policy. The SDK protocol version remains unchanged because the field is optional.
Older runtimes may ignore the additive field, so hosts must not rely on injected policy until they ship a compatible runtime. This PR should publish only after
github/copilot-agent-runtime#14000is released, then downstream hosts such as VS Code can bump the SDK and remove temporary type shims.Validation
go vet