From c9081ee2da7a8dbad071cc173325c608e75d5cbe Mon Sep 17 00:00:00 2001 From: Alexandru Macocian Date: Mon, 25 May 2026 10:37:21 +0200 Subject: [PATCH] Remove broker --- README.md | 3 +- cmd/sherlock-broker/main.go | 207 ------------------ cmd/sherlock/main.go | 207 ++++++++---------- docs/agents.md | 17 +- docs/architecture.md | 73 ++++--- docs/auth-model.md | 35 ++- docs/conventions.md | 8 +- docs/gssh-integration.md | 4 +- docs/rpc.md | 77 ------- docs/service-registry.md | 16 +- docs/storage.md | 2 +- go.mod | 2 - go.sum | 4 - internal/agent/agent.go | 12 +- internal/agent/agent_test.go | 7 +- internal/agent/claude.go | 7 +- internal/agent/copilot.go | 7 +- internal/authn/authn.go | 54 +++-- internal/authn/flow.go | 339 ------------------------------ internal/authn/flow_test.go | 199 ------------------ internal/authn/login.go | 226 ++++++++++++++++++++ internal/authn/login_test.go | 331 +++++++++++++++++++++++++++++ internal/authn/refresh.go | 174 +++++++++------ internal/broker/doc.go | 9 - internal/broker/lifecycle.go | 70 ------ internal/broker/lifecycle_test.go | 33 --- internal/broker/methods.go | 226 -------------------- internal/broker/methods_test.go | 110 ---------- internal/broker/server.go | 68 ------ internal/broker/server_test.go | 58 ----- internal/broker/spawn.go | 142 ------------- internal/keyring/keyring.go | 24 ++- internal/rpc/rpc.go | 113 ---------- internal/rpc/rpc_test.go | 69 ------ internal/socket/helpers_test.go | 10 - internal/socket/socket.go | 251 ---------------------- internal/socket/socket_test.go | 166 --------------- internal/xdg/xdg.go | 26 +-- internal/xdg/xdg_test.go | 4 +- plan.md | 16 +- 40 files changed, 902 insertions(+), 2504 deletions(-) delete mode 100644 cmd/sherlock-broker/main.go delete mode 100644 docs/rpc.md delete mode 100644 internal/authn/flow.go delete mode 100644 internal/authn/flow_test.go create mode 100644 internal/authn/login.go create mode 100644 internal/authn/login_test.go delete mode 100644 internal/broker/doc.go delete mode 100644 internal/broker/lifecycle.go delete mode 100644 internal/broker/lifecycle_test.go delete mode 100644 internal/broker/methods.go delete mode 100644 internal/broker/methods_test.go delete mode 100644 internal/broker/server.go delete mode 100644 internal/broker/server_test.go delete mode 100644 internal/broker/spawn.go delete mode 100644 internal/rpc/rpc.go delete mode 100644 internal/rpc/rpc_test.go delete mode 100644 internal/socket/helpers_test.go delete mode 100644 internal/socket/socket.go delete mode 100644 internal/socket/socket_test.go diff --git a/README.md b/README.md index 0fb5af6..61f9861 100644 --- a/README.md +++ b/README.md @@ -9,8 +9,7 @@ Design + phasing: [plan.md](plan.md). - [Architecture](docs/architecture.md) - [Auth model](docs/auth-model.md) - [Storage & keyring](docs/storage.md) -- [Broker RPC](docs/rpc.md) - [Service registry](docs/service-registry.md) -- [Agent profiles](docs/agents.md) +- [Agents](docs/agents.md) - [gssh integration](docs/gssh-integration.md) - [Conventions](docs/conventions.md) diff --git a/cmd/sherlock-broker/main.go b/cmd/sherlock-broker/main.go deleted file mode 100644 index dc6fbbb..0000000 --- a/cmd/sherlock-broker/main.go +++ /dev/null @@ -1,207 +0,0 @@ -// Command sherlock-broker is the long-running per-user daemon that -// owns the Authentik OAuth state, caches tokens in the OS keyring, -// hosts the loopback HTTP listener for browser callbacks, and answers -// JSON-over-newline RPCs on $XDG_RUNTIME_DIR/sherlock.sock. -// -// Decisions locked in Phase 1: -// - OS keyring for token persistence (probe at startup, fail fast). -// - JSON-over-newline framing on the UDS RPC. -// - Forked-child lifecycle: the wrapper CLI starts the broker as a -// setsid-detached process; this binary refuses to start twice via -// a PID-file flock. -// - Loopback HTTP callback on 127.0.0.1:6990. -package main - -import ( - "context" - "flag" - "fmt" - "log" - "os" - "os/signal" - "strings" - "syscall" - "time" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/agent" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/authn" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/broker" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring" - "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 ( - envIssuer = "SHERLOCK_AUTHENTIK_ISSUER" - envClientID = "SHERLOCK_AUTHENTIK_CLIENT_ID" - envScopes = "SHERLOCK_AUTHENTIK_SCOPES" - envIdleTimeout = "SHERLOCK_BROKER_IDLE" - - defaultScopes = "openid profile email offline_access" - defaultIdleTimeout = time.Hour -) - -func main() { - flag.Usage = func() { - fmt.Fprintf(os.Stderr, `sherlock-broker - Charlie credential broker daemon - -Usage: - sherlock-broker - -Subcommands: - daemon Run the broker (foreground; the wrapper CLI usually invokes - this via fork+setsid). Refuses to start if another broker - holds the per-user PID lock. - version Print the broker version and exit. - -Environment: - %s Authentik OIDC issuer URL (required for login). - %s Authentik OAuth2 client ID (required for login). - %s Space-separated scopes (default: "%s"). - %s Idle timeout, e.g. 30m, 1h (default: %s). -`, - envIssuer, envClientID, envScopes, defaultScopes, - envIdleTimeout, defaultIdleTimeout) - } - flag.Parse() - - args := flag.Args() - if len(args) == 0 { - flag.Usage() - os.Exit(2) - } - - switch args[0] { - case "version", "--version", "-v": - fmt.Println(Version) - case "daemon": - runDaemon() - default: - fmt.Fprintf(os.Stderr, "sherlock-broker: unknown subcommand %q\n", args[0]) - flag.Usage() - os.Exit(2) - } -} - -func runDaemon() { - log.SetFlags(log.LstdFlags | log.Lmicroseconds) - log.SetPrefix("sherlock-broker: ") - - if err := keyring.Probe(); err != nil { - fmt.Fprintln(os.Stderr, "sherlock-broker:", err) - fmt.Fprintln(os.Stderr, "Hint:", keyring.RemediationHint()) - os.Exit(3) - } - - pidPath, err := xdg.PidPath() - if err != nil { - log.Fatalf("pid path: %v", err) - } - lock, err := broker.AcquirePidLock(pidPath) - if err != nil { - fmt.Fprintln(os.Stderr, "sherlock-broker:", err) - os.Exit(4) - } - defer func() { - if err := lock.Release(); err != nil { - log.Printf("pidlock release: %v", err) - } - }() - - cfg := loadConfig() - - flow := authn.NewFlow(cfg, nil) - defer func() { - if err := flow.Close(); err != nil { - log.Printf("flow close: %v", err) - } - }() - refresher := authn.NewRefresher(cfg, nil) - - rootCtx, cancel := context.WithCancel(context.Background()) - defer cancel() - - methods := &broker.Methods{ - Store: keyring.NewStore(), - Flow: flow, - Refresher: refresher, - Cfg: cfg, - ShutdownFn: cancel, - AgentNames: agent.Names, - } - - svc := broker.NewService(methods.MethodTable()) - - sockPath, err := xdg.SocketPath() - if err != nil { - log.Fatalf("socket path: %v", err) - } - srv, err := socket.Listen(sockPath, svc) - if err != nil { - log.Fatalf("listen: %v", err) - } - log.Printf("listening on %s (pid=%d)", sockPath, os.Getpid()) - - // Signal handler — graceful shutdown on SIGINT/SIGTERM. - sigCh := make(chan os.Signal, 1) - signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) - go func() { - s := <-sigCh - log.Printf("received %s, shutting down", s) - cancel() - }() - - // Idle-timeout watcher. - idle := loadIdleTimeout() - if idle > 0 { - go watchIdle(rootCtx, svc, idle, cancel) - } - - if err := srv.Serve(rootCtx); err != nil { - log.Printf("serve: %v", err) - } - log.Printf("shutdown complete") -} - -func watchIdle(ctx context.Context, svc *broker.Service, idle time.Duration, cancel context.CancelFunc) { - ticker := time.NewTicker(idle / 4) - defer ticker.Stop() - for { - select { - case <-ctx.Done(): - return - case <-ticker.C: - if time.Since(svc.LastActivity()) >= idle { - log.Printf("idle for %s, exiting", idle) - cancel() - return - } - } - } -} - -func loadConfig() authn.Config { - scopes := defaultScopes - if s := os.Getenv(envScopes); s != "" { - scopes = s - } - return authn.Config{ - Issuer: os.Getenv(envIssuer), - ClientID: os.Getenv(envClientID), - Scopes: strings.Fields(scopes), - } -} - -func loadIdleTimeout() time.Duration { - if v := os.Getenv(envIdleTimeout); v != "" { - d, err := time.ParseDuration(v) - if err == nil { - return d - } - log.Printf("invalid %s=%q: %v (using default)", envIdleTimeout, v, err) - } - return defaultIdleTimeout -} diff --git a/cmd/sherlock/main.go b/cmd/sherlock/main.go index a0745bf..a90140b 100644 --- a/cmd/sherlock/main.go +++ b/cmd/sherlock/main.go @@ -1,22 +1,14 @@ -// Command sherlock is the operator-facing CLI: login / logout / status -// and the agent-spawning router. It owns no auth state of its own — -// every credential operation is an RPC to sherlock-broker, which it -// transparently forks the first time it's needed (Decision #10). -// -// 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 [...] → ensure broker, load profile, exec agent -// sherlock [...] → alias for `run ` when the -// -// first arg matches a loaded profile name +// Command sherlock is the operator's single binary: it logs in to +// Authentik (one-shot loopback HTTP listener), persists tokens in the +// OS keyring, and dispatches `sherlock ` to the matching agent +// CLI (Copilot, Claude Code, …). There is no daemon — refresh races +// across concurrent invocations are serialised via flock on a +// per-user lock file. See docs/architecture.md. package main import ( "context" + "errors" "flag" "fmt" "os" @@ -26,24 +18,26 @@ import ( "time" "gitea.alexandru.macocian.me/Charlie/sherlock/internal/agent" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/broker" + "gitea.alexandru.macocian.me/Charlie/sherlock/internal/authn" "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 + exitGeneric = 1 + exitUsage = 2 + exitKeyring = 3 + exitAuth = 5 +) + +const ( + envIssuer = "SHERLOCK_AUTHENTIK_ISSUER" + envClientID = "SHERLOCK_AUTHENTIK_CLIENT_ID" + envScopes = "SHERLOCK_AUTHENTIK_SCOPES" + + defaultScopes = "openid profile email offline_access" ) func main() { @@ -51,7 +45,6 @@ func main() { usage(os.Stderr) os.Exit(exitUsage) } - sub := os.Args[1] rest := os.Args[2:] @@ -64,7 +57,6 @@ func main() { 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()) @@ -85,7 +77,6 @@ func main() { } runAgent(rest[0], rest[1:]) default: - // Agent alias: `sherlock copilot ...` -> `run copilot ...` if _, ok := agent.Get(sub); ok { runAgent(sub, rest) return @@ -104,116 +95,109 @@ Usage: Subcommands: login Open a browser, log in to Authentik, persist tokens. - logout [--shutdown] Clear tokens; optionally stop the broker daemon. + logout Clear stored tokens. status Show login state, token expiry, available agents. - run [args...] Spawn an agent with broker-injected credentials. + run [args...] Spawn an agent. [args...] Alias for `+"`run ...`"+`. Known agents: %s. version Print the sherlock version and exit. -Environment (consumed by the broker, set before `+"`sherlock login`"+`): - SHERLOCK_AUTHENTIK_ISSUER, SHERLOCK_AUTHENTIK_CLIENT_ID, - SHERLOCK_AUTHENTIK_SCOPES (optional), SHERLOCK_BROKER_IDLE (optional). -`, strings.Join(agent.Names(), ", ")) +Environment (consumed by `+"`sherlock login`"+`): + %s Authentik OIDC issuer URL (required). + %s Authentik OAuth2 client ID (required). + %s Space-separated scopes (default: %q). +`, + strings.Join(agent.Names(), ", "), + envIssuer, envClientID, envScopes, defaultScopes) } -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) +func loadAuthnConfig() authn.Config { + scopes := defaultScopes + if v := os.Getenv(envScopes); v != "" { + scopes = v } - 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 authn.Config{ + Issuer: os.Getenv(envIssuer), + ClientID: os.Getenv(envClientID), + Scopes: strings.Fields(scopes), } - return cli } func runLogin(args []string) { fs := flag.NewFlagSet("login", flag.ExitOnError) - timeout := fs.Duration("timeout", exitLoginWait, "how long to wait for browser callback") + timeout := fs.Duration("timeout", 5*time.Minute, "how long to wait for the browser callback") _ = fs.Parse(args) - ctx, cancel := context.WithTimeout(context.Background(), *timeout+10*time.Second) + cfg := loadAuthnConfig() + if !cfg.Configured() { + fmt.Fprintf(os.Stderr, + "sherlock: Authentik not configured. Set %s and %s in your environment.\n", + envIssuer, envClientID) + os.Exit(exitAuth) + } + + ctx, cancel := context.WithTimeout(context.Background(), *timeout) 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) + opts := authn.LoginOptions{ + OnAuthURL: func(u string) { + fmt.Println("Opening browser to:") + fmt.Println(" ", u) + if err := openBrowser(u); err != nil { + fmt.Fprintln(os.Stderr, "sherlock: open browser:", err) + fmt.Println("(Browser didn't open — visit the URL above manually.)") + } + }, } - 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) + res, err := authn.Login(ctx, cfg, opts) + if err != nil { + fmt.Fprintln(os.Stderr, "sherlock: login:", err) os.Exit(exitAuth) } - fmt.Println("Logged in.") + if err := keyring.NewStore().SetAuthentikTokens(res.Tokens); err != nil { + fmt.Fprintln(os.Stderr, "sherlock: persist tokens:", err) + os.Exit(exitKeyring) + } + fmt.Printf("Logged in as %s <%s>.\n", res.Tokens.Name, res.Tokens.Email) } -func runLogout(args []string) { - 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 func() { _ = cli.Close() }() - - if err := cli.Call(ctx, rpc.MethodLogout, struct{}{}, nil); err != nil { +func runLogout(_ []string) { + if err := keyring.NewStore().ClearAuthentikTokens(); err != nil { fmt.Fprintln(os.Stderr, "sherlock: logout:", err) - os.Exit(exitAuth) + os.Exit(exitKeyring) } 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 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 { + store := keyring.NewStore() + ts, err := store.GetAuthentikTokens() + switch { + case errors.Is(err, keyring.ErrNoTokens): 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 { + case err != nil: + fmt.Fprintln(os.Stderr, "sherlock: status:", err) + os.Exit(exitKeyring) + default: + fmt.Printf("Logged in as %s <%s>\n", ts.Name, ts.Email) + fmt.Printf(" subject: %s\n", ts.Subject) + fmt.Printf(" issuer: %s\n", ts.Issuer) + if !ts.IDExpiresAt.IsZero() { fmt.Printf(" id token until: %s (%s)\n", - st.IDExpiresAt.Format(time.RFC3339), - humanizeUntil(*st.IDExpiresAt)) + ts.IDExpiresAt.Format(time.RFC3339), + humanizeUntil(ts.IDExpiresAt)) } - if st.RefreshExpAt != nil { + if !ts.RefreshExpAt.IsZero() { fmt.Printf(" refresh until: %s (%s)\n", - st.RefreshExpAt.Format(time.RFC3339), - humanizeUntil(*st.RefreshExpAt)) + ts.RefreshExpAt.Format(time.RFC3339), + humanizeUntil(ts.RefreshExpAt)) } } - if len(st.Agents) > 0 { + if names := agent.Names(); len(names) > 0 { fmt.Println("Available agents:") - for _, a := range st.Agents { - fmt.Printf(" - %s\n", a) + for _, n := range names { + a, _ := agent.Get(n) + fmt.Printf(" - %-10s %s\n", n, a.Description()) } } } @@ -224,20 +208,7 @@ func runAgent(name string, userArgs []string) { fmt.Fprintf(os.Stderr, "sherlock: unknown agent %q. Known: %v\n", name, agent.Names()) 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() - - sockPath, err := xdg.SocketPath() - if err != nil { - fmt.Fprintln(os.Stderr, "sherlock: socket path:", err) - os.Exit(exitGeneric) - } - - if err := a.Spawn(agent.Context{BrokerSocket: sockPath}, userArgs); err != nil { + if err := a.Spawn(agent.Context{}, userArgs); err != nil { fmt.Fprintln(os.Stderr, "sherlock:", err) os.Exit(exitGeneric) } diff --git a/docs/agents.md b/docs/agents.md index 9b49846..c00deb1 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -8,7 +8,7 @@ How sherlock decides which CLI to spawn when you type | Name | Binary | MCP config flag | Notes | |---|---|---|---| | `copilot` | `copilot` (npm `@github/copilot`) | `--additional-mcp-config @` | Augments user's `~/.copilot/mcp-config.json`. JSON shape is the VS Code MCP schema (`{"servers": ...}`). | -| `claude` | `claude` (npm `@anthropic-ai/claude-code`) | `--mcp-config ` | JSON shape is Claude Code's `.mcp.json` schema (`{"mcpServers": ...}`). `ANTHROPIC_API_KEY` is stripped from the child env so a personal key can't override the broker-managed session. | +| `claude` | `claude` (npm `@anthropic-ai/claude-code`) | `--mcp-config ` | JSON shape is Claude Code's `.mcp.json` schema (`{"mcpServers": ...}`). `ANTHROPIC_API_KEY` is stripped from the child env so a personal key can't override the sherlock-managed session. | ## Routing @@ -26,16 +26,15 @@ bare alias is the daily-driver form. ## What sherlock does per spawn 1. `keyring.Probe()` — fail fast if the OS keyring isn't available. -2. `broker.EnsureRunning` — dial the broker socket, fork it if absent. -3. Resolve the agent binary on `$PATH`. Friendly error if missing. -4. Render the per-agent MCP config to +2. Resolve the agent binary on `$PATH`. Friendly error if missing. +3. Render the per-agent MCP config to `$XDG_RUNTIME_DIR/sherlock/.mcp.json` (0600). In Phase 1 the servers map is always empty; Phase 2 populates it from `services.d/`. -5. Build the child argv with the agent-specific flag. -6. Build the child env: parent env → drop forbidden keys → overlay - `SHERLOCK_SOCKET=` so future MCPs (Phase 2+) can reach the - broker. -7. `syscall.Exec` — sherlock disappears, the agent takes its place. +4. Build the child argv with the agent-specific flag. +5. Build the child env: parent env minus per-agent forbids. MCPs + spawned by the agent will read tokens straight from the OS keyring + (Phase 2+). +6. `syscall.Exec` — sherlock disappears, the agent takes its place. ## Adding a new agent diff --git a/docs/architecture.md b/docs/architecture.md index 68f13fd..a40c17f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -4,58 +4,57 @@ Condensed from [`../plan.md`](../plan.md). Read this first; jump to the plan for ## One-paragraph summary -Sherlock is a per-user credential broker + agent-CLI wrapper that runs on the operator's workstation. It owns a single Authentik session, exchanges it for per-service tokens on demand, injects those tokens as environment variables into thin stdio MCP servers, and then `exec`s the agent CLI of your choice (Copilot, Claude Code, …) with the right MCP config. No long-lived service tokens live on disk in the clear, and the agent never sees a credential it isn't supposed to. +Sherlock is a per-user credential broker + agent-CLI wrapper that runs on the operator's workstation. It owns a single Authentik session (persisted in the OS keyring), exchanges it for per-service tokens on demand, injects those tokens as environment variables into thin stdio MCP servers, and then `exec`s the agent CLI of your choice (Copilot, Claude Code, …) with the right MCP config. No long-lived service tokens live on disk in the clear, and the agent never sees a credential it isn't supposed to. ## Diagram -``` -┌────────────────────────────────────────────────────────────┐ -│ user │ -│ │ │ -│ │ `sherlock run copilot` (or `sherlock copilot`) │ -│ ▼ │ -│ ┌─────────────┐ spawns, injects env ┌──────────────┐ │ -│ │ copilot CLI │ ────────────────────▶ │ gitea-mcp │ │ -│ │ │ │ grafana-mcp │ │ -│ │ │ │ gssh-mcp │ │ -│ └─────┬───────┘ └──────┬───────┘ │ -│ │ unix socket │ │ -│ ▼ ▼ │ -│ ┌──────────────────────────────────────────────────┐ │ -│ │ sherlock-broker (systemd --user daemon) │ │ -│ │ - owns the Authentik OAuth state │ │ -│ │ - per-service token cache + refresh │ │ -│ │ - RFC 8693 token exchange │ │ -│ │ - hosts loopback HTTP for browser callbacks │ │ -│ │ - exposes sherlock-mcp (stdio) for consent flow │ │ -│ └────────────────────┬─────────────────────────────┘ │ -│ │ OIDC / token exchange │ -│ ▼ │ -│ Authentik (id.alexandru.macocian.me) │ -└────────────────────────────────────────────────────────────┘ +```mermaid +flowchart TD + user["user shell"] -->|`sherlock copilot`
or `sherlock run copilot`| sh[sherlock CLI] + + sh -->|syscall.Exec| agent[copilot / claude CLI] + sh -.->|writes MCP config| mcpfile[(XDG_RUNTIME_DIR/sherlock/<agent>.mcp.json)] + agent -.->|reads| mcpfile + + agent -->|stdio| gitea[gitea-mcp] + agent -->|stdio| grafana[grafana-mcp] + agent -->|stdio| gssh[gssh-mcp] + + gitea --> keyring[(OS keyring
Secret Service / Keychain / Cred Mgr)] + grafana --> keyring + gssh --> keyring + sh --> keyring + + keyring -->|OIDC refresh on demand
flock-serialised| authentik[Authentik
id.alexandru.macocian.me] ``` ## Components | Component | Lives at | Owns | |---|---|---| -| `sherlock` (CLI) | `cmd/sherlock/` | `login`, `logout`, `status`, `run`, agent-name aliases (`copilot`, `claude`, …). Renders the per-session MCP config, then `exec`s the agent. | -| `sherlock-broker` (daemon) | `cmd/sherlock-broker/` | Long-running per-user daemon. Authentik PKCE; loopback redirect; token cache in the OS keyring; single-flight refresh; RFC 8693 token exchange; Unix-socket RPC. | +| `sherlock` (CLI) | `cmd/sherlock/` | The only binary. `login`, `logout`, `status`, `run`, agent-name aliases (`copilot`, `claude`, …). At login: spins up a one-shot loopback HTTP listener on `127.0.0.1:6990`, runs PKCE, writes the TokenSet to the OS keyring. At spawn: looks up the agent in `internal/agent/`, renders the per-session MCP config, `exec`s the agent. | | `internal/agent/` | — | One Go file per supported CLI (`copilot.go`, `claude.go`, …), each registering itself via `init()`. Shared exec/env helpers live alongside. See [agents.md](agents.md). | -| `internal/authn/` | — | OIDC / PKCE / token-exchange primitives. See [auth-model.md](auth-model.md). | -| `internal/mcp/` | — | Per-format MCP-config renderers (VS Code shape for Copilot, `.mcp.json` shape for Claude Code). Shared by every agent. | -| `internal/socket/` | — | UDS RPC server (broker side) + client (CLI + MCPs). | -| `cmd/gitea-mcp/` (Phase 2) | — | First per-service MCP. Reads `GITEA_TOKEN` from env, refreshes via broker socket on 401. | +| `internal/authn/` | — | OIDC discovery, PKCE, one-shot Login, file-locked EnsureFresh. See [auth-model.md](auth-model.md). | +| `internal/keyring/` | — | OS keyring wrapper: `Probe` (called at every CLI start), `TokenSet` persistence. See [storage.md](storage.md). | +| `internal/mcp/` | — | Per-format MCP-config renderers (VS Code shape for Copilot, `.mcp.json` shape for Claude Code). | +| `internal/xdg/` | — | Resolves `$XDG_RUNTIME_DIR/sherlock/.mcp.json` and the refresh-lock path. | +| `cmd/gitea-mcp/` (Phase 2) | — | First per-service MCP. Reads tokens directly from the keyring (via `internal/authn`); refreshes via flock-serialised EnsureFresh on 401. | | `cmd/gssh-mcp/` (Phase 3) | — | Thin HTTP+WS client to the existing gssh server. No local certs. See [gssh-integration.md](gssh-integration.md). | | `cmd/grafana-mcp/` (Phase 4) | — | Grafana tools, OIDC-federated via Authentik. | | `cmd/sherlock-mcp/` (Phase 4) | — | The only **interactive** MCP. Exposes `oauth.consent(service)`, `auth.whoami()`, `auth.list_services()`, `auth.revoke(service)`. | -## Why the broker is separate from the wrapper +## Why there is no daemon -- One Authentik browser flow per workstation, shared across every terminal. -- A stable loopback listener for non-Authentik OAuth callbacks. -- Token refresh races are solved once, in one place. -- The wrapper can stay agent-agnostic: it only knows "ask the broker for a token, then exec the agent". Swap Copilot for Claude Code without touching auth code. +The original Phase 1 design had a separate `sherlock-broker` daemon (forked-child, UDS RPC, PID-file flock, idle timer). It was removed in the post-Phase-1 refactor — Decisions #9 (JSON-over-newline RPC) and #10 (forked-child broker) are both superseded. + +Reasoning: + +- The OS keyring is already the single source of truth across processes; a daemon caching it in RAM is redundant on the homelab single-user scale. +- The loopback HTTP listener for PKCE only needs to be alive during `sherlock login` (a few seconds). No reason to keep it running. +- Cross-process refresh races (multiple MCPs hitting 401 simultaneously) are solved by an exclusive `flock()` on `$XDG_RUNTIME_DIR/sherlock.refresh.lock` with the canonical "take lock → re-read → maybe refresh" pattern. +- Service-specific OAuth (Phase 2 own-oauth services like GitHub.com) gets its own explicit `sherlock auth ` command that spins up a fresh listener for the duration of the flow. + +What we lose: a small amount of latency overhead per refresh (libsecret D-Bus round-trip ~1ms) and the ability to handle a mid-session OAuth pop-up without an explicit command. What we gain: no daemon lifecycle, no PID files, no IPC protocol, no idle timers, ~800 LoC less, and "is sherlock-broker still running" stops being a debugging path. ## Boundaries diff --git a/docs/auth-model.md b/docs/auth-model.md index 66f7056..f557ad7 100644 --- a/docs/auth-model.md +++ b/docs/auth-model.md @@ -4,8 +4,8 @@ How sherlock obtains and distributes credentials. ## The two layers -1. **Operator → Authentik.** A single OAuth 2.1 authorization-code + PKCE flow against Authentik (`id.alexandru.macocian.me`). The broker owns the resulting ID/access/refresh tokens. This is the only browser flow per workstation per session. -2. **Broker → downstream service.** For each per-service request from an MCP, the broker mints a service-scoped token using one of the three grant kinds below. +1. **Operator → Authentik.** A single OAuth 2.1 authorization-code + PKCE flow against Authentik (`id.alexandru.macocian.me`), driven by `sherlock login`. The resulting ID/access/refresh tokens land in the OS keyring (see [storage.md](storage.md)). This is the only browser flow per workstation per session. +2. **MCP → downstream service.** Each per-service MCP reads the operator's stored tokens from the keyring (via `internal/authn`) and mints a service-scoped token using one of the three grant kinds below. There is no daemon mediating this — see [architecture.md#why-there-is-no-daemon](architecture.md#why-there-is-no-daemon). ## Grant kinds @@ -13,44 +13,41 @@ Set per service in `~/.config/sherlock/services.d/.toml` (see [service-reg ### `oidc-federated` -The downstream service already federates against Authentik (Gitea, Grafana, Caddy-protected apps). The broker either: +The downstream service already federates against Authentik (Gitea, Grafana, Caddy-protected apps). The MCP either: + - Hands the existing Authentik-issued ID token (when the service's expected audience matches), or - Performs **RFC 8693 token exchange** against Authentik's token endpoint, swapping the operator's ID token for a service-scoped access token. -The MCP receives the token in an env var (e.g. `GITEA_TOKEN`). +The MCP receives the token in an env var (e.g. `GITEA_TOKEN`) injected by sherlock at agent spawn. ### `own-oauth` -The service runs its own OAuth/OIDC stack, not federated through Authentik (think GitHub.com, a SaaS API, …). The broker: -- Holds a per-(operator, service) OAuth client registration. -- Runs the auth-code + PKCE flow in a browser the first time, with the callback received on the broker's loopback listener. -- Caches the refresh token alongside the Authentik creds. -- Refreshes silently after that. +The service runs its own OAuth/OIDC stack, not federated through Authentik (think GitHub.com, a SaaS API, …). Triggered by an explicit `sherlock auth ` command: -Triggered explicitly via `oauth.consent(service)` from `sherlock-mcp` (Phase 4) or via `sherlock login --service `. +- The CLI spins up a fresh loopback listener (separate from the Authentik one on :6990). +- Runs the auth-code + PKCE flow in a browser, with the callback received on that listener. +- Persists the refresh token alongside the Authentik creds, keyed under `service:` in the keyring. +- Subsequent MCP runs refresh silently from the stored refresh token. ### `static-pat` -For services that have no OAuth at all, or where the only sane option is a long-lived PAT. The broker reads the PAT from an `age`-encrypted blob and injects it. Avoid where possible; document why every time it's used. +For services that have no OAuth at all, or where the only sane option is a long-lived PAT. The operator stores the PAT in the keyring once via `sherlock auth --pat`, and the MCP reads it from there. Avoid where possible; document why every time it's used. ## Token storage -See [storage.md](storage.md) for the full decision (OS keyring via -`zalando/go-keyring`, pre-flight at startup, exit code 3 on failure). +See [storage.md](storage.md) for the full decision (OS keyring via `zalando/go-keyring`, pre-flight at startup of every CLI invocation, exit code 3 on failure). -- Authentik creds persist in the OS keyring (Decision #8); per-service - tokens follow the same pattern in Phase 2+. -- In-memory cache mirrors the keyring; writes happen on rotation. -- Refresh is single-flight (`golang.org/x/sync/singleflight`): a 401 - storm across N MCPs collapses to one refresh. +- Authentik creds persist in the OS keyring (Decision #8); per-service tokens follow the same pattern in Phase 2+. +- Refresh races across concurrent processes (multiple MCPs hitting 401 at once) are serialised via an exclusive `flock()` on `$XDG_RUNTIME_DIR/sherlock.refresh.lock`. The canonical "take lock → re-read keyring → maybe refresh" pattern; see `internal/authn/refresh.go`. ## Audience binding -Per the MCP 2025-06-18 spec, downstream tokens MUST be audience-bound. The broker never hands an MCP a token whose `aud` claim doesn't name that service. This is the "confused deputy" mitigation the spec calls out. +Per the MCP 2025-06-18 spec, downstream tokens MUST be audience-bound. The MCP never hands its tool implementation a token whose `aud` claim doesn't name the right service. This is the "confused deputy" mitigation the spec calls out. ## Scope minimisation Default service registrations request read-only scopes. Write scopes require either: + - The operator passing `--write` to `sherlock run` (Phase 5), or - An explicit `scopes` override in the service's TOML. diff --git a/docs/conventions.md b/docs/conventions.md index e57463f..09c99ee 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -34,11 +34,11 @@ No top-level `pkg/` until we have an external consumer. - **One topic per file under `docs/`.** Never append a new section to an existing doc to cover a new concern; create `docs/.md` and link it from `README.md`. - `plan.md` is the long-form design + phasing doc. It is allowed to be long. - Cross-link aggressively: every doc should link to the other docs whose concerns it touches. -- ASCII diagrams over images. Diagrams live next to the prose that explains them. +- Mermaid diagrams over images or ASCII art. Diagrams live next to the prose that explains them. ## Naming -- Binaries are kebab-case (`sherlock-broker`, `gitea-mcp`). +- Binaries are kebab-case (`gitea-mcp`, `gssh-mcp`). - Built-in agent profiles match the agent's canonical CLI name (`copilot`, `claude`, `aider`). - Service registry files match the service's canonical name (`gitea.toml`, `grafana.toml`). - Env vars are `_TOKEN` (e.g. `GITEA_TOKEN`, `GSSH_TOKEN`). @@ -46,11 +46,11 @@ No top-level `pkg/` until we have an external consumer. ## Extensibility invariants - **Agent extensibility:** adding a new agent CLI is a Go file under `internal/agent/` registering itself via `init()`. See [agents.md](agents.md). The TOML-overlay design was tried and dropped — per-CLI quirks (auth subcommands, flag schemas, MCP config shapes) deserve a real code home. -- **Service extensibility:** adding a new downstream service MUST be a TOML drop-in under `~/.config/sherlock/services.d/` + (optionally) a new `cmd/-mcp/` binary. The broker itself does not learn about individual services in Go code. +- **Service extensibility:** adding a new downstream service is a TOML drop-in under `~/.config/sherlock/services.d/` + (optionally) a new `cmd/-mcp/` binary. Sherlock's own code does not learn about individual services. ## Commits -- Conventional-ish: `area: short imperative` (e.g. `broker: persist tokens with age`, `docs: add gssh-integration`). +- Conventional-ish: `area: short imperative` (e.g. `authn: persist client_id in TokenSet`, `docs: add gssh-integration`). - One logical change per commit. CI must pass on every commit on `main`. ## CI diff --git a/docs/gssh-integration.md b/docs/gssh-integration.md index 1d3b447..4b874f7 100644 --- a/docs/gssh-integration.md +++ b/docs/gssh-integration.md @@ -16,14 +16,14 @@ That's the entire job of "let an authorized human run a command on a Charlie hos A thin Go HTTP+WebSocket client to the existing gssh server, wrapped in a stdio MCP. It: -1. Reads `GSSH_TOKEN` from env at startup (an Authentik JWT minted by the broker, `aud=gssh`). +1. Reads `GSSH_TOKEN` from env at startup (an Authentik JWT for `aud=gssh`, written into the env by sherlock at agent spawn from the operator's stored TokenSet). 2. Calls `POST /api/v1/session/initialize` with `Authorization: Bearer ` to materialise the user's session on the gssh server. 3. Caches the list of permitted hosts from `GET /api/v1/session/hosts`. 4. Exposes MCP tools: - `ssh.list_hosts()` — returns the cached host list. - `ssh.run(host, command, timeout?)` — opens a WebSocket to the session route for `host`, writes `command + "; echo __SHERLOCK_DONE__$?\n"`, reads until it sees the sentinel, returns `{stdout, exit_code}`. - `ssh.put_file(host, path, content)` (later) — same shell channel, base64-decode + `tee` on the far side, sentinel as above. -5. On 401, asks the broker for a refreshed token via the UDS RPC and retries once. +5. On 401, re-reads the keyring (calling `authn.EnsureFresh`, which `flock`-serialises against concurrent MCP refreshes) and retries once. No SSH key handling. No cert minting. No `~/Dev/gssh` shell-out. diff --git a/docs/rpc.md b/docs/rpc.md deleted file mode 100644 index 8c22d38..0000000 --- a/docs/rpc.md +++ /dev/null @@ -1,77 +0,0 @@ -# Broker RPC - -The wire protocol between `sherlock` (CLI) and `sherlock-broker` (daemon). - -## Framing — Decision #9 - -JSON-over-newline on a Unix domain socket at -`$XDG_RUNTIME_DIR/sherlock.sock`. - -- One JSON object per line, `\n`-terminated, both directions. -- UTF-8. -- Each connection is independent; the client serialises its own writes, - the server fans out one goroutine per connection. -- Debuggable with `socat - UNIX-CONNECT:$XDG_RUNTIME_DIR/sherlock.sock`. - -### Request - -```json -{ "id": 7, "method": "status", "params": {} } -``` - -### Response - -```json -{ "id": 7, "result": { ... } } -``` - -or - -```json -{ "id": 7, "error": { "code": "not_logged_in", "message": "no Authentik tokens stored" } } -``` - -`id` echoes the request `id`. `result` and `error` are mutually -exclusive; exactly one is present (the other is `null`/omitted). - -## Method catalogue - -| Method | Params | Result | -|---|---|---| -| `status` | `{}` | `{ logged_in, sub?, name?, email?, id_expires_at?, refresh_expires_at?, issuer?, authn_configured, agents[] }` | -| `whoami` | `{}` | `{ sub, name, email }` — errors with `not_logged_in` if no tokens. | -| `login_start` | `{}` | `{ auth_url, state }` — starts the loopback listener if not already up. | -| `login_wait` | `{ state, timeout_seconds }` | `{}` — blocks until the OAuth callback for `state` arrives or the timeout elapses. | -| `logout` | `{}` | `{}` — idempotent; clears the keyring entry. | -| `get_id_token` | `{}` | `{ token, expires_at }` — refreshes via `refresh_token` if needed (single-flight). | -| `shutdown` | `{}` | `{}` — broker exits after flushing the response. | - -## Error codes - -| Code | Meaning | -|---|---| -| `invalid_request` | Malformed JSON, missing `method`, or bad `params`. | -| `unknown_method` | Server doesn't recognise `method`. | -| `not_logged_in` | Operation requires a live token set; none is stored. | -| `not_configured` | Broker is missing Authentik issuer/client ID (Decision #11 deferral). The error message names the env vars to set. | -| `flow_expired` | `login_wait` timed out or the in-flight state was cleared. | -| `keyring_unavailable` | Keyring write/read failed mid-session (rare; probe runs at startup). | -| `internal` | Anything else — message contains the underlying error. | - -Codes are stable strings, not numbers. The CLI matches on them when it -needs branching behaviour (e.g. `sherlock login` suppresses -`already_in_flight` and just calls `login_wait`). - -## Lifecycle interaction - -Per [Decision #10](../plan.md), the wrapper CLI auto-forks the broker -when the socket isn't there. Concretely: - -1. CLI dials `$XDG_RUNTIME_DIR/sherlock.sock`. -2. On `ECONNREFUSED`/`ENOENT`: `fork+exec sherlock-broker daemon` - with `setsid`, stdin/out/err pointed at `$XDG_RUNTIME_DIR/sherlock.log`. -3. CLI polls the socket every 50ms for up to 2s, then retries the dial. -4. Broker holds an exclusive `flock()` on `sherlock.pid`; a second - `daemon` invocation exits silently if the lock is taken. -5. Broker auto-exits after `SHERLOCK_BROKER_IDLE` of no RPC activity - (default 1h). diff --git a/docs/service-registry.md b/docs/service-registry.md index 0930104..fbaddd4 100644 --- a/docs/service-registry.md +++ b/docs/service-registry.md @@ -1,10 +1,10 @@ # Service registry -How operators register a downstream service so the broker can mint per-service tokens. +How operators register a downstream service so sherlock can mint per-service tokens. ## Location -`~/.config/sherlock/services.d/.toml` — one file per service. The broker hot-reloads on change. +`~/.config/sherlock/services.d/.toml` — one file per service. Sherlock loads them at the start of each CLI invocation. A built-in registry baked into the binary would lock the service set at release time. Every service is operator-registered. @@ -38,7 +38,7 @@ authorization_endpoint = "https://github.com/login/oauth/authorize" token_endpoint = "https://github.com/login/oauth/access_token" client_id = "Iv1.abc123" # client_secret is read from a sibling `.secret.age` file, never inline. -redirect_path = "/cb/github" # appended to the broker's loopback base URL +redirect_path = "/cb/github" # appended to sherlock's loopback base URL during `sherlock auth github` scopes = ["read:org", "read:user"] ``` @@ -46,7 +46,7 @@ scopes = ["read:org", "read:user"] ```toml [pat] -# Path to an age-encrypted file containing the PAT. Decrypted by the broker +# Path to an age-encrypted file containing the PAT. Decrypted by sherlock # on demand using the operator's age key. file = "~/.config/sherlock/secrets/dockerhub.pat.age" ``` @@ -55,7 +55,7 @@ file = "~/.config/sherlock/secrets/dockerhub.pat.age" | Field | Required | Notes | |---|---|---| -| `service.name` | yes | Must equal the filename stem. The broker rejects mismatches. | +| `service.name` | yes | Must equal the filename stem. Sherlock rejects mismatches. | | `service.kind` | yes | See [auth-model.md](auth-model.md) for the three grant kinds. | | `service.issuer` | `oidc-federated` only | OIDC issuer URL; used for metadata discovery. | | `service.audience` | `oidc-federated` only | Goes into the `audience` parameter of the token-exchange request. | @@ -66,9 +66,9 @@ file = "~/.config/sherlock/secrets/dockerhub.pat.age" ## Loading -1. The broker scans `~/.config/sherlock/services.d/*.toml` on startup. -2. On filesystem change (fsnotify), it re-parses and atomically swaps its registry. -3. Parse errors are logged but do not crash the broker — the offending service is just marked unavailable. +1. Sherlock scans `~/.config/sherlock/services.d/*.toml` at the start of each CLI invocation (`status`, `run `, etc.). +2. Parse errors are logged to stderr but do not abort — the offending service is just marked unavailable for that invocation. +3. No hot-reload (no daemon). Re-running the command picks up changes. ## Worked examples diff --git a/docs/storage.md b/docs/storage.md index 88f0dbe..22fa35e 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -7,7 +7,7 @@ How sherlock persists secrets and runtime state. | Topic | Decision | |---|---| | Token storage | OS keyring via [`github.com/zalando/go-keyring`](https://github.com/zalando/go-keyring). No on-disk credential files, no `age` blobs, no plaintext. | -| Pre-flight | Both `sherlock` and `sherlock-broker` call `keyring.Probe()` at startup. A missing/locked keyring fails fast with a platform-appropriate hint and exit code `3`. | +| Pre-flight | `sherlock` calls `keyring.Probe()` at startup of every subcommand. A missing/locked keyring fails fast with a platform-appropriate hint and exit code `3`. | | Service name | `sherlock` for real tokens; `sherlock-preflight` for the probe sentinel. | | Account key | The Authentik `sub` claim once logged in, `default` before. | | Runtime files | `$XDG_RUNTIME_DIR/sherlock.sock`, `sherlock.pid`, `sherlock.log`, `sherlock/.mcp.json`. All `0600` (files) / `0700` (dirs). | diff --git a/go.mod b/go.mod index 22a5b58..89fbe9b 100644 --- a/go.mod +++ b/go.mod @@ -3,12 +3,10 @@ module gitea.alexandru.macocian.me/Charlie/sherlock go 1.25.0 require ( - github.com/BurntSushi/toml v1.6.0 github.com/coreos/go-oidc/v3 v3.18.0 github.com/go-jose/go-jose/v4 v4.1.4 github.com/zalando/go-keyring v0.2.8 golang.org/x/oauth2 v0.36.0 - golang.org/x/sync v0.20.0 ) require ( diff --git a/go.sum b/go.sum index 65ee875..9de170b 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= -github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/coreos/go-oidc/v3 v3.18.0 h1:V9orjXynvu5wiC9SemFTWnG4F45v403aIcjWo0d41+A= github.com/coreos/go-oidc/v3 v3.18.0/go.mod h1:DYCf24+ncYi+XkIH97GY1+dqoRlbaSI26KVTCI9SrY4= github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= @@ -20,8 +18,6 @@ github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPc github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= -golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= -golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s= golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 5bc4561..dd1c18e 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -18,14 +18,10 @@ import ( ) // Context is everything the CLI hands an agent at spawn time. Kept -// deliberately small — anything an agent needs from the broker should -// be reachable via SHERLOCK_SOCKET, not threaded through here. -type Context struct { - // BrokerSocket is the absolute path to the broker's UDS. Set to - // "" when the agent is invoked without a live broker (currently - // never — the CLI always calls EnsureRunning first). - BrokerSocket string -} +// deliberately small — Phase 1 doesn't need anything from sherlock at +// the agent level; Phase 2+ will add fields (e.g. an ID token) as MCPs +// land. +type Context struct{} // Agent is the contract one concrete type implements per supported CLI. type Agent interface { diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index bcdc086..d962126 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -67,7 +67,7 @@ func TestSpawn_HonoursDefaultExecer(t *testing.T) { rec := &recordExecer{} DefaultExecer = rec - if err := (copilot{}).Spawn(Context{BrokerSocket: "/run/s"}, []string{"chat", "hi"}); err != nil { + if err := (copilot{}).Spawn(Context{}, []string{"chat", "hi"}); err != nil { t.Fatalf("Spawn: %v", err) } if !strings.HasSuffix(rec.argv0, "/copilot") { @@ -86,9 +86,6 @@ func TestSpawn_HonoursDefaultExecer(t *testing.T) { if !foundAt { t.Fatalf("argv missing @-prefixed mcp path: %v", rec.argv) } - if !envEquals(rec.env, "SHERLOCK_SOCKET", "/run/s") { - t.Fatal("SHERLOCK_SOCKET not propagated to child env") - } } func TestClaude_ForbidsAnthropicKey(t *testing.T) { @@ -103,7 +100,7 @@ func TestClaude_ForbidsAnthropicKey(t *testing.T) { rec := &recordExecer{} DefaultExecer = rec - if err := (claude{}).Spawn(Context{BrokerSocket: "/run/s"}, []string{"hi"}); err != nil { + if err := (claude{}).Spawn(Context{}, []string{"hi"}); err != nil { t.Fatalf("Spawn: %v", err) } if hasEnv(rec.env, "ANTHROPIC_API_KEY") { diff --git a/internal/agent/claude.go b/internal/agent/claude.go index 7a7b860..e5dea58 100644 --- a/internal/agent/claude.go +++ b/internal/agent/claude.go @@ -19,7 +19,7 @@ type claude struct{} func (claude) Name() string { return "claude" } func (claude) Description() string { return "Anthropic Claude Code CLI" } -func (c claude) Spawn(ctx Context, args []string) error { +func (c claude) Spawn(_ Context, args []string) error { bin, err := LookPath("claude") if err != nil { return err @@ -29,10 +29,7 @@ func (c claude) Spawn(ctx Context, args []string) error { return err } argv := c.buildArgv(bin, mcpPath, args) - env := BuildEnv( - []string{"ANTHROPIC_API_KEY"}, - map[string]string{"SHERLOCK_SOCKET": ctx.BrokerSocket}, - ) + env := BuildEnv([]string{"ANTHROPIC_API_KEY"}, nil) return DefaultExecer.Exec(bin, argv, env) } diff --git a/internal/agent/copilot.go b/internal/agent/copilot.go index fefd1c9..20d71ba 100644 --- a/internal/agent/copilot.go +++ b/internal/agent/copilot.go @@ -18,7 +18,7 @@ type copilot struct{} func (copilot) Name() string { return "copilot" } func (copilot) Description() string { return "GitHub Copilot CLI" } -func (c copilot) Spawn(ctx Context, args []string) error { +func (c copilot) Spawn(_ Context, args []string) error { bin, err := LookPath("copilot") if err != nil { return err @@ -29,10 +29,7 @@ func (c copilot) Spawn(ctx Context, args []string) error { return err } argv := c.buildArgv(bin, mcpPath, args) - env := BuildEnv(nil, map[string]string{ - "SHERLOCK_SOCKET": ctx.BrokerSocket, - }) - return DefaultExecer.Exec(bin, argv, env) + return DefaultExecer.Exec(bin, argv, BuildEnv(nil, nil)) } func (copilot) buildArgv(bin, mcpPath string, userArgs []string) []string { diff --git a/internal/authn/authn.go b/internal/authn/authn.go index 1f0b445..2cb7c2a 100644 --- a/internal/authn/authn.go +++ b/internal/authn/authn.go @@ -1,12 +1,17 @@ // Package authn implements the operator-facing OAuth/OIDC flow against // Authentik: discovery, PKCE, loopback redirect on a fixed port, -// ID-token verification, and single-flight refresh. +// ID-token verification, and refresh. // -// Decision #11 (Phase 1): redirect URI is fixed at -// http://127.0.0.1:6990/callback (loopback, IANA-unassigned port). -// The Authentik OAuth provider configuration is deferred — the broker -// ships with a clean "not configured" error so we can land the -// scaffolding now and wire the real provider once we can drive a flow. +// Decision #11: redirect URI is fixed at http://127.0.0.1:6990/callback +// (loopback, IANA-unassigned port). The Authentik OAuth provider +// configuration is deferred — Login surfaces a clean "not configured" +// error until SHERLOCK_AUTHENTIK_ISSUER / SHERLOCK_AUTHENTIK_CLIENT_ID +// are set in the operator's environment. +// +// Refactored post-Phase-1: no daemon, no broker, no in-process +// state-table; both Login and EnsureFresh are one-shot functions +// safe to call across concurrent CLI invocations (refresh races are +// resolved via a file lock; see refresh.go). package authn import ( @@ -21,26 +26,28 @@ import ( "github.com/coreos/go-oidc/v3/oidc" ) -// LoopbackPort is the fixed loopback port used for the PKCE redirect. -// IANA-unassigned per the Wikipedia port list; locked in this session. -const LoopbackPort = 6990 +// LoopbackAddr is the canonical address:port for the PKCE callback. +// IANA-unassigned per the Wikipedia port list; locked in Phase 1. +const LoopbackAddr = "127.0.0.1:6990" // LoopbackRedirectURL is the canonical redirect URI registered with -// Authentik. -const LoopbackRedirectURL = "http://127.0.0.1:6990/callback" +// Authentik. The path matches Login's HTTP handler. +const LoopbackRedirectURL = "http://" + LoopbackAddr + "/callback" -// Config carries the bits the broker needs to start a flow. Issuer is -// the Authentik application's OIDC issuer URL; ClientID is the OAuth2 -// public client ID; Scopes are the OAuth scopes to request. +// Config carries the bits Login needs. Issuer is the Authentik +// application's OIDC issuer URL; ClientID is the OAuth2 public client +// ID; Scopes are the OAuth scopes to request. LoopbackAddrOverride is +// optional and only used by tests. type Config struct { - Issuer string - ClientID string - Scopes []string + Issuer string + ClientID string + Scopes []string + LoopbackAddrOverride string } -// Configured reports whether the broker has enough state to start a -// flow. We use this to surface a clean "not configured" error from -// login_start until the real Authentik provider exists. +// Configured reports whether sherlock has enough state to start Login. +// Used to surface a clean "not configured" error until the real +// Authentik provider exists. func (c Config) Configured() bool { if c.Issuer == "" || c.ClientID == "" { return false @@ -51,6 +58,13 @@ func (c Config) Configured() bool { return true } +func (c Config) loopbackAddr() string { + if c.LoopbackAddrOverride != "" { + return c.LoopbackAddrOverride + } + return LoopbackAddr +} + // Discoverer wraps go-oidc's per-issuer discovery so we can fake it // in tests. type Discoverer interface { diff --git a/internal/authn/flow.go b/internal/authn/flow.go deleted file mode 100644 index 5e81d5c..0000000 --- a/internal/authn/flow.go +++ /dev/null @@ -1,339 +0,0 @@ -package authn - -import ( - "context" - "errors" - "fmt" - "net" - "net/http" - "strings" - "sync" - "time" - - "github.com/coreos/go-oidc/v3/oidc" - "golang.org/x/oauth2" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring" -) - -// FlowResult is what AwaitCallback delivers after a successful exchange. -// The broker turns it into a keyring.TokenSet for persistence. -type FlowResult struct { - IDToken string - AccessToken string - RefreshToken string - IDExpiresAt time.Time - RefreshExpAt time.Time - Issuer string - Subject string - Email string - Name string -} - -// ToTokenSet projects a FlowResult into the broker's persistence shape. -func (r FlowResult) ToTokenSet() keyring.TokenSet { - return keyring.TokenSet{ - IDToken: r.IDToken, - AccessToken: r.AccessToken, - RefreshToken: r.RefreshToken, - IDExpiresAt: r.IDExpiresAt, - RefreshExpAt: r.RefreshExpAt, - Issuer: r.Issuer, - Subject: r.Subject, - Email: r.Email, - Name: r.Name, - } -} - -// Flow owns the loopback HTTP listener and tracks in-flight states. -// Construct one per broker; reuse across logins. -type Flow struct { - cfg Config - discoverer Discoverer - - startOnce sync.Once - startErr error - listener net.Listener - server *http.Server - - mu sync.Mutex - pending map[string]*pendingFlow -} - -type pendingFlow struct { - pkce PKCE - oauthCfg *oauth2.Config - provider *oidc.Provider - verifier *oidc.IDTokenVerifier - result chan flowOutcome - createdAt time.Time - finishedAt time.Time -} - -type flowOutcome struct { - res FlowResult - err error -} - -// NewFlow returns a configured Flow. The listener is opened lazily on -// the first StartFlow call so unconfigured brokers don't bind the port. -func NewFlow(cfg Config, d Discoverer) *Flow { - if d == nil { - d = DefaultDiscoverer{} - } - return &Flow{ - cfg: cfg, - discoverer: d, - pending: make(map[string]*pendingFlow), - } -} - -// Close stops the loopback HTTP server. Safe to call zero or many times. -func (f *Flow) Close() error { - if f.server == nil { - return nil - } - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - return f.server.Shutdown(ctx) -} - -// ensureListener starts the loopback HTTP server on first use. -func (f *Flow) ensureListener() error { - f.startOnce.Do(func() { - addr := fmt.Sprintf("127.0.0.1:%d", LoopbackPort) - ln, err := net.Listen("tcp", addr) - if err != nil { - f.startErr = fmt.Errorf("authn: bind %s: %w", addr, err) - return - } - f.listener = ln - - mux := http.NewServeMux() - mux.HandleFunc("/callback", f.handleCallback) - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - http.NotFound(w, r) - }) - f.server = &http.Server{Handler: mux, ReadHeaderTimeout: 5 * time.Second} - - go func() { _ = f.server.Serve(ln) }() - }) - return f.startErr -} - -// StartResult is returned by StartFlow so the CLI can show the URL and -// then call AwaitCallback with the matching state. -type StartResult struct { - AuthURL string - State string -} - -// StartFlow performs OIDC discovery, mints PKCE, registers a pending -// state, and returns the URL the operator should open. -func (f *Flow) StartFlow(ctx context.Context) (StartResult, error) { - if !f.cfg.Configured() { - return StartResult{}, ErrNotConfigured - } - if err := f.ensureListener(); err != nil { - return StartResult{}, err - } - - provider, err := f.discoverer.Discover(ctx, f.cfg.Issuer) - if err != nil { - return StartResult{}, err - } - verifier := provider.Verifier(&oidc.Config{ClientID: f.cfg.ClientID}) - - oauthCfg := &oauth2.Config{ - ClientID: f.cfg.ClientID, - Endpoint: provider.Endpoint(), - RedirectURL: LoopbackRedirectURL, - Scopes: f.cfg.Scopes, - } - - pkce, err := NewPKCE() - if err != nil { - return StartResult{}, err - } - state, err := newState() - if err != nil { - return StartResult{}, err - } - - authURL := oauthCfg.AuthCodeURL(state, - oauth2.SetAuthURLParam("code_challenge", pkce.Challenge), - oauth2.SetAuthURLParam("code_challenge_method", pkce.Method), - ) - - pf := &pendingFlow{ - pkce: pkce, - oauthCfg: oauthCfg, - provider: provider, - verifier: verifier, - result: make(chan flowOutcome, 1), - createdAt: time.Now(), - } - - f.mu.Lock() - f.pending[state] = pf - f.mu.Unlock() - - return StartResult{AuthURL: authURL, State: state}, nil -} - -// AwaitCallback blocks until the callback for state arrives, the -// provided context is cancelled, or timeout elapses. The pending state -// is always cleared on return. -func (f *Flow) AwaitCallback(ctx context.Context, state string, timeout time.Duration) (FlowResult, error) { - f.mu.Lock() - pf, ok := f.pending[state] - f.mu.Unlock() - if !ok { - return FlowResult{}, ErrUnknownState - } - defer f.dropPending(state) - - if timeout <= 0 { - timeout = 5 * time.Minute - } - t := time.NewTimer(timeout) - defer t.Stop() - select { - case out := <-pf.result: - return out.res, out.err - case <-ctx.Done(): - return FlowResult{}, ctx.Err() - case <-t.C: - return FlowResult{}, ErrFlowExpired - } -} - -func (f *Flow) dropPending(state string) { - f.mu.Lock() - delete(f.pending, state) - f.mu.Unlock() -} - -func (f *Flow) handleCallback(w http.ResponseWriter, r *http.Request) { - q := r.URL.Query() - state := q.Get("state") - - f.mu.Lock() - pf, ok := f.pending[state] - f.mu.Unlock() - if !ok { - http.Error(w, "unknown or stale state", http.StatusBadRequest) - return - } - - if errParam := q.Get("error"); errParam != "" { - desc := q.Get("error_description") - err := fmt.Errorf("authentik: %s: %s", errParam, desc) - f.deliver(pf, flowOutcome{err: err}) - _, _ = w.Write(failPage(errParam, desc)) - return - } - - code := q.Get("code") - if code == "" { - f.deliver(pf, flowOutcome{err: errors.New("authentik: missing code in callback")}) - http.Error(w, "missing code", http.StatusBadRequest) - return - } - - tok, err := pf.oauthCfg.Exchange(r.Context(), code, - oauth2.SetAuthURLParam("code_verifier", pf.pkce.Verifier), - ) - if err != nil { - f.deliver(pf, flowOutcome{err: fmt.Errorf("token exchange: %w", err)}) - http.Error(w, "token exchange failed", http.StatusBadGateway) - return - } - - rawID, _ := tok.Extra("id_token").(string) - if rawID == "" { - f.deliver(pf, flowOutcome{err: errors.New("token response missing id_token")}) - http.Error(w, "missing id_token", http.StatusBadGateway) - return - } - idTok, err := pf.verifier.Verify(r.Context(), rawID) - if err != nil { - f.deliver(pf, flowOutcome{err: fmt.Errorf("verify id_token: %w", err)}) - http.Error(w, "id_token verification failed", http.StatusBadGateway) - return - } - - var claims struct { - Sub string `json:"sub"` - Email string `json:"email"` - Name string `json:"name"` - } - _ = idTok.Claims(&claims) - - res := FlowResult{ - IDToken: rawID, - AccessToken: tok.AccessToken, - RefreshToken: tok.RefreshToken, - IDExpiresAt: idTok.Expiry, - Issuer: f.cfg.Issuer, - Subject: claims.Sub, - Email: claims.Email, - Name: claims.Name, - } - // Authentik returns a refresh-token expiry as a number-of-seconds - // extra; fall back to "now + 30d" when missing so the field is - // always populated. - if v, ok := tok.Extra("refresh_expires_in").(float64); ok && v > 0 { - res.RefreshExpAt = time.Now().Add(time.Duration(v) * time.Second) - } else if res.RefreshToken != "" { - res.RefreshExpAt = time.Now().Add(30 * 24 * time.Hour) - } - - f.deliver(pf, flowOutcome{res: res}) - _, _ = w.Write(successPage(res)) -} - -func (f *Flow) deliver(pf *pendingFlow, out flowOutcome) { - pf.finishedAt = time.Now() - select { - case pf.result <- out: - default: - } -} - -// Errors surfaced via the broker RPC layer. -var ( - ErrNotConfigured = errors.New("authn: broker not configured for Authentik (see plan.md Open #11)") - ErrUnknownState = errors.New("authn: unknown or expired state") - ErrFlowExpired = errors.New("authn: flow expired before callback arrived") -) - -func successPage(r FlowResult) []byte { - who := r.Name - if who == "" { - who = r.Subject - } - body := `sherlock - - -

