From 4531f2244f3d0042244fd13e9afd4436e7807bb9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 10:10:33 +0200 Subject: [PATCH 1/3] A secret reaches a process as a file (ADR 0086) The broker settings take a _FILE twin like the store connections; the catalogue engine refuses a secret placeholder in a container's env and a secret-carrying env-file unless the container says why with secrets-in-environment, which stays in the catalogue and never reaches the machine. --- internal/broker/broker.go | 6 +- internal/broker/management.go | 7 +- internal/catalogue/declaration.go | 10 +++ .../catalogue/secrets_in_environment_test.go | 72 +++++++++++++++++++ internal/catalogue/secrets_into_files.go | 65 +++++++++++++++++ internal/envfile/envfile.go | 39 ++++++++++ internal/envfile/envfile_test.go | 44 ++++++++++++ internal/link/serve.go | 7 +- 8 files changed, 245 insertions(+), 5 deletions(-) create mode 100644 internal/catalogue/secrets_in_environment_test.go create mode 100644 internal/envfile/envfile.go create mode 100644 internal/envfile/envfile_test.go diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 84025ef..51e5c87 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -16,6 +16,7 @@ import ( "encoding/pem" "errors" "fmt" + "github.com/novox/mesh-controller/internal/envfile" "os" "strings" ) @@ -40,7 +41,10 @@ var ErrNotConfigured = errors.New("this control plane has not been told about it // FromEnvironment reads the two settings, if they are there. func FromEnvironment() (Broker, error) { - address := strings.TrimSpace(os.Getenv(AddressVar)) + address, err := envfile.Value(AddressVar) + if err != nil { + return Broker{}, err + } path := strings.TrimSpace(os.Getenv(CertificateVar)) if address == "" && path == "" { diff --git a/internal/broker/management.go b/internal/broker/management.go index 684d0a1..cea2a44 100644 --- a/internal/broker/management.go +++ b/internal/broker/management.go @@ -5,10 +5,10 @@ import ( "context" "encoding/json" "fmt" + "github.com/novox/mesh-controller/internal/envfile" "io" "net/http" "net/url" - "os" "regexp" "strings" "time" @@ -42,7 +42,10 @@ type Management struct { // ManagementFromEnvironment reads where the management API is, if it is configured. func ManagementFromEnvironment() (*Management, error) { - raw := strings.TrimSpace(os.Getenv(ManagementVar)) + raw, err := envfile.Value(ManagementVar) + if err != nil { + return nil, err + } if raw == "" { return nil, ErrNotConfigured } diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index c6cf0f3..616908c 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -442,6 +442,10 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { // And the machine underneath, which no binding of its own can tell it. thisMachine := machineFacts(r) + // Which of this module's files carry a secret, for the rule that a container may not read + // one of them as its environment without saying so (ADR 0086, issue 041). + secretFiles := secretFilesOf(resources) + for _, unsettled := range resources { resource, err := ApplySettings(unsettled, with.Settings[m.Module]) if err != nil { @@ -451,6 +455,12 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { for k, v := range resource { copied[k] = v } + if err := refuseSecretsInEnvironment(copied, secretFiles, m.Module); err != nil { + return nil, err + } + // Said in the catalogue, not on the machine: the host parses strictly and knows no + // such field, and the reason is for a reader of the manifest. + delete(copied, SecretsInEnvironment) // **After settings, and that is the whole reason it is here.** A module's file // content is where a setting lands, so a placeholder may only exist once the setting // has been put in — filling secrets first would look at content that is not yet what diff --git a/internal/catalogue/secrets_in_environment_test.go b/internal/catalogue/secrets_in_environment_test.go new file mode 100644 index 0000000..9bbde92 --- /dev/null +++ b/internal/catalogue/secrets_in_environment_test.go @@ -0,0 +1,72 @@ +package catalogue + +import ( + "strings" + "testing" +) + +func aModuleWithAnEnvFileSecret(exception string) Manifest { + container := map[string]any{ + "id": "server", "type": "container", "name": "app", "image": "app@sha256:" + strings.Repeat("a", 64), + "env-file": []any{"/var/lib/app/server.env"}, + } + if exception != "" { + container[SecretsInEnvironment] = exception + } + return Manifest{ + Module: "app", Version: "1", + OwnSecrets: map[string]string{"token": "/var/lib/app/token.secret"}, + Resources: []map[string]any{ + {"id": "env", "type": "file", "path": "/var/lib/app/server.env", "mode": "0600", + "content": "APP_TOKEN=${secret:token}\n"}, + container, + }, + } +} + +func declare(t *testing.T, m Manifest) ([]map[string]any, error) { + t.Helper() + got, err := Resolve(shelf(m), []string{m.Module}, + Node{Name: "anchor", At: "10.0.0.1", Capabilities: map[string]bool{}}, World{}) + if err != nil { + t.Fatal(err) + } + return got.Declaration(Rendering{Needed: map[string]map[string]string{"app": {"token": "SEALED"}}}) +} + +// A container that reads a file carrying a secret as its environment is refused, unless it says +// why — and then the reason stays in the catalogue and never reaches the machine. +func TestASecretInAnEnvFileIsRefusedUnlessDeclared(t *testing.T) { + _, err := declare(t, aModuleWithAnEnvFileSecret("")) + if err == nil || !strings.Contains(err.Error(), "04-ISSUES/041") { + t.Fatalf("an env-file carrying a secret was declared without a word: %v", err) + } + + out, err := declare(t, aModuleWithAnEnvFileSecret("the image reads its configuration from the environment only")) + if err != nil { + t.Fatal(err) + } + for _, r := range out { + if _, leaked := r[SecretsInEnvironment]; leaked { + t.Fatalf("the catalogue's reason was sent to the machine: %v", r) + } + } +} + +// A secret placeholder inside a container's env is refused outright: nothing fills it there. +func TestASecretInEnvIsAlwaysRefused(t *testing.T) { + m := aModuleWithAnEnvFileSecret("said") + m.Resources[1]["env"] = map[string]any{"APP_TOKEN": "${secret:token}"} + if _, err := declare(t, m); err == nil || !strings.Contains(err.Error(), "never filled into an environment variable") { + t.Fatalf("a secret in env was accepted: %v", err) + } +} + +// A file with no secret in it is an ordinary env-file and needs no word. +func TestAnEnvFileWithoutASecretNeedsNothing(t *testing.T) { + m := aModuleWithAnEnvFileSecret("") + m.Resources[0]["content"] = "APP_MODE=production\n" + if _, err := declare(t, m); err != nil { + t.Fatal(err) + } +} diff --git a/internal/catalogue/secrets_into_files.go b/internal/catalogue/secrets_into_files.go index 07ae2c0..f47656c 100644 --- a/internal/catalogue/secrets_into_files.go +++ b/internal/catalogue/secrets_into_files.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" "sort" + "strings" ) // A credential and a configuration file meeting. @@ -83,6 +84,70 @@ func sealedFor(m Manifest, needs []Needed, with Rendering) (map[string]string, e return sealed, nil } +// SecretsInEnvironment is the key a container carries to say, out loud, that a secret reaches +// its process through the environment — and why. +// +// **A secret reaches a process as a file** (novox/hq ADR 0086). The mesh seals a value to the +// machine and the host writes it at 0600; a container that then reads it from an env-file hands +// it to the runtime, which prints it in `docker inspect` and keeps it in the process's /proc entry +// for anything on the machine that can talk to the runtime (novox/hq 04-ISSUES/041). Some software +// reads its configuration from the environment and nothing else, and for that this key exists: a +// reason, on the container, so a reader can tell from the manifest which secrets are exposed that +// way and which are not. Without it, a container whose env-file carries a secret is refused. +// +// Catalogue-level: the host never sees this key. +const SecretsInEnvironment = "secrets-in-environment" + +// refuseSecretsInEnvironment is the check. `secretFiles` is every file of this module whose +// content names a secret. +func refuseSecretsInEnvironment(resource map[string]any, secretFiles map[string]bool, module string) error { + if fmt.Sprint(resource["type"]) != "container" { + return nil + } + name := fmt.Sprint(resource["name"]) + if env, ok := resource["env"].(map[string]any); ok { + for key, value := range env { + if strings.Contains(fmt.Sprint(value), "${secret:") { + return fmt.Errorf( + "%s's container %s puts ${secret:…} in its env (%s). A secret is never filled into an "+ + "environment variable: put it in a file the module declares and mount that, or name the "+ + "file in env-file and say %q why", module, name, key, SecretsInEnvironment) + } + } + } + reason, _ := resource[SecretsInEnvironment].(string) + if strings.TrimSpace(reason) != "" { + return nil + } + if files, ok := resource["env-file"].([]any); ok { + for _, f := range files { + if secretFiles[fmt.Sprint(f)] { + return fmt.Errorf( + "%s's container %s reads %s as an env-file, and that file carries a secret, so the secret "+ + "reaches the process environment — readable in `docker inspect` and /proc (novox/hq "+ + "04-ISSUES/041). Mount the secret's file and point the program at it, or, if the program "+ + "reads only its environment, say so on the container: %q: \"\"", + module, name, f, SecretsInEnvironment) + } + } + } + return nil +} + +// secretFilesOf is the paths of a module's file resources whose content names a secret. +func secretFilesOf(resources []map[string]any) map[string]bool { + out := map[string]bool{} + for _, r := range resources { + if fmt.Sprint(r["type"]) != "file" { + continue + } + if content, ok := r["content"].(string); ok && len(secretsUsed(content)) > 0 { + out[fmt.Sprint(r["path"])] = true + } + } + return out +} + // intoFile gives a file the sealed values its content asks for. // // A name the module never declared is refused here rather than on the machine. The host would diff --git a/internal/envfile/envfile.go b/internal/envfile/envfile.go new file mode 100644 index 0000000..d408b54 --- /dev/null +++ b/internal/envfile/envfile.go @@ -0,0 +1,39 @@ +// Package envfile reads a setting that may be a secret from the environment or, preferably, from +// a file the environment names. +// +// **A secret reaches a process as a file** (novox/hq ADR 0086). An environment variable is +// readable in `docker inspect`, in the process's /proc entry and in whatever composed it; a file +// the mesh sealed to the machine and the host wrote at 0600 is readable where it is used and +// nowhere else. So every variable of the control plane's that carries a credential has a `_FILE` +// twin naming such a file, and the plain form remains only for a control plane a person starts by +// hand and for the bundle that raises the first one. The store's connections had this shape +// already (internal/store); this is the same rule for the rest. +package envfile + +import ( + "fmt" + "os" + "strings" +) + +// Value is the setting named by `name`: the content of the file `name_FILE` points at when that +// is set, else the variable itself. Both set is refused — two sources that could disagree is how +// a setting silently stops meaning what it says. Neither set is "", nil. +func Value(name string) (string, error) { + plain, hasPlain := os.LookupEnv(name) + path, hasFile := os.LookupEnv(name + "_FILE") + switch { + case hasFile && hasPlain && strings.TrimSpace(plain) != "" && strings.TrimSpace(path) != "": + return "", fmt.Errorf("both %s and %s_FILE are set; one of them, not both", name, name) + case hasFile && strings.TrimSpace(path) != "": + raw, err := os.ReadFile(strings.TrimSpace(path)) + if err != nil { + return "", fmt.Errorf("%s_FILE names %s, which cannot be read: %w", name, path, err) + } + // A file has a line ending and a value does not — trimmed, and only the ending, because a + // value may begin or end with a space and still be the value. + return strings.TrimRight(string(raw), "\r\n"), nil + default: + return strings.TrimSpace(plain), nil + } +} diff --git a/internal/envfile/envfile_test.go b/internal/envfile/envfile_test.go new file mode 100644 index 0000000..f8c04e4 --- /dev/null +++ b/internal/envfile/envfile_test.go @@ -0,0 +1,44 @@ +package envfile + +import ( + "os" + "path/filepath" + "testing" +) + +func TestAFileTwinIsPreferredAndOnlyItsLineEndingGoes(t *testing.T) { + path := filepath.Join(t.TempDir(), "value") + if err := os.WriteFile(path, []byte(" amqp://u:p@h/ \n"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("MESH_X", "") + t.Setenv("MESH_X_FILE", path) + got, err := Value("MESH_X") + if err != nil || got != " amqp://u:p@h/ " { + t.Fatalf("got %q, %v", got, err) + } +} + +func TestThePlainVariableStillWorks(t *testing.T) { + t.Setenv("MESH_Y", " plain ") + if got, err := Value("MESH_Y"); err != nil || got != "plain" { + t.Fatalf("got %q, %v", got, err) + } +} + +func TestBothSetIsRefused(t *testing.T) { + path := filepath.Join(t.TempDir(), "value") + _ = os.WriteFile(path, []byte("a"), 0o600) + t.Setenv("MESH_Z", "b") + t.Setenv("MESH_Z_FILE", path) + if _, err := Value("MESH_Z"); err == nil { + t.Fatal("two sources were accepted") + } +} + +func TestAMissingFileIsSaid(t *testing.T) { + t.Setenv("MESH_W_FILE", filepath.Join(t.TempDir(), "absent")) + if _, err := Value("MESH_W"); err == nil { + t.Fatal("a missing file produced a value") + } +} diff --git a/internal/link/serve.go b/internal/link/serve.go index 2d402e7..f0a8ec3 100644 --- a/internal/link/serve.go +++ b/internal/link/serve.go @@ -5,9 +5,9 @@ import ( "encoding/json" "errors" "fmt" + "github.com/novox/mesh-controller/internal/envfile" "log" "os" - "strings" "time" amqp "github.com/rabbitmq/amqp091-go" @@ -107,7 +107,10 @@ func (s *Server) Answers(r Replayer) error { // Connect opens the control plane's own connection to the broker. func Connect(enroller Enroller, listener Listener) (*Server, error) { - url := strings.TrimSpace(os.Getenv(AMQPVar)) + url, err := envfile.Value(AMQPVar) + if err != nil { + return nil, err + } if url == "" { return nil, fmt.Errorf( "this control plane has no %s, so it cannot reach its broker. Nodes talk to it over "+ -- 2.54.0 From 69bb0fcb6754075589c497b8d1bb10c4857c7a9b Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 10:19:00 +0200 Subject: [PATCH 2/3] A module names who its secret files belong to (secrets-owner) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The control plane runs as 65534 and crash-looped on permission denied the first time its credentials were mounted as files the host wrote as root at 0600 — the env-file shape hid this because the daemon reads an env-file on the host side. The composer now gives a module's secret files the owner the manifest names. --- internal/catalogue/declaration.go | 16 +++++++--- internal/catalogue/manifest.go | 12 +++++++ .../catalogue/secrets_in_environment_test.go | 31 +++++++++++++++++++ 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 616908c..f06e91e 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -271,9 +271,9 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { return nil, fmt.Errorf( "%s needs a secret called %q and none was made for it", m.Module, name) } - first = append(first, map[string]any{ + first = append(first, ownedBy(m.SecretsOwner, map[string]any{ "id": NeedID(name), "type": "file", "path": m.OwnSecrets[name], "sealed": sealed, - }) + })) } // Operator-owned paths this module is granted use of (novox/hq ADR 0051). Written before // the module's own resources, and so before the container that mounts them: the host must @@ -320,10 +320,10 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { // worse than none: something would read it and fail authenticating. continue } - first = append(first, map[string]any{ + first = append(first, ownedBy(m.SecretsOwner, map[string]any{ "id": SecretID(to), "type": "file", "path": m.Secrets[to], "sealed": found.Sealed, - }) + })) } for _, to := range sortedKeys(m.Grants) { for _, g := range with.Grants { @@ -799,6 +799,14 @@ func keptFile(dir string, kept *KeptExport) (map[string]any, error) { }, nil } +// ownedBy gives a secret file the owner the module named, when it named one (Manifest.SecretsOwner). +func ownedBy(owner string, file map[string]any) map[string]any { + if owner != "" { + file["owner"] = owner + } + return file +} + // sortedKeys is map iteration made repeatable, which everything written to a machine needs. func sortedKeys[V any](m map[string]V) []string { out := make([]string, 0, len(m)) diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 638fe86..d2096cb 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -297,6 +297,18 @@ type Manifest struct { // module, in a file anybody can read, for ever. OwnSecrets map[string]string `json:"own-secrets,omitempty"` + // SecretsOwner is who the files holding this module's secrets belong to on the machine — + // `uid:gid`, or a name — when its process is not root. + // + // **A secret reaches a process as a file** (novox/hq ADR 0086), and a file the host writes at + // 0600 as root is a file a container running as another account cannot read: the control + // plane, `USER 65534` in a scratch image, crash-looped on `permission denied` the first time + // its credentials were mounted instead of read from an env-file (which the daemon reads, as + // root, on the host side — which is exactly why that shape hid the problem). Absent means + // root, which is what a process that runs as root needs and what a process that does not + // cannot use. + SecretsOwner string `json:"secrets-owner,omitempty"` + // Keeps is where this module wants every operator-sealed secret in the mesh written — the // vault's field, and so far nobody else's (novox/hq ADR 0085, amended). // diff --git a/internal/catalogue/secrets_in_environment_test.go b/internal/catalogue/secrets_in_environment_test.go index 9bbde92..da911b4 100644 --- a/internal/catalogue/secrets_in_environment_test.go +++ b/internal/catalogue/secrets_in_environment_test.go @@ -70,3 +70,34 @@ func TestAnEnvFileWithoutASecretNeedsNothing(t *testing.T) { t.Fatal(err) } } + +// A module whose process is not root names who its secret files belong to, and every secret file +// the composer writes for it carries that owner — the mounted file is readable where it is used. +func TestSecretFilesCarryTheOwnerTheModuleNamed(t *testing.T) { + m := aModuleWithAnEnvFileSecret("said") + m.SecretsOwner = "65534:65534" + out, err := declare(t, m) + if err != nil { + t.Fatal(err) + } + var seen bool + for _, r := range out { + if r["path"] != "/var/lib/app/token.secret" { + continue + } + seen = true + if r["owner"] != "65534:65534" { + t.Errorf("the secret file is owned by %v", r["owner"]) + } + } + if !seen { + t.Fatal("no secret file in the declaration") + } + m.SecretsOwner = "" + out, _ = declare(t, m) + for _, r := range out { + if r["path"] == "/var/lib/app/token.secret" && r["owner"] != nil { + t.Errorf("an owner was invented: %v", r["owner"]) + } + } +} -- 2.54.0 From 67de21616091092de97830a377a03902c3e7cea7 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 10:33:22 +0200 Subject: [PATCH 3/3] The controller's own manifest reads its credentials from files too The mesh-controller repository carries the manifest the mesh builds the controller from; the catalogue's copy is what genesis registers. The two must say the same thing, and the first rebuild from source proved they did not. --- module.json | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/module.json b/module.json index 7d55ffc..3b80760 100644 --- a/module.json +++ b/module.json @@ -19,6 +19,7 @@ "broker-management": "/var/lib/mesh/mesh-controller/broker-management", "broker-address": "/var/lib/mesh/mesh-controller/broker-address" }, + "secrets-owner": "65534:65534", "resources": [ { "id": "mesh-state", @@ -26,13 +27,6 @@ "path": "/var/lib/mesh/mesh-controller", "mode": "0700" }, - { - "id": "control-env", - "type": "file", - "path": "/var/lib/mesh/mesh-controller/control.env", - "mode": "0600", - "content": "MESH_STORE_INVENTORY=${secret:inventory}\nMESH_STORE_IDENTITY=${secret:identity}\nMESH_STORE_LICENCES=${secret:licences}\nMESH_BROKER_AMQP=${secret:broker}\nMESH_BROKER_MANAGEMENT=${secret:broker-management}\nMESH_BROKER_ADDRESS=${secret:broker-address}\n" - }, { "id": "server", "type": "container", @@ -41,19 +35,28 @@ "args": [ "serve" ], - "env-file": [ - "/var/lib/mesh/mesh-controller/control.env" - ], "env": { - "MESH_BROKER_CERTIFICATE": "/broker-tls/tls.crt" + "MESH_BROKER_CERTIFICATE": "/broker-tls/tls.crt", + "MESH_STORE_INVENTORY_FILE": "/run/secrets/inventory", + "MESH_STORE_IDENTITY_FILE": "/run/secrets/identity", + "MESH_STORE_LICENCES_FILE": "/run/secrets/licences", + "MESH_BROKER_AMQP_FILE": "/run/secrets/broker", + "MESH_BROKER_MANAGEMENT_FILE": "/run/secrets/broker-management", + "MESH_BROKER_ADDRESS_FILE": "/run/secrets/broker-address" }, "volumes": [ - "mesh-broker-tls:/broker-tls:ro" + "mesh-broker-tls:/broker-tls:ro", + "/var/lib/mesh/mesh-controller/inventory:/run/secrets/inventory:ro", + "/var/lib/mesh/mesh-controller/identity:/run/secrets/identity:ro", + "/var/lib/mesh/mesh-controller/licences:/run/secrets/licences:ro", + "/var/lib/mesh/mesh-controller/broker:/run/secrets/broker:ro", + "/var/lib/mesh/mesh-controller/broker-management:/run/secrets/broker-management:ro", + "/var/lib/mesh/mesh-controller/broker-address:/run/secrets/broker-address:ro" ], + "artifact": "server", "restart-on": [ "control-env" - ], - "artifact": "server" + ] } ], "build": { -- 2.54.0