Keep secrets off command lines the runtime records (hq issue 282)

mosquitto passed the broker's admin password to mosquitto_ctrl as -P on
every docker exec, and the container runtime keeps every exec's command
line in its event stream, where docker_events returned it. The admin
credentials now reach mosquitto_ctrl as a 0600 options file fed on
stdin, client passwords at its own prompt, and an argv carrying a secret
is refused before it runs. The admin secret says it is taken at start:
the bootstrap re-runs when the mesh replaces it and re-keys the broker
online from the value it last applied, so it can be rotated.

docker_events redacts what an exec's command line carried, and
docker_secrets_in_events names such secrets by name. keycloak's repair
hands kcadm its passwords through KC_CLI_PASSWORD; minio gives mc its
root alias through MC_HOST_mesh.

modules/docker/cmd/docker-tools/cmdline.go

@@ -0,0 +1,256 @@
+package main
+
+// A secret on a command line (novox/hq issue 282).
+//
+// **The leak this catches.** The runtime records the command line of every exec — `docker exec`, and
+// a health check, which is one — in its event stream, as the event's action (`exec_create: <argv
+// joined by spaces>`). A program that hands a password to a tool as an argument (`-P <password>`,
+// `--password <password>`, `PGPASSWORD=<password> psql`) has therefore given it to everyone who may
+// ask the runtime what happened, for as long as the runtime keeps its events — and, through
+// docker_events, to every transcript of an agent that asked. The mosquitto module did exactly that
+// with the broker's admin password, on every administrative call.
+//
+// **What is known here.** As for a log: the values of the container's environment named like a
+// secret and the passwords inside its URIs, by name; and, whatever their source, the values a
+// command line carries by its shape — the word after a flag that takes a password, a NAME=value
+// whose name says secret, the password a dynsec command sets. A secret given as a file and passed by
+// a flag the shapes do not know is not caught.
+//
+// **Never the value.** What is shown carries `[redacted: <what it was>]` in its place, and a finding
+// names the container, the module and what it was, by name.
+
+import (
+	"context"
+	"fmt"
+	"path"
+	"sort"
+	"strings"
+	"time"
+)
+
+// passwordFlags take a secret as their next word, whatever the program.
+var passwordFlags = map[string]bool{
+	"-P": true, "--password": true, "--pass": true, "--passwd": true, "--secret": true, "--secret-key": true,
+	"--token": true, "--api-key": true, "--apikey": true, "--auth": true,
+}
+
+// programFlags take a secret as their next word for one program only: elsewhere the same flag means
+// something else (redis-cli's -a is its password; nft's -a is not).
+var programFlags = map[string]map[string]bool{
+	"redis-cli":      {"-a": true},
+	"keydb-cli":      {"-a": true},
+	"valkey-cli":     {"-a": true},
+	"mosquitto_ctrl": {"-p": true}, // createClient -p <password>; the connect -p is a port, and a port is ordinary
+}
+
+// positionalSecret is where a dynsec command carries a password as an argument: the word that many
+// places after the command's name.
+var positionalSecret = map[string]int{"setClientPassword": 2, "init": 3}
+
+// commandSecret is one secret a command line carried, by what it was.
+type commandSecret struct {
+	Name  string
+	Value string
+}
+
+// secretsOnCommandLine are the values a command line carries by their shape, by what each one is.
+func secretsOnCommandLine(words []string) []commandSecret {
+	var out []commandSecret
+	add := func(name, value string) {
+		if len(value) < leastSecret || masked.MatchString(value) || ordinary.MatchString(value) {
+			return
+		}
+		out = append(out, commandSecret{name, value})
+	}
+	program := ""
+	for i, w := range words {
+		base := path.Base(w)
+		if _, known := programFlags[base]; known || base == "mosquitto_ctrl" {
+			program = base
+		}
+		if flag, value, ok := strings.Cut(w, "="); ok && strings.HasPrefix(flag, "-") {
+			if passwordFlags[flag] || programFlags[program][flag] {
+				add("the value of "+flag, value)
+			}
+			continue
+		}
+		if name, value, ok := strings.Cut(w, "="); ok && name != "" && !strings.HasPrefix(name, "-") &&
+			secretName.MatchString(name) && !notAValue.MatchString(name) && !strings.ContainsAny(name, "/:") {
+			add("the value of "+name, value)
+			continue
+		}
+		if i+1 < len(words) && (passwordFlags[w] || programFlags[program][w]) {
+			add("the word after "+w+" in a "+orProgram(program, words)+" command line", words[i+1])
+		}
+		if program == "mosquitto_ctrl" {
+			if at, ok := positionalSecret[w]; ok && i+at < len(words) && dynsecVerb(words, i) {
+				add("the password given to dynsec "+w, words[i+at])
+			}
+		}
+	}
+	return out
+}
+
+// dynsecVerb says the word at i is a dynsec command's name: it follows "dynsec".
+func dynsecVerb(words []string, i int) bool {
+	for j := i - 1; j >= 0; j-- {
+		if words[j] == "dynsec" {
+			return true
+		}
+	}
+	return false
+}
+
+func orProgram(program string, words []string) string {
+	if program != "" {
+		return program
+	}
+	if len(words) > 0 {
+		return path.Base(words[0])
+	}
+	return "program's"
+}
+
+// redactCommand is a command line with every known secret, every password inside a URI and every
+// value its shape says is a secret replaced by a mark naming what was there; and what was replaced,
+// by name.
+func redactCommand(command string, known []knownSecret) (string, []string) {
+	var names []string
+	for _, s := range known {
+		for _, f := range forms(s.Value) {
+			if strings.Contains(command, f) {
+				command = strings.ReplaceAll(command, f, "[redacted: "+s.Name+"]")
+				names = append(names, s.Name)
+				break
+			}
+		}
+	}
+	for _, s := range secretsOnCommandLine(strings.Fields(command)) {
+		if strings.Contains(command, s.Value) {
+			command = strings.ReplaceAll(command, s.Value, "[redacted: "+s.Name+"]")
+			names = append(names, s.Name)
+		}
+	}
+	if line, n := redact(command, nil); n > 0 {
+		command = line
+		names = append(names, "a password in a URI")
+	}
+	return command, names
+}
+
+// execCommand is the command line an exec event carries, and whether it carries one.
+func execCommand(action string) (verb, command string, ok bool) {
+	verb, command, ok = strings.Cut(action, ": ")
+	if !ok || !strings.HasPrefix(verb, "exec_") {
+		return "", "", false
+	}
+	return verb, command, true
+}
+
+// envCache reads each container's environment once per call.
+type envCache struct {
+	c    *Client
+	ctx  context.Context
+	seen map[string][]knownSecret
+}
+
+func (e *envCache) of(id string) []knownSecret {
+	if e.seen == nil {
+		e.seen = map[string][]knownSecret{}
+	}
+	if k, ok := e.seen[id]; ok {
+		return k
+	}
+	env, _ := e.c.envOf(e.ctx, id) // a container gone since: its shapes are still caught
+	e.seen[id] = secretsIn(env)
+	return e.seen[id]
+}
+
+// CommandLeak is one secret the runtime recorded on exec command lines: by name, never by value.
+type CommandLeak struct {
+	Container string `json:"container"`
+	HeldBy    string `json:"held_by,omitempty"`
+	Module    string `json:"module,omitempty"`
+	Secret    string `json:"secret"`
+	Execs     int    `json:"execs"`
+	Program   string `json:"program"`
+	First     string `json:"first"`
+	Last      string `json:"last"`
+}
+
+// SecretsInEvents reads the runtime's exec events in a window ending now and says which secrets
+// their command lines carried, by container and name.
+func (c *Client) SecretsInEvents(ctx context.Context, minutes int) (map[string]any, error) {
+	out, err := c.docker(ctx, "events", "--since", fmt.Sprintf("%dm", minutes), "--until", "0s",
+		"--filter", "type=container", "--filter", "event=exec_create", "--format", "{{json .}}")
+	if err != nil {
+		return nil, err
+	}
+	raw, err := jsonLines[runtimeEvent](out)
+	if err != nil {
+		return nil, err
+	}
+	envs := &envCache{c: c, ctx: ctx}
+	type key struct{ container, secret string }
+	found := map[key]*CommandLeak{}
+	execs := 0
+	for _, e := range raw {
+		verb, command, ok := execCommand(e.Action)
+		if !ok || verb != "exec_create" {
+			continue // an exec_start repeats its exec_create's command line
+		}
+		execs++
+		_, names := redactCommand(command, envs.of(e.Actor.ID))
+		if len(names) == 0 {
+			continue
+		}
+		at := time.Unix(0, e.TimeNano).UTC().Format(time.RFC3339)
+		held := e.Actor.Attributes[MeshLabel]
+		module, _, _ := strings.Cut(held, ".")
+		program := ""
+		if f := strings.Fields(command); len(f) > 0 {
+			program = path.Base(f[0])
+		}
+		for _, n := range names {
+			k := key{e.Actor.Attributes["name"], n}
+			l, ok := found[k]
+			if !ok {
+				l = &CommandLeak{Container: k.container, HeldBy: held, Module: module, Secret: n, Program: program, First: at}
+				found[k] = l
+			}
+			l.Execs++
+			l.Last = at
+		}
+	}
+	leaks := []CommandLeak{}
+	for _, l := range found {
+		leaks = append(leaks, *l)
+	}
+	sort.Slice(leaks, func(i, j int) bool {
+		if leaks[i].Container != leaks[j].Container {
+			return leaks[i].Container < leaks[j].Container
+		}
+		return leaks[i].Secret < leaks[j].Secret
+	})
+	verdict := fmt.Sprintf("no exec in the last %d minutes carried a secret on its command line", minutes)
+	if len(leaks) > 0 {
+		verdict = fmt.Sprintf("%d secret(s) on exec command lines the runtime recorded: the code that runs the exec must hand "+
+			"them over another way (a file, stdin), and each is rotated once it does (novox/hq issue 282)", len(leaks))
+	}
+	return map[string]any{
+		"verdict": verdict, "leaks": leaks, "count": len(leaks), "execs_read": execs, "minutes": minutes,
+		"knows": "values of each container's environment named like a secret, passwords in URIs, and by shape: the word after " +
+			"a password flag, a NAME=value named like a secret, and the password a dynsec command sets",
+		"history": "the runtime keeps a bounded number of events, so a window longer than what it holds reads only what it still has",
+	}, nil
+}
+
+// runtimeEvent is one line of `docker events --format '{{json .}}'`.
+type runtimeEvent struct {
+	Type, Action string
+	Actor        struct {
+		ID         string
+		Attributes map[string]string
+	}
+	TimeNano int64 `json:"timeNano"`
+}

