Let an own secret name its owner (issue 1170)

internal/catalogue/declaration.go

@@ -680,7 +680,13 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 			if !ok || family == "" || with.Needed[m.Module][name] == "" {
 				continue
 			}
-			first = append(first, ownedBy(m.SecretsOwner, map[string]any{
+			// A per-secret owner (novox/hq ADR 0436 §1) stands in for the module's secrets-owner, when
+			// the manifest named one for this secret (or the family it belongs to).
+			owner := m.SecretsOwner
+			if own.Owner != "" {
+				owner = own.Owner
+			}
+			first = append(first, ownedBy(owner, map[string]any{
 				"id": NeedID(name), "type": "file", "path": own.Path, "sealed": with.Needed[m.Module][name],
 			}))
 		}
@@ -694,6 +700,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 				if m.Module == RuntimeModule && r.Account != "" {
 					owner = r.Account
 				}
+				if o := m.OwnSecrets[name].Owner; o != "" {
+					owner = o
+				}
 				first = append(first, ownedBy(owner, map[string]any{
 					"id": NeedID(name), "type": "file", "path": m.OwnSecrets[name].Path, HandedSecretField: name,
 				}))
@@ -709,6 +718,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 				if m.Module == RuntimeModule && r.Account != "" {
 					owner = r.Account
 				}
+				if o := m.OwnSecrets[name].Owner; o != "" {
+					owner = o
+				}
 				first = append(first, ownedBy(owner, map[string]any{
 					"id": NeedID(name), "type": "file", "path": m.OwnSecrets[name].Path, VaultSealedField: box,
 				}))
@@ -742,6 +754,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 			if m.Module == RuntimeModule && r.Account != "" {
 				owner = r.Account
 			}
+			if o := m.OwnSecrets[name].Owner; o != "" {
+				owner = o
+			}
 			first = append(first, ownedBy(owner, map[string]any{
 				"id": NeedID(name), "type": "file", "path": m.OwnSecrets[name].Path, "sealed": sealed,
 			}))

internal/catalogue/dir_into.go

@@ -168,7 +168,7 @@ func placedManifest(m Manifest, with Rendering) (Manifest, error) {
 			if err != nil {
 				return m, err
 			}
-			own[name] = OwnSecret{Path: filled, Taken: s.Taken, IssuedBy: s.IssuedBy}
+			own[name] = OwnSecret{Path: filled, Taken: s.Taken, IssuedBy: s.IssuedBy, Prompt: s.Prompt, Owner: s.Owner}
 		}
 		m.OwnSecrets = own
 	}

internal/catalogue/manifest.go

@@ -2348,6 +2348,7 @@ func parseManifest(raw []byte, predefined PredefinedList) (Manifest, error) {
 				m.Module, name, own.IssuedBy, IssuedOutside))
 		}
 		problems = append(problems, ownSecretPromptProblems(m, name, own.Prompt, IsFamily(name))...)
+		problems = append(problems, ownSecretOwnerProblems(m, name, own.Owner)...)
 	}
 	localOf := map[string]string{}
 	for _, to := range m.SecretRequirements() {
@@ -2930,6 +2931,11 @@ type OwnSecret struct {
 	// family's member suffix, filled where the secret is asked for. Optional; reviewed in the
 	// module's pull request like the rest of the manifest.
 	Prompt string
+	// Owner is the account this secret's files belong to on the machine, in place of the module's
+	// secrets-owner (novox/hq ADR 0436 §1): an account the module makes (a `user` resource of that
+	// name), never root and never the operator account. Empty means the module's secrets-owner, or
+	// root where that is absent too.
+	Owner string
 }
 
 // MeshMayMake says the mesh may put a value it makes in place of the one the secret holds: the
@@ -2984,6 +2990,34 @@ func ownSecretPromptProblems(m Manifest, name, prompt string, family bool) []str
 	return problems
 }
 
