mesh-mail: move password reading out of the tools' reach into its own package, so only keep and the send actor import it (ADR 0436 clause 12; issue 1193)

modules/mesh-mail/cmd/mesh-mail/config.go

@@ -159,33 +159,17 @@ func (s Server) check(which string) error {
 	return nil
 }
 
-// SecretPath is where the account's own secret is placed in the secrets directory, as the manifest declares the
-// family: <dir>/mail-credential-<account>.secret.
-func SecretPath(dir string, a Account) string { return filepath.Join(dir, a.Secret()+".secret") }
-
-// ReadSecret is the account's password, or given false when nobody gave it yet: the mesh never makes a member of the
-// family, so a member not given is no file (ADR 0283 §6).
-func ReadSecret(dir string, a Account) (string, bool, error) {
-	raw, err := os.ReadFile(SecretPath(dir, a))
-	if errors.Is(err, os.ErrNotExist) {
-		return "", false, nil
-	}
-	if err != nil {
-		return "", false, err
-	}
-	pw := strings.TrimRight(string(raw), "\r\n")
-	if pw == "" {
-		return "", false, nil
-	}
-	return pw, true, nil
-}
+// ReadSecret and SecretPath moved into cmd/mesh-mail/internal/secret (novox/hq ADR 0436 §12, issue 1193): only
+// keep (imap.go) and the sender's own actor (sendactor.go) read a password, never the tools, which the operator
+// account's tool runner serves and which must never open a password file.
 
 // SecretGiven says whether the account's own secret was given.
 //
 // It looks at the file and never reads it: whoever judges whether a password was given has no need of the password.
-// A file that is there and not empty was given; whether it signs in is the keeper's to say.
+// A file that is there and not empty was given; whether it signs in is the keeper's to say. It computes the
+// secret's path itself (not through the moved package) so the tools, which call this, never import it.
 func SecretGiven(dir string, a Account) bool {
-	info, err := os.Stat(SecretPath(dir, a))
+	info, err := os.Stat(filepath.Join(dir, a.Secret()+".secret"))
 	return err == nil && info.Mode().IsRegular() && info.Size() > 0
 }
 

modules/mesh-mail/cmd/mesh-mail/config_test.go

@@ -6,6 +6,8 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+
+	"mesh-mail/cmd/mesh-mail/internal/secret"
 )
 
 // The first stage's account, as the operator sets the setting `accounts` (ADR 0359 §5): its address and its server,
