Skip to content

Deny writes to macOS terminal sandbox config - #336378

Draft
azure-pipelines[bot] wants to merge 1 commit into
mainfrom
msrc-resolver/icm-31000000679378-run-474251
Draft

azure-pipelines[bot] wants to merge 1 commit into
mainfrom
msrc-resolver/icm-31000000679378-run-474251

Conversation

@azure-pipelines

Copy link
Copy Markdown

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.

🟡 Changes recommended

Add a macOS test asserting that checkFileAccess('write', [configPath]) denies the configuration path.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Prevents sandboxed macOS terminal commands from writing to their sandbox configuration file.

Changes:

  • Adds the configuration path to macOS denyWrite rules.
  • Adds regression coverage for the serialized deny-write configuration.
  • The test does not yet verify checkFileAccess enforcement for the config path.
File summaries
File Summary
src/vs/platform/sandbox/test/common/terminalSandboxEngine.test.ts Tests the macOS deny-write configuration.
src/vs/platform/sandbox/common/terminalSandboxEngine.ts Denies writes to the generated macOS sandbox configuration.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +417 to +423
deepStrictEqual({
configDenyWrite: config.filesystem.denyWrite.includes(configPath),
tempDirAllowWrite: config.filesystem.allowWrite.includes(tempDirPath),
}, {
configDenyWrite: true,
tempDirAllowWrite: true,
});
@github-actions

Copy link
Copy Markdown
Contributor

Screenshot Changes

Base: 4ae4d960 Current: 32bac449

Changed (1)

imageCarousel/imageCarousel/SingleSectionMiddleImage/Dark
Before After
before after

1 insignificant change(s) omitted (≤20 px, Δ≤2). See CI logs for details.

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.

2 participants