+// ownSecretOwnerProblems is what is wrong with an own secret's declared owner (novox/hq ADR 0436 §1): naming
+// an account the module makes — a `user` resource of that name — never root, which a secret file owned by it
+// would make readable by anything, and never the operator account's placeholder (`${machine:account}`), which
+// the mesh fills per machine and a manifest cannot say safely (novox/hq ADR 0436, settled by the lead).
+func ownSecretOwnerProblems(m Manifest, name, owner string) []string {
+	if owner == "" {
+		return nil
+	}
+	if owner == "root" {
+		return []string{fmt.Sprintf(
+			"%s names %q as the owner of its secret %q; an own secret's owner is an account the module "+
+				"makes, never root (novox/hq ADR 0436 §1)", m.Module, owner, name)}
+	}
+	if owner == "${machine:account}" {
+		return []string{fmt.Sprintf(
+			"%s names %q as the owner of its secret %q; an own secret's owner is an account the module "+
+				"makes, never the operator account (novox/hq ADR 0436 §1)", m.Module, owner, name)}
+	}
+	for _, r := range m.Resources {
+		if fmt.Sprint(r["type"]) == "user" && fmt.Sprint(r["name"]) == owner {
+			return nil
+		}
+	}
+	return []string{fmt.Sprintf(
+		"%s names %q as the owner of its secret %q, which it does not make: a user resource named %q "+
+			"(novox/hq ADR 0436 §1)", m.Module, owner, name, owner)}
+}
+
 // OwnSecrets is a module's own secrets by name. On the wire each is a path, or an object naming
 // the path and how it is taken; written back the way it was read, so a manifest the mesh holds
 // keeps its bytes.
@@ -3006,13 +3040,14 @@ func (o *OwnSecrets) UnmarshalJSON(raw []byte) error {
 			Taken    string `json:"taken,omitempty"`
 			IssuedBy string `json:"issued-by,omitempty"`
 			Prompt   string `json:"prompt,omitempty"`
+			Owner    string `json:"owner,omitempty"`
 		}
 		dec := json.NewDecoder(bytes.NewReader(body))
 		dec.DisallowUnknownFields()
 		if err := dec.Decode(&long); err != nil {
-			return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\", \"issued-by\", \"prompt\"}: %w", name, typedUnknown(err))
+			return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\", \"issued-by\", \"prompt\", \"owner\"}: %w", name, typedUnknown(err))
 		}
-		out[name] = OwnSecret{Path: long.Path, Taken: long.Taken, IssuedBy: long.IssuedBy, Prompt: long.Prompt}
+		out[name] = OwnSecret{Path: long.Path, Taken: long.Taken, IssuedBy: long.IssuedBy, Prompt: long.Prompt, Owner: long.Owner}
 	}
 	*o = out
 	return nil
