Skip to content

fix(sandbox): allow tty device in seatbelt profile#2524

Closed
cyq1017 wants to merge 1 commit into
Hmbown:mainfrom
cyq1017:codex/2372-seatbelt-dev-tty
Closed

fix(sandbox): allow tty device in seatbelt profile#2524
cyq1017 wants to merge 1 commit into
Hmbown:mainfrom
cyq1017:codex/2372-seatbelt-dev-tty

Conversation

@cyq1017
Copy link
Copy Markdown
Contributor

@cyq1017 cyq1017 commented Jun 1, 2026

Refs #2372

Problem:

  • macOS seatbelt currently allows PTY allocation and /dev/ptmx//dev/ttysN, but not /dev/tty.
  • Tools such as ssh/sshpass/sudo can open /dev/tty directly when running inside a TTY-backed shell task.

Change:

  • Allow read/write/ioctl access to /dev/tty in the base seatbelt profile.
  • Add a focused policy test so this path stays covered.

Verification:

  • rustfmt crates/tui/src/sandbox/seatbelt.rs --edition 2024 --check
  • cargo test -p codewhale-tui --all-features --locked seatbelt::tests
  • git diff --check

Note:

  • This intentionally stays scoped to the macOS seatbelt path and does not claim to close the broader controlling-terminal issue.

Greptile Summary

Adds /dev/tty (the controlling terminal device) to the macOS seatbelt sandbox base policy so tools like ssh, sshpass, and sudo can open it for password/passphrase prompts when running inside a TTY-backed shell task.

  • One line added to SEATBELT_BASE_POLICY granting file-read*, file-write*, and file-ioctl on the literal path /dev/tty, consistent with the already-allowed /dev/ptmx and /dev/ttys[0-9]+ entries.
  • A focused unit test verifies the generated policy string contains the new rule.

Confidence Score: 5/5

Safe to merge — the change is a single-line policy addition with a matching test, and does not alter any logic paths.

The diff is two additions: one SBPL rule and one assertion-only test. The new /dev/tty rule is the natural complement to the already-present /dev/ptmx and /dev/ttys[0-9]+ allowances, so no policy surface is opened unexpectedly. The test is a straightforward string-contains check on the generated policy, which is sufficient given that the policy is a static constant — any future edit that removes the line will immediately break the test.

No files require special attention.

Important Files Changed

Filename Overview
crates/tui/src/sandbox/seatbelt.rs Adds /dev/tty to the base seatbelt policy (line 72) and a corresponding string-based test (lines 655-666); the change is minimal, correctly scoped, and consistent with adjacent PTY permissions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[sandbox-exec invoked] --> B[SEATBELT_BASE_POLICY applied]
    B --> C{Device opened?}
    C -->|/dev/ptmx| D[Allowed: PTY master]
    C -->|/dev/ttysN| E[Allowed: PTY slave via regex]
    C -->|/dev/tty NEW| F[Allowed: controlling terminal]
    C -->|other /dev/*| G[Denied by default]
    F --> H[ssh / sshpass / sudo password prompt works]
Loading

Reviews (1): Last reviewed commit: "fix(sandbox): allow tty device in seatbe..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the macOS seatbelt sandbox base policy in crates/tui/src/sandbox/seatbelt.rs to allow read, write, and ioctl access to /dev/tty, which is required for TTY-mode shells to handle prompts (such as sshpass or sudo). It also adds a unit test to verify that the generated policy correctly includes this rule. There are no review comments, and I have no feedback to provide.

Hmbown pushed a commit that referenced this pull request Jun 1, 2026
@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented Jun 1, 2026

Harvested the scoped macOS seatbelt /dev/tty allowance into #2504 for v0.8.50 as 1605d8de, preserving @cyq1017 as the commit author and adding the release-harvest trailer.

Local validation after the harvest:

  • rustfmt crates/tui/src/sandbox/seatbelt.rs --edition 2024 --check
  • cargo test -p codewhale-tui --all-features --locked seatbelt::tests -- --nocapture
  • git diff --check origin/codex/v0.8.50-triage..HEAD
  • ./scripts/release/check-versions.sh

I am keeping #2372 open because this is intentionally only the seatbelt policy slice. The broader controlling-terminal setup (setsid() / TIOCSCTTY) still needs a separate targeted fix before that issue should close.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

Thanks @cyq1017 — your contribution landed in 1605d8de449e on main:

fix(sandbox): allow tty device in seatbelt profile

Closing this PR now that the code is on main. Credit lives in the commit message and (where applicable) the CHANGELOG.md entry for the next release. Apologies for not closing this at the time of the merge — the auto-close workflow is new in v0.8.31.

If you want to land more work and would prefer your future PRs merge cleanly without a harvest step, the CONTRIBUTING.md doc has a short note on what makes a contribution mergeable as-is.

@github-actions github-actions Bot closed this Jun 2, 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.

2 participants