Skip to content

Commit a239892

Browse files
authored
feat(broker): route fduty egress through the runner broker (#35)
* feat(broker): per-dial SCM_RIGHTS http client for sandbox egress * feat(broker): route fduty egress through runner broker when FLASHDUTY_CRED_FD is set * fix(broker): reject stdio fds and cap idle broker conns - FLASHDUTY_CRED_FD must be >= 3: fds 0/1/2 are stdio and can never be the runner-injected control end, so reject them instead of handshaking on stdin/stdout. - MaxIdleConnsPerHost=1 on the broker transport: all dials target the same sentinel host over the one control fd, so a single idle keep-alive conn suffices and dispatched conns don't linger. - Tests: stdio/invalid fd rejection, the 0xFF broker-refusal path surfaces as an error (no hang), and both-keys-set still takes the broker path (the sentinel, not the configured app key, reaches the wire). * fix(broker): check deferred Close return values in test (errcheck) First broker PR, so the linter surfaces the original test helpers too. Wrap the deferred syscall.Close / conn.Close calls so errcheck passes. * fix(broker): wake test broker goroutine with Shutdown, not Close (Linux) The CLI broker tests deadlocked on linux-amd64 CI (10m timeout) while passing on macOS/Windows: a bare close() does not interrupt a recvmsg blocked on that fd in another goroutine on Linux (it does on darwin/BSD). fakeBroker and TestBrokerHTTPClient_RefusedReturnsError join their control goroutine after teardown, so they hung waiting for a recvmsg that never returned. Use syscall.Shutdown(SHUT_RDWR) to wake the blocked recvmsg portably, then join, then close. Verified: the cross-compiled linux/arm64 test binary runs clean (count=2) in an ubuntu container; native darwin still passes.
1 parent 8c0898c commit a239892

6 files changed

Lines changed: 437 additions & 6 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//go:build darwin
2+
3+
package cli
4+
5+
import "syscall"
6+
7+
// controlSockType is the control-channel socket type used by the broker dial
8+
// test. darwin's AF_UNIX has no SOCK_SEQPACKET support, so the native test
9+
// falls back to SOCK_DGRAM, which preserves datagram boundaries identically for
10+
// the CLI dialer's Sendmsg/Recvmsg+SCM_RIGHTS path. Production runners are
11+
// Linux-only (SOCK_SEQPACKET).
12+
const controlSockType = syscall.SOCK_DGRAM
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//go:build linux
2+
3+
package cli
4+
5+
import "syscall"
6+
7+
// controlSockType is the control-channel socket type used by the broker dial
8+
// test. On Linux (production) the runner uses SOCK_SEQPACKET.
9+
const controlSockType = syscall.SOCK_SEQPACKET

internal/cli/broker_dial_other.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//go:build !unix
2+
3+
package cli
4+
5+
import (
6+
"errors"
7+
"net/http"
8+
)
9+
10+
func newBrokerHTTPClient(int) *http.Client { return nil }
11+
12+
var errBrokerUnsupported = errors.New("flashduty: broker mode is not supported on this platform")

internal/cli/broker_dial_unix.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
//go:build unix
2+
3+
package cli
4+
5+
import (
6+
"context"
7+
"errors"
8+
"fmt"
9+
"net"
10+
"net/http"
11+
"os"
12+
"sync"
13+
"syscall"
14+
"time"
15+
)
16+
17+
// errBrokerUnsupported is returned when broker mode is requested on a build that
18+
// cannot provide it. On unix this is effectively unreachable (newBrokerHTTPClient
19+
// never returns nil), but defaultNewClient references it on every platform.
20+
var errBrokerUnsupported = errors.New("flashduty: broker mode is not supported on this platform")
21+
22+
// brokerDialer owns the inherited control fd and serializes per-dial handshakes.
23+
// Each Dial sends a 1-byte request datagram on the control channel and receives
24+
// one dedicated SOCK_STREAM fd back via SCM_RIGHTS.
25+
type brokerDialer struct {
26+
mu sync.Mutex // serialize send+recv so concurrent dials don't cross fds
27+
credFD int
28+
}
29+
30+
func (d *brokerDialer) dial(_ context.Context, _, _ string) (net.Conn, error) {
31+
d.mu.Lock()
32+
defer d.mu.Unlock()
33+
34+
if err := syscall.Sendmsg(d.credFD, []byte{0x01}, nil, nil, 0); err != nil {
35+
return nil, fmt.Errorf("broker handshake send: %w", err)
36+
}
37+
body := make([]byte, 1)
38+
oob := make([]byte, syscall.CmsgSpace(4)) // room for exactly one fd
39+
n, oobn, _, _, err := syscall.Recvmsg(d.credFD, body, oob, 0)
40+
if err != nil {
41+
return nil, fmt.Errorf("broker handshake recv: %w", err)
42+
}
43+
if n < 1 || body[0] != 0x01 {
44+
return nil, fmt.Errorf("broker refused connection (code %v)", body[:n])
45+
}
46+
scms, err := syscall.ParseSocketControlMessage(oob[:oobn])
47+
if err != nil {
48+
return nil, fmt.Errorf("broker parse scm: %w", err)
49+
}
50+
if len(scms) == 0 {
51+
return nil, fmt.Errorf("broker sent no fd")
52+
}
53+
fds, err := syscall.ParseUnixRights(&scms[0])
54+
if err != nil || len(fds) == 0 {
55+
return nil, fmt.Errorf("broker parse rights: %w", err)
56+
}
57+
f := os.NewFile(uintptr(fds[0]), "broker-conn")
58+
conn, err := net.FileConn(f) // dups + registers with the netpoller
59+
_ = f.Close()
60+
if err != nil {
61+
return nil, fmt.Errorf("broker fileconn: %w", err)
62+
}
63+
return conn, nil
64+
}
65+
66+
// newBrokerHTTPClient builds an *http.Client whose Transport.DialContext routes
67+
// every connection over the inherited control fd. Timeout matches the SDK's
68+
// historical default (30s) so behavior is unchanged for non-streaming calls;
69+
// streaming export relies on request context like before.
70+
func newBrokerHTTPClient(credFD int) *http.Client {
71+
d := &brokerDialer{credFD: credFD}
72+
return &http.Client{
73+
Timeout: 30 * time.Second,
74+
Transport: &http.Transport{
75+
DialContext: d.dial,
76+
DisableCompression: false,
77+
MaxIdleConns: 0,
78+
// All dials target the same logical host (the broker sentinel base
79+
// URL) over the one control fd, so a single idle keep-alive conn is
80+
// enough for pagination loops; cap it so dispatched conns don't linger.
81+
MaxIdleConnsPerHost: 1,
82+
IdleConnTimeout: 90 * time.Second,
83+
ResponseHeaderTimeout: 0,
84+
},
85+
}
86+
}

0 commit comments

Comments
 (0)