diff --git a/CHANGELOG.md b/CHANGELOG.md index 3376cc783..9d95bcbfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and - AI assistants can read vault metadata through three MCP tools: `listEntries`, `expiryReport` and `rotationStatus`. The tools return names, addresses, types, folders and dates, never a password, login or other secret value. They act only for the signed-in user and change nothing. Every call is written to the audit log with the tool name and the number of results. The tools appear only when OpenRegister is installed. (hermiq-ai-tooling) +### Fixed + +- `keepiq ssh-agent` works as a systemd user service. Under systemd the agent moved to the background and left the unit, which systemd then stopped, so the agent never kept running. The new `--foreground` option keeps it in place, and the unit in `cli/README.md` now uses it. + ### Removed - The `vault_admin` group no longer counts as the People and offboarding admin area, and the General area no longer shows a notice about it. A member of that group who holds no delegation is now refused the admin handover and team offboarding. To keep those actions for them, delegate the People and offboarding area to their group on Nextcloud's administration privileges page before you upgrade. (admin-scoped-roles, #1043) diff --git a/cli/README.md b/cli/README.md index 3698b83ad..ab5ebecb3 100644 --- a/cli/README.md +++ b/cli/README.md @@ -103,6 +103,10 @@ Options: (60 by default, 0 turns it off). - `--confirm`: ask before each signature through the program in `SSH_ASKPASS`. Without `SSH_ASKPASS` the agent does not start. +- `--foreground`: never move to the background, even when the output is not a + terminal. A service manager needs this: systemd sends the output to its + journal, and without `--foreground` the agent would leave the unit, which + systemd then stops. - `--locked`: start without keys, for a service manager. Unlock it with `ssh-add -X` and your master password. Lock it again with `ssh-add -x`: it asks for a lock password, which the agent ignores, because unlocking @@ -124,7 +128,7 @@ systemd user unit, `~/.config/systemd/user/keepiq-agent.service`: Description=Keepiq SSH agent [Service] -ExecStart=%h/.local/bin/keepiq ssh-agent --locked --socket %t/keepiq/agent.sock +ExecStart=%h/.local/bin/keepiq ssh-agent --locked --foreground --socket %t/keepiq/agent.sock Restart=on-failure [Install] diff --git a/cli/sshagent_cmd.go b/cli/sshagent_cmd.go index 1b53b5e87..ba5c54ebd 100644 --- a/cli/sshagent_cmd.go +++ b/cli/sshagent_cmd.go @@ -58,7 +58,7 @@ func cmdSSHAgent(args []string) error { return err } detached := os.Getenv(detachedEnv) == "1" - if !detached && !isTerminal(os.Stdout) { + if shouldDetach(flags, detached, isTerminal(os.Stdout)) { return startDetached(args, flags) } var confirm sshagent.Confirmer @@ -164,6 +164,14 @@ func startDetached(args []string, flags agentFlags) error { return cmd.Process.Release() } +// shouldDetach is true only for `eval "$(keepiq ssh-agent)"`: stdout is a +// pipe, this is not already the background copy, and --foreground is not set. +// A service manager also gives a stdout that is no terminal (systemd's +// journal), and there the agent must stay the unit's main process. +func shouldDetach(flags agentFlags, detached bool, stdoutIsTerminal bool) bool { + return !flags.Foreground && !detached && !stdoutIsTerminal +} + // isTerminal reports whether f is a character device, so a terminal and not a // pipe such as the one `eval "$(...)"` reads. func isTerminal(f *os.File) bool { diff --git a/cli/sshagent_cmd_test.go b/cli/sshagent_cmd_test.go index bc8d3eca2..7a84cfe1b 100644 --- a/cli/sshagent_cmd_test.go +++ b/cli/sshagent_cmd_test.go @@ -136,3 +136,31 @@ func TestVaultUnlockerReadsOnlyCiphertext(t *testing.T) { t.Fatalf("requests: %+v", requests) } } + +// A service manager such as systemd gives the agent a stdout that is not a +// terminal. Without --foreground the agent would detach, the unit's main +// process would exit, and systemd would stop the unit and kill the agent. +func TestTheAgentDetachesOnlyForEvalNotUnderAServiceManager(t *testing.T) { + cases := []struct { + name string + flags agentFlags + isDetached bool + terminal bool + want bool + }{ + {"eval pipe, plain", agentFlags{}, false, false, true}, + {"terminal", agentFlags{}, false, true, false}, + {"already the background copy", agentFlags{}, true, false, false}, + {"systemd: journal stdout with --foreground", agentFlags{Foreground: true}, false, false, false}, + {"systemd: --locked --foreground", agentFlags{Locked: true, Foreground: true}, false, false, false}, + } + for _, c := range cases { + if got := shouldDetach(c.flags, c.isDetached, c.terminal); got != c.want { + t.Errorf("%s: shouldDetach = %v, want %v", c.name, got, c.want) + } + } + f, err := parseAgentFlags([]string{"--locked", "--foreground"}) + if err != nil || !f.Foreground || !f.Locked { + t.Fatalf("parseAgentFlags(--locked --foreground) = %+v, %v", f, err) + } +} diff --git a/cli/sshagent_flags.go b/cli/sshagent_flags.go index 397f6a061..7d43c5b00 100644 --- a/cli/sshagent_flags.go +++ b/cli/sshagent_flags.go @@ -15,6 +15,9 @@ type agentFlags struct { Idle int // minutes; 0 disables Folder string Locked bool + // Foreground keeps the agent in this process even when stdout is not a + // terminal, for a service manager such as systemd. + Foreground bool } // parseAgentFlags reads the ssh-agent flags. Idle defaults to 60 minutes. @@ -35,6 +38,8 @@ func parseAgentFlags(args []string) (agentFlags, error) { f.Confirm = true case "--locked": f.Locked = true + case "--foreground": + f.Foreground = true case "--socket": f.Socket, err = value() case "--folder":