Skip to content

fix(linux): avoid AF_UNIX mediation starving network connects#1147

Open
stacktrace-agent-dansowter wants to merge 3 commits into
always-further:mainfrom
stacktrace-agent-dansowter:issue-1128-af-unix-mediation
Open

fix(linux): avoid AF_UNIX mediation starving network connects#1147
stacktrace-agent-dansowter wants to merge 3 commits into
always-further:mainfrom
stacktrace-agent-dansowter:issue-1128-af-unix-mediation

Conversation

@stacktrace-agent-dansowter

Copy link
Copy Markdown

Fixes #1128.

This PR was prepared by an AI coding agent under human direction and validation.

Summary

This keeps linux.af_unix_mediation = "pathname" focused on AF_UNIX pathname mediation without exhausting the supervisor's small network-notification rate-limit bucket on unrelated TCP/DNS traffic.

The change has two parts:

  • In AF_UNIX-only mediation, parse socket addresses before rate limiting and only spend the network-notification token bucket when the notification actually involves AF_UNIX. Proxy mediation still rate-limits network notifications broadly.
  • Permit the supervised child to bootstrap seccomp notification by allowing sendmsg only on the private child-supervisor socket fd, so it can pass the seccomp notify fd via SCM_RIGHTS. That inherited bootstrap fd is closed before exec so the exemption is not available to the sandboxed program.

References Consulted

Attribution

The implementation extends existing project code in the Linux supervisor and seccomp filter builders. The new rate-limit decision helper, bootstrap-fd filter variants, and tests were newly written for this PR, following the existing in-repository BPF/filter construction style.

Validation

Targeted local validation:

  • nix shell nixpkgs#rustc nixpkgs#cargo -c cargo build -p nono-cli
  • nix shell nixpkgs#rustc nixpkgs#cargo -c cargo test -p nono bootstrap_sendmsg_exemption
  • nix shell nixpkgs#rustc nixpkgs#cargo -c cargo test -p nono-cli network_decision
  • cargo fmt --all --check

Manual acceptance battery against the rebuilt fork binary with af_unix_mediation=pathname:

  • 40 sequential loopback connects: 40/40
  • getaddrinfo loop: 12/12
  • External UDP and TCP checks: passed
  • git ls-remote: passed
  • curl: HTTP 200
  • Mediated AF_UNIX pathname access: denied as expected
  • Credential isolation checks: denied as expected

Note: I did not rerun the full repository make test after a local run attempted to invoke workstation credential-manager tooling. The targeted tests above and the acceptance battery cover the changed behavior.

Agent Compliance Check

  • I am not prohibited from contributing under this policy
  • An issue already exists
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths
  • This PR matches the approved or disclosed issue scope

Signed-off-by: Dan Sowter's Agent <dan.sowter+stacktrace-agent-dansowter@stacktracehq.com>
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +653
Lines removed -32
Total changed 685
Classification Large (> 300 lines)

Affected crates

  • crates/nono (core library) — careful review required. This is the security-critical sandbox primitive. A bug here bypasses OS-level isolation for every downstream user.
  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Moderate

This PR touches: source code,configuration / policy files


Updated automatically on each push to this PR.

@github-actions github-actions Bot added the bug Something isn't working label Jun 15, 2026

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a mechanism to allow a bootstrap sendmsg file descriptor to bypass seccomp network filters during child process initialization, ensuring the supervisor can receive the notify fd without getting trapped. It also refines network notification rate-limiting to avoid charging non-AF_UNIX syscalls in AF_UNIX-only mode. However, a critical double-close bug was identified in exec_strategy.rs where manually closing the child supervisor socket fd and removing it from child_keep_fds causes close_inherited_fds to attempt to close it a second time.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +1015 to +1024
if close_child_supervisor_socket_before_exec && let Some(fd) = child_sock_fd {
// SAFETY: `fd` is the child end of the private supervisor
// socket. All seccomp notify fds have already been sent to
// the parent, so the child must not inherit the bootstrap
// fd-number exemption into the sandboxed program.
unsafe {
libc::close(fd);
}
child_keep_fds.retain(|keep_fd| *keep_fd != fd);
}

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.

high

This block introduces a double-close bug on the file descriptor fd.

Cause

  1. libc::close(fd) is called manually on line 1021.
  2. fd is removed from child_keep_fds on line 1023.
  3. close_inherited_fds(max_fd, &child_keep_fds) is called on line 1026, which iterates from 3 to max_fd and calls libc::close(fd) on any file descriptor not present in child_keep_fds.

Since fd was removed from child_keep_fds, close_inherited_fds will attempt to close fd a second time.

Solution

Remove the manual libc::close(fd) call and let close_inherited_fds handle closing the file descriptor naturally.

            if close_child_supervisor_socket_before_exec && let Some(fd) = child_sock_fd {
                // SAFETY: fd is the child end of the private supervisor
                // socket. All seccomp notify fds have already been sent to
                // the parent, so the child must not inherit the bootstrap
                // fd-number exemption into the sandboxed program.
                child_keep_fds.retain(|keep_fd| *keep_fd != fd);
            }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 92bef16 by removing the manual close and letting close_inherited_fds close the descriptor after it is removed from child_keep_fds. Re-ran cargo fmt --all --check, cargo build -p nono-cli, cargo test -p nono bootstrap_sendmsg_exemption, and cargo test -p nono-cli network_decision.

Signed-off-by: Dan Sowter's Agent <dan.sowter+stacktrace-agent-dansowter@stacktracehq.com>
Path-glob filter that hides known-benign, intentionally-denied Unix-socket
denials from the IPC-denial footer (e.g. /tmp/tmux-*/default for the
deliberately-blocked tmux control socket, /var/run/nscd/socket for glibc's
nscd probe). Reporting-only: the sandbox still DENIES these sockets; this
only suppresses footer noise.

- New DiagnosticsConfig.suppress_unix_sockets (Vec<String>), merged via
  dedup_append, threaded through PreparedProfile -> PreparedSandbox ->
  ExecConfig/ExecutionFlags to the diagnostic emitter.
- Globset matching against the raw target string verbatim (never
  canonicalized), so non-existent sockets ("could not be canonicalized")
  still match. Invalid globs are skipped, not errored, so a malformed entry
  cannot mask unrelated denials. Fast-path when unconfigured keeps output
  byte-identical to before.
- A footer whose only IPC denials are fully suppressed renders identically
  to one with no IPC denials at all.

Tests: nono-cli 1176 passed (incl. new merge dedup test); clippy
(-D warnings -D clippy::unwrap_used) + cargo fmt --check clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

af_unix_mediation: "pathname" exhausts outbound connect() after ~5 calls (EPERM, session-global) on Linux/Landlock

2 participants