@@ -3021,7 +3056,7 @@ func (o *OwnSecrets) UnmarshalJSON(raw []byte) error {
 func (o OwnSecrets) MarshalJSON() ([]byte, error) {
 	entries := make(map[string]any, len(o))
 	for name, s := range o {
-		if s.Taken == "" && s.IssuedBy == "" && s.Prompt == "" {
+		if s.Taken == "" && s.IssuedBy == "" && s.Prompt == "" && s.Owner == "" {
 			entries[name] = s.Path
 			continue
 		}
@@ -3035,6 +3070,9 @@ func (o OwnSecrets) MarshalJSON() ([]byte, error) {
 		if s.Prompt != "" {
 			long["prompt"] = s.Prompt
 		}
+		if s.Owner != "" {
+			long["owner"] = s.Owner
+		}
 		entries[name] = long
 	}
 	return json.Marshal(entries)
@@ -3074,7 +3112,7 @@ func (o OwnSecrets) Lookup(name string) (s OwnSecret, family string, ok bool) {
 		}
 		if family == "" || len(declared) > len(family) {
 			s, family, ok = OwnSecret{Path: strings.Replace(f.Path, "*", rest, 1), Taken: f.Taken, IssuedBy: f.IssuedBy,
-				Prompt: f.Prompt}, declared, true
+				Prompt: f.Prompt, Owner: f.Owner}, declared, true
 		}
 	}
 	return s, family, ok

internal/catalogue/own_secret_owner_test.go

@@ -0,0 +1,114 @@
+package catalogue
+
+import (
+	"strings"
+	"testing"
+)
+
+// An own secret (or a family) may declare `owner` (novox/hq ADR 0436 §1): the account the files holding it
+// belong to on the machine, when that account is not the module's secrets-owner default. The fixture is
+// mesh-mail's future manifest shape (ADR 0436 §7): a process account `mesh-mail-send` the module makes, and its
+// own-secret family `mail-credential-*` owned by it.
+
+// mailSenderManifest is the fixture every mail-sender issue builds on: the real parts of mesh-mail's manifest
+// taken from mesh-catalog/modules/mesh-mail/module.json, extended as ADR 0436 §7-§8 describe, with `own` naming
+// the account that owns the family's files.
+func mailSenderManifest(t *testing.T, own string) Manifest {
+	t.Helper()
+	ownSecrets := `"mail-credential-*":{"path":"/var/lib/mesh-mail/mail-credential-*.secret","issued-by":"outside"`
+	if own != "" {
+		ownSecrets += `,"owner":"` + own + `"`
+	}
+	ownSecrets += `}`
+	m, err := ParseManifest([]byte(`{"module":"mesh-mail","version":"1","secrets-owner":"mesh-mail","own-secrets":{` +
+		ownSecrets + `},"resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`))
+	if err != nil {
+		t.Fatalf("a manifest with own=%q was refused: %v", own, err)
+	}
+	return m
+}
+
+// 1170/AC1: a family owned by the module's own account lands at 0600, owned by that account — and a module's
+// other own secrets (no per-secret owner declared) keep secrets-owner, unchanged.
+func TestAnOwnSecretsOwnerIsGivenToItsFiles(t *testing.T) {
+	m := mailSenderManifest(t, "mesh-mail-send")
+	m.OwnSecrets["smtp-relay-key"] = OwnSecret{Path: "/var/lib/mesh-mail/smtp-relay-key", Taken: TakenAtStart}
+	r := Resolution{Node: "novox", Modules: []Manifest{m}}
+	out, err := r.Declaration(Rendering{
+		Needed: map[string]map[string]string{"mesh-mail": {"mail-credential-smtp": "sealed-credential", "smtp-relay-key": "sealed-relay"}},
+	})
+	if err != nil {
+		t.Fatalf("composing the declaration: %v", err)
+	}
+	var family, plain map[string]any
+	for _, res := range out {
+		if res["type"] != "file" {
+			continue
+		}
+		switch res["path"] {
+		case "/var/lib/mesh-mail/mail-credential-smtp.secret":
+			family = res
+		case "/var/lib/mesh-mail/smtp-relay-key":
+			plain = res
+		}
+	}
+	if family == nil {
+		t.Fatalf("the family member was not composed at all: %v", out)
+	}
+	if got := family["owner"]; got != "mesh-mail-send" {
+		t.Fatalf("the family member's owner: %v; want mesh-mail-send", got)
+	}
+	if got := family["mode"]; got != "" && got != nil {
+		// mode is not set by ownedBy; the host applies 0600 to every secret file regardless (ADR 0086). Still,
+		// a mode this test sees must not have been changed by the owner clause.
+		t.Logf("family mode: %v", got)
+	}
+	if plain == nil {
+		t.Fatalf("the module's other own secret was not composed at all: %v", out)
+	}
+	if got := plain["owner"]; got != "mesh-mail" {
+		t.Fatalf("a secret with no per-secret owner kept %v; want the module's secrets-owner %q", got, "mesh-mail")
+	}
+}
+
+// 1170/AC2: an owner naming an account the module does not make, root, or the operator account's placeholder is
+// refused, naming the secret and the account.
+func TestAnOwnSecretsOwnerIsRefusedWhenItIsNotAnAccountTheModuleMakes(t *testing.T) {
+	for name, c := range map[string]struct{ owner, says string }{
+		"root":                    {"root", "root"},
+		"the operator account":    {"${machine:account}", "operator account"},
+		"an account not declared": {"not-made", "not-made"},
+	} {
+		_, err := ParseManifest([]byte(`{"module":"mesh-mail","version":"1","own-secrets":{` +
+			`"mail-credential-*":{"path":"/var/lib/mesh-mail/mail-credential-*.secret","issued-by":"outside","owner":"` +
+			c.owner + `"}},"resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`))
+		if err == nil || !strings.Contains(err.Error(), "mail-credential-*") || !strings.Contains(err.Error(), c.says) {
+			t.Errorf("%s: %v; want a refusal naming the secret and %q", name, err, c.says)
+		}
+	}
+}
+
+// 1170/AC3: a manifest with no owner on any own secret composes exactly as it did before this clause existed:
+// every secret file belongs to the module's secrets-owner (or nothing, when that is absent too).
+func TestNoOwnerOnAnySecretComposesAsMainDoes(t *testing.T) {
+	m := mailSenderManifest(t, "")
+	r := Resolution{Node: "novox", Modules: []Manifest{m}}
+	out, err := r.Declaration(Rendering{
+		Needed: map[string]map[string]string{"mesh-mail": {"mail-credential-smtp": "sealed-credential"}},
+	})
+	if err != nil {
+		t.Fatalf("composing the declaration: %v", err)
+	}
+	var found bool
+	for _, res := range out {
+		if res["type"] == "file" && res["path"] == "/var/lib/mesh-mail/mail-credential-smtp.secret" {
+			found = true
+			if got := res["owner"]; got != "mesh-mail" {
+				t.Fatalf("with no per-secret owner declared, the file's owner: %v; want secrets-owner %q", got, "mesh-mail")
+			}
+		}
+	}
+	if !found {
+		t.Fatalf("the family member was not composed at all: %v", out)
+	}
+}