modules/docker/cmd/docker-tools/docker.go

@@ -345,6 +345,27 @@ func (c *Client) Inspect(ctx context.Context, ref string) (map[string]any, error
 		return nil, fmt.Errorf("docker inspect answered something that is not one container")
 	}
 	obj := got[0]
+	// A container's command line is shown without the secrets it carries, as an exec's is (novox/hq
+	// issue 282): known from its environment, and by shape.
+	var known []knownSecret
+	if cfg, ok := obj["Config"].(map[string]any); ok {
+		if env, ok := cfg["Env"].([]any); ok {
+			list := make([]string, 0, len(env))
+			for _, e := range env {
+				list = append(list, fmt.Sprint(e))
+			}
+			known = secretsIn(list)
+		}
+		for _, k := range []string{"Cmd", "Entrypoint"} {
+			if words, ok := cfg[k].([]any); ok {
+				cfg[k] = redactWords(words, known)
+			}
+		}
+	}
+	if words, ok := obj["Args"].([]any); ok {
+		path, _ := obj["Path"].(string)
+		obj["Args"] = redactWords(append([]any{path}, words...), known)[1:]
+	}
 	if cfg, ok := obj["Config"].(map[string]any); ok {
 		if env, ok := cfg["Env"].([]any); ok {
 			names := []string{}
@@ -979,24 +1000,29 @@ func (c *Client) Events(ctx context.Context, minutes int, kind string, limit int
 	if err != nil {
 		return nil, err
 	}
-	raw, err := jsonLines[struct {
-		Type, Action string
-		Actor        struct {
-			ID         string
-			Attributes map[string]string
-		}
-		TimeNano int64 `json:"timeNano"`
-	}](out)
+	raw, err := jsonLines[runtimeEvent](out)
 	if err != nil {
 		return nil, err
 	}
+	// An exec's command line is shown without the secrets it carried (novox/hq issue 282): this answer
+	// is read by agents and kept in their transcripts, which would make it a second copy of the leak.
+	envs := &envCache{c: c, ctx: ctx}
+	redacted := map[string]bool{}
 	events := []map[string]any{}
 	for _, e := range raw {
 		if !execs && strings.HasPrefix(e.Action, "exec_") {
 			continue
 		}
+		action := e.Action
+		if verb, command, ok := execCommand(action); ok {
+			shown, names := redactCommand(command, envs.of(e.Actor.ID))
+			action = verb + ": " + shown
+			for _, n := range names {
+				redacted[n] = true
+			}
+		}
 		_, held := e.Actor.Attributes[MeshLabel]
-		ev := map[string]any{"time": time.Unix(0, e.TimeNano).UTC().Format(time.RFC3339), "type": e.Type, "action": e.Action,
+		ev := map[string]any{"time": time.Unix(0, e.TimeNano).UTC().Format(time.RFC3339), "type": e.Type, "action": action,
 			"id": shortID(e.Actor.ID), "name": e.Actor.Attributes["name"]}
 		if e.Type == "container" {
 			ev["mesh_held"] = held
@@ -1011,7 +1037,41 @@ func (c *Client) Events(ctx context.Context, minutes int, kind string, limit int
 	if len(events) > limit {
 		events = events[len(events)-limit:]
 	}
-	return map[string]any{"minutes": minutes, "count": total, "shown": len(events), "events": events}, nil
+	answer := map[string]any{"minutes": minutes, "count": total, "shown": len(events), "events": events}
+	if len(redacted) > 0 {
+		answer["redacted"] = sortedSet(redacted)
+		answer["leak"] = "exec command lines carried secrets, which the runtime keeps in its events; docker_secrets_in_events names them (novox/hq issue 282)"
+	}
+	return answer, nil
+}
+
+func sortedSet(set map[string]bool) []string {
+	out := make([]string, 0, len(set))
+	for k := range set {
+		out = append(out, k)
+	}
+	sort.Strings(out)
+	return out
+}
+
+// redactWords is a command's words with what redactCommand hides hidden, word by word.
+func redactWords(words []any, known []knownSecret) []any {
+	list := make([]string, len(words))
+	for i, w := range words {
+		list[i] = fmt.Sprint(w)
+	}
+	shapes := secretsOnCommandLine(list)
+	out := make([]any, len(list))
+	for i, w := range list {
+		for _, s := range shapes {
+			if strings.Contains(w, s.Value) {
+				w = strings.ReplaceAll(w, s.Value, "[redacted: "+s.Name+"]")
+			}
+		}
+		w, _ = redact(w, known)
+		out[i] = w
+	}
+	return out
 }
 
 // restartOnly are the daemon keys the runtime reads only when it starts: a reload leaves them as

modules/docker/cmd/docker-tools/main.go

@@ -112,6 +112,22 @@ func tools(c *Client) []stdio.Tool {
 				return c.SecretsInLogs(ctx, held, n)
 			},
 		},
+		{
+			Name: "docker_secrets_in_events",
+			Description: "Which secrets exec command lines carried in a window ending now (default the last 60 minutes, at most 24 hours) — the runtime records every exec's command line in its events, " +
+				"so a password passed as an argument is kept there for anyone who may ask it. By container, module and the secret's name, never its value. " +
+				"A finding is code to change (hand the secret over as a file or on stdin) and then a secret to rotate (novox/hq issue 282).",
+			Input: map[string]any{
+				"minutes": map[string]any{"type": "integer", "description": "how far back (default 60, at most 1440)"},
+			},
+			Run: func(args map[string]any) (any, error) {
+				minutes, err := bounded(args, "minutes", 60, 1440)
+				if err != nil {
+					return nil, err
+				}
+				return c.SecretsInEvents(ctx, minutes)
+			},
+		},
 		{
 			Name:        "docker_stats",
 			Description: "What the running containers use now — CPU, memory, network and disk I/O, processes — the heaviest by memory first; or one container's.",
@@ -211,8 +227,9 @@ func tools(c *Client) []stdio.Tool {
 			},
 		},
 		{
-			Name:        "docker_events",
-			Description: "What the runtime did in a window ending now (default the last 60 minutes, at most 24 hours): containers created, started, died, health changes, images pulled — with mesh_held. Exec events are left out unless asked.",
+			Name: "docker_events",
+			Description: "What the runtime did in a window ending now (default the last 60 minutes, at most 24 hours): containers created, started, died, health changes, images pulled — with mesh_held. Exec events are left out unless asked; " +
+				"an exec's command line is shown with any secret it carried as [redacted: <what it was>] — a value of the container's environment named like a secret, a password in a URI, the word after a password flag.",
 			Input: map[string]any{
 				"minutes": map[string]any{"type": "integer", "description": "how far back (default 60, at most 1440)"},
 				"type":    map[string]any{"type": "string", "description": "only one kind: container, image, network, volume, daemon, plugin or builder"},

modules/docker/cmd/docker-tools/secrets_test.go

@@ -154,3 +154,109 @@ func keys(m map[string]string) []string {
 	}
 	return out
 }
+
+// The shape of the leak in hq issue 282: a broker's admin password on an exec's command line. The
+// values are made up for the test.
+const (
+	adminPassword  = "Adm1n-pass_word-xyz"
+	clientPassword = "Cl1ent-pass_word-abc"
+)
+
+func TestACommandLineIsShownWithoutTheSecretsItCarried(t *testing.T) {
+	for _, tc := range []struct{ command, mark string }{
+		{"mosquitto_ctrl -h 127.0.0.1 -p 1883 -u mesh-admin -P " + adminPassword + " dynsec listClients", "the word after -P"},
+		{"mosquitto_ctrl -h 127.0.0.1 -p 1883 dynsec createClient alice -p " + clientPassword, "the word after -p in a mosquitto_ctrl"},
+		{"mosquitto_ctrl -o /tmp/x dynsec setClientPassword alice " + clientPassword, "the password given to dynsec setClientPassword"},
+		{"redis-cli -a " + adminPassword + " ping", "the word after -a"},
+		{"env PGPASSWORD=" + adminPassword + " psql -U app", "the value of PGPASSWORD"},
+		{"tool --password=" + adminPassword, "the value of --password"},
+		{"psql postgresql://app:" + adminPassword + "@db/app", "a password in a URI"},
+	} {
+		shown, names := redactCommand(tc.command, nil)
+		if strings.Contains(shown, adminPassword) || strings.Contains(shown, clientPassword) {
+			t.Errorf("%q: still carries the value: %q", tc.command, shown)
+		}
+		if len(names) == 0 || !strings.Contains(strings.Join(names, "|"), tc.mark) {
+			t.Errorf("%q: named %v, want %q", tc.command, names, tc.mark)
+		}
+	}
+	// A port, a path and a plain command are not secrets.
+	for _, plain := range []string{
+		"mosquitto_ctrl -h 127.0.0.1 -p 1883 dynsec listClients",
+		"/usr/bin/lavinmqctl status",
+		"sh -c umask 077\nf=$(mktemp) || exit 1 mosquitto_ctrl -h 127.0.0.1 -p 1883 dynsec getClient alice",
+		"pg_dump -Fc -f /dumps/app.dump app",
+	} {
+		if shown, names := redactCommand(plain, nil); shown != plain || len(names) > 0 {
+			t.Errorf("%q: redacted %v as %q", plain, names, shown)
+		}
+	}
+	// What the container's environment holds is known by its name, wherever it appears.
+	known := []knownSecret{{"SERVER_PASSWORD", adminPassword}}
+	if shown, names := redactCommand("app login "+adminPassword, known); strings.Contains(shown, adminPassword) ||
+		len(names) != 1 || names[0] != "SERVER_PASSWORD" {
+		t.Errorf("an environment secret on a command line: %q %v", shown, names)
+	}
+}
+
+const execEvents = `{"Type":"container","Action":"exec_create: mosquitto_ctrl -h 127.0.0.1 -p 1883 -u mesh-admin -P ` + adminPassword + ` dynsec listClients","Actor":{"ID":"aaaaaaaaaaaaaaaa","Attributes":{"name":"mosquitto","mesh-host.id":"mosquitto.server","execID":"e1"}},"timeNano":1791320000000000000}
+{"Type":"container","Action":"exec_start: mosquitto_ctrl -h 127.0.0.1 -p 1883 -u mesh-admin -P ` + adminPassword + ` dynsec listClients","Actor":{"ID":"aaaaaaaaaaaaaaaa","Attributes":{"name":"mosquitto","mesh-host.id":"mosquitto.server","execID":"e1"}},"timeNano":1791320000000100000}
+{"Type":"container","Action":"exec_die","Actor":{"ID":"aaaaaaaaaaaaaaaa","Attributes":{"name":"mosquitto","exitCode":"0"}},"timeNano":1791320000000200000}
+{"Type":"container","Action":"exec_create: /usr/bin/healthcheck","Actor":{"ID":"bbbbbbbbbbbbbbbb","Attributes":{"name":"other"}},"timeNano":1791320060000000000}
+{"Type":"container","Action":"exec_create: mosquitto_ctrl -h 127.0.0.1 -p 1883 -u mesh-admin -P ` + adminPassword + ` dynsec getClient a","Actor":{"ID":"aaaaaaaaaaaaaaaa","Attributes":{"name":"mosquitto","mesh-host.id":"mosquitto.server","execID":"e2"}},"timeNano":1791320120000000000}
+`
+
+func TestEventsShowAnExecsCommandLineWithoutItsSecrets(t *testing.T) {
+	f := (&fake{}).
+		on("docker events", Ran{Stdout: execEvents}).
+		on("docker container inspect --format {{json .Config.Env}}", Ran{Stdout: "[]\n"})
+	got, err := client(f, 1000).Events(context.Background(), 30, "", 100, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	b, _ := json.Marshal(got)
+	if strings.Contains(string(b), adminPassword) {
+		t.Fatalf("the answer carries the value: %s", b)
+	}
+	if !strings.Contains(string(b), "[redacted: the word after -P") || got["leak"] == nil {
+		t.Fatalf("not marked as redacted: %s", b)
+	}
+}
+
+func TestAScanOfExecEventsNamesEachSecretAndNeverItsValue(t *testing.T) {
+	f := (&fake{}).
+		on("docker events", Ran{Stdout: execEvents}).
+		on("docker container inspect --format {{json .Config.Env}}", Ran{Stdout: "[]\n"})
+	got, err := client(f, 1000).SecretsInEvents(context.Background(), 60)
+	if err != nil {
+		t.Fatal(err)
+	}
+	b, _ := json.Marshal(got)
+	if strings.Contains(string(b), adminPassword) {
+		t.Fatalf("the finding carries the value: %s", b)
+	}
+	leaks := got["leaks"].([]CommandLeak)
+	if len(leaks) != 1 || leaks[0].Container != "mosquitto" || leaks[0].Module != "mosquitto" || leaks[0].Execs != 2 ||
+		leaks[0].Program != "mosquitto_ctrl" || !strings.Contains(leaks[0].Secret, "-P") {
+		t.Fatalf("leaks: %+v", leaks)
+	}
+	if got["execs_read"] != 3 {
+		t.Fatalf("read %v exec_create events, want 3 (a start repeats a create and is not counted)", got["execs_read"])
+	}
+	if !f.ran("docker events --since 60m --until 0s --filter type=container --filter event=exec_create") {
+		t.Fatalf("not asked for exec creations only: %+v", f.calls)
+	}
+}
+
+func TestInspectShowsACommandLineWithoutItsSecrets(t *testing.T) {
+	obj := `[{"Path":"mosquitto_ctrl","Args":["-P","` + adminPassword + `","dynsec","listClients"],"Config":{"Env":["A=b"],"Cmd":["mosquitto_ctrl","-P","` + adminPassword + `"],"Labels":{}}}]`
+	f := (&fake{}).on("docker container inspect", Ran{Stdout: obj})
+	got, err := client(f, 1000).Inspect(context.Background(), "mosquitto")
+	if err != nil {
+		t.Fatal(err)
+	}
+	b, _ := json.Marshal(got)
+	if strings.Contains(string(b), adminPassword) || !strings.Contains(string(b), "[redacted:") {
+		t.Fatalf("inspect: %s", b)
+	}
+}