Skip to content

terminal: Disable read all by default#311850

Merged
dileepyavan merged 18 commits intomainfrom
DileepY/sandbox_homeDir
Apr 24, 2026
Merged

terminal: Disable read all by default#311850
dileepyavan merged 18 commits intomainfrom
DileepY/sandbox_homeDir

Conversation

@dileepyavan
Copy link
Copy Markdown
Member

@dileepyavan dileepyavan commented Apr 22, 2026

Summary

Copilot AI review requested due to automatic review settings April 22, 2026 06:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

Screenshot Changes

Base: 490e64db Current: b452e8d6

Changed (1)

editor/inlineCompletions/other/JumpToHint/Dark
Before After
before after

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for a filesystem read allow-list to the terminal sandbox configuration (Linux/macOS), aiming to default-deny reads from the user home while re-allowing common developer/tooling paths and configured exceptions.

Changes:

  • Introduces allowRead to the Linux/macOS filesystem sandbox settings schema and runtime config type.
  • Generates sandbox config with default denyRead (home) plus derived allowRead from configured allow-list, workspace folders, write-allowed paths, VS Code runtime/data paths, and a shared tooling allow-list.
  • Adds a shared terminal sandbox read allow-list module and updates tests for the new deny/read-allow behavior.
Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts Updates/extends tests to validate new deny-home + allowRead derivation behavior (currently Linux-only).
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts Implements allowRead generation, home-deny defaults, allow-list integration, and path expansion logic.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxReadAllowList.ts Adds shared read allow-list groups for Git/Node/common dev tooling paths.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandbox.ts Extends local runtime config type with filesystem.allowRead.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts Adds allowRead to Linux/macOS configuration schema defaults and docs.
extensions/copilot/.vscode/settings.json Enables chat.agent.sandbox.enabled in the Copilot extension workspace settings.

Copilot's findings

Comments suppressed due to low confidence (2)

src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts:532

  • On macOS, the generated filesystem paths are not normalized/expanded (eg ~ is left intact): macAllowWrite, macDenyRead, macAllowRead, and denyWrite are passed through without _expandHomePath, but the allow-list (getTerminalSandboxReadAllowList) and default write paths contain many ~/* entries. Unless the sandbox runtime expands ~ itself, this will make the macOS allow/deny lists ineffective. Consider applying the same home expansion for macOS (and for all filesystem arrays), and add coverage to ensure the written config contains absolute paths.
			const linuxAllowWrite = this._resolveLinuxFileSystemPaths(this._updateAllowWritePathsWithWorkspaceFolders(linuxFileSystemSetting.allowWrite));
			const macAllowWrite = this._updateAllowWritePathsWithWorkspaceFolders(macFileSystemSetting.allowWrite);
			const linuxDenyRead = this._resolveLinuxFileSystemPaths(this._updateDenyReadPathsWithHome(linuxFileSystemSetting.denyRead));
			const macDenyRead = this._updateDenyReadPathsWithHome(macFileSystemSetting.denyRead);
			const linuxAllowRead = this._resolveLinuxFileSystemPaths(this._updateAllowReadPathsWithAllowWrite(linuxFileSystemSetting.allowRead, linuxAllowWrite));
			const macAllowRead = this._updateAllowReadPathsWithAllowWrite(macFileSystemSetting.allowRead, macAllowWrite);
			const linuxDenyWrite = this._resolveLinuxFileSystemPaths(linuxFileSystemSetting.denyWrite);

			const sandboxSettings = {
				network: {
					allowedDomains: allowedDomainsSetting,
					deniedDomains: deniedDomainsSetting
				},
				filesystem: {
					denyRead: this._os === OperatingSystem.Macintosh ? macDenyRead : linuxDenyRead,
					allowRead: this._os === OperatingSystem.Macintosh ? macAllowRead : linuxAllowRead,
					allowWrite: this._os === OperatingSystem.Macintosh ? macAllowWrite : linuxAllowWrite,
					denyWrite: this._os === OperatingSystem.Macintosh ? macFileSystemSetting.denyWrite : linuxDenyWrite,
				},

src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts:664

  • _getVSCodeDataReadPaths hardcodes ~/vscode-server-insiders and ~/.vscode-server-insiders. This seems build-variant specific and will miss stable (vscode-server) or other product variants. Consider deriving these names from this._productService.serverApplicationName (and including both dotted and non-dotted forms if needed) rather than hardcoding the insiders folder name.
	private _getVSCodeDataReadPaths(): string[] {
		const paths = ['~/vscode-server-insiders', '~/.vscode-server-insiders'];
		const userHome = this._getUserHomePath();
		if (userHome) {
			paths.push(this._pathJoin(userHome, this._productService.dataFolderName));
			if (this._productService.serverDataFolderName) {
				paths.push(this._pathJoin(userHome, this._productService.serverDataFolderName));
			}
		}
		return paths;
  • Files reviewed: 6/6 changed files
  • Comments generated: 3

Comment thread extensions/copilot/.vscode/settings.json Outdated
@dileepyavan dileepyavan marked this pull request as ready for review April 23, 2026 18:15
@dileepyavan dileepyavan requested a review from alexdima April 23, 2026 18:15
@dileepyavan dileepyavan changed the title terminal: add sandbox read allow list terminal: Disable read all by default Apr 23, 2026
@dileepyavan dileepyavan removed the request for review from alexdima April 24, 2026 01:07
@dileepyavan dileepyavan enabled auto-merge (squash) April 24, 2026 01:07
@dileepyavan dileepyavan disabled auto-merge April 24, 2026 01:55
@dileepyavan dileepyavan enabled auto-merge (squash) April 24, 2026 06:25
@dileepyavan dileepyavan merged commit bb09e73 into main Apr 24, 2026
26 checks passed
@dileepyavan dileepyavan deleted the DileepY/sandbox_homeDir branch April 24, 2026 18:07
@vs-code-engineering vs-code-engineering Bot added this to the 1.118.0 milestone Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Terminal_ Sandbox]: Do not allow reading arbitrary paths inside the $HOME folder

4 participants