fix(host): never follow redirects and refuse plain http except loopback (M1); read the register token from stdin or the environment before the flag (m5)

commit: 495cf95009ea5dcde213aa5cd0244690c6c2ac71
author: brice · 9d ago · 2026-10-12 19:35 UTC
committer: brice · 9d ago · 2026-10-12 19:35 UTC
parent: 508a002
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +271 -22

M host_cmd.go +17 -17


@@ -7,7 +7,6 @@ import (
7	7		"errors"
8	8		"flag"
9	9		"fmt"
10		-	"net/http"
11	10		"os"
12	11
13	12		"github.com/bnema/zerowrap"

@@ -35,33 +34,34 @@ func buildDriver(ctx context.Context, scfg sandbox.Config) (sandbox.Driver, erro
35	34		return sandbox.Select(ctx, scfg, drivers.Factories(), sandbox.Prober{})
36	35		}
37	36
38		-	// registerCmd is `ermite-runner register [--token T] [--replace]`: it chooses
39		-	// the isolation mode like `run` will, registers the runner with it, and stores
40		-	// the runner token (mode 0600) in the state directory. The register token comes
41		-	// from --token or ERMITE_RUNNER_REGISTER_TOKEN, so that it need not appear in a
42		-	// process list.
	37	+	// registerCmd is `ermite-runner register [--token-stdin] [--token T] [--replace]`:
	38	+	// it chooses the isolation mode like `run` will, registers the runner with it,
	39	+	// and stores the runner token (mode 0600) in the state directory. The register
	40	+	// token is read from standard input (--token-stdin) or from
	41	+	// ERMITE_RUNNER_REGISTER_TOKEN; --token is accepted but the arguments of a
	42	+	// process are visible to every user of the host.
43	43		func registerCmd(ctx context.Context, args []string) int {
44	44		fs := flag.NewFlagSet("register", flag.ContinueOnError)
45		-	token := fs.String("token", "", "token with the runner:register scope (default: $"+host.EnvRegisterToken+")")
	45	+	token := fs.String("token", "", "token with the runner:register scope (visible in the process list: prefer $"+host.EnvRegisterToken+" or --token-stdin)")
	46	+	fromStdin := fs.Bool("token-stdin", false, "read the token from standard input")
46	47		replace := fs.Bool("replace", false, "replace a runner of the same name")
47	48		if err := fs.Parse(args); err != nil {
48	49		return 2
49	50		}
50		-	if *token == "" {
51		-	*token = os.Getenv(host.EnvRegisterToken)
	51	+	tok, err := host.ResolveRegisterToken(token, fromStdin, os.Stdin, os.Getenv)
	52	+	if err != nil {
	53	+	fmt.Fprintln(os.Stderr, "ermite-runner:", err)
	54	+	return 2
52	55		}
	56	+	*token = tok
53	57		scfg, hcfg, err := loadHostConfig()
54	58		if err != nil {
55	59		fmt.Fprintln(os.Stderr, "ermite-runner:", err)
56	60		return 2
57	61		}
58		-	switch {
59		-	case hcfg.URL == "":
	62	+	if hcfg.URL == "" {
60	63		fmt.Fprintf(os.Stderr, "ermite-runner: %s is not set\n", host.EnvURL)
61	64		return 2
62		-	case *token == "":
63		-	fmt.Fprintf(os.Stderr, "ermite-runner: give a token with --token or %s\n", host.EnvRegisterToken)
64		-	return 2
65	65		}
66	66		drv, err := buildDriver(ctx, scfg)
67	67		if err != nil {

@@ -71,7 +71,7 @@ func registerCmd(ctx context.Context, args []string) int {
71	71		iso := drv.Isolation()
72	72		_ = drv.Close()
73	73
74		-	client := host.NewClient(hcfg.URL, *token, http.DefaultTransport)
	74	+	client := hcfg.Client(hcfg.URL, *token)
75	75		st, err := host.Register(ctx, hcfg, client, iso, *replace)
76	76		if err != nil {
77	77		fmt.Fprintln(os.Stderr, "ermite-runner:", err)

@@ -115,9 +115,9 @@ func runCmd(ctx context.Context) int {
115	115		r := &host.Runner{
116	116		Cfg: hcfg,
117	117		Driver: drv,
118		-	Poll: host.NewClient(url, st.RunnerToken, http.DefaultTransport),
	118	+	Poll: hcfg.Client(url, st.RunnerToken),
119	119		Job: func(tok string) ermiteconnect.RunnerServiceClient {
120		-	return host.NewClient(url, tok, http.DefaultTransport)
	120	+	return hcfg.Client(url, tok)
121	121		},
122	122		}
123	123		r.Cfg.URL = url

 	"errors"
 	"flag"
 	"fmt"
-	"net/http"
 	"os"
 	"github.com/bnema/zerowrap"
 	return sandbox.Select(ctx, scfg, drivers.Factories(), sandbox.Prober{})
 }
-// registerCmd is `ermite-runner register [--token T] [--replace]`: it chooses
+// registerCmd is `ermite-runner register [--token-stdin] [--token T] [--replace]`:
-// the isolation mode like `run` will, registers the runner with it, and stores
+// it chooses the isolation mode like `run` will, registers the runner with it,
-// the runner token (mode 0600) in the state directory. The register token comes
+// and stores the runner token (mode 0600) in the state directory. The register
-// from --token or ERMITE_RUNNER_REGISTER_TOKEN, so that it need not appear in a
+// token is read from standard input (--token-stdin) or from
-// process list.
+// ERMITE_RUNNER_REGISTER_TOKEN; --token is accepted but the arguments of a
+// process are visible to every user of the host.
 func registerCmd(ctx context.Context, args []string) int {
 	fs := flag.NewFlagSet("register", flag.ContinueOnError)
-	token := fs.String("token", "", "token with the runner:register scope (default: $"+host.EnvRegisterToken+")")
+	token := fs.String("token", "", "token with the runner:register scope (visible in the process list: prefer $"+host.EnvRegisterToken+" or --token-stdin)")
+	fromStdin := fs.Bool("token-stdin", false, "read the token from standard input")
 	replace := fs.Bool("replace", false, "replace a runner of the same name")
 	if err := fs.Parse(args); err != nil {
 		return 2
 	}
-	if *token == "" {
+	tok, err := host.ResolveRegisterToken(*token, *fromStdin, os.Stdin, os.Getenv)
-		*token = os.Getenv(host.EnvRegisterToken)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "ermite-runner:", err)
+		return 2
 	}
+	*token = tok
 	scfg, hcfg, err := loadHostConfig()
 	if err != nil {
 		fmt.Fprintln(os.Stderr, "ermite-runner:", err)
 		return 2
 	}
-	switch {
+	if hcfg.URL == "" {
-	case hcfg.URL == "":
 		fmt.Fprintf(os.Stderr, "ermite-runner: %s is not set\n", host.EnvURL)
 		return 2
-	case *token == "":
-		fmt.Fprintf(os.Stderr, "ermite-runner: give a token with --token or %s\n", host.EnvRegisterToken)
-		return 2
 	}
 	drv, err := buildDriver(ctx, scfg)
 	if err != nil {
 	iso := drv.Isolation()
 	_ = drv.Close()
-	client := host.NewClient(hcfg.URL, *token, http.DefaultTransport)
+	client := hcfg.Client(hcfg.URL, *token)
 	st, err := host.Register(ctx, hcfg, client, iso, *replace)
 	if err != nil {
 		fmt.Fprintln(os.Stderr, "ermite-runner:", err)
 	r := &host.Runner{
 		Cfg:    hcfg,
 		Driver: drv,
-		Poll:   host.NewClient(url, st.RunnerToken, http.DefaultTransport),
+		Poll:   hcfg.Client(url, st.RunnerToken),
 		Job: func(tok string) ermiteconnect.RunnerServiceClient {
-			return host.NewClient(url, tok, http.DefaultTransport)
+			return hcfg.Client(url, tok)
 		},
 	}
 	r.Cfg.URL = url

M internal/host/config.go +54 -0


@@ -6,10 +6,14 @@ import (
6	6		"encoding/json"
7	7		"errors"
8	8		"fmt"
	9	+	"io"
	10	+	"net/http"
9	11		"os"
10	12		"path/filepath"
11	13		"regexp"
12	14		"strings"
	15	+
	16	+	"bnema.dev/ermite-api/gen/ermite/ermiteconnect"
13	17		)
14	18
15	19		// Environment variables of the host process. The sandbox settings

@@ -20,6 +24,9 @@ const (
20	24		EnvName = "ERMITE_RUNNER_NAME"
21	25		EnvLabels = "ERMITE_RUNNER_LABELS"
22	26		EnvRegisterToken = "ERMITE_RUNNER_REGISTER_TOKEN"
	27	+	// EnvInsecureHTTP allows a plain http server URL that is not loopback. The
	28	+	// runner token and the job tokens then travel in clear: tests only.
	29	+	EnvInsecureHTTP = "ERMITE_RUNNER_INSECURE_HTTP"
23	30		)
24	31
25	32		// Config is the host part of the runner configuration.

@@ -36,6 +43,16 @@ type Config struct {
36	43		Capacity int
37	44		// Version is the version of the runner binary.
38	45		Version string
	46	+	// InsecureHTTP allows a non-loopback http URL (ERMITE_RUNNER_INSECURE_HTTP=1).
	47	+	InsecureHTTP bool
	48	+	}
	49	+
	50	+	// Client returns the client for a token, honouring InsecureHTTP.
	51	+	func (c Config) Client(url, token string) ermiteconnect.RunnerServiceClient {
	52	+	if c.InsecureHTTP {
	53	+	return NewInsecureClient(url, token, http.DefaultTransport)
	54	+	}
	55	+	return NewClient(url, token, http.DefaultTransport)
39	56		}
40	57
41	58		var nameRe = regexp.MustCompile(`^[A-Za-z0-9._-]{1,64}$`)

@@ -48,6 +65,18 @@ func LoadConfig(getenv func(string) string, hostname, stateDir string, capacity
48	65		if c.Name == "" {
49	66		c.Name = sanitizeName(hostname)
50	67		}
	68	+	switch strings.ToLower(strings.TrimSpace(getenv(EnvInsecureHTTP))) {
	69	+	case "", "0", "false", "no":
	70	+	case "1", "true", "yes":
	71	+	c.InsecureHTTP = true
	72	+	default:
	73	+	return c, fmt.Errorf("%s: %q is not 0 or 1", EnvInsecureHTTP, getenv(EnvInsecureHTTP))
	74	+	}
	75	+	if c.URL != "" {
	76	+	if err := CheckURL(c.URL, c.InsecureHTTP); err != nil {
	77	+	return c, fmt.Errorf("%s: %w", EnvURL, err)
	78	+	}
	79	+	}
51	80		if !nameRe.MatchString(c.Name) {
52	81		return c, fmt.Errorf("%s: %q is not a valid runner name (1 to 64 letters, digits, '.', '_' and '-')", EnvName, c.Name)
53	82		}

@@ -158,3 +187,28 @@ func LoadState(dir string) (State, error) {
158	187		}
159	188		return s, nil
160	189		}
	190	+
	191	+	// ResolveRegisterToken returns the token for `register`, from the first source
	192	+	// that has one: standard input (--token-stdin, read to the end and trimmed),
	193	+	// the environment (ERMITE_RUNNER_REGISTER_TOKEN), then the --token flag. The
	194	+	// flag is last because the arguments of a process are visible to every user of
	195	+	// the host (ps, /proc/<pid>/cmdline); the other two are not.
	196	+	func ResolveRegisterToken(flagValue string, fromStdin bool, stdin io.Reader, getenv func(string) string) (string, error) {
	197	+	if fromStdin {
	198	+	b, err := io.ReadAll(io.LimitReader(stdin, 4096))
	199	+	if err != nil {
	200	+	return "", fmt.Errorf("reading the token from standard input: %w", err)
	201	+	}
	202	+	if t := strings.TrimSpace(string(b)); t != "" {
	203	+	return t, nil
	204	+	}
	205	+	return "", errors.New("standard input has no token")
	206	+	}
	207	+	if t := strings.TrimSpace(getenv(EnvRegisterToken)); t != "" {
	208	+	return t, nil
	209	+	}
	210	+	if t := strings.TrimSpace(flagValue); t != "" {
	211	+	return t, nil
	212	+	}
	213	+	return "", fmt.Errorf("give the token in %s or on standard input (--token-stdin); --token works too but is visible in the process list", EnvRegisterToken)
	214	+	}


@@ -6,10 +6,14 @@ import (
6	"encoding/json"	6		"encoding/json"
7	"errors"	7		"errors"
8	"fmt"	8		"fmt"
		9	+	"io"
		10	+	"net/http"
9	"os"	11		"os"
10	"path/filepath"	12		"path/filepath"
11	"regexp"	13		"regexp"
12	"strings"	14		"strings"
		15	+
		16	+	"bnema.dev/ermite-api/gen/ermite/ermiteconnect"
13	)	17		)
14		18
15	// Environment variables of the host process. The sandbox settings	19		// Environment variables of the host process. The sandbox settings

@@ -20,6 +24,9 @@ const (
20	EnvName = "ERMITE_RUNNER_NAME"	24		EnvName = "ERMITE_RUNNER_NAME"
21	EnvLabels = "ERMITE_RUNNER_LABELS"	25		EnvLabels = "ERMITE_RUNNER_LABELS"
22	EnvRegisterToken = "ERMITE_RUNNER_REGISTER_TOKEN"	26		EnvRegisterToken = "ERMITE_RUNNER_REGISTER_TOKEN"
		27	+	// EnvInsecureHTTP allows a plain http server URL that is not loopback. The
		28	+	// runner token and the job tokens then travel in clear: tests only.
		29	+	EnvInsecureHTTP = "ERMITE_RUNNER_INSECURE_HTTP"
23	)	30		)
24		31
25	// Config is the host part of the runner configuration.	32		// Config is the host part of the runner configuration.

@@ -36,6 +43,16 @@ type Config struct {
36	Capacity int	43		Capacity int
37	// Version is the version of the runner binary.	44		// Version is the version of the runner binary.
38	Version string	45		Version string
		46	+	// InsecureHTTP allows a non-loopback http URL (ERMITE_RUNNER_INSECURE_HTTP=1).
		47	+	InsecureHTTP bool
		48	+	}
		49	+
		50	+	// Client returns the client for a token, honouring InsecureHTTP.
		51	+	func (c Config) Client(url, token string) ermiteconnect.RunnerServiceClient {
		52	+	if c.InsecureHTTP {
		53	+	return NewInsecureClient(url, token, http.DefaultTransport)
		54	+	}
		55	+	return NewClient(url, token, http.DefaultTransport)
39	}	56		}
40		57
41	var nameRe = regexp.MustCompile(`^[A-Za-z0-9._-]{1,64}$`)	58		var nameRe = regexp.MustCompile(`^[A-Za-z0-9._-]{1,64}$`)

@@ -48,6 +65,18 @@ func LoadConfig(getenv func(string) string, hostname, stateDir string, capacity
48	if c.Name == "" {	65		if c.Name == "" {
49	c.Name = sanitizeName(hostname)	66		c.Name = sanitizeName(hostname)
50	}	67		}
		68	+	switch strings.ToLower(strings.TrimSpace(getenv(EnvInsecureHTTP))) {
		69	+	case "", "0", "false", "no":
		70	+	case "1", "true", "yes":
		71	+	c.InsecureHTTP = true
		72	+	default:
		73	+	return c, fmt.Errorf("%s: %q is not 0 or 1", EnvInsecureHTTP, getenv(EnvInsecureHTTP))
		74	+	}
		75	+	if c.URL != "" {
		76	+	if err := CheckURL(c.URL, c.InsecureHTTP); err != nil {
		77	+	return c, fmt.Errorf("%s: %w", EnvURL, err)
		78	+	}
		79	+	}
51	if !nameRe.MatchString(c.Name) {	80		if !nameRe.MatchString(c.Name) {
52	return c, fmt.Errorf("%s: %q is not a valid runner name (1 to 64 letters, digits, '.', '_' and '-')", EnvName, c.Name)	81		return c, fmt.Errorf("%s: %q is not a valid runner name (1 to 64 letters, digits, '.', '_' and '-')", EnvName, c.Name)
53	}	82		}

@@ -158,3 +187,28 @@ func LoadState(dir string) (State, error) {
158	}	187		}
159	return s, nil	188		return s, nil
160	}	189		}
		190	+
		191	+	// ResolveRegisterToken returns the token for `register`, from the first source
		192	+	// that has one: standard input (--token-stdin, read to the end and trimmed),
		193	+	// the environment (ERMITE_RUNNER_REGISTER_TOKEN), then the --token flag. The
		194	+	// flag is last because the arguments of a process are visible to every user of
		195	+	// the host (ps, /proc/<pid>/cmdline); the other two are not.
		196	+	func ResolveRegisterToken(flagValue string, fromStdin bool, stdin io.Reader, getenv func(string) string) (string, error) {
		197	+	if fromStdin {
		198	+	b, err := io.ReadAll(io.LimitReader(stdin, 4096))
		199	+	if err != nil {
		200	+	return "", fmt.Errorf("reading the token from standard input: %w", err)
		201	+	}
		202	+	if t := strings.TrimSpace(string(b)); t != "" {
		203	+	return t, nil
		204	+	}
		205	+	return "", errors.New("standard input has no token")
		206	+	}
		207	+	if t := strings.TrimSpace(getenv(EnvRegisterToken)); t != "" {
		208	+	return t, nil
		209	+	}
		210	+	if t := strings.TrimSpace(flagValue); t != "" {
		211	+	return t, nil
		212	+	}
		213	+	return "", fmt.Errorf("give the token in %s or on standard input (--token-stdin); --token works too but is visible in the process list", EnvRegisterToken)
		214	+	}

M internal/host/rpc.go +65 -5


@@ -9,7 +9,10 @@ package host
9	9		import (
10	10		"context"
11	11		"errors"
	12	+	"fmt"
12	13		"net/http"
	14	+	"net/netip"
	15	+	"net/url"
13	16		"strings"
14	17		"time"
15	18

@@ -20,13 +23,52 @@ import (
20	23		)
21	24
22	25		// bearer adds "Authorization: Bearer <token>" to every request. The token is
23		-	// never logged.
	26	+	// never logged. It refuses to send the token over a connection that is not
	27	+	// protected: plain http is only for loopback, unless AllowInsecure is set.
24	28		type bearer struct {
25		-	token string
26		-	base http.RoundTripper
	29	+	token string
	30	+	base http.RoundTripper
	31	+	allowInsecure bool
	32	+	}
	33	+
	34	+	// ErrInsecureURL is returned for a server URL that is not https and not
	35	+	// loopback: the runner token and the job tokens would travel in clear.
	36	+	var ErrInsecureURL = errors.New("the server URL must be https (plain http is only accepted for localhost; ERMITE_RUNNER_INSECURE_HTTP=1 allows it, for tests, and sends every token in clear)")
	37	+
	38	+	// CheckURL validates the URL of the server: http or https, a host, no
	39	+	// credentials, and https unless the host is loopback or allowInsecure is set.
	40	+	func CheckURL(raw string, allowInsecure bool) error {
	41	+	u, err := url.Parse(raw)
	42	+	if err != nil \|\| u.Host == "" {
	43	+	return fmt.Errorf("%q is not a valid URL", raw)
	44	+	}
	45	+	if u.User != nil {
	46	+	return errors.New("the server URL must not contain credentials")
	47	+	}
	48	+	switch u.Scheme {
	49	+	case "https":
	50	+	return nil
	51	+	case "http":
	52	+	if allowInsecure \|\| isLoopback(u.Hostname()) {
	53	+	return nil
	54	+	}
	55	+	return ErrInsecureURL
	56	+	}
	57	+	return fmt.Errorf("%q: the scheme must be https", raw)
	58	+	}
	59	+
	60	+	func isLoopback(host string) bool {
	61	+	if host == "localhost" {
	62	+	return true
	63	+	}
	64	+	ip, err := netip.ParseAddr(host)
	65	+	return err == nil && ip.IsLoopback()
27	66		}
28	67
29	68		func (b bearer) RoundTrip(r http.Request) (http.Response, error) {
	69	+	if err := CheckURL(r.URL.String(), b.allowInsecure); err != nil {
	70	+	return nil, err
	71	+	}
30	72		r = r.Clone(r.Context())
31	73		if b.token != "" {
32	74		r.Header.Set("Authorization", "Bearer "+b.token)

@@ -40,9 +82,27 @@ func (b bearer) RoundTrip(r http.Request) (http.Response, error) {
40	82
41	83		// NewClient returns a RunnerService client that authenticates with token. The
42	84		// HTTP client has no overall timeout: calls carry their own context deadline
43		-	// (WatchJob is a long stream).
	85	+	// (WatchJob is a long stream). It never follows a redirect: a response from the
	86	+	// server that points elsewhere is returned as is, so that the token is not
	87	+	// carried to another host (Go would keep the Authorization header on a redirect
	88	+	// to the same host and drop it for others, but the client has no business
	89	+	// following any).
44	90		func NewClient(baseURL, token string, base http.RoundTripper) ermiteconnect.RunnerServiceClient {
45		-	return ermiteconnect.NewRunnerServiceClient(&http.Client{Transport: bearer{token, base}}, strings.TrimRight(baseURL, "/"))
	91	+	return newClient(baseURL, token, base, false)
	92	+	}
	93	+
	94	+	// NewInsecureClient is NewClient for a server reached over plain http that is
	95	+	// not loopback. Only for tests and trusted networks.
	96	+	func NewInsecureClient(baseURL, token string, base http.RoundTripper) ermiteconnect.RunnerServiceClient {
	97	+	return newClient(baseURL, token, base, true)
	98	+	}
	99	+
	100	+	func newClient(baseURL, token string, base http.RoundTripper, insecure bool) ermiteconnect.RunnerServiceClient {
	101	+	hc := &http.Client{
	102	+	Transport: bearer{token: token, base: base, allowInsecure: insecure},
	103	+	CheckRedirect: func(http.Request, []http.Request) error { return http.ErrUseLastResponse },
	104	+	}
	105	+	return ermiteconnect.NewRunnerServiceClient(hc, strings.TrimRight(baseURL, "/"))
46	106		}
47	107
48	108		// ClientFactory builds a client for a token; tests replace it.


@@ -9,7 +9,10 @@ package host
9		import (	9		import (
10		"context"	10		"context"
11		"errors"	11		"errors"
			12	+	"fmt"
12		"net/http"	13		"net/http"
			14	+	"net/netip"
			15	+	"net/url"
13		"strings"	16		"strings"
14		"time"	17		"time"
15			18

@@ -20,13 +23,52 @@ import (
20		)	23		)
21			24
22		// bearer adds "Authorization: Bearer <token>" to every request. The token is	25		// bearer adds "Authorization: Bearer <token>" to every request. The token is
23	-	// never logged.	26	+	// never logged. It refuses to send the token over a connection that is not
			27	+	// protected: plain http is only for loopback, unless AllowInsecure is set.
24		type bearer struct {	28		type bearer struct {
25	-	token string	29	+	token string
26	-	base http.RoundTripper	30	+	base http.RoundTripper
			31	+	allowInsecure bool
			32	+	}
			33	+
			34	+	// ErrInsecureURL is returned for a server URL that is not https and not
			35	+	// loopback: the runner token and the job tokens would travel in clear.
			36	+	var ErrInsecureURL = errors.New("the server URL must be https (plain http is only accepted for localhost; ERMITE_RUNNER_INSECURE_HTTP=1 allows it, for tests, and sends every token in clear)")
			37	+
			38	+	// CheckURL validates the URL of the server: http or https, a host, no
			39	+	// credentials, and https unless the host is loopback or allowInsecure is set.
			40	+	func CheckURL(raw string, allowInsecure bool) error {
			41	+	u, err := url.Parse(raw)
			42	+	if err != nil \|\| u.Host == "" {
			43	+	return fmt.Errorf("%q is not a valid URL", raw)
			44	+	}
			45	+	if u.User != nil {
			46	+	return errors.New("the server URL must not contain credentials")
			47	+	}
			48	+	switch u.Scheme {
			49	+	case "https":
			50	+	return nil
			51	+	case "http":
			52	+	if allowInsecure \|\| isLoopback(u.Hostname()) {
			53	+	return nil
			54	+	}
			55	+	return ErrInsecureURL
			56	+	}
			57	+	return fmt.Errorf("%q: the scheme must be https", raw)
			58	+	}
			59	+
			60	+	func isLoopback(host string) bool {
			61	+	if host == "localhost" {
			62	+	return true
			63	+	}
			64	+	ip, err := netip.ParseAddr(host)
			65	+	return err == nil && ip.IsLoopback()
27		}	66		}
28			67
29		func (b bearer) RoundTrip(r http.Request) (http.Response, error) {	68		func (b bearer) RoundTrip(r http.Request) (http.Response, error) {
			69	+	if err := CheckURL(r.URL.String(), b.allowInsecure); err != nil {
			70	+	return nil, err
			71	+	}
30		r = r.Clone(r.Context())	72		r = r.Clone(r.Context())
31		if b.token != "" {	73		if b.token != "" {
32		r.Header.Set("Authorization", "Bearer "+b.token)	74		r.Header.Set("Authorization", "Bearer "+b.token)

@@ -40,9 +82,27 @@ func (b bearer) RoundTrip(r http.Request) (http.Response, error) {
40			82
41		// NewClient returns a RunnerService client that authenticates with token. The	83		// NewClient returns a RunnerService client that authenticates with token. The
42		// HTTP client has no overall timeout: calls carry their own context deadline	84		// HTTP client has no overall timeout: calls carry their own context deadline
43	-	// (WatchJob is a long stream).	85	+	// (WatchJob is a long stream). It never follows a redirect: a response from the
			86	+	// server that points elsewhere is returned as is, so that the token is not
			87	+	// carried to another host (Go would keep the Authorization header on a redirect
			88	+	// to the same host and drop it for others, but the client has no business
			89	+	// following any).
44		func NewClient(baseURL, token string, base http.RoundTripper) ermiteconnect.RunnerServiceClient {	90		func NewClient(baseURL, token string, base http.RoundTripper) ermiteconnect.RunnerServiceClient {
45	-	return ermiteconnect.NewRunnerServiceClient(&http.Client{Transport: bearer{token, base}}, strings.TrimRight(baseURL, "/"))	91	+	return newClient(baseURL, token, base, false)
			92	+	}
			93	+
			94	+	// NewInsecureClient is NewClient for a server reached over plain http that is
			95	+	// not loopback. Only for tests and trusted networks.
			96	+	func NewInsecureClient(baseURL, token string, base http.RoundTripper) ermiteconnect.RunnerServiceClient {
			97	+	return newClient(baseURL, token, base, true)
			98	+	}
			99	+
			100	+	func newClient(baseURL, token string, base http.RoundTripper, insecure bool) ermiteconnect.RunnerServiceClient {
			101	+	hc := &http.Client{
			102	+	Transport: bearer{token: token, base: base, allowInsecure: insecure},
			103	+	CheckRedirect: func(http.Request, []http.Request) error { return http.ErrUseLastResponse },
			104	+	}
			105	+	return ermiteconnect.NewRunnerServiceClient(hc, strings.TrimRight(baseURL, "/"))
46		}	106		}
47			107
48		// ClientFactory builds a client for a token; tests replace it.	108		// ClientFactory builds a client for a token; tests replace it.

A internal/host/rpc_test.go +135 -0

@@ -0,0 +1,135 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package host
	4	+
	5	+	import (
	6	+	"context"
	7	+	"net/http"
	8	+	"net/http/httptest"
	9	+	"strings"
	10	+	"sync/atomic"
	11	+	"testing"
	12	+
	13	+	"connectrpc.com/connect"
	14	+
	15	+	ermite "bnema.dev/ermite-api/gen/ermite"
	16	+	)
	17	+
	18	+	func TestCheckURL(t *testing.T) {
	19	+	for _, tc := range []struct {
	20	+	url string
	21	+	insecure bool
	22	+	want string // "" = accepted
	23	+	}{
	24	+	{"https://ermite.example.org", false, ""},
	25	+	{"https://ermite.example.org:8443/base", false, ""},
	26	+	{"http://localhost:8080", false, ""},
	27	+	{"http://127.0.0.1:8080", false, ""},
	28	+	{"http://[::1]:8080", false, ""},
	29	+	{"http://ermite.example.org", false, "must be https"},
	30	+	{"http://10.0.0.5:8080", false, "must be https"},
	31	+	{"http://localhost.evil.example", false, "must be https"},
	32	+	{"http://127.0.0.1.evil.example", false, "must be https"},
	33	+	{"http://ermite.example.org", true, ""},
	34	+	{"ftp://ermite.example.org", false, "scheme must be https"},
	35	+	{"ermite.example.org", false, "not a valid URL"},
	36	+	{"https://user:pw@ermite.example.org", false, "credentials"},
	37	+	{"", false, "not a valid URL"},
	38	+	} {
	39	+	err := CheckURL(tc.url, tc.insecure)
	40	+	if tc.want == "" {
	41	+	if err != nil {
	42	+	t.Errorf("%q: %v", tc.url, err)
	43	+	}
	44	+	continue
	45	+	}
	46	+	if err == nil \|\| !strings.Contains(err.Error(), tc.want) {
	47	+	t.Errorf("%q (insecure=%v): %v, want %q", tc.url, tc.insecure, err, tc.want)
	48	+	}
	49	+	}
	50	+	}
	51	+
	52	+	// The client does not follow a redirect: the token must not reach the target,
	53	+	// whoever it is.
	54	+	func TestClientDoesNotFollowRedirects(t *testing.T) {
	55	+	var hits atomic.Int32
	56	+	var leaked atomic.Value
	57	+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	58	+	hits.Add(1)
	59	+	leaked.Store(r.Header.Get("Authorization"))
	60	+	w.WriteHeader(200)
	61	+	}))
	62	+	defer target.Close()
	63	+	redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	64	+	http.Redirect(w, r, target.URL+"/stolen", http.StatusTemporaryRedirect)
	65	+	}))
	66	+	defer redirector.Close()
	67	+
	68	+	c := NewClient(redirector.URL, "ermiterun_secret", nil)
	69	+	_, err := c.FetchJob(context.Background(), connect.NewRequest(&ermite.FetchJobRequest{}))
	70	+	if err == nil {
	71	+	t.Fatal("a redirect answered a call")
	72	+	}
	73	+	if hits.Load() != 0 {
	74	+	t.Fatalf("the redirect was followed (%d hits), the token was sent to the target: %v", hits.Load(), leaked.Load())
	75	+	}
	76	+	}
	77	+
	78	+	// The token is not sent over plain http to a host that is not loopback.
	79	+	func TestClientRefusesToSendTheTokenInClear(t *testing.T) {
	80	+	var hits atomic.Int32
	81	+	rt := roundTripFunc(func(http.Request) (http.Response, error) { hits.Add(1); return nil, nil })
	82	+	c := NewClient("http://ermite.example.org", "ermiterun_secret", rt)
	83	+	_, err := c.FetchJob(context.Background(), connect.NewRequest(&ermite.FetchJobRequest{}))
	84	+	if err == nil \|\| !strings.Contains(err.Error(), "must be https") \|\| hits.Load() != 0 {
	85	+	t.Fatalf("hits=%d err=%v", hits.Load(), err)
	86	+	}
	87	+	// The explicit flag allows it.
	88	+	c = NewInsecureClient("http://ermite.example.org", "ermiterun_secret", rt)
	89	+	_, _ = c.FetchJob(context.Background(), connect.NewRequest(&ermite.FetchJobRequest{}))
	90	+	if hits.Load() != 1 {
	91	+	t.Fatal("the insecure client did not send the request")
	92	+	}
	93	+	}
	94	+
	95	+	type roundTripFunc func(http.Request) (http.Response, error)
	96	+
	97	+	func (f roundTripFunc) RoundTrip(r http.Request) (http.Response, error) { return f(r) }
	98	+
	99	+	func TestLoadConfigChecksTheURL(t *testing.T) {
	100	+	env := map[string]string{EnvURL: "http://ermite.example.org"}
	101	+	get := func(k string) string { return env[k] }
	102	+	if _, err := LoadConfig(get, "h", "/s", 1, ""); err == nil \|\| !strings.Contains(err.Error(), EnvURL) {
	103	+	t.Fatalf("%v", err)
	104	+	}
	105	+	env[EnvInsecureHTTP] = "1"
	106	+	c, err := LoadConfig(get, "h", "/s", 1, "")
	107	+	if err != nil \|\| !c.InsecureHTTP {
	108	+	t.Fatalf("%+v %v", c, err)
	109	+	}
	110	+	env[EnvInsecureHTTP] = "maybe"
	111	+	if _, err := LoadConfig(get, "h", "/s", 1, ""); err == nil {
	112	+	t.Fatal("a bad value was accepted")
	113	+	}
	114	+	}
	115	+
	116	+	func TestResolveRegisterToken(t *testing.T) {
	117	+	env := map[string]string{}
	118	+	get := func(k string) string { return env[k] }
	119	+	if _, err := ResolveRegisterToken("", false, strings.NewReader(""), get); err == nil \|\| !strings.Contains(err.Error(), EnvRegisterToken) {
	120	+	t.Fatalf("%v", err)
	121	+	}
	122	+	if tok, _ := ResolveRegisterToken("from-flag", false, nil, get); tok != "from-flag" {
	123	+	t.Fatal(tok)
	124	+	}
	125	+	env[EnvRegisterToken] = " from-env \n"
	126	+	if tok, _ := ResolveRegisterToken("from-flag", false, nil, get); tok != "from-env" {
	127	+	t.Fatalf("the environment must win over the flag: %q", tok)
	128	+	}
	129	+	if tok, _ := ResolveRegisterToken("from-flag", true, strings.NewReader("from-stdin\n"), get); tok != "from-stdin" {
	130	+	t.Fatalf("standard input must win: %q", tok)
	131	+	}
	132	+	if _, err := ResolveRegisterToken("x", true, strings.NewReader(" \n"), get); err == nil {
	133	+	t.Fatal("an empty standard input was accepted")
	134	+	}
	135	+	}

@@ -0,0 +1,135 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package host
	4	+
	5	+	import (
	6	+	"context"
	7	+	"net/http"
	8	+	"net/http/httptest"
	9	+	"strings"
	10	+	"sync/atomic"
	11	+	"testing"
	12	+
	13	+	"connectrpc.com/connect"
	14	+
	15	+	ermite "bnema.dev/ermite-api/gen/ermite"
	16	+	)
	17	+
	18	+	func TestCheckURL(t *testing.T) {
	19	+	for _, tc := range []struct {
	20	+	url string
	21	+	insecure bool
	22	+	want string // "" = accepted
	23	+	}{
	24	+	{"https://ermite.example.org", false, ""},
	25	+	{"https://ermite.example.org:8443/base", false, ""},
	26	+	{"http://localhost:8080", false, ""},
	27	+	{"http://127.0.0.1:8080", false, ""},
	28	+	{"http://[::1]:8080", false, ""},
	29	+	{"http://ermite.example.org", false, "must be https"},
	30	+	{"http://10.0.0.5:8080", false, "must be https"},
	31	+	{"http://localhost.evil.example", false, "must be https"},
	32	+	{"http://127.0.0.1.evil.example", false, "must be https"},
	33	+	{"http://ermite.example.org", true, ""},
	34	+	{"ftp://ermite.example.org", false, "scheme must be https"},
	35	+	{"ermite.example.org", false, "not a valid URL"},
	36	+	{"https://user:pw@ermite.example.org", false, "credentials"},
	37	+	{"", false, "not a valid URL"},
	38	+	} {
	39	+	err := CheckURL(tc.url, tc.insecure)
	40	+	if tc.want == "" {
	41	+	if err != nil {
	42	+	t.Errorf("%q: %v", tc.url, err)
	43	+	}
	44	+	continue
	45	+	}
	46	+	if err == nil \|\| !strings.Contains(err.Error(), tc.want) {
	47	+	t.Errorf("%q (insecure=%v): %v, want %q", tc.url, tc.insecure, err, tc.want)
	48	+	}
	49	+	}
	50	+	}
	51	+
	52	+	// The client does not follow a redirect: the token must not reach the target,
	53	+	// whoever it is.
	54	+	func TestClientDoesNotFollowRedirects(t *testing.T) {
	55	+	var hits atomic.Int32
	56	+	var leaked atomic.Value
	57	+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	58	+	hits.Add(1)
	59	+	leaked.Store(r.Header.Get("Authorization"))
	60	+	w.WriteHeader(200)
	61	+	}))
	62	+	defer target.Close()
	63	+	redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	64	+	http.Redirect(w, r, target.URL+"/stolen", http.StatusTemporaryRedirect)
	65	+	}))
	66	+	defer redirector.Close()
	67	+
	68	+	c := NewClient(redirector.URL, "ermiterun_secret", nil)
	69	+	_, err := c.FetchJob(context.Background(), connect.NewRequest(&ermite.FetchJobRequest{}))
	70	+	if err == nil {
	71	+	t.Fatal("a redirect answered a call")
	72	+	}
	73	+	if hits.Load() != 0 {
	74	+	t.Fatalf("the redirect was followed (%d hits), the token was sent to the target: %v", hits.Load(), leaked.Load())
	75	+	}
	76	+	}
	77	+
	78	+	// The token is not sent over plain http to a host that is not loopback.
	79	+	func TestClientRefusesToSendTheTokenInClear(t *testing.T) {
	80	+	var hits atomic.Int32
	81	+	rt := roundTripFunc(func(http.Request) (http.Response, error) { hits.Add(1); return nil, nil })
	82	+	c := NewClient("http://ermite.example.org", "ermiterun_secret", rt)
	83	+	_, err := c.FetchJob(context.Background(), connect.NewRequest(&ermite.FetchJobRequest{}))
	84	+	if err == nil \|\| !strings.Contains(err.Error(), "must be https") \|\| hits.Load() != 0 {
	85	+	t.Fatalf("hits=%d err=%v", hits.Load(), err)
	86	+	}
	87	+	// The explicit flag allows it.
	88	+	c = NewInsecureClient("http://ermite.example.org", "ermiterun_secret", rt)
	89	+	_, _ = c.FetchJob(context.Background(), connect.NewRequest(&ermite.FetchJobRequest{}))
	90	+	if hits.Load() != 1 {
	91	+	t.Fatal("the insecure client did not send the request")
	92	+	}
	93	+	}
	94	+
	95	+	type roundTripFunc func(http.Request) (http.Response, error)
	96	+
	97	+	func (f roundTripFunc) RoundTrip(r http.Request) (http.Response, error) { return f(r) }
	98	+
	99	+	func TestLoadConfigChecksTheURL(t *testing.T) {
	100	+	env := map[string]string{EnvURL: "http://ermite.example.org"}
	101	+	get := func(k string) string { return env[k] }
	102	+	if _, err := LoadConfig(get, "h", "/s", 1, ""); err == nil \|\| !strings.Contains(err.Error(), EnvURL) {
	103	+	t.Fatalf("%v", err)
	104	+	}
	105	+	env[EnvInsecureHTTP] = "1"
	106	+	c, err := LoadConfig(get, "h", "/s", 1, "")
	107	+	if err != nil \|\| !c.InsecureHTTP {
	108	+	t.Fatalf("%+v %v", c, err)
	109	+	}
	110	+	env[EnvInsecureHTTP] = "maybe"
	111	+	if _, err := LoadConfig(get, "h", "/s", 1, ""); err == nil {
	112	+	t.Fatal("a bad value was accepted")
	113	+	}
	114	+	}
	115	+
	116	+	func TestResolveRegisterToken(t *testing.T) {
	117	+	env := map[string]string{}
	118	+	get := func(k string) string { return env[k] }
	119	+	if _, err := ResolveRegisterToken("", false, strings.NewReader(""), get); err == nil \|\| !strings.Contains(err.Error(), EnvRegisterToken) {
	120	+	t.Fatalf("%v", err)
	121	+	}
	122	+	if tok, _ := ResolveRegisterToken("from-flag", false, nil, get); tok != "from-flag" {
	123	+	t.Fatal(tok)
	124	+	}
	125	+	env[EnvRegisterToken] = " from-env \n"
	126	+	if tok, _ := ResolveRegisterToken("from-flag", false, nil, get); tok != "from-env" {
	127	+	t.Fatalf("the environment must win over the flag: %q", tok)
	128	+	}
	129	+	if tok, _ := ResolveRegisterToken("from-flag", true, strings.NewReader("from-stdin\n"), get); tok != "from-stdin" {
	130	+	t.Fatalf("standard input must win: %q", tok)
	131	+	}
	132	+	if _, err := ResolveRegisterToken("x", true, strings.NewReader(" \n"), get); err == nil {
	133	+	t.Fatal("an empty standard input was accepted")
	134	+	}
	135	+	}