Code fixes

2026-05-25 09:51:14 +02:00
parent 24f77e7b74
commit fe71a4e60c
9 changed files with 295 additions and 266 deletions
@@ -35,9 +35,9 @@ import (
 var Version = "0.0.0-dev"

 const (
-	envIssuer     = "SHERLOCK_AUTHENTIK_ISSUER"
-	envClientID   = "SHERLOCK_AUTHENTIK_CLIENT_ID"
-	envScopes     = "SHERLOCK_AUTHENTIK_SCOPES"
+	envIssuer      = "SHERLOCK_AUTHENTIK_ISSUER"
+	envClientID    = "SHERLOCK_AUTHENTIK_CLIENT_ID"
+	envScopes      = "SHERLOCK_AUTHENTIK_SCOPES"
 	envIdleTimeout = "SHERLOCK_BROKER_IDLE"

 	defaultScopes      = "openid profile email offline_access"
@@ -105,12 +105,20 @@ func runDaemon() {
 		fmt.Fprintln(os.Stderr, "sherlock-broker:", err)
 		os.Exit(4)
 	}
-	defer lock.Release()
+	defer func() {
+		if err := lock.Release(); err != nil {
+			log.Printf("pidlock release: %v", err)
+		}
+	}()

 	cfg := loadConfig()

 	flow := authn.NewFlow(cfg, nil)
-	defer flow.Close()
+	defer func() {
+		if err := flow.Close(); err != nil {
+			log.Printf("flow close: %v", err)
+		}
+	}()
 	refresher := authn.NewRefresher(cfg, nil)

 	profiles, err := agent.Load(agent.LoadOptions{UserDir: mustAgentsDir()})
@@ -5,99 +5,98 @@
 //
 // Routing rules:
 //
-//sherlock version            → print version, no broker
-//sherlock login              → ensure broker, OAuth flow, persist tokens
-//sherlock logout [--shutdown]→ ensure broker, clear tokens, maybe stop it
-//sherlock status             → ensure broker, print status table
-//sherlock run <agent> [...]  → ensure broker, load profile, exec agent
-//sherlock <agent-name> [...] → alias for `run <agent-name>` when the
-//                              first arg matches a loaded profile name
+// sherlock version            → print version, no broker
+// sherlock login              → ensure broker, OAuth flow, persist tokens
+// sherlock logout [--shutdown]→ ensure broker, clear tokens, maybe stop it
+// sherlock status             → ensure broker, print status table
+// sherlock run <agent> [...]  → ensure broker, load profile, exec agent
+// sherlock <agent-name> [...] → alias for `run <agent-name>` when the
+//
+//	first arg matches a loaded profile name
 package main

 import (
-"context"
-"encoding/json"
-"errors"
-"flag"
-"fmt"
-"os"
-"os/exec"
-"runtime"
-"time"
+	"context"
+	"flag"
+	"fmt"
+	"os"
+	"os/exec"
+	"runtime"
+	"time"

-"gitea.alexandru.macocian.me/Charlie/sherlock/internal/agent"
-"gitea.alexandru.macocian.me/Charlie/sherlock/internal/broker"
-"gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring"
-"gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc"
-"gitea.alexandru.macocian.me/Charlie/sherlock/internal/socket"
-"gitea.alexandru.macocian.me/Charlie/sherlock/internal/xdg"
+	"gitea.alexandru.macocian.me/Charlie/sherlock/internal/agent"
+	"gitea.alexandru.macocian.me/Charlie/sherlock/internal/broker"
+	"gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring"
+	"gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc"
+	"gitea.alexandru.macocian.me/Charlie/sherlock/internal/socket"
+	"gitea.alexandru.macocian.me/Charlie/sherlock/internal/xdg"
 )

 // Version is overwritten at build time via -ldflags "-X main.Version=...".
 var Version = "0.0.0-dev"

 const (
-exitOK         = 0
-exitGeneric    = 1
-exitUsage      = 2
-exitKeyring    = 3
-exitBroker     = 4
-exitAuth       = 5
-exitLoginWait  = 5 * time.Minute
+	exitOK        = 0
+	exitGeneric   = 1
+	exitUsage     = 2
+	exitKeyring   = 3
+	exitBroker    = 4
+	exitAuth      = 5
+	exitLoginWait = 5 * time.Minute
 )

 func main() {
-if len(os.Args) < 2 {
-usage(os.Stderr)
-os.Exit(exitUsage)
-}
+	if len(os.Args) < 2 {
+		usage(os.Stderr)
+		os.Exit(exitUsage)
+	}

-sub := os.Args[1]
-rest := os.Args[2:]
+	sub := os.Args[1]
+	rest := os.Args[2:]

-switch sub {
-case "version", "--version", "-v":
-fmt.Println(Version)
-return
-case "help", "--help", "-h":
-usage(os.Stdout)
-return
-}
+	switch sub {
+	case "version", "--version", "-v":
+		fmt.Println(Version)
+		return
+	case "help", "--help", "-h":
+		usage(os.Stdout)
+		return
+	}

-// Every non-trivial subcommand needs the keyring up.
-if err := keyring.Probe(); err != nil {
-fmt.Fprintln(os.Stderr, "sherlock:", err)
-fmt.Fprintln(os.Stderr, "Hint:", keyring.RemediationHint())
-os.Exit(exitKeyring)
-}
+	// Every non-trivial subcommand needs the keyring up.
+	if err := keyring.Probe(); err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock:", err)
+		fmt.Fprintln(os.Stderr, "Hint:", keyring.RemediationHint())
+		os.Exit(exitKeyring)
+	}

-switch sub {
-case "login":
-runLogin(rest)
-case "logout":
-runLogout(rest)
-case "status":
-runStatus(rest)
-case "run":
-if len(rest) == 0 {
-fmt.Fprintln(os.Stderr, "sherlock: run requires an agent name")
-os.Exit(exitUsage)
-}
-runAgent(rest[0], rest[1:])
-default:
-// Agent alias: `sherlock copilot ...` -> `run copilot ...`
-if profileExists(sub) {
-runAgent(sub, rest)
-return
-}
-fmt.Fprintf(os.Stderr, "sherlock: unknown subcommand %q\n", sub)
-usage(os.Stderr)
-os.Exit(exitUsage)
-}
+	switch sub {
+	case "login":
+		runLogin(rest)
+	case "logout":
+		runLogout(rest)
+	case "status":
+		runStatus(rest)
+	case "run":
+		if len(rest) == 0 {
+			fmt.Fprintln(os.Stderr, "sherlock: run requires an agent name")
+			os.Exit(exitUsage)
+		}
+		runAgent(rest[0], rest[1:])
+	default:
+		// Agent alias: `sherlock copilot ...` -> `run copilot ...`
+		if profileExists(sub) {
+			runAgent(sub, rest)
+			return
+		}
+		fmt.Fprintf(os.Stderr, "sherlock: unknown subcommand %q\n", sub)
+		usage(os.Stderr)
+		os.Exit(exitUsage)
+	}
 }

 func usage(w *os.File) {
-fmt.Fprintf(w, `sherlock - Charlie credential broker + agent wrapper
+	res, err := fmt.Fprintf(w, `sherlock - Charlie credential broker + agent wrapper

 Usage:
  sherlock <subcommand> [args...]
@@ -114,199 +113,199 @@ Environment (consumed by the broker, set before `+"`sherlock login`"+`):
  SHERLOCK_AUTHENTIK_ISSUER, SHERLOCK_AUTHENTIK_CLIENT_ID,
  SHERLOCK_AUTHENTIK_SCOPES (optional), SHERLOCK_BROKER_IDLE (optional).
 `)
+
+	if err != nil || res == 0 {
+		fmt.Fprintln(os.Stderr, "sherlock: usage:", err)
+		os.Exit(exitGeneric)
+	}
 }

 func dialBroker(ctx context.Context) *socket.Client {
-sockPath, err := xdg.SocketPath()
-if err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: socket path:", err)
-os.Exit(exitBroker)
-}
-cli, err := broker.EnsureRunning(ctx, broker.EnsureRunningOptions{SocketPath: sockPath})
-if err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: cannot reach broker:", err)
-os.Exit(exitBroker)
-}
-return cli
+	sockPath, err := xdg.SocketPath()
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: socket path:", err)
+		os.Exit(exitBroker)
+	}
+	cli, err := broker.EnsureRunning(ctx, broker.EnsureRunningOptions{SocketPath: sockPath})
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: cannot reach broker:", err)
+		os.Exit(exitBroker)
+	}
+	return cli
 }

 func runLogin(args []string) {
-fs := flag.NewFlagSet("login", flag.ExitOnError)
-timeout := fs.Duration("timeout", exitLoginWait, "how long to wait for browser callback")
-_ = fs.Parse(args)
+	fs := flag.NewFlagSet("login", flag.ExitOnError)
+	timeout := fs.Duration("timeout", exitLoginWait, "how long to wait for browser callback")
+	_ = fs.Parse(args)

-ctx, cancel := context.WithTimeout(context.Background(), *timeout+10*time.Second)
-defer cancel()
-cli := dialBroker(ctx)
-defer cli.Close()
+	ctx, cancel := context.WithTimeout(context.Background(), *timeout+10*time.Second)
+	defer cancel()
+	cli := dialBroker(ctx)
+	defer func() { _ = cli.Close() }()

-var start broker.LoginStartResult
-if err := cli.Call(ctx, rpc.MethodLoginStart, struct{}{}, &start); err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: login_start:", err)
-os.Exit(exitAuth)
-}
-fmt.Println("Opening browser for Authentik login...")
-fmt.Println("If it doesn't open, visit:")
-fmt.Println("  ", start.AuthURL)
-if err := openBrowser(start.AuthURL); err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: could not open browser:", err)
-}
+	var start broker.LoginStartResult
+	if err := cli.Call(ctx, rpc.MethodLoginStart, struct{}{}, &start); err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: login_start:", err)
+		os.Exit(exitAuth)
+	}
+	fmt.Println("Opening browser for Authentik login...")
+	fmt.Println("If it doesn't open, visit:")
+	fmt.Println("  ", start.AuthURL)
+	if err := openBrowser(start.AuthURL); err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: could not open browser:", err)
+	}

-wait := broker.LoginWaitParams{State: start.State, TimeoutSeconds: int(timeout.Seconds())}
-if err := cli.Call(ctx, rpc.MethodLoginWait, wait, nil); err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: login failed:", err)
-os.Exit(exitAuth)
-}
-fmt.Println("Logged in.")
+	wait := broker.LoginWaitParams{State: start.State, TimeoutSeconds: int(timeout.Seconds())}
+	if err := cli.Call(ctx, rpc.MethodLoginWait, wait, nil); err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: login failed:", err)
+		os.Exit(exitAuth)
+	}
+	fmt.Println("Logged in.")
 }

 func runLogout(args []string) {
-fs := flag.NewFlagSet("logout", flag.ExitOnError)
-shutdown := fs.Bool("shutdown", false, "also stop the broker daemon")
-_ = fs.Parse(args)
+	fs := flag.NewFlagSet("logout", flag.ExitOnError)
+	shutdown := fs.Bool("shutdown", false, "also stop the broker daemon")
+	_ = fs.Parse(args)

-ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-defer cancel()
-cli := dialBroker(ctx)
-defer cli.Close()
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	cli := dialBroker(ctx)
+	defer func() { _ = cli.Close() }()

-if err := cli.Call(ctx, rpc.MethodLogout, struct{}{}, nil); err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: logout:", err)
-os.Exit(exitAuth)
-}
-fmt.Println("Logged out.")
-if *shutdown {
-// Best-effort: the broker tears down the socket as part of
-// shutdown so the response may race with EOF.
-_ = cli.Call(ctx, rpc.MethodShutdown, struct{}{}, nil)
-fmt.Println("Broker stopped.")
-}
+	if err := cli.Call(ctx, rpc.MethodLogout, struct{}{}, nil); err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: logout:", err)
+		os.Exit(exitAuth)
+	}
+	fmt.Println("Logged out.")
+	if *shutdown {
+		// Best-effort: the broker tears down the socket as part of
+		// shutdown so the response may race with EOF.
+		_ = cli.Call(ctx, rpc.MethodShutdown, struct{}{}, nil)
+		fmt.Println("Broker stopped.")
+	}
 }

 func runStatus(_ []string) {
-ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-defer cancel()
-cli := dialBroker(ctx)
-defer cli.Close()
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	cli := dialBroker(ctx)
+	defer func() { _ = cli.Close() }()

-var st broker.StatusResult
-if err := cli.Call(ctx, rpc.MethodStatus, struct{}{}, &st); err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: status:", err)
-os.Exit(exitGeneric)
-}
-if !st.LoggedIn {
-fmt.Println("Not logged in.")
-} else {
-fmt.Printf("Logged in as %s <%s>\n", st.Name, st.Email)
-fmt.Printf("  subject:        %s\n", st.Subject)
-if st.IDExpiresAt != nil {
-fmt.Printf("  id token until: %s (%s)\n",
-st.IDExpiresAt.Format(time.RFC3339),
-humanizeUntil(*st.IDExpiresAt))
-}
-if st.RefreshExpAt != nil {
-fmt.Printf("  refresh until:  %s (%s)\n",
-st.RefreshExpAt.Format(time.RFC3339),
-humanizeUntil(*st.RefreshExpAt))
-}
-}
-if len(st.Agents) > 0 {
-fmt.Println("Available agents:")
-for _, a := range st.Agents {
-fmt.Printf("  - %s\n", a)
-}
-}
+	var st broker.StatusResult
+	if err := cli.Call(ctx, rpc.MethodStatus, struct{}{}, &st); err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: status:", err)
+		os.Exit(exitGeneric)
+	}
+	if !st.LoggedIn {
+		fmt.Println("Not logged in.")
+	} else {
+		fmt.Printf("Logged in as %s <%s>\n", st.Name, st.Email)
+		fmt.Printf("  subject:        %s\n", st.Subject)
+		if st.IDExpiresAt != nil {
+			fmt.Printf("  id token until: %s (%s)\n",
+				st.IDExpiresAt.Format(time.RFC3339),
+				humanizeUntil(*st.IDExpiresAt))
+		}
+		if st.RefreshExpAt != nil {
+			fmt.Printf("  refresh until:  %s (%s)\n",
+				st.RefreshExpAt.Format(time.RFC3339),
+				humanizeUntil(*st.RefreshExpAt))
+		}
+	}
+	if len(st.Agents) > 0 {
+		fmt.Println("Available agents:")
+		for _, a := range st.Agents {
+			fmt.Printf("  - %s\n", a)
+		}
+	}
 }

 func runAgent(name string, userArgs []string) {
-profiles, err := loadProfiles()
-if err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: load agent profiles:", err)
-os.Exit(exitGeneric)
-}
-p, ok := profiles[name]
-if !ok {
-fmt.Fprintf(os.Stderr, "sherlock: unknown agent %q. Loaded: %v\n", name, agent.SortedNames(profiles))
-os.Exit(exitUsage)
-}
+	profiles, err := loadProfiles()
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: load agent profiles:", err)
+		os.Exit(exitGeneric)
+	}
+	p, ok := profiles[name]
+	if !ok {
+		fmt.Fprintf(os.Stderr, "sherlock: unknown agent %q. Loaded: %v\n", name, agent.SortedNames(profiles))
+		os.Exit(exitUsage)
+	}

-// Ensure broker is up so future MCPs (Phase 2+) can reach it.
-ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-cli := dialBroker(ctx)
-_ = cli.Close()
-cancel()
+	// Ensure broker is up so future MCPs (Phase 2+) can reach it.
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	cli := dialBroker(ctx)
+	_ = cli.Close()
+	cancel()

-// Phase 1: empty servers map. Phase 2 fills this from services.d.
-mcpPath, err := agent.RenderMCPConfig(p, agent.MCPConfig{Servers: map[string]agent.MCPServer{}})
-if err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: render mcp config:", err)
-os.Exit(exitGeneric)
-}
+	// Phase 1: empty servers map. Phase 2 fills this from services.d.
+	mcpPath, err := agent.RenderMCPConfig(p, agent.MCPConfig{Servers: map[string]agent.MCPServer{}})
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: render mcp config:", err)
+		os.Exit(exitGeneric)
+	}

-argv0, err := agent.ResolveExecutable(p)
-if err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: resolve executable:", err)
-os.Exit(exitGeneric)
-}
-argv := agent.BuildArgv(p, mcpPath, userArgs)
-argv[0] = argv0
-sockPath, _ := xdg.SocketPath()
-env := agent.BuildEnv(p, mcpPath, map[string]string{"SHERLOCK_SOCKET": sockPath})
+	argv0, err := agent.ResolveExecutable(p)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: resolve executable:", err)
+		os.Exit(exitGeneric)
+	}
+	argv := agent.BuildArgv(p, mcpPath, userArgs)
+	argv[0] = argv0
+	sockPath, _ := xdg.SocketPath()
+	env := agent.BuildEnv(p, mcpPath, map[string]string{"SHERLOCK_SOCKET": sockPath})

-if err := (agent.SyscallExecer{}).Exec(argv0, argv, env); err != nil {
-fmt.Fprintln(os.Stderr, "sherlock: exec:", err)
-os.Exit(exitGeneric)
-}
+	if err := (agent.SyscallExecer{}).Exec(argv0, argv, env); err != nil {
+		fmt.Fprintln(os.Stderr, "sherlock: exec:", err)
+		os.Exit(exitGeneric)
+	}
 }

 func profileExists(name string) bool {
-profiles, err := loadProfiles()
-if err != nil {
-return false
-}
-_, ok := profiles[name]
-return ok
+	profiles, err := loadProfiles()
+	if err != nil {
+		return false
+	}
+	_, ok := profiles[name]
+	return ok
 }

 func loadProfiles() (map[string]agent.Profile, error) {
-dir, err := xdg.AgentsConfigDir()
-if err != nil {
-return nil, err
-}
-return agent.Load(agent.LoadOptions{UserDir: dir})
+	dir, err := xdg.AgentsConfigDir()
+	if err != nil {
+		return nil, err
+	}
+	return agent.Load(agent.LoadOptions{UserDir: dir})
 }

 func openBrowser(url string) error {
-var bin string
-var args []string
-switch runtime.GOOS {
-case "darwin":
-bin = "open"
-args = []string{url}
-case "windows":
-bin = "rundll32"
-args = []string{"url.dll,FileProtocolHandler", url}
-default:
-bin = "xdg-open"
-args = []string{url}
-}
-cmd := exec.Command(bin, args...)
-if err := cmd.Start(); err != nil {
-return err
-}
-go func() { _ = cmd.Wait() }()
-return nil
+	var bin string
+	var args []string
+	switch runtime.GOOS {
+	case "darwin":
+		bin = "open"
+		args = []string{url}
+	case "windows":
+		bin = "rundll32"
+		args = []string{"url.dll,FileProtocolHandler", url}
+	default:
+		bin = "xdg-open"
+		args = []string{url}
+	}
+	cmd := exec.Command(bin, args...)
+	if err := cmd.Start(); err != nil {
+		return err
+	}
+	go func() { _ = cmd.Wait() }()
+	return nil
 }

 func humanizeUntil(t time.Time) string {
-d := time.Until(t).Round(time.Second)
-if d < 0 {
-return "expired"
+	d := time.Until(t).Round(time.Second)
+	if d < 0 {
+		return "expired"
+	}
+	return "in " + d.String()
 }
-return "in " + d.String()
-}
-
-// Silence unused-import noise if a stub is wired in later (keeps the
-// json/errors imports honest in case the CLI grows a custom parser).
-var _ = json.Marshal
-var _ = errors.New