Skip to content

Fixing rgpath for terminal sandboxing#301948

Merged
dileepyavan merged 24 commits intomainfrom
DileepY/terminal_sandbox
Mar 16, 2026
Merged

Fixing rgpath for terminal sandboxing#301948
dileepyavan merged 24 commits intomainfrom
DileepY/terminal_sandbox

Conversation

@dileepyavan
Copy link
Copy Markdown
Member

fixes(#301928)

Copilot AI review requested due to automatic review settings March 16, 2026 05:14
@dileepyavan dileepyavan enabled auto-merge (squash) March 16, 2026 05:14
@vs-code-engineering vs-code-engineering bot added this to the 1.112.0 milestone Mar 16, 2026
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

This PR targets terminal sandboxing reliability by adjusting how the sandbox helper exposes the bundled rg (ripgrep) dependency, addressing failures where sandboxed terminal tools never run due to missing ripgrep.

Changes:

  • Inject ILogService into SandboxHelperService and log the computed ripgrep path.
  • Stop overriding SandboxManager.initialize’s ripgrep config from _rgPath, and instead try to make rg discoverable by extending PATH in the wrapped command’s environment prefix.
  • Add helper logic to append the ripgrep binary directory to PATH.
Comments suppressed due to low confidence (3)

src/vs/platform/sandbox/node/sandboxHelperService.ts:81

  • SandboxManager.initialize(...) runs before the wrapped command is produced, so updating PATH only in _getSandboxEnvironmentPrefix() won’t affect the sandbox runtime’s dependency check that currently fails with “Required: ripgrep (rg)”. Also, normalizedRuntimeConfig.ripgrep is now only provided when the caller supplies runtimeConfig.ripgrep, but TerminalSandboxService never sets it, so initialize will fall back to searching for rg on the current process PATH and likely keep failing. Consider always providing a ripgrep config here (e.g. using VS Code’s bundled ripgrep path) or otherwise ensuring rg is discoverable in the environment used during initialize.

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

  • line 30
  • line 104
			ripgrep: runtimeConfig.ripgrep ? {
				command: runtimeConfig.ripgrep.command,
				args: runtimeConfig.ripgrep?.args ? [...runtimeConfig.ripgrep.args] : undefined,
			} : undefined,
			mandatoryDenySearchDepth: runtimeConfig.mandatoryDenySearchDepth,
			allowPty: runtimeConfig.allowPty,
		};
		await SandboxManager.initialize(normalizedRuntimeConfig, request => this._requestSandboxPermission(request));
		return SandboxManager.wrapWithSandbox(`${this._getSandboxEnvironmentPrefix()} ${command}`);

src/vs/platform/sandbox/node/sandboxHelperService.ts:35

  • The ripgrep path is currently derived from appRoot/node_modules/@vscode/ripgrep/bin/rg. In built distributions the module/binary is commonly stored under node_modules.asar and the executable under node_modules.asar.unpacked (see other ripgrep usages that rewrite rgPath to the unpacked location). If _rgPath points at a non-existent location, both the new PATH injection and any ripgrep command resolution will be ineffective. Suggest resolving the ripgrep binary using @vscode/ripgrep’s rgPath (and rewriting to the unpacked path when needed) instead of constructing it manually from appRoot.
		this._rgPath = nativeEnvironmentService.appRoot
			? this._pathJoin(nativeEnvironmentService.appRoot, 'node_modules', '@vscode', 'ripgrep', 'bin', 'rg')
			: undefined;
		this._tempDir = nativeEnvironmentService.tmpDir?.path;
		logService.info('SandboxHelperService#constructor ripgrep path', this._rgPath ?? 'undefined');
	}

src/vs/platform/sandbox/node/sandboxHelperService.ts:108

  • _getPathWithRipgrepDir appends the ripgrep directory to the end of PATH. If another rg exists earlier on PATH, the sandbox runtime will pick that one, which can reintroduce “missing/incorrect dependency” issues and makes behavior less deterministic. Consider prepending the ripgrep directory so the bundled rg takes precedence.
		const rgDir = dirname(this._rgPath);
		const currentPath = process.env['PATH'];
		const path = process.platform === 'win32' ? win32 : posix;
		return currentPath ? `${currentPath}${path.delimiter}${rgDir}` : rgDir;
	}

You can also share your feedback on Copilot code review. Take the survey.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@dileepyavan dileepyavan merged commit 00d6a08 into main Mar 16, 2026
20 checks passed
@dileepyavan dileepyavan deleted the DileepY/terminal_sandbox branch March 16, 2026 05:39
dileepyavan added a commit that referenced this pull request Mar 16, 2026
dileepyavan added a commit that referenced this pull request Mar 16, 2026
dileepyavan added a commit that referenced this pull request Mar 17, 2026
* Revert "Fixing rgpath for terminal sandboxing (#301948)"

This reverts commit 00d6a08.

* Revert "[Terminal_Sandboxing] Enable default trusted domains and updates to sandbox config (#301852)"

This reverts commit af55754.

* Revert "[Terminal_sandbox] Notify when user consent is needed for permissions in sandbox. (#301413)"

This reverts commit 0cb8d31.
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.

3 participants