diff --git a/internal/apply/process.go b/internal/apply/process.go index 5e05d99..7eb2a4d 100644 --- a/internal/apply/process.go +++ b/internal/apply/process.go @@ -4,10 +4,13 @@ import ( "context" "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" + "maps" "os" "path/filepath" "runtime" + "slices" "strings" "time" @@ -260,6 +263,9 @@ func applyProcess(ctx context.Context, r *declaration.Process, run Runner, } if because != "" { out.Detail += ", restarted because " + because + " changed" + } else if what := unitDifference(previous.Wrote, want); what != "" { + // Said, so a restart nobody asked for names its cause (issue 1966): the file and the input of it, never a value. + out.Detail += ", restarted because " + what + " changed" } if placing.Detail != "" { out.Detail += "; " + placing.Detail @@ -553,3 +559,81 @@ func digestWritten(wrote string) string { // DaemonRoot is where unpacked daemons live — and what a witness keeps beside them. func DaemonRoot() string { return daemonRoot } + +// unitDifference names what differs between what a process's record says was written and what is wanted now, both +// "sha256: ": its bundle when the digest differs, and in the unit file each directive and each environment +// word that differs, with inside a word the keys of a JSON object (and, one level down, of its objects) that differ. Names only, never a value, so a secret in a word is never said. Empty +// when nothing differs or there was no record. +func unitDifference(was, want string) string { + if was == "" { + return "" + } + wasDigest, wasUnit, _ := strings.Cut(was, " ") + wantDigest, wantUnit, _ := strings.Cut(want, " ") + var names []string + if wasDigest != wantDigest { + names = append(names, "its bundle") + } + before, after := unitDirectives(wasUnit), unitDirectives(wantUnit) + keys := map[string]bool{} + for k := range before { + keys[k] = true + } + for k := range after { + keys[k] = true + } + for _, k := range slices.Sorted(maps.Keys(keys)) { + if before[k] != after[k] { + names = append(names, "its unit's "+k+valueDifference(before[k], after[k], 2)) + } + } + return strings.Join(names, ", ") +} + +// unitDirectives is a unit's directives by name: "Environment " for an environment word, the directive's own +// name for the rest, each with its value as written (an environment word's value unescaped, as unitValue escapes it). +func unitDirectives(unit string) map[string]string { + out := map[string]string{} + for _, line := range strings.Split(unit, "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, "[") { + continue + } + key, value, _ := strings.Cut(line, "=") + if key == "Environment" && strings.HasPrefix(value, `"`) && strings.HasSuffix(value, `"`) { + word, rest, _ := strings.Cut(value[1:len(value)-1], "=") + rest = strings.NewReplacer(`\\`, `\`, `\"`, `"`, "%%", "%").Replace(rest) + out["Environment "+word] = rest + continue + } + out[key] += value + } + return out +} + +// valueDifference is what differs inside one directive's value, as " (a, b)": the keys of a JSON object whose value +// differs (going down depth levels), else the names of the name=value entries of a comma-separated list that were +// added, removed or changed. Empty when the value says nothing more than that it differs. +func valueDifference(was, now string, depth int) string { + var a, b map[string]json.RawMessage + if depth > 0 && json.Unmarshal([]byte(was), &a) == nil && json.Unmarshal([]byte(now), &b) == nil && (a != nil || b != nil) { + all := map[string]bool{} + for k := range a { + all[k] = true + } + for k := range b { + all[k] = true + } + var names []string + for _, k := range slices.Sorted(maps.Keys(all)) { + if string(a[k]) == string(b[k]) { + continue + } + inner := valueDifference(string(a[k]), string(b[k]), depth-1) + names = append(names, k+inner) + } + return " (" + strings.Join(names, ", ") + ")" + } + // Anything else is a value: said as differing, never split, since a name before an "=" in it can be a secret. + return "" +} diff --git a/internal/apply/process_test.go b/internal/apply/process_test.go index 124d8ca..146ce1b 100644 --- a/internal/apply/process_test.go +++ b/internal/apply/process_test.go @@ -357,6 +357,53 @@ func TestAProcessAppliedAgainIsUnchangedAndNotRestarted(t *testing.T) { if o := outcomeOf(third, "node-tools.runtime"); o.Action != "unchanged" { t.Errorf("third apply: %+v, want unchanged", o) } + // An input of its unit that differs restarts it, and the line says which (issue 1966). + changed := declare(t, `{"id":"node-tools.runtime","type":"process","name":"node-tools","source":"`+serving(t, body)+ + `","digest":"`+digest+`","run":["node","main.js"],"env":{"MESH_TOOL_MODULES":"a=/x/index.js,b=/y/index.js"}}`) + fourth, _, err := Apply(context.Background(), archHost(t), changed, state, store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(fourth, "node-tools.runtime"); o.Action != "updated" || !strings.Contains(o.Detail, "its unit's Environment MESH_TOOL_MODULES") || + strings.Contains(o.Detail, "/y/") { + t.Errorf("fourth apply: %+v, want updated and the word that changed named, no value", o) + } + for _, row := range []struct { + name, was, now, says string + }{ + {"a word of a module inside a JSON word", `{"files":{"OWNED":"/a"},"mail":{"X":"1"}}`, `{"files":{"OWNED":"/b"},"mail":{"X":"1"}}`, + "Environment TOOL_ENV (files (OWNED))"}, + {"a secret-shaped value", "c2VjcmV0MQ==", "c2VjcmV0Mg==", "Environment TOOL_VALUE"}, + {"a URL with a password", "postgres://u:hunter1@h/db?sslmode=x", "postgres://u:hunter2@h/db?sslmode=x", "Environment TOOL_VALUE"}, + {"nothing changed", "a=/x/1", "a=/x/1", ""}, + } { + t.Run(row.name, func(t *testing.T) { + word := "TOOL_VALUE" + if strings.HasPrefix(row.was, "{") { + word = "TOOL_ENV" + } + r := aProcess() + mk := func(v string) string { + r.Env = map[string]string{word: v} + return "sha256:" + strings.Repeat("a", 64) + " " + unitFor(r) + } + got := unitDifference(mk(row.was), mk(row.now)) + if row.says == "" { + if got != "" { + t.Fatalf("said %q for nothing changed", got) + } + return + } + for _, leak := range []string{"c2VjcmV0", "hunter", "sslmode", "postgres", "/b"} { + if strings.Contains(got, leak) { + t.Fatalf("said %q, which holds part of a value (%s)", got, leak) + } + } + if !strings.Contains(got, row.says) { + t.Fatalf("said %q, want it to name %q", got, row.says) + } + }) + } } // novox/hq ADR 0193: a bundle compiled to a binary runs itself — `./name` is that file in the diff --git a/internal/compose/resolution/seat_paths.go b/internal/compose/resolution/seat_paths.go index d1c41f9..5f8ece6 100644 --- a/internal/compose/resolution/seat_paths.go +++ b/internal/compose/resolution/seat_paths.go @@ -26,7 +26,7 @@ func declaredPaths(m mcat.Manifest) map[string]bool { } for _, r := range m.Resources { if p, ok := r["path"].(string); ok && strings.HasPrefix(p, "/") { - out[path.Clean(p)] = true + out[path.Clean(versionedPlaceOwner(r, p))] = true } } for _, f := range m.Facts { @@ -123,3 +123,18 @@ func seatPathsOnMachine(modules []mcat.Manifest) []string { } return problems } + +// versionedPlaceOwner is the path a resource owns: its own, unless the resource is an archive unpacked into a place named +// for its build (the artifact's digest, short: `.../versions/`, catalogue `${version}`), whose owner is the +// directory holding every such place, because the place changes with each build and the owned paths are an input of the +// tool runner's unit (the holder of node-files reads them at its start): a build of the engine itself renamed its place +// and restarted the tool runner, with nothing of the tool runner changed (issue 1966). +func versionedPlaceOwner(r map[string]any, p string) string { + digest, _ := r["digest"].(string) + version := strings.TrimPrefix(digest, "sha256:") + base := path.Base(path.Clean(p)) + if r["type"] == "archive" && len(base) >= 8 && strings.HasPrefix(version, base) { + return path.Dir(path.Clean(p)) + } + return p +} diff --git a/internal/compose/resolution/seat_paths_test.go b/internal/compose/resolution/seat_paths_test.go index 38bfa8f..82b7e22 100644 --- a/internal/compose/resolution/seat_paths_test.go +++ b/internal/compose/resolution/seat_paths_test.go @@ -28,6 +28,26 @@ func TestAGeneratedResourceOnASeatsPathIsRefused(t *testing.T) { t.Errorf("the refusal does not name %s: %v", want, err) } } + // And the paths a module owns do not change with its build (issue 1966): the holder of node-files is told them in a + // word of the tool runner's unit, so a place named for each build restarted the tool runner with each build. + digest := "sha256:" + strings.Repeat("ab12", 16) + for _, row := range []struct { + name string + res map[string]any + owns string + }{ + {"a build's own place", map[string]any{"id": "next", "type": "archive", "digest": digest, "path": "/usr/lib/x/versions/ab12ab12ab12"}, "/usr/lib/x/versions"}, + {"another build's place", map[string]any{"id": "next", "type": "archive", "digest": "sha256:" + strings.Repeat("cd34", 16), "path": "/usr/lib/x/versions/cd34cd34cd34"}, "/usr/lib/x/versions"}, + {"a fixed place", map[string]any{"id": "app", "type": "archive", "digest": digest, "path": "/opt/app"}, "/opt/app"}, + {"a file", map[string]any{"id": "f", "type": "file", "path": "/etc/f"}, "/etc/f"}, + } { + t.Run(row.name, func(t *testing.T) { + got := DeclaredPaths(mcat.Manifest{Module: "m", Resources: []map[string]any{row.res}}) + if len(got) != 1 || got[0] != row.owns { + t.Fatalf("owns %v, want [%s]", got, row.owns) + } + }) + } } // The stepping back comes from the seat, and is of a rendered file only: an uplink holder declaring the