fix: allow native SSH-signed commits in sandbox - #2383
Conversation
603195a to
0263e66
Compare
🖥️ PR preview
|
|
I fixed the sole CI blocker in the branch history: gitleaks matched the intentionally fake private-key marker in the test fixture. Because the scan covers full history, I amended the single PR commit rather than adding a follow-up, then rebased onto current Validation after the rebase: both focused signing test files pass, 12/12, under the pinned Node 24.20.0 runtime; |
Reference screenshotsCI run 34123700910 published no changed screenshot candidates for |
|
Final rerun result on |
0263e66 to
b4cf0af
Compare
Add an explicit macOS permission for the native git_commit subprocess to reach the configured SSH agent socket while preserving the existing filesystem and network sandbox. Supply path-configured signing keys as inline public identities, and patch sandbox-runtime so per-spawn Unix socket allowances reach the seatbelt profile. Co-Authored-By: Copse <noreply@copse.dev> Copse-Models: acp:codex-acp#gpt-5.6-sol
b4cf0af to
700a4c4
Compare
Screenshot candidates rendered for parent PR #2383 at `700a4c4e20e1fcb1f8042ea6e1deae7e88903469` by [CI run 34057539633](https://github.com/copse-dev/agent-pane/actions/runs/34057539633). Review GitHub's image diffs, then merge this PR (or enable auto-merge) to apply the accepted references to `copse/something-went-wrong-in-another-copse-thre-d07699`. This branch contains only PNG candidates from the immutable `reference-screenshot-candidates-34057539633` artifact and never targets `main`. If the parent branch has advanced beyond the source SHA above, do not merge this PR; the successful CI run for the new head will replace it. Co-authored-by: jonathanKingston <338988+jonathanKingston@users.noreply.github.com>
…#2551) Closes #2474 (P2, `area:ui`). ## The bug Cmd/Ctrl+W deletes the active thread. Nothing checked whether a dialog was on screen, so **closing Settings with the keystroke that closes things everywhere else destroyed a conversation instead.** There is no undo. ```ts if (meta && e.key === 'w') { e.preventDefault() void confirmDeleteThread() // ← no idea Settings is open } ``` ## The fix The guard reads the DOM — *is any `<dialog>` open* — rather than asking a list of `isXOpen()` predicates. That shape is deliberate. The renderer has **seventeen** dialogs, and the one hand-maintained list of them sits three lines above this in the Cmd/Ctrl+F handler, naming **four**: settings, the command palette, file search, keyboard shortcuts. That list is exactly the failure the issue anticipates when it says *"or any other dialog for that matter"*, so the check is built so a new dialog is covered the moment it exists, with nobody having to remember to register it. **A behaviour change worth noticing:** the find bar now uses the same check, so it also defers to the thirteen dialogs the list missed — an approval, an SSH passphrase, a confirm. Each is a question it should not open underneath, but it is a change beyond the reported bug, so push back if you'd rather I left the four-item list alone. `show()` counts as well as `showModal()`: the approval prompt appears **inline over the chat** rather than modally, and it is still something the user is answering. ### `preventDefault()` stays outside the guard It is what keeps the keystroke from also reaching macOS's File ▸ Close accelerator (`app-menu-file-items.ts` registers `{ role: 'close' }` there). Moving it inside would hand Cmd+W back to the menu exactly when a dialog is up. So the keystroke is still swallowed; only the delete is skipped. Doing nothing is the right answer rather than closing the dialog for them — Esc already does that, in the handler immediately below, and every dialog honours it. ## Left alone deliberately The shortcut is matched inline as `meta && e.key === 'w'` rather than through a `matchXShortcut` helper like its neighbours in `keyboard-shortcuts.ts`. One consequence: with Caps Lock on, `e.key` is `'W'` and the shortcut silently does nothing. Routing it through a matcher would be tidier and testable in the same place as its siblings — but it would also **widen** which keystrokes delete a thread, and that is the wrong direction to move a destructive shortcut without being asked. Happy to do it if you want it. ## Testing `main.ts` binds its shortcuts inside a boot function with no seam to call, so the wiring is pinned at the source level — the file's existing test does the same for the layout boot — while the guard itself is unit-tested against real `<dialog>` elements in happy-dom. The wiring assertions test the **property**, not the text: *every* `confirmDeleteThread()` call site is guarded, so a second unguarded one added later fails the suite rather than slipping past an assertion that only looked at the first. | Check | Result | |---|---| | `node scripts/run-tests.mts` (whole repo) | **8641 tests, 8631 pass, 10 skipped, 0 fail** | | `tsc --noEmit` (node + web) | clean | | `pnpm run lint` / `oxfmt --check .` | clean | | `check:dead-code` (840 modules) / `check:oracle` (243 specs, 15 invariants) | clean | | `check:e2e-syntax` (269 files) / `demo:site:check` | clean | | all three wiring assertions with the guard removed | fail, as designed | 9 new tests: 5 on `isAnyDialogOpen` (closed dialog, modal, non-modal, several at once, and one inserted as raw HTML that nothing registered), 3 on the Cmd+W / Cmd+F wiring, and the existing suite unchanged. Two local-environment notes, neither affecting CI: reaching a full run needs an uncommitted shim over node-pty's native loader (it cannot be rebuilt in this sandbox — the agent proxy denies `iojs.org` and `www.electronjs.org`), reverted before committing; and this container's `node_modules` predates #2383, so `patches/@anthropic-ai__sandbox-runtime@0.0.74.patch` was not applied and `git-commit-signing.test.ts` failed until I applied its one-line hunk by hand. That failure reproduces identically on `52c839a` with my changes stashed, so it is a stale install here, not a base-branch break. Refs #2474 --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Hpk1gEma9LhMUuvr2TwUj6 --- _Generated by [Claude Code](https://claude.ai/code/session_01Hpk1gEma9LhMUuvr2TwUj6)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jonathan Kingston <KingstonMailBox@gmail.com>
Summary
git_commitsubprocess to reach the single Unix socket named bySSH_AUTH_SOCK.sshdirectory remain unreadableallowUnixSocketsoverride reaches the macOS seatbelt profileCloses copse-dev/agent-pane#2320.
The dependency patch can be removed after anthropics/sandbox-runtime#420 ships in a release.
Security boundary
The grant is macOS-only, requires explicit user consent, validates that
SSH_AUTH_SOCKis an absolute normalized socket, and applies only to the native commit spawn. Network domains remain denied. Git hooks inherit the commit process and can ask ssh-agent to use any loaded key, so the UI recommendsssh-add -c.Validation
pnpm test -- git-commit-signing— 12 passedpnpm test— 8,555 passedpnpm run typecheckpnpm run lintpnpm run format:checkpnpm run check:dead-codepnpm run check:oraclepnpm run check:e2e-syntaxpnpm run demo:site:checkpnpm run buildpnpm install --frozen-lockfile --ignore-scriptsgit diff --checkVisual evidence
Co-Authored-By: Copse noreply@copse.dev
Copse-Models: acp:codex-acp#gpt-5.6-sol, acp:claude-acp#opus[1m]