Skip to content

fix(seatbelt): Apply the cwd to the spawn_open (in-terminal) launch path. - #1079

Open
Elliot (theelliotm) wants to merge 5 commits into
mainfrom
user/emichlin/fix-noisy-getcwd
Open

fix(seatbelt): Apply the cwd to the spawn_open (in-terminal) launch path.#1079
Elliot (theelliotm) wants to merge 5 commits into
mainfrom
user/emichlin/fix-noisy-getcwd

Conversation

@theelliotm

@theelliotmElliot (theelliotm) commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📖 Description

spawn_open never applied process.cwd. open hands the launch to Terminal,
which starts the helper in its own directory (normally $HOME), so the
resolved working directory was silently discarded — only spawn_exec set it.

The generated helper script now does what spawn_exec does before the sandbox
is applied: cd into resolve_working_directory(request) and export PWD to
it. The cd is guarded (|| exit 1) — a bare one would leave the workload
running in Terminal's directory. A directory that doesn't exist is rejected
before Terminal launches, matching the exec path's spawn failure, since
open -W reports its own status and not the helper's.

🔗 References

None.

🔍 Validation

  • 5 new unit tests in seatbelt_runner: cd + PWD emitted, abort guard
    ordering, PWD assigned after caller env, single-quote escaping, and parity
    with the exec path's resolution.

  • cargo test -p seatbelt_common — 106 passed.

  • cargo fmt --all -- --check, cargo clippy -p seatbelt_common -p mxc_darwin --all-targets -- -D warnings — clean.

  • Manual, macOS 26.6.2, against the real generated profile with cwd granted in readwritePaths:

    probebeforeafter
    pwd/Users/agos/tmp/mxc611_work
    cat f.txt (relative)No such file or directoryhello

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

CopilotAI balanced review requested due to automatic review settings September 1, 2026 18:44
@theelliotm
Elliot (theelliotm) requested a review from a team as a code ownerSeptember 1, 2026 18:44
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

CopilotAI 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.

Pull request overview

Fixes Seatbelt’s Terminal-based launch path to honor the requested working directory.

Changes:

  • Resolves, validates, and applies cwd in open mode.
  • Extracts shell escaping/helper generation and adds unit tests.
  • Documents Seatbelt working-directory access requirements.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/backends/seatbelt/common/src/seatbelt_runner.rsApplies cwd in the generated helper and adds tests.
docs/seatbelt/seatbelt-backend.mdDocuments profile access requirements for cwd.

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

Comment threadsrc/backends/seatbelt/common/src/seatbelt_runner.rs Outdated
@theelliotmElliot (theelliotm) changed the title fix(seatbelt): apply the request's working directory on the open launch pathfix(seatbelt): Apply the cwd to the spawn_open (in-terminal) launch path.Sep 1, 2026
@theelliotm
Elliot (theelliotm) marked this pull request as draft September 1, 2026 18:53
process.cwd may be relative; the exec path resolves it against the MXC
process's directory via chdir. The helper script runs from Terminal's
directory, so a relative value could enter a different same-named
directory while open -W still reported success.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings September 1, 2026 18:54

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/backends/seatbelt/common/src/seatbelt_runner.rs:348

  • is_dir() verifies only the file type, not whether this user can enter the directory. For example, metadata can identify a directory whose execute/search permission is denied; this check then passes, the helper's cd fails, and open -W still reports a clean run without executing the workload. Use a preflight child with Command::current_dir(&cwd) (or propagate the helper's status back) so an unusable cwd is reported like the exec path.
 if !Path::new(&cwd).is_dir() {

Comment threadsrc/backends/seatbelt/common/src/seatbelt_runner.rs Outdated
chdir accepts an absolute path even when the launching process own
directory has been removed, so an eager current_dir() call rejected a
request the exec path launches without complaint.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings September 1, 2026 21:41

CopilotAI 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

The preflight accepts directories lacking the search permission required by cd, allowing hidden launch failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment threadsrc/backends/seatbelt/common/src/seatbelt_runner.rs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 21:56

CopilotAI 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.

🟢 Approval recommended

The implementation matches the exec-path semantics and includes comprehensive edge-case coverage.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@theelliotm
Elliot (theelliotm) marked this pull request as ready for review September 1, 2026 22:34
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Sign up for freeto 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

@theelliotm@richiemsft