feat: nested ptys, forward sigwinch, keychain access#322
Conversation
macOS' default 256-soft NOFILE cap is too low for Node and other agent runtimes that open many file descriptors (sockets, ptys, file watchers) inside the seatbelt sandbox; they hit EMFILE under load. Raise the soft limit to the kernel hard cap on startup (best-effort) so the inner workload gets the full available headroom. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Two new opt-in/opt-out fields under `experimental.seatbelt` to unblock real agent workflows that the baseline profile breaks: - `nestedPty` (default `true`): allow the inner workload to call `posix_openpt` and friends. Emits `pseudo-tty`, broad `(allow iokit-open)`, and read/write/ioctl on `/dev/ptmx`. Without this anything that spawns a real shell (tests, git, gh, REPLs) dies inside the sandbox because `TTY_ALLOW` only covers the outer pty. - `keychainAccess` (default `false`): allow Security.framework / keytar to talk to the system keychain stack. Emits Mach lookups for SecurityServer, securityd, cfprefsd.daemon, xpcd, and the `com.apple.lsd.*` family (via `global-name-regex`); read access to `/Library/Keychains` and `/private/var/db/mds`; read+write on `$HOME/Library/Keychains` and `/private/var/folders`; and `(allow iokit-open)` (redundant when `nestedPty` is on, but keeps the capability self-contained). Both rule blocks are emitted before `write_ui_rules` so the HID `iokit-open` deny still wins under Seatbelt's last-match-wins semantics, and `nestedPty` is suppressed when `guiAccess` already emits a strict superset. Includes schema, docs, and unit tests covering defaults, on/off, interaction with `guiAccess` and `ui.disable`, and HID deny ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
The pty bridge had several gaps that broke any nested-pty workload (inner shell + its own children — e.g. node TUIs, REPLs, anything that tcgetattr-s its tty) when run under the seatbelt sandbox via a host that wraps mxc-exec-mac in its own outer pty: - The outer pty slave (our fd 0) was left in cooked mode, so input bytes were echoed back and control chars rendered as `^X` on the way to the inner child, corrupting any TUI it tried to render. Install a RAII RawSlaveGuard that puts the outer slave into raw mode for the duration of the bridge and restores the original termios on drop. No-op when fd 0 is not a tty. - The inner pty was created with the kernel default 0×0 winsize, so TUIs sized to a zero-by-zero terminal. Inherit the outer pty's TIOCGWINSZ when available. - SIGWINCH was never propagated. The kernel delivers it to us (because fd 0 is the outer slave), but we did nothing with it — the inner child never resized. Install a self-pipe SIGWINCH handler + a forwarder thread that re-reads the outer winsize and pushes it to the inner pty master via TIOCSWINSZ on every event. Self-pipe rather than sigwait so we don't depend on which thread the runtime lets the signal land on. - The inner child inherited whatever sigmask the parent had installed (signal_cleanup blocks a bunch via sigwait). Add SIGWINCH to the unblock set unconditionally in pre_exec — execve resets the handler to default but preserves the inherited mask, so a child that installs its own SIGWINCH handler (node, neovim, …) would otherwise silently never see it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
This reverts commit 6b8c32d.
There was a problem hiding this comment.
Pull request overview
This PR extends the macOS Seatbelt backend with nested PTY and optional Keychain access support, and improves the shared Unix PTY bridge to propagate terminal sizing and raw-mode behavior for nested interactive sessions.
Changes:
- Adds
nestedPtyandkeychainAccessconfig fields for Seatbelt in Rust models, parser tests, schema, and macOS docs. - Emits additional Seatbelt profile rules for nested PTY allocation and Keychain-related Mach/filesystem access.
- Updates
mxc_ptyto inherit window size, forward SIGWINCH, and set the outer PTY slave to raw mode.
Show a summary per file
| File | Description |
|---|---|
src/wxc_common/src/models.rs |
Adds Seatbelt config fields and manual defaults. |
src/wxc_common/src/config_parser.rs |
Parses new Seatbelt fields and adds parser tests. |
src/seatbelt_common/src/profile_builder.rs |
Adds profile rules and tests for nested PTYs and Keychain access. |
src/mxc_pty/src/lib.rs |
Adds raw-mode handling, initial window sizing, and SIGWINCH forwarding. |
schemas/dev/mxc-config.schema.0.6.0-dev.json |
Documents new Seatbelt schema fields. |
docs/macos-support/seatbelt-backend.md |
Documents new Seatbelt options. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/mxc_pty/src/lib.rs:342
- The SIGWINCH self-pipe is created in blocking mode, but the signal handler writes to it directly. If resize events arrive while the pipe is full or the reader is stalled,
write(2)can block inside the signal handler and hang the process. Please make the write end non-blocking (and handleEAGAIN) before installing the handler.
let (read_end, write_end) = nix::unistd::pipe().ok()?;
let read_fd = read_end.as_raw_fd();
let write_fd = write_end.as_raw_fd();
// Leak the OwnedFds — they live for the rest of the process.
// Closing them would race the signal handler.
std::mem::forget(read_end);
std::mem::forget(write_end);
SIGWINCH_PIPE_WRITE_FD.store(write_fd, std::sync::atomic::Ordering::Release);
src/mxc_pty/src/lib.rs:352
- Each
run_with_ptycall installs a new global SIGWINCH handler and overwrites the single static pipe fd. Previous resize threads and pipe fds are never shut down, and only the most recent invocation will receive future resize notifications. This makes repeated or concurrent pty runs leak resources and breaks resize forwarding for any earlier active session; the handler/pipe state needs to be installed once with per-session registration or cleaned up when the pty run completes.
SIGWINCH_PIPE_WRITE_FD.store(write_fd, std::sync::atomic::Ordering::Release);
// SIGWINCH's default action is "ignore", so without an installed
// handler the kernel drops the signal entirely. SA_RESTART so we
// don't break unrelated syscalls.
unsafe {
let mut sa: libc::sigaction = std::mem::zeroed();
sa.sa_sigaction = sigwinch_handler as *const () as usize;
libc::sigemptyset(&mut sa.sa_mask);
sa.sa_flags = libc::SA_RESTART;
if libc::sigaction(libc::SIGWINCH, &sa, std::ptr::null_mut()) != 0 {
- Files reviewed: 6/6 changed files
- Comments generated: 5
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
The Windows x64 CI lane runs `cargo test` and the keychain tests that set `keychain_access: true` exercise `write_keychain_rules`, which expands `~/Library/Keychains` from $HOME. Windows CI doesn't set $HOME so `build_profile().unwrap()` panics there. Keychain access only applies on macOS — gate the four tests that enable it to `cfg(target_os = "macos")`. The off-by-default test stays cross-platform because it never triggers the HOME lookup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
The SIGWINCH handler write()s to the self-pipe to wake the forwarder thread. If the pipe fills (reader stalled or pathological resize burst), a blocking write(2) inside an async-signal-safe handler can deadlock the process — and the existing comment already assumes EAGAIN-drop behavior that didn't actually hold without O_NONBLOCK. Set O_NONBLOCK on the write end before installing the handler. Best-effort: if fcntl fails we stay in blocking mode, same as before. Addresses Copilot reviewer feedback on #322. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
|
Addressed Copilot reviewer feedback:
Also fixed the x64 WXC CI failure in 6eb69e7 — the keychain tests need |
profile_builder is compiled (and unit-tested) on every host so reviewers can validate generated profiles without a Mac. `write_keychain_rules` expands `~/Library/Keychains` from $HOME, which Windows CI doesn't set — so on non-macOS the keychainAccess branch was either panicking in tests or silently emitting an invalid path. Have the builder simply ignore the option on non-macOS targets. Seatbelt itself only runs on macOS, so emitting nothing there is the correct behaviour, not a workaround. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
| // posix_openpt → grantpt → ioctls bottom out in IOKit user-clients | ||
| // (IOSerialBSDClient and friends). Without iokit-open the kernel | ||
| // returns EPERM and the master fd is unusable. | ||
| out.push_str("(allow iokit-open)\n"); |
There was a problem hiding this comment.
isn't this too broad? Is it possible to narrow it down to only what's needed?
There was a problem hiding this comment.
Yes — verified empirically and dropped entirely in cb21f36. I tested on macOS 15 (Apple Silicon) under sandbox-exec with (allow pseudo-tty) + /dev/ptmx but no iokit-open: posix_openpt → grantpt → unlockpt → ptsname → slave open all succeed. Then end-to-end through mxc-exec-mac itself with the new profile builder — posix_openpt returns OK slave=/dev/ttys014, opened slave fd=7. So the rule is removed from write_nested_pty_rules along with the now-stale ordering test and build_profile comment.
| out.push_str(" (subpath \"/private/var/folders\"))\n"); | ||
|
|
||
| out.push_str(";; --- keychainAccess: iokit-open for crypto accelerators ---\n"); | ||
| out.push_str("(allow iokit-open)\n"); |
There was a problem hiding this comment.
same comment about iokit-open in here.
There was a problem hiding this comment.
Same fix in cb21f36. Verified SecItemAdd + SecItemCopyMatching on a generic-password item under sandbox-exec with the keychain Mach surface (securityd, trustd, ocspd, cfprefsd, xpcd, lsd.*) and the FS surface (~/Library/Keychains, /private/var/folders, /private/var/db/mds) but no iokit-open — both return status 0. Dropped the rule. Also dropped the redundant /Library/Keychains and /System/Library/Keychains reads since the baseline (subpath "/Library") and (subpath "/System") already cover them, and updated the docs to match.
The forwarder was holding a borrowed RawFd from `master_writer`. The input thread later moves `master_writer` and drops it when stdin closes mid-session, so any later TIOCSWINSZ from the forwarder would ioctl on a closed (and possibly reused) descriptor. Hand the forwarder its own try_clone of the master File and leak it inside, so the resize fd has the same lifetime as the signal handler that targets it. Addresses Copilot review feedback on PR #322. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
…aths
Verified empirically on macOS 15 (Apple Silicon) with sandbox-exec:
* posix_openpt + grantpt + unlockpt + ptsname + slave open all
succeed under a profile that allows pseudo-tty and /dev/ptmx but
NOT iokit-open. Removed the broad (allow iokit-open) from
write_nested_pty_rules.
* SecItemAdd / SecItemCopyMatching against a generic-password item
succeed without iokit-open as long as the Mach surface (securityd,
trustd, ocspd, cfprefsd, xpcd, lsd.*) and the filesystem surface
(~/Library/Keychains, /private/var/folders, /private/var/db/mds)
are reachable. Removed (allow iokit-open) from write_keychain_rules.
* /Library/Keychains and /System/Library/Keychains are already
covered by the baseline (subpath "/Library") and (subpath
"/System") read-only allows. Removed the redundant subpath rules.
This drops every IOKit user-client class from the keychain + nested
pty paths, so HID injection stays denied without needing the
last-match-wins ordering trick. Removed the now-stale ordering tests
and the now-stale comment on build_profile.
Docs updated to match. The HID iokit deny that
ui.injection: false emits is unchanged.
Addresses richiemsft + Copilot review feedback on PR #322.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
…reference * sdk/src/types.ts: add nestedPty and keychainAccess to SeatbeltConfig with JSDoc matching the JSON schema. Without this, TypeScript callers using ContainerConfig.experimental.seatbelt couldn't set the new fields without a cast. * docs/schema.md: extend the experimental.seatbelt example to include every option (profileOverride, guiAccess, launchMethod, nestedPty, keychainAccess) so the reference matches the JSON schema. Addresses Copilot review feedback on PR #322. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
…fix/pty-sigwinch-keychain
📖 Description
Three independent fixes that together unblock interactive agent workflows under the macOS seatbelt backend. The baseline profile breaks anything that needs to spawn a real shell (tests,
git,gh, REPLs) or talk to the system Keychain, and the pty bridge corrupts TUIs whenever the host wrapsmxc-exec-macin its own pty.feat(seatbelt): add nestedPty + keychainAccess experimental optionsTwo new opt-in/opt-out fields under
experimental.seatbelt:nestedPty(defaulttrue): allow the inner workload to callposix_openpt. Emits(allow pseudo-tty),(allow iokit-open), and read/write/ioctl on/dev/ptmx. Default-on because almost any agent shell tool needs it.keychainAccess(defaultfalse): allowSecurity.framework/keytarto reach the system keychain stack. Emits Mach lookups forSecurityServer,securityd,trustd,cfprefsd.daemon,xpcd, and thecom.apple.lsd.*family (viaglobal-name-regex); read access to/Library/Keychains+/private/var/db/mds; read+write on$HOME/Library/Keychains+/private/var/folders; and(allow iokit-open).Both blocks are emitted before
write_ui_rulesso the HIDiokit-opendeny still wins under Seatbelt's last-match-wins semantics.nestedPtyis suppressed whenguiAccessalready emits a strict superset.fix(pty): make nested ptys work end-to-end under macOSFour gaps in
mxc_pty::run_with_ptythat broke any nested-pty workload under a host-allocated outer pty:^X. Added a RAIIRawSlaveGuardthat puts the outer slave into raw mode and restores termios on drop. No-op when fd 0 isn't a tty.TIOCGWINSZ.TIOCSWINSZ. Self-pipe rather thansigwaitso we don't depend on which thread the runtime lets the signal land on.sigwait).pre_execnow unconditionally addsSIGWINCHto the unblock set —execveresets the handler but preserves the inherited mask.🔗 References
https://github.com/github/copilot-agent-runtime/issues/164
🔍 Validation
cargo fmt --all -- --checkclean.cargo clippy -p mxc_pty -p seatbelt_common -p wxc_common -p mxc_darwin --all-targets -- -D warningsclean.cargo test -p mxc_pty -p seatbelt_common -p wxc_common— 4 + 33 + 190 = 227 tests pass. New unit tests cover:nestedPtydefaults / on / off / interaction withguiAccess/ HID deny ordering;keychainAccessmach + FS + iokit + HID deny ordering; parser pass-through of both fields.gh,git,nodeREPLs, and resize-on-the-fly all work;keytarreads/writes Keychain entries withkeychainAccess: true.✅ Checklist
📋 Issue Type