Logged in

-

Signed in as ` + htmlEscape(who) + `. You can close this tab and return to the terminal.

-` - return []byte(body) -} - -func failPage(code, desc string) []byte { - body := `sherlock - - -

Login failed

-

` + htmlEscape(code) + `: ` + htmlEscape(desc) + `

-

Check the terminal for details.

-` - return []byte(body) -} - -func htmlEscape(s string) string { - r := strings.NewReplacer("&", "&", "<", "<", ">", ">", `"`, """, "'", "'") - return r.Replace(s) -} diff --git a/internal/authn/flow_test.go b/internal/authn/flow_test.go deleted file mode 100644 index 2b1baf1..0000000 --- a/internal/authn/flow_test.go +++ /dev/null @@ -1,199 +0,0 @@ -package authn_test - -import ( - "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "crypto/sha256" - "encoding/base64" - "encoding/json" - "fmt" - "math/big" - "net/http" - "net/http/httptest" - "net/url" - "strings" - "testing" - "time" - - "github.com/go-jose/go-jose/v4" - "github.com/go-jose/go-jose/v4/jwt" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/authn" -) - -// stubAuthentik is a minimal OIDC provider sufficient to drive Flow -// end-to-end: it implements discovery, an authorize endpoint that -// immediately redirects with a code, and a token endpoint that returns -// a signed id_token. -type stubAuthentik struct { - server *httptest.Server - signer jose.Signer - pubJWK jose.JSONWebKey - issuer string - clientID string - - // State remembered between authorize and token calls. - expectedCodeChallenge string -} - -func newStub(t *testing.T, clientID string) *stubAuthentik { - t.Helper() - - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - if err != nil { - t.Fatalf("genkey: %v", err) - } - signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: key}, - (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", "test")) - if err != nil { - t.Fatalf("signer: %v", err) - } - pub := jose.JSONWebKey{Key: &key.PublicKey, KeyID: "test", Algorithm: "ES256", Use: "sig"} - - s := &stubAuthentik{signer: signer, pubJWK: pub, clientID: clientID} - - mux := http.NewServeMux() - mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ - "issuer": s.issuer, - "authorization_endpoint": s.issuer + "/auth", - "token_endpoint": s.issuer + "/token", - "jwks_uri": s.issuer + "/jwks", - "response_types_supported": []string{"code"}, - "subject_types_supported": []string{"public"}, - "id_token_signing_alg_values_supported": []string{"ES256"}, - }) - }) - mux.HandleFunc("/jwks", func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(jose.JSONWebKeySet{Keys: []jose.JSONWebKey{pub}}) - }) - mux.HandleFunc("/auth", func(w http.ResponseWriter, r *http.Request) { - // Capture code_challenge so we can demand the right verifier - // on the token call. Redirect to the redirect_uri with state+code. - s.expectedCodeChallenge = r.URL.Query().Get("code_challenge") - redirect := r.URL.Query().Get("redirect_uri") - state := r.URL.Query().Get("state") - u, _ := url.Parse(redirect) - q := u.Query() - q.Set("code", "stub-code") - q.Set("state", state) - u.RawQuery = q.Encode() - http.Redirect(w, r, u.String(), http.StatusFound) - }) - mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { - _ = r.ParseForm() - verifier := r.PostForm.Get("code_verifier") - sum := sha256.Sum256([]byte(verifier)) - got := base64.RawURLEncoding.EncodeToString(sum[:]) - if got != s.expectedCodeChallenge { - http.Error(w, "verifier mismatch", http.StatusBadRequest) - return - } - now := time.Now() - claims := struct { - jwt.Claims - Email string `json:"email"` - Name string `json:"name"` - }{ - Claims: jwt.Claims{ - Issuer: s.issuer, - Subject: "stub-user", - Audience: jwt.Audience{s.clientID}, - Expiry: jwt.NewNumericDate(now.Add(time.Hour)), - IssuedAt: jwt.NewNumericDate(now), - }, - Email: "stub@example", - Name: "Stub User", - } - idToken, err := jwt.Signed(s.signer).Claims(claims).Serialize() - if err != nil { - http.Error(w, err.Error(), 500) - return - } - w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ - "access_token": "access-1", - "token_type": "Bearer", - "refresh_token": "refresh-1", - "expires_in": 3600, - "refresh_expires_in": 30 * 24 * 3600, - "id_token": idToken, - }) - }) - - s.server = httptest.NewServer(mux) - s.issuer = s.server.URL - t.Cleanup(s.server.Close) - return s -} - -// ensure big.Int is "imported" (referenced) so future tweaks don't -// require touching imports. -var _ = big.NewInt - -func TestFlow_EndToEnd(t *testing.T) { - stub := newStub(t, "sherlock-client") - - flow := authn.NewFlow(authn.Config{ - Issuer: stub.issuer, - ClientID: "sherlock-client", - Scopes: []string{"openid", "profile", "email"}, - }, nil) - defer func() { _ = flow.Close() }() - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - start, err := flow.StartFlow(ctx) - if err != nil { - t.Fatalf("StartFlow: %v", err) - } - if start.State == "" || !strings.HasPrefix(start.AuthURL, stub.issuer+"/auth?") { - t.Fatalf("bad start: %+v", start) - } - - // Simulate the browser: follow the redirect chain. We DON'T want - // the http.Client to follow into our loopback callback as part of - // the same client (it would try and the cb would 200), so just let - // the chain go. - client := &http.Client{Timeout: 5 * time.Second} - resp, err := client.Get(start.AuthURL) - if err != nil { - t.Fatalf("GET auth: %v", err) - } - _ = resp.Body.Close() - if resp.StatusCode >= 400 { - t.Fatalf("auth chain ended at %s", resp.Status) - } - - res, err := flow.AwaitCallback(ctx, start.State, 5*time.Second) - if err != nil { - t.Fatalf("AwaitCallback: %v", err) - } - if res.Subject != "stub-user" || res.Email != "stub@example" { - t.Fatalf("bad claims: %+v", res) - } - if res.IDToken == "" || res.RefreshToken != "refresh-1" { - t.Fatalf("bad tokens: %+v", res) - } - if time.Until(res.IDExpiresAt) < 30*time.Minute { - t.Fatalf("id token expiry too soon: %v", res.IDExpiresAt) - } -} - -func TestFlow_NotConfigured(t *testing.T) { - flow := authn.NewFlow(authn.Config{}, nil) - defer func() { _ = flow.Close() }() - _, err := flow.StartFlow(context.Background()) - if err == nil || !strings.Contains(err.Error(), "not configured") { - t.Fatalf("expected not-configured error, got %v", err) - } -} - -// Sanity check that fmt is referenced by tests so an unused import -// linter doesn't complain after future edits. -var _ = fmt.Sprintf diff --git a/internal/authn/login.go b/internal/authn/login.go new file mode 100644 index 0000000..af01618 --- /dev/null +++ b/internal/authn/login.go @@ -0,0 +1,226 @@ +package authn + +import ( + "context" + "crypto/tls" + "errors" + "fmt" + "html" + "net" + "net/http" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "golang.org/x/oauth2" + + "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring" +) + +// ErrNotConfigured is returned by Login when Config.Configured() is +// false. The CLI surfaces it with the env vars to set. +var ErrNotConfigured = errors.New("authn: Authentik issuer/client ID not configured") + +// Result is everything Login returns on success. +type Result struct { + Tokens keyring.TokenSet +} + +// LoginOptions tunes Login for tests. Production code passes zero. +type LoginOptions struct { + // Discoverer overrides OIDC well-known discovery. Default is + // DefaultDiscoverer{}. + Discoverer Discoverer + // HTTPClient is plumbed into oauth2.HTTPClient context. Default + // is &http.Client{} with TLS using the system cert pool. + HTTPClient *http.Client + // OnAuthURL is called once with the URL the operator must visit + // (after the listener has bound, before blocking on the callback). + // Tests set this to drive the flow programmatically; the CLI + // opens it in the browser. + OnAuthURL func(url string) +} + +// Login runs the full Authentik PKCE flow synchronously: bind a +// loopback listener, generate state + PKCE, build the auth URL and +// hand it to OnAuthURL, then block until the browser redirects to +// /callback (or ctx is cancelled). On success it verifies the ID +// token and returns the populated TokenSet. +// +// Login is safe to call from multiple processes only one at a time — +// the loopback port is a global resource. A second concurrent Login +// will fail with EADDRINUSE; that's the right behavior (the operator +// is doing two `sherlock login` invocations). +func Login(ctx context.Context, cfg Config, opts LoginOptions) (Result, error) { + if !cfg.Configured() { + return Result{}, ErrNotConfigured + } + if opts.Discoverer == nil { + opts.Discoverer = DefaultDiscoverer{} + } + if opts.HTTPClient == nil { + opts.HTTPClient = &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12}}} + } + if opts.OnAuthURL == nil { + opts.OnAuthURL = func(string) {} + } + + ln, err := net.Listen("tcp", cfg.loopbackAddr()) + if err != nil { + return Result{}, fmt.Errorf("authn: listen on %s: %w", cfg.loopbackAddr(), err) + } + defer func() { _ = ln.Close() }() + + // The redirect URL must match what we registered with Authentik + // exactly. We use the listener's reported address so tests can + // bind :0 — production always binds the canonical LoopbackAddr. + redirectURL := "http://" + ln.Addr().String() + "/callback" + + ctx = oidc.ClientContext(ctx, opts.HTTPClient) + provider, err := opts.Discoverer.Discover(ctx, cfg.Issuer) + if err != nil { + return Result{}, err + } + + pkce, err := NewPKCE() + if err != nil { + return Result{}, err + } + state, err := newState() + if err != nil { + return Result{}, err + } + + oauthCfg := &oauth2.Config{ + ClientID: cfg.ClientID, + Endpoint: provider.Endpoint(), + RedirectURL: redirectURL, + Scopes: cfg.Scopes, + } + authURL := oauthCfg.AuthCodeURL(state, + oauth2.SetAuthURLParam("code_challenge", pkce.Challenge), + oauth2.SetAuthURLParam("code_challenge_method", pkce.Method), + ) + cb := make(chan callbackOutcome, 1) + srv := &http.Server{ + Handler: callbackHandler(state, cb), + ReadHeaderTimeout: 5 * time.Second, + } + go func() { + // Serve returns once Shutdown is called from below. + _ = srv.Serve(ln) + }() + defer func() { + shutdownCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = srv.Shutdown(shutdownCtx) + }() + + // Open the browser asynchronously so callers can do blocking work + // here (e.g. test stubs that drive the redirect themselves). + go opts.OnAuthURL(authURL) + + select { + case <-ctx.Done(): + return Result{}, ctx.Err() + case out := <-cb: + if out.err != nil { + return Result{}, out.err + } + return exchangeAndVerify(ctx, cfg, provider, oauthCfg, pkce, out.code) + } +} + +type callbackOutcome struct { + code string + err error +} + +func callbackHandler(expectedState string, out chan<- callbackOutcome) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/callback" { + http.NotFound(w, r) + return + } + q := r.URL.Query() + if e := q.Get("error"); e != "" { + desc := q.Get("error_description") + renderHTML(w, "Login failed", fmt.Sprintf("%s: %s", e, desc), http.StatusBadRequest) + out <- callbackOutcome{err: fmt.Errorf("authn: callback error: %s: %s", e, desc)} + return + } + gotState := q.Get("state") + if gotState != expectedState { + renderHTML(w, "Login failed", "State mismatch (possible CSRF).", http.StatusBadRequest) + out <- callbackOutcome{err: errors.New("authn: callback state mismatch")} + return + } + code := q.Get("code") + if code == "" { + renderHTML(w, "Login failed", "Authorization server returned no code.", http.StatusBadRequest) + out <- callbackOutcome{err: errors.New("authn: callback missing code")} + return + } + renderHTML(w, "Logged in", "You may close this tab and return to your terminal.", http.StatusOK) + out <- callbackOutcome{code: code} + }) +} + +func exchangeAndVerify(ctx context.Context, cfg Config, provider *oidc.Provider, oauthCfg *oauth2.Config, pkce PKCE, code string) (Result, error) { + tok, err := oauthCfg.Exchange(ctx, code, + oauth2.SetAuthURLParam("code_verifier", pkce.Verifier), + ) + if err != nil { + return Result{}, fmt.Errorf("authn: token exchange: %w", err) + } + rawID, _ := tok.Extra("id_token").(string) + if rawID == "" { + return Result{}, errors.New("authn: token response missing id_token") + } + verifier := provider.Verifier(&oidc.Config{ClientID: cfg.ClientID}) + idTok, err := verifier.Verify(ctx, rawID) + if err != nil { + return Result{}, fmt.Errorf("authn: id_token verify: %w", err) + } + var claims struct { + Sub string `json:"sub"` + Email string `json:"email"` + Name string `json:"name"` + } + if err := idTok.Claims(&claims); err != nil { + return Result{}, fmt.Errorf("authn: id_token claims: %w", err) + } + ts := keyring.TokenSet{ + IDToken: rawID, + AccessToken: tok.AccessToken, + RefreshToken: tok.RefreshToken, + IDExpiresAt: idTok.Expiry, + RefreshExpAt: refreshExpiry(tok), + Issuer: cfg.Issuer, + ClientID: cfg.ClientID, + Scopes: cfg.Scopes, + Subject: claims.Sub, + Email: claims.Email, + Name: claims.Name, + } + return Result{Tokens: ts}, nil +} + +// refreshExpiry pulls Authentik's `refresh_expires_in` (an extension +// to RFC 6749). If absent we conservatively assume 30 days, matching +// Authentik's default. +func refreshExpiry(tok *oauth2.Token) time.Time { + if v, ok := tok.Extra("refresh_expires_in").(float64); ok && v > 0 { + return time.Now().Add(time.Duration(v) * time.Second) + } + return time.Now().Add(30 * 24 * time.Hour) +} + +func renderHTML(w http.ResponseWriter, title, body string, status int) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(status) + const tmpl = `%s — sherlock` + + `` + + `

