From b664d2f12f5185eb39003674644365683057bdc3 Mon Sep 17 00:00:00 2001
From: unkin-agent <unkin-agent@unkin.net>
Date: Fri, 23 Oct 2026 21:38:43 +1100
Subject: [PATCH] fix: require https VAULT_ADDR and never follow redirects in
 set-testing

---
 README.md          |  5 +++--
 settesting.go      | 20 +++++++++++++++++---
 settesting_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/README.md b/README.md
index d5b00c1..a4d108c 100644
--- a/README.md
+++ b/README.md
@@ -44,8 +44,9 @@ It mints a JWT from `${VAULT_ADDR}/v1/identity/oidc/token/${ENCAPI_OIDC_ROLE}`
 (defaults `https://vault.service.consul:8200`, `encapi-testing`) using
 `VAULT_TOKEN` or `~/.vault-token`, trusting `VAULT_CACERT` when set, then
 `PUT`s `{"environment":"testing"}` to
-`${ENCAPI_URL}/api/v1/nodes/<certname>/environment`. `ENCAPI_URL` must be
-`https`; the bearer JWT is never sent over plain http.
+`${ENCAPI_URL}/api/v1/nodes/<certname>/environment`. `ENCAPI_URL` and
+`VAULT_ADDR` must be `https` and redirects are never followed, so neither
+the Vault token nor the JWT is sent over plain http or to another host.
 
 ## Behaviour (drop-in for the python ENC)
 
diff --git a/settesting.go b/settesting.go
index 8673526..46218bd 100644
--- a/settesting.go
+++ b/settesting.go
@@ -24,8 +24,8 @@ func setTesting(certname string, out io.Writer) error {
 	if baseURL == "" {
 		baseURL = defaultBaseURL
 	}
-	if parsed, err := url.Parse(baseURL); err != nil || parsed.Scheme != "https" {
-		return fmt.Errorf("set-testing requires an https ENCAPI_URL, got %q", baseURL)
+	if err := requireHTTPS("ENCAPI_URL", baseURL); err != nil {
+		return err
 	}
 	client, err := vaultClient()
 	if err != nil {
@@ -55,6 +55,9 @@ func mintToken(client *http.Client) (string, error) {
 	if addr == "" {
 		addr = defaultVaultAddr
 	}
+	if err := requireHTTPS("VAULT_ADDR", addr); err != nil {
+		return "", err
+	}
 	role := os.Getenv("ENCAPI_OIDC_ROLE")
 	if role == "" {
 		role = "encapi-testing"
@@ -86,6 +89,13 @@ func mintToken(client *http.Client) (string, error) {
 	return resp.Data.Token, nil
 }
 
+func requireHTTPS(name, raw string) error {
+	if parsed, err := url.Parse(raw); err != nil || parsed.Scheme != "https" {
+		return fmt.Errorf("set-testing requires an https %s, got %q", name, raw)
+	}
+	return nil
+}
+
 func readVaultToken() (string, error) {
 	if t := os.Getenv("VAULT_TOKEN"); t != "" {
 		return t, nil
@@ -102,7 +112,11 @@ func readVaultToken() (string, error) {
 }
 
 func vaultClient() (*http.Client, error) {
-	client := &http.Client{Timeout: httpTimeout}
+	client := &http.Client{
+		Timeout: httpTimeout,
+		// Never follow redirects: they would re-send the vault token or JWT.
+		CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },
+	}
 	caFile := os.Getenv("VAULT_CACERT")
 	if caFile == "" {
 		return client, nil
diff --git a/settesting_test.go b/settesting_test.go
index 6bd4905..9121e16 100644
--- a/settesting_test.go
+++ b/settesting_test.go
@@ -14,7 +14,7 @@ import (
 
 func fakeVault(t *testing.T, status int, body string) *httptest.Server {
 	t.Helper()
-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodGet || r.URL.Path != "/v1/identity/oidc/token/encapi-testing" {
 			http.NotFound(w, r)
 			return
@@ -27,6 +27,7 @@ func fakeVault(t *testing.T, status int, body string) *httptest.Server {
 		_, _ = io.WriteString(w, body)
 	}))
 	t.Cleanup(srv.Close)
+	trustServer(t, srv)
 	return srv
 }
 
@@ -47,7 +48,8 @@ func fakeEncapi(t *testing.T, status int, seen *seenPut) *httptest.Server {
 	return srv
 }
 
-// trustServer points VAULT_CACERT at the TLS server's certificate.
+// trustServer points VAULT_CACERT at the TLS server's certificate; all
+// httptest TLS servers share one certificate, so this trusts both fakes.
 func trustServer(t *testing.T, srv *httptest.Server) {
 	t.Helper()
 	path := filepath.Join(t.TempDir(), "ca.pem")
@@ -139,6 +141,46 @@ func TestSetTestingRefusesPlainHTTP(t *testing.T) {
 	}
 }
 
+func TestSetTestingRefusesPlainHTTPVault(t *testing.T) {
+	vaultHit := false
+	vault := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vaultHit = true }))
+	t.Cleanup(vault.Close)
+	var seen seenPut
+	t.Setenv("ENCAPI_URL", fakeEncapi(t, 200, &seen).URL)
+	t.Setenv("VAULT_TOKEN", "vt")
+
+	for _, addr := range []string{vault.URL, "vault.example:8200", "://bad"} {
+		t.Setenv("VAULT_ADDR", addr)
+		err := run([]string{"encapic", "set-testing", "host.example"}, io.Discard)
+		if err == nil || !strings.Contains(err.Error(), "requires an https VAULT_ADDR") {
+			t.Errorf("VAULT_ADDR=%q: err = %v, want https refusal", addr, err)
+		}
+	}
+	if vaultHit || seen.path != "" {
+		t.Errorf("request sent despite non-https VAULT_ADDR: vault=%v encapi=%+v", vaultHit, seen)
+	}
+}
+
+func TestSetTestingDoesNotFollowRedirect(t *testing.T) {
+	var hits []string
+	encapi := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hits = append(hits, r.URL.Path+" "+r.Header.Get("Authorization"))
+		http.Redirect(w, r, "/elsewhere", http.StatusTemporaryRedirect)
+	}))
+	t.Cleanup(encapi.Close)
+	t.Setenv("VAULT_ADDR", fakeVault(t, 200, `{"data":{"token":"jwt123"}}`).URL)
+	t.Setenv("VAULT_TOKEN", "vt")
+	t.Setenv("ENCAPI_URL", encapi.URL)
+
+	err := run([]string{"encapic", "set-testing", "host.example"}, io.Discard)
+	if err == nil || !strings.Contains(err.Error(), "HTTP 307") {
+		t.Fatalf("err = %v, want HTTP 307", err)
+	}
+	if len(hits) != 1 {
+		t.Errorf("encapi requests = %v, want exactly one", hits)
+	}
+}
+
 func TestSetTestingVaultNoToken(t *testing.T) {
 	for _, body := range []string{`{"data":{"token":""}}`, `{"data":{}}`, `{}`} {
 		var seen seenPut