7fad59b fix(cli): kill the whole token.cmd process tree and simplify the client setup · ermite

M go.mod +1 -1


@@ -26,6 +26,7 @@ require (
26	26		github.com/yuin/goldmark v1.8.6
27	27		golang.org/x/crypto v0.57.0
28	28		golang.org/x/sys v0.49.0
	29	+	golang.org/x/term v0.46.0
29	30		google.golang.org/genproto/googleapis/rpc v0.0.0-20260904194346-d0f1323225a4
30	31		google.golang.org/protobuf v1.36.12
31	32		)

@@ -152,7 +153,6 @@ require (
152	153		golang.org/x/mod v0.41.0 // indirect
153	154		golang.org/x/net v0.59.0 // indirect
154	155		golang.org/x/sync v0.23.0 // indirect
155		-	golang.org/x/term v0.46.0 // indirect
156	156		golang.org/x/text v0.42.0 // indirect
157	157		golang.org/x/tools v0.51.0 // indirect
158	158		google.golang.org/genproto/googleapis/api v0.0.0-20260904194346-d0f1323225a4 // indirect

 	github.com/yuin/goldmark v1.8.6
 	golang.org/x/crypto v0.57.0
 	golang.org/x/sys v0.49.0
+	golang.org/x/term v0.46.0
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20260904194346-d0f1323225a4
 	google.golang.org/protobuf v1.36.12
 )
 	golang.org/x/mod v0.41.0 // indirect
 	golang.org/x/net v0.59.0 // indirect
 	golang.org/x/sync v0.23.0 // indirect
-	golang.org/x/term v0.46.0 // indirect
 	golang.org/x/text v0.42.0 // indirect
 	golang.org/x/tools v0.51.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20260904194346-d0f1323225a4 // indirect

M internal/adapters/client/config.go +1 -1


@@ -58,7 +58,7 @@ func NewConfig(rawURL, token string) (Config, error) {
58	58		}
59	59		base, err := ParseURL(raw)
60	60		if err != nil {
61		-	return Config{}, fmt.Errorf("invalid server URL (%s, or `ermite config set url <url>`): %w", EnvURL, err)
	61	+	return Config{}, fmt.Errorf("invalid server URL: %w", err)
62	62		}
63	63		return Config{URL: base.String(), Token: strings.TrimSpace(token)}, nil
64	64		}


@@ -58,7 +58,7 @@ func NewConfig(rawURL, token string) (Config, error) {
58		}	58		}
59		base, err := ParseURL(raw)	59		base, err := ParseURL(raw)
60		if err != nil {	60		if err != nil {
61	-	return Config{}, fmt.Errorf("invalid server URL (%s, or `ermite config set url <url>`): %w", EnvURL, err)	61	+	return Config{}, fmt.Errorf("invalid server URL: %w", err)
62		}	62		}
63		return Config{URL: base.String(), Token: strings.TrimSpace(token)}, nil	63		return Config{URL: base.String(), Token: strings.TrimSpace(token)}, nil
64		}	64		}

M internal/adapters/client/config_test.go +7 -2


@@ -18,6 +18,13 @@ func TestNewConfig(t *testing.T) {
18	18		assert.Equal(t, "ermite_abc", cfg.Token)
19	19		}
20	20
	21	+	func TestNewConfigUnsetTellsHowToFixIt(t *testing.T) {
	22	+	_, err := client.NewConfig(" ", "")
	23	+	require.Error(t, err)
	24	+	assert.Contains(t, err.Error(), "ERMITE_URL")
	25	+	assert.Contains(t, err.Error(), "ermite config set url")
	26	+	}
	27	+
21	28		func TestNewConfigRejects(t *testing.T) {
22	29		tests := []struct {
23	30		name, url, want string

@@ -36,8 +43,6 @@ func TestNewConfigRejects(t *testing.T) {
36	43		_, err := client.NewConfig(tt.url, "")
37	44		require.Error(t, err)
38	45		assert.Contains(t, err.Error(), tt.want)
39		-	assert.Contains(t, err.Error(), "ermite config set url", "the message tells how to fix it")
40		-	assert.Contains(t, err.Error(), "ERMITE_URL")
41	46		assert.NotContains(t, err.Error(), "s3cret", "credentials must never be echoed")
42	47		})
43	48		}

 	assert.Equal(t, "ermite_abc", cfg.Token)
 }
+func TestNewConfigUnsetTellsHowToFixIt(t *testing.T) {
+	_, err := client.NewConfig("  ", "")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "ERMITE_URL")
+	assert.Contains(t, err.Error(), "ermite config set url")
+}
 func TestNewConfigRejects(t *testing.T) {
 	tests := []struct {
 		name, url, want string
 			_, err := client.NewConfig(tt.url, "")
 			require.Error(t, err)
 			assert.Contains(t, err.Error(), tt.want)
-			assert.Contains(t, err.Error(), "ermite config set url", "the message tells how to fix it")
-			assert.Contains(t, err.Error(), "ERMITE_URL")
 			assert.NotContains(t, err.Error(), "s3cret", "credentials must never be echoed")
 		})
 	}

M internal/app/cli.go +22 -24


@@ -36,10 +36,8 @@ type cliEnv struct {
36	36		// (ERMITE_CONFIG, else ~/.config/ermite/config).
37	37		configPath func() (string, error)
38	38		// runTokenCmd runs the token.cmd command line and returns its raw standard
39		-	// output; nil means running it with the system shell. Tests replace it.
	39	+	// output. Tests replace it.
40	40		runTokenCmd func(ctx context.Context, cmd string) (string, error)
41		-	// tokenTimeout bounds the default runTokenCmd; zero means 60 s.
42		-	tokenTimeout time.Duration
43	41
44	42		// The config file is read at most once and token.cmd run at most once per
45	43		// process, and only when a command needs them (see cli_configfile.go).

@@ -54,7 +52,7 @@ type cliEnv struct {
54	52		}
55	53
56	54		func defaultCLIEnv() *cliEnv {
57		-	return &cliEnv{getenv: os.Getenv, now: time.Now, version: buildVersion()}
	55	+	return &cliEnv{getenv: os.Getenv, now: time.Now, version: buildVersion(), runTokenCmd: runTokenShell}
58	56		}
59	57
60	58		// userAgent identifies this CLI to the server.

@@ -75,43 +73,43 @@ func (e cliEnv) ctx(cmd cobra.Command) context.Context {
75	73		return ctx
76	74		}
77	75
78		-	// config builds the client configuration: ERMITE_URL and ERMITE_TOKEN win,
79		-	// then the config file (url, and the output of token.cmd for the token).
80		-	func (e cliEnv) config(cmd cobra.Command, warn bool) (client.Config, error) {
81		-	return e.buildConfig(cmd, warn, true)
82		-	}
83		-
84		-	// buildConfig is config; runTokenCmd false leaves token.cmd alone (ERMITE_TOKEN
85		-	// is still used), for the calls that need no authentication.
	76	+	// buildConfig builds the client configuration: ERMITE_URL and ERMITE_TOKEN
	77	+	// win, then the config file (url, and the output of token.cmd for the token).
	78	+	// runTokenCmd false leaves token.cmd alone (ERMITE_TOKEN is still used), for
	79	+	// the calls that need no authentication.
86	80		func (e cliEnv) buildConfig(cmd cobra.Command, warn, runTokenCmd bool) (client.Config, error) {
87	81		rawURL, err := e.resolveURL(cmd, warn)
88	82		if err != nil {
89	83		return client.Config{}, err
90	84		}
91		-	// A missing or invalid URL is reported before token.cmd gets to prompt.
92		-	if _, err := client.NewConfig(rawURL, ""); err != nil {
93		-	return client.Config{}, err
	85	+	if strings.TrimSpace(rawURL) != "" {
	86	+	if _, err := client.ParseURL(strings.TrimSpace(rawURL)); err != nil {
	87	+	return client.Config{}, fmt.Errorf("%s: %w", e.urlSource(), err)
	88	+	}
94	89		}
95		-	token, err := e.resolveToken(cmd, warn, runTokenCmd)
	90	+	// A missing or invalid URL is reported before token.cmd gets to prompt.
	91	+	cfg, err := client.NewConfig(rawURL, "")
96	92		if err != nil {
97	93		return client.Config{}, err
98	94		}
99		-	cfg, err := client.NewConfig(rawURL, token)
100		-	if err != nil {
101		-	return cfg, err
	95	+	cfg.Token = strings.TrimSpace(e.getenv(client.EnvToken))
	96	+	if cfg.Token == "" && runTokenCmd {
	97	+	if cfg.Token, err = e.tokenFromCmd(cmd, warn); err != nil {
	98	+	return client.Config{}, err
	99	+	}
102	100		}
103	101		cfg.UserAgent = e.userAgent()
104	102		return cfg, nil
105	103		}
106	104
107		-	// client builds the API client. Commands warn when the token would travel in
108		-	// the clear; completion stays silent.
109		-	func (e cliEnv) client(cmd cobra.Command, warn bool) (*client.Client, error) {
110		-	cfg, err := e.config(cmd, warn)
	105	+	// client builds the API client. It warns when the token would travel in the
	106	+	// clear.
	107	+	func (e cliEnv) client(cmd cobra.Command) (*client.Client, error) {
	108	+	cfg, err := e.buildConfig(cmd, true, true)
111	109		if err != nil {
112	110		return nil, err
113	111		}
114		-	return newClient(cmd, cfg, warn)
	112	+	return newClient(cmd, cfg, true)
115	113		}
116	114
117	115		// clientNoTokenCmd is client without running token.cmd (ERMITE_TOKEN is still

 	// (ERMITE_CONFIG, else ~/.config/ermite/config).
 	configPath func() (string, error)
 	// runTokenCmd runs the token.cmd command line and returns its raw standard
-	// output; nil means running it with the system shell. Tests replace it.
+	// output. Tests replace it.
 	runTokenCmd func(ctx context.Context, cmd string) (string, error)
-	// tokenTimeout bounds the default runTokenCmd; zero means 60 s.
-	tokenTimeout time.Duration
 	// The config file is read at most once and token.cmd run at most once per
 	// process, and only when a command needs them (see cli_configfile.go).
 }
 func defaultCLIEnv() *cliEnv {
-	return &cliEnv{getenv: os.Getenv, now: time.Now, version: buildVersion()}
+	return &cliEnv{getenv: os.Getenv, now: time.Now, version: buildVersion(), runTokenCmd: runTokenShell}
 }
 // userAgent identifies this CLI to the server.
 	return ctx
 }
-// config builds the client configuration: ERMITE_URL and ERMITE_TOKEN win,
+// buildConfig builds the client configuration: ERMITE_URL and ERMITE_TOKEN
-// then the config file (url, and the output of token.cmd for the token).
+// win, then the config file (url, and the output of token.cmd for the token).
-func (e *cliEnv) config(cmd *cobra.Command, warn bool) (client.Config, error) {
+// runTokenCmd false leaves token.cmd alone (ERMITE_TOKEN is still used), for
-	return e.buildConfig(cmd, warn, true)
+// the calls that need no authentication.
-}
-// buildConfig is config; runTokenCmd false leaves token.cmd alone (ERMITE_TOKEN
-// is still used), for the calls that need no authentication.
 func (e *cliEnv) buildConfig(cmd *cobra.Command, warn, runTokenCmd bool) (client.Config, error) {
 	rawURL, err := e.resolveURL(cmd, warn)
 	if err != nil {
 		return client.Config{}, err
 	}
-	// A missing or invalid URL is reported before token.cmd gets to prompt.
+	if strings.TrimSpace(rawURL) != "" {
-	if _, err := client.NewConfig(rawURL, ""); err != nil {
+		if _, err := client.ParseURL(strings.TrimSpace(rawURL)); err != nil {
-		return client.Config{}, err
+			return client.Config{}, fmt.Errorf("%s: %w", e.urlSource(), err)
+		}
 	}
-	token, err := e.resolveToken(cmd, warn, runTokenCmd)
+	// A missing or invalid URL is reported before token.cmd gets to prompt.
+	cfg, err := client.NewConfig(rawURL, "")
 	if err != nil {
 		return client.Config{}, err
 	}
-	cfg, err := client.NewConfig(rawURL, token)
+	cfg.Token = strings.TrimSpace(e.getenv(client.EnvToken))
-	if err != nil {
+	if cfg.Token == "" && runTokenCmd {
-		return cfg, err
+		if cfg.Token, err = e.tokenFromCmd(cmd, warn); err != nil {
+			return client.Config{}, err
+		}
 	}
 	cfg.UserAgent = e.userAgent()
 	return cfg, nil
 }
-// client builds the API client. Commands warn when the token would travel in
+// client builds the API client. It warns when the token would travel in the
-// the clear; completion stays silent.
+// clear.
-func (e *cliEnv) client(cmd *cobra.Command, warn bool) (*client.Client, error) {
+func (e *cliEnv) client(cmd *cobra.Command) (*client.Client, error) {
-	cfg, err := e.config(cmd, warn)
+	cfg, err := e.buildConfig(cmd, true, true)
 	if err != nil {
 		return nil, err
 	}
-	return newClient(cmd, cfg, warn)
+	return newClient(cmd, cfg, true)
 }
 // clientNoTokenCmd is client without running token.cmd (ERMITE_TOKEN is still

M internal/app/cli_configfile.go +36 -49


@@ -11,11 +11,14 @@ import (
11	11		"io/fs"
12	12		"os"
13	13		"os/exec"
	14	+	"os/signal"
14	15		"strings"
	16	+	"syscall"
15	17		"time"
16	18
17	19		"github.com/bnema/kvconf"
18	20		"github.com/spf13/cobra"
	21	+	"golang.org/x/term"
19	22
20	23		"bnema.dev/ermite/internal/adapters/client"
21	24		)

@@ -107,7 +110,7 @@ func loadConfigFile(path string) (f *cliFile, warnings []kvconf.Warning, trust,
107	110		if !info.Mode().IsRegular() {
108	111		return nil, nil, nil, fmt.Errorf("%s: %w", path, kvconf.ErrNotRegular)
109	112		}
110		-	trust = checkPrivate(path, info)
	113	+	trust = checkPrivate(path, info, os.Geteuid())
111	114		data, err := io.ReadAll(io.LimitReader(h, kvconf.DefaultMaxBytes+1))
112	115		if err != nil {
113	116		return nil, nil, nil, err

@@ -123,6 +126,14 @@ func loadConfigFile(path string) (f *cliFile, warnings []kvconf.Warning, trust,
123	126		return f, warnings, trust, nil
124	127		}
125	128
	129	+	// urlSource says where the server address in use comes from, for error messages.
	130	+	func (e *cliEnv) urlSource() string {
	131	+	if strings.TrimSpace(e.getenv(client.EnvURL)) != "" {
	132	+	return client.EnvURL
	133	+	}
	134	+	return fmt.Sprintf("url in %s (fix it with `ermite config set url <url>`)", e.filePath)
	135	+	}
	136	+
126	137		// resolveURL is ERMITE_URL if set, else the url of the config file, else "".
127	138		func (e cliEnv) resolveURL(cmd cobra.Command, warn bool) (string, error) {
128	139		if v := strings.TrimSpace(e.getenv(client.EnvURL)); v != "" {

@@ -135,18 +146,6 @@ func (e cliEnv) resolveURL(cmd cobra.Command, warn bool) (string, error) {
135	146		return strings.TrimSpace(f.URL), nil
136	147		}
137	148
138		-	// resolveToken is ERMITE_TOKEN if set, else (when runCmd is set) the output
139		-	// of token.cmd, else "" (anonymous). token.cmd runs at most once per process.
140		-	func (e cliEnv) resolveToken(cmd cobra.Command, warn, runCmd bool) (string, error) {
141		-	if v := strings.TrimSpace(e.getenv(client.EnvToken)); v != "" {
142		-	return v, nil
143		-	}
144		-	if !runCmd {
145		-	return "", nil
146		-	}
147		-	return e.tokenFromCmd(cmd, warn)
148		-	}
149		-
150	149		// tokenFromCmd is the output of token.cmd, or "" when the file sets none or
151	150		// when ERMITE_URL points somewhere else than the url of the file: the token
152	151		// the command fetches belongs to the server of the file, and must not be sent

@@ -193,11 +192,7 @@ func (e *cliEnv) fetchToken(ctx context.Context, line string) (string, error) {
193	192		if e.fileTrust != nil {
194	193		return "", e.fileTrust
195	194		}
196		-	run := e.runTokenCmd
197		-	if run == nil {
198		-	run = e.shell
199		-	}
200		-	out, err := run(ctx, line)
	195	+	out, err := e.runTokenCmd(ctx, line)
201	196		if err != nil {
202	197		return "", fmt.Errorf("token.cmd failed: %w", err)
203	198		}

@@ -211,60 +206,52 @@ func (e *cliEnv) fetchToken(ctx context.Context, line string) (string, error) {
211	206		return out, nil
212	207		}
213	208
214		-	// shell runs a token.cmd command line, with the timeout of the environment.
215		-	func (e *cliEnv) shell(ctx context.Context, line string) (string, error) {
216		-	timeout := e.tokenTimeout
217		-	if timeout <= 0 {
218		-	timeout = tokenCmdTimeout
219		-	}
220		-	return runShell(ctx, line, timeout)
	209	+	// runTokenShell is the default runTokenCmd: the system shell, for at most
	210	+	// tokenCmdTimeout.
	211	+	func runTokenShell(ctx context.Context, line string) (string, error) {
	212	+	return runShell(ctx, line, tokenCmdTimeout)
221	213		}
222	214
223	215		// runShell runs line with the system shell. Standard error is the terminal's,
224	216		// and so is standard input when it is a terminal, so a pinentry or a biometric
225	217		// prompt works; a script or an agent gets no input. Standard output is
226	218		// captured, up to tokenCmdMaxOutput bytes. The environment is the process's.
	219	+	// An interrupt or SIGTERM sent to ermite, or the end of ctx, stops the command
	220	+	// like a timeout does (see shellCommand for what is killed).
227	221		func runShell(ctx context.Context, line string, timeout time.Duration) (string, error) {
228		-	parent := ctx
229		-	ctx, cancel := context.WithTimeout(parent, timeout)
	222	+	sigCtx, stop := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM)
	223	+	defer stop()
	224	+	runCtx, cancel := context.WithTimeout(sigCtx, timeout)
230	225		defer cancel()
231		-	c := shellCommand(ctx, line)
	226	+
	227	+	foreground := term.IsTerminal(int(os.Stdin.Fd()))
	228	+	c := shellCommand(runCtx, line, foreground)
232	229		out := &cappedBuffer{max: tokenCmdMaxOutput}
233	230		c.Stdout, c.Stderr = out, os.Stderr
234		-	if stdinIsTerminal() {
	231	+	if foreground {
235	232		c.Stdin = os.Stdin
236	233		} // else nil: the command reads os.DevNull
237	234		// Do not wait forever for a background process that kept the pipe open.
238	235		c.WaitDelay = 2 * time.Second
239	236		err := c.Run()
240		-	switch {
241		-	case parent.Err() != nil:
242		-	return "", errors.New("interrupted")
243		-	case errors.Is(ctx.Err(), context.DeadlineExceeded):
244		-	return "", fmt.Errorf("timed out after %s", timeout)
245		-	case err != nil:
246		-	var exit *exec.ExitError
247		-	if errors.As(err, &exit) {
	237	+	if err != nil {
	238	+	switch {
	239	+	case sigCtx.Err() != nil:
	240	+	return "", errors.New("interrupted")
	241	+	case errors.Is(runCtx.Err(), context.DeadlineExceeded):
	242	+	return "", fmt.Errorf("timed out after %s", timeout)
	243	+	}
	244	+	if exit := (*exec.ExitError)(nil); errors.As(err, &exit) {
248	245		return "", errors.New(exit.String())
249	246		}
250	247		return "", err
251		-	case out.over:
	248	+	}
	249	+	if out.over {
252	250		return "", fmt.Errorf("printed more than %d bytes", tokenCmdMaxOutput)
253	251		}
254	252		return out.buf.String(), nil
255	253		}
256	254
257		-	// stdinIsTerminal reports whether standard input is a terminal: a character
258		-	// device other than the null device.
259		-	func stdinIsTerminal() bool {
260		-	fi, err := os.Stdin.Stat()
261		-	if err != nil \|\| fi.Mode()&os.ModeCharDevice == 0 {
262		-	return false
263		-	}
264		-	null, err := os.Stat(os.DevNull)
265		-	return err != nil \|\| !os.SameFile(fi, null)
266		-	}
267		-
268	255		// cappedBuffer keeps the first max bytes written and notes an overflow.
269	256		type cappedBuffer struct {
270	257		buf bytes.Buffer


@@ -11,11 +11,14 @@ import (
11		"io/fs"	11		"io/fs"
12		"os"	12		"os"
13		"os/exec"	13		"os/exec"
			14	+	"os/signal"
14		"strings"	15		"strings"
			16	+	"syscall"
15		"time"	17		"time"
16			18
17		"github.com/bnema/kvconf"	19		"github.com/bnema/kvconf"
18		"github.com/spf13/cobra"	20		"github.com/spf13/cobra"
			21	+	"golang.org/x/term"
19			22
20		"bnema.dev/ermite/internal/adapters/client"	23		"bnema.dev/ermite/internal/adapters/client"
21		)	24		)

@@ -107,7 +110,7 @@ func loadConfigFile(path string) (f *cliFile, warnings []kvconf.Warning, trust,
107		if !info.Mode().IsRegular() {	110		if !info.Mode().IsRegular() {
108		return nil, nil, nil, fmt.Errorf("%s: %w", path, kvconf.ErrNotRegular)	111		return nil, nil, nil, fmt.Errorf("%s: %w", path, kvconf.ErrNotRegular)
109		}	112		}
110	-	trust = checkPrivate(path, info)	113	+	trust = checkPrivate(path, info, os.Geteuid())
111		data, err := io.ReadAll(io.LimitReader(h, kvconf.DefaultMaxBytes+1))	114		data, err := io.ReadAll(io.LimitReader(h, kvconf.DefaultMaxBytes+1))
112		if err != nil {	115		if err != nil {
113		return nil, nil, nil, err	116		return nil, nil, nil, err

@@ -123,6 +126,14 @@ func loadConfigFile(path string) (f *cliFile, warnings []kvconf.Warning, trust,
123		return f, warnings, trust, nil	126		return f, warnings, trust, nil
124		}	127		}
125			128
			129	+	// urlSource says where the server address in use comes from, for error messages.
			130	+	func (e *cliEnv) urlSource() string {
			131	+	if strings.TrimSpace(e.getenv(client.EnvURL)) != "" {
			132	+	return client.EnvURL
			133	+	}
			134	+	return fmt.Sprintf("url in %s (fix it with `ermite config set url <url>`)", e.filePath)
			135	+	}
			136	+
126		// resolveURL is ERMITE_URL if set, else the url of the config file, else "".	137		// resolveURL is ERMITE_URL if set, else the url of the config file, else "".
127		func (e cliEnv) resolveURL(cmd cobra.Command, warn bool) (string, error) {	138		func (e cliEnv) resolveURL(cmd cobra.Command, warn bool) (string, error) {
128		if v := strings.TrimSpace(e.getenv(client.EnvURL)); v != "" {	139		if v := strings.TrimSpace(e.getenv(client.EnvURL)); v != "" {

@@ -135,18 +146,6 @@ func (e cliEnv) resolveURL(cmd cobra.Command, warn bool) (string, error) {
135		return strings.TrimSpace(f.URL), nil	146		return strings.TrimSpace(f.URL), nil
136		}	147		}
137			148
138	-	// resolveToken is ERMITE_TOKEN if set, else (when runCmd is set) the output
139	-	// of token.cmd, else "" (anonymous). token.cmd runs at most once per process.
140	-	func (e cliEnv) resolveToken(cmd cobra.Command, warn, runCmd bool) (string, error) {
141	-	if v := strings.TrimSpace(e.getenv(client.EnvToken)); v != "" {
142	-	return v, nil
143	-	}
144	-	if !runCmd {
145	-	return "", nil
146	-	}
147	-	return e.tokenFromCmd(cmd, warn)
148	-	}
149	-
150		// tokenFromCmd is the output of token.cmd, or "" when the file sets none or	149		// tokenFromCmd is the output of token.cmd, or "" when the file sets none or
151		// when ERMITE_URL points somewhere else than the url of the file: the token	150		// when ERMITE_URL points somewhere else than the url of the file: the token
152		// the command fetches belongs to the server of the file, and must not be sent	151		// the command fetches belongs to the server of the file, and must not be sent

@@ -193,11 +192,7 @@ func (e *cliEnv) fetchToken(ctx context.Context, line string) (string, error) {
193		if e.fileTrust != nil {	192		if e.fileTrust != nil {
194		return "", e.fileTrust	193		return "", e.fileTrust
195		}	194		}
196	-	run := e.runTokenCmd	195	+	out, err := e.runTokenCmd(ctx, line)
197	-	if run == nil {
198	-	run = e.shell
199	-	}
200	-	out, err := run(ctx, line)
201		if err != nil {	196		if err != nil {
202		return "", fmt.Errorf("token.cmd failed: %w", err)	197		return "", fmt.Errorf("token.cmd failed: %w", err)
203		}	198		}

@@ -211,60 +206,52 @@ func (e *cliEnv) fetchToken(ctx context.Context, line string) (string, error) {
211		return out, nil	206		return out, nil
212		}	207		}
213			208
214	-	// shell runs a token.cmd command line, with the timeout of the environment.	209	+	// runTokenShell is the default runTokenCmd: the system shell, for at most
215	-	func (e *cliEnv) shell(ctx context.Context, line string) (string, error) {	210	+	// tokenCmdTimeout.
216	-	timeout := e.tokenTimeout	211	+	func runTokenShell(ctx context.Context, line string) (string, error) {
217	-	if timeout <= 0 {	212	+	return runShell(ctx, line, tokenCmdTimeout)
218	-	timeout = tokenCmdTimeout
219	-	}
220	-	return runShell(ctx, line, timeout)
221		}	213		}
222			214
223		// runShell runs line with the system shell. Standard error is the terminal's,	215		// runShell runs line with the system shell. Standard error is the terminal's,
224		// and so is standard input when it is a terminal, so a pinentry or a biometric	216		// and so is standard input when it is a terminal, so a pinentry or a biometric
225		// prompt works; a script or an agent gets no input. Standard output is	217		// prompt works; a script or an agent gets no input. Standard output is
226		// captured, up to tokenCmdMaxOutput bytes. The environment is the process's.	218		// captured, up to tokenCmdMaxOutput bytes. The environment is the process's.
			219	+	// An interrupt or SIGTERM sent to ermite, or the end of ctx, stops the command
			220	+	// like a timeout does (see shellCommand for what is killed).
227		func runShell(ctx context.Context, line string, timeout time.Duration) (string, error) {	221		func runShell(ctx context.Context, line string, timeout time.Duration) (string, error) {
228	-	parent := ctx	222	+	sigCtx, stop := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM)
229	-	ctx, cancel := context.WithTimeout(parent, timeout)	223	+	defer stop()
			224	+	runCtx, cancel := context.WithTimeout(sigCtx, timeout)
230		defer cancel()	225		defer cancel()
231	-	c := shellCommand(ctx, line)	226	+
			227	+	foreground := term.IsTerminal(int(os.Stdin.Fd()))
			228	+	c := shellCommand(runCtx, line, foreground)
232		out := &cappedBuffer{max: tokenCmdMaxOutput}	229		out := &cappedBuffer{max: tokenCmdMaxOutput}
233		c.Stdout, c.Stderr = out, os.Stderr	230		c.Stdout, c.Stderr = out, os.Stderr
234	-	if stdinIsTerminal() {	231	+	if foreground {
235		c.Stdin = os.Stdin	232		c.Stdin = os.Stdin
236		} // else nil: the command reads os.DevNull	233		} // else nil: the command reads os.DevNull
237		// Do not wait forever for a background process that kept the pipe open.	234		// Do not wait forever for a background process that kept the pipe open.
238		c.WaitDelay = 2 * time.Second	235		c.WaitDelay = 2 * time.Second
239		err := c.Run()	236		err := c.Run()
240	-	switch {	237	+	if err != nil {
241	-	case parent.Err() != nil:	238	+	switch {
242	-	return "", errors.New("interrupted")	239	+	case sigCtx.Err() != nil:
243	-	case errors.Is(ctx.Err(), context.DeadlineExceeded):	240	+	return "", errors.New("interrupted")
244	-	return "", fmt.Errorf("timed out after %s", timeout)	241	+	case errors.Is(runCtx.Err(), context.DeadlineExceeded):
245	-	case err != nil:	242	+	return "", fmt.Errorf("timed out after %s", timeout)
246	-	var exit *exec.ExitError	243	+	}
247	-	if errors.As(err, &exit) {	244	+	if exit := (*exec.ExitError)(nil); errors.As(err, &exit) {
248		return "", errors.New(exit.String())	245		return "", errors.New(exit.String())
249		}	246		}
250		return "", err	247		return "", err
251	-	case out.over:	248	+	}
			249	+	if out.over {
252		return "", fmt.Errorf("printed more than %d bytes", tokenCmdMaxOutput)	250		return "", fmt.Errorf("printed more than %d bytes", tokenCmdMaxOutput)
253		}	251		}
254		return out.buf.String(), nil	252		return out.buf.String(), nil
255		}	253		}
256			254
257	-	// stdinIsTerminal reports whether standard input is a terminal: a character
258	-	// device other than the null device.
259	-	func stdinIsTerminal() bool {
260	-	fi, err := os.Stdin.Stat()
261	-	if err != nil \|\| fi.Mode()&os.ModeCharDevice == 0 {
262	-	return false
263	-	}
264	-	null, err := os.Stat(os.DevNull)
265	-	return err != nil \|\| !os.SameFile(fi, null)
266	-	}
267	-
268		// cappedBuffer keeps the first max bytes written and notes an overflow.	255		// cappedBuffer keeps the first max bytes written and notes an overflow.
269		type cappedBuffer struct {	256		type cappedBuffer struct {
270		buf bytes.Buffer	257		buf bytes.Buffer

M internal/app/cli_configfile_test.go +79 -65


@@ -14,6 +14,7 @@ import (
14	14
15	15		"github.com/stretchr/testify/assert"
16	16		"github.com/stretchr/testify/require"
	17	+	"golang.org/x/term"
17	18
18	19		"bnema.dev/ermite-api/gen/ermite"
19	20		)

@@ -88,30 +89,38 @@ func TestEnvironmentWinsOverConfigFile(t *testing.T) {
88	89
89	90		func TestEnvURLDifferentFromFileKeepsTokenCmdOut(t *testing.T) {
90	91		f := listFake()
91		-	env := testEnv(t, map[string]string{"ERMITE_URL": f.serve(t)})
92		-	writeConfigFile(t, env, "url = http://file.invalid\ntoken.cmd = get-token\n", 0o600)
93		-	ran := tokenCmdStub(env, "ermite_cmd", nil)
94		-
95		-	res := runCLIWith(t, env, "", "repo", "list").ok(t)
96		-	assert.Empty(t, *ran, "the token of the file's server must not go to another server")
97		-	assert.Empty(t, lastAuth(f))
98		-	assert.Equal(t, 1, strings.Count(res.stderr, "ermite: ERMITE_URL differs from the config file url: token.cmd not run (set ERMITE_TOKEN)\n"), res.stderr)
	92	+	tests := []struct {
	93	+	name string
	94	+	envURL string // "" means the fake server
	95	+	file string // the content of the config file
	96	+	wantErr bool
	97	+	}{
	98	+	{name: "other url in the file", file: "url = http://file.invalid\ntoken.cmd = get-token\n"},
	99	+	{name: "file without url", file: "token.cmd = get-token\n"},
	100	+	{name: "invalid ERMITE_URL", envURL: "nope", file: "url = http://file.invalid\ntoken.cmd = get-token\n", wantErr: true},
	101	+	}
	102	+	for _, tt := range tests {
	103	+	t.Run(tt.name, func(t *testing.T) {
	104	+	envURL := tt.envURL
	105	+	if envURL == "" {
	106	+	envURL = f.serve(t)
	107	+	}
	108	+	env := testEnv(t, map[string]string{"ERMITE_URL": envURL})
	109	+	writeConfigFile(t, env, tt.file, 0o600)
	110	+	ran := tokenCmdStub(env, "ermite_cmd", nil)
99	111
100		-	t.Run("file without url", func(t *testing.T) {
101		-	env := testEnv(t, map[string]string{"ERMITE_URL": f.serve(t)})
102		-	writeConfigFile(t, env, "token.cmd = get-token\n", 0o600)
103		-	ran := tokenCmdStub(env, "ermite_cmd", nil)
104		-	res := runCLIWith(t, env, "", "repo", "list").ok(t)
105		-	assert.Empty(t, *ran)
106		-	assert.Contains(t, res.stderr, "token.cmd not run")
107		-	})
108		-	t.Run("invalid ERMITE_URL", func(t *testing.T) {
109		-	env := testEnv(t, map[string]string{"ERMITE_URL": "nope"})
110		-	writeConfigFile(t, env, "url = http://file.invalid\ntoken.cmd = get-token\n", 0o600)
111		-	ran := tokenCmdStub(env, "ermite_cmd", nil)
112		-	require.Error(t, runCLIWith(t, env, "", "repo", "list").err)
113		-	assert.Empty(t, *ran)
114		-	})
	112	+	res := runCLIWith(t, env, "", "repo", "list")
	113	+	assert.Empty(t, *ran, "the token of the file's server must not go to another server")
	114	+	if tt.wantErr {
	115	+	require.Error(t, res.err)
	116	+	return
	117	+	}
	118	+	require.NoError(t, res.err)
	119	+	assert.Empty(t, lastAuth(f))
	120	+	assert.Equal(t, 1, strings.Count(res.stderr, "token.cmd not run"), res.stderr)
	121	+	assert.Contains(t, res.stderr, "ERMITE_TOKEN")
	122	+	})
	123	+	}
115	124		}
116	125
117	126		func TestEnvURLSameAsFileUsesTokenCmd(t *testing.T) {

@@ -187,7 +196,7 @@ func TestTokenCmdRunsOncePerProcess(t *testing.T) {
187	196		for range 2 {
188	197		c, _, err := root.Find([]string{"repo", "list"})
189	198		require.NoError(t, err)
190		-	_, err = env.client(c, true)
	199	+	_, err = env.client(c)
191	200		require.NoError(t, err)
192	201		}
193	202		assert.Len(t, *ran, 1)

@@ -256,10 +265,11 @@ func TestMissingURLIsReportedBeforeTokenCmdRuns(t *testing.T) {
256	265
257	266		func TestInvalidFileURLIsReported(t *testing.T) {
258	267		env := testEnv(t, nil)
259		-	writeConfigFile(t, env, "url = git.example.com\n", 0o600)
	268	+	path := writeConfigFile(t, env, "url = git.example.com\n", 0o600)
260	269		res := runCLIWith(t, env, "", "repo", "list")
261	270		require.Error(t, res.err)
262		-	assert.Contains(t, res.err.Error(), "invalid server URL")
	271	+	assert.Contains(t, res.err.Error(), "url in "+filepath.Dir(path)) // the file named
	272	+	assert.Contains(t, res.err.Error(), "ermite config set url")
263	273		assert.Contains(t, res.err.Error(), "http")
264	274		}
265	275

@@ -298,40 +308,63 @@ func TestTokenCmdRunsThroughTheShell(t *testing.T) {
298	308		f := listFake()
299	309		env := testEnv(t, nil)
300	310		writeConfigFile(t, env, "url = "+f.serve(t)+"\ntoken.cmd = printf secret\n", 0o600)
301		-	env.runTokenCmd = nil // the real thing
	311	+	env.runTokenCmd = runTokenShell // the real thing
302	312
303	313		runCLIWith(t, env, "", "repo", "list").ok(t)
304	314		assert.Equal(t, "Bearer secret", lastAuth(f))
305	315		}
306	316
307		-	func TestRunShellReportsFailuresWithoutOutput(t *testing.T) {
	317	+	func TestRunShell(t *testing.T) {
308	318		if runtime.GOOS == "windows" {
309	319		t.Skip("uses sh")
310	320		}
311		-	_, err := runShell(t.Context(), "echo s3cret; exit 3", time.Minute)
312		-	require.Error(t, err)
313		-	assert.Contains(t, err.Error(), "exit status 3")
314		-	assert.NotContains(t, err.Error(), "s3cret")
315		-
316		-	_, err = runShell(t.Context(), "head -c 5000 /dev/zero \| tr '\\0' a", time.Minute)
317		-	require.Error(t, err)
318		-	assert.Contains(t, err.Error(), "more than 4096 bytes")
319		-	assert.NotContains(t, err.Error(), "aaa")
320		-
321		-	out, err := runShell(t.Context(), "echo ermite_ok", time.Minute)
322		-	require.NoError(t, err)
323		-	assert.Equal(t, "ermite_ok\n", out)
	321	+	tests := []struct {
	322	+	name string
	323	+	line string
	324	+	want string // the output on success
	325	+	errHas string // the error on failure
	326	+	errNot string // what the error must not show
	327	+	wantErr bool
	328	+	}{
	329	+	{name: "exit status", line: "echo s3cret; exit 3", wantErr: true, errHas: "exit status 3", errNot: "s3cret"},
	330	+	{name: "too much output", line: "head -c 5000 /dev/zero \| tr '\\0' a", wantErr: true, errHas: "more than 4096 bytes", errNot: "aaa"},
	331	+	{name: "output", line: "echo ermite_ok", want: "ermite_ok\n"},
	332	+	}
	333	+	for _, tt := range tests {
	334	+	t.Run(tt.name, func(t *testing.T) {
	335	+	out, err := runShell(t.Context(), tt.line, time.Minute)
	336	+	if !tt.wantErr {
	337	+	require.NoError(t, err)
	338	+	assert.Equal(t, tt.want, out)
	339	+	return
	340	+	}
	341	+	require.Error(t, err)
	342	+	assert.Contains(t, err.Error(), tt.errHas)
	343	+	assert.NotContains(t, err.Error(), tt.errNot)
	344	+	})
	345	+	}
324	346		}
325	347
326		-	func TestTokenCmdTimesOut(t *testing.T) {
	348	+	// requireNoTerminal skips a test of the non-terminal path when the tests run
	349	+	// with a terminal as standard input.
	350	+	func requireNoTerminal(t *testing.T) {
	351	+	t.Helper()
327	352		if runtime.GOOS == "windows" {
328	353		t.Skip("uses sh")
329	354		}
	355	+	if term.IsTerminal(int(os.Stdin.Fd())) {
	356	+	t.Skip("standard input is a terminal")
	357	+	}
	358	+	}
	359	+
	360	+	func TestTokenCmdTimesOut(t *testing.T) {
	361	+	requireNoTerminal(t)
330	362		f := listFake()
331	363		env := testEnv(t, nil)
332		-	writeConfigFile(t, env, "url = "+f.serve(t)+"\ntoken.cmd = exec sleep 30\n", 0o600)
333		-	env.runTokenCmd = nil
334		-	env.tokenTimeout = 100 * time.Millisecond
	364	+	writeConfigFile(t, env, "url = "+f.serve(t)+"\ntoken.cmd = sleep 30; :\n", 0o600)
	365	+	env.runTokenCmd = func(ctx context.Context, line string) (string, error) {
	366	+	return runShell(ctx, line, 100*time.Millisecond)
	367	+	}
335	368
336	369		start := time.Now()
337	370		res := runCLIWith(t, env, "", "repo", "list")

@@ -341,11 +374,7 @@ func TestTokenCmdTimesOut(t *testing.T) {
341	374		}
342	375
343	376		func TestTokenCmdGetsNoInputWithoutATerminal(t *testing.T) {
344		-	if runtime.GOOS == "windows" {
345		-	t.Skip("uses sh")
346		-	}
347		-	// under `go test` stdin is the null device or a pipe, never a terminal
348		-	require.False(t, stdinIsTerminal())
	377	+	requireNoTerminal(t)
349	378		out, err := runShell(t.Context(), "cat; echo done", time.Minute)
350	379		require.NoError(t, err)
351	380		assert.Equal(t, "done\n", out, "the command read nothing")

@@ -376,21 +405,6 @@ func TestTokenCmdRefusedWhenDirectoryIsWritableByOthers(t *testing.T) {
376	405		})
377	406		}
378	407
379		-	func TestTokenCmdRefusedWhenFileIsOwnedByAnotherUser(t *testing.T) {
380		-	if runtime.GOOS == "windows" \|\| os.Geteuid() != 0 {
381		-	t.Skip("needs root to give a file away")
382		-	}
383		-	env := testEnv(t, nil)
384		-	path := writeConfigFile(t, env, "url = http://ermite.invalid:1\ntoken.cmd = get-token\n", 0o600)
385		-	require.NoError(t, os.Chown(path, 65534, -1))
386		-	ran := tokenCmdStub(env, "ermite_cmd", nil)
387		-
388		-	res := runCLIWith(t, env, "", "repo", "list")
389		-	require.Error(t, res.err)
390		-	assert.Equal(t, "config file "+path+" is owned by another user: token.cmd not run", res.err.Error())
391		-	assert.Empty(t, *ran)
392		-	}
393		-
394	408		func TestTokenCmdFollowsASymlinkedConfigFile(t *testing.T) {
395	409		if runtime.GOOS == "windows" {
396	410		t.Skip("symlinks")


@@ -14,6 +14,7 @@ import (
14			14
15		"github.com/stretchr/testify/assert"	15		"github.com/stretchr/testify/assert"
16		"github.com/stretchr/testify/require"	16		"github.com/stretchr/testify/require"
			17	+	"golang.org/x/term"
17			18
18		"bnema.dev/ermite-api/gen/ermite"	19		"bnema.dev/ermite-api/gen/ermite"
19		)	20		)

@@ -88,30 +89,38 @@ func TestEnvironmentWinsOverConfigFile(t *testing.T) {
88			89
89		func TestEnvURLDifferentFromFileKeepsTokenCmdOut(t *testing.T) {	90		func TestEnvURLDifferentFromFileKeepsTokenCmdOut(t *testing.T) {
90		f := listFake()	91		f := listFake()
91	-	env := testEnv(t, map[string]string{"ERMITE_URL": f.serve(t)})	92	+	tests := []struct {
92	-	writeConfigFile(t, env, "url = http://file.invalid\ntoken.cmd = get-token\n", 0o600)	93	+	name string
93	-	ran := tokenCmdStub(env, "ermite_cmd", nil)	94	+	envURL string // "" means the fake server
94	-		95	+	file string // the content of the config file
95	-	res := runCLIWith(t, env, "", "repo", "list").ok(t)	96	+	wantErr bool
96	-	assert.Empty(t, *ran, "the token of the file's server must not go to another server")	97	+	}{
97	-	assert.Empty(t, lastAuth(f))	98	+	{name: "other url in the file", file: "url = http://file.invalid\ntoken.cmd = get-token\n"},
98	-	assert.Equal(t, 1, strings.Count(res.stderr, "ermite: ERMITE_URL differs from the config file url: token.cmd not run (set ERMITE_TOKEN)\n"), res.stderr)	99	+	{name: "file without url", file: "token.cmd = get-token\n"},
			100	+	{name: "invalid ERMITE_URL", envURL: "nope", file: "url = http://file.invalid\ntoken.cmd = get-token\n", wantErr: true},
			101	+	}
			102	+	for _, tt := range tests {
			103	+	t.Run(tt.name, func(t *testing.T) {
			104	+	envURL := tt.envURL
			105	+	if envURL == "" {
			106	+	envURL = f.serve(t)
			107	+	}
			108	+	env := testEnv(t, map[string]string{"ERMITE_URL": envURL})
			109	+	writeConfigFile(t, env, tt.file, 0o600)
			110	+	ran := tokenCmdStub(env, "ermite_cmd", nil)
99			111
100	-	t.Run("file without url", func(t *testing.T) {	112	+	res := runCLIWith(t, env, "", "repo", "list")
101	-	env := testEnv(t, map[string]string{"ERMITE_URL": f.serve(t)})	113	+	assert.Empty(t, *ran, "the token of the file's server must not go to another server")
102	-	writeConfigFile(t, env, "token.cmd = get-token\n", 0o600)	114	+	if tt.wantErr {
103	-	ran := tokenCmdStub(env, "ermite_cmd", nil)	115	+	require.Error(t, res.err)
104	-	res := runCLIWith(t, env, "", "repo", "list").ok(t)	116	+	return
105	-	assert.Empty(t, *ran)	117	+	}
106	-	assert.Contains(t, res.stderr, "token.cmd not run")	118	+	require.NoError(t, res.err)
107	-	})	119	+	assert.Empty(t, lastAuth(f))
108	-	t.Run("invalid ERMITE_URL", func(t *testing.T) {	120	+	assert.Equal(t, 1, strings.Count(res.stderr, "token.cmd not run"), res.stderr)
109	-	env := testEnv(t, map[string]string{"ERMITE_URL": "nope"})	121	+	assert.Contains(t, res.stderr, "ERMITE_TOKEN")
110	-	writeConfigFile(t, env, "url = http://file.invalid\ntoken.cmd = get-token\n", 0o600)	122	+	})
111	-	ran := tokenCmdStub(env, "ermite_cmd", nil)	123	+	}
112	-	require.Error(t, runCLIWith(t, env, "", "repo", "list").err)
113	-	assert.Empty(t, *ran)
114	-	})
115		}	124		}
116			125
117		func TestEnvURLSameAsFileUsesTokenCmd(t *testing.T) {	126		func TestEnvURLSameAsFileUsesTokenCmd(t *testing.T) {

@@ -187,7 +196,7 @@ func TestTokenCmdRunsOncePerProcess(t *testing.T) {
187		for range 2 {	196		for range 2 {
188		c, _, err := root.Find([]string{"repo", "list"})	197		c, _, err := root.Find([]string{"repo", "list"})
189		require.NoError(t, err)	198		require.NoError(t, err)
190	-	_, err = env.client(c, true)	199	+	_, err = env.client(c)
191		require.NoError(t, err)	200		require.NoError(t, err)
192		}	201		}
193		assert.Len(t, *ran, 1)	202		assert.Len(t, *ran, 1)

@@ -256,10 +265,11 @@ func TestMissingURLIsReportedBeforeTokenCmdRuns(t *testing.T) {
256			265
257		func TestInvalidFileURLIsReported(t *testing.T) {	266		func TestInvalidFileURLIsReported(t *testing.T) {
258		env := testEnv(t, nil)	267		env := testEnv(t, nil)
259	-	writeConfigFile(t, env, "url = git.example.com\n", 0o600)	268	+	path := writeConfigFile(t, env, "url = git.example.com\n", 0o600)
260		res := runCLIWith(t, env, "", "repo", "list")	269		res := runCLIWith(t, env, "", "repo", "list")
261		require.Error(t, res.err)	270		require.Error(t, res.err)
262	-	assert.Contains(t, res.err.Error(), "invalid server URL")	271	+	assert.Contains(t, res.err.Error(), "url in "+filepath.Dir(path)) // the file named
			272	+	assert.Contains(t, res.err.Error(), "ermite config set url")
263		assert.Contains(t, res.err.Error(), "http")	273		assert.Contains(t, res.err.Error(), "http")
264		}	274		}
265			275

@@ -298,40 +308,63 @@ func TestTokenCmdRunsThroughTheShell(t *testing.T) {
298		f := listFake()	308		f := listFake()
299		env := testEnv(t, nil)	309		env := testEnv(t, nil)
300		writeConfigFile(t, env, "url = "+f.serve(t)+"\ntoken.cmd = printf secret\n", 0o600)	310		writeConfigFile(t, env, "url = "+f.serve(t)+"\ntoken.cmd = printf secret\n", 0o600)
301	-	env.runTokenCmd = nil // the real thing	311	+	env.runTokenCmd = runTokenShell // the real thing
302			312
303		runCLIWith(t, env, "", "repo", "list").ok(t)	313		runCLIWith(t, env, "", "repo", "list").ok(t)
304		assert.Equal(t, "Bearer secret", lastAuth(f))	314		assert.Equal(t, "Bearer secret", lastAuth(f))
305		}	315		}
306			316
307	-	func TestRunShellReportsFailuresWithoutOutput(t *testing.T) {	317	+	func TestRunShell(t *testing.T) {
308		if runtime.GOOS == "windows" {	318		if runtime.GOOS == "windows" {
309		t.Skip("uses sh")	319		t.Skip("uses sh")
310		}	320		}
311	-	_, err := runShell(t.Context(), "echo s3cret; exit 3", time.Minute)	321	+	tests := []struct {
312	-	require.Error(t, err)	322	+	name string
313	-	assert.Contains(t, err.Error(), "exit status 3")	323	+	line string
314	-	assert.NotContains(t, err.Error(), "s3cret")	324	+	want string // the output on success
315	-		325	+	errHas string // the error on failure
316	-	_, err = runShell(t.Context(), "head -c 5000 /dev/zero \| tr '\\0' a", time.Minute)	326	+	errNot string // what the error must not show
317	-	require.Error(t, err)	327	+	wantErr bool
318	-	assert.Contains(t, err.Error(), "more than 4096 bytes")	328	+	}{
319	-	assert.NotContains(t, err.Error(), "aaa")	329	+	{name: "exit status", line: "echo s3cret; exit 3", wantErr: true, errHas: "exit status 3", errNot: "s3cret"},
320	-		330	+	{name: "too much output", line: "head -c 5000 /dev/zero \| tr '\\0' a", wantErr: true, errHas: "more than 4096 bytes", errNot: "aaa"},
321	-	out, err := runShell(t.Context(), "echo ermite_ok", time.Minute)	331	+	{name: "output", line: "echo ermite_ok", want: "ermite_ok\n"},
322	-	require.NoError(t, err)	332	+	}
323	-	assert.Equal(t, "ermite_ok\n", out)	333	+	for _, tt := range tests {
			334	+	t.Run(tt.name, func(t *testing.T) {
			335	+	out, err := runShell(t.Context(), tt.line, time.Minute)
			336	+	if !tt.wantErr {
			337	+	require.NoError(t, err)
			338	+	assert.Equal(t, tt.want, out)
			339	+	return
			340	+	}
			341	+	require.Error(t, err)
			342	+	assert.Contains(t, err.Error(), tt.errHas)
			343	+	assert.NotContains(t, err.Error(), tt.errNot)
			344	+	})
			345	+	}
324		}	346		}
325			347
326	-	func TestTokenCmdTimesOut(t *testing.T) {	348	+	// requireNoTerminal skips a test of the non-terminal path when the tests run
			349	+	// with a terminal as standard input.
			350	+	func requireNoTerminal(t *testing.T) {
			351	+	t.Helper()
327		if runtime.GOOS == "windows" {	352		if runtime.GOOS == "windows" {
328		t.Skip("uses sh")	353		t.Skip("uses sh")
329		}	354		}
			355	+	if term.IsTerminal(int(os.Stdin.Fd())) {
			356	+	t.Skip("standard input is a terminal")
			357	+	}
			358	+	}
			359	+
			360	+	func TestTokenCmdTimesOut(t *testing.T) {
			361	+	requireNoTerminal(t)
330		f := listFake()	362		f := listFake()
331		env := testEnv(t, nil)	363		env := testEnv(t, nil)
332	-	writeConfigFile(t, env, "url = "+f.serve(t)+"\ntoken.cmd = exec sleep 30\n", 0o600)	364	+	writeConfigFile(t, env, "url = "+f.serve(t)+"\ntoken.cmd = sleep 30; :\n", 0o600)
333	-	env.runTokenCmd = nil	365	+	env.runTokenCmd = func(ctx context.Context, line string) (string, error) {
334	-	env.tokenTimeout = 100 * time.Millisecond	366	+	return runShell(ctx, line, 100*time.Millisecond)
			367	+	}
335			368
336		start := time.Now()	369		start := time.Now()
337		res := runCLIWith(t, env, "", "repo", "list")	370		res := runCLIWith(t, env, "", "repo", "list")

@@ -341,11 +374,7 @@ func TestTokenCmdTimesOut(t *testing.T) {
341		}	374		}
342			375
343		func TestTokenCmdGetsNoInputWithoutATerminal(t *testing.T) {	376		func TestTokenCmdGetsNoInputWithoutATerminal(t *testing.T) {
344	-	if runtime.GOOS == "windows" {	377	+	requireNoTerminal(t)
345	-	t.Skip("uses sh")
346	-	}
347	-	// under `go test` stdin is the null device or a pipe, never a terminal
348	-	require.False(t, stdinIsTerminal())
349		out, err := runShell(t.Context(), "cat; echo done", time.Minute)	378		out, err := runShell(t.Context(), "cat; echo done", time.Minute)
350		require.NoError(t, err)	379		require.NoError(t, err)
351		assert.Equal(t, "done\n", out, "the command read nothing")	380		assert.Equal(t, "done\n", out, "the command read nothing")

@@ -376,21 +405,6 @@ func TestTokenCmdRefusedWhenDirectoryIsWritableByOthers(t *testing.T) {
376		})	405		})
377		}	406		}
378			407
379	-	func TestTokenCmdRefusedWhenFileIsOwnedByAnotherUser(t *testing.T) {
380	-	if runtime.GOOS == "windows" \|\| os.Geteuid() != 0 {
381	-	t.Skip("needs root to give a file away")
382	-	}
383	-	env := testEnv(t, nil)
384	-	path := writeConfigFile(t, env, "url = http://ermite.invalid:1\ntoken.cmd = get-token\n", 0o600)
385	-	require.NoError(t, os.Chown(path, 65534, -1))
386	-	ran := tokenCmdStub(env, "ermite_cmd", nil)
387	-
388	-	res := runCLIWith(t, env, "", "repo", "list")
389	-	require.Error(t, res.err)
390	-	assert.Equal(t, "config file "+path+" is owned by another user: token.cmd not run", res.err.Error())
391	-	assert.Empty(t, *ran)
392	-	}
393	-
394		func TestTokenCmdFollowsASymlinkedConfigFile(t *testing.T) {	408		func TestTokenCmdFollowsASymlinkedConfigFile(t *testing.T) {
395		if runtime.GOOS == "windows" {	409		if runtime.GOOS == "windows" {
396		t.Skip("symlinks")	410		t.Skip("symlinks")

M internal/app/cli_mr.go +7 -7


@@ -265,7 +265,7 @@ func newMRCreateCmd(env cliEnv) cobra.Command {
265	265		Repo: args[0], Title: title, Description: desc,
266	266		SourceBranch: trimBranch(source), TargetBranch: trimBranch(target),
267	267		}
268		-	c, err := env.client(cmd, true)
	268	+	c, err := env.client(cmd)
269	269		if err != nil {
270	270		return err
271	271		}

@@ -310,7 +310,7 @@ func newMRListCmd(env cliEnv) cobra.Command {
310	310		if err != nil {
311	311		return err
312	312		}
313		-	c, err := env.client(cmd, true)
	313	+	c, err := env.client(cmd)
314	314		if err != nil {
315	315		return err
316	316		}

@@ -474,7 +474,7 @@ func newMRShowCmd(env cliEnv) cobra.Command {
474	474		if err != nil {
475	475		return err
476	476		}
477		-	c, err := env.client(cmd, true)
	477	+	c, err := env.client(cmd)
478	478		if err != nil {
479	479		return err
480	480		}

@@ -687,7 +687,7 @@ func newMRCommentCmd(env cliEnv) cobra.Command {
687	687		case line != 0 \|\| cmd.Flags().Changed("side"):
688	688		return errors.New("--line and --side need --path")
689	689		}
690		-	c, err := env.client(cmd, true)
	690	+	c, err := env.client(cmd)
691	691		if err != nil {
692	692		return err
693	693		}

@@ -722,7 +722,7 @@ func newMRResolveCmd(env cliEnv) cobra.Command {
722	722		return err
723	723		}
724	724		id := strings.TrimPrefix(strings.TrimSpace(args[2]), "#")
725		-	c, err := env.client(cmd, true)
	725	+	c, err := env.client(cmd)
726	726		if err != nil {
727	727		return err
728	728		}

@@ -781,7 +781,7 @@ func newMRMergeCmd(env cliEnv) cobra.Command {
781	781		return fmt.Errorf("--head is required: run `ermite mr show %s %d` and give the head id it prints, "+
782	782		"so that only the commit you reviewed is merged", args[0], n)
783	783		}
784		-	c, err := env.client(cmd, true)
	784	+	c, err := env.client(cmd)
785	785		if err != nil {
786	786		return err
787	787		}

@@ -830,7 +830,7 @@ func newMRCloseCmd(env cliEnv) cobra.Command {
830	830		if err != nil {
831	831		return err
832	832		}
833		-	c, err := env.client(cmd, true)
	833	+	c, err := env.client(cmd)
834	834		if err != nil {
835	835		return err
836	836		}


@@ -265,7 +265,7 @@ func newMRCreateCmd(env cliEnv) cobra.Command {
265		Repo: args[0], Title: title, Description: desc,	265		Repo: args[0], Title: title, Description: desc,
266		SourceBranch: trimBranch(source), TargetBranch: trimBranch(target),	266		SourceBranch: trimBranch(source), TargetBranch: trimBranch(target),
267		}	267		}
268	-	c, err := env.client(cmd, true)	268	+	c, err := env.client(cmd)
269		if err != nil {	269		if err != nil {
270		return err	270		return err
271		}	271		}

@@ -310,7 +310,7 @@ func newMRListCmd(env cliEnv) cobra.Command {
310		if err != nil {	310		if err != nil {
311		return err	311		return err
312		}	312		}
313	-	c, err := env.client(cmd, true)	313	+	c, err := env.client(cmd)
314		if err != nil {	314		if err != nil {
315		return err	315		return err
316		}	316		}

@@ -474,7 +474,7 @@ func newMRShowCmd(env cliEnv) cobra.Command {
474		if err != nil {	474		if err != nil {
475		return err	475		return err
476		}	476		}
477	-	c, err := env.client(cmd, true)	477	+	c, err := env.client(cmd)
478		if err != nil {	478		if err != nil {
479		return err	479		return err
480		}	480		}

@@ -687,7 +687,7 @@ func newMRCommentCmd(env cliEnv) cobra.Command {
687		case line != 0 \|\| cmd.Flags().Changed("side"):	687		case line != 0 \|\| cmd.Flags().Changed("side"):
688		return errors.New("--line and --side need --path")	688		return errors.New("--line and --side need --path")
689		}	689		}
690	-	c, err := env.client(cmd, true)	690	+	c, err := env.client(cmd)
691		if err != nil {	691		if err != nil {
692		return err	692		return err
693		}	693		}

@@ -722,7 +722,7 @@ func newMRResolveCmd(env cliEnv) cobra.Command {
722		return err	722		return err
723		}	723		}
724		id := strings.TrimPrefix(strings.TrimSpace(args[2]), "#")	724		id := strings.TrimPrefix(strings.TrimSpace(args[2]), "#")
725	-	c, err := env.client(cmd, true)	725	+	c, err := env.client(cmd)
726		if err != nil {	726		if err != nil {
727		return err	727		return err
728		}	728		}

@@ -781,7 +781,7 @@ func newMRMergeCmd(env cliEnv) cobra.Command {
781		return fmt.Errorf("--head is required: run `ermite mr show %s %d` and give the head id it prints, "+	781		return fmt.Errorf("--head is required: run `ermite mr show %s %d` and give the head id it prints, "+
782		"so that only the commit you reviewed is merged", args[0], n)	782		"so that only the commit you reviewed is merged", args[0], n)
783		}	783		}
784	-	c, err := env.client(cmd, true)	784	+	c, err := env.client(cmd)
785		if err != nil {	785		if err != nil {
786		return err	786		return err
787		}	787		}

@@ -830,7 +830,7 @@ func newMRCloseCmd(env cliEnv) cobra.Command {
830		if err != nil {	830		if err != nil {
831		return err	831		return err
832		}	832		}
833	-	c, err := env.client(cmd, true)	833	+	c, err := env.client(cmd)
834		if err != nil {	834		if err != nil {
835		return err	835		return err
836		}	836		}

M internal/app/cli_repo.go +5 -5


@@ -146,7 +146,7 @@ func newRepoListCmd(env cliEnv) cobra.Command {
146	146		}
147	147		asJSON := addJSONFlag(cmd)
148	148		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
149		-	c, err := env.client(cmd, true)
	149	+	c, err := env.client(cmd)
150	150		if err != nil {
151	151		return err
152	152		}

@@ -205,7 +205,7 @@ func newRepoShowCmd(env cliEnv) cobra.Command {
205	205		}
206	206		asJSON := addJSONFlag(cmd)
207	207		cmd.RunE = func(cmd *cobra.Command, args []string) error {
208		-	c, err := env.client(cmd, true)
	208	+	c, err := env.client(cmd)
209	209		if err != nil {
210	210		return err
211	211		}

@@ -239,7 +239,7 @@ func newRepoCreateCmd(env cliEnv) cobra.Command {
239	239		if err != nil {
240	240		return err
241	241		}
242		-	c, err := env.client(cmd, true)
	242	+	c, err := env.client(cmd)
243	243		if err != nil {
244	244		return err
245	245		}

@@ -295,7 +295,7 @@ func newRepoEditCmd(env cliEnv) cobra.Command {
295	295		if err != nil {
296	296		return err
297	297		}
298		-	c, err := env.client(cmd, true)
	298	+	c, err := env.client(cmd)
299	299		if err != nil {
300	300		return err
301	301		}

@@ -377,7 +377,7 @@ func newRepoDeleteCmd(env cliEnv) cobra.Command {
377	377		cmd.RunE = func(cmd *cobra.Command, args []string) error {
378	378		name := args[0]
379	379		// Check the configuration before asking anything.
380		-	c, err := env.client(cmd, true)
	380	+	c, err := env.client(cmd)
381	381		if err != nil {
382	382		return err
383	383		}


@@ -146,7 +146,7 @@ func newRepoListCmd(env cliEnv) cobra.Command {
146		}	146		}
147		asJSON := addJSONFlag(cmd)	147		asJSON := addJSONFlag(cmd)
148		cmd.RunE = func(cmd *cobra.Command, _ []string) error {	148		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
149	-	c, err := env.client(cmd, true)	149	+	c, err := env.client(cmd)
150		if err != nil {	150		if err != nil {
151		return err	151		return err
152		}	152		}

@@ -205,7 +205,7 @@ func newRepoShowCmd(env cliEnv) cobra.Command {
205		}	205		}
206		asJSON := addJSONFlag(cmd)	206		asJSON := addJSONFlag(cmd)
207		cmd.RunE = func(cmd *cobra.Command, args []string) error {	207		cmd.RunE = func(cmd *cobra.Command, args []string) error {
208	-	c, err := env.client(cmd, true)	208	+	c, err := env.client(cmd)
209		if err != nil {	209		if err != nil {
210		return err	210		return err
211		}	211		}

@@ -239,7 +239,7 @@ func newRepoCreateCmd(env cliEnv) cobra.Command {
239		if err != nil {	239		if err != nil {
240		return err	240		return err
241		}	241		}
242	-	c, err := env.client(cmd, true)	242	+	c, err := env.client(cmd)
243		if err != nil {	243		if err != nil {
244		return err	244		return err
245		}	245		}

@@ -295,7 +295,7 @@ func newRepoEditCmd(env cliEnv) cobra.Command {
295		if err != nil {	295		if err != nil {
296		return err	296		return err
297		}	297		}
298	-	c, err := env.client(cmd, true)	298	+	c, err := env.client(cmd)
299		if err != nil {	299		if err != nil {
300		return err	300		return err
301		}	301		}

@@ -377,7 +377,7 @@ func newRepoDeleteCmd(env cliEnv) cobra.Command {
377		cmd.RunE = func(cmd *cobra.Command, args []string) error {	377		cmd.RunE = func(cmd *cobra.Command, args []string) error {
378		name := args[0]	378		name := args[0]
379		// Check the configuration before asking anything.	379		// Check the configuration before asking anything.
380	-	c, err := env.client(cmd, true)	380	+	c, err := env.client(cmd)
381		if err != nil {	381		if err != nil {
382		return err	382		return err
383		}	383		}

M internal/app/cli_shell_unix.go +34 -10


@@ -15,8 +15,27 @@ import (
15	15		)
16	16
17	17		// shellCommand runs line with `sh -c`.
18		-	func shellCommand(ctx context.Context, line string) *exec.Cmd {
19		-	return exec.CommandContext(ctx, "sh", "-c", line)
	18	+	//
	19	+	// When foreground is false (no terminal: a script or an agent), the command
	20	+	// gets its own process group and a cancellation, a timeout or a signal kills
	21	+	// the whole group, so a grandchild such as `sleep` or a secret manager helper
	22	+	// cannot outlive ermite or keep the output pipe open.
	23	+	//
	24	+	// When foreground is true (standard input is a terminal) the command stays in
	25	+	// the foreground process group of the terminal: a pinentry or a password
	26	+	// prompt then works, and a background group would be stopped by SIGTTIN
	27	+	// when it reads the terminal. Ctrl+C reaches the whole group. The limit: on a
	28	+	// timeout or a SIGTERM only sh is killed, not what it started.
	29	+	func shellCommand(ctx context.Context, line string, foreground bool) *exec.Cmd {
	30	+	c := exec.CommandContext(ctx, "sh", "-c", line)
	31	+	if !foreground {
	32	+	c.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
	33	+	c.Cancel = func() error {
	34	+	// A negative pid is the process group, led by sh.
	35	+	return syscall.Kill(-c.Process.Pid, syscall.SIGKILL)
	36	+	}
	37	+	}
	38	+	return c
20	39		}
21	40
22	41		// openConfig opens the config file without blocking on a FIFO swapped in for it.

@@ -26,13 +45,13 @@ func openConfig(path string) (*os.File, error) {
26	45
27	46		// checkPrivate says whether token.cmd may be run from the config file at
28	47		// path, whose open handle reported info: whoever can change the file can make
29		-	// token.cmd any command. It returns nil when the file belongs to the current
30		-	// user and cannot be written by group or others, and when its directory
31		-	// belongs to the current user or root and cannot be written by group or
32		-	// others (a sticky directory excepted), as they could replace the file.
33		-	func checkPrivate(path string, info fs.FileInfo) error {
34		-	euid := uint32(os.Geteuid())
35		-	if st, ok := info.Sys().(*syscall.Stat_t); ok && st.Uid != euid {
	48	+	// token.cmd any command. It returns nil when the file belongs to euid (the
	49	+	// effective user id of the process) and cannot be written by group or others,
	50	+	// and when its directory belongs to euid or root and cannot be written by
	51	+	// group or others (a sticky directory excepted), as they could replace the
	52	+	// file.
	53	+	func checkPrivate(path string, info fs.FileInfo, euid int) error {
	54	+	if st, ok := info.Sys().(*syscall.Stat_t); ok && int(st.Uid) != euid {
36	55		return fmt.Errorf("config file %s is owned by another user: token.cmd not run", path)
37	56		}
38	57		if info.Mode().Perm()&0o022 != 0 {

@@ -42,11 +61,16 @@ func checkPrivate(path string, info fs.FileInfo) error {
42	61		if resolved, err := filepath.EvalSymlinks(path); err == nil {
43	62		dir = filepath.Dir(resolved) // a symlinked dotfile: the directory of the target
44	63		}
	64	+	return checkPrivateDir(dir, euid)
	65	+	}
	66	+
	67	+	// checkPrivateDir is the directory half of checkPrivate.
	68	+	func checkPrivateDir(dir string, euid int) error {
45	69		di, err := os.Stat(dir)
46	70		if err != nil {
47	71		return fmt.Errorf("config directory %s: %w: token.cmd not run", dir, err)
48	72		}
49		-	if st, ok := di.Sys().(*syscall.Stat_t); ok && st.Uid != euid && st.Uid != 0 {
	73	+	if st, ok := di.Sys().(*syscall.Stat_t); ok && int(st.Uid) != euid && st.Uid != 0 {
50	74		return fmt.Errorf("config directory %s is owned by another user: token.cmd not run", dir)
51	75		}
52	76		if di.Mode().Perm()&0o022 != 0 && di.Mode()&fs.ModeSticky == 0 {

 )
 // shellCommand runs line with `sh -c`.
-func shellCommand(ctx context.Context, line string) *exec.Cmd {
+//
-	return exec.CommandContext(ctx, "sh", "-c", line)
+// When foreground is false (no terminal: a script or an agent), the command
+// gets its own process group and a cancellation, a timeout or a signal kills
+// the whole group, so a grandchild such as `sleep` or a secret manager helper
+// cannot outlive ermite or keep the output pipe open.
+//
+// When foreground is true (standard input is a terminal) the command stays in
+// the foreground process group of the terminal: a pinentry or a password
+// prompt then works, and a background group would be stopped by SIGTTIN
+// when it reads the terminal. Ctrl+C reaches the whole group. The limit: on a
+// timeout or a SIGTERM only sh is killed, not what it started.
+func shellCommand(ctx context.Context, line string, foreground bool) *exec.Cmd {
+	c := exec.CommandContext(ctx, "sh", "-c", line)
+	if !foreground {
+		c.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+		c.Cancel = func() error {
+			// A negative pid is the process group, led by sh.
+			return syscall.Kill(-c.Process.Pid, syscall.SIGKILL)
+		}
+	}
+	return c
 }
 // openConfig opens the config file without blocking on a FIFO swapped in for it.
 // checkPrivate says whether token.cmd may be run from the config file at
 // path, whose open handle reported info: whoever can change the file can make
-// token.cmd any command. It returns nil when the file belongs to the current
+// token.cmd any command. It returns nil when the file belongs to euid (the
-// user and cannot be written by group or others, and when its directory
+// effective user id of the process) and cannot be written by group or others,
-// belongs to the current user or root and cannot be written by group or
+// and when its directory belongs to euid or root and cannot be written by
-// others (a sticky directory excepted), as they could replace the file.
+// group or others (a sticky directory excepted), as they could replace the
-func checkPrivate(path string, info fs.FileInfo) error {
+// file.
-	euid := uint32(os.Geteuid())
+func checkPrivate(path string, info fs.FileInfo, euid int) error {
-	if st, ok := info.Sys().(*syscall.Stat_t); ok && st.Uid != euid {
+	if st, ok := info.Sys().(*syscall.Stat_t); ok && int(st.Uid) != euid {
 		return fmt.Errorf("config file %s is owned by another user: token.cmd not run", path)
 	}
 	if info.Mode().Perm()&0o022 != 0 {
 	if resolved, err := filepath.EvalSymlinks(path); err == nil {
 		dir = filepath.Dir(resolved) // a symlinked dotfile: the directory of the target
 	}
+	return checkPrivateDir(dir, euid)
+}
+// checkPrivateDir is the directory half of checkPrivate.
+func checkPrivateDir(dir string, euid int) error {
 	di, err := os.Stat(dir)
 	if err != nil {
 		return fmt.Errorf("config directory %s: %w: token.cmd not run", dir, err)
 	}
-	if st, ok := di.Sys().(*syscall.Stat_t); ok && st.Uid != euid && st.Uid != 0 {
+	if st, ok := di.Sys().(*syscall.Stat_t); ok && int(st.Uid) != euid && st.Uid != 0 {
 		return fmt.Errorf("config directory %s is owned by another user: token.cmd not run", dir)
 	}
 	if di.Mode().Perm()&0o022 != 0 && di.Mode()&fs.ModeSticky == 0 {

A internal/app/cli_shell_unix_test.go +160 -0

@@ -0,0 +1,160 @@
	1	+	// SPDX-License-Identifier: AGPL-3.0-or-later
	2	+
	3	+	//go:build !windows
	4	+
	5	+	package app
	6	+
	7	+	import (
	8	+	"context"
	9	+	"errors"
	10	+	"io/fs"
	11	+	"os"
	12	+	"path/filepath"
	13	+	"strconv"
	14	+	"strings"
	15	+	"syscall"
	16	+	"testing"
	17	+	"time"
	18	+
	19	+	"github.com/stretchr/testify/assert"
	20	+	"github.com/stretchr/testify/require"
	21	+	)
	22	+
	23	+	// processGone reports whether pid is gone (a zombie that nobody reaped counts).
	24	+	func processGone(pid int) bool {
	25	+	if err := syscall.Kill(pid, 0); errors.Is(err, syscall.ESRCH) {
	26	+	return true
	27	+	}
	28	+	stat, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat")
	29	+	if err != nil {
	30	+	return false
	31	+	}
	32	+	_, after, _ := strings.Cut(string(stat), ") ")
	33	+	return strings.HasPrefix(after, "Z")
	34	+	}
	35	+
	36	+	// readPid waits for the pid file written by a grandchild.
	37	+	func readPid(t *testing.T, path string) int {
	38	+	t.Helper()
	39	+	var pid int
	40	+	require.Eventually(t, func() bool {
	41	+	data, err := os.ReadFile(path)
	42	+	if err != nil {
	43	+	return false
	44	+	}
	45	+	pid, err = strconv.Atoi(strings.TrimSpace(string(data)))
	46	+	return err == nil && pid > 0
	47	+	}, 5time.Second, 10time.Millisecond)
	48	+	return pid
	49	+	}
	50	+
	51	+	func TestRunShellKillsTheWholeProcessTreeOnTimeoutAndCancel(t *testing.T) {
	52	+	requireNoTerminal(t)
	53	+	for name, tc := range map[string]struct {
	54	+	ctx func(t *testing.T) context.Context
	55	+	timeout time.Duration
	56	+	want string
	57	+	}{
	58	+	"timeout": {func(t testing.T) context.Context { return t.Context() }, 700 time.Millisecond, "timed out after 700ms"},
	59	+	"cancel": {func(t *testing.T) context.Context {
	60	+	ctx, cancel := context.WithCancel(t.Context())
	61	+	time.AfterFunc(700*time.Millisecond, cancel)
	62	+	return ctx
	63	+	}, time.Minute, "interrupted"},
	64	+	} {
	65	+	t.Run(name, func(t *testing.T) {
	66	+	pidFile := filepath.Join(t.TempDir(), "pid")
	67	+	// sh starts a grandchild, records its pid and waits for it
	68	+	line := "sleep 30 & echo $! > '" + pidFile + "'; wait"
	69	+	_, err := runShell(tc.ctx(t), line, tc.timeout)
	70	+	require.Error(t, err)
	71	+	assert.Equal(t, tc.want, err.Error())
	72	+
	73	+	pid := readPid(t, pidFile)
	74	+	require.Eventually(t, func() bool { return processGone(pid) }, 5time.Second, 20time.Millisecond,
	75	+	"the grandchild %d must not outlive the command", pid)
	76	+	})
	77	+	}
	78	+	}
	79	+
	80	+	func TestRunShellStopsOnSIGTERM(t *testing.T) {
	81	+	requireNoTerminal(t)
	82	+	pidFile := filepath.Join(t.TempDir(), "pid")
	83	+	line := "sleep 30 & echo $! > '" + pidFile + "'; wait"
	84	+	errc := make(chan error, 1)
	85	+	go func() {
	86	+	_, err := runShell(t.Context(), line, time.Minute)
	87	+	errc <- err
	88	+	}()
	89	+	pid := readPid(t, pidFile)
	90	+	// the signal is delivered to the test process itself: runShell catches it
	91	+	require.NoError(t, syscall.Kill(os.Getpid(), syscall.SIGTERM))
	92	+	select {
	93	+	case err := <-errc:
	94	+	require.Error(t, err)
	95	+	assert.Equal(t, "interrupted", err.Error())
	96	+	case <-time.After(10 * time.Second):
	97	+	t.Fatal("runShell did not stop on SIGTERM")
	98	+	}
	99	+	require.Eventually(t, func() bool { return processGone(pid) }, 5time.Second, 20time.Millisecond)
	100	+	}
	101	+
	102	+	// fakeInfo is a file whose owner and mode are chosen by the test.
	103	+	type fakeInfo struct {
	104	+	fs.FileInfo
	105	+	mode fs.FileMode
	106	+	uid uint32
	107	+	}
	108	+
	109	+	func (f fakeInfo) Mode() fs.FileMode { return f.mode }
	110	+	func (f fakeInfo) Sys() any { return &syscall.Stat_t{Uid: f.uid} }
	111	+
	112	+	func TestCheckPrivate(t *testing.T) {
	113	+	dir := t.TempDir() // owned by the test user, mode 0700
	114	+	path := filepath.Join(dir, "config")
	115	+	require.NoError(t, os.WriteFile(path, nil, 0o600))
	116	+	real, err := os.Stat(path)
	117	+	require.NoError(t, err)
	118	+	me := os.Geteuid()
	119	+
	120	+	tests := []struct {
	121	+	name string
	122	+	mode fs.FileMode
	123	+	uid int // owner of the file
	124	+	euid int
	125	+	want string // error text, "" for accepted
	126	+	}{
	127	+	{"own private file", 0o600, me, me, ""},
	128	+	{"file of another user", 0o600, me + 1, me, "is owned by another user"},
	129	+	{"file owned by root, not me", 0o600, 0, me + 1, "is owned by another user"},
	130	+	{"group writable", 0o620, me, me, "is writable by others"},
	131	+	{"world writable", 0o602, me, me, "is writable by others"},
	132	+	{"group readable", 0o640, me, me, ""},
	133	+	// the directory is the real one: owned by me
	134	+	{"directory of another user", 0o600, me + 1, me + 1, "config directory"},
	135	+	}
	136	+	for _, tt := range tests {
	137	+	t.Run(tt.name, func(t *testing.T) {
	138	+	if tt.want == "config directory" && me == 0 {
	139	+	t.Skip("root owns every directory it creates")
	140	+	}
	141	+	err := checkPrivate(path, fakeInfo{FileInfo: real, mode: tt.mode, uid: uint32(tt.uid)}, tt.euid)
	142	+	if tt.want == "" {
	143	+	assert.NoError(t, err)
	144	+	return
	145	+	}
	146	+	require.Error(t, err)
	147	+	assert.Contains(t, err.Error(), tt.want)
	148	+	})
	149	+	}
	150	+
	151	+	t.Run("directory owned by root is fine", func(t *testing.T) {
	152	+	// /tmp and friends are root's; use a root-owned directory when there is one
	153	+	fi, err := os.Stat("/")
	154	+	require.NoError(t, err)
	155	+	if st, ok := fi.Sys().(*syscall.Stat_t); !ok \|\| st.Uid != 0 \|\| fi.Mode().Perm()&0o022 != 0 {
	156	+	t.Skip("/ is not a private root directory here")
	157	+	}
	158	+	assert.NoError(t, checkPrivateDir("/", me+1))
	159	+	})
	160	+	}

@@ -0,0 +1,160 @@
	1	+	// SPDX-License-Identifier: AGPL-3.0-or-later
	2	+
	3	+	//go:build !windows
	4	+
	5	+	package app
	6	+
	7	+	import (
	8	+	"context"
	9	+	"errors"
	10	+	"io/fs"
	11	+	"os"
	12	+	"path/filepath"
	13	+	"strconv"
	14	+	"strings"
	15	+	"syscall"
	16	+	"testing"
	17	+	"time"
	18	+
	19	+	"github.com/stretchr/testify/assert"
	20	+	"github.com/stretchr/testify/require"
	21	+	)
	22	+
	23	+	// processGone reports whether pid is gone (a zombie that nobody reaped counts).
	24	+	func processGone(pid int) bool {
	25	+	if err := syscall.Kill(pid, 0); errors.Is(err, syscall.ESRCH) {
	26	+	return true
	27	+	}
	28	+	stat, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat")
	29	+	if err != nil {
	30	+	return false
	31	+	}
	32	+	_, after, _ := strings.Cut(string(stat), ") ")
	33	+	return strings.HasPrefix(after, "Z")
	34	+	}
	35	+
	36	+	// readPid waits for the pid file written by a grandchild.
	37	+	func readPid(t *testing.T, path string) int {
	38	+	t.Helper()
	39	+	var pid int
	40	+	require.Eventually(t, func() bool {
	41	+	data, err := os.ReadFile(path)
	42	+	if err != nil {
	43	+	return false
	44	+	}
	45	+	pid, err = strconv.Atoi(strings.TrimSpace(string(data)))
	46	+	return err == nil && pid > 0
	47	+	}, 5time.Second, 10time.Millisecond)
	48	+	return pid
	49	+	}
	50	+
	51	+	func TestRunShellKillsTheWholeProcessTreeOnTimeoutAndCancel(t *testing.T) {
	52	+	requireNoTerminal(t)
	53	+	for name, tc := range map[string]struct {
	54	+	ctx func(t *testing.T) context.Context
	55	+	timeout time.Duration
	56	+	want string
	57	+	}{
	58	+	"timeout": {func(t testing.T) context.Context { return t.Context() }, 700 time.Millisecond, "timed out after 700ms"},
	59	+	"cancel": {func(t *testing.T) context.Context {
	60	+	ctx, cancel := context.WithCancel(t.Context())
	61	+	time.AfterFunc(700*time.Millisecond, cancel)
	62	+	return ctx
	63	+	}, time.Minute, "interrupted"},
	64	+	} {
	65	+	t.Run(name, func(t *testing.T) {
	66	+	pidFile := filepath.Join(t.TempDir(), "pid")
	67	+	// sh starts a grandchild, records its pid and waits for it
	68	+	line := "sleep 30 & echo $! > '" + pidFile + "'; wait"
	69	+	_, err := runShell(tc.ctx(t), line, tc.timeout)
	70	+	require.Error(t, err)
	71	+	assert.Equal(t, tc.want, err.Error())
	72	+
	73	+	pid := readPid(t, pidFile)
	74	+	require.Eventually(t, func() bool { return processGone(pid) }, 5time.Second, 20time.Millisecond,
	75	+	"the grandchild %d must not outlive the command", pid)
	76	+	})
	77	+	}
	78	+	}
	79	+
	80	+	func TestRunShellStopsOnSIGTERM(t *testing.T) {
	81	+	requireNoTerminal(t)
	82	+	pidFile := filepath.Join(t.TempDir(), "pid")
	83	+	line := "sleep 30 & echo $! > '" + pidFile + "'; wait"
	84	+	errc := make(chan error, 1)
	85	+	go func() {
	86	+	_, err := runShell(t.Context(), line, time.Minute)
	87	+	errc <- err
	88	+	}()
	89	+	pid := readPid(t, pidFile)
	90	+	// the signal is delivered to the test process itself: runShell catches it
	91	+	require.NoError(t, syscall.Kill(os.Getpid(), syscall.SIGTERM))
	92	+	select {
	93	+	case err := <-errc:
	94	+	require.Error(t, err)
	95	+	assert.Equal(t, "interrupted", err.Error())
	96	+	case <-time.After(10 * time.Second):
	97	+	t.Fatal("runShell did not stop on SIGTERM")
	98	+	}
	99	+	require.Eventually(t, func() bool { return processGone(pid) }, 5time.Second, 20time.Millisecond)
	100	+	}
	101	+
	102	+	// fakeInfo is a file whose owner and mode are chosen by the test.
	103	+	type fakeInfo struct {
	104	+	fs.FileInfo
	105	+	mode fs.FileMode
	106	+	uid uint32
	107	+	}
	108	+
	109	+	func (f fakeInfo) Mode() fs.FileMode { return f.mode }
	110	+	func (f fakeInfo) Sys() any { return &syscall.Stat_t{Uid: f.uid} }
	111	+
	112	+	func TestCheckPrivate(t *testing.T) {
	113	+	dir := t.TempDir() // owned by the test user, mode 0700
	114	+	path := filepath.Join(dir, "config")
	115	+	require.NoError(t, os.WriteFile(path, nil, 0o600))
	116	+	real, err := os.Stat(path)
	117	+	require.NoError(t, err)
	118	+	me := os.Geteuid()
	119	+
	120	+	tests := []struct {
	121	+	name string
	122	+	mode fs.FileMode
	123	+	uid int // owner of the file
	124	+	euid int
	125	+	want string // error text, "" for accepted
	126	+	}{
	127	+	{"own private file", 0o600, me, me, ""},
	128	+	{"file of another user", 0o600, me + 1, me, "is owned by another user"},
	129	+	{"file owned by root, not me", 0o600, 0, me + 1, "is owned by another user"},
	130	+	{"group writable", 0o620, me, me, "is writable by others"},
	131	+	{"world writable", 0o602, me, me, "is writable by others"},
	132	+	{"group readable", 0o640, me, me, ""},
	133	+	// the directory is the real one: owned by me
	134	+	{"directory of another user", 0o600, me + 1, me + 1, "config directory"},
	135	+	}
	136	+	for _, tt := range tests {
	137	+	t.Run(tt.name, func(t *testing.T) {
	138	+	if tt.want == "config directory" && me == 0 {
	139	+	t.Skip("root owns every directory it creates")
	140	+	}
	141	+	err := checkPrivate(path, fakeInfo{FileInfo: real, mode: tt.mode, uid: uint32(tt.uid)}, tt.euid)
	142	+	if tt.want == "" {
	143	+	assert.NoError(t, err)
	144	+	return
	145	+	}
	146	+	require.Error(t, err)
	147	+	assert.Contains(t, err.Error(), tt.want)
	148	+	})
	149	+	}
	150	+
	151	+	t.Run("directory owned by root is fine", func(t *testing.T) {
	152	+	// /tmp and friends are root's; use a root-owned directory when there is one
	153	+	fi, err := os.Stat("/")
	154	+	require.NoError(t, err)
	155	+	if st, ok := fi.Sys().(*syscall.Stat_t); !ok \|\| st.Uid != 0 \|\| fi.Mode().Perm()&0o022 != 0 {
	156	+	t.Skip("/ is not a private root directory here")
	157	+	}
	158	+	assert.NoError(t, checkPrivateDir("/", me+1))
	159	+	})
	160	+	}

M internal/app/cli_shell_windows.go +10 -3


@@ -14,7 +14,11 @@ import (
14	14
15	15		// shellCommand runs line with `cmd /C`. The command line is passed as is:
16	16		// Go's argument quoting would mangle quotes cmd.exe means to read itself.
17		-	func shellCommand(ctx context.Context, line string) *exec.Cmd {
	17	+	//
	18	+	// The command is killed with exec.CommandContext on a timeout or a signal:
	19	+	// only cmd.exe itself, not what it started (there is no process group kill
	20	+	// here); foreground is ignored.
	21	+	func shellCommand(ctx context.Context, line string, _ bool) *exec.Cmd {
18	22		c := exec.CommandContext(ctx, "cmd")
19	23		c.SysProcAttr = &syscall.SysProcAttr{CmdLine: "cmd /C " + line}
20	24		return c

@@ -23,5 +27,8 @@ func shellCommand(ctx context.Context, line string) *exec.Cmd {
23	27		// openConfig opens the config file.
24	28		func openConfig(path string) (*os.File, error) { return os.Open(path) }
25	29
26		-	// checkPrivate: Windows has no Unix mode bits to check.
27		-	func checkPrivate(string, fs.FileInfo) error { return nil }
	30	+	// checkPrivate is a no-op on Windows: it has no Unix mode bits or owner ids to
	31	+	// check, so token.cmd is run from a config file whoever can write it. The
	32	+	// per-user config directory (%AppData%) is private to the user by default;
	33	+	// keep ERMITE_CONFIG out of shared folders.
	34	+	func checkPrivate(string, fs.FileInfo, int) error { return nil }

 // shellCommand runs line with `cmd /C`. The command line is passed as is:
 // Go's argument quoting would mangle quotes cmd.exe means to read itself.
-func shellCommand(ctx context.Context, line string) *exec.Cmd {
+//
+// The command is killed with exec.CommandContext on a timeout or a signal:
+// only cmd.exe itself, not what it started (there is no process group kill
+// here); foreground is ignored.
+func shellCommand(ctx context.Context, line string, _ bool) *exec.Cmd {
 	c := exec.CommandContext(ctx, "cmd")
 	c.SysProcAttr = &syscall.SysProcAttr{CmdLine: "cmd /C " + line}
 	return c
 // openConfig opens the config file.
 func openConfig(path string) (*os.File, error) { return os.Open(path) }
-// checkPrivate: Windows has no Unix mode bits to check.
+// checkPrivate is a no-op on Windows: it has no Unix mode bits or owner ids to
-func checkPrivate(string, fs.FileInfo) error { return nil }
+// check, so token.cmd is run from a config file whoever can write it. The
+// per-user config directory (%AppData%) is private to the user by default;
+// keep ERMITE_CONFIG out of shared folders.
+func checkPrivate(string, fs.FileInfo, int) error { return nil }

M internal/app/cli_signing.go +1 -1


@@ -55,7 +55,7 @@ func newSigningKeyShowCmd(env cliEnv) cobra.Command {
55	55		cmd.MarkFlagsMutuallyExclusive("json", "public-key")
56	56		cmd.MarkFlagsMutuallyExclusive("json", "allowed-signers")
57	57		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
58		-	c, err := env.client(cmd, true)
	58	+	c, err := env.client(cmd)
59	59		if err != nil {
60	60		return err
61	61		}


@@ -55,7 +55,7 @@ func newSigningKeyShowCmd(env cliEnv) cobra.Command {
55		cmd.MarkFlagsMutuallyExclusive("json", "public-key")	55		cmd.MarkFlagsMutuallyExclusive("json", "public-key")
56		cmd.MarkFlagsMutuallyExclusive("json", "allowed-signers")	56		cmd.MarkFlagsMutuallyExclusive("json", "allowed-signers")
57		cmd.RunE = func(cmd *cobra.Command, _ []string) error {	57		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
58	-	c, err := env.client(cmd, true)	58	+	c, err := env.client(cmd)
59		if err != nil {	59		if err != nil {
60		return err	60		return err
61		}	61		}

M internal/app/cli_token.go +3 -3


@@ -107,7 +107,7 @@ func newTokenCreateCmd(env cliEnv) cobra.Command {
107	107		if err != nil {
108	108		return err
109	109		}
110		-	c, err := env.client(cmd, true)
	110	+	c, err := env.client(cmd)
111	111		if err != nil {
112	112		return err
113	113		}

@@ -185,7 +185,7 @@ func newTokenListCmd(env cliEnv) cobra.Command {
185	185		}
186	186		asJSON := addJSONFlag(cmd)
187	187		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
188		-	c, err := env.client(cmd, true)
	188	+	c, err := env.client(cmd)
189	189		if err != nil {
190	190		return err
191	191		}

@@ -239,7 +239,7 @@ func newTokenRevokeCmd(env cliEnv) cobra.Command {
239	239		return ids, cobra.ShellCompDirectiveNoFileComp
240	240		},
241	241		RunE: func(cmd *cobra.Command, args []string) error {
242		-	c, err := env.client(cmd, true)
	242	+	c, err := env.client(cmd)
243	243		if err != nil {
244	244		return err
245	245		}


@@ -107,7 +107,7 @@ func newTokenCreateCmd(env cliEnv) cobra.Command {
107		if err != nil {	107		if err != nil {
108		return err	108		return err
109		}	109		}
110	-	c, err := env.client(cmd, true)	110	+	c, err := env.client(cmd)
111		if err != nil {	111		if err != nil {
112		return err	112		return err
113		}	113		}

@@ -185,7 +185,7 @@ func newTokenListCmd(env cliEnv) cobra.Command {
185		}	185		}
186		asJSON := addJSONFlag(cmd)	186		asJSON := addJSONFlag(cmd)
187		cmd.RunE = func(cmd *cobra.Command, _ []string) error {	187		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
188	-	c, err := env.client(cmd, true)	188	+	c, err := env.client(cmd)
189		if err != nil {	189		if err != nil {
190		return err	190		return err
191		}	191		}

@@ -239,7 +239,7 @@ func newTokenRevokeCmd(env cliEnv) cobra.Command {
239		return ids, cobra.ShellCompDirectiveNoFileComp	239		return ids, cobra.ShellCompDirectiveNoFileComp
240		},	240		},
241		RunE: func(cmd *cobra.Command, args []string) error {	241		RunE: func(cmd *cobra.Command, args []string) error {
242	-	c, err := env.client(cmd, true)	242	+	c, err := env.client(cmd)
243		if err != nil {	243		if err != nil {
244		return err	244		return err
245		}	245		}

M internal/app/cli_watch.go +1 -1


@@ -163,7 +163,7 @@ func newWatchCmd(env cliEnv) cobra.Command {
163	163		if len(args) == 1 {
164	164		topics = []string{"repo:" + args[0]}
165	165		}
166		-	c, err := env.client(cmd, true)
	166	+	c, err := env.client(cmd)
167	167		if err != nil {
168	168		return err
169	169		}


@@ -163,7 +163,7 @@ func newWatchCmd(env cliEnv) cobra.Command {
163		if len(args) == 1 {	163		if len(args) == 1 {
164		topics = []string{"repo:" + args[0]}	164		topics = []string{"repo:" + args[0]}
165		}	165		}
166	-	c, err := env.client(cmd, true)	166	+	c, err := env.client(cmd)
167		if err != nil {	167		if err != nil {
168		return err	168		return err
169		}	169		}

M internal/app/cli_web.go +2 -2


@@ -43,7 +43,7 @@ func newWebLoginCmd(env cliEnv) cobra.Command {
43	43		}
44	44		asJSON := addJSONFlag(cmd)
45	45		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
46		-	c, err := env.client(cmd, true)
	46	+	c, err := env.client(cmd)
47	47		if err != nil {
48	48		return err
49	49		}

@@ -75,7 +75,7 @@ func newWebLogoutCmd(env cliEnv) cobra.Command {
75	75		if !all {
76	76		return errors.New("--all is required: this signs every browser out (one browser signs out from its page)")
77	77		}
78		-	c, err := env.client(cmd, true)
	78	+	c, err := env.client(cmd)
79	79		if err != nil {
80	80		return err
81	81		}


@@ -43,7 +43,7 @@ func newWebLoginCmd(env cliEnv) cobra.Command {
43		}	43		}
44		asJSON := addJSONFlag(cmd)	44		asJSON := addJSONFlag(cmd)
45		cmd.RunE = func(cmd *cobra.Command, _ []string) error {	45		cmd.RunE = func(cmd *cobra.Command, _ []string) error {
46	-	c, err := env.client(cmd, true)	46	+	c, err := env.client(cmd)
47		if err != nil {	47		if err != nil {
48		return err	48		return err
49		}	49		}

@@ -75,7 +75,7 @@ func newWebLogoutCmd(env cliEnv) cobra.Command {
75		if !all {	75		if !all {
76		return errors.New("--all is required: this signs every browser out (one browser signs out from its page)")	76		return errors.New("--all is required: this signs every browser out (one browser signs out from its page)")
77		}	77		}
78	-	c, err := env.client(cmd, true)	78	+	c, err := env.client(cmd)
79		if err != nil {	79		if err != nil {
80		return err	80		return err
81		}	81		}