%s

%s

` + _, _ = fmt.Fprintf(w, tmpl, html.EscapeString(title), html.EscapeString(title), html.EscapeString(body)) +} diff --git a/internal/authn/login_test.go b/internal/authn/login_test.go new file mode 100644 index 0000000..d29006c --- /dev/null +++ b/internal/authn/login_test.go @@ -0,0 +1,331 @@ +package authn + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/jwt" + + "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring" +) + +// stubAuthentik signs ES256 ID tokens and returns them through a real +// httptest.Server, exercising the full Login path end-to-end without +// touching a live Authentik instance. +type stubAuthentik struct { + srv *httptest.Server + signer jose.Signer + pubJWK jose.JSONWebKey + verifyCalled bool + mu sync.Mutex + codeToPKCE map[string]string +} + +func newStubAuthentik(t *testing.T) *stubAuthentik { + t.Helper() + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: priv}, + (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", "test")) + if err != nil { + t.Fatal(err) + } + stub := &stubAuthentik{ + signer: signer, + pubJWK: jose.JSONWebKey{ + Key: priv.Public(), + KeyID: "test", + Algorithm: "ES256", + Use: "sig", + }, + codeToPKCE: map[string]string{}, + } + mux := http.NewServeMux() + stub.srv = httptest.NewServer(mux) + + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "issuer": stub.srv.URL, + "authorization_endpoint": stub.srv.URL + "/authorize", + "token_endpoint": stub.srv.URL + "/token", + "jwks_uri": stub.srv.URL + "/jwks", + "response_types_supported": []string{"code"}, + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"ES256"}, + }) + }) + mux.HandleFunc("/jwks", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(jose.JSONWebKeySet{Keys: []jose.JSONWebKey{stub.pubJWK}}) + }) + mux.HandleFunc("/authorize", func(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + stub.mu.Lock() + stub.codeToPKCE["code-"+q.Get("state")] = q.Get("code_challenge") + stub.mu.Unlock() + // Redirect to the loopback redirect URI with the code+state. + ru, _ := url.Parse(q.Get("redirect_uri")) + rq := ru.Query() + rq.Set("code", "code-"+q.Get("state")) + rq.Set("state", q.Get("state")) + ru.RawQuery = rq.Encode() + http.Redirect(w, r, ru.String(), http.StatusFound) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + _ = r.ParseForm() + code := r.Form.Get("code") + verifier := r.Form.Get("code_verifier") + stub.mu.Lock() + challenge, ok := stub.codeToPKCE[code] + stub.mu.Unlock() + if !ok { + http.Error(w, "unknown code", http.StatusBadRequest) + return + } + sum := sha256.Sum256([]byte(verifier)) + want := base64.RawURLEncoding.EncodeToString(sum[:]) + if want != challenge { + http.Error(w, "bad code_verifier", http.StatusBadRequest) + return + } + stub.verifyCalled = true + // Public PKCE clients don't always send client_id in the body + // (oauth2 prefers Basic-Auth when AuthStyle defaults to header). + // Pull it from Authorization or fall back to the form/test value. + clientID := r.Form.Get("client_id") + if clientID == "" { + if user, _, ok := r.BasicAuth(); ok { + clientID = user + } + } + if clientID == "" { + clientID = "test-client" + } + idToken := stub.signIDToken(t, clientID, map[string]any{ + "sub": "user-1", + "email": "ops@example.com", + "name": "Test Operator", + }) + resp := map[string]any{ + "access_token": "at-1", + "refresh_token": "rt-1", + "id_token": idToken, + "token_type": "Bearer", + "expires_in": 3600, + "refresh_expires_in": 86400, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + }) + t.Cleanup(stub.srv.Close) + return stub +} + +func (s *stubAuthentik) signIDToken(t *testing.T, audience string, claims map[string]any) string { + t.Helper() + now := time.Now() + all := map[string]any{ + "iss": s.srv.URL, + "aud": audience, + "iat": now.Unix(), + "exp": now.Add(time.Hour).Unix(), + } + for k, v := range claims { + all[k] = v + } + tok, err := jwt.Signed(s.signer).Claims(all).Serialize() + if err != nil { + t.Fatal(err) + } + return tok +} + +func TestLogin_HappyPath(t *testing.T) { + stub := newStubAuthentik(t) + cfg := Config{ + Issuer: stub.srv.URL, + ClientID: "test-client", + Scopes: []string{"openid", "profile", "email"}, + LoopbackAddrOverride: "127.0.0.1:0", + } + + // The browser-open hook drives the flow programmatically. + var browserErr error + opts := LoginOptions{ + OnAuthURL: func(u string) { + resp, err := (&http.Client{ + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + }).Get(u) + if err != nil { + browserErr = err + return + } + defer func() { _ = resp.Body.Close() }() + loc := resp.Header.Get("Location") + if loc == "" { + browserErr = fmt.Errorf("expected redirect, got %d", resp.StatusCode) + return + } + // Follow the redirect into the loopback listener. + cb, err := http.Get(loc) + if err != nil { + browserErr = err + return + } + _, _ = io.Copy(io.Discard, cb.Body) + _ = cb.Body.Close() + }, + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + res, err := Login(ctx, cfg, opts) + if err != nil { + t.Fatalf("Login: %v", err) + } + if browserErr != nil { + t.Fatalf("browser stub: %v", browserErr) + } + if res.Tokens.IDToken == "" || res.Tokens.RefreshToken == "" { + t.Fatalf("missing tokens: %+v", res.Tokens) + } + if res.Tokens.Subject != "user-1" || res.Tokens.Email != "ops@example.com" { + t.Fatalf("claims wrong: %+v", res.Tokens) + } + if res.Tokens.ClientID != "test-client" { + t.Fatalf("ClientID not persisted: %q", res.Tokens.ClientID) + } + if !strings.HasPrefix(res.Tokens.Issuer, "http://127.0.0.1:") { + t.Fatalf("Issuer = %q", res.Tokens.Issuer) + } +} + +func TestLogin_NotConfigured(t *testing.T) { + _, err := Login(context.Background(), Config{}, LoginOptions{}) + if err != ErrNotConfigured { + t.Fatalf("err = %v, want ErrNotConfigured", err) + } +} + +func TestLogin_ContextCancel(t *testing.T) { + stub := newStubAuthentik(t) + cfg := Config{ + Issuer: stub.srv.URL, + ClientID: "test-client", + Scopes: []string{"openid"}, + LoopbackAddrOverride: "127.0.0.1:0", + } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(100 * time.Millisecond) + cancel() + }() + _, err := Login(ctx, cfg, LoginOptions{OnAuthURL: func(string) {}}) + if err == nil { + t.Fatal("expected error on cancel") + } +} + +// Sanity check that the verifier built off the stub provider works +// outside Login too — guards against accidental client-ID drift. +func TestIDTokenVerifies_AgainstStub(t *testing.T) { + stub := newStubAuthentik(t) + provider, err := oidc.NewProvider(context.Background(), stub.srv.URL) + if err != nil { + t.Fatal(err) + } + raw := stub.signIDToken(t, "client-x", map[string]any{"sub": "u"}) + if _, err := provider.Verifier(&oidc.Config{ClientID: "client-x"}).Verify(context.Background(), raw); err != nil { + t.Fatal(err) + } +} + +func TestEnsureFresh_FastPath(t *testing.T) { + store := newMemStore() + store.tokens = keyring.TokenSet{ + IDToken: "ok", + IDExpiresAt: time.Now().Add(time.Hour), + Issuer: "x", + ClientID: "y", + } + ts, err := EnsureFresh(context.Background(), store, EnsureFreshOptions{LockPath: t.TempDir() + "/lock"}) + if err != nil { + t.Fatalf("EnsureFresh: %v", err) + } + if ts.IDToken != "ok" { + t.Fatal("fast path should return current token") + } +} + +func TestEnsureFresh_NoTokens(t *testing.T) { + store := newMemStore() + _, err := EnsureFresh(context.Background(), store, EnsureFreshOptions{LockPath: t.TempDir() + "/lock"}) + if err != keyring.ErrNoTokens { + t.Fatalf("err = %v, want ErrNoTokens", err) + } +} + +func TestEnsureFresh_NeedsRefreshButNoRefreshToken(t *testing.T) { + store := newMemStore() + store.tokens = keyring.TokenSet{ + IDToken: "stale", + IDExpiresAt: time.Now().Add(-time.Minute), + Issuer: "x", + ClientID: "y", + } + _, err := EnsureFresh(context.Background(), store, EnsureFreshOptions{LockPath: t.TempDir() + "/lock"}) + if err != ErrNoRefreshToken { + t.Fatalf("err = %v, want ErrNoRefreshToken", err) + } +} + +type memStore struct { + mu sync.Mutex + tokens keyring.TokenSet + cleared bool +} + +func newMemStore() *memStore { return &memStore{} } + +func (m *memStore) GetAuthentikTokens() (keyring.TokenSet, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.tokens.Empty() { + return keyring.TokenSet{}, keyring.ErrNoTokens + } + return m.tokens, nil +} + +func (m *memStore) SetAuthentikTokens(ts keyring.TokenSet) error { + m.mu.Lock() + defer m.mu.Unlock() + m.tokens = ts + return nil +} + +func (m *memStore) ClearAuthentikTokens() error { + m.mu.Lock() + defer m.mu.Unlock() + m.tokens = keyring.TokenSet{} + m.cleared = true + return nil +} diff --git a/internal/authn/refresh.go b/internal/authn/refresh.go index 786fb9c..9665bd1 100644 --- a/internal/authn/refresh.go +++ b/internal/authn/refresh.go @@ -1,110 +1,152 @@ +//go:build unix + package authn import ( "context" "errors" "fmt" - "sync" + "os" + "path/filepath" + "syscall" "time" + "github.com/coreos/go-oidc/v3/oidc" "golang.org/x/oauth2" - "golang.org/x/sync/singleflight" "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring" ) -// RefreshSkew is how far ahead of id_token expiry we trigger a refresh. +// RefreshSkew is how much headroom EnsureFresh leaves before the ID +// token actually expires. Refreshing 30s ahead of expiry avoids +// shipping a token that will be rejected by clock-skewed resource +// servers. const RefreshSkew = 30 * time.Second -// Refresher refreshes an Authentik TokenSet. Construct one per Flow -// (it caches the discovered provider via the Flow's discoverer). -type Refresher struct { - cfg Config - discoverer Discoverer +// ErrNoRefreshToken is returned by EnsureFresh when the stored +// TokenSet has no refresh token to swap. +var ErrNoRefreshToken = errors.New("authn: no refresh token stored") - mu sync.Mutex - endpoint oauth2.Endpoint - have bool - - group singleflight.Group +// EnsureFreshOptions tunes EnsureFresh for tests. +type EnsureFreshOptions struct { + // LockPath is the absolute path to the cross-process flock file. + // Required. + LockPath string + // Discoverer overrides OIDC discovery for the refresh round-trip. + Discoverer Discoverer + // Clock is injected for time-warp tests. + Clock func() time.Time } -// NewRefresher builds a Refresher. Pass the same Config the Flow uses. -func NewRefresher(cfg Config, d Discoverer) *Refresher { - if d == nil { - d = DefaultDiscoverer{} +// EnsureFresh returns the stored TokenSet, refreshing it via Authentik +// if the ID token has < RefreshSkew left. Cross-process safe: takes +// an exclusive flock on opts.LockPath, re-reads the keyring after +// acquiring the lock, and skips the refresh if a concurrent process +// already rotated the tokens. +// +// The lockfile is created with mode 0600 and never removed — its +// only role is to anchor the flock; multiple sherlock invocations +// across an operator's lifetime share it. +func EnsureFresh(ctx context.Context, store keyring.Store, opts EnsureFreshOptions) (keyring.TokenSet, error) { + if opts.LockPath == "" { + return keyring.TokenSet{}, errors.New("authn: EnsureFresh requires LockPath") + } + if opts.Discoverer == nil { + opts.Discoverer = DefaultDiscoverer{} + } + if opts.Clock == nil { + opts.Clock = time.Now } - return &Refresher{cfg: cfg, discoverer: d} -} -// EnsureFresh returns ts unchanged if its IDToken is good for at least -// RefreshSkew. Otherwise it performs a single-flight refresh using the -// stored refresh token and returns the new TokenSet. -func (r *Refresher) EnsureFresh(ctx context.Context, ts keyring.TokenSet) (keyring.TokenSet, error) { - if ts.IDToken != "" && time.Until(ts.IDExpiresAt) > RefreshSkew { + ts, err := store.GetAuthentikTokens() + if err != nil { + return keyring.TokenSet{}, err + } + if ts.Empty() { + return ts, keyring.ErrNoTokens + } + if opts.Clock().Add(RefreshSkew).Before(ts.IDExpiresAt) { + return ts, nil + } + + // Slow path: take the lock, double-check, refresh under the lock + // so concurrent processes serialise. + if err := os.MkdirAll(filepath.Dir(opts.LockPath), 0o700); err != nil { + return ts, fmt.Errorf("authn: mkdir lockdir: %w", err) + } + f, err := os.OpenFile(opts.LockPath, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return ts, fmt.Errorf("authn: open lockfile: %w", err) + } + defer func() { _ = f.Close() }() + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { + return ts, fmt.Errorf("authn: flock lockfile: %w", err) + } + defer func() { _ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN) }() + + // Re-read after acquiring the lock. A concurrent process may have + // refreshed for us while we were waiting on flock. + ts, err = store.GetAuthentikTokens() + if err != nil { + return ts, err + } + if opts.Clock().Add(RefreshSkew).Before(ts.IDExpiresAt) { return ts, nil } if ts.RefreshToken == "" { return ts, ErrNoRefreshToken } - v, err, _ := r.group.Do("refresh", func() (any, error) { - return r.refresh(ctx, ts) - }) + + fresh, err := refreshOnce(ctx, ts, opts.Discoverer) if err != nil { return ts, err } - return v.(keyring.TokenSet), nil + if err := store.SetAuthentikTokens(fresh); err != nil { + return ts, fmt.Errorf("authn: persist refreshed tokens: %w", err) + } + return fresh, nil } -func (r *Refresher) refresh(ctx context.Context, ts keyring.TokenSet) (keyring.TokenSet, error) { - if !r.cfg.Configured() { - return ts, ErrNotConfigured +func refreshOnce(ctx context.Context, ts keyring.TokenSet, disc Discoverer) (keyring.TokenSet, error) { + if ts.Issuer == "" || ts.ClientID == "" { + return ts, errors.New("authn: stored tokens missing issuer or client_id (re-login)") } - if err := r.ensureEndpoint(ctx); err != nil { + provider, err := disc.Discover(ctx, ts.Issuer) + if err != nil { return ts, err } - src := (&oauth2.Config{ - ClientID: r.cfg.ClientID, - Endpoint: r.endpoint, - }).TokenSource(ctx, &oauth2.Token{RefreshToken: ts.RefreshToken}) - + cfg := &oauth2.Config{ + ClientID: ts.ClientID, + Endpoint: provider.Endpoint(), + Scopes: ts.Scopes, + } + src := cfg.TokenSource(ctx, &oauth2.Token{ + AccessToken: ts.AccessToken, + RefreshToken: ts.RefreshToken, + // Force the source to refresh by claiming we expired in the past. + Expiry: time.Now().Add(-1 * time.Minute), + }) tok, err := src.Token() if err != nil { return ts, fmt.Errorf("authn: refresh: %w", err) } rawID, _ := tok.Extra("id_token").(string) + if rawID == "" { + // Some IdPs omit id_token on refresh; keep the old one if so. + rawID = ts.IDToken + } + verifier := provider.Verifier(&oidc.Config{ClientID: ts.ClientID}) + idTok, err := verifier.Verify(ctx, rawID) + if err != nil { + return ts, fmt.Errorf("authn: refreshed id_token verify: %w", err) + } out := ts - if rawID != "" { - out.IDToken = rawID - } - if tok.AccessToken != "" { - out.AccessToken = tok.AccessToken - } + out.IDToken = rawID + out.AccessToken = tok.AccessToken if tok.RefreshToken != "" { out.RefreshToken = tok.RefreshToken } - out.IDExpiresAt = tok.Expiry - if v, ok := tok.Extra("refresh_expires_in").(float64); ok && v > 0 { - out.RefreshExpAt = time.Now().Add(time.Duration(v) * time.Second) - } + out.IDExpiresAt = idTok.Expiry + out.RefreshExpAt = refreshExpiry(tok) return out, nil } - -func (r *Refresher) ensureEndpoint(ctx context.Context) error { - r.mu.Lock() - defer r.mu.Unlock() - if r.have { - return nil - } - p, err := r.discoverer.Discover(ctx, r.cfg.Issuer) - if err != nil { - return err - } - r.endpoint = p.Endpoint() - r.have = true - return nil -} - -// ErrNoRefreshToken is returned when a refresh is required but the -// stored TokenSet has no refresh_token. -var ErrNoRefreshToken = errors.New("authn: no refresh token; re-login required") diff --git a/internal/broker/doc.go b/internal/broker/doc.go deleted file mode 100644 index 113cee5..0000000 --- a/internal/broker/doc.go +++ /dev/null @@ -1,9 +0,0 @@ -// Package broker implements the sherlock per-user daemon: process -// lifecycle (PID file + flock + idle timeout), JSON-over-newline RPC -// dispatch, and the EnsureRunning helper that the wrapper CLI uses to -// dial-or-fork the broker. -// -// Decision #10 (Phase 1): the broker runs as a forked, setsid-detached -// child process — no systemd unit. A second invocation will be refused -// by the PID-file flock. -package broker diff --git a/internal/broker/lifecycle.go b/internal/broker/lifecycle.go deleted file mode 100644 index b6cd533..0000000 --- a/internal/broker/lifecycle.go +++ /dev/null @@ -1,70 +0,0 @@ -//go:build unix - -package broker - -import ( - "errors" - "fmt" - "os" - "strconv" - "syscall" -) - -// ErrAlreadyRunning is returned by AcquirePidLock when another broker -// holds the lock. -var ErrAlreadyRunning = errors.New("broker: another instance is running") - -// PidLock is a file-based singleton guard. The file at Path is opened -// for writing, flock(LOCK_EX|LOCK_NB) is taken, and the current PID is -// written. Releasing closes the fd (and the kernel drops the lock) and -// removes the file. -type PidLock struct { - Path string - f *os.File -} - -// AcquirePidLock obtains an exclusive lock on path or returns -// ErrAlreadyRunning if another broker holds it. The directory must -// already exist. -func AcquirePidLock(path string) (*PidLock, error) { - f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600) - if err != nil { - return nil, fmt.Errorf("broker: open pid file %s: %w", path, err) - } - if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB); err != nil { - _ = f.Close() - if errors.Is(err, syscall.EWOULDBLOCK) { - return nil, ErrAlreadyRunning - } - return nil, fmt.Errorf("broker: flock pid file: %w", err) - } - if err := f.Truncate(0); err != nil { - _ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN) - _ = f.Close() - return nil, fmt.Errorf("broker: truncate pid file: %w", err) - } - if _, err := f.WriteString(strconv.Itoa(os.Getpid()) + "\n"); err != nil { - _ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN) - _ = f.Close() - return nil, fmt.Errorf("broker: write pid: %w", err) - } - return &PidLock{Path: path, f: f}, nil -} - -// Release drops the lock and removes the PID file. Safe to call twice. -func (l *PidLock) Release() error { - if l == nil || l.f == nil { - return nil - } - _ = syscall.Flock(int(l.f.Fd()), syscall.LOCK_UN) - cerr := l.f.Close() - rerr := os.Remove(l.Path) - l.f = nil - if cerr != nil { - return cerr - } - if rerr != nil && !errors.Is(rerr, os.ErrNotExist) { - return rerr - } - return nil -} diff --git a/internal/broker/lifecycle_test.go b/internal/broker/lifecycle_test.go deleted file mode 100644 index d7b7b4b..0000000 --- a/internal/broker/lifecycle_test.go +++ /dev/null @@ -1,33 +0,0 @@ -//go:build unix - -package broker - -import ( - "errors" - "os" - "path/filepath" - "testing" -) - -func TestPidLock_Singleton(t *testing.T) { - path := filepath.Join(t.TempDir(), "test.pid") - first, err := AcquirePidLock(path) - if err != nil { - t.Fatalf("first AcquirePidLock: %v", err) - } - if _, err := AcquirePidLock(path); !errors.Is(err, ErrAlreadyRunning) { - t.Fatalf("expected ErrAlreadyRunning on second acquire, got %v", err) - } - if err := first.Release(); err != nil { - t.Fatalf("Release: %v", err) - } - if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { - t.Fatalf("pid file should be removed, stat err = %v", err) - } - // After release, a fresh acquire must succeed. - second, err := AcquirePidLock(path) - if err != nil { - t.Fatalf("third AcquirePidLock: %v", err) - } - _ = second.Release() -} diff --git a/internal/broker/methods.go b/internal/broker/methods.go deleted file mode 100644 index 5ce4c74..0000000 --- a/internal/broker/methods.go +++ /dev/null @@ -1,226 +0,0 @@ -package broker - -import ( - "context" - "encoding/json" - "errors" - "time" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/authn" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc" -) - -// Methods bundles the broker's dependencies and exposes a MethodTable -// for socket dispatch. The fields are exported so cmd/sherlock-broker -// can construct one with real implementations and tests can swap in -// fakes (the *keyring.Store interface is the obvious seam). -type Methods struct { - Store keyring.Store - Flow *authn.Flow - Refresher *authn.Refresher - Cfg authn.Config - - // ShutdownFn is invoked by the "shutdown" RPC. Wired by the daemon - // to cancel the root context. - ShutdownFn func() - - // AgentNames feeds the status response so the CLI can show which - // profiles are available without re-loading them. - AgentNames func() []string -} - -// MethodTable returns the map suitable for NewService. -func (m *Methods) MethodTable() map[string]MethodHandler { - return map[string]MethodHandler{ - rpc.MethodStatus: m.handleStatus, - rpc.MethodWhoami: m.handleWhoami, - rpc.MethodLoginStart: m.handleLoginStart, - rpc.MethodLoginWait: m.handleLoginWait, - rpc.MethodLogout: m.handleLogout, - rpc.MethodGetIDToken: m.handleGetIDToken, - rpc.MethodShutdown: m.handleShutdown, - } -} - -// loadTokens reads the keyring once and caches the result in memory. -// We re-read on every status to keep things simple — the keyring is -// the source of truth. -func (m *Methods) currentTokens() (keyring.TokenSet, bool, error) { - ts, err := m.Store.GetAuthentikTokens() - if errors.Is(err, keyring.ErrNoTokens) { - return keyring.TokenSet{}, false, nil - } - if err != nil { - return keyring.TokenSet{}, false, err - } - return ts, !ts.Empty(), nil -} - -// StatusResult is what the status RPC returns. -type StatusResult struct { - LoggedIn bool `json:"logged_in"` - Subject string `json:"sub,omitempty"` - Name string `json:"name,omitempty"` - Email string `json:"email,omitempty"` - IDExpiresAt *time.Time `json:"id_expires_at,omitempty"` - RefreshExpAt *time.Time `json:"refresh_expires_at,omitempty"` - Issuer string `json:"issuer,omitempty"` - AuthnConfigured bool `json:"authn_configured"` - Agents []string `json:"agents"` -} - -func ptrTime(t time.Time) *time.Time { - if t.IsZero() { - return nil - } - tt := t - return &tt -} - -func (m *Methods) handleStatus(_ context.Context, _ json.RawMessage) (any, *rpc.Error) { - ts, ok, err := m.currentTokens() - if err != nil { - return nil, rpc.Errorf(rpc.CodeInternal, "read keyring: %v", err) - } - out := StatusResult{ - LoggedIn: ok, - AuthnConfigured: m.Cfg.Configured(), - } - if m.AgentNames != nil { - out.Agents = m.AgentNames() - } - if ok { - out.Subject = ts.Subject - out.Name = ts.Name - out.Email = ts.Email - out.Issuer = ts.Issuer - out.IDExpiresAt = ptrTime(ts.IDExpiresAt) - out.RefreshExpAt = ptrTime(ts.RefreshExpAt) - } - return out, nil -} - -// WhoamiResult is what the whoami RPC returns. -type WhoamiResult struct { - Subject string `json:"sub"` - Name string `json:"name,omitempty"` - Email string `json:"email,omitempty"` -} - -func (m *Methods) handleWhoami(_ context.Context, _ json.RawMessage) (any, *rpc.Error) { - ts, ok, err := m.currentTokens() - if err != nil { - return nil, rpc.Errorf(rpc.CodeInternal, "read keyring: %v", err) - } - if !ok { - return nil, rpc.NewError(rpc.CodeNotLoggedIn, "not logged in") - } - return WhoamiResult{Subject: ts.Subject, Name: ts.Name, Email: ts.Email}, nil -} - -// LoginStartResult is what the login_start RPC returns. -type LoginStartResult struct { - AuthURL string `json:"auth_url"` - State string `json:"state"` -} - -func (m *Methods) handleLoginStart(ctx context.Context, _ json.RawMessage) (any, *rpc.Error) { - if !m.Cfg.Configured() { - return nil, rpc.NewError(rpc.CodeNotConfigured, - "broker is not configured for Authentik yet; see plan.md Open #11 — set SHERLOCK_AUTHENTIK_ISSUER and SHERLOCK_AUTHENTIK_CLIENT_ID") - } - res, err := m.Flow.StartFlow(ctx) - if err != nil { - if errors.Is(err, authn.ErrNotConfigured) { - return nil, rpc.NewError(rpc.CodeNotConfigured, err.Error()) - } - return nil, rpc.Errorf(rpc.CodeInternal, "start flow: %v", err) - } - return LoginStartResult{AuthURL: res.AuthURL, State: res.State}, nil -} - -// LoginWaitParams are accepted by the login_wait RPC. -type LoginWaitParams struct { - State string `json:"state"` - TimeoutSeconds int `json:"timeout_seconds"` -} - -// LoginWaitResult is what login_wait returns. -type LoginWaitResult struct { - OK bool `json:"ok"` - Subject string `json:"sub,omitempty"` -} - -func (m *Methods) handleLoginWait(ctx context.Context, raw json.RawMessage) (any, *rpc.Error) { - var p LoginWaitParams - if len(raw) > 0 { - if err := json.Unmarshal(raw, &p); err != nil { - return nil, rpc.Errorf(rpc.CodeBadRequest, "decode params: %v", err) - } - } - if p.State == "" { - return nil, rpc.NewError(rpc.CodeBadRequest, "state is required") - } - timeout := time.Duration(p.TimeoutSeconds) * time.Second - res, err := m.Flow.AwaitCallback(ctx, p.State, timeout) - if err != nil { - if errors.Is(err, authn.ErrFlowExpired) { - return nil, rpc.NewError(rpc.CodeFlowExpired, err.Error()) - } - if errors.Is(err, authn.ErrUnknownState) { - return nil, rpc.NewError(rpc.CodeBadRequest, err.Error()) - } - return nil, rpc.Errorf(rpc.CodeInternal, "await callback: %v", err) - } - if err := m.Store.SetAuthentikTokens(res.ToTokenSet()); err != nil { - return nil, rpc.Errorf(rpc.CodeInternal, "persist tokens: %v", err) - } - return LoginWaitResult{OK: true, Subject: res.Subject}, nil -} - -func (m *Methods) handleLogout(_ context.Context, _ json.RawMessage) (any, *rpc.Error) { - if err := m.Store.ClearAuthentikTokens(); err != nil { - return nil, rpc.Errorf(rpc.CodeInternal, "clear keyring: %v", err) - } - return struct{}{}, nil -} - -// GetIDTokenResult is what get_id_token returns. -type GetIDTokenResult struct { - Token string `json:"token"` - ExpiresAt time.Time `json:"expires_at"` -} - -func (m *Methods) handleGetIDToken(ctx context.Context, _ json.RawMessage) (any, *rpc.Error) { - ts, ok, err := m.currentTokens() - if err != nil { - return nil, rpc.Errorf(rpc.CodeInternal, "read keyring: %v", err) - } - if !ok { - return nil, rpc.NewError(rpc.CodeNotLoggedIn, "not logged in") - } - if m.Refresher != nil { - fresh, err := m.Refresher.EnsureFresh(ctx, ts) - if err != nil { - if errors.Is(err, authn.ErrNoRefreshToken) { - return nil, rpc.NewError(rpc.CodeNotLoggedIn, "refresh token missing — re-login required") - } - return nil, rpc.Errorf(rpc.CodeInternal, "refresh: %v", err) - } - if fresh.IDToken != ts.IDToken { - if err := m.Store.SetAuthentikTokens(fresh); err != nil { - return nil, rpc.Errorf(rpc.CodeInternal, "persist refreshed tokens: %v", err) - } - ts = fresh - } - } - return GetIDTokenResult{Token: ts.IDToken, ExpiresAt: ts.IDExpiresAt}, nil -} - -func (m *Methods) handleShutdown(_ context.Context, _ json.RawMessage) (any, *rpc.Error) { - if m.ShutdownFn != nil { - go m.ShutdownFn() // run async so we can return a response first - } - return struct{}{}, nil -} diff --git a/internal/broker/methods_test.go b/internal/broker/methods_test.go deleted file mode 100644 index 631bd7a..0000000 --- a/internal/broker/methods_test.go +++ /dev/null @@ -1,110 +0,0 @@ -package broker - -import ( - "context" - "encoding/json" - "testing" - "time" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/authn" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring" - fakekeyring "gitea.alexandru.macocian.me/Charlie/sherlock/internal/keyring/fake" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc" -) - -func newMethods(t *testing.T) *Methods { - t.Helper() - return &Methods{ - Store: fakekeyring.New(), - Cfg: authn.Config{}, - AgentNames: func() []string { return []string{"copilot"} }, - } -} - -func TestStatus_NotLoggedIn(t *testing.T) { - m := newMethods(t) - res, rpcErr := m.handleStatus(context.Background(), nil) - if rpcErr != nil { - t.Fatalf("rpc err: %v", rpcErr) - } - s := res.(StatusResult) - if s.LoggedIn { - t.Fatal("expected not logged in") - } - if s.AuthnConfigured { - t.Fatal("expected not configured") - } - if len(s.Agents) != 1 || s.Agents[0] != "copilot" { - t.Fatalf("agents = %v", s.Agents) - } -} - -func TestStatus_LoggedIn(t *testing.T) { - m := newMethods(t) - exp := time.Now().Add(time.Hour).UTC().Truncate(time.Second) - if err := m.Store.SetAuthentikTokens(keyring.TokenSet{ - IDToken: "id", Subject: "alice", Email: "a@e", Name: "Alice", - IDExpiresAt: exp, Issuer: "https://id.example/", - }); err != nil { - t.Fatal(err) - } - res, rpcErr := m.handleStatus(context.Background(), nil) - if rpcErr != nil { - t.Fatalf("rpc err: %v", rpcErr) - } - s := res.(StatusResult) - if !s.LoggedIn || s.Subject != "alice" { - t.Fatalf("got %+v", s) - } - if s.IDExpiresAt == nil || !s.IDExpiresAt.Equal(exp) { - t.Fatalf("expiry: got %v want %v", s.IDExpiresAt, exp) - } -} - -func TestWhoami_NotLoggedIn(t *testing.T) { - m := newMethods(t) - _, rpcErr := m.handleWhoami(context.Background(), nil) - if rpcErr == nil || rpcErr.Code != rpc.CodeNotLoggedIn { - t.Fatalf("want not_logged_in, got %+v", rpcErr) - } -} - -func TestLoginStart_NotConfigured(t *testing.T) { - m := newMethods(t) - _, rpcErr := m.handleLoginStart(context.Background(), nil) - if rpcErr == nil || rpcErr.Code != rpc.CodeNotConfigured { - t.Fatalf("want not_configured, got %+v", rpcErr) - } -} - -func TestLogout_Idempotent(t *testing.T) { - m := newMethods(t) - if _, err := m.handleLogout(context.Background(), nil); err != nil { - t.Fatalf("logout1: %v", err) - } - if _, err := m.handleLogout(context.Background(), nil); err != nil { - t.Fatalf("logout2: %v", err) - } -} - -func TestShutdown_InvokesFn(t *testing.T) { - done := make(chan struct{}) - m := newMethods(t) - m.ShutdownFn = func() { close(done) } - if _, err := m.handleShutdown(context.Background(), nil); err != nil { - t.Fatalf("shutdown: %v", err) - } - select { - case <-done: - case <-time.After(time.Second): - t.Fatal("ShutdownFn not invoked") - } -} - -func TestLoginWait_BadParams(t *testing.T) { - m := newMethods(t) - _, rpcErr := m.handleLoginWait(context.Background(), json.RawMessage(`{}`)) - if rpcErr == nil || rpcErr.Code != rpc.CodeBadRequest { - t.Fatalf("want bad_request, got %+v", rpcErr) - } -} diff --git a/internal/broker/server.go b/internal/broker/server.go deleted file mode 100644 index 0fc3396..0000000 --- a/internal/broker/server.go +++ /dev/null @@ -1,68 +0,0 @@ -package broker - -import ( - "context" - "encoding/json" - "sync" - "time" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc" -) - -// MethodHandler is the per-method signature used by Service. Handlers -// receive the raw params (so each can unmarshal into its own typed -// struct) and return either a marshallable result or an *rpc.Error. -type MethodHandler func(ctx context.Context, params json.RawMessage) (any, *rpc.Error) - -// Service is the broker's method table. It is the socket.Handler used -// by socket.Server. Tracks last-activity for the idle-timeout watcher -// in cmd/sherlock-broker. -type Service struct { - methods map[string]MethodHandler - - mu sync.RWMutex - lastActivity time.Time -} - -// NewService builds a Service from the provided method table. The -// reserved method "shutdown" is wired separately by the caller (it -// needs to cancel the broker's root context). -func NewService(methods map[string]MethodHandler) *Service { - cp := make(map[string]MethodHandler, len(methods)) - for k, v := range methods { - cp[k] = v - } - return &Service{ - methods: cp, - lastActivity: time.Now(), - } -} - -// LastActivity returns the timestamp of the most recent request. -func (s *Service) LastActivity() time.Time { - s.mu.RLock() - defer s.mu.RUnlock() - return s.lastActivity -} - -// Handle is the socket.Handler implementation. It looks up the method, -// invokes it, and packs the result. -func (s *Service) Handle(ctx context.Context, req rpc.Request) rpc.Response { - s.mu.Lock() - s.lastActivity = time.Now() - s.mu.Unlock() - - h, ok := s.methods[req.Method] - if !ok { - return rpc.Response{Error: rpc.Errorf(rpc.CodeUnknownMethod, "unknown method %q", req.Method)} - } - result, rpcErr := h(ctx, req.Params) - if rpcErr != nil { - return rpc.Response{Error: rpcErr} - } - raw, err := rpc.MarshalResult(result) - if err != nil { - return rpc.Response{Error: rpc.Errorf(rpc.CodeInternal, "marshal result: %v", err)} - } - return rpc.Response{Result: raw} -} diff --git a/internal/broker/server_test.go b/internal/broker/server_test.go deleted file mode 100644 index 2dee5c3..0000000 --- a/internal/broker/server_test.go +++ /dev/null @@ -1,58 +0,0 @@ -package broker - -import ( - "context" - "encoding/json" - "testing" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc" -) - -func TestService_UnknownMethod(t *testing.T) { - s := NewService(nil) - resp := s.Handle(context.Background(), rpc.Request{Method: "nope"}) - if resp.Error == nil || resp.Error.Code != rpc.CodeUnknownMethod { - t.Fatalf("expected unknown_method, got %+v", resp.Error) - } -} - -func TestService_DispatchAndActivity(t *testing.T) { - calls := 0 - s := NewService(map[string]MethodHandler{ - "ping": func(ctx context.Context, params json.RawMessage) (any, *rpc.Error) { - calls++ - return map[string]string{"ok": "true"}, nil - }, - }) - before := s.LastActivity() - resp := s.Handle(context.Background(), rpc.Request{Method: "ping"}) - if resp.Error != nil { - t.Fatalf("unexpected error: %v", resp.Error) - } - if calls != 1 { - t.Fatalf("handler not called") - } - if !s.LastActivity().After(before) && s.LastActivity().Equal(before) { - // Equal is OK if clock resolution is coarse; just sanity-check that it didn't go backwards. - t.Fatalf("LastActivity moved backwards: before=%v after=%v", before, s.LastActivity()) - } - var out map[string]string - if err := json.Unmarshal(resp.Result, &out); err != nil { - t.Fatalf("unmarshal: %v", err) - } - if out["ok"] != "true" { - t.Fatalf("got %v", out) - } -} - -func TestService_HandlerError(t *testing.T) { - s := NewService(map[string]MethodHandler{ - "fail": func(ctx context.Context, params json.RawMessage) (any, *rpc.Error) { - return nil, rpc.NewError(rpc.CodeNotLoggedIn, "no session") - }, - }) - resp := s.Handle(context.Background(), rpc.Request{Method: "fail"}) - if resp.Error == nil || resp.Error.Code != rpc.CodeNotLoggedIn { - t.Fatalf("expected not_logged_in, got %+v", resp.Error) - } -} diff --git a/internal/broker/spawn.go b/internal/broker/spawn.go deleted file mode 100644 index 5ef92c5..0000000 --- a/internal/broker/spawn.go +++ /dev/null @@ -1,142 +0,0 @@ -//go:build unix - -package broker - -import ( - "context" - "fmt" - "os" - "os/exec" - "path/filepath" - "syscall" - "time" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/socket" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/xdg" -) - -// EnsureRunningOptions tunes EnsureRunning for tests. Production code -// passes a zero value. -type EnsureRunningOptions struct { - // BrokerBinary overrides the auto-detected sherlock-broker path. - // When empty, EnsureRunning resolves a sibling of os.Executable() - // then falls back to $PATH. - BrokerBinary string - // SpawnTimeout is how long to wait for the socket to appear after - // forking. Zero means 2 seconds. - SpawnTimeout time.Duration - // SocketPath overrides the canonical broker socket. Tests use - // this to drive a temp socket. - SocketPath string - // ExtraEnv is appended to the child env. Useful for tests pinning - // XDG_RUNTIME_DIR. - ExtraEnv []string -} - -// EnsureRunning dials the broker socket, forking + setsid-detaching -// the broker binary if no one is listening. Returns a connected Client. -// -// Decision #10: zero host setup. The wrapper CLI is what starts the -// broker the first time it's needed. -func EnsureRunning(ctx context.Context, opts EnsureRunningOptions) (*socket.Client, error) { - sockPath := opts.SocketPath - if sockPath == "" { - p, err := xdg.SocketPath() - if err != nil { - return nil, err - } - sockPath = p - } - - if c, err := socket.Dial(sockPath); err == nil { - return c, nil - } - - // Not running. Fork a fresh broker. - bin := opts.BrokerBinary - if bin == "" { - resolved, err := resolveBrokerBinary() - if err != nil { - return nil, err - } - bin = resolved - } - - logPath, err := xdg.LogPath() - if err != nil { - return nil, err - } - logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) - if err != nil { - return nil, fmt.Errorf("broker: open log %s: %w", logPath, err) - } - - cmd := exec.Command(bin, "daemon") - cmd.Stdin = nil - cmd.Stdout = logFile - cmd.Stderr = logFile - cmd.Env = append(os.Environ(), opts.ExtraEnv...) - // setsid so the child outlives the parent and isn't part of the - // CLI's process group. - cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} - - if err := cmd.Start(); err != nil { - _ = logFile.Close() - return nil, fmt.Errorf("broker: spawn %s: %w", bin, err) - } - // We don't Wait on the child — it's detached. Release the fd we - // kept open in the parent. - _ = logFile.Close() - // Release the child so it isn't reparented to us / left as a zombie - // if the CLI sticks around (it normally execs into the agent and - // disappears, but tests don't). - _ = cmd.Process.Release() - - deadline := opts.SpawnTimeout - if deadline <= 0 { - deadline = 2 * time.Second - } - return pollDial(ctx, sockPath, deadline) -} - -func pollDial(ctx context.Context, sockPath string, timeout time.Duration) (*socket.Client, error) { - end := time.Now().Add(timeout) - for { - if c, err := socket.Dial(sockPath); err == nil { - return c, nil - } - if time.Now().After(end) { - return nil, fmt.Errorf("broker: socket %s did not appear within %s", sockPath, timeout) - } - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-time.After(50 * time.Millisecond): - } - } -} - -// resolveBrokerBinary looks for a sherlock-broker sibling of the -// running executable (so a `go install`'d pair stays consistent) then -// falls back to $PATH. -func resolveBrokerBinary() (string, error) { - const name = "sherlock-broker" - if exe, err := os.Executable(); err == nil { - candidate := filepath.Join(filepath.Dir(exe), name) - if isExecutable(candidate) { - return candidate, nil - } - } - if p, err := exec.LookPath(name); err == nil { - return p, nil - } - return "", fmt.Errorf("broker: could not locate %s on PATH or beside sherlock binary", name) -} - -func isExecutable(p string) bool { - fi, err := os.Stat(p) - if err != nil || fi.IsDir() { - return false - } - return fi.Mode()&0o111 != 0 -} diff --git a/internal/keyring/keyring.go b/internal/keyring/keyring.go index 18fbd5e..14301f7 100644 --- a/internal/keyring/keyring.go +++ b/internal/keyring/keyring.go @@ -1,14 +1,13 @@ // Package keyring is sherlock's gateway to the OS credential store. // -// Decision #8 (Phase 1): persist tokens in the OS keyring (Secret -// Service on Linux, Keychain on macOS, Credential Manager on Windows). -// We do not write plaintext or age-encrypted tokens to disk. +// Decision #8: persist tokens in the OS keyring (Secret Service on +// Linux, Keychain on macOS, Credential Manager on Windows). No +// plaintext or age-encrypted blobs on disk. // -// Both `sherlock` and `sherlock-broker` MUST call Probe at startup -// and exit early (with platform-specific guidance) if the keyring -// is unavailable. The broker holds the long-lived refresh token, so -// running without a real keyring would silently lose tokens on every -// restart — better to fail loudly. +// The CLI MUST call Probe at startup and exit early (with +// platform-specific guidance) if the keyring is unavailable. Without +// the keyring, refresh tokens would be lost on every command — better +// to fail loudly. package keyring import ( @@ -87,7 +86,10 @@ func Probe() error { return nil } -// TokenSet is what the broker persists after a successful Authentik flow. +// TokenSet is what sherlock persists after a successful Authentik +// flow. ClientID + Scopes are recorded alongside the tokens so that +// background refresh works without depending on environment variables +// set in the original login shell. type TokenSet struct { IDToken string `json:"id_token"` AccessToken string `json:"access_token,omitempty"` @@ -95,6 +97,8 @@ type TokenSet struct { IDExpiresAt time.Time `json:"id_expires_at"` RefreshExpAt time.Time `json:"refresh_expires_at,omitempty"` Issuer string `json:"issuer"` + ClientID string `json:"client_id,omitempty"` + Scopes []string `json:"scopes,omitempty"` Subject string `json:"sub"` Email string `json:"email,omitempty"` Name string `json:"name,omitempty"` @@ -103,7 +107,7 @@ type TokenSet struct { // Empty reports whether the TokenSet carries no usable session. func (t TokenSet) Empty() bool { return t.IDToken == "" && t.RefreshToken == "" } -// Store is the typed wrapper the broker uses. Implementations are +// Store is the typed wrapper sherlock uses. Implementations are // expected to be safe for concurrent use. type Store interface { GetAuthentikTokens() (TokenSet, error) diff --git a/internal/rpc/rpc.go b/internal/rpc/rpc.go deleted file mode 100644 index c13ef87..0000000 --- a/internal/rpc/rpc.go +++ /dev/null @@ -1,113 +0,0 @@ -// Package rpc defines the wire types and error codes for sherlock's -// JSON-over-newline RPC, spoken on the broker UDS at -// $XDG_RUNTIME_DIR/sherlock.sock. -// -// Wire format: each Request and each Response is a single JSON object -// terminated by '\n'. The framing is documented in docs/rpc.md. -package rpc - -import ( - "encoding/json" - "fmt" -) - -// Method names. Constants live here so client and server agree on spelling. -const ( - MethodStatus = "status" - MethodWhoami = "whoami" - MethodLoginStart = "login_start" - MethodLoginWait = "login_wait" - MethodLogout = "logout" - MethodGetIDToken = "get_id_token" - MethodShutdown = "shutdown" -) - -// ErrorCode is the numeric machine-readable code on a failed Response. -type ErrorCode int - -const ( - CodeUnknownMethod ErrorCode = 1 - CodeBadRequest ErrorCode = 2 - CodeInternal ErrorCode = 3 - CodeNotLoggedIn ErrorCode = 4 - CodeKeyringUnavailable ErrorCode = 5 - CodeFlowExpired ErrorCode = 6 - CodeNotConfigured ErrorCode = 7 - CodeAlreadyInFlight ErrorCode = 8 -) - -func (c ErrorCode) String() string { - switch c { - case CodeUnknownMethod: - return "unknown_method" - case CodeBadRequest: - return "bad_request" - case CodeInternal: - return "internal" - case CodeNotLoggedIn: - return "not_logged_in" - case CodeKeyringUnavailable: - return "keyring_unavailable" - case CodeFlowExpired: - return "flow_expired" - case CodeNotConfigured: - return "not_configured" - case CodeAlreadyInFlight: - return "already_in_flight" - default: - return fmt.Sprintf("code_%d", int(c)) - } -} - -// Request is one JSON object on one line. -type Request struct { - ID uint64 `json:"id"` - Method string `json:"method"` - Params json.RawMessage `json:"params,omitempty"` -} - -// Response is one JSON object on one line. Exactly one of Result/Error is set. -type Response struct { - ID uint64 `json:"id"` - Result json.RawMessage `json:"result,omitempty"` - Error *Error `json:"error,omitempty"` -} - -// Error is the structured failure payload on a Response. -type Error struct { - Code ErrorCode `json:"code"` - Message string `json:"message"` -} - -func (e *Error) Error() string { - if e == nil { - return "" - } - return fmt.Sprintf("%s: %s", e.Code, e.Message) -} - -// NewError is a convenience for handlers that want to fail a request. -func NewError(code ErrorCode, msg string) *Error { - return &Error{Code: code, Message: msg} -} - -// Errorf is the printf-style sibling of NewError. -func Errorf(code ErrorCode, format string, a ...any) *Error { - return &Error{Code: code, Message: fmt.Sprintf(format, a...)} -} - -// MarshalParams is a small helper so handlers don't repeat the -// json.Marshal + RawMessage dance. -func MarshalParams(v any) (json.RawMessage, error) { - if v == nil { - return nil, nil - } - b, err := json.Marshal(v) - if err != nil { - return nil, err - } - return json.RawMessage(b), nil -} - -// MarshalResult is the symmetric helper for Response.Result. -func MarshalResult(v any) (json.RawMessage, error) { return MarshalParams(v) } diff --git a/internal/rpc/rpc_test.go b/internal/rpc/rpc_test.go deleted file mode 100644 index f72c725..0000000 --- a/internal/rpc/rpc_test.go +++ /dev/null @@ -1,69 +0,0 @@ -package rpc - -import ( - "encoding/json" - "testing" -) - -func TestErrorCodeString(t *testing.T) { - cases := map[ErrorCode]string{ - CodeUnknownMethod: "unknown_method", - CodeBadRequest: "bad_request", - CodeInternal: "internal", - CodeNotLoggedIn: "not_logged_in", - CodeKeyringUnavailable: "keyring_unavailable", - CodeFlowExpired: "flow_expired", - CodeNotConfigured: "not_configured", - CodeAlreadyInFlight: "already_in_flight", - ErrorCode(99): "code_99", - } - for code, want := range cases { - if got := code.String(); got != want { - t.Errorf("%d.String() = %q, want %q", int(code), got, want) - } - } -} - -func TestRequestRoundTrip(t *testing.T) { - params, err := MarshalParams(map[string]string{"foo": "bar"}) - if err != nil { - t.Fatalf("MarshalParams: %v", err) - } - req := Request{ID: 42, Method: "status", Params: params} - b, err := json.Marshal(req) - if err != nil { - t.Fatalf("Marshal: %v", err) - } - var got Request - if err := json.Unmarshal(b, &got); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - if got.ID != 42 || got.Method != "status" { - t.Fatalf("round-trip lost fields: %+v", got) - } - var p map[string]string - if err := json.Unmarshal(got.Params, &p); err != nil { - t.Fatalf("params: %v", err) - } - if p["foo"] != "bar" { - t.Fatalf("params: %v", p) - } -} - -func TestResponseErrorRoundTrip(t *testing.T) { - resp := Response{ID: 7, Error: NewError(CodeNotLoggedIn, "no session")} - b, err := json.Marshal(resp) - if err != nil { - t.Fatalf("Marshal: %v", err) - } - var got Response - if err := json.Unmarshal(b, &got); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - if got.Error == nil || got.Error.Code != CodeNotLoggedIn { - t.Fatalf("error not preserved: %+v", got.Error) - } - if got.Error.Error() != "not_logged_in: no session" { - t.Fatalf("Error() = %q", got.Error.Error()) - } -} diff --git a/internal/socket/helpers_test.go b/internal/socket/helpers_test.go deleted file mode 100644 index e0e7a15..0000000 --- a/internal/socket/helpers_test.go +++ /dev/null @@ -1,10 +0,0 @@ -package socket_test - -import "net" - -// dialRaw is a tiny helper used by tests that need to send raw bytes -// without going through the RPC client (to exercise framing/decode -// error paths). -func dialRaw(path string) (net.Conn, error) { - return net.Dial("unix", path) -} diff --git a/internal/socket/socket.go b/internal/socket/socket.go deleted file mode 100644 index ebe2ff4..0000000 --- a/internal/socket/socket.go +++ /dev/null @@ -1,251 +0,0 @@ -// Package socket implements a tiny JSON-over-newline RPC transport on -// top of a Unix domain socket. The broker is the server; the sherlock -// CLI and (later) per-service MCPs are clients. -// -// Framing: each rpc.Request and rpc.Response is one JSON object -// terminated by a single '\n'. No length prefixes, no chunked frames. -// This keeps the protocol trivially debuggable with `socat - UNIX-CONNECT:...`. -package socket - -import ( - "bufio" - "context" - "encoding/json" - "errors" - "fmt" - "io" - "net" - "os" - "path/filepath" - "sync" - "sync/atomic" - "time" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc" -) - -// Max bytes for a single framed JSON object. Plenty of headroom for -// status / token payloads; guards against runaway parser allocations. -const maxFrameBytes = 1 << 20 // 1 MiB - -// Handler turns a request into a response. Implementations should not -// write directly to the connection — the server takes care of framing. -type Handler interface { - Handle(ctx context.Context, req rpc.Request) rpc.Response -} - -// HandlerFunc adapts a function to the Handler interface. -type HandlerFunc func(ctx context.Context, req rpc.Request) rpc.Response - -func (f HandlerFunc) Handle(ctx context.Context, req rpc.Request) rpc.Response { - return f(ctx, req) -} - -// Server listens on a Unix socket and dispatches each framed request -// to a Handler on its own goroutine per connection. -type Server struct { - ln net.Listener - handler Handler - wg sync.WaitGroup - closing atomic.Bool - onAccept func() // test hook: called once per accepted conn before dispatch. -} - -// Listen creates the socket at path, removing any stale leftover file. -// Mode 0600 ensures only the owner can dial. -func Listen(path string, handler Handler) (*Server, error) { - if handler == nil { - return nil, errors.New("socket: nil handler") - } - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - return nil, fmt.Errorf("socket: mkdir parent: %w", err) - } - // Best-effort cleanup of a stale socket file. We deliberately don't - // touch it if another broker is actively listening — Listen below - // will fail with EADDRINUSE in that case. - if fi, err := os.Stat(path); err == nil && fi.Mode()&os.ModeSocket != 0 { - _ = os.Remove(path) - } - ln, err := net.Listen("unix", path) - if err != nil { - return nil, fmt.Errorf("socket: listen %s: %w", path, err) - } - if err := os.Chmod(path, 0o600); err != nil { - _ = ln.Close() - return nil, fmt.Errorf("socket: chmod %s: %w", path, err) - } - return &Server{ln: ln, handler: handler}, nil -} - -// Addr returns the socket path. -func (s *Server) Addr() string { return s.ln.Addr().String() } - -// Serve accepts connections until Close is called or ctx is cancelled. -// Returns nil on a clean shutdown. -func (s *Server) Serve(ctx context.Context) error { - go func() { - <-ctx.Done() - _ = s.Close() - }() - for { - conn, err := s.ln.Accept() - if err != nil { - if s.closing.Load() { - s.wg.Wait() - return nil - } - return fmt.Errorf("socket: accept: %w", err) - } - if s.onAccept != nil { - s.onAccept() - } - s.wg.Add(1) - go s.serveConn(ctx, conn) - } -} - -// Close stops accepting new connections and removes the socket file. -// In-flight handlers continue until they return; Serve waits for them. -func (s *Server) Close() error { - if !s.closing.CompareAndSwap(false, true) { - return nil - } - err := s.ln.Close() - _ = os.Remove(s.Addr()) - return err -} - -func (s *Server) serveConn(ctx context.Context, conn net.Conn) { - defer s.wg.Done() - defer func() { _ = conn.Close() }() - - scanner := bufio.NewScanner(conn) - scanner.Buffer(make([]byte, 64*1024), maxFrameBytes) - w := bufio.NewWriter(conn) - - for scanner.Scan() { - line := scanner.Bytes() - if len(line) == 0 { - continue - } - var req rpc.Request - var resp rpc.Response - if err := json.Unmarshal(line, &req); err != nil { - resp = rpc.Response{Error: rpc.Errorf(rpc.CodeBadRequest, "decode request: %v", err)} - } else { - resp = s.handler.Handle(ctx, req) - resp.ID = req.ID - } - if err := writeFrame(w, resp); err != nil { - return - } - } - // Drain a non-EOF scanner error silently — the client likely hung up. -} - -func writeFrame(w *bufio.Writer, resp rpc.Response) error { - b, err := json.Marshal(resp) - if err != nil { - // Last-ditch: send a generic internal error. If even that fails - // we have nothing useful to do but close the connection. - fallback, _ := json.Marshal(rpc.Response{ - ID: resp.ID, - Error: rpc.NewError(rpc.CodeInternal, "marshal response failed"), - }) - if _, err := w.Write(fallback); err != nil { - return err - } - if err := w.WriteByte('\n'); err != nil { - return err - } - return w.Flush() - } - if _, err := w.Write(b); err != nil { - return err - } - if err := w.WriteByte('\n'); err != nil { - return err - } - return w.Flush() -} - -// Client is a thin RPC client over the UDS. It is safe to use a single -// Client from multiple goroutines: Call serialises writes and matches -// responses by request ID. -type Client struct { - conn net.Conn - mu sync.Mutex - rd *bufio.Reader - nextID atomic.Uint64 -} - -// Dial connects to the broker UDS at path. The caller owns the returned -// Client and must Close it when done. -func Dial(path string) (*Client, error) { - conn, err := net.Dial("unix", path) - if err != nil { - return nil, err - } - return &Client{conn: conn, rd: bufio.NewReader(conn)}, nil -} - -// Close closes the underlying socket. -func (c *Client) Close() error { return c.conn.Close() } - -// Call sends a request and unmarshals the response. -// -// We hold the per-client mutex for the whole round-trip. Sherlock RPC -// calls are short (sub-second) and never streamed, so request pipelining -// would be pointless — keeping the protocol synchronous keeps the client -// trivially correct and lets us reuse a single connection across calls. -func (c *Client) Call(ctx context.Context, method string, params, result any) error { - c.mu.Lock() - defer c.mu.Unlock() - - id := c.nextID.Add(1) - pb, err := rpc.MarshalParams(params) - if err != nil { - return fmt.Errorf("socket: marshal params: %w", err) - } - req := rpc.Request{ID: id, Method: method, Params: pb} - reqBytes, err := json.Marshal(req) - if err != nil { - return fmt.Errorf("socket: marshal request: %w", err) - } - - // Honour ctx with a connection-level deadline. Cheap and avoids a - // reader goroutine just for cancellation. - if dl, ok := ctx.Deadline(); ok { - _ = c.conn.SetDeadline(dl) - defer func() { _ = c.conn.SetDeadline(time.Time{}) }() - } - - if _, err := c.conn.Write(append(reqBytes, '\n')); err != nil { - return fmt.Errorf("socket: write: %w", err) - } - - line, err := c.rd.ReadBytes('\n') - if err != nil { - if errors.Is(err, io.EOF) { - return fmt.Errorf("socket: broker closed connection") - } - return fmt.Errorf("socket: read: %w", err) - } - var resp rpc.Response - if err := json.Unmarshal(line, &resp); err != nil { - return fmt.Errorf("socket: decode response: %w", err) - } - if resp.ID != id { - return fmt.Errorf("socket: response id mismatch: got %d want %d", resp.ID, id) - } - if resp.Error != nil { - return resp.Error - } - if result == nil || len(resp.Result) == 0 { - return nil - } - if err := json.Unmarshal(resp.Result, result); err != nil { - return fmt.Errorf("socket: decode result: %w", err) - } - return nil -} diff --git a/internal/socket/socket_test.go b/internal/socket/socket_test.go deleted file mode 100644 index 5281ea0..0000000 --- a/internal/socket/socket_test.go +++ /dev/null @@ -1,166 +0,0 @@ -package socket_test - -import ( - "context" - "encoding/json" - "path/filepath" - "sync" - "testing" - "time" - - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/rpc" - "gitea.alexandru.macocian.me/Charlie/sherlock/internal/socket" -) - -func newServer(t *testing.T, handler socket.Handler) (string, func()) { - t.Helper() - path := filepath.Join(t.TempDir(), "test.sock") - srv, err := socket.Listen(path, handler) - if err != nil { - t.Fatalf("Listen: %v", err) - } - ctx, cancel := context.WithCancel(context.Background()) - done := make(chan struct{}) - go func() { - defer close(done) - if err := srv.Serve(ctx); err != nil { - t.Errorf("Serve: %v", err) - } - }() - return path, func() { - cancel() - _ = srv.Close() - <-done - } -} - -func TestRoundTrip_Success(t *testing.T) { - handler := socket.HandlerFunc(func(_ context.Context, req rpc.Request) rpc.Response { - if req.Method != "echo" { - return rpc.Response{Error: rpc.NewError(rpc.CodeUnknownMethod, "nope")} - } - out, _ := rpc.MarshalResult(map[string]string{"got": "ok"}) - return rpc.Response{Result: out} - }) - path, stop := newServer(t, handler) - defer stop() - - c, err := socket.Dial(path) - if err != nil { - t.Fatalf("Dial: %v", err) - } - defer func() { _ = c.Close() }() - - var resp map[string]string - if err := c.Call(context.Background(), "echo", nil, &resp); err != nil { - t.Fatalf("Call: %v", err) - } - if resp["got"] != "ok" { - t.Fatalf("got %v", resp) - } -} - -func TestRoundTrip_Error(t *testing.T) { - handler := socket.HandlerFunc(func(_ context.Context, req rpc.Request) rpc.Response { - return rpc.Response{Error: rpc.NewError(rpc.CodeNotLoggedIn, "session missing")} - }) - path, stop := newServer(t, handler) - defer stop() - - c, err := socket.Dial(path) - if err != nil { - t.Fatalf("Dial: %v", err) - } - defer func() { _ = c.Close() }() - - err = c.Call(context.Background(), "status", nil, nil) - if err == nil { - t.Fatal("expected error") - } - rpcErr, ok := err.(*rpc.Error) - if !ok { - t.Fatalf("expected *rpc.Error, got %T", err) - } - if rpcErr.Code != rpc.CodeNotLoggedIn { - t.Fatalf("code = %v", rpcErr.Code) - } -} - -func TestRoundTrip_BadJSON(t *testing.T) { - handler := socket.HandlerFunc(func(_ context.Context, req rpc.Request) rpc.Response { - return rpc.Response{} - }) - path, stop := newServer(t, handler) - defer stop() - - // Send garbage directly so we exercise the server-side decode path. - conn, err := dialRaw(path) - if err != nil { - t.Fatalf("dialRaw: %v", err) - } - defer func() { _ = conn.Close() }() - - if _, err := conn.Write([]byte("not json\n")); err != nil { - t.Fatalf("Write: %v", err) - } - buf := make([]byte, 1024) - n, err := conn.Read(buf) - if err != nil { - t.Fatalf("Read: %v", err) - } - var resp rpc.Response - if err := json.Unmarshal(buf[:n], &resp); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - if resp.Error == nil || resp.Error.Code != rpc.CodeBadRequest { - t.Fatalf("expected bad_request, got %+v", resp.Error) - } -} - -func TestConcurrentCalls(t *testing.T) { - handler := socket.HandlerFunc(func(_ context.Context, req rpc.Request) rpc.Response { - var in struct{ N int } - _ = json.Unmarshal(req.Params, &in) - out, _ := rpc.MarshalResult(map[string]int{"n": in.N}) - return rpc.Response{Result: out} - }) - path, stop := newServer(t, handler) - defer stop() - - const goroutines = 20 - const callsEach = 50 - - var wg sync.WaitGroup - for i := 0; i < goroutines; i++ { - wg.Add(1) - go func() { - defer wg.Done() - c, err := socket.Dial(path) - if err != nil { - t.Errorf("Dial: %v", err) - return - } - defer func() { _ = c.Close() }() - for j := 0; j < callsEach; j++ { - var out map[string]int - err := c.Call(context.Background(), "echo", - map[string]int{"N": j}, &out) - if err != nil { - t.Errorf("Call: %v", err) - return - } - if out["n"] != j { - t.Errorf("got %d want %d", out["n"], j) - return - } - } - }() - } - doneCh := make(chan struct{}) - go func() { wg.Wait(); close(doneCh) }() - select { - case <-doneCh: - case <-time.After(10 * time.Second): - t.Fatal("concurrent calls timed out") - } -} diff --git a/internal/xdg/xdg.go b/internal/xdg/xdg.go index e2039a3..6ec5d07 100644 --- a/internal/xdg/xdg.go +++ b/internal/xdg/xdg.go @@ -45,31 +45,15 @@ func ConfigHome() (string, error) { return ensureDir(filepath.Join(home, ".config"), 0o700) } -// SocketPath returns the canonical UDS path the broker listens on. -func SocketPath() (string, error) { +// RefreshLockPath returns the canonical flock path used by EnsureFresh +// to serialise concurrent Authentik refresh attempts across sherlock +// invocations. +func RefreshLockPath() (string, error) { d, err := RuntimeDir() if err != nil { return "", err } - return filepath.Join(d, "sherlock.sock"), nil -} - -// PidPath returns the canonical PID/lock file path for the broker. -func PidPath() (string, error) { - d, err := RuntimeDir() - if err != nil { - return "", err - } - return filepath.Join(d, "sherlock.pid"), nil -} - -// LogPath returns the canonical log file path for the forked broker. -func LogPath() (string, error) { - d, err := RuntimeDir() - if err != nil { - return "", err - } - return filepath.Join(d, "sherlock.log"), nil + return filepath.Join(d, "sherlock.refresh.lock"), nil } // MCPConfigDir returns $XDG_RUNTIME_DIR/sherlock, where per-agent MCP diff --git a/internal/xdg/xdg_test.go b/internal/xdg/xdg_test.go index 5c01128..4e8596a 100644 --- a/internal/xdg/xdg_test.go +++ b/internal/xdg/xdg_test.go @@ -55,9 +55,7 @@ func TestDerivedPaths(t *testing.T) { fn func() (string, error) want string }{ - {"SocketPath", SocketPath, filepath.Join(dir, "sherlock.sock")}, - {"PidPath", PidPath, filepath.Join(dir, "sherlock.pid")}, - {"LogPath", LogPath, filepath.Join(dir, "sherlock.log")}, + {"RefreshLockPath", RefreshLockPath, filepath.Join(dir, "sherlock.refresh.lock")}, {"MCPConfigDir", MCPConfigDir, filepath.Join(dir, "sherlock")}, } for _, c := range checks { diff --git a/plan.md b/plan.md index 4cb6a44..265fc28 100644 --- a/plan.md +++ b/plan.md @@ -228,20 +228,20 @@ Reuses the existing gssh server (`gssh.alexandru.macocian.me`) — sherlock does - **Name:** `sherlock`. (Replaces "sherlock" everywhere — `sherlock-broker`, `sherlock-mcp`, `sherlock copilot …`, etc.) - **Repo location:** new repo `Charlie/sherlock`, cloned to `~/Dev/charlie/sherlock/` (sibling of the existing per-app stack repos like `gitea/`, `runners/`, etc.). Added to `docs/inventory.md` under a new "Operator tooling" section and linked from `project-charlie/README.md`. -- **Language:** Go. Single static binaries per `cmd//`; best-in-class OAuth/OIDC libs (`golang.org/x/oauth2`, `coreos/go-oidc`); official MCP Go SDK; native UDS + systemd integration. -- **Phase 2 Gitea auth model:** broker mints an OIDC token *as the operator* via Authentik. Every MCP call shows up under the operator in Gitea's audit log. No dedicated `sherlock` service account. -- **Service-registry format:** TOML, under `~/.config/sherlock/services.d/*.toml`. Hot-reloaded by the broker. +- **Language:** Go. Single static binary `cmd/sherlock/`; best-in-class OAuth/OIDC libs (`golang.org/x/oauth2`, `coreos/go-oidc`); official MCP Go SDK; native Unix primitives (flock, syscall.Exec). +- **Phase 2 Gitea auth model:** the gitea-mcp uses an OIDC token *as the operator* via Authentik. Every MCP call shows up under the operator in Gitea's audit log. No dedicated `sherlock` service account. +- **Service-registry format:** TOML, under `~/.config/sherlock/services.d/*.toml`. Loaded at the start of each CLI invocation (no daemon to hot-reload). - **SSH backend:** reuse the existing gssh server. `gssh-mcp` is a thin JWT-authenticated HTTP+WS client; no local SSH-cert work in sherlock. See [docs/gssh-integration.md](docs/gssh-integration.md). - **Wrapper extensibility:** agent-agnostic. Each supported agent CLI (Copilot, Claude Code, Aider, …) is a single Go file under `internal/agent/` that registers itself with the package via `init()`. `sherlock copilot` and `sherlock claude` are sugar for `sherlock run copilot` / `sherlock run claude`. Adding a new agent is a small code change, by design — per-CLI quirks (auth subcommands, MCP config schema, env-var quirks) need a real code home. The TOML-overlay design from the Phase 0 plan was tried in Phase 1 and dropped. See [docs/agents.md](docs/agents.md). - **Docs convention:** `README.md` is TOC only; every topic lives as its own file under `docs/`. New concerns get new files, never appended sections. -- **Token persistence (Phase 1):** OS keyring via `zalando/go-keyring`. Strict pre-flight (`keyring.Probe()`) at startup of both `sherlock` and `sherlock-broker`; fail fast with exit code 3 and a per-OS hint if Secret Service / Keychain / Credential Manager is unavailable. See [docs/storage.md](docs/storage.md). -- **RPC framing (Phase 1):** JSON-over-newline on the UDS at `$XDG_RUNTIME_DIR/sherlock.sock`. One JSON object per line, both directions. Debuggable with `socat`/`nc`. See [docs/rpc.md](docs/rpc.md). -- **Broker lifecycle (Phase 1):** forked child process. The wrapper CLI dials the socket; if no one is listening it `fork+exec`s `sherlock-broker daemon` with `setsid`, polls the socket, then proceeds. Per-user PID-file flock prevents double-start; broker auto-exits after `SHERLOCK_BROKER_IDLE` (default 1h). No systemd in Phase 1. -- **Loopback redirect port:** `127.0.0.1:6990` (unassigned per IANA). The actual Authentik OAuth2 provider config is deferred — the broker ships with empty defaults and `login_start` returns `not_configured` until `SHERLOCK_AUTHENTIK_ISSUER` / `SHERLOCK_AUTHENTIK_CLIENT_ID` are set. End-to-end flow is exercised by an integration test against a stub Authentik (`internal/authn/flow_test.go`). +- **Token persistence:** OS keyring via `zalando/go-keyring`. Strict pre-flight (`keyring.Probe()`) at startup of every CLI invocation; fail fast with exit code 3 and a per-OS hint if Secret Service / Keychain / Credential Manager is unavailable. See [docs/storage.md](docs/storage.md). +- **Loopback redirect port:** `127.0.0.1:6990` (unassigned per IANA). Bound only for the duration of `sherlock login`. The actual Authentik OAuth2 provider config is deferred — sherlock ships with empty defaults and `sherlock login` returns `not_configured` until `SHERLOCK_AUTHENTIK_ISSUER` / `SHERLOCK_AUTHENTIK_CLIENT_ID` are set. End-to-end flow is exercised by an integration test against a stub Authentik (`internal/authn/login_test.go`). +- **No daemon (reverses Phase 1 #9 + #10):** the Phase 1 design had a `sherlock-broker` daemon with UDS RPC (`docs/rpc.md`), JSON-over-newline framing, fork+setsid lifecycle, PID-file flock, and idle-timeout watcher. It was scrapped in the post-Phase-1 refactor — see [docs/architecture.md#why-there-is-no-daemon](docs/architecture.md#why-there-is-no-daemon). Multi-process refresh races are serialised via an exclusive `flock()` on `$XDG_RUNTIME_DIR/sherlock.refresh.lock` in `internal/authn/refresh.go`. Net: -800 LoC, no `internal/{broker,socket,rpc}/` packages, no `cmd/sherlock-broker/` binary. ## Still open Tracked here and revisited at the top of the relevant phase. 1. **Phase 2 — Authentik provider creation:** stand up the actual `sherlock` OAuth2 provider in Authentik and document the env vars in `docs/storage.md` (carry-over from the Phase 1 deferral above). -2. **Phase 3 — gssh completion semantics:** keep the client-side `__SHERLOCK_DONE__` sentinel, or land a server-side one-shot run-and-close mode in gssh. +2. **Phase 2 — own-oauth UX:** confirm `sherlock auth ` is the right surface for non-Authentik services like GitHub.com (vs. an interactive `sherlock-mcp.oauth.consent` flow). The no-daemon refactor pushed this off — without a long-lived listener, an explicit command is the natural way to bind the loopback port. +3. **Phase 3 — gssh completion semantics:** keep the client-side `__SHERLOCK_DONE__` sentinel, or land a server-side one-shot run-and-close mode in gssh.