From 25d5da8ed1d1e3d303bfb898b821c06dbfd32be1 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Mon, 8 Jun 2026 20:45:43 +0800 Subject: [PATCH 1/2] feat(version): advertise broker_egress capability in version --json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runner probes `fduty version --json` to learn whether the bundled fduty can act as a broker-mode client (read FLASHDUTY_CRED_FD and dial over the inherited control fd). It advertises broker mode to safari only when the probe reports broker_egress=true; otherwise safari would deliver the per-person key out-of-band to an fduty that cannot read it, breaking auth. broker_egress is a compile-time capability (true on unix, where broker_dial_unix.go is built; false elsewhere) — not a version comparison — so dev builds and future versions are handled uniformly. An older fduty ignores --json for the version command and prints the plain line, so the missing field reads as 'not capable', and the runner falls back to the legacy env-key path. --- internal/cli/broker_dial_other.go | 6 ++++ internal/cli/broker_dial_unix.go | 7 +++++ internal/cli/version.go | 22 ++++++++++++++- internal/cli/version_test.go | 46 +++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 internal/cli/version_test.go diff --git a/internal/cli/broker_dial_other.go b/internal/cli/broker_dial_other.go index d516138..0b7cb51 100644 --- a/internal/cli/broker_dial_other.go +++ b/internal/cli/broker_dial_other.go @@ -10,3 +10,9 @@ import ( func newBrokerHTTPClient(int) *http.Client { return nil } var errBrokerUnsupported = errors.New("flashduty: broker mode is not supported on this platform") + +// brokerEgressCapable reports whether this build can act as a broker-mode client. +// False on non-unix builds, which lack the FLASHDUTY_CRED_FD dial path. The +// runner reads this (via `fduty version --json`) to decide whether to advertise +// broker mode to safari. See broker_dial_unix.go for the rationale. +const brokerEgressCapable = false diff --git a/internal/cli/broker_dial_unix.go b/internal/cli/broker_dial_unix.go index 7070ed7..4e631d2 100644 --- a/internal/cli/broker_dial_unix.go +++ b/internal/cli/broker_dial_unix.go @@ -19,6 +19,13 @@ import ( // never returns nil), but defaultNewClient references it on every platform. var errBrokerUnsupported = errors.New("flashduty: broker mode is not supported on this platform") +// brokerEgressCapable reports whether this build can act as a broker-mode client +// (read FLASHDUTY_CRED_FD and dial over the inherited control fd). The runner +// probes it via `fduty version --json` and only advertises broker mode to safari +// when true — otherwise safari would deliver the per-person key out-of-band to an +// fduty that can't read it. True on unix, where this file is built. +const brokerEgressCapable = true + // brokerDialer owns the inherited control fd and serializes per-dial handshakes. // Each Dial sends a 1-byte request datagram on the control channel and receives // one dedicated SOCK_STREAM fd back via SCM_RIGHTS. diff --git a/internal/cli/version.go b/internal/cli/version.go index 28bc276..6cc9391 100644 --- a/internal/cli/version.go +++ b/internal/cli/version.go @@ -24,7 +24,27 @@ func newVersionCmd() *cobra.Command { Use: "version", Short: "Print version information", Run: func(cmd *cobra.Command, args []string) { - fmt.Printf("flashduty version %s (%s) built %s\n", versionStr, commitStr, dateStr) + // A structured (--json / --output-format) request emits a + // machine-readable object that includes broker_egress, the capability + // the runner probes before advertising broker mode to safari. An older + // fduty ignores these flags and prints the plain line below, so the + // absence of the broker_egress field reads as "not capable". The plain + // human output is unchanged. + if currentOutputFormat().Structured() { + b, err := marshalStructured(map[string]any{ + "version": versionStr, + "commit": commitStr, + "date": dateStr, + "broker_egress": brokerEgressCapable, + }) + if err != nil { + fmt.Fprintln(cmd.ErrOrStderr(), err) + return + } + fmt.Fprintln(cmd.OutOrStdout(), string(b)) + return + } + fmt.Fprintf(cmd.OutOrStdout(), "flashduty version %s (%s) built %s\n", versionStr, commitStr, dateStr) }, } } diff --git a/internal/cli/version_test.go b/internal/cli/version_test.go new file mode 100644 index 0000000..e387fb6 --- /dev/null +++ b/internal/cli/version_test.go @@ -0,0 +1,46 @@ +package cli + +import ( + "bytes" + "encoding/json" + "strings" + "testing" +) + +// TestVersionPlainOutput keeps the human-readable line stable (the runner's +// install scripts and humans rely on it). +func TestVersionPlainOutput(t *testing.T) { + cmd := newVersionCmd() + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.Run(cmd, nil) + if got := buf.String(); !strings.HasPrefix(got, "flashduty version ") { + t.Fatalf("plain version output changed: %q", got) + } +} + +// TestVersionJSONAdvertisesBrokerEgress locks the capability contract the runner +// depends on: `fduty version --json` must emit a broker_egress field that is true +// on this (unix) build. If this regresses, broker-capable runners would stop +// advertising broker mode and silently fall back to the legacy env-key path. +func TestVersionJSONAdvertisesBrokerEgress(t *testing.T) { + origJSON := flagJSON + flagJSON = true + defer func() { flagJSON = origJSON }() + + cmd := newVersionCmd() + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.Run(cmd, nil) + + var got struct { + Version string `json:"version"` + BrokerEgress bool `json:"broker_egress"` + } + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatalf("version --json must be valid JSON, got %q: %v", buf.String(), err) + } + if !got.BrokerEgress { + t.Fatalf("broker_egress must be true on a unix build, got: %q", buf.String()) + } +} From 5da0748cb09fd0de6920bebd9d504482655d4801 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Mon, 8 Jun 2026 21:31:31 +0800 Subject: [PATCH 2/2] fix(version): satisfy errcheck + make broker_egress test platform-correct - check fmt.Fprintln/Fprintf returns (errcheck under golangci-lint v2.11) - assert broker_egress matches the compile-time capability (true on unix, false on windows) instead of hard-asserting true, which failed the windows build --- internal/cli/version.go | 7 ++++--- internal/cli/version_test.go | 20 +++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/internal/cli/version.go b/internal/cli/version.go index 6cc9391..e7c72c0 100644 --- a/internal/cli/version.go +++ b/internal/cli/version.go @@ -24,6 +24,7 @@ func newVersionCmd() *cobra.Command { Use: "version", Short: "Print version information", Run: func(cmd *cobra.Command, args []string) { + out := cmd.OutOrStdout() // A structured (--json / --output-format) request emits a // machine-readable object that includes broker_egress, the capability // the runner probes before advertising broker mode to safari. An older @@ -38,13 +39,13 @@ func newVersionCmd() *cobra.Command { "broker_egress": brokerEgressCapable, }) if err != nil { - fmt.Fprintln(cmd.ErrOrStderr(), err) + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), err) return } - fmt.Fprintln(cmd.OutOrStdout(), string(b)) + _, _ = fmt.Fprintln(out, string(b)) return } - fmt.Fprintf(cmd.OutOrStdout(), "flashduty version %s (%s) built %s\n", versionStr, commitStr, dateStr) + _, _ = fmt.Fprintf(out, "flashduty version %s (%s) built %s\n", versionStr, commitStr, dateStr) }, } } diff --git a/internal/cli/version_test.go b/internal/cli/version_test.go index e387fb6..7163b6b 100644 --- a/internal/cli/version_test.go +++ b/internal/cli/version_test.go @@ -20,9 +20,10 @@ func TestVersionPlainOutput(t *testing.T) { } // TestVersionJSONAdvertisesBrokerEgress locks the capability contract the runner -// depends on: `fduty version --json` must emit a broker_egress field that is true -// on this (unix) build. If this regresses, broker-capable runners would stop -// advertising broker mode and silently fall back to the legacy env-key path. +// depends on: `fduty version --json` must emit a broker_egress field whose value +// matches this build's compile-time capability (true on unix, false elsewhere). +// If the field went missing, broker-capable runners would stop advertising +// broker mode and silently fall back to the legacy env-key path. func TestVersionJSONAdvertisesBrokerEgress(t *testing.T) { origJSON := flagJSON flagJSON = true @@ -33,14 +34,15 @@ func TestVersionJSONAdvertisesBrokerEgress(t *testing.T) { cmd.SetOut(&buf) cmd.Run(cmd, nil) - var got struct { - Version string `json:"version"` - BrokerEgress bool `json:"broker_egress"` - } + var got map[string]any if err := json.Unmarshal(buf.Bytes(), &got); err != nil { t.Fatalf("version --json must be valid JSON, got %q: %v", buf.String(), err) } - if !got.BrokerEgress { - t.Fatalf("broker_egress must be true on a unix build, got: %q", buf.String()) + v, ok := got["broker_egress"] + if !ok { + t.Fatalf("version --json must include the broker_egress field, got: %q", buf.String()) + } + if v != brokerEgressCapable { + t.Fatalf("broker_egress = %v, want %v (compile-time capability)", v, brokerEgressCapable) } }