From c5705ed5517d59b594ca393353893887d3b22406 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:03:17 +0000 Subject: [PATCH 01/13] Move the key keeper into mesh-sdk and add the build modules' shared steps (issues 1469, 1470, 1471, 1473, 1474, 1479), part 1 of 4 --- go/forgekey/keykeeper.go | 127 +++++++++++++++++++++++++++++++++++++++ go/forgekey/keys.go | 110 +++++++++++++++++++++++++++++++++ go/forgekey/runner.go | 61 +++++++++++++++++++ 3 files changed, 298 insertions(+) create mode 100644 go/forgekey/keykeeper.go create mode 100644 go/forgekey/keys.go create mode 100644 go/forgekey/runner.go diff --git a/go/forgekey/keykeeper.go b/go/forgekey/keykeeper.go new file mode 100644 index 0000000..cb75668 --- /dev/null +++ b/go/forgekey/keykeeper.go @@ -0,0 +1,127 @@ +// Package forgekey is the key keeper every module that reaches the forge over SSH shares: ssh-client for a +// machine's accounts, a build module for its own (design 47 R4c-3, ADR 0466 §2; issue 1469). +package forgekey + +// The key this machine's account uses to reach the forge over SSH (novox/hq ADR 0399, ADR 0400; issue 955, +// area S1). It is made here, on the machine, once: ssh-keygen writes ~/.ssh/mesh_ed25519 only when it is +// missing, so the private half never leaves the machine and an existing key is never replaced. The public +// half and its fingerprint go into this module's `keys` state, one entry per account under ., +// for the forge's provider to register. The tools bundle is the one process of this module, so the keeper +// runs inside it beside the tools (main.go), the way claude-code reports what it holds. + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "time" +) + +// KeyFile is the key's name under ~/.ssh: never one of the operator's own id_* files. +const KeyFile = "mesh_ed25519" + +// The kinds of key (ADR 0400 §1, ADR 0466 §2): a machine's account's own, and a build module's. +const ( + KindMachine = "machine" + KindBuilder = "builder" +) + +// KeyRetry is how soon it looks again after a failure. +const KeyRetry = 30 * time.Second + +// KeyRound is how often the keeper looks again. +const KeyRound = 10 * time.Minute + +// KeyEntry is one value of the keys state. It carries the public half only. +type KeyEntry struct { + Node string `json:"node"` + Account string `json:"account"` + Kind string `json:"kind"` + Public string `json:"public"` + Fingerprint string `json:"fingerprint"` + At string `json:"at"` +} + +// KeyPutter is the keys state, as far as the keeper needs it. +type KeyPutter interface { + Put(key string, value any) (uint64, error) +} + +// KeyKeeper keeps one account's key and its entry. +type KeyKeeper struct { + Home, Node, Account string + Kind string // KindMachine or KindBuilder + Run Runner + State KeyPutter + Now func() time.Time + // After waits between rounds; time.After when nil. A test sets it to see which pause was chosen. + After func(time.Duration) <-chan time.Time + + reported string // the public half and fingerprint last put +} + +func (k *KeyKeeper) path() string { return filepath.Join(k.Home, ".ssh", KeyFile) } + +// Once makes the key when it is missing and puts the entry when it differs from the one last put. +func (k *KeyKeeper) Once(ctx context.Context) error { + if err := k.ensure(ctx); err != nil { + return err + } + raw, err := os.ReadFile(k.path() + ".pub") + if err != nil { + return fmt.Errorf("the public half: %w", err) + } + pk, err := ParseKeyLine(string(raw)) + if err != nil { + return fmt.Errorf("%s.pub: %w", k.path(), err) + } + public := strings.TrimSpace(string(raw)) + if sig := public + "|" + pk.Fingerprint; sig == k.reported { + return nil + } + entry := KeyEntry{Node: k.Node, Account: k.Account, Kind: k.Kind, Public: public, + Fingerprint: pk.Fingerprint, At: k.Now().UTC().Format(time.RFC3339)} + if _, err := k.State.Put(k.Node+"."+k.Account, entry); err != nil { + return err + } + k.reported = public + "|" + pk.Fingerprint + return nil +} + +// ensure makes the pair when the private half is missing. Never over an existing key. +func (k *KeyKeeper) ensure(ctx context.Context) error { + if _, err := os.Lstat(k.path()); err == nil { + return nil + } else if !os.IsNotExist(err) { + return err + } + if err := os.MkdirAll(filepath.Dir(k.path()), 0o700); err != nil { + return err + } + r := k.Run(ctx, nil, "ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-C", k.Account+"@"+k.Node+" (mesh)", "-f", k.path()) + if r.Status != 0 || r.Err != "" { + return fmt.Errorf("ssh-keygen: %s", firstLine(r.Err+" "+r.Stderr)) + } + return nil +} + +// Keep runs Once now and every KeyRound, reporting a failure through say and trying again at the next round. +func (k *KeyKeeper) Keep(ctx context.Context, say func(string, ...any)) { + for { + pause := KeyRound + if err := k.Once(ctx); err != nil { + say("the forge key: %v", err) + pause = KeyRetry // the state or the bus's grant may arrive after the process + } + after := k.After + if after == nil { + after = time.After + } + select { + case <-ctx.Done(): + return + case <-after(pause): + } + } +} diff --git a/go/forgekey/keys.go b/go/forgekey/keys.go new file mode 100644 index 0000000..4af0499 --- /dev/null +++ b/go/forgekey/keys.go @@ -0,0 +1,110 @@ +package forgekey + +// Keys, by their public half only. A fingerprint is computed here from the public key's bytes, as +// ssh-keygen -l does (SHA256 of the key blob, unpadded base64); a private key is never read by this +// process — its first line, the PEM header, says it is one, and ssh-keygen is asked about it. + +import ( + "crypto/sha256" + "encoding/base64" + "encoding/binary" + "fmt" + "math/big" + "strings" +) + +// KeyTypes are the public key algorithms an authorized_keys or known_hosts line can carry. +var KeyTypes = map[string]bool{ + "ssh-ed25519": true, "ssh-rsa": true, "ssh-dss": true, + "ecdsa-sha2-nistp256": true, "ecdsa-sha2-nistp384": true, "ecdsa-sha2-nistp521": true, + "sk-ssh-ed25519@openssh.com": true, "sk-ecdsa-sha2-nistp256@openssh.com": true, +} + +// PublicKey is one public key as a line carries it. +type PublicKey struct { + Type string `json:"type"` + Bits int `json:"bits"` + Fingerprint string `json:"fingerprint"` + Comment string `json:"comment"` + Options string `json:"options,omitempty"` + Weak string `json:"weak,omitempty"` +} + +// ParseKeyLine reads one authorized_keys line (options, type, key, comment) or a public key file's. +func ParseKeyLine(line string) (PublicKey, error) { + fields := fieldsQuoted(strings.TrimSpace(line)) + for i, f := range fields { + if !KeyTypes[f] || i+1 >= len(fields) { + continue + } + k := PublicKey{Type: f, Options: strings.Join(fields[:i], " "), Comment: strings.Join(fields[i+2:], " ")} + blob, err := base64.StdEncoding.DecodeString(fields[i+1]) + if err != nil { + return k, fmt.Errorf("the key after %s is not base64", f) + } + sum := sha256.Sum256(blob) + k.Fingerprint = "SHA256:" + base64.RawStdEncoding.EncodeToString(sum[:]) + k.Bits = bitsOf(f, blob) + switch { + case f == "ssh-dss": + k.Weak = "DSA: refused by current OpenSSH" + case f == "ssh-rsa" && k.Bits > 0 && k.Bits < 3072: + k.Weak = fmt.Sprintf("RSA of %d bits: below 3072", k.Bits) + } + return k, nil + } + return PublicKey{}, fmt.Errorf("no public key on this line") +} + +// fieldsQuoted splits on spaces outside double quotes, as sshd reads an options field. +func fieldsQuoted(s string) []string { + var out []string + var cur strings.Builder + quoted := false + for _, ch := range s { + switch { + case ch == '"': + quoted = !quoted + cur.WriteRune(ch) + case (ch == ' ' || ch == '\t') && !quoted: + if cur.Len() > 0 { + out = append(out, cur.String()) + cur.Reset() + } + default: + cur.WriteRune(ch) + } + } + if cur.Len() > 0 { + out = append(out, cur.String()) + } + return out +} + +// bitsOf reads a key's size from its blob: the RSA modulus, the curve, or ed25519's fixed 256. +func bitsOf(kind string, blob []byte) int { + strs := [][]byte{} + for len(blob) >= 4 { + n := binary.BigEndian.Uint32(blob) + if int(n) > len(blob)-4 { + break + } + strs = append(strs, blob[4:4+n]) + blob = blob[4+n:] + } + switch { + case strings.Contains(kind, "ed25519"): + return 256 + case kind == "ssh-rsa" && len(strs) >= 3: + return new(big.Int).SetBytes(strs[2]).BitLen() + case kind == "ssh-dss" && len(strs) >= 2: + return new(big.Int).SetBytes(strs[1]).BitLen() + case strings.Contains(kind, "nistp256"): + return 256 + case strings.Contains(kind, "nistp384"): + return 384 + case strings.Contains(kind, "nistp521"): + return 521 + } + return 0 +} diff --git a/go/forgekey/runner.go b/go/forgekey/runner.go new file mode 100644 index 0000000..9cc08e1 --- /dev/null +++ b/go/forgekey/runner.go @@ -0,0 +1,61 @@ +package forgekey + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "strings" + "time" +) + +// Ran is what a command did: its output, its exit status, and why it never ran to an answer. +type Ran struct { + Stdout string + Stderr string + Status int + // Err is "ENOENT" when the program is not installed, or says it was ended for taking too long. + Err string +} + +// Runner runs one command with extra environment words, so every tool can be tested without ssh. +type Runner func(ctx context.Context, env []string, name string, args ...string) Ran + +// CallTimeout bounds one command: below the runtime's thirty-second call limit. +const CallTimeout = 20 * time.Second + +// ExecRunner runs a command on this machine, with no terminal and nothing on its stdin, bounded by +// CallTimeout. +func ExecRunner(ctx context.Context, env []string, name string, args ...string) Ran { + ctx, cancel := context.WithTimeout(ctx, CallTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, name, args...) + cmd.Env = append(os.Environ(), env...) + var out, errb bytes.Buffer + cmd.Stdout, cmd.Stderr = &out, &errb + err := cmd.Run() + r := Ran{Stdout: out.String(), Stderr: errb.String()} + var exitErr *exec.ExitError + switch { + case errors.Is(ctx.Err(), context.DeadlineExceeded): + r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(CallTimeout/time.Second)) + case errors.Is(err, exec.ErrNotFound): + r.Status, r.Err = 127, "ENOENT" + case errors.As(err, &exitErr): + r.Status = exitErr.ExitCode() + case err != nil: + r.Status, r.Err = 1, err.Error() + } + return r +} + +func firstLine(s string) string { + for _, l := range strings.Split(s, "\n") { + if l = strings.TrimSpace(l); l != "" { + return l + } + } + return "" +} -- 2.54.0 From a51dc144a157582541762b9df994156d15d29922 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:04:27 +0000 Subject: [PATCH 02/13] Test the key keeper (issue 1469), part 2 of 6 --- go/forgekey/keykeeper_test.go | 229 ++++++++++++++++++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 go/forgekey/keykeeper_test.go diff --git a/go/forgekey/keykeeper_test.go b/go/forgekey/keykeeper_test.go new file mode 100644 index 0000000..a52e176 --- /dev/null +++ b/go/forgekey/keykeeper_test.go @@ -0,0 +1,229 @@ +package forgekey + +import ( + "context" + "encoding/base64" + "encoding/binary" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// Issue 955, area S1: the key the machine's account makes for the forge, and its entry in the keys state. + +type fakeState struct{ puts map[string][]byte } + +func (f *fakeState) Put(key string, value any) (uint64, error) { + raw, _ := json.Marshal(value) + if f.puts == nil { + f.puts = map[string][]byte{} + } + f.puts[key] = raw + return uint64(len(f.puts)), nil +} + +// ed25519Line is a public key line with a well-formed ed25519 blob. +func ed25519Line(seed byte, comment string) string { + blob := []byte{} + for _, part := range [][]byte{[]byte("ssh-ed25519"), make([]byte, 32)} { + part2 := append([]byte(nil), part...) + if len(part2) == 32 { + part2[0] = seed + } + l := make([]byte, 4) + binary.BigEndian.PutUint32(l, uint32(len(part2))) + blob = append(append(blob, l...), part2...) + } + return "ssh-ed25519 " + base64.StdEncoding.EncodeToString(blob) + " " + comment +} + +// keygenRunner stands in for ssh-keygen -t ed25519: it writes the pair the command names. +func keygenRunner(calls *[][]string) Runner { + return func(ctx context.Context, env []string, name string, args ...string) Ran { + *calls = append(*calls, append([]string{name}, args...)) + var file, comment string + for i, a := range args { + switch a { + case "-f": + file = args[i+1] + case "-C": + comment = args[i+1] + } + } + if name != "ssh-keygen" || file == "" { + return Ran{Status: 1, Stderr: "unexpected"} + } + os.WriteFile(file, []byte("-----BEGIN OPENSSH PRIVATE KEY-----\nPRIVATE-SECRET\n"), 0o600) + os.WriteFile(file+".pub", []byte(ed25519Line(7, comment)+"\n"), 0o644) + return Ran{} + } +} + +func newKeeper(t *testing.T, home string, st *fakeState, calls *[][]string) *KeyKeeper { + return &KeyKeeper{Home: home, Node: "g14", Account: "nox", Kind: KindMachine, Run: keygenRunner(calls), State: st, + Now: func() time.Time { return time.Date(2026, 10, 16, 12, 0, 0, 0, time.UTC) }} +} + +func TestKeeperMakesTheKeyAndWritesItsEntry(t *testing.T) { + home := t.TempDir() + st, calls := &fakeState{}, &[][]string{} + if err := newKeeper(t, home, st, calls).Once(context.Background()); err != nil { + t.Fatal(err) + } + raw, ok := st.puts["g14.nox"] + if !ok || len(st.puts) != 1 { + t.Fatalf("entries = %v, want one under g14.nox", st.puts) + } + var e map[string]string + json.Unmarshal(raw, &e) + if e["node"] != "g14" || e["account"] != "nox" || e["kind"] != "machine" || e["at"] != "2026-10-16T12:00:00Z" { + t.Errorf("entry = %v", e) + } + if !strings.HasPrefix(e["public"], "ssh-ed25519 ") || !strings.HasSuffix(e["public"], " nox@g14 (mesh)") { + t.Errorf("public = %q", e["public"]) + } + if !strings.HasPrefix(e["fingerprint"], "SHA256:") { + t.Errorf("fingerprint = %q", e["fingerprint"]) + } + if strings.Contains(strings.ToUpper(string(raw)), "PRIVATE") { + t.Errorf("the entry carries private material: %s", raw) + } + cmd := strings.Join((*calls)[0], " ") + for _, want := range []string{"-t ed25519", "-N ", "-C nox@g14 (mesh)", filepath.Join(home, ".ssh", "mesh_ed25519")} { + if !strings.Contains(cmd, want) { + t.Errorf("ssh-keygen call %q lacks %q", cmd, want) + } + } +} + +func TestKeeperKeepsAnExistingKeyByteForByte(t *testing.T) { + home := t.TempDir() + os.MkdirAll(filepath.Join(home, ".ssh"), 0o700) + priv, pub := filepath.Join(home, ".ssh", "mesh_ed25519"), filepath.Join(home, ".ssh", "mesh_ed25519.pub") + os.WriteFile(priv, []byte("mine"), 0o600) + os.WriteFile(pub, []byte(ed25519Line(9, "nox@g14 (mesh)")+"\n"), 0o644) + st, calls := &fakeState{}, &[][]string{} + k := newKeeper(t, home, st, calls) + if err := k.Once(context.Background()); err != nil { + t.Fatal(err) + } + if len(*calls) != 0 { + t.Errorf("ssh-keygen ran over an existing key: %v", *calls) + } + if b, _ := os.ReadFile(priv); string(b) != "mine" { + t.Errorf("private file changed: %q", b) + } + if _, ok := st.puts["g14.nox"]; !ok { + t.Errorf("no entry for the existing key") + } + // A second round with nothing changed writes nothing. + delete(st.puts, "g14.nox") + k.Once(context.Background()) + if len(st.puts) != 0 { + t.Errorf("an unchanged key was put again") + } +} + +func TestKeeperNeverTouchesTheOperatorsOwnKeys(t *testing.T) { + home := t.TempDir() + os.MkdirAll(filepath.Join(home, ".ssh"), 0o700) + own := filepath.Join(home, ".ssh", "id_ed25519") + os.WriteFile(own, []byte("operator"), 0o600) + st, calls := &fakeState{}, &[][]string{} + newKeeper(t, home, st, calls).Once(context.Background()) + if b, _ := os.ReadFile(own); string(b) != "operator" { + t.Errorf("the operator's key changed") + } +} + +// 1469/AC1: an empty home, kind builder, account mesh-build-go: ssh-keygen once, for the one file, and the entry +// carries the kind, the public half and its fingerprint. +func TestBuilderKeyIsMadeOnceAndPutWithItsKind(t *testing.T) { + home := t.TempDir() + st, calls := &fakeState{}, &[][]string{} + k := newKeeper(t, home, st, calls) + k.Account, k.Kind = "mesh-build-go", KindBuilder + if err := k.Once(context.Background()); err != nil { + t.Fatal(err) + } + if len(*calls) != 1 || !strings.Contains(strings.Join((*calls)[0], " "), filepath.Join(home, ".ssh", "mesh_ed25519")) { + t.Errorf("ssh-keygen calls = %v, want one for mesh_ed25519 in the home", *calls) + } + var e KeyEntry + if err := json.Unmarshal(st.puts["g14.mesh-build-go"], &e); err != nil { + t.Fatalf("entry under g14.mesh-build-go: %v (%v)", err, st.puts) + } + pub, _ := os.ReadFile(filepath.Join(home, ".ssh", "mesh_ed25519.pub")) + pk, _ := ParseKeyLine(string(pub)) + if e.Kind != "builder" || e.Public != strings.TrimSpace(string(pub)) || e.Fingerprint != pk.Fingerprint || e.Fingerprint == "" { + t.Errorf("entry = %+v", e) + } +} + +// 1469/AC2: an existing key, Once twice: ssh-keygen never, one put. +func TestExistingKeyIsPutOnceAcrossTwoRounds(t *testing.T) { + home := t.TempDir() + os.MkdirAll(filepath.Join(home, ".ssh"), 0o700) + os.WriteFile(filepath.Join(home, ".ssh", "mesh_ed25519"), []byte("mine"), 0o600) + os.WriteFile(filepath.Join(home, ".ssh", "mesh_ed25519.pub"), []byte(ed25519Line(9, "x")+"\n"), 0o644) + st, calls := &countState{}, &[][]string{} + k := newKeeper(t, home, &fakeState{}, calls) + k.State = st + for i := 0; i < 2; i++ { + if err := k.Once(context.Background()); err != nil { + t.Fatal(err) + } + } + if len(*calls) != 0 || st.n != 1 { + t.Errorf("ssh-keygen calls = %d, puts = %d, want 0 and 1", len(*calls), st.n) + } +} + +type countState struct{ n int } + +func (c *countState) Put(string, any) (uint64, error) { c.n++; return uint64(c.n), nil } + +// 1469/AC3: the entry's JSON is ssh-client's own (mesh-catalog modules/ssh-client keykeeper_test.go's keys +// entry: node, account, kind, public, fingerprint, at, in that order); mesh-catalog's gitea provider reads it. +func TestKeyEntryJSONIsSSHClients(t *testing.T) { + raw, err := json.Marshal(KeyEntry{Node: "g14", Account: "nox", Kind: "machine", Public: "ssh-ed25519 AAAA nox@g14 (mesh)", + Fingerprint: "SHA256:abc", At: "2026-10-16T12:00:00Z"}) + if err != nil { + t.Fatal(err) + } + want := `{"node":"g14","account":"nox","kind":"machine","public":"ssh-ed25519 AAAA nox@g14 (mesh)","fingerprint":"SHA256:abc","at":"2026-10-16T12:00:00Z"}` + if string(raw) != want { + t.Errorf("entry JSON = %s\nwant %s", raw, want) + } +} + +type failState struct{} + +func (failState) Put(string, any) (uint64, error) { return 0, errors.New("no grant yet") } + +// 1469/AC4: a failing put is said, and the next look is after KeyRetry, not KeyRound. +func TestKeepSaysAFailedPutAndRetriesSoon(t *testing.T) { + home := t.TempDir() + k := newKeeper(t, home, &fakeState{}, &[][]string{}) + k.State = failState{} + ctx, cancel := context.WithCancel(context.Background()) + var said []string + var pauses []time.Duration + k.After = func(d time.Duration) <-chan time.Time { + pauses = append(pauses, d) + cancel() + return make(chan time.Time) + } + k.Keep(ctx, func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) }) + if len(said) != 1 || !strings.Contains(said[0], "no grant yet") { + t.Errorf("said = %v", said) + } + if len(pauses) != 1 || pauses[0] != KeyRetry { + t.Errorf("pauses = %v, want [%v]", pauses, KeyRetry) + } +} -- 2.54.0 From 4d4112a008eadd71bff7e87dcb2e7dadbf3de869 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:04:47 +0000 Subject: [PATCH 03/13] Call a module's tool or a seat verb on a named machine (issue 1470), part 3 of 6 --- go/process/call.go | 42 +++++++++++++++++++++++++++-- go/process/call_integration_test.go | 42 +++++++++++++++++++++++++++++ go/process/call_test.go | 42 ++++++++++++++++++++++++++++- 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/go/process/call.go b/go/process/call.go index 8b79160..a9f609c 100644 --- a/go/process/call.go +++ b/go/process/call.go @@ -28,6 +28,26 @@ func (c *Conn) Call(key string, body any) (json.RawMessage, error) { if err != nil { return nil, err } + return c.callOn(called, fallback, body) +} + +// CallOn is Call to the tool or seat verb of one named machine (novox/hq ADR 0469 §3; issue 1470): the tool +// runner of that node alone answers, on mesh.mod..call... (a seat verb: +// mesh.seat..call...), falling back to the subject that names no caller +// (mesh-tools node-tools/internal/bus/caller.go:15-19). node is one subject token: an empty one, or one +// holding '.', '*', '>' or white space, is refused before anything is published. +func (c *Conn) CallOn(node, key string, body any) (json.RawMessage, error) { + if err := checkNode(node); err != nil { + return nil, err + } + called, fallback, err := callSubjectsOn(key, node, c.callerToken()) + if err != nil { + return nil, err + } + return c.callOn(called, fallback, body) +} + +func (c *Conn) callOn(called, fallback string, body any) (json.RawMessage, error) { data, err := json.Marshal(body) if err != nil { return nil, err @@ -46,18 +66,36 @@ func (c *Conn) callerToken() string { return strings.ReplaceAll(c.account.User, // callSubjects is the caller-named subject key is called on, and its plain fallback (mesh-tools // node-tools/internal/bus/caller.go CalledSubject, toolShape, calledPattern). func callSubjects(key, token string) (called, fallback string, err error) { + return callSubjectsOn(key, "", token) +} + +// checkNode refuses a node name that is not one subject token. +func checkNode(node string) error { + if node == "" || strings.ContainsAny(node, ".*> \t\r\n") { + return fmt.Errorf("%q is not a node name (one subject token: no '.', '*', '>' or space)", node) + } + return nil +} + +// callSubjectsOn is callSubjects for one named node ("" names none): the node is a token between the tool and +// the caller on the called subject, and the last one of the fallback. +func callSubjectsOn(key, node, token string) (called, fallback string, err error) { + at := "" + if node != "" { + at = "." + node + } if rest, ok := strings.CutPrefix(key, "seat:"); ok { seat, verb, ok := strings.Cut(rest, ".") if !ok || seat == "" || verb == "" { return "", "", fmt.Errorf("%q names no seat and verb (seat:.)", key) } - return "mesh.seat." + seat + ".call." + verb + "." + token, "mesh.seat." + seat + ".tool." + verb, nil + return "mesh.seat." + seat + ".call." + verb + at + "." + token, "mesh.seat." + seat + ".tool." + verb + at, nil } module, tool, ok := strings.Cut(key, ".") if !ok || module == "" || tool == "" { return "", "", fmt.Errorf("%q is not ., and does not start seat:.", key) } - return "mesh.mod." + module + ".call." + tool + "." + token, "mesh.mod." + module + ".tool." + tool, nil + return "mesh.mod." + module + ".call." + tool + at + "." + token, "mesh.mod." + module + ".tool." + tool + at, nil } // request asks one subject once and reads its reply ({result, error, node}); a reply naming an error is diff --git a/go/process/call_integration_test.go b/go/process/call_integration_test.go index d08be4d..708e38f 100644 --- a/go/process/call_integration_test.go +++ b/go/process/call_integration_test.go @@ -235,3 +235,45 @@ func TestReleaseAcceptanceAProcessDoesExactlyItsBusDeclaration(t *testing.T) { t.Error("subscribing a tool-call subject the process does not serve was not refused") } } + +// 1470/AC4: two answerers of the same verb on the subjects of nodes ace and g14; CallOn naming g14 gets g14's +// answer, and ace's answerer hears nothing. +func TestCallOnReachesTheNamedNodeAlone(t *testing.T) { + url, account := grantedSendProcess(t) + adminNC, _ := adminJS(t, url) + aceHeard := make(chan struct{}, 1) + for _, node := range []string{"ace", "g14"} { + node := node + sub, err := adminNC.Subscribe("mesh.seat.operator-channel.call.tell."+node+".novox~mesh-mail~send", func(m *nats.Msg) { + if node == "ace" { + aceHeard <- struct{}{} + } + _ = m.Respond([]byte(`{"result":{"node":"` + node + `"}}`)) + }) + if err != nil { + t.Fatal(err) + } + defer func() { _ = sub.Unsubscribe() }() + } + if err := adminNC.Flush(); err != nil { + t.Fatal(err) + } + conn, err := Connect(account) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + + result, err := conn.CallOn("g14", "seat:operator-channel.tell", map[string]any{}) + if err != nil { + t.Fatalf("CallOn: %v", err) + } + if string(result) != `{"node":"g14"}` { + t.Fatalf("result: %s, want g14's answer", result) + } + select { + case <-aceHeard: + t.Error("ace's answerer heard a call named for g14") + default: + } +} diff --git a/go/process/call_test.go b/go/process/call_test.go index eb50aa9..6f33eb2 100644 --- a/go/process/call_test.go +++ b/go/process/call_test.go @@ -1,6 +1,9 @@ package process -import "testing" +import ( + "strings" + "testing" +) // 1191/AC3: given a module tool key ., when called, then the subject is // mesh.mod..call.. with the same fallback (mesh.mod..tool.). @@ -39,3 +42,40 @@ func TestCallSubjectsRefusesAKeyNamingNeither(t *testing.T) { } } } + +// 1470/AC1: key mesh-build-go.fetch, node ace, caller ace.mesh-delivery-builder. +func TestCallSubjectsOnANodeForAModuleTool(t *testing.T) { + c := &Conn{account: Account{User: "ace.mesh-delivery-builder"}} + called, fallback, err := callSubjectsOn("mesh-build-go.fetch", "ace", c.callerToken()) + if err != nil { + t.Fatal(err) + } + if called != "mesh.mod.mesh-build-go.call.fetch.ace.ace~mesh-delivery-builder" { + t.Errorf("called: %q", called) + } + if fallback != "mesh.mod.mesh-build-go.tool.fetch.ace" { + t.Errorf("fallback: %q", fallback) + } +} + +// 1470/AC2: seat:node-files.read on g14. +func TestCallSubjectsOnANodeForASeatVerb(t *testing.T) { + called, fallback, err := callSubjectsOn("seat:node-files.read", "g14", "tok") + if err != nil { + t.Fatal(err) + } + if called != "mesh.seat.node-files.call.read.g14.tok" || fallback != "mesh.seat.node-files.tool.read.g14" { + t.Errorf("called %q, fallback %q", called, fallback) + } +} + +// 1470/AC3: a node that is not one token is refused before anything is published (the Conn has no +// connection, so only the name check can answer with this error). +func TestCallOnRefusesANodeThatIsNotOneToken(t *testing.T) { + c := &Conn{} + for _, node := range []string{"", "a.b", "*", ">", "a b", "a\tb"} { + if _, err := c.CallOn(node, "mesh-build-go.fetch", nil); err == nil || !strings.Contains(err.Error(), "not a node name") { + t.Errorf("CallOn(%q) = %v, want the refusal of the name", node, err) + } + } +} -- 2.54.0 From 927852fda8f3198e51ad532c1db86a21231504cf Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:05:21 +0000 Subject: [PATCH 04/13] Lay out a build module's long steps, guard its toolchain and fetch one commit over SSH (issues 1471, 1473, 1474), part 4 of 7 --- go/buildstep/fetch.go | 114 ++++++++++++++++++++++++++++++++++++++++ go/buildstep/guard.go | 52 ++++++++++++++++++ go/buildstep/refusal.go | 13 +++++ go/buildstep/step.go | 99 ++++++++++++++++++++++++++++++++++ 4 files changed, 278 insertions(+) create mode 100644 go/buildstep/fetch.go create mode 100644 go/buildstep/guard.go create mode 100644 go/buildstep/refusal.go create mode 100644 go/buildstep/step.go diff --git a/go/buildstep/fetch.go b/go/buildstep/fetch.go new file mode 100644 index 0000000..df6a1b0 --- /dev/null +++ b/go/buildstep/fetch.go @@ -0,0 +1,114 @@ +package buildstep + +import ( + "context" + "fmt" + "net/url" + "regexp" + "strings" + + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +// Option changes FetchCommit. +type Option func(*fetchOptions) + +type fetchOptions struct{ allowFile bool } + +// AllowFileRemote lets FetchCommit take a file:// remote. It is for a test's local repository; production +// code does not pass it, and FetchCommit then refuses anything but ssh://. +func AllowFileRemote() Option { return func(o *fetchOptions) { o.allowFile = true } } + +var ( + commitOK = regexp.MustCompile(`^[0-9a-f]{40}$`) + refOK = regexp.MustCompile(`^refs/[A-Za-z0-9._/-]+$`) +) + +// FetchCommit fetches one commit of a forge repository into dir, an empty directory, and checks it out detached +// (novox/hq ADR 0466 §4, §5; issue 1474). remote is ssh:////.git, the ssh name being the +// one the build module's templates set up (ADR 0409 §1); commit is a full 40-hex sha; ref is optional, and names +// the pull request's head (refs/pull//head) when the commit is only there. Every git command runs through run +// as `git -C dir ...` with GIT_TERMINAL_PROMPT=0 and no credential helper, so nothing is asked for and no +// credential is used. A remote that is not ssh://, or one with user information, and a commit that is not 40 +// hex digits are refused before anything runs. +func FetchCommit(ctx context.Context, run forgekey.Runner, dir, remote, commit, ref string, opts ...Option) error { + var o fetchOptions + for _, opt := range opts { + opt(&o) + } + if err := checkFetch(o, dir, remote, commit, ref); err != nil { + return err + } + env := []string{"GIT_TERMINAL_PROMPT=0"} + git := func(args ...string) (string, error) { + full := append([]string{"-c", "credential.helper=", "-C", dir}, args...) + r := run(ctx, env, "git", full...) + if r.Status != 0 || r.Err != "" { + return "", fmt.Errorf("git %s: %s", args[0], firstLine(r.Err+" "+r.Stderr)) + } + return r.Stdout, nil + } + if _, err := git("init", "-q"); err != nil { + return err + } + if _, err := git("remote", "add", "origin", remote); err != nil { + return err + } + if _, err := git("fetch", "--no-tags", "--depth", "1", "origin", commit); err != nil { + if ref == "" { + return err + } + if _, rerr := git("fetch", "--no-tags", "origin", ref); rerr != nil { + return fmt.Errorf("%v; then the ref %s: %w", err, ref, rerr) + } + if _, verr := git("cat-file", "-e", commit+"^{commit}"); verr != nil { + return fmt.Errorf("the commit %s is not in %s: %w", commit, ref, verr) + } + } + if _, err := git("checkout", "-q", "--detach", commit); err != nil { + return err + } + out, err := git("rev-parse", "HEAD") + if err != nil { + return err + } + if head := strings.TrimSpace(out); head != commit { + return fmt.Errorf("the checkout is at %s, not at the commit %s", head, commit) + } + return nil +} + +func checkFetch(o fetchOptions, dir, remote, commit, ref string) error { + if dir == "" { + return fmt.Errorf("buildstep: no workspace directory") + } + if !commitOK.MatchString(commit) { + return fmt.Errorf("buildstep: %q is not a full commit (40 hex digits)", commit) + } + if ref != "" && !refOK.MatchString(ref) { + return fmt.Errorf("buildstep: %q is not a ref (refs/...)", ref) + } + u, err := url.Parse(remote) + if err != nil { + return fmt.Errorf("buildstep: the remote is not a URL: %w", err) + } + if u.User != nil { + return fmt.Errorf("buildstep: the remote carries user information; the key is the credential") + } + switch { + case u.Scheme == "ssh" && u.Host != "": + case u.Scheme == "file" && o.allowFile: + default: + return fmt.Errorf("buildstep: the remote %q is not ssh://", remote) + } + return nil +} + +func firstLine(s string) string { + for _, l := range strings.Split(s, "\n") { + if l = strings.TrimSpace(l); l != "" { + return l + } + } + return "" +} diff --git a/go/buildstep/guard.go b/go/buildstep/guard.go new file mode 100644 index 0000000..a9dc49c --- /dev/null +++ b/go/buildstep/guard.go @@ -0,0 +1,52 @@ +package buildstep + +import ( + "context" + "encoding/json" + "fmt" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// Guard is a build module's check that the toolchain installed on its node is the one its recipe builds with +// (novox/hq ADR 0469 §1; issue 1473). The same guard serves ADR 0470 §4's postgres major for the integration +// level, with its own Probe and Want. +type Guard struct { + // Language is the toolchain's key part: go or typescript (delivery.ToolchainKey). + Language string + // Want is the toolchain version the recipe builds with, and Recipe the recipe's own version. The indexer + // compares both texts as they are written here. + Want, Recipe string + // Probe reads the installed version (go version, node --version). + Probe func(ctx context.Context) (installed string, err error) + Node string + // Put is the toolchains state's Put (stdio.State(delivery.ToolchainsState).Put). + Put Put +} + +// Check probes the installed version, writes it to . of the toolchains state every time it runs, +// and answers nil when it is the recipe's. Otherwise it answers a *Refusal naming both versions; the installed +// version is written even then, so the indexer sees that builders disagree. A probe that fails is a *Refusal +// naming its failure, and nothing is written. A write that fails is an error that is not a Refusal. +func (g Guard) Check(ctx context.Context) error { + installed, err := g.Probe(ctx) + if err != nil { + return &Refusal{Text: fmt.Sprintf("the installed %s version could not be read: %v", g.Language, err), Want: g.Want} + } + key, err := delivery.ToolchainKey(g.Node, g.Language) + if err != nil { + return err + } + raw, err := delivery.EncodeToolchain(delivery.Toolchain{Language: g.Language, Version: installed, Recipe: g.Recipe, At: now().UTC()}) + if err != nil { + return err + } + if _, err := g.Put(key, json.RawMessage(raw)); err != nil { + return fmt.Errorf("buildstep: writing the toolchain %s: %w", key, err) + } + if installed != g.Want { + return &Refusal{Text: fmt.Sprintf("%s %s installed, the recipe builds with %s %s", g.Language, installed, g.Language, g.Want), + Want: g.Want, Have: installed} + } + return nil +} diff --git a/go/buildstep/refusal.go b/go/buildstep/refusal.go new file mode 100644 index 0000000..0f88529 --- /dev/null +++ b/go/buildstep/refusal.go @@ -0,0 +1,13 @@ +package buildstep + +// Refusal is a step that is not run on purpose, as against one that failed: the toolchain installed is not the +// recipe's, the integration level's postgres major is not the one it is built for (novox/hq ADR 0469 §1, ADR 0470 +// §4; issue 1473). A step's run returns one and the runner ends the step refused with its text. +type Refusal struct { + // Text is the reason, in words the operator reads. + Text string + // Want is the version the recipe asks for and Have the one found; either may be empty. + Want, Have string +} + +func (r *Refusal) Error() string { return r.Text } diff --git a/go/buildstep/step.go b/go/buildstep/step.go new file mode 100644 index 0000000..c5ae0d9 --- /dev/null +++ b/go/buildstep/step.go @@ -0,0 +1,99 @@ +// Package buildstep is what both build modules share: the runner of a long step that answers a tool call at once +// and ends on its key in the module's `steps` state (novox/hq ADR 0469 §2; issue 1471), the toolchain guard +// (issue 1473) and the fetch of one commit over SSH (issue 1474). +package buildstep + +import ( + "context" + "crypto/rand" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "strconv" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// Put writes one key of a state: stdio.State(delivery.StepsState).Put's shape (go/state.go). +type Put func(key string, value any) (uint64, error) + +// EndAttempts and EndRetry bound how a step's end is written: a write that fails is tried again, since the end +// is the only way the step's reader learns it is over. +var ( + EndAttempts = 5 + EndRetry = 2 * time.Second +) + +var now = time.Now + +// NewID is a step id: the time in milliseconds in base 36 and six random hex digits, lower-case letters and +// digits only, so it is one key token and differs between two steps of one node even in one millisecond. +func NewID() string { + var b [3]byte + _, _ = rand.Read(b[:]) + return strconv.FormatInt(now().UnixMilli(), 36) + hex.EncodeToString(b[:]) +} + +// Start begins a step of tool on node and answers its id at once: the key . holds state running when +// Start returns. run is then called in a goroutine of its own, under ctx (the process's, not the tool call's: the +// call is cut at 30 s, the step is not). When it returns, the key is written passed with run's result, failed with +// the error's text, or refused when the error is a *Refusal; a panic in run ends the step failed, naming it, and +// the process goes on. If the running write fails, nothing was started and Start answers the error. +func Start(ctx context.Context, put Put, node, tool string, run func(ctx context.Context) (result any, err error)) (string, error) { + step := delivery.Step{ID: NewID(), Tool: tool, Node: node, State: delivery.StepRunning, Started: now().UTC()} + key, err := delivery.StepKey(node, step.ID) + if err != nil { + return "", err + } + if err := write(put, key, step); err != nil { + return "", fmt.Errorf("buildstep: writing the running step %s: %w", key, err) + } + go func() { + result, err := protect(ctx, run) + step.Ended = now().UTC() + switch ref := (*Refusal)(nil); { + case err == nil: + step.State = delivery.StepPassed + if result != nil { + raw, merr := json.Marshal(result) + if merr != nil { + step.State, step.Reason = delivery.StepFailed, "the result is not JSON: "+merr.Error() + } else { + step.Result = raw + } + } + case errors.As(err, &ref): + step.State, step.Reason = delivery.StepRefused, err.Error() + default: + step.State, step.Reason = delivery.StepFailed, err.Error() + } + for attempt := 1; ; attempt++ { + if werr := write(put, key, step); werr == nil || attempt >= EndAttempts { + return + } + time.Sleep(EndRetry) + } + }() + return step.ID, nil +} + +// protect is run with a panic turned into the error that ends the step. +func protect(ctx context.Context, run func(ctx context.Context) (any, error)) (result any, err error) { + defer func() { + if p := recover(); p != nil { + result, err = nil, fmt.Errorf("the step panicked: %v", p) + } + }() + return run(ctx) +} + +func write(put Put, key string, step delivery.Step) error { + raw, err := delivery.EncodeStep(step) + if err != nil { + return err + } + _, err = put(key, json.RawMessage(raw)) + return err +} -- 2.54.0 From 43b6361d09b38c0368be5394ebce91ce3e042318 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:05:49 +0000 Subject: [PATCH 05/13] Test the step runner and the toolchain guard (issues 1471, 1473), part 5 of 7 --- go/buildstep/guard_test.go | 79 +++++++++++++++++++ go/buildstep/step_test.go | 154 +++++++++++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+) create mode 100644 go/buildstep/guard_test.go create mode 100644 go/buildstep/step_test.go diff --git a/go/buildstep/guard_test.go b/go/buildstep/guard_test.go new file mode 100644 index 0000000..30514b1 --- /dev/null +++ b/go/buildstep/guard_test.go @@ -0,0 +1,79 @@ +package buildstep + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +type toolchainKeys map[string]delivery.Toolchain + +func (k toolchainKeys) put(key string, value any) (uint64, error) { + tc, err := delivery.DecodeToolchain(value.(json.RawMessage)) + if err != nil { + return 0, err + } + k[key] = tc + return uint64(len(k)), nil +} + +func guardFor(k toolchainKeys, installed string, probeErr error) Guard { + return Guard{Language: "go", Want: "1.27.0", Recipe: "r7", Node: "ace", Put: k.put, + Probe: func(context.Context) (string, error) { return installed, probeErr }} +} + +// 1473/AC1: the probe answers the recipe's version: nil, and .go holds that version and the recipe's. +func TestCheckPassesAndWritesTheToolchain(t *testing.T) { + k := toolchainKeys{} + if err := guardFor(k, "1.27.0", nil).Check(context.Background()); err != nil { + t.Fatal(err) + } + tc, ok := k["ace.go"] + if !ok || tc.Version != "1.27.0" || tc.Recipe != "r7" || tc.Language != "go" || tc.At.IsZero() { + t.Errorf("keys = %+v", k) + } +} + +// 1473/AC2: another version: a Refusal naming both, and the key holds the installed version. +func TestCheckRefusesAnotherVersionAndWritesTheInstalledOne(t *testing.T) { + k := toolchainKeys{} + err := guardFor(k, "1.27.1", nil).Check(context.Background()) + var ref *Refusal + if !errors.As(err, &ref) { + t.Fatalf("Check = %v, want a Refusal", err) + } + if ref.Text != "go 1.27.1 installed, the recipe builds with go 1.27.0" || ref.Want != "1.27.0" || ref.Have != "1.27.1" { + t.Errorf("refusal = %+v", ref) + } + if k["ace.go"].Version != "1.27.1" { + t.Errorf("the key holds %q, want the installed 1.27.1", k["ace.go"].Version) + } +} + +// 1473/AC3: a probe that fails: a Refusal naming the failure, and nothing put. +func TestCheckRefusesAFailedProbeAndPutsNothing(t *testing.T) { + k := toolchainKeys{} + err := guardFor(k, "", errors.New("go: not found")).Check(context.Background()) + var ref *Refusal + if !errors.As(err, &ref) || !strings.Contains(ref.Text, "go: not found") { + t.Fatalf("Check = %v, want a Refusal naming the probe's failure", err) + } + if len(k) != 0 { + t.Errorf("put %v", k) + } +} + +// A failed write is an error, not a refusal: the step fails instead of refusing. +func TestCheckAnswersAFailedWriteAsAnError(t *testing.T) { + g := guardFor(toolchainKeys{}, "1.27.0", nil) + g.Put = func(string, any) (uint64, error) { return 0, errors.New("no grant") } + err := g.Check(context.Background()) + var ref *Refusal + if err == nil || errors.As(err, &ref) { + t.Errorf("Check = %v", err) + } +} diff --git a/go/buildstep/step_test.go b/go/buildstep/step_test.go new file mode 100644 index 0000000..6e7f998 --- /dev/null +++ b/go/buildstep/step_test.go @@ -0,0 +1,154 @@ +package buildstep + +import ( + "context" + "encoding/json" + "errors" + "sync" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// keys is a steps state in memory: every value put, in order, per key, and a signal for each end. +type keys struct { + mu sync.Mutex + puts map[string][]delivery.Step + ended chan string +} + +func newKeys() *keys { return &keys{puts: map[string][]delivery.Step{}, ended: make(chan string, 16)} } + +func (k *keys) put(key string, value any) (uint64, error) { + step, err := delivery.DecodeStep(value.(json.RawMessage)) + if err != nil { + return 0, err + } + k.mu.Lock() + k.puts[key] = append(k.puts[key], step) + n := uint64(len(k.puts[key])) + k.mu.Unlock() + if step.State != delivery.StepRunning { + k.ended <- key + } + return n, nil +} + +func (k *keys) last(key string) delivery.Step { + k.mu.Lock() + defer k.mu.Unlock() + s := k.puts[key] + return s[len(s)-1] +} + +func (k *keys) waitEnd(t *testing.T) string { + t.Helper() + select { + case key := <-k.ended: + return key + case <-time.After(3 * time.Second): + t.Fatal("the step never ended on its key") + return "" + } +} + +// 1471/AC1: a run that blocks: Start answers an id within 100 ms and the key holds running. +func TestStartAnswersAtOnceWithTheStepRunning(t *testing.T) { + k := newKeys() + release := make(chan struct{}) + defer close(release) + began := time.Now() + id, err := Start(context.Background(), k.put, "ace", "fetch", func(ctx context.Context) (any, error) { + <-release + return nil, nil + }) + if err != nil { + t.Fatal(err) + } + if d := time.Since(began); d > 100*time.Millisecond { + t.Errorf("Start took %v", d) + } + s := k.last("ace." + id) + if s.State != delivery.StepRunning || s.Tool != "fetch" || s.Node != "ace" || s.ID != id || s.Started.IsZero() { + t.Errorf("step = %+v", s) + } +} + +// 1471/AC2 and 1473/AC4: a result, an error and a Refusal end the step passed, failed and refused. +func TestStepEndsPassedFailedOrRefusedOnItsKey(t *testing.T) { + for _, c := range []struct { + name string + run func(context.Context) (any, error) + state string + reason string + result string + }{ + {"result", func(context.Context) (any, error) { return map[string]string{"sha": "abc"}, nil }, delivery.StepPassed, "", `{"sha":"abc"}`}, + {"error", func(context.Context) (any, error) { return nil, errors.New("exit 2") }, delivery.StepFailed, "exit 2", ""}, + {"refusal", func(context.Context) (any, error) { + return nil, &Refusal{Text: "go 1.27.1 installed, the recipe builds with go 1.27.0", Want: "1.27.0", Have: "1.27.1"} + }, delivery.StepRefused, "go 1.27.1 installed, the recipe builds with go 1.27.0", ""}, + } { + t.Run(c.name, func(t *testing.T) { + k := newKeys() + id, err := Start(context.Background(), k.put, "ace", "test", c.run) + if err != nil { + t.Fatal(err) + } + if got := k.waitEnd(t); got != "ace."+id { + t.Fatalf("ended key %q", got) + } + s := k.last("ace." + id) + if s.State != c.state || s.Reason != c.reason || string(s.Result) != c.result || s.Ended.IsZero() { + t.Errorf("step = %+v", s) + } + }) + } +} + +// 1471/AC3: a panic in run ends the step failed, naming it, and the test process goes on. +func TestAPanicEndsTheStepFailed(t *testing.T) { + k := newKeys() + id, _ := Start(context.Background(), k.put, "ace", "build", func(context.Context) (any, error) { panic("boom") }) + k.waitEnd(t) + s := k.last("ace." + id) + if s.State != delivery.StepFailed || s.Reason != "the step panicked: boom" { + t.Errorf("step = %+v", s) + } +} + +// 1471/AC5: ids made in one and the same millisecond differ, and each is one key token. The clock is fixed +// while no step runs, so no goroutine reads it meanwhile. +func TestTwoIDsInOneMillisecondDiffer(t *testing.T) { + was := now + defer func() { now = was }() + at := time.Date(2026, 10, 20, 12, 0, 0, 0, time.UTC) + now = func() time.Time { return at } + seen := map[string]bool{} + for i := 0; i < 50; i++ { + id := NewID() + if seen[id] { + t.Fatalf("id %q handed out twice", id) + } + seen[id] = true + if _, err := delivery.StepKey("ace", id); err != nil { + t.Errorf("id %q: %v", id, err) + } + } +} + +// A first write that fails starts nothing. +func TestStartAnswersTheErrorWhenTheRunningWriteFails(t *testing.T) { + ran := make(chan struct{}, 1) + _, err := Start(context.Background(), func(string, any) (uint64, error) { return 0, errors.New("no grant") }, "ace", "fetch", + func(context.Context) (any, error) { ran <- struct{}{}; return nil, nil }) + if err == nil { + t.Fatal("Start answered no error") + } + select { + case <-ran: + t.Error("run was called") + case <-time.After(50 * time.Millisecond): + } +} -- 2.54.0 From a776ca525a499bf6351ab27d078e7a2567d7d35e Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:06:12 +0000 Subject: [PATCH 06/13] Test the fetch of one commit over SSH (issue 1474), part 6 of 7 --- go/buildstep/fetch_integration_test.go | 59 +++++++++++ go/buildstep/fetch_test.go | 137 +++++++++++++++++++++++++ 2 files changed, 196 insertions(+) create mode 100644 go/buildstep/fetch_integration_test.go create mode 100644 go/buildstep/fetch_test.go diff --git a/go/buildstep/fetch_integration_test.go b/go/buildstep/fetch_integration_test.go new file mode 100644 index 0000000..5f782a1 --- /dev/null +++ b/go/buildstep/fetch_integration_test.go @@ -0,0 +1,59 @@ +//go:build integration + +package buildstep + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +func gitIn(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir, "-c", "user.name=t", "-c", "user.email=t@example.invalid"}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +// 1474/AC5: a local bare repository with a commit only under refs/pull/1/head (a file:// remote, allowed by the +// test-only option): the workspace's HEAD is the commit. The forge itself is proven live after the build +// modules' delivery (ADR 0466 §5). +func TestFetchCommitFromAPullRefOnly(t *testing.T) { + root := t.TempDir() + bare := filepath.Join(root, "r.git") + work := filepath.Join(root, "work") + for _, d := range []string{bare, work} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + gitIn(t, bare, "init", "-q", "--bare") + gitIn(t, work, "init", "-q") + if err := os.WriteFile(filepath.Join(work, "f"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + gitIn(t, work, "add", "f") + gitIn(t, work, "commit", "-q", "-m", "pull request head") + commit := gitIn(t, work, "rev-parse", "HEAD") + gitIn(t, work, "push", "-q", bare, "HEAD:refs/pull/1/head") + + ws := filepath.Join(root, "ws") + if err := os.MkdirAll(ws, 0o755); err != nil { + t.Fatal(err) + } + err := FetchCommit(context.Background(), forgekey.ExecRunner, ws, "file://"+bare, commit, "refs/pull/1/head", AllowFileRemote()) + if err != nil { + t.Fatal(err) + } + if head := gitIn(t, ws, "rev-parse", "HEAD"); head != commit { + t.Errorf("HEAD = %s, want %s", head, commit) + } +} diff --git a/go/buildstep/fetch_test.go b/go/buildstep/fetch_test.go new file mode 100644 index 0000000..b989ab6 --- /dev/null +++ b/go/buildstep/fetch_test.go @@ -0,0 +1,137 @@ +package buildstep + +import ( + "context" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +const sha = "0123456789abcdef0123456789abcdef01234567" + +// fakeGit records every command and answers by a rule. +type fakeGit struct { + calls []string + envs [][]string + rule func(args string) forgekey.Ran +} + +func (f *fakeGit) run(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + line := name + " " + strings.Join(args, " ") + f.calls = append(f.calls, line) + f.envs = append(f.envs, env) + if f.rule != nil { + if r := f.rule(line); r.Status != 0 || r.Stdout != "" || r.Err != "" { + return r + } + } + if len(args) > 4 && args[4] == "rev-parse" { + return forgekey.Ran{Stdout: sha + "\n"} + } + return forgekey.Ran{} +} + +const pre = "git -c credential.helper= -C /w " + +// 1474/AC1: the git commands are exactly the sequence, none with a credential or an https remote. +func TestFetchCommitRunsTheSequence(t *testing.T) { + f := &fakeGit{} + if err := FetchCommit(context.Background(), f.run, "/w", "ssh://mesh-forge/novox/mesh-sdk.git", sha, ""); err != nil { + t.Fatal(err) + } + want := []string{ + pre + "init -q", + pre + "remote add origin ssh://mesh-forge/novox/mesh-sdk.git", + pre + "fetch --no-tags --depth 1 origin " + sha, + pre + "checkout -q --detach " + sha, + pre + "rev-parse HEAD", + } + if strings.Join(f.calls, "\n") != strings.Join(want, "\n") { + t.Errorf("commands:\n%s\nwant:\n%s", strings.Join(f.calls, "\n"), strings.Join(want, "\n")) + } + for i, c := range f.calls { + if strings.Contains(c, "https://") || strings.Contains(c, "@") || !strings.Contains(c, "credential.helper= ") { + t.Errorf("command %q carries a credential or https", c) + } + if len(f.envs[i]) != 1 || f.envs[i][0] != "GIT_TERMINAL_PROMPT=0" { + t.Errorf("env %v", f.envs[i]) + } + } +} + +// 1474/AC2: the depth-1 fetch fails and a pull ref is given: the ref is fetched and the commit verified; with no +// ref, the fetch's failure is the answer. +func TestFetchCommitFallsBackToThePullRef(t *testing.T) { + f := &fakeGit{rule: func(l string) forgekey.Ran { + if strings.Contains(l, "--depth 1") { + return forgekey.Ran{Status: 128, Stderr: "fatal: couldn't find remote ref " + sha + "\n"} + } + return forgekey.Ran{} + }} + if err := FetchCommit(context.Background(), f.run, "/w", "ssh://mesh-forge/o/r.git", sha, "refs/pull/1/head"); err != nil { + t.Fatal(err) + } + want := []string{pre + "fetch --no-tags origin refs/pull/1/head", pre + "cat-file -e " + sha + "^{commit}", pre + "checkout -q --detach " + sha} + got := strings.Join(f.calls[3:6], "\n") + if got != strings.Join(want, "\n") { + t.Errorf("after the failed fetch:\n%s\nwant:\n%s", got, strings.Join(want, "\n")) + } + + g := &fakeGit{rule: f.rule} + err := FetchCommit(context.Background(), g.run, "/w", "ssh://mesh-forge/o/r.git", sha, "") + if err == nil || !strings.Contains(err.Error(), "couldn't find remote ref") || len(g.calls) != 3 { + t.Errorf("without a ref: %v after %d commands", err, len(g.calls)) + } + + h := &fakeGit{rule: func(l string) forgekey.Ran { + if strings.Contains(l, "--depth 1") || strings.Contains(l, "cat-file") { + return forgekey.Ran{Status: 1, Stderr: "no"} + } + return forgekey.Ran{} + }} + if err := FetchCommit(context.Background(), h.run, "/w", "ssh://mesh-forge/o/r.git", sha, "refs/pull/1/head"); err == nil { + t.Error("a commit that is not in the ref was accepted") + } +} + +// 1474/AC3: HEAD differs from the commit after the checkout: an error naming both. +func TestFetchCommitAnswersAHeadThatIsNotTheCommit(t *testing.T) { + other := strings.Repeat("a", 40) + f := &fakeGit{rule: func(l string) forgekey.Ran { + if strings.HasSuffix(l, "rev-parse HEAD") { + return forgekey.Ran{Stdout: other + "\n"} + } + return forgekey.Ran{} + }} + err := FetchCommit(context.Background(), f.run, "/w", "ssh://mesh-forge/o/r.git", sha, "") + if err == nil || !strings.Contains(err.Error(), other) || !strings.Contains(err.Error(), sha) { + t.Errorf("error = %v, want both shas", err) + } +} + +// 1474/AC4: an https remote, user information, a file remote without the option, or a commit that is not 40 +// hex digits is refused before anything runs. +func TestFetchCommitRefusesBeforeRunningAnything(t *testing.T) { + for _, c := range []struct{ remote, commit, ref string }{ + {"https://forge.example/o/r.git", sha, ""}, + {"http://forge.example/o/r.git", sha, ""}, + {"ssh://git@mesh-forge/o/r.git", sha, ""}, + {"ssh://user:pw@mesh-forge/o/r.git", sha, ""}, + {"file:///tmp/r.git", sha, ""}, + {"/tmp/r.git", sha, ""}, + {"ssh://mesh-forge/o/r.git", "abc123", ""}, + {"ssh://mesh-forge/o/r.git", strings.ToUpper(sha), ""}, + {"ssh://mesh-forge/o/r.git", "main", ""}, + {"ssh://mesh-forge/o/r.git", sha, "--upload-pack=x"}, + } { + f := &fakeGit{} + if err := FetchCommit(context.Background(), f.run, "/w", c.remote, c.commit, c.ref); err == nil || len(f.calls) != 0 { + t.Errorf("%+v: err %v, %d commands", c, err, len(f.calls)) + } + } + f := &fakeGit{} + if err := FetchCommit(context.Background(), f.run, "/w", "file:///tmp/r.git", sha, "", AllowFileRemote()); err != nil { + t.Errorf("a file remote with the option: %v", err) + } +} -- 2.54.0 From 512472c02af7bf15a9a85b9250cfa488514ec932 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:06:48 +0000 Subject: [PATCH 07/13] Pack a build's output like the build agent and keep the steps state's shape (issues 1471, 1479), part 7 of 7 --- go/buildstep/pack.go | 100 +++++++++++++++++++ go/buildstep/pack_test.go | 69 ++++++++++++++ go/buildstep/pack_unix_test.go | 25 +++++ go/buildstep/testdata/pack-golden.tar.gz | Bin 0 -> 182 bytes go/delivery/steps.go | 116 +++++++++++++++++++++++ go/delivery/steps_test.go | 71 ++++++++++++++ 6 files changed, 381 insertions(+) create mode 100644 go/buildstep/pack.go create mode 100644 go/buildstep/pack_test.go create mode 100644 go/buildstep/pack_unix_test.go create mode 100644 go/buildstep/testdata/pack-golden.tar.gz create mode 100644 go/delivery/steps.go create mode 100644 go/delivery/steps_test.go diff --git a/go/buildstep/pack.go b/go/buildstep/pack.go new file mode 100644 index 0000000..bab02b2 --- /dev/null +++ b/go/buildstep/pack.go @@ -0,0 +1,100 @@ +package buildstep + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "sort" +) + +// Pack tars and gzips a directory the way the build agent does, byte for byte, and answers the archive and its +// digest, "sha256:" and the hex of the archive's bytes (issue 1479; novox/hq ADR 0405 §7, ADR 0453 §7). It is a +// copy of mesh-controller's internal/builder/builder.go pack (origin/main 073ccc29, :1351-1425), which stays +// where it is until the build agent goes: a build module's digest is compared with the build agent's for one +// fingerprint, so the bytes must be equal. +// +// Deterministically: regular files only (a symlink or a device is refused), sorted relative paths with +// slashes, each header carrying its name, permission bits and size and nothing else, the gzip default writer. +func Pack(root string) (archive []byte, digest string, err error) { + raw, err := pack(root) + if err != nil { + return nil, "", err + } + sum := sha256.Sum256(raw) + return raw, "sha256:" + hex.EncodeToString(sum[:]), nil +} + +func pack(root string) ([]byte, error) { + info, err := os.Stat(root) + if err != nil { + return nil, err + } + if !info.IsDir() { + return nil, fmt.Errorf("%s is not a directory", root) + } + + var paths []string + err = filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() || !info.Mode().IsRegular() { + // Only files. A symlink or a device in an archive is refused by the host that unpacks + // it, so putting one in would build something that cannot be applied. + if !info.IsDir() && !info.Mode().IsRegular() { + return fmt.Errorf("%s is neither a file nor a directory, and an archive carries "+ + "only those", path) + } + return nil + } + paths = append(paths, path) + return nil + }) + if err != nil { + return nil, err + } + // filepath.Walk walks in lexical order; the sort is kept as the build agent keeps it, since an archive + // whose digest changes because the traversal did is a silent failure. + sort.Strings(paths) + + var out bytes.Buffer + zipped := gzip.NewWriter(&out) + writer := tar.NewWriter(zipped) + for _, path := range paths { + body, err := os.ReadFile(path) + if err != nil { + return nil, err + } + relative, err := filepath.Rel(root, path) + if err != nil { + return nil, err + } + info, err := os.Stat(path) + if err != nil { + return nil, err + } + mode := int64(info.Mode().Perm()) + if err := writer.WriteHeader(&tar.Header{ + Name: filepath.ToSlash(relative), Mode: mode, Size: int64(len(body)), + Typeflag: tar.TypeReg, + // Everything else left at its zero value on purpose: no times, owners or names. + }); err != nil { + return nil, err + } + if _, err := writer.Write(body); err != nil { + return nil, err + } + } + if err := writer.Close(); err != nil { + return nil, err + } + if err := zipped.Close(); err != nil { + return nil, err + } + return out.Bytes(), nil +} diff --git a/go/buildstep/pack_test.go b/go/buildstep/pack_test.go new file mode 100644 index 0000000..3f5282d --- /dev/null +++ b/go/buildstep/pack_test.go @@ -0,0 +1,69 @@ +package buildstep + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "os" + "path/filepath" + "testing" + "time" +) + +// fixture writes three files in two directories (modes 0644 and 0755). +func fixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + for _, f := range []struct { + p string + mode os.FileMode + body string + }{{"a/x.txt", 0o644, "x\n"}, {"a/run.sh", 0o755, "#!/bin/sh\necho hi\n"}, {"b/c/y.txt", 0o644, "why\n"}} { + path := filepath.Join(root, f.p) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(f.body), f.mode); err != nil { + t.Fatal(err) + } + if err := os.Chmod(path, f.mode); err != nil { + t.Fatal(err) + } + } + return root +} + +// 1479/AC1: the archive equals the bytes mesh-controller's pack makes for this tree. testdata/pack-golden.tar.gz +// was made once by running mesh-controller internal/builder/builder.go pack (origin/main 073ccc29, :1351-1425) +// on this fixture (fixture above, modes set by chmod). +func TestPackEqualsTheBuildAgentsArchive(t *testing.T) { + golden, err := os.ReadFile("testdata/pack-golden.tar.gz") + if err != nil { + t.Fatal(err) + } + archive, digest, err := Pack(fixture(t)) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(archive, golden) { + t.Errorf("the archive is %d bytes, the build agent's %d, and they differ", len(archive), len(golden)) + } + sum := sha256.Sum256(golden) + if digest != "sha256:"+hex.EncodeToString(sum[:]) { + t.Errorf("digest = %s", digest) + } +} + +// 1479/AC2: touching every file's time changes nothing. +func TestPackDigestIgnoresFileTimes(t *testing.T) { + root := fixture(t) + _, first, err := Pack(root) + if err != nil { + t.Fatal(err) + } + later := time.Now().Add(48 * time.Hour) + filepath.Walk(root, func(p string, _ os.FileInfo, _ error) error { return os.Chtimes(p, later, later) }) + if _, second, err := Pack(root); err != nil || second != first { + t.Errorf("digests %s and %s (%v)", first, second, err) + } +} diff --git a/go/buildstep/pack_unix_test.go b/go/buildstep/pack_unix_test.go new file mode 100644 index 0000000..57a5052 --- /dev/null +++ b/go/buildstep/pack_unix_test.go @@ -0,0 +1,25 @@ +//go:build unix + +package buildstep + +import ( + "path/filepath" + "strings" + "syscall" + "testing" +) + +// 1479/AC3: anything but a file or a directory is refused, naming it. The rule of this repository is that no +// test makes a symlink, so the non-regular entry is a named pipe: Pack refuses a symlink on the same branch +// (filepath.Walk reports it by lstat as not regular). +func TestPackRefusesAnEntryThatIsNotAFile(t *testing.T) { + root := fixture(t) + pipe := filepath.Join(root, "a", "pipe") + if err := syscall.Mkfifo(pipe, 0o644); err != nil { + t.Fatal(err) + } + _, _, err := Pack(root) + if err == nil || !strings.Contains(err.Error(), pipe) { + t.Errorf("Pack = %v, want a refusal naming %s", err, pipe) + } +} diff --git a/go/buildstep/testdata/pack-golden.tar.gz b/go/buildstep/testdata/pack-golden.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..e5ea0a8420d04b076252552dbfca36625b1783e4 GIT binary patch literal 182 zcmV;n07?HJiwFP!00000|Ll~p3d0}}hMn^iDcy5_t)|aoLcuI4L_^Knm(ZGm9Xb?4 z!TbK<^aOSJ&d@ZEjk6*n0C3S-;7M8mDc|e0e}ZnMs1Yy7$?UwD1b%*N`~0iM{-1L^ zP&bU^Y<2DJt+qoIPnZ%g6dE6V+-70*p;c;ABdGvB{8v)5|9ki!*!QA*Pt#FPOXdmi k-G7z(-|92_zlZ;KyYK`kdIE4boKo``0CCQ}QveVE07`{fK>z>% literal 0 HcmV?d00001 diff --git a/go/delivery/steps.go b/go/delivery/steps.go new file mode 100644 index 0000000..61f8114 --- /dev/null +++ b/go/delivery/steps.go @@ -0,0 +1,116 @@ +package delivery + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + "time" +) + +// StepsState is the name of the state a build module keeps its long steps in; its bucket is `_steps`, built +// by StepsBucket (novox/hq ADR 0469 clause 2; issue 1471). A tool call is cut at 30 s, so a fetch, a test level or a +// build answers at once with a step id and writes its progress and its end to the key `.`. +const StepsState = "steps" + +// StepsBucket is the bucket of a build module's steps. +func StepsBucket(module string) string { return module + "_" + StepsState } + +// The states of a step. A step ends in passed, failed or refused; its key is written for each of them, and that +// write is the event the step's reader waits on. +const ( + StepRunning = "running" + StepPassed = "passed" + StepFailed = "failed" + StepRefused = "refused" +) + +var stepIDOK = regexp.MustCompile(`^[a-z0-9]+$`) + +func checkStepID(id string) error { + if !stepIDOK.MatchString(id) { + return fmt.Errorf("delivery: the step id %q is not lower-case letters and digits", id) + } + return nil +} + +// StepKey is the key of a node's step: `.`. It refuses a node that is not a name and an id that is not +// lower-case letters and digits (one key token, never a dot). +func StepKey(node, id string) (string, error) { + if err := checkName("node", node); err != nil { + return "", err + } + if err := checkStepID(id); err != nil { + return "", err + } + return node + "." + id, nil +} + +// ParseStepKey is the inverse of StepKey. +func ParseStepKey(key string) (node, id string, err error) { + node, id, ok := strings.Cut(key, ".") + if !ok { + return "", "", fmt.Errorf("delivery: %q is not a step key, .", key) + } + if _, err := StepKey(node, id); err != nil { + return "", "", fmt.Errorf("delivery: %q is not a step key: %w", key, err) + } + return node, id, nil +} + +// Step is the value of a steps key: which tool runs on which node, how far it is, when it started and ended (UTC, as +// the toolchains' At), why it was refused or failed, and what it returned. +type Step struct { + V int `json:"v"` + ID string `json:"id"` + Tool string `json:"tool"` + Node string `json:"node"` + State string `json:"state"` + Started time.Time `json:"started"` + Ended time.Time `json:"ended,omitzero"` + Reason string `json:"reason,omitempty"` + Result json.RawMessage `json:"result,omitempty"` +} + +func (s Step) check() error { + if err := checkStepID(s.ID); err != nil { + return err + } + if err := checkName("node", s.Node); err != nil { + return err + } + if s.Tool == "" { + return fmt.Errorf("delivery: a step needs its tool") + } + switch s.State { + case StepRunning, StepPassed, StepFailed, StepRefused: + default: + return fmt.Errorf("delivery: the step state %q is not running, passed, failed or refused", s.State) + } + if len(s.Result) > 0 && !json.Valid(s.Result) { + return fmt.Errorf("delivery: the step's result is not JSON") + } + return nil +} + +// EncodeStep is s as JSON at Version; it refuses an id that is not one key token, a node that is not a name, an empty +// tool, a state outside the four and a result that is not JSON. +func EncodeStep(s Step) ([]byte, error) { + if err := s.check(); err != nil { + return nil, err + } + s.V = Version + return json.Marshal(s) +} + +// DecodeStep reads a steps value; it refuses an unknown version (ErrUnknownVersion) and what EncodeStep refuses. +func DecodeStep(data []byte) (Step, error) { + var s Step + if err := decode(data, &s); err != nil { + return Step{}, err + } + if err := s.check(); err != nil { + return Step{}, err + } + return s, nil +} diff --git a/go/delivery/steps_test.go b/go/delivery/steps_test.go new file mode 100644 index 0000000..b9dbb7f --- /dev/null +++ b/go/delivery/steps_test.go @@ -0,0 +1,71 @@ +package delivery + +import ( + "errors" + "strings" + "testing" + "time" +) + +// 1471/AC4: an encoded step with V 99 is refused with ErrUnknownVersion; so is a state 'done'. +func TestDecodeStepRefusesAnUnknownVersionAndAState(t *testing.T) { + good, err := EncodeStep(Step{ID: "k3j2", Tool: "fetch", Node: "ace", State: StepRunning, Started: time.Date(2026, 10, 20, 12, 0, 0, 0, time.UTC)}) + if err != nil { + t.Fatal(err) + } + if _, err := DecodeStep(good); err != nil { + t.Fatalf("a good step: %v", err) + } + v99 := strings.Replace(string(good), `"v":1`, `"v":99`, 1) + if _, err := DecodeStep([]byte(v99)); !errors.Is(err, ErrUnknownVersion) { + t.Errorf("version 99: %v, want ErrUnknownVersion", err) + } + done := strings.Replace(string(good), `"running"`, `"done"`, 1) + if _, err := DecodeStep([]byte(done)); err == nil || errors.Is(err, ErrUnknownVersion) { + t.Errorf("state done: %v, want a refusal of the state", err) + } + if _, err := EncodeStep(Step{ID: "a", Tool: "fetch", Node: "ace", State: "done"}); err == nil { + t.Error("EncodeStep wrote the state done") + } +} + +func TestStepKeyIsNodeDotIDAndRefusesWhatIsNotOneToken(t *testing.T) { + key, err := StepKey("ace", "k3j2") + if err != nil || key != "ace.k3j2" { + t.Fatalf("StepKey = %q, %v", key, err) + } + node, id, err := ParseStepKey(key) + if err != nil || node != "ace" || id != "k3j2" { + t.Errorf("ParseStepKey = %q, %q, %v", node, id, err) + } + for _, id := range []string{"", "a.b", "A", "a-b", "a b", "*"} { + if _, err := StepKey("ace", id); err == nil { + t.Errorf("StepKey accepted the id %q", id) + } + } + for _, key := range []string{"ace", "ace.", "ace.a.b"} { + if _, _, err := ParseStepKey(key); err == nil { + t.Errorf("ParseStepKey accepted %q", key) + } + } + if StepsBucket("mesh-build-go") != "mesh-build-go_steps" { + t.Errorf("bucket = %q", StepsBucket("mesh-build-go")) + } +} + +func TestStepEncodingKeepsItsFieldsAndOmitsAnOpenEnd(t *testing.T) { + at := time.Date(2026, 10, 20, 12, 0, 0, 0, time.UTC) + raw, _ := EncodeStep(Step{ID: "a1", Tool: "fetch", Node: "ace", State: StepRunning, Started: at}) + want := `{"v":1,"id":"a1","tool":"fetch","node":"ace","state":"running","started":"2026-10-20T12:00:00Z"}` + if string(raw) != want { + t.Errorf("running = %s", raw) + } + raw, _ = EncodeStep(Step{ID: "a1", Tool: "fetch", Node: "ace", State: StepPassed, Started: at, Ended: at, Result: []byte(`{"sha":"x"}`)}) + got, err := DecodeStep(raw) + if err != nil || got.State != StepPassed || string(got.Result) != `{"sha":"x"}` || !got.Ended.Equal(at) { + t.Errorf("passed = %+v, %v", got, err) + } + if _, err := EncodeStep(Step{ID: "a1", Tool: "fetch", Node: "ace", State: StepPassed, Result: []byte(`{`)}); err == nil { + t.Error("a result that is not JSON was written") + } +} -- 2.54.0 From 9e296295c05e4162db9dca9dcf6c05f5d2795e21 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:07:15 +0000 Subject: [PATCH 08/13] Let a long step's exec runner be bound by its context alone (issue 1497), part 8 of 8 --- go/forgekey/runner.go | 26 ++++++++++++++++++----- go/forgekey/runner_test.go | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 go/forgekey/runner_test.go diff --git a/go/forgekey/runner.go b/go/forgekey/runner.go index 9cc08e1..bc8628c 100644 --- a/go/forgekey/runner.go +++ b/go/forgekey/runner.go @@ -23,14 +23,30 @@ type Ran struct { // Runner runs one command with extra environment words, so every tool can be tested without ssh. type Runner func(ctx context.Context, env []string, name string, args ...string) Ran -// CallTimeout bounds one command: below the runtime's thirty-second call limit. -const CallTimeout = 20 * time.Second +// CallTimeout bounds one command of ExecRunner: below the runtime's thirty-second call limit. A variable so a +// test can lower it. +var CallTimeout = 20 * time.Second // ExecRunner runs a command on this machine, with no terminal and nothing on its stdin, bounded by // CallTimeout. func ExecRunner(ctx context.Context, env []string, name string, args ...string) Ran { - ctx, cancel := context.WithTimeout(ctx, CallTimeout) - defer cancel() + return ExecRunnerFor(CallTimeout)(ctx, env, name, args...) +} + +// ExecRunnerFor is ExecRunner with its own bound (issue 1497): a build module's long step passes 0, so only the +// step's own context ends a command, and a fetch may outlast a tool call's thirty seconds. +func ExecRunnerFor(bound time.Duration) Runner { + return func(ctx context.Context, env []string, name string, args ...string) Ran { + return execRun(ctx, bound, env, name, args...) + } +} + +func execRun(ctx context.Context, bound time.Duration, env []string, name string, args ...string) Ran { + if bound > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, bound) + defer cancel() + } cmd := exec.CommandContext(ctx, name, args...) cmd.Env = append(os.Environ(), env...) var out, errb bytes.Buffer @@ -40,7 +56,7 @@ func ExecRunner(ctx context.Context, env []string, name string, args ...string) var exitErr *exec.ExitError switch { case errors.Is(ctx.Err(), context.DeadlineExceeded): - r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(CallTimeout/time.Second)) + r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(bound/time.Second)) case errors.Is(err, exec.ErrNotFound): r.Status, r.Err = 127, "ENOENT" case errors.As(err, &exitErr): diff --git a/go/forgekey/runner_test.go b/go/forgekey/runner_test.go new file mode 100644 index 0000000..8b94fd2 --- /dev/null +++ b/go/forgekey/runner_test.go @@ -0,0 +1,42 @@ +package forgekey + +import ( + "context" + "testing" + "time" +) + +func lowered(t *testing.T) { + was := CallTimeout + CallTimeout = 50 * time.Millisecond + t.Cleanup(func() { CallTimeout = was }) +} + +// 1497/AC1: ExecRunnerFor(0), no deadline: a command longer than the lowered CallTimeout is not cut. +func TestExecRunnerForZeroIsNotCutByCallTimeout(t *testing.T) { + lowered(t) + r := ExecRunnerFor(0)(context.Background(), nil, "sleep", "0.2") + if r.Status != 0 || r.Err != "" { + t.Errorf("ran = %+v, want status 0", r) + } +} + +// 1497/AC2: ExecRunner is unchanged: cut at CallTimeout with status 124. +func TestExecRunnerIsStillCutAtCallTimeout(t *testing.T) { + lowered(t) + r := ExecRunner(context.Background(), nil, "sleep", "0.2") + if r.Status != 124 { + t.Errorf("ran = %+v, want status 124", r) + } +} + +// 1497/AC3: ExecRunnerFor(0) is cut when its context is cancelled. +func TestExecRunnerForZeroIsCutByItsContext(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + began := time.Now() + r := ExecRunnerFor(0)(ctx, nil, "sleep", "2") + if r.Status == 0 || time.Since(began) > time.Second { + t.Errorf("ran = %+v after %v, want it cut", r, time.Since(began)) + } +} -- 2.54.0 From 0b990a9c31dad7c5ab953b00bc33dd246b8fe1b2 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:09:32 +0000 Subject: [PATCH 09/13] Name the step's deadline, not a bound never set, when an unbounded command is cut (issue 1497) --- go/forgekey/runner.go | 5 ++++- go/forgekey/runner_test.go | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/go/forgekey/runner.go b/go/forgekey/runner.go index bc8628c..8fe5040 100644 --- a/go/forgekey/runner.go +++ b/go/forgekey/runner.go @@ -56,7 +56,10 @@ func execRun(ctx context.Context, bound time.Duration, env []string, name string var exitErr *exec.ExitError switch { case errors.Is(ctx.Err(), context.DeadlineExceeded): - r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(bound/time.Second)) + r.Status, r.Err = 124, "ended by its step's deadline" + if bound > 0 { + r.Err = fmt.Sprintf("no answer within %d s", int(bound/time.Second)) + } case errors.Is(err, exec.ErrNotFound): r.Status, r.Err = 127, "ENOENT" case errors.As(err, &exitErr): diff --git a/go/forgekey/runner_test.go b/go/forgekey/runner_test.go index 8b94fd2..79f9825 100644 --- a/go/forgekey/runner_test.go +++ b/go/forgekey/runner_test.go @@ -36,6 +36,9 @@ func TestExecRunnerForZeroIsCutByItsContext(t *testing.T) { defer cancel() began := time.Now() r := ExecRunnerFor(0)(ctx, nil, "sleep", "2") + if r.Err != "ended by its step's deadline" { + t.Errorf("Err = %q, want the step's deadline named, not a bound that was never set", r.Err) + } if r.Status == 0 || time.Since(began) > time.Second { t.Errorf("ran = %+v after %v, want it cut", r, time.Since(began)) } -- 2.54.0 From 8406bb3a77276ee4b21b8a250dfab38600c3a5a5 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:38:47 +0000 Subject: [PATCH 10/13] Compare Pack's tar stream with the build agent's, since gzip's bytes differ between Go versions (issue 1479) --- go/buildstep/pack_test.go | 31 +++++++++++++++++------ go/buildstep/testdata/pack-golden.tar.gz | Bin 182 -> 0 bytes 2 files changed, 23 insertions(+), 8 deletions(-) delete mode 100644 go/buildstep/testdata/pack-golden.tar.gz diff --git a/go/buildstep/pack_test.go b/go/buildstep/pack_test.go index 3f5282d..d789052 100644 --- a/go/buildstep/pack_test.go +++ b/go/buildstep/pack_test.go @@ -2,8 +2,10 @@ package buildstep import ( "bytes" + "compress/gzip" "crypto/sha256" "encoding/hex" + "io" "os" "path/filepath" "testing" @@ -33,11 +35,13 @@ func fixture(t *testing.T) string { return root } -// 1479/AC1: the archive equals the bytes mesh-controller's pack makes for this tree. testdata/pack-golden.tar.gz -// was made once by running mesh-controller internal/builder/builder.go pack (origin/main 073ccc29, :1351-1425) -// on this fixture (fixture above, modes set by chmod). +// 1479/AC1: the tar stream inside the archive equals the bytes mesh-controller's pack makes for this tree. +// testdata/pack-golden.tar is the uncompressed stream, made once by running mesh-controller +// internal/builder/builder.go pack (origin/main 073ccc29, :1351-1425) on this fixture (above, modes set by chmod) +// and gunzipping its archive. The gzip layer is not compared byte for byte, since compress/gzip's output differs +// between Go versions; its header is checked to carry no name, no comment and no time. func TestPackEqualsTheBuildAgentsArchive(t *testing.T) { - golden, err := os.ReadFile("testdata/pack-golden.tar.gz") + golden, err := os.ReadFile("testdata/pack-golden.tar") if err != nil { t.Fatal(err) } @@ -45,12 +49,23 @@ func TestPackEqualsTheBuildAgentsArchive(t *testing.T) { if err != nil { t.Fatal(err) } - if !bytes.Equal(archive, golden) { - t.Errorf("the archive is %d bytes, the build agent's %d, and they differ", len(archive), len(golden)) + zr, err := gzip.NewReader(bytes.NewReader(archive)) + if err != nil { + t.Fatal(err) } - sum := sha256.Sum256(golden) + if zr.Name != "" || zr.Comment != "" || !zr.ModTime.IsZero() { + t.Errorf("gzip header = name %q, comment %q, time %v; want none", zr.Name, zr.Comment, zr.ModTime) + } + tarBytes, err := io.ReadAll(zr) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(tarBytes, golden) { + t.Errorf("the tar stream is %d bytes, the build agent's %d, and they differ", len(tarBytes), len(golden)) + } + sum := sha256.Sum256(archive) if digest != "sha256:"+hex.EncodeToString(sum[:]) { - t.Errorf("digest = %s", digest) + t.Errorf("digest = %s, not the sha256 of the archive", digest) } } diff --git a/go/buildstep/testdata/pack-golden.tar.gz b/go/buildstep/testdata/pack-golden.tar.gz deleted file mode 100644 index e5ea0a8420d04b076252552dbfca36625b1783e4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 182 zcmV;n07?HJiwFP!00000|Ll~p3d0}}hMn^iDcy5_t)|aoLcuI4L_^Knm(ZGm9Xb?4 z!TbK<^aOSJ&d@ZEjk6*n0C3S-;7M8mDc|e0e}ZnMs1Yy7$?UwD1b%*N`~0iM{-1L^ zP&bU^Y<2DJt+qoIPnZ%g6dE6V+-70*p;c;ABdGvB{8v)5|9ki!*!QA*Pt#FPOXdmi k-G7z(-|92_zlZ;KyYK`kdIE4boKo``0CCQ}QveVE07`{fK>z>% -- 2.54.0 From df9a2bba8c4bdbc51bce70a8ee20e12952d64d3d Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:38:59 +0000 Subject: [PATCH 11/13] Add the build agent's uncompressed tar stream as Pack's golden (issue 1479) --- go/buildstep/testdata/pack-golden.tar | Bin 0 -> 512 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 go/buildstep/testdata/pack-golden.tar diff --git a/go/buildstep/testdata/pack-golden.tar b/go/buildstep/testdata/pack-golden.tar new file mode 100644 index 0000000000000000000000000000000000000000..99075dd36fb9447ec5d7eeddbd8226ddfb9854dd GIT binary patch literal 512 zcmYe)FDlK`E6!k`EieEAb5m0W5Df(Aw1I(<5ljpsZ)jj@X2hUiKvie4hnu^JD=;%t iY$y)|Fu-Vv4WyF+%8L3)nR)uf8C Date: Tue, 20 Oct 2026 20:39:23 +0000 Subject: [PATCH 12/13] Restore the full tar stream of Pack's golden (issue 1479) --- go/buildstep/testdata/pack-golden.tar | Bin 512 -> 4096 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/go/buildstep/testdata/pack-golden.tar b/go/buildstep/testdata/pack-golden.tar index 99075dd36fb9447ec5d7eeddbd8226ddfb9854dd..7e844277839d00f940226333ad9463b439c2695a 100644 GIT binary patch literal 4096 zcmeHH!488U4CU-sxa@8#1NJ>=mf*!Df|>sPz)iBa!x9sihxSrZps#)Ks$m%W${Lo? z(A!l?7QDBuN925sz4mLAin9_E))Z5Z*6G2+$H30T(%MIJJJaO8c0NJV^g9Sly~xp^qzy}!(ueh!kl;l{MLV* g4dE!m8kM||{&ReqhnZ;tjK}4yItWt$ -- 2.54.0 From 1572e50e0ef0e91ffe224dbec545735d7b51b60e Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:39:45 +0000 Subject: [PATCH 13/13] Give a build step's commands only the environment its caller names (issue 1497) --- go/forgekey/runner.go | 19 ++++++++++++++++--- go/forgekey/runner_test.go | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/go/forgekey/runner.go b/go/forgekey/runner.go index 8fe5040..14bba8a 100644 --- a/go/forgekey/runner.go +++ b/go/forgekey/runner.go @@ -37,18 +37,31 @@ func ExecRunner(ctx context.Context, env []string, name string, args ...string) // step's own context ends a command, and a fetch may outlast a tool call's thirty seconds. func ExecRunnerFor(bound time.Duration) Runner { return func(ctx context.Context, env []string, name string, args ...string) Ran { - return execRun(ctx, bound, env, name, args...) + return execRun(ctx, bound, false, env, name, args...) } } -func execRun(ctx context.Context, bound time.Duration, env []string, name string, args ...string) Ran { +// ExecRunnerClean is ExecRunnerFor whose commands get exactly the env argument as their environment, and nothing of +// this process's (issue 1497; novox/hq ADR 0466 §4: no registry password in a fetch's environment). The caller +// gives PATH and HOME itself. A directory is the command's own business (`git -C`, `go -C`). +func ExecRunnerClean(bound time.Duration) Runner { + return func(ctx context.Context, env []string, name string, args ...string) Ran { + return execRun(ctx, bound, true, env, name, args...) + } +} + +func execRun(ctx context.Context, bound time.Duration, clean bool, env []string, name string, args ...string) Ran { if bound > 0 { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, bound) defer cancel() } cmd := exec.CommandContext(ctx, name, args...) - cmd.Env = append(os.Environ(), env...) + if clean { + cmd.Env = append([]string{}, env...) // non-nil, so an empty list is an empty environment + } else { + cmd.Env = append(os.Environ(), env...) + } var out, errb bytes.Buffer cmd.Stdout, cmd.Stderr = &out, &errb err := cmd.Run() diff --git a/go/forgekey/runner_test.go b/go/forgekey/runner_test.go index 79f9825..9c26b65 100644 --- a/go/forgekey/runner_test.go +++ b/go/forgekey/runner_test.go @@ -2,6 +2,8 @@ package forgekey import ( "context" + "os" + "strings" "testing" "time" ) @@ -43,3 +45,21 @@ func TestExecRunnerForZeroIsCutByItsContext(t *testing.T) { t.Errorf("ran = %+v after %v, want it cut", r, time.Since(began)) } } + +// 1497/AC4: ExecRunnerClean(0) runs env with only the given words: PATH and HOME, not the process's secret. +func TestExecRunnerCleanGivesTheCommandOnlyTheEnvArgument(t *testing.T) { + t.Setenv("MESH_TEST_SECRET", "x") + r := ExecRunnerClean(0)(context.Background(), []string{"PATH=" + os.Getenv("PATH"), "HOME=/h"}, "env") + if r.Status != 0 || !strings.Contains(r.Stdout, "HOME=/h") || !strings.Contains(r.Stdout, "PATH=") || strings.Contains(r.Stdout, "MESH_TEST_SECRET") { + t.Errorf("ran = %+v", r) + } +} + +// 1497/AC5: ExecRunnerFor(0) is unchanged: the env argument is added to the process's environment. +func TestExecRunnerForAddsToTheProcessEnvironment(t *testing.T) { + t.Setenv("MESH_TEST_SECRET", "x") + r := ExecRunnerFor(0)(context.Background(), []string{"HOME=/h"}, "env") + if !strings.Contains(r.Stdout, "MESH_TEST_SECRET=x") || !strings.Contains(r.Stdout, "HOME=/h") { + t.Errorf("ran = %+v", r) + } +} -- 2.54.0