feat(broker): route fduty egress through the runner broker#35
Merged
Conversation
- 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).
First broker PR, so the linter surfaces the original test helpers too. Wrap the deferred syscall.Close / conn.Close calls so errcheck passes.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Route fduty egress through the runner broker
When the runner provides a broker control fd (
FLASHDUTY_CRED_FD), the CLI sends every API request over that inherited fd instead of dialing the upstream directly, with a sentinelapp_keythe broker overwrites. This keeps the real per-person key out of the sandbox/bash environment entirely.What changes
internal/cli/broker_dial_unix.go(//go:build unix): a*http.ClientwhoseTransport.DialContextperforms a per-dial SCM_RIGHTS handshake on the control fd — sends a 1-byte request, receives a dedicatedSOCK_STREAMfd viaSCM_RIGHTS, wraps it withnet.FileConn. Per-dial (not once at startup) so keep-alive reconnects and pagination loops each get a fresh connection. A mutex serializes send+recv on the control fd so concurrent requests never cross fds.MaxIdleConnsPerHost=1caps lingering dispatched conns (all dials target the same sentinel host).internal/cli/broker_dial_other.go(//go:build !unix): compile-only stub.internal/cli/root.godefaultNewClient: whenFLASHDUTY_CRED_FDis set (and>= 3— fds 0/1/2 are stdio and rejected), inject the broker client viaflashduty.WithHTTPClientand use a sentinel app_key (go-flashduty rejects empty keys and always appends?app_key=; the broker overwrites it). When both a configured app key and a control fd are present, broker mode wins. No change to the go-flashduty SDK (consumer logic stays in the CLI, per the 1:1 rule).Verification
TestBrokerHTTPClient_DialAndRewrite+TestDefaultNewClient_BrokerModepass natively (unix syscalls work on darwin); 8 concurrent requests each get their own dispatched connection and the upstream sees the real (rewritten) key.TestDefaultNewClient_RejectsStdioFD: fds -1/0/1/2 are rejected.TestBrokerHTTPClient_RefusedReturnsError: a0xFFbroker refusal surfaces as an error (no hang). Both-keys-set takes the broker path.fdutyran inside a BYOC runner and authenticated against the real pgy purely via the broker fd (no env key present).