@@ -22,7 +24,7 @@ func TestTheFirstStagesAccountIsReadAndItsSecretNamed(t *testing.T) {
 		got[0].Incoming.Address() != "mail.novox.be:993" || got[0].Secret() != "mail-credential-mesh" || got[0].Outgoing != nil {
 		t.Fatalf("%+v", got)
 	}
-	if p := SecretPath("/state", got[0]); p != "/state/mail-credential-mesh.secret" {
+	if p := secret.Path("/state", got[0].Secret()); p != "/state/mail-credential-mesh.secret" {
 		t.Fatalf("the secret is placed at %s", p)
 	}
 }
@@ -99,16 +101,16 @@ func TestASecretIsGivenByItsFileNotByReadingIt(t *testing.T) {
 	if SecretGiven(dir, a) {
 		t.Fatal("given before anything was written")
 	}
-	_ = os.WriteFile(SecretPath(dir, a), nil, 0o600)
+	_ = os.WriteFile(secret.Path(dir, a.Secret()), nil, 0o600)
 	if SecretGiven(dir, a) {
 		t.Fatal("an empty file is a password")
 	}
-	_ = os.WriteFile(SecretPath(dir, a), []byte("pw\n"), 0o600)
-	if pw, given, _ := ReadSecret(dir, a); !given || pw != "pw" || strings.Contains(pw, "\n") {
+	_ = os.WriteFile(secret.Path(dir, a.Secret()), []byte("pw\n"), 0o600)
+	if pw, given, _ := secret.Read(dir, a.Secret()); !given || pw != "pw" || strings.Contains(pw, "\n") {
 		t.Fatalf("%q %v", pw, given)
 	}
 	// Unreadable to the judge, as a secret owned by root is to the watch: still given.
-	if err := os.Chmod(SecretPath(dir, a), 0); err != nil {
+	if err := os.Chmod(secret.Path(dir, a.Secret()), 0); err != nil {
 		t.Fatal(err)
 	}
 	if !SecretGiven(dir, a) {

modules/mesh-mail/cmd/mesh-mail/imap.go

@@ -22,6 +22,8 @@ import (
 
 	"github.com/emersion/go-imap/v2"
 	"github.com/emersion/go-imap/v2/imapclient"
+
+	"mesh-mail/cmd/mesh-mail/internal/secret"
 )
 
 // How the keeper paces itself. IdleCycle is how long one IDLE lasts before it is ended and begun again, which proves
@@ -170,7 +172,7 @@ func (k *Keeper) Watch(ctx context.Context, a Account) {
 	retry := orDefault(k.RetryFirst, RetryFirst)
 	var heard, synced time.Time
 	for ctx.Err() == nil {
-		pw, given, err := ReadSecret(k.SecretsDir, a)
+		pw, given, err := secret.Read(k.SecretsDir, a.Secret())
 		switch {
 		case err != nil:
 			k.say(ctx, a, AccountFailing, "its password was given and cannot be read: "+err.Error(), heard, synced)

modules/mesh-mail/cmd/mesh-mail/internal/secret/secret.go

@@ -0,0 +1,36 @@
+// Package secret reads an account's own password from the secrets directory (novox/hq ADR 0283 §6). It is
+// imported only by mesh-mail's keeper (which signs in to IMAP as root) and its sender (which signs in to SMTP
+// as its own account, mesh-mail-send): never by the tools, which the operator account's tool runner serves and
+// which must never open a password file (novox/hq ADR 0436 §12, issue 1193). Moved out of package main's own
+// files so a test (secret_boundary_test.go) can prove the tools' bundle (main.go's serve path, tools.go,
+// send.go, health.go) never imports it.
+package secret
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// Path is where an account's own secret is placed in the secrets directory, given the family member name
+// (mail-credential-<account>): <dir>/<member>.secret.
+func Path(dir, member string) string { return filepath.Join(dir, member+".secret") }
+
+// Read is the member's password, or given false when nobody gave it yet: the mesh never makes a member of the
+// family, so a member not given is no file (ADR 0283 §6). A directory that does not exist at all answers the
+// same as a file that does not exist: given false, no error (novox/hq issue 1193, AC3).
+func Read(dir, member string) (string, bool, error) {
+	raw, err := os.ReadFile(Path(dir, member))
+	if errors.Is(err, os.ErrNotExist) {
+		return "", false, nil
+	}
+	if err != nil {
+		return "", false, err
+	}
+	pw := strings.TrimRight(string(raw), "\r\n")
+	if pw == "" {
+		return "", false, nil
+	}
+	return pw, true, nil
+}

modules/mesh-mail/cmd/mesh-mail/kit_test.go

@@ -22,6 +22,8 @@ import (
 	"github.com/emersion/go-imap/v2/imapclient"
 	"github.com/emersion/go-imap/v2/imapserver"
 	"github.com/emersion/go-imap/v2/imapserver/imapmemserver"
+
+	"mesh-mail/cmd/mesh-mail/internal/secret"
 )
 
 func TestMain(m *testing.M) {
@@ -134,7 +136,7 @@ func testAccount() Account {
 
 func giveSecret(t *testing.T, dir string, a Account, pw string) {
 	t.Helper()
-	if err := os.WriteFile(SecretPath(dir, a), []byte(pw+"\n"), 0o600); err != nil {
+	if err := os.WriteFile(secret.Path(dir, a.Secret()), []byte(pw+"\n"), 0o600); err != nil {
 		t.Fatal(err)
 	}
 }

modules/mesh-mail/cmd/mesh-mail/secret_boundary_test.go

@@ -0,0 +1,120 @@
+package main
+
+// The tools never open a password file (novox/hq ADR 0436 §12, issue 1193): ReadSecret and SecretPath moved
+// into cmd/mesh-mail/internal/secret, imported only by keep (imap.go) and the sender's own actor (sendactor.go,
+// which — ahead of issue 1195's separate process — is the one piece of the Serve path still reached from
+// Decide's "send" case, exactly as 1194's own report still finds it: "Decide/decideSend call SendActor
+// directly"). The build seat runs every test under one account, so no permission test can show the split
+// (context file, "Clause 12's proof"); this is the file-level proxy the lead chose instead: an AST check over
+// the tools' own files.
+
+import (
+	"encoding/json"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// secretPackage is the import path the tools' bundle must never carry.
+const secretPackage = "mesh-mail/cmd/mesh-mail/internal/secret"
+
+// 1193/AC1: none of the tools' bundle files (main.go's serve path, tools.go, send.go, health.go) imports the
+// password-reading package; the failure names the offending file.
+func TestTheToolsBundleNeverImportsThePasswordPackage(t *testing.T) {
+	for _, file := range []string{"main.go", "tools.go", "send.go", "health.go"} {
+		fset := token.NewFileSet()
+		f, err := parser.ParseFile(fset, file, nil, parser.ImportsOnly)
+		if err != nil {
+			t.Fatalf("%s: %v", file, err)
+		}
+		for _, imp := range f.Imports {
+			path := strings.Trim(imp.Path.Value, `"`)
+			if path == secretPackage {
+				t.Errorf("%s imports %s, which only keep and the sender's actor may (novox/hq ADR 0436 §12)", file, secretPackage)
+			}
+		}
+	}
+}
+
+// 1193/AC2: keep (imap.go) and the sender's actor (sendactor.go) read the password through the moved package;
+// SecretGiven (stat only) stays in config.go, available to the tools, and never reads the password itself.
+func TestKeepAndTheSenderReadThePasswordThroughTheMovedPackage(t *testing.T) {
+	for _, file := range []string{"imap.go", "sendactor.go"} {
+		fset := token.NewFileSet()
+		f, err := parser.ParseFile(fset, file, nil, parser.ImportsOnly)
+		if err != nil {
+			t.Fatalf("%s: %v", file, err)
+		}
+		found := false
+		for _, imp := range f.Imports {
+			if strings.Trim(imp.Path.Value, `"`) == secretPackage {
+				found = true
+			}
+		}
+		if !found {
+			t.Errorf("%s does not import %s", file, secretPackage)
+		}
+	}
+	// SecretGiven itself (config.go) never calls ReadFile, os.Open or the moved package: whether a password was
+	// given is read from the file's presence and size alone (issue 733, kept by config_test.go's own check too).
+	src, err := os.ReadFile("config.go")
+	if err != nil {
+		t.Fatal(err)
+	}
+	body := string(src)
+	body = body[strings.Index(body, "func SecretGiven("):]
+	body = body[:strings.Index(body, "\n}\n")]
+	for _, bad := range []string{"ReadSecret", "ReadFile", "os.Open", "secret.Read"} {
+		if strings.Contains(body, bad) {
+			t.Fatalf("SecretGiven reads the password (%s):\n%s", bad, body)
+		}
+	}
+}
+
+// 1193/AC3: with the secrets directory absent, mail_send raises its ask, the warrant of an Approve is heard,
+// and mail_send_status reads it back — each succeeds, and no error mentions a secret file. (The outcome here
+// is "failed: ... has no password yet", because the tools still perform the send through sendactor.go until
+// issue 1194 replaces that path with queueing for the sender process; what this proves is narrower and holds
+// either way: nothing on this path ever names a secret file or path.)
+func TestTheToolPathSucceedsWithTheSecretsDirectoryAbsent(t *testing.T) {
+	dir := t.TempDir()
+	missing := filepath.Join(dir, "never-created")
+	env := Env{Accounts: filepath.Join(dir, "accounts.json"), Grants: filepath.Join(dir, "grants.json"), Secrets: missing}
+	writeFile(t, env.Accounts, `{"accounts": {"mesh": {"kind": "imap", "address": "jschoubben@novox.be",
+	  "incoming": {"host": "mail.novox.be", "port": 993, "security": "tls"},
+	  "outgoing": {"host": "h", "port": 465, "security": "tls"}}}}`)
+	writeFile(t, env.Grants, `{"grants": {"mesh": ["triage"]}}`)
+	router, sends := newSendRouter(), newMemSends()
+	b := &Bundle{Env: env, Now: func() time.Time { return judgedAt }, Asks: router, Sends: sends}
+	rt := startBundle(t, b.Tools())
+
+	text, isErr := rt.call(SendTool, "novox.triage", sendArgs)
+	if isErr {
+		t.Fatalf("mail_send: %s", text)
+	}
+	var out map[string]any
+	_ = json.Unmarshal([]byte(text), &out)
+	id := out["ask"].(string)
+
+	w := router.chose(id, OptionSend, judgedAt.Add(time.Minute))
+	router.close(w)
+	d, err := Decide(sends, b.lookupAccount, b.sendActor(), w, judgedAt.Add(2*time.Minute))
+	if err != nil {
+		t.Fatalf("the warrant was not heard cleanly: %v", err)
+	}
+	if strings.Contains(d.Outcome, missing) || strings.Contains(strings.ToLower(d.Outcome), "secret") {
+		t.Fatalf("the decision names a secret file: %q", d.Outcome)
+	}
+
+	text, isErr = rt.call(SendStatusTool, "novox.triage", map[string]any{"id": id})
+	if isErr {
+		t.Fatalf("mail_send_status: %s", text)
+	}
+	if strings.Contains(text, missing) || strings.Contains(strings.ToLower(text), "secret") {
+		t.Fatalf("mail_send_status names a secret file: %s", text)
+	}
+}

modules/mesh-mail/cmd/mesh-mail/sendactor.go

@@ -0,0 +1,34 @@
+package main
+
+// The one piece of the Serve path that still reads a password (novox/hq ADR 0436 §12, issue 1193): moved out
+// of tools.go so a test (secret_boundary_test.go) can prove the tools' own files — main.go's serve path,
+// tools.go, send.go, health.go — never import the password-reading package. Decide still calls this directly
+// (send.go's Heard and Sweep, tools.go's mail_send_status): issue 1194 replaces that with the tools queueing
+// the approved send for the sender process instead, at which point this file's job moves to it (issue 1195)
+// and this one is removed.
+
+import (
+	"fmt"
+	"time"
+
+	"mesh-mail/cmd/mesh-mail/internal/secret"
+)
+
+// sendActor performs one send on the operator's Approve: it reads the account's own secret itself, so Decide and
+// the `sends` state never hold a password.
+func (b *Bundle) sendActor() SendActor {
+	dial := b.Dial
+	if dial == nil {
+		dial = DialSMTP
+	}
+	return func(a Account, to []string, subject, body string, now time.Time) error {
+		pw, given, err := secret.Read(b.Env.Secrets, a.Secret())
+		if err != nil {
+			return fmt.Errorf("the account %s's password cannot be read: %w", a.Name, err)
+		}
+		if !given {
+			return fmt.Errorf("the account %s has no password yet; the operator gives it at the desk", a.Name)
+		}
+		return SendMail(dial, a, pw, to, subject, body, now)
+	}
+}

modules/mesh-mail/cmd/mesh-mail/tools.go

@@ -416,25 +416,6 @@ func accountByName(accounts []Account, name string) (Account, bool) {
 	return Account{}, false
 }
 
-// sendActor performs one send on the operator's Approve: it reads the account's own secret itself, so Decide and
-// the `sends` state never hold a password.
-func (b *Bundle) sendActor() SendActor {
-	dial := b.Dial
-	if dial == nil {
-		dial = DialSMTP
-	}
-	return func(a Account, to []string, subject, body string, now time.Time) error {
-		pw, given, err := ReadSecret(b.Env.Secrets, a)
-		if err != nil {
-			return fmt.Errorf("the account %s's password cannot be read: %w", a.Name, err)
-		}
-		if !given {
-			return fmt.Errorf("the account %s has no password yet; the operator gives it at the desk", a.Name)
-		}
-		return SendMail(dial, a, pw, to, subject, body, now)
-	}
-}
-
 func stringList(v any) []string {
 	list, _ := v.([]any)
 	out := make([]string, 0, len(list))