mirror of
https://github.com/microsoft/vscode.git
synced 2026-09-30 16:19:23 +01:00
07864e41d5
* agentHost: let managed permissions see shell paths, and stop misstating why a read is gated Two independent fixes to Copilot managed-permission handling in the agent host. **Enable the runtime's shell-script safety classifier.** Managed permission rules such as `Read(...)` and `Edit(...)` match on filesystem paths. For a shell command the runtime only populates `possiblePaths` and `hasWriteFileRedirection` when the session option `enableScriptSafety` is set; otherwise `assess_safety` short-circuits and returns a degenerate assessment with no paths, no redirection flag, and the whole script as a single non-read-only command. The agent host never set the option, so every shell command reached the permission layer carrying no paths at all. Observed effect: with a managed `deny: Edit(/.github/workflows/**)` in force, `echo x >> .github/workflows/f.yml` surfaced as an ordinary shell prompt and modified the file. Even `tail -3 f` was reported as non-read-only. The Copilot CLI opts in at session creation; the agent host did not. The SDK exposes the option on `SessionUpdateOptionsParams` rather than on `SessionConfig`/`ResumeSessionConfig`, so it is applied from `_finalizeSession` via `rpc.options.update`. That is the single point shared by the create, resume, and resume-fallback paths, and it runs before the wrapper is handed to a caller, so no turn can start without it. Failure is logged at error rather than thrown, matching the surrounding post-launch option updates. This restores the path data the permission layer needs. Note that matching managed path rules against a shell command's `possiblePaths` is a separate runtime-side concern and is not addressed here. **Report the actual reason a read needs approval.** `getPermissionDisplay` returned "Allow reading file outside of workspace?" for every `read` request, but a read is gated for several reasons: the path is outside the workspace, a managed or scoped rule matched it, managed rules are active and an unmatched request defaults to ask, or the model asked to escape the sandbox. Reads inside the workspace were therefore told they were outside it. The branch also ignored `requestSandboxBypass`, which was already being computed for reads, while the neighbouring `shell` and `write` branches vary their titles. The title is now chosen from the request: a sandbox-bypass read mirrors the shell wording, a genuinely outside path keeps the original title, and anything else uses a neutral "Allow reading file?". Containment uses `extUriBiasedIgnorePathCase.isEqualOrParent` so platform case-insensitivity is respected, and a relative path, unknown path, or unknown working directory falls back to the neutral title rather than asserting a location the request does not establish. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: address review — multi-root read titles and fail-closed script safety Two review findings on the previous commit. **Account for every workspace root before claiming a read is outside it.** The read confirmation title compared the path only against the primary process root, so in a multi-root session a read under a peer root (`ICopilotSessionLaunchBase.additionalDirectories`) still reported "outside of workspace" — the same misleading diagnosis this change set exists to remove. `getPermissionDisplay` now takes the session's additional directories and treats a path as outside only when no root contains it. **Fail closed when script safety cannot be enabled under managed rules.** Logging and continuing left the session usable after the runtime failed or refused to enable the classifier, which silently degrades enterprise enforcement: managed `Read(...)`/`Edit(...)` rules cannot inspect shell redirect targets, permitting exactly the bypass this change targets. Launch now fails and the orphaned session is disconnected when managed permission rules are in force. Without managed rules there is no policy to escape, so a transient or compatibility failure is logged as a warning and the session continues rather than making every user's sessions unlaunchable. `_finalizeSession` disconnects `raw` before rethrowing, since nothing owns that session until it is wrapped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: keep the out-of-workspace read title reachable The unauthorized-path gate is the case the original hardcoded title described, and narrowing the title by path containment alone made it unreachable. That gate runs ahead of the per-kind gates and fires only for paths outside the allowed directories. It is not a distinct request kind: it reuses the access kind, so it arrives as `kind: "read"`, but it carries `paths` (plural) rather than the per-kind `path`. `getPermissionDisplay` reads `request.path`, which is therefore undefined for these requests, so the containment check could not succeed and every out-of-workspace read fell through to the neutral title — losing the one diagnosis that was correct before this change set. A request carrying `paths` is treated as out-of-workspace by construction, which is a stronger signal than comparing against the workspace roots: it is the runtime's own verdict that the path is outside the allowed directories, and it covers directories the user approved earlier that are not workspace roots. The SDK's `PermissionRequestRead` does not model `paths`, hence the structural check rather than a typed discriminator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: make the script-safety launcher tests exercise the success path `Array.push` returns a number, so a mocked `options.update` that returned it gave `_applyScriptSafety` a `result.success` of `undefined`. Every launcher test therefore took the SDK-rejection branch and passed without ever exercising a successful enablement. The mocks now return the SDK-shaped `{ success: true }` after recording the options, and the enablement test runs with managed rules in force so anything short of a successful enablement fails the launch instead of warning. Verified by flipping the mock to `{ success: false }`, which fails that test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: fix the SDK-sandbox smoke test for script-safety classification Enabling the runtime's script-safety classifier changed which commands reach the host's permission layer. `commands_requiring_approval` skips commands the classifier reports as read-only, so the scenario's `echo` is now resolved by the runtime without ever raising a permission request. The host's "Auto-approving sandboxed shell command" branch therefore never runs, and the SDK-sandbox smoke test failed waiting for that log entry on Linux and macOS. (Windows skips both sandbox tests, which is why it stayed green.) Before this PR the classifier short-circuited and reported the whole script as a single non-read-only command, which forced the prompt the test was keyed to. The prompt was redundant: the command still runs mxc-wrapped under the pushed sandbox policy, so not asking is the better behavior, and the assertion is simply stale rather than the code being wrong. The test now keys on `Tool started: bash`, which proves the SDK's own shell tool ran the command, and keeps the `Applied SDK sandboxConfig` and negative `[ShellManager]` assertions. Polling on the tool run is also a stricter gate for the negative assertion than the old pre-execution entry, since a competing ShellManager run would have been logged by that point. The host auto-approve branch still governs non-read-only sandboxed commands and remains asserted by the custom-terminal-tool test above. Verified by replaying all three assertions against the agenthost.log artifact from the failing macOS run: both matches hold and the ShellManager pattern is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: fail closed on script safety for every session `IAgentHostManagedSettingsService` only carries the legacy VS Code settings bridge — its own documentation says so, and that new enterprise controls belong directly in the SDK's managed-settings contract. Server and MDM policy is discovered by the runtime itself under `enableManagedSettings`, so it never appears in `permissions`. Gating fail-closed on that signal therefore inverted the intent: a session governed solely by GitHub or MDM policy looks unmanaged from the host's side, so an `options.update` failure left exactly the policy-bearing sessions running without the classifier that managed `Read(...)`/`Edit(...)` rules depend on to see shell paths. Querying the runtime's effective managed-settings result instead would cost a proxy resolution and a separate SDK import on every launch, and would make a security control depend on a second fallible lookup. The option is cheap, present in the pinned SDK, and accepted by the runtime, so it is now treated as required for every session and a failure fails the launch. The launcher tests cover both a client-bridged session and one where the host sees no managed rules at all, since that is what an enterprise session governed only by server or MDM policy looks like from here. The shared resume-fallback mock gained the `rpc.options.update` surface a real session always has; without it the fixture failed on a missing `rpc` rather than on the behavior under test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: cover the rejected-RPC path of the script-safety fail-closed guard `session.options.update` reports `success: true` for any patch the runtime accepts and signals real problems by failing the request, so a rejected RPC — not a `success: false` body — is the path a genuine enablement failure takes. Both existing fail-closed tests drove the `success: false` branch, leaving the `catch` that logs the reason and aborts the launch uncovered. Adds a test that rejects the update and asserts the launch fails closed, the original error propagates, and the orphaned session is disconnected. Also drops the managed rules from the success test. They no longer gate the behavior, so passing a `deny` rule implied a dependency that is exactly the one the fail-closed change removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Dmitriy Vasyura <dmitriv@microsoft.com>
VS Code Tests
Contents
This folder contains the various test runners for VS Code. Please refer to the documentation within for how to run them: