Skip to content

Commit 8075782

Browse files
authored
fix(ipc): scope the IPC endpoint to the user running the daemon (#1524)
1 parent a919ca7 commit 8075782

20 files changed

Lines changed: 1418 additions & 36 deletions

.agents/skills/add-cli-command/SKILL.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ description: "Add a Neru CLI command or flag: cobra command in internal/cli, IPC
66
# Adding a CLI command to Neru
77

88
Neru is a daemon plus a thin CLI: almost every command just serializes an IPC
9-
request to the running daemon over the Unix socket (`$TMPDIR/neru.sock`) or
10-
Windows named pipe. A new command therefore touches three layers plus docs —
9+
request to the running daemon over its per-user Unix socket or Windows named
10+
pipe. A new command therefore touches three layers plus docs —
1111
skipping the IPC layer and calling app code directly from `internal/cli` is
1212
wrong even when it compiles.
1313

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ Hard rules that apply everywhere:
6868
- **Errors**: use `derrors` (`derrors.New` / `derrors.Wrap`). Unsupported platform behavior returns `derrors.CodeNotSupported` explicitly — never a silent no-op; callers degrade via `IsNotSupported`.
6969
- **`modes.Handler` has a strict locking contract** — read `internal/app/modes/AGENTS.md` before touching modes or anything that calls back into the handler.
7070

71-
Runtime shape: a daemon plus a thin CLI. `neru launch` starts the daemon; other commands dial a Unix socket (`$TMPDIR/neru.sock`, 0600) or Windows named pipe — transport in `internal/adapter/ipc`, handlers in `internal/app/ipcctrl`. New user-facing behavior usually needs a CLI command, an IPC handler, and the service/mode work behind it (the `add-cli-command` skill walks it). Startup is a numbered, individually-unwound phase sequence in `internal/app/new.go`. Input flow: native event tap → `adapter/eventtap``app/modes/handler.go` → active `Mode``app/services/*` → adapter → native API.
71+
Runtime shape: a daemon plus a thin CLI. `neru launch` starts the daemon; other commands dial a per-user Unix socket (`$XDG_RUNTIME_DIR/neru/neru.sock`, else `$TMPDIR/neru-<uid>/neru.sock`, 0600 in a 0700 directory) or a per-user Windows named pipe (`\\.\pipe\neru-<SID>`) — transport in `internal/adapter/ipc`, handlers in `internal/app/ipcctrl`. The endpoint stays scoped to one user and never widens; `docs/ARCHITECTURE.md` (Runtime Shape) owns the detail. New user-facing behavior usually needs a CLI command, an IPC handler, and the service/mode work behind it (the `add-cli-command` skill walks it). Startup is a numbered, individually-unwound phase sequence in `internal/app/new.go`. Input flow: native event tap → `adapter/eventtap``app/modes/handler.go` → active `Mode``app/services/*` → adapter → native API.
7272

7373
Configuration is hot-reloadable TOML; adding an option touches five links every time and up to four more when it needs them, with a guardrail test behind most of them — read `internal/config/AGENTS.md` or use the `add-config-option` skill. One of the five is the option's **platform column**: every option, mode flag and action declares which of macOS, Linux and Windows writing it does anything on, beside the vocabulary that owns it, and writing an inert one warns at load rather than refusing (`docs/adr/0013-parity-is-measured-in-words-not-subsystems.md`).
7474

SECURITY.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ Neru does **not**:
5858
- Send telemetry, analytics, or crash reports.
5959
- Contact update servers or phone home.
6060

61-
All communication is strictly local — the CLI and daemon talk over a **Unix domain socket** created with owner-only permissions (`0600`) in the system temporary directory.
61+
All communication is strictly local — the CLI and daemon talk over a **Unix domain socket** on macOS and Linux, and a **named pipe** on Windows.
6262

6363
### IPC
6464

65-
The CLI communicates with the running daemon via a local Unix socket using a JSON-based message protocol. The socket is not exposed over the network. Only the local user can connect to it.
65+
The CLI communicates with the running daemon over that endpoint using a JSON-based message protocol. It is never exposed over the network, and it is scoped to the user running the daemon: the socket is mode `0600` inside a `0700` directory that user owns, the named pipe carries that user's SID in its name and a security descriptor naming that SID alone, and on macOS and Linux the daemon additionally reads the connecting process's uid from the kernel and serves only its own. See [ARCHITECTURE.md](docs/ARCHITECTURE.md#runtime-shape) for the exact locations.
6666

6767
### CGo / Objective-C Bridge
6868

docs/ARCHITECTURE.md

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,25 @@ current per-platform support is tracked in
5858

5959
Neru is a **daemon plus a thin CLI**. `neru launch` starts the daemon;
6060
`neru hints`, `neru action left_click`, `neru config reload` and friends dial a
61-
Unix domain socket (`$TMPDIR/neru.sock`, mode 0600) or a Windows named pipe —
62-
see `internal/adapter/ipc` for the transport and
61+
Unix domain socket or a Windows named pipe — see `internal/adapter/ipc` for the
62+
transport and
6363
`internal/app/ipcctrl` for the command handlers.
6464

65+
The endpoint is scoped to one user, in where it lives and in what the daemon
66+
checks before serving a connection:
67+
68+
- **Unix socket**`$XDG_RUNTIME_DIR/neru/neru.sock` where the session
69+
provides a runtime directory, otherwise `$TMPDIR/neru-<uid>/neru.sock`, mode
70+
0600 inside a directory the daemon creates 0700 and owns. The daemon then
71+
reads the connecting process's uid from the kernel and serves only its own.
72+
- **Named pipe**`\\.\pipe\neru-<SID>`, created with a protected DACL naming
73+
that SID alone. There the kernel checks the descriptor before the connection
74+
is ever accepted, which is the same question asked earlier.
75+
76+
`neru doctor` prints the endpoint in use. What each *client* can establish
77+
about the daemon before it connects differs by platform, and is a
78+
[Known Gap](CROSS_PLATFORM.md#known-gaps) rather than part of this shape.
79+
6580
New user-facing behavior therefore usually needs three pieces: a CLI command
6681
(`internal/cli/`, registered in an `init()`), an IPC handler, and the
6782
service/mode work behind it.
@@ -458,8 +473,9 @@ its own.
458473
key logging.
459474
2. **Permissions** — Accessibility permission is required on macOS; Neru requests
460475
only the minimum needed for UI interaction.
461-
3. **IPC security** — the Unix domain socket is created with restricted file
462-
permissions (0600), so only the current user can talk to the daemon.
476+
3. **IPC security** — the endpoint is scoped to one user, and the daemon checks
477+
that for itself rather than trusting the scoping; see Runtime Shape above,
478+
which owns the detail.
463479

464480
---
465481

docs/CROSS_PLATFORM.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,15 @@ working, which is exactly why the build exists.
11801180
11. Font resolution — alias mapping only, no system font enumeration
11811181
12. `neru services` — every subcommand returns `CodeNotSupported`, where macOS
11821182
installs a launchd agent and Linux a systemd user unit
1183+
13. IPC endpoint, client side — the daemon's endpoint is scoped to one user on
1184+
every platform, but only the Unix client checks that for itself before
1185+
connecting. A named pipe carries no ownership a client can read without
1186+
opening it, so the Windows CLI trusts the name it derives from its own SID.
1187+
The same gap covers the upgrade path: a Unix CLI still reaches a daemon
1188+
left running on the previous endpoint and gets the version-mismatch message
1189+
asking for a restart, while on Windows the previous name is named in the
1190+
failure text rather than dialed, and an old daemon has to be stopped by
1191+
hand before `neru launch` starts a new one
11831192

11841193
**macOS**
11851194

docs/TROUBLESHOOTING.md

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -413,8 +413,10 @@ neru launch
413413
# Check status
414414
neru status
415415

416-
# If still failing, check for stale socket (path is printed in logs; typically under /var/folders/.../T)
417-
rm -f /var/folders/*/*/T/neru.sock
416+
# If still failing, clear a stale socket. The daemon prints its endpoint at
417+
# startup; both of these are places it can be, and only one will exist.
418+
rm -f "${XDG_RUNTIME_DIR:-/run/user/$(id -u)}"/neru/neru.sock
419+
rm -f "${TMPDIR:-/tmp}"/neru-"$(id -u)"/neru.sock
418420
neru launch
419421
```
420422

@@ -444,6 +446,26 @@ EOF
444446
neru launch
445447
```
446448

449+
### "Is it running?" right after an upgrade
450+
451+
**The daemon still running is the one the old binary started.**
452+
453+
The IPC endpoint moved to a per-user location (see
454+
[ARCHITECTURE.md](ARCHITECTURE.md#runtime-shape)), so a daemon that has been up
455+
since before the upgrade is listening in the old one.
456+
457+
On macOS and Linux the new CLI still finds it and answers with a version
458+
mismatch asking you to restart the daemon — do that and it moves to the new
459+
endpoint. On Windows it cannot: stop the old daemon before starting the new
460+
one, or you end up running two.
461+
462+
**Solution:**
463+
464+
```bash
465+
pkill neru # taskkill /IM neru.exe on Windows
466+
neru launch
467+
```
468+
447469
### Daemon stops responding
448470

449471
**IPC socket issue or daemon hung.**
@@ -454,8 +476,10 @@ neru launch
454476
# Force quit
455477
pkill -9 neru
456478

457-
# Clean up socket (path is printed in logs; typically under /var/folders/.../T)
458-
rm -f /var/folders/*/*/T/neru.sock
479+
# Clean up the socket. The daemon prints its endpoint at startup; both of these
480+
# are places it can be, and only one will exist.
481+
rm -f "${XDG_RUNTIME_DIR:-/run/user/$(id -u)}"/neru/neru.sock
482+
rm -f "${TMPDIR:-/tmp}"/neru-"$(id -u)"/neru.sock
459483

460484
# Restart
461485
neru launch

internal/adapter/ipc/ipc.go

Lines changed: 85 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ const (
4646
// DefaultSocketPerms is the default socket permissions.
4747
DefaultSocketPerms = 0o600
4848

49+
// maxCommandBytes bounds one command. A command is a verb, a small
50+
// parameter map and its arguments; the largest realistic one — a config
51+
// value or a sequence definition traveling in Args — is orders of
52+
// magnitude below this. The cap exists so a peer cannot make the daemon
53+
// buffer without limit, not to police command shape, which
54+
// DisallowUnknownFields already does.
55+
maxCommandBytes = 64 << 10
56+
4957
// defaultBuildVersion is the fallback version when SetBuildVersion is not called.
5058
defaultBuildVersion = "dev"
5159
)
@@ -133,9 +141,43 @@ type Server struct {
133141
// CommandHandler is the interface for processing IPC commands.
134142
type CommandHandler func(ctx context.Context, cmd Command) Response
135143

136-
// SocketPath returns the platform IPC endpoint path (Unix socket or named pipe).
144+
// SocketPath returns the platform IPC endpoint path (Unix socket or named pipe)
145+
// a client should use.
146+
//
147+
// It is the endpoint the daemon listens on in every ordinary case. Where the
148+
// transport can tell that a live daemon answers somewhere else — an endpoint
149+
// left by a daemon started before this version, or one in a runtime directory
150+
// this process's environment does not name — it returns that instead, so a CLI
151+
// reaches the daemon that is actually running rather than reporting none.
137152
func SocketPath() string {
138-
return endpointPath()
153+
return clientEndpointPath()
154+
}
155+
156+
// errCommandTooLarge is returned by boundedReader once a command has spent its
157+
// budget. It is a sentinel so the decode failure can be reported as the size
158+
// refusal it is rather than as the truncated JSON it looks like.
159+
var errCommandTooLarge = errors.New("command exceeds the maximum size")
160+
161+
// boundedReader is io.LimitReader with a distinguishable ending: hitting the
162+
// limit is an error rather than a clean EOF.
163+
type boundedReader struct {
164+
reader io.Reader
165+
remaining int64
166+
}
167+
168+
func (b *boundedReader) Read(buf []byte) (int, error) {
169+
if b.remaining <= 0 {
170+
return 0, errCommandTooLarge
171+
}
172+
173+
if int64(len(buf)) > b.remaining {
174+
buf = buf[:b.remaining]
175+
}
176+
177+
read, err := b.reader.Read(buf)
178+
b.remaining -= int64(read)
179+
180+
return read, err
139181
}
140182

141183
// NewServer creates a new IPC server instance with the specified handler.
@@ -145,7 +187,9 @@ func NewServer(handler CommandHandler, logger *zap.Logger) (*Server, error) {
145187
}
146188

147189
logger = logger.Named("ipc")
148-
socketPath := SocketPath()
190+
// The daemon binds the transport's preferred endpoint, never the one
191+
// SocketPath may have resolved to some other running daemon's.
192+
socketPath := daemonEndpointPath()
149193

150194
listener, listenerErr := listenEndpoint(context.Background(), socketPath)
151195
if listenerErr != nil {
@@ -273,6 +317,17 @@ func (s *Server) handleConnection(connection net.Conn) {
273317
s.wg.Done()
274318
}()
275319

320+
// Who is on the other end is settled before anything they sent is read.
321+
// A connection that fails this gets no reply at all: there is nothing
322+
// useful to tell a caller that should not have reached the daemon, and a
323+
// reply would confirm the endpoint to whoever found it.
324+
authErr := authorizePeer(connection)
325+
if authErr != nil {
326+
logger.Warn("Refused an IPC connection from another user", zap.Error(authErr))
327+
328+
return
329+
}
330+
276331
// Only the read side is bounded here: this guards a client that connects
277332
// and never sends a command. The write side gets its own deadline once the
278333
// handler has finished, in writeResponse.
@@ -283,7 +338,7 @@ func (s *Server) handleConnection(connection net.Conn) {
283338
return
284339
}
285340

286-
decoder := json.NewDecoder(connection)
341+
decoder := json.NewDecoder(&boundedReader{reader: connection, remaining: maxCommandBytes})
287342
decoder.DisallowUnknownFields()
288343

289344
encoder := json.NewEncoder(connection)
@@ -306,6 +361,18 @@ func (s *Server) handleConnection(connection net.Conn) {
306361

307362
decodeCommandErr := decoder.Decode(&cmd)
308363
if decodeCommandErr != nil {
364+
if errors.Is(decodeCommandErr, errCommandTooLarge) {
365+
logger.Error("Refused an oversized command", zap.Int("limit_bytes", maxCommandBytes))
366+
367+
reply(Response{
368+
Success: false,
369+
Message: fmt.Sprintf("command exceeds the %d byte limit", maxCommandBytes),
370+
Code: CodeInvalidInput,
371+
})
372+
373+
return
374+
}
375+
309376
logger.Error("Failed to decode command", zap.Error(decodeCommandErr))
310377

311378
reply(Response{
@@ -422,7 +489,7 @@ func (c *Client) SendWithTimeout(cmd Command, timeout time.Duration) (Response,
422489
return Response{}, derrors.Wrap(
423490
connectionErr,
424491
derrors.CodeIPCFailed,
425-
"failed to connect to neru (is it running?)",
492+
connectFailureMessage(),
426493
)
427494
}
428495

@@ -510,6 +577,19 @@ func (c *Client) SendWithTimeout(cmd Command, timeout time.Duration) (Response,
510577
return response, nil
511578
}
512579

580+
// connectFailureMessage explains a connection that never got off the ground,
581+
// adding whatever the transport can say about where else a daemon might be.
582+
func connectFailureMessage() string {
583+
message := "failed to connect to neru (is it running?)"
584+
585+
hint := endpointHint()
586+
if hint != "" {
587+
message += "; " + hint
588+
}
589+
590+
return message
591+
}
592+
513593
// IsServerRunning determines if the IPC server is currently accepting connections.
514594
// It returns true even when the daemon has a different build version — the
515595
// version mismatch error will surface when the actual command is sent.

internal/adapter/ipc/ipc_internal_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,73 @@ package ipc
33
import (
44
"encoding/json"
55
"errors"
6+
"io"
67
"net"
8+
"strings"
79
"testing"
810
"time"
911
)
1012

13+
// Running out of budget has to be distinguishable from a stream that simply
14+
// ended, or an oversized command would be reported as malformed JSON.
15+
func TestBoundedReader_Read(t *testing.T) {
16+
t.Parallel()
17+
18+
const payload = "hello"
19+
20+
tests := []struct {
21+
name string
22+
input string
23+
remaining int64
24+
want string
25+
wantErr error
26+
}{
27+
{
28+
name: "reads a payload that fits",
29+
input: payload,
30+
remaining: 16,
31+
want: payload,
32+
wantErr: nil,
33+
},
34+
{
35+
// A decoder stops as soon as it has a whole JSON value, so it never
36+
// asks for the byte past the budget; io.ReadAll always does.
37+
name: "spends the budget exactly",
38+
input: payload,
39+
remaining: 5,
40+
want: payload,
41+
wantErr: errCommandTooLarge,
42+
},
43+
{
44+
name: "refuses a payload past the budget",
45+
input: payload + " world",
46+
remaining: 5,
47+
want: payload,
48+
wantErr: errCommandTooLarge,
49+
},
50+
}
51+
52+
for _, testCase := range tests {
53+
t.Run(testCase.name, func(t *testing.T) {
54+
t.Parallel()
55+
56+
reader := &boundedReader{
57+
reader: strings.NewReader(testCase.input),
58+
remaining: testCase.remaining,
59+
}
60+
61+
read, err := io.ReadAll(reader)
62+
if !errors.Is(err, testCase.wantErr) {
63+
t.Fatalf("io.ReadAll() error = %v, want %v", err, testCase.wantErr)
64+
}
65+
66+
if string(read) != testCase.want {
67+
t.Errorf("io.ReadAll() = %q, want %q", read, testCase.want)
68+
}
69+
})
70+
}
71+
}
72+
1173
// A handler is free to outlive the deadline set when the connection was
1274
// accepted — an action sequence can sleep, or wait for the user to finish a
1375
// mode. The reply must still be attempted, or a command that succeeded would

0 commit comments

Comments
 (0)