Repository navigation
fix(cli): fail fast when the broker credential fd is not inherited - #211
Merged
Merged
Conversation
In broker mode fduty reaches Flashduty through a control socket the runner passes as an inherited fd (FLASHDUTY_CRED_FD). Programs that spawn fduty without passing inherited fds - Python's subprocess (close_fds=True by default), Node's child_process, sudo - leave that fd closed or reused by an unrelated file. The failure then only surfaced at the first request as "broker handshake send: bad file descriptor" (or "socket operation on non-socket"), after the SDK's request URL prefix, with no hint of the fix. newBrokerHTTPClient now checks that the fd is an open socket before building the client and returns an error that names the fd and how to keep it open (run from the shell, Python pass_fds, Node stdio entry). It returns (*http.Client, error), so the non-unix stub returns errBrokerUnsupported directly and root.go drops its nil check.
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
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.
Problem
In broker mode fduty talks to Flashduty through a control socket that the runner passes as an inherited fd (
FLASHDUTY_CRED_FD). Callers that spawn fduty without passing inherited fds close or reuse that fd:subprocess(close_fds=Trueby default)child_processsudoUntil now this failed only at the first request, and the cause came last, after the SDK's request URL:
Nothing in the message says how to fix it.
Change
newBrokerHTTPClientnow checksgetsockopt(SOL_SOCKET, SO_TYPE)on the fd before it builds the client. If the fd is not an open socket, it returns this error:The function now returns
(*http.Client, error). As a result:errBrokerUnsupporteddirectly, and that sentinel is defined only inbroker_dial_other.go;root.gono longer needs its nil check.Verification
makepasses: golangci-lint reports 0 issues,go test -race ./...passes, and the build succeeds.GOOS=linux go vetandGOOS=windows go buildboth pass.internal/clitests pass on Linux (SOCK_SEQPACKET) in apython:3.12-slimcontainer.TestDefaultNewClient_CredFDNotInherited, covers a closed fd and a regular-file fd.subprocess.run(["fduty", ...])call with default arguments now prints the error above as its first stderr line. Withpass_fds=(N,), the handshake reaches the control socket.