From b1d875410ae0bbec9203f6ba3dce21923bdca773 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 09:10:09 +0200 Subject: [PATCH 1/9] logrotate: dry-run the base against logs of its own, so the shut-off unit level does not stat the machine's /var/log (issue 1891) The unit level runs in bubblewrap, where /var/log is not there; the base's /var/log/wtmp block has no missingok, so logrotate -d failed its stat and every mesh-catalog check that reached this test failed. --- .../cmd/logrotate-tools/manifest_test.go | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/modules/logrotate/cmd/logrotate-tools/manifest_test.go b/modules/logrotate/cmd/logrotate-tools/manifest_test.go index b6ca26cd..84bbfaa0 100644 --- a/modules/logrotate/cmd/logrotate-tools/manifest_test.go +++ b/modules/logrotate/cmd/logrotate-tools/manifest_test.go @@ -39,12 +39,8 @@ func TestTheBaseParses(t *testing.T) { t.Skip("logrotate is not installed here; the base configuration is not dry-run") } dir := t.TempDir() - content := strings.ReplaceAll(templateText(t, manifest(t).resource(t, "config")), "include "+RulesDir, "include "+filepath.Join(dir, "d")) - if err := os.Mkdir(filepath.Join(dir, "d"), 0o755); err != nil { - t.Fatal(err) - } conf := filepath.Join(dir, "logrotate.conf") - if err := os.WriteFile(conf, []byte(content), 0o644); err != nil { + if err := os.WriteFile(conf, []byte(dryRunBase(t, dir)), 0o644); err != nil { t.Fatal(err) } out, err := exec.Command(logrotate, "-d", "-s", filepath.Join(dir, "state"), conf).CombinedOutput() @@ -52,3 +48,40 @@ func TestTheBaseParses(t *testing.T) { t.Fatalf("logrotate -d: %v\n%s", err, out) } } + +// The dry run proves the base parses, not what the machine's logs are: the unit level runs shut off in +// bubblewrap (mesh-sdk buildstep.Isolated), where /var/log is not there, and a base block without +// missingok then fails its stat (issue 1891). +func TestTheDryRunReadsNothingOfTheMachine(t *testing.T) { + dir := t.TempDir() + content := dryRunBase(t, dir) + for _, machine := range []string{"include " + RulesDir, "/var/log/"} { + if strings.Contains(content, machine) { + t.Fatalf("the dry run's base still names %s of the machine:\n%s", machine, content) + } + } + for _, log := range []string{"wtmp", "btmp"} { + if _, err := os.Stat(filepath.Join(dir, "log", log)); err != nil { + t.Fatalf("the base's own log %s is not in the dry run's directory: %v", log, err) + } + } +} + +// dryRunBase is the base configuration pointed into dir: its include at dir/d, and its own logs at dir/log, +// both made there. +func dryRunBase(t *testing.T, dir string) string { + t.Helper() + content := strings.ReplaceAll(templateText(t, manifest(t).resource(t, "config")), "include "+RulesDir, "include "+filepath.Join(dir, "d")) + content = strings.ReplaceAll(content, "/var/log/", filepath.Join(dir, "log")+"/") + for _, d := range []string{"d", "log"} { + if err := os.Mkdir(filepath.Join(dir, d), 0o755); err != nil { + t.Fatal(err) + } + } + for _, log := range []string{"wtmp", "btmp"} { + if err := os.WriteFile(filepath.Join(dir, "log", log), nil, 0o644); err != nil { + t.Fatal(err) + } + } + return content +} -- 2.54.0 From f9247342d1759b5f27b9945d99302dc3c633dcf0 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 09:38:40 +0200 Subject: [PATCH 2/9] pacman, ssh-client: tests read nothing of the machine, so the unit level passes shut off and on a desk (issue 1891) pacman-conf proved the shipped pacman.conf through its Include of /etc/pacman.d/mirrorlist, which the shut-off unit level does not have: the proven file now reads a mirror list of its own, and a guard test holds that every repository of the shipped file includes the list reflector writes. ssh-client's Test case for a run without an agent read the account's SSH_AUTH_SOCK, so it failed on any desk with an agent: it now sets its own, and also proves the environment's agent is the one reported. --- .../pacman/cmd/pacman-tools/manifest_test.go | 31 ++++++++++++++++++- .../cmd/ssh-client-tools/ssh_test.go | 10 +++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/modules/pacman/cmd/pacman-tools/manifest_test.go b/modules/pacman/cmd/pacman-tools/manifest_test.go index b55f6908..af36b100 100644 --- a/modules/pacman/cmd/pacman-tools/manifest_test.go +++ b/modules/pacman/cmd/pacman-tools/manifest_test.go @@ -57,7 +57,7 @@ func TestPacmanConfIsWholeTheUnionOfRepositoriesAndTheImprovedOptions(t *testing t.Skip("pacman-conf is not installed here; the rendered file is not proven") } file := filepath.Join(t.TempDir(), "pacman.conf") - if err := os.WriteFile(file, []byte(content), 0o644); err != nil { + if err := os.WriteFile(file, []byte(provenConf(t, content, filepath.Dir(file))), 0o644); err != nil { t.Fatal(err) } repos, err := exec.Command(conf, "--config", file, "--repo-list").CombinedOutput() @@ -74,6 +74,35 @@ func TestPacmanConfIsWholeTheUnionOfRepositoriesAndTheImprovedOptions(t *testing } } +// Every repository of the shipped file reads the mirror list reflector writes, and the file pacman-conf proves reads +// nothing of the machine: the unit level runs shut off (novox/hq ADR 0448), where /etc/pacman.d is not there +// (issue 1891). +func TestTheProvenConfReadsNothingOfTheMachine(t *testing.T) { + content := templateText(t, manifest(t).resource(t, "config")) + if n := strings.Count(content, "Include = "+Mirrorlist+"\n"); n != 3 || strings.Count(content, "Include") != n { + t.Fatalf("every repository reads %s, and nothing else is included: %d of 3\n%s", Mirrorlist, n, content) + } + dir := t.TempDir() + proven := provenConf(t, content, dir) + if strings.Contains(proven, "/etc/") { + t.Fatalf("the proven file still names the machine's /etc:\n%s", proven) + } + if _, err := os.Stat(filepath.Join(dir, "mirrorlist")); err != nil { + t.Fatalf("the proven file's mirror list is not in its directory: %v", err) + } +} + +// provenConf is the shipped pacman.conf with its repositories reading a mirror list of its own in dir, made there, +// so pacman-conf proves the file the module ships and reads nothing of the machine. +func provenConf(t *testing.T, content, dir string) string { + t.Helper() + list := filepath.Join(dir, "mirrorlist") + if err := os.WriteFile(list, []byte("Server = https://mirror.example/$repo/os/$arch\n"), 0o644); err != nil { + t.Fatal(err) + } + return strings.ReplaceAll(content, "Include = "+Mirrorlist, "Include = "+list) +} + func TestReflectorWritesTheListPacmanReads(t *testing.T) { content := templateText(t, manifest(t).resource(t, "mirrors")) opts := map[string]string{} diff --git a/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go b/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go index 3428e042..93002138 100644 --- a/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go +++ b/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go @@ -311,6 +311,8 @@ func TestAnUnreachableHostIsNotRefreshed(t *testing.T) { } func TestTestSaysHowItAuthenticatedOrWhyNot(t *testing.T) { + // The test's own agent, none: the account running it may have one, and the unit level runs with none (issue 1891). + t.Setenv("SSH_AUTH_SOCK", "") ok := "debug1: Connecting to ace.internal [10.0.0.2] port 22.\ndebug1: Server accepts key: /h/.ssh/id_ed25519 ED25519 " + edFP + "\nAuthenticated to ace.internal ([10.0.0.2]:22) using \"publickey\".\n" f := &fake{answer: func(c call) forgekey.Ran { return forgekey.Ran{Stderr: ok} }} got, err := client(t.TempDir(), f).Test(context.Background(), "ace") @@ -327,9 +329,15 @@ func TestTestSaysHowItAuthenticatedOrWhyNot(t *testing.T) { if got["ok"] != false || got["why"] != "no key it offered was accepted" || !reflect.DeepEqual(got["offered"], []string{"/h/.ssh/id_box"}) { t.Fatalf("%v", got) } - if !strings.Contains(got["note"].(string), "agent") { + if note, _ := got["note"].(string); !strings.Contains(note, "agent") { t.Fatalf("no word on the agent: %v", got) } + agent := filepath.Join(t.TempDir(), "agent.sock") + t.Setenv("SSH_AUTH_SOCK", agent) + got, _ = client(t.TempDir(), f).Test(context.Background(), "ace") + if got["agent"] != agent || got["note"] != nil { + t.Fatalf("the environment's agent is not the one it ran with, or a passphrase note without a passphrase: %v", got) + } } func TestCheckFindsWhatIsWrongAndSaysWhatItDidNotCheck(t *testing.T) { -- 2.54.0 From dc96d7c2ddef8287ebe46c98c6eadd5fd992c839 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 10:00:09 +0200 Subject: [PATCH 3/9] gitea: take a rule's null contexts as the forge's type says, so the strict build the checker runs compiles (issue 1891) The checker builds a package with a build script by its tsconfig (strict), where requiredOf and TrunkForge's appliedProtection said status_check_contexts is string[] and BranchProtection says string[] | null: four TS2345 errors, hidden on main behind earlier failures. Types only; the emitted code is the same, and a test holds that null contexts require nothing. --- modules/gitea/pulls.ts | 2 +- modules/gitea/test/pulls.test.ts | 9 +++++++++ modules/gitea/trunk.ts | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index 0c8fb96a..602013a8 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -25,7 +25,7 @@ export const CHECK_STATUS = "mesh/check"; /** The statuses a branch's rule requires, from the rule the forge applies to it: none when it is unprotected or checks * no status; undefined when the rule could not be read. */ -export function requiredOf(rule: { enable_status_check?: boolean; status_check_contexts?: string[] } | null | undefined): string[] | undefined { +export function requiredOf(rule: { enable_status_check?: boolean; status_check_contexts?: string[] | null } | null | undefined): string[] | undefined { if (rule === undefined) return undefined; if (!rule || !rule.enable_status_check) return []; return (rule.status_check_contexts ?? []).filter(Boolean); diff --git a/modules/gitea/test/pulls.test.ts b/modules/gitea/test/pulls.test.ts index bb3d257b..fcecc44b 100644 --- a/modules/gitea/test/pulls.test.ts +++ b/modules/gitea/test/pulls.test.ts @@ -216,3 +216,12 @@ test("a base's required statuses decide which of the controller's statuses are w const comment = commentFor(c, {}, ["mesh/repo-check"]); assert.ok(comment === null || !comment.includes("mesh/merge-gate"), comment ?? ""); }); + +test("a rule the forge gives with no contexts (null) requires nothing, and an unread rule is unknown", async () => { + const { requiredOf } = await import("../pulls.ts"); + // The forge's BranchProtection says status_check_contexts may be null (protection.ts); requiredOf takes it as it comes. + assert.deepEqual(requiredOf({ enable_status_check: true, status_check_contexts: null }), []); + assert.deepEqual(requiredOf({ enable_status_check: true, status_check_contexts: ["mesh/check", ""] }), ["mesh/check"]); + assert.deepEqual(requiredOf(null), []); + assert.equal(requiredOf(undefined), undefined); +}); diff --git a/modules/gitea/trunk.ts b/modules/gitea/trunk.ts index 40433b90..11100ca1 100644 --- a/modules/gitea/trunk.ts +++ b/modules/gitea/trunk.ts @@ -128,7 +128,7 @@ export interface TrunkForge { branchHead(owner: string, repo: string, branch: string): Promise; /** The rule the forge applies to a branch: which of the controller's statuses are kept there (issue 1301). Not given, * or not read: both, as before. */ - appliedProtection?(owner: string, repo: string, branch: string): Promise<{ enable_status_check?: boolean; status_check_contexts?: string[] } | null>; + appliedProtection?(owner: string, repo: string, branch: string): Promise<{ enable_status_check?: boolean; status_check_contexts?: string[] | null } | null>; } /** What it writes with: the merge check's one writer (gate.ts), which decides in the head's own turn. */ -- 2.54.0 From 437c524b81c78da4092a9f85ceeb1c32a51ea10b Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 10:18:57 +0200 Subject: [PATCH 4/9] gitea, minio, mosquitto: tests run as the Node build module runs them, tsc -p . then npm test (issue 1891) The old TypeScript check compiled every source beside itself, and into dist/, before it ran the tests; mesh-build-node runs `tsc -p .` (these tsconfigs say noEmit) and then the package's test script, so: - gitea and minio: the tests import the .ts sources, which import each other as ./x.js. A resolve hook in test/ resolves such an import, when no .js is there, to the .ts beside it and compiles it with the toolchain's TypeScript (gitea's sources use parameter properties, which type stripping refuses). 57 of gitea's tests and minio's links test failed. - mosquitto: its ctrl test imports ../dist/client.js; its test script now builds first, as gitea's does. Test scripts and a test helper only; what is built and delivered is the same. --- modules/gitea/package.json | 2 +- modules/gitea/test/resolve-ts.mjs | 37 +++++++++++++++++++++++++++++++ modules/minio/package.json | 2 +- modules/minio/test/resolve-ts.mjs | 37 +++++++++++++++++++++++++++++++ modules/mosquitto/package.json | 2 +- 5 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 modules/gitea/test/resolve-ts.mjs create mode 100644 modules/minio/test/resolve-ts.mjs diff --git a/modules/gitea/package.json b/modules/gitea/package.json index 21a6feb5..008e85a7 100644 --- a/modules/gitea/package.json +++ b/modules/gitea/package.json @@ -6,7 +6,7 @@ "private": true, "scripts": { "build": "tsc client.ts token.ts index.ts provisioner/index.ts provisioner/teams.ts tools/index.ts --module NodeNext --moduleResolution NodeNext --target ES2022 --outDir dist", - "test": "npm run build && node --test --experimental-strip-types 'test/*.test.ts'" + "test": "npm run build && node --import ./test/resolve-ts.mjs --test --experimental-strip-types 'test/*.test.ts'" }, "dependencies": { "@novox/mesh-sdk": "^0.1.1" diff --git a/modules/gitea/test/resolve-ts.mjs b/modules/gitea/test/resolve-ts.mjs new file mode 100644 index 00000000..e7197721 --- /dev/null +++ b/modules/gitea/test/resolve-ts.mjs @@ -0,0 +1,37 @@ +// The tests import the module's TypeScript sources (node --experimental-strip-types), and the sources import each other +// by the .js name tsc compiles them to (NodeNext). Nothing compiles them beside themselves before the tests run: the +// Node build module runs `tsc -p .` and then `npm test` (novox/hq issue 1891). So this hook resolves a relative .js import +// from a .ts file, when no such .js is there, to the .ts beside it, and compiles that file with the toolchain's own +// TypeScript (transpileModule: no type-check, which `tsc -p .` did), since a source may use what type stripping refuses +// (a parameter property). Every other import resolves and loads as Node does. +import { readFileSync } from "node:fs"; +import { registerHooks } from "node:module"; +import { fileURLToPath } from "node:url"; +import ts from "typescript"; + +const compiled = new Set(); + +registerHooks({ + resolve(specifier, context, nextResolve) { + try { + return nextResolve(specifier, context); + } catch (err) { + const relative = specifier.startsWith("./") || specifier.startsWith("../"); + if (err?.code !== "ERR_MODULE_NOT_FOUND" || !relative || !specifier.endsWith(".js") || !context.parentURL?.endsWith(".ts")) { + throw err; + } + const resolved = nextResolve(specifier.slice(0, -".js".length) + ".ts", context); + compiled.add(resolved.url); + return resolved; + } + }, + load(url, context, nextLoad) { + if (!compiled.has(url)) return nextLoad(url, context); + const file = fileURLToPath(url); + const out = ts.transpileModule(readFileSync(file, "utf8"), { + fileName: file, + compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022, sourceMap: false }, + }); + return { format: "module", source: out.outputText, shortCircuit: true }; + }, +}); diff --git a/modules/minio/package.json b/modules/minio/package.json index 017e2af8..d3351990 100644 --- a/modules/minio/package.json +++ b/modules/minio/package.json @@ -5,7 +5,7 @@ "type": "module", "private": true, "scripts": { - "test": "node --test --experimental-strip-types 'test/*.test.ts'" + "test": "node --import ./test/resolve-ts.mjs --test --experimental-strip-types 'test/*.test.ts'" }, "dependencies": { "@novox/mesh-sdk": "^0.1.7" diff --git a/modules/minio/test/resolve-ts.mjs b/modules/minio/test/resolve-ts.mjs new file mode 100644 index 00000000..e7197721 --- /dev/null +++ b/modules/minio/test/resolve-ts.mjs @@ -0,0 +1,37 @@ +// The tests import the module's TypeScript sources (node --experimental-strip-types), and the sources import each other +// by the .js name tsc compiles them to (NodeNext). Nothing compiles them beside themselves before the tests run: the +// Node build module runs `tsc -p .` and then `npm test` (novox/hq issue 1891). So this hook resolves a relative .js import +// from a .ts file, when no such .js is there, to the .ts beside it, and compiles that file with the toolchain's own +// TypeScript (transpileModule: no type-check, which `tsc -p .` did), since a source may use what type stripping refuses +// (a parameter property). Every other import resolves and loads as Node does. +import { readFileSync } from "node:fs"; +import { registerHooks } from "node:module"; +import { fileURLToPath } from "node:url"; +import ts from "typescript"; + +const compiled = new Set(); + +registerHooks({ + resolve(specifier, context, nextResolve) { + try { + return nextResolve(specifier, context); + } catch (err) { + const relative = specifier.startsWith("./") || specifier.startsWith("../"); + if (err?.code !== "ERR_MODULE_NOT_FOUND" || !relative || !specifier.endsWith(".js") || !context.parentURL?.endsWith(".ts")) { + throw err; + } + const resolved = nextResolve(specifier.slice(0, -".js".length) + ".ts", context); + compiled.add(resolved.url); + return resolved; + } + }, + load(url, context, nextLoad) { + if (!compiled.has(url)) return nextLoad(url, context); + const file = fileURLToPath(url); + const out = ts.transpileModule(readFileSync(file, "utf8"), { + fileName: file, + compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022, sourceMap: false }, + }); + return { format: "module", source: out.outputText, shortCircuit: true }; + }, +}); diff --git a/modules/mosquitto/package.json b/modules/mosquitto/package.json index 4c74b1fe..705ca055 100644 --- a/modules/mosquitto/package.json +++ b/modules/mosquitto/package.json @@ -7,7 +7,7 @@ "scripts": { "build": "tsc topics.ts client.ts index.ts tools/index.ts provisioner/index.ts bootstrap/index.ts --module NodeNext --moduleResolution NodeNext --target ES2022 --outDir dist", "typecheck": "tsc -p tsconfig.json", - "test": "node --test --experimental-strip-types 'test/*.test.ts'" + "test": "npm run build && node --test --experimental-strip-types 'test/*.test.ts'" }, "dependencies": { "@novox/mesh-sdk": "^0.1.1" -- 2.54.0 From 933370df7cb20d6c006aaa823aeabca2a3a07018 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 10:22:41 +0200 Subject: [PATCH 5/9] gitea, minio: compile the sources beside themselves in the test script, as the old check did (issue 1891) The resolve hook needed the typescript package, which mesh-build-node keeps out of the workspace (notCopied): tsc is reached only as the bin/tsc command. So the test scripts now do what merge-check-typescript.sh did before the tests: tsc over the module's sources with --outDir ., so the sources' ./x.js imports resolve. The emitted .js is ignored by git. --- modules/gitea/.gitignore | 2 ++ modules/gitea/package.json | 2 +- modules/gitea/test/resolve-ts.mjs | 37 ------------------------------- modules/minio/.gitignore | 2 ++ modules/minio/package.json | 2 +- modules/minio/test/resolve-ts.mjs | 37 ------------------------------- 6 files changed, 6 insertions(+), 76 deletions(-) create mode 100644 modules/gitea/.gitignore delete mode 100644 modules/gitea/test/resolve-ts.mjs create mode 100644 modules/minio/.gitignore delete mode 100644 modules/minio/test/resolve-ts.mjs diff --git a/modules/gitea/.gitignore b/modules/gitea/.gitignore new file mode 100644 index 00000000..b5e90585 --- /dev/null +++ b/modules/gitea/.gitignore @@ -0,0 +1,2 @@ +# The test script compiles the sources beside themselves before the tests import them (package.json, issue 1891). +*.js diff --git a/modules/gitea/package.json b/modules/gitea/package.json index 008e85a7..8c6171fe 100644 --- a/modules/gitea/package.json +++ b/modules/gitea/package.json @@ -6,7 +6,7 @@ "private": true, "scripts": { "build": "tsc client.ts token.ts index.ts provisioner/index.ts provisioner/teams.ts tools/index.ts --module NodeNext --moduleResolution NodeNext --target ES2022 --outDir dist", - "test": "npm run build && node --import ./test/resolve-ts.mjs --test --experimental-strip-types 'test/*.test.ts'" + "test": "npm run build && tsc --module NodeNext --moduleResolution NodeNext --target ES2022 --types node --rootDir . --outDir . $(find . -name '*.ts' ! -name '*.d.ts' ! -path './test/*' ! -path './node_modules/*' ! -path './dist/*') && node --test --experimental-strip-types 'test/*.test.ts'" }, "dependencies": { "@novox/mesh-sdk": "^0.1.1" diff --git a/modules/gitea/test/resolve-ts.mjs b/modules/gitea/test/resolve-ts.mjs deleted file mode 100644 index e7197721..00000000 --- a/modules/gitea/test/resolve-ts.mjs +++ /dev/null @@ -1,37 +0,0 @@ -// The tests import the module's TypeScript sources (node --experimental-strip-types), and the sources import each other -// by the .js name tsc compiles them to (NodeNext). Nothing compiles them beside themselves before the tests run: the -// Node build module runs `tsc -p .` and then `npm test` (novox/hq issue 1891). So this hook resolves a relative .js import -// from a .ts file, when no such .js is there, to the .ts beside it, and compiles that file with the toolchain's own -// TypeScript (transpileModule: no type-check, which `tsc -p .` did), since a source may use what type stripping refuses -// (a parameter property). Every other import resolves and loads as Node does. -import { readFileSync } from "node:fs"; -import { registerHooks } from "node:module"; -import { fileURLToPath } from "node:url"; -import ts from "typescript"; - -const compiled = new Set(); - -registerHooks({ - resolve(specifier, context, nextResolve) { - try { - return nextResolve(specifier, context); - } catch (err) { - const relative = specifier.startsWith("./") || specifier.startsWith("../"); - if (err?.code !== "ERR_MODULE_NOT_FOUND" || !relative || !specifier.endsWith(".js") || !context.parentURL?.endsWith(".ts")) { - throw err; - } - const resolved = nextResolve(specifier.slice(0, -".js".length) + ".ts", context); - compiled.add(resolved.url); - return resolved; - } - }, - load(url, context, nextLoad) { - if (!compiled.has(url)) return nextLoad(url, context); - const file = fileURLToPath(url); - const out = ts.transpileModule(readFileSync(file, "utf8"), { - fileName: file, - compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022, sourceMap: false }, - }); - return { format: "module", source: out.outputText, shortCircuit: true }; - }, -}); diff --git a/modules/minio/.gitignore b/modules/minio/.gitignore new file mode 100644 index 00000000..b5e90585 --- /dev/null +++ b/modules/minio/.gitignore @@ -0,0 +1,2 @@ +# The test script compiles the sources beside themselves before the tests import them (package.json, issue 1891). +*.js diff --git a/modules/minio/package.json b/modules/minio/package.json index d3351990..f756a5f7 100644 --- a/modules/minio/package.json +++ b/modules/minio/package.json @@ -5,7 +5,7 @@ "type": "module", "private": true, "scripts": { - "test": "node --import ./test/resolve-ts.mjs --test --experimental-strip-types 'test/*.test.ts'" + "test": "tsc --module NodeNext --moduleResolution NodeNext --target ES2022 --types node --rootDir . --outDir . $(find . -name '*.ts' ! -name '*.d.ts' ! -path './test/*' ! -path './node_modules/*' ! -path './dist/*') && node --test --experimental-strip-types 'test/*.test.ts'" }, "dependencies": { "@novox/mesh-sdk": "^0.1.7" diff --git a/modules/minio/test/resolve-ts.mjs b/modules/minio/test/resolve-ts.mjs deleted file mode 100644 index e7197721..00000000 --- a/modules/minio/test/resolve-ts.mjs +++ /dev/null @@ -1,37 +0,0 @@ -// The tests import the module's TypeScript sources (node --experimental-strip-types), and the sources import each other -// by the .js name tsc compiles them to (NodeNext). Nothing compiles them beside themselves before the tests run: the -// Node build module runs `tsc -p .` and then `npm test` (novox/hq issue 1891). So this hook resolves a relative .js import -// from a .ts file, when no such .js is there, to the .ts beside it, and compiles that file with the toolchain's own -// TypeScript (transpileModule: no type-check, which `tsc -p .` did), since a source may use what type stripping refuses -// (a parameter property). Every other import resolves and loads as Node does. -import { readFileSync } from "node:fs"; -import { registerHooks } from "node:module"; -import { fileURLToPath } from "node:url"; -import ts from "typescript"; - -const compiled = new Set(); - -registerHooks({ - resolve(specifier, context, nextResolve) { - try { - return nextResolve(specifier, context); - } catch (err) { - const relative = specifier.startsWith("./") || specifier.startsWith("../"); - if (err?.code !== "ERR_MODULE_NOT_FOUND" || !relative || !specifier.endsWith(".js") || !context.parentURL?.endsWith(".ts")) { - throw err; - } - const resolved = nextResolve(specifier.slice(0, -".js".length) + ".ts", context); - compiled.add(resolved.url); - return resolved; - } - }, - load(url, context, nextLoad) { - if (!compiled.has(url)) return nextLoad(url, context); - const file = fileURLToPath(url); - const out = ts.transpileModule(readFileSync(file, "utf8"), { - fileName: file, - compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022, sourceMap: false }, - }); - return { format: "module", source: out.outputText, shortCircuit: true }; - }, -}); -- 2.54.0 From 00e54d46d7169b319eb9bc414e7ee11e0724aa43 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 11:22:05 +0200 Subject: [PATCH 6/9] Give the integration level the node's installed Node toolchains read-only, so the Node build module's tests build offline (issue 1891, ADR 0533) mesh-build-go binds /var/lib/mesh-build-node-cache/toolchain read-only into the integration level's one shut-off process when it is there; the unit level never. mesh-build-node opens its installed toolchains to other accounts (folders 0755, files 0644, writable by its owner alone; its cache 0711, so the npm cache beside them stays its own), and realMachine copies the toolchain of its own lock from there instead of npm ci from the registry, which the level cannot reach. --- .../mesh-build-go/cmd/mesh-build-go/level.go | 20 +++ .../mesh-build-go/cmd/mesh-build-go/module.go | 3 + .../cmd/mesh-build-go/node_toolchains_test.go | 128 ++++++++++++++++++ .../mesh-build-node/build_integration_test.go | 73 +++++++++- .../cmd/mesh-build-node/toolchain.go | 62 ++++++++- .../mesh-build-node/toolchain_open_test.go | 100 ++++++++++++++ modules/mesh-build-node/module.json | 2 +- 7 files changed, 379 insertions(+), 9 deletions(-) create mode 100644 modules/mesh-build-go/cmd/mesh-build-go/node_toolchains_test.go create mode 100644 modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go diff --git a/modules/mesh-build-go/cmd/mesh-build-go/level.go b/modules/mesh-build-go/cmd/mesh-build-go/level.go index 30f4299a..520962dc 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/level.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/level.go @@ -68,6 +68,7 @@ func (m *Module) integrationShutOff(ctx context.Context, workspace string, packa } iso := m.isolation(workspace, false) iso.ReadOnly = append(iso.ReadOnly, exe) // the program alone, not the directory it lies in + iso.ReadOnly = append(iso.ReadOnly, m.nodeToolchains()...) r := buildstep.Isolated(m.Run, iso)(ctx, nil, exe, levelArg, string(spec)) var answer levelAnswer line := lastLine(r.Stdout) @@ -80,6 +81,25 @@ func (m *Module) integrationShutOff(ctx context.Context, workspace string, packa return answer.Result, nil } +// NodeToolchains is the Node build module's toolchains directory on this node: its cache directory (module.json +// resource `cache`, mode 0711) joined with toolchain, where it installs typescript, esbuild, @novox/mesh-sdk and nats +// from its own lock (ADR 0533 §1, §2; issue 1891). +const NodeToolchains = "/var/lib/mesh-build-node-cache/toolchain" + +// nodeToolchains is what the integration level is given of the Node build module's toolchains: their directory, +// read-only, when it is there and this account may enter it, and nothing on a node where the Node build module has +// installed none (ADR 0533 §1). The unit level is never given it. +func (m *Module) nodeToolchains() []string { + dir := m.NodeToolchains + if dir == "" { + dir = NodeToolchains + } + if _, err := os.ReadDir(dir); err != nil { + return nil + } + return []string{dir} +} + func lastLine(s string) string { lines := strings.Split(strings.TrimSpace(s), "\n") return strings.TrimSpace(lines[len(lines)-1]) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/module.go b/modules/mesh-build-go/cmd/mesh-build-go/module.go index ee3cc99c..92148e95 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/module.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/module.go @@ -62,6 +62,9 @@ type Module struct { Push func(ctx context.Context, registry, repository, digest string, body []byte) error // Executable is this program's path, which the integration level starts again shut off; os.Executable when empty. Executable string + // NodeToolchains is where the Node build module keeps its installed toolchains, bound read-only into the + // integration level (ADR 0533 §1); NodeToolchains when empty. A test points it elsewhere. + NodeToolchains string // StartBus starts the throwaway bus of a run in dir and answers its address and what stops it (the module's own // program, StartBus in bus.go, when nil). StartBus func(ctx context.Context, dir string) (url string, stop func(), err error) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/node_toolchains_test.go b/modules/mesh-build-go/cmd/mesh-build-go/node_toolchains_test.go new file mode 100644 index 00000000..0c48ac73 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/node_toolchains_test.go @@ -0,0 +1,128 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "slices" + "strconv" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +// integrationShutOffWith runs the integration level through the fake bubblewrap and answers what its one shut-off +// process was given. +func integrationShutOffWith(t *testing.T, toolchains string) shutOff { + t.Helper() + m, states, ws, _ := workspaceFor(t, "failing") + m.Executable = "/opt/bundle/mesh-build-go" + m.NodeToolchains = toolchains + want := TestResult{Level: LevelIntegration, Passed: true, Tests: 1, Packages: []PackageResult{}, Store: "postgres 18"} + var seen []shutOff + m.Run = throughBubblewrap(t, func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + switch { + case name == "postgres" && len(args) == 1 && args[0] == "--version": + return forgekey.Ran{Stdout: "postgres (PostgreSQL) 18.6\n"} + case name == m.Executable && len(args) == 2 && args[0] == levelArg: + raw, _ := json.Marshal(levelAnswer{Result: &want}) + return forgekey.Ran{Stdout: string(raw) + "\n"} + } + return forgekey.Ran{Status: 1, Err: "not expected here"} + }, &seen) + run := stepTool(t, m, "build_go_test") + if _, err := run(map[string]any{"workspace": ws, "level": LevelIntegration}); err != nil { + t.Fatal(err) + } + if st := states.waitEnd(t); st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + if len(seen) != 1 { + t.Fatalf("shut off %d processes, want one: %v", len(seen), seen) + } + checkShutOff(t, m, seen[0], false) + return seen[0] +} + +// 1891 (ADR 0533 §1): the integration level is given the Node build module's installed toolchains, read-only, so the +// Node build module's own integration tests build with the real tsc and esbuild and reach no registry. +func TestTheIntegrationLevelIsGivenTheNodeToolchainsReadOnly(t *testing.T) { + dir := t.TempDir() + s := integrationShutOffWith(t, dir) + if !slices.Contains(s.roBinds, dir) { + t.Errorf("the node toolchains %s are not bound read-only: %v", dir, s.roBinds) + } + if slices.Contains(s.binds, dir) || slices.Contains(s.overlays, dir) { + t.Errorf("the node toolchains %s may be written: binds %v, overlays %v", dir, s.binds, s.overlays) + } + if slices.Contains(s.roBinds, filepath.Dir(dir)) { + t.Errorf("the whole cache %s is bound, not its toolchains alone: %v", filepath.Dir(dir), s.roBinds) + } +} + +// 1891 (ADR 0533 §1): a node where the Node build module has installed no toolchain runs the level as before, with +// nothing more bound (bubblewrap binds only what is there). +func TestANodeWithoutNodeToolchainsRunsTheLevelWithNothingMoreBound(t *testing.T) { + missing := filepath.Join(t.TempDir(), "none") + s := integrationShutOffWith(t, missing) + if slices.Contains(s.roBinds, missing) { + t.Errorf("a missing %s is bound: %v", missing, s.roBinds) + } +} + +// 1891 (ADR 0533 §1): the unit level is not given them. +func TestTheUnitLevelIsNotGivenTheNodeToolchains(t *testing.T) { + m, states, ws, _ := workspaceFor(t, "failing") + dir := t.TempDir() + m.NodeToolchains = dir + var seen []shutOff + m.Run = throughBubblewrap(t, m.Run, &seen) + if _, st := runUnit(t, m, states, ws); st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + for _, s := range seen { + if slices.Contains(s.roBinds, dir) || slices.Contains(s.binds, dir) { + t.Errorf("%s was given the node toolchains", s.command) + } + } +} + +// 1891 (ADR 0533 §1, §2): the place bound is the Node build module's own: its manifest's cache directory joined with +// toolchain, and that cache directory lets other accounts enter it without listing it (0711). +func TestTheNodeToolchainsAreTheNodeBuildModulesCache(t *testing.T) { + if _, err := catalogue.ReadTreeManifest(catalogueRoot, filepath.Join(catalogueRoot, "modules", "mesh-build-node")); err != nil { + t.Fatalf("mesh-sdk's validator refuses mesh-build-node's manifest: %v", err) + } + raw, err := os.ReadFile(filepath.Join(catalogueRoot, "modules", "mesh-build-node", "module.json")) + if err != nil { + t.Fatal(err) + } + var man struct { + Resources []struct { + ID, Path, Mode string + } `json:"resources"` + } + if err := json.Unmarshal(raw, &man); err != nil { + t.Fatal(err) + } + found := false + for _, r := range man.Resources { + if r.ID != "cache" { + continue + } + found = true + if want := filepath.Join(r.Path, "toolchain"); NodeToolchains != want { + t.Errorf("NodeToolchains = %s, want %s (mesh-build-node's cache)", NodeToolchains, want) + } + mode, err := strconv.ParseUint(r.Mode, 8, 32) + if err != nil || mode&0o001 == 0 || mode&0o004 != 0 { + t.Errorf("mesh-build-node's cache has mode %q; want others to enter it and not list it (0711)", r.Mode) + } + } + if !found { + t.Fatal("mesh-build-node's manifest declares no cache directory") + } +} diff --git a/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go b/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go index e4c9e5b8..85809ed2 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go @@ -3,7 +3,10 @@ package main import ( + "bytes" "context" + "fmt" + "io/fs" "os" "os/exec" "path/filepath" @@ -13,8 +16,11 @@ import ( "git.novox.be/novox/mesh-sdk/go/forgekey" ) -// realMachine installs the real toolchain from the module's own lock (npm ci --ignore-scripts, from the mesh's -// registry for @novox, which reads without a credential) and makes two workspaces holding one fixture bundle. +// realMachine gives the tests the real toolchain of the module's own lock and makes two workspaces holding one fixture +// bundle. The toolchain is the one this node's installed Node build module keeps (ADR 0533 §1): the integration level +// is given its toolchains read-only and no network, so it is copied into the test's own cache from there. Only where +// no toolchain is installed (a desk) is it installed with `npm ci --ignore-scripts` from the mesh's registry, as the +// module does, which needs the network. func realMachine(t *testing.T) (*Module, string) { t.Helper() for _, p := range []string{"node", "npm", "bwrap"} { // bwrap: tsc, esbuild and the scripts run shut off (1309) @@ -40,8 +46,11 @@ func realMachine(t *testing.T) (*Module, string) { return Registry{URL: "https://git.novox.be/api/packages/novox/npm/"}, nil }}} withStore(t, m) + if from := installedToolchain(t, filepath.Join(CacheDir, "toolchain"), filepath.Join(home, "toolchain", "package-lock.json")); from != "" { + copyToolchain(t, from, m.Toolchain.Paths.Dir(m.Recipe.Version())) + } if err := m.Toolchain.Ensure(context.Background(), m.Recipe.Version()); err != nil { - t.Fatal(err) + t.Fatalf("no toolchain of this lock is installed under %s, and installing it from the registry failed: %v", filepath.Join(CacheDir, "toolchain"), err) } for _, id := range []string{"w1", "w2"} { for p, body := range map[string]string{ @@ -115,3 +124,61 @@ func TestAPackageWhoseTestsImportItsBuildPassesTheUnitLevel(t *testing.T) { t.Errorf("result = %+v", r) } } + +// installedToolchain is the toolchain directory under toolchains that this node's Node build module installed from +// the lock at lockPath, byte for byte; "" when there is none (or the directory cannot be read: no Node build module +// here). A toolchain of another lock is never taken: the tests prove the lock they carry. +func installedToolchain(t *testing.T, toolchains, lockPath string) string { + t.Helper() + want, err := os.ReadFile(lockPath) + if err != nil { + t.Fatal(err) + } + entries, err := os.ReadDir(toolchains) + if err != nil { + return "" + } + for _, e := range entries { + dir := filepath.Join(toolchains, e.Name()) + if _, err := os.Stat(filepath.Join(dir, installedMark)); err != nil { + continue + } + if got, err := os.ReadFile(filepath.Join(dir, "package-lock.json")); err == nil && bytes.Equal(got, want) { + t.Logf("the toolchain is this node's installed %s", dir) + return dir + } + } + return "" +} + +// copyToolchain copies the installed toolchain from into to whole, files and folders with their modes, so the test's +// steps run behind a toolchain of their own and the installed one is only read. +func copyToolchain(t *testing.T, from, to string) { + t.Helper() + err := filepath.WalkDir(from, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, _ := filepath.Rel(from, p) + dst := filepath.Join(to, rel) + info, err := d.Info() + if err != nil { + return err + } + switch { + case d.IsDir(): + return os.MkdirAll(dst, 0o755) + case info.Mode().IsRegular(): + raw, err := os.ReadFile(p) + if err != nil { + return err + } + return os.WriteFile(dst, raw, info.Mode().Perm()|0o600) + default: + return fmt.Errorf("%s is neither a file nor a folder", p) + } + }) + if err != nil { + t.Fatalf("copying the installed toolchain %s: %v", from, err) + } +} diff --git a/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go b/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go index 2feaac95..93901cf2 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go @@ -10,6 +10,7 @@ package main import ( "context" "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -53,14 +54,17 @@ func (t Toolchain) Present(version string) bool { // are copied in, and `npm ci --ignore-scripts` runs once, the mesh's registry named for the @novox scope. A directory // an earlier install left unfinished is removed first. func (t Toolchain) Ensure(ctx context.Context, version string) error { - if t.Present(version) { - return nil - } dir := t.Paths.Dir(version) + if t.Present(version) { + if opened(dir) { + return nil + } + return t.open(dir) + } if err := os.RemoveAll(dir); err != nil { return err } - if err := os.MkdirAll(dir, 0o700); err != nil { + if err := os.MkdirAll(dir, 0o755); err != nil { return err } for _, name := range []string{"package.json", "package-lock.json"} { @@ -87,7 +91,55 @@ func (t Toolchain) Ensure(ctx context.Context, version string) error { return fmt.Errorf("npm ci --ignore-scripts for the toolchain %s failed (status %d %s): %s", version, ran.Status, ran.Err, tail(ran.Stderr)) } - return os.WriteFile(filepath.Join(dir, installedMark), []byte(version+"\n"), 0o600) + if err := os.WriteFile(filepath.Join(dir, installedMark), []byte(version+"\n"), 0o600); err != nil { + return err + } + return t.open(dir) +} + +// opened says whether the toolchain directory dir was opened to other accounts: its installed mark, opened last, is +// readable by others. +func opened(dir string) bool { + st, err := os.Stat(filepath.Join(dir, installedMark)) + return err == nil && st.Mode().Perm()&0o004 != 0 +} + +// open lets every account read the toolchain directory dir and nobody but its owner write it (ADR 0533 §2): the +// toolchains folder and every folder beneath dir 0755, every file 0644 and every program 0755, whatever umask npm ran +// under; the installed mark last, so a toolchain opened only in part is opened again. The other build module's +// integration level reads it through the cache directory, which others may enter and not list (0711); the npm cache +// beside it stays the owner's. +func (t Toolchain) open(dir string) error { + if err := os.Chmod(filepath.Join(t.Paths.Cache, "toolchain"), 0o755); err != nil { + return err + } + mark := filepath.Join(dir, installedMark) + err := filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if p == mark { + return nil + } + info, err := d.Info() + if err != nil { + return err + } + switch mode := info.Mode(); { + case mode.IsDir(): + return os.Chmod(p, 0o755) + case mode.IsRegular() && mode.Perm()&0o100 != 0: + return os.Chmod(p, 0o755) + case mode.IsRegular(): + return os.Chmod(p, 0o644) + default: + return fmt.Errorf("%s is neither a file nor a folder, and a toolchain holds only those", p) + } + }) + if err != nil { + return fmt.Errorf("the toolchain %s cannot be opened to the integration level: %w", dir, err) + } + return os.Chmod(mark, 0o644) } func tail(s string) string { diff --git a/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go new file mode 100644 index 00000000..a64ed321 --- /dev/null +++ b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go @@ -0,0 +1,100 @@ +package main + +import ( + "context" + "io/fs" + "os" + "path/filepath" + "testing" + + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +// npmAsUmask077 installs as npm does under a umask of 077: node_modules with a package whose folders are 0700, its +// file 0600 and its program 0700. +func npmAsUmask077(f *fakeNpm) forgekey.Runner { + return func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + ran := f.run(ctx, env, name, args...) + for i, a := range args { + if a == "--prefix" { + pkg := filepath.Join(args[i+1], "node_modules", "typescript", "bin") + os.MkdirAll(pkg, 0o700) + os.WriteFile(filepath.Join(args[i+1], "node_modules", "typescript", "package.json"), []byte("{}"), 0o600) + os.WriteFile(filepath.Join(pkg, "tsc"), []byte("#!/bin/sh\n"), 0o700) + } + } + return ran + } +} + +// checkOpen fails unless every folder from the cache's toolchain down lets others enter and read it and every file lets +// others read it, a program others run, and nothing lets anyone but the owner write. +func checkOpen(t *testing.T, tc Toolchain) { + t.Helper() + root := filepath.Join(tc.Paths.Cache, "toolchain") + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + info, err := d.Info() + if err != nil { + return err + } + mode := info.Mode().Perm() + switch { + case mode&0o022 != 0: + t.Errorf("%s is %o: writable by others than its owner", p, mode) + case d.IsDir() && mode&0o005 != 0o005: + t.Errorf("%s is %o: others may not enter and read it", p, mode) + case !d.IsDir() && mode&0o004 == 0: + t.Errorf("%s is %o: others may not read it", p, mode) + case !d.IsDir() && mode&0o100 != 0 && mode&0o001 == 0: + t.Errorf("%s is %o: a program others may not run", p, mode) + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} + +// 1891 (ADR 0533 §2): a toolchain the module installs can be read by the other build module's integration level: +// its folders 0755 and its files 0644 (programs 0755), whatever umask npm ran under, and writable by its owner alone. +func TestAnInstalledToolchainIsReadableByOthersAndWritableByItsOwnerAlone(t *testing.T) { + fake := &fakeNpm{} + tc := toolchainIn(t, fake) + tc.Run = npmAsUmask077(fake) + if err := tc.Ensure(context.Background(), "v1"); err != nil { + t.Fatal(err) + } + checkOpen(t, tc) +} + +// 1891 (ADR 0533 §2): a toolchain installed before this release, shut to its owner, is opened by the next Ensure +// without installing it again. +func TestAToolchainInstalledShutIsOpenedWithoutAnInstall(t *testing.T) { + fake := &fakeNpm{} + tc := toolchainIn(t, fake) + dir := tc.Paths.Dir("v1") + os.MkdirAll(filepath.Join(dir, "node_modules", "esbuild", "bin"), 0o700) + os.WriteFile(filepath.Join(dir, "node_modules", "esbuild", "bin", "esbuild"), []byte("x"), 0o700) + os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("{}"), 0o600) + os.WriteFile(filepath.Join(dir, installedMark), []byte("v1\n"), 0o600) + os.Chmod(filepath.Join(tc.Paths.Cache, "toolchain"), 0o700) + if err := tc.Ensure(context.Background(), "v1"); err != nil { + t.Fatal(err) + } + if len(fake.calls) != 0 { + t.Errorf("npm ran %d times for a toolchain already there", len(fake.calls)) + } + checkOpen(t, tc) +} + +// 1891 (ADR 0533 §2): the cache directory lets other accounts enter it and not list it, so the toolchains beneath it +// can be reached by their path and the npm cache beside them (0700) cannot. +func TestTheCacheDirectoryIsEnteredByOthersNotListed(t *testing.T) { + r := resource(t, readManifest(t), "cache") + if r["mode"] != "0711" { + t.Errorf("the cache directory's mode is %v, want 0711", r["mode"]) + } +} diff --git a/modules/mesh-build-node/module.json b/modules/mesh-build-node/module.json index c73aa9a1..21378546 100644 --- a/modules/mesh-build-node/module.json +++ b/modules/mesh-build-node/module.json @@ -87,7 +87,7 @@ "id": "cache", "type": "directory", "path": "/var/lib/mesh-build-node-cache", - "mode": "0700", + "mode": "0711", "owner": "mesh-build-node", "unknown": "program" }, -- 2.54.0 From 2ebcb180977473b62a87b51226b2c319d7eae0ba Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 11:23:11 +0200 Subject: [PATCH 7/9] =?UTF-8?q?realMachine:=20a=20change=20carrying=20a=20?= =?UTF-8?q?lock=20no=20release=20has=20installed=20runs=20with=20the=20too?= =?UTF-8?q?lchain=20installed=20last,=20and=20says=20so,=20so=20a=20lock?= =?UTF-8?q?=20change=20is=20not=20refused=20by=20its=20own=20check=20(issu?= =?UTF-8?q?e=201891,=20ADR=200533=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../mesh-build-node/build_integration_test.go | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go b/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go index 85809ed2..bd66ceea 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/build_integration_test.go @@ -12,6 +12,7 @@ import ( "path/filepath" "slices" "testing" + "time" "git.novox.be/novox/mesh-sdk/go/forgekey" ) @@ -125,9 +126,10 @@ func TestAPackageWhoseTestsImportItsBuildPassesTheUnitLevel(t *testing.T) { } } -// installedToolchain is the toolchain directory under toolchains that this node's Node build module installed from -// the lock at lockPath, byte for byte; "" when there is none (or the directory cannot be read: no Node build module -// here). A toolchain of another lock is never taken: the tests prove the lock they carry. +// installedToolchain is the toolchain directory under toolchains that this node's Node build module installed: the +// one installed from the lock at lockPath, byte for byte, or, when the change carries a lock no delivered release has +// installed yet, the one installed last, said in the test's log (ADR 0533 §3: a new lock is proven by the module that +// installs it on its first node). "" when none is there or the directory cannot be read: no Node build module here. func installedToolchain(t *testing.T, toolchains, lockPath string) string { t.Helper() want, err := os.ReadFile(lockPath) @@ -138,17 +140,29 @@ func installedToolchain(t *testing.T, toolchains, lockPath string) string { if err != nil { return "" } + last, lastAt := "", time.Time{} for _, e := range entries { dir := filepath.Join(toolchains, e.Name()) - if _, err := os.Stat(filepath.Join(dir, installedMark)); err != nil { + mark, err := os.Stat(filepath.Join(dir, installedMark)) + if err != nil { continue } - if got, err := os.ReadFile(filepath.Join(dir, "package-lock.json")); err == nil && bytes.Equal(got, want) { - t.Logf("the toolchain is this node's installed %s", dir) + got, err := os.ReadFile(filepath.Join(dir, "package-lock.json")) + if err != nil { + continue + } + if bytes.Equal(got, want) { + t.Logf("the toolchain is this node's installed %s, of this change's lock", dir) return dir } + if mark.ModTime().After(lastAt) { + last, lastAt = dir, mark.ModTime() + } } - return "" + if last != "" { + t.Logf("NOT THIS LOCK: no toolchain of this change's lock is installed here; the tests run with the one installed last, %s", last) + } + return last } // copyToolchain copies the installed toolchain from into to whole, files and folders with their modes, so the test's -- 2.54.0 From 845479d7cdbd3893ab5bd46aa6bed03b164c194c Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 11:30:26 +0200 Subject: [PATCH 8/9] =?UTF-8?q?mesh-build-node:=20prove=20the=20npm=20down?= =?UTF-8?q?load=20cache=20beside=20the=20toolchains=20stays=20its=20owner'?= =?UTF-8?q?s=20(ADR=200533=20=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-build-node/toolchain_open_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go index a64ed321..015dc6e9 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go @@ -64,10 +64,17 @@ func TestAnInstalledToolchainIsReadableByOthersAndWritableByItsOwnerAlone(t *tes fake := &fakeNpm{} tc := toolchainIn(t, fake) tc.Run = npmAsUmask077(fake) + npm := filepath.Join(tc.Paths.Cache, "npm") + if err := os.MkdirAll(npm, 0o700); err != nil { + t.Fatal(err) + } if err := tc.Ensure(context.Background(), "v1"); err != nil { t.Fatal(err) } checkOpen(t, tc) + if st, err := os.Stat(npm); err != nil || st.Mode().Perm() != 0o700 { + t.Errorf("the npm download cache beside the toolchains is %v (%v), want 0700: only the toolchains are opened", st.Mode().Perm(), err) + } } // 1891 (ADR 0533 §2): a toolchain installed before this release, shut to its owner, is opened by the next Ensure -- 2.54.0 From a2e5a20f52ed765c7e0609a44a3e5e80df2893f7 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 11:45:20 +0200 Subject: [PATCH 9/9] =?UTF-8?q?mesh-build-node:=20write=20the=20install's?= =?UTF-8?q?=20registry=20credential=20in=20the=20account's=20home,=20never?= =?UTF-8?q?=20beneath=20the=20opened=20cache,=20and=20refuse=20to=20open?= =?UTF-8?q?=20a=20toolchain=20holding=20one=20(issue=201891=20review,=20AD?= =?UTF-8?q?R=200533=20=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-build-node/toolchain.go | 33 ++++++++++- .../mesh-build-node/toolchain_open_test.go | 59 +++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go b/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go index 93901cf2..76d68521 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/toolchain.go @@ -80,13 +80,24 @@ func (t Toolchain) Ensure(ctx context.Context, version string) error { if err != nil { return err } - npmrc := filepath.Join(dir, ".npmrc-install") - if err := os.WriteFile(npmrc, []byte(reg.Npmrc()), 0o600); err != nil { + // The credential is written in the account's own home (0700), never beneath the cache, whose toolchains are + // opened to every account (ADR 0533 §2), and removed as soon as npm has read it. + f, err := os.CreateTemp(t.Paths.Home, ".npmrc-install-") + if err != nil { return err } + npmrc := f.Name() defer os.Remove(npmrc) + _, err = f.WriteString(reg.Npmrc()) + if cerr := f.Close(); err == nil { + err = cerr + } + if err != nil { + return err + } ran := t.Run(ctx, []string{"PATH=" + pathOrDefault(), "HOME=" + t.Paths.Home, "LC_ALL=C", "npm_config_cache=" + filepath.Join(t.Paths.Cache, "npm")}, "npm", "ci", "--ignore-scripts", "--no-bin-links", "--no-audit", "--no-fund", "--prefix", dir, "--userconfig", npmrc) + os.Remove(npmrc) if ran.Err != "" || ran.Status != 0 { return fmt.Errorf("npm ci --ignore-scripts for the toolchain %s failed (status %d %s): %s", version, ran.Status, ran.Err, tail(ran.Stderr)) @@ -97,6 +108,21 @@ func (t Toolchain) Ensure(ctx context.Context, version string) error { return t.open(dir) } +// refuseNpmConfig refuses a toolchain folder that holds an install's npm config file (one a dead install of an older release left with +// the registry credential in it), naming it, before anything of the folder is opened. +func refuseNpmConfig(dir string) error { + return filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && strings.HasPrefix(d.Name(), ".npmrc-install") { + return fmt.Errorf("the toolchain %s holds the npm config file %s, which may hold the registry credential, "+ + "so it is not opened to other accounts; remove it", dir, p) + } + return nil + }) +} + // opened says whether the toolchain directory dir was opened to other accounts: its installed mark, opened last, is // readable by others. func opened(dir string) bool { @@ -110,6 +136,9 @@ func opened(dir string) bool { // integration level reads it through the cache directory, which others may enter and not list (0711); the npm cache // beside it stays the owner's. func (t Toolchain) open(dir string) error { + if err := refuseNpmConfig(dir); err != nil { + return err + } if err := os.Chmod(filepath.Join(t.Paths.Cache, "toolchain"), 0o755); err != nil { return err } diff --git a/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go index 015dc6e9..2ee819e2 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_open_test.go @@ -5,6 +5,7 @@ import ( "io/fs" "os" "path/filepath" + "strings" "testing" "git.novox.be/novox/mesh-sdk/go/forgekey" @@ -105,3 +106,61 @@ func TestTheCacheDirectoryIsEnteredByOthersNotListed(t *testing.T) { t.Errorf("the cache directory's mode is %v, want 0711", r["mode"]) } } + +// 1891 review (ADR 0533 §2): the registry credential npm is told is never written beneath the cache, so opening a +// toolchain can never open it: npm reads it from a file in the account's own home, readable by its owner alone, and +// the file is gone when Ensure returns. +func TestTheInstallCredentialIsNeverInTheFolderThatIsOpened(t *testing.T) { + fake := &fakeNpm{} + tc := toolchainIn(t, fake) + var userconfig string + inner := npmAsUmask077(fake) + tc.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + for i, a := range args { + if a == "--userconfig" { + userconfig = args[i+1] + } + } + if st, err := os.Stat(userconfig); err != nil || st.Mode().Perm() != 0o600 { + t.Errorf("the credential file %s is %v (%v), want 0600", userconfig, st.Mode().Perm(), err) + } + return inner(ctx, env, name, args...) + } + if err := tc.Ensure(context.Background(), "v1"); err != nil { + t.Fatal(err) + } + if rel, err := filepath.Rel(tc.Paths.Home, userconfig); err != nil || rel != filepath.Base(userconfig) { + t.Errorf("the credential file %s is not in the account's home %s", userconfig, tc.Paths.Home) + } + if _, err := os.Stat(userconfig); err == nil { + t.Errorf("the credential file %s is left after the install", userconfig) + } + filepath.WalkDir(tc.Paths.Cache, func(p string, d fs.DirEntry, err error) error { + if err == nil && strings.Contains(d.Name(), "npmrc") { + t.Errorf("a credential file %s lies beneath the cache", p) + } + return nil + }) +} + +// 1891 review (ADR 0533 §2): open refuses a toolchain folder that holds an npm config file, naming it, and opens +// nothing of it, so a credential left by a dead install is never made readable. +func TestOpenRefusesAFolderHoldingAnNpmConfig(t *testing.T) { + fake := &fakeNpm{} + tc := toolchainIn(t, fake) + dir := tc.Paths.Dir("v1") + os.MkdirAll(filepath.Join(dir, "node_modules"), 0o700) + left := filepath.Join(dir, ".npmrc-install") + os.WriteFile(left, []byte("//x/:_auth=secret\n"), 0o600) + os.WriteFile(filepath.Join(dir, installedMark), []byte("v1\n"), 0o600) + err := tc.Ensure(context.Background(), "v1") + if err == nil || !strings.Contains(err.Error(), left) { + t.Errorf("Ensure = %v, want a refusal naming %s", err, left) + } + if st, _ := os.Stat(left); st.Mode().Perm() != 0o600 { + t.Errorf("the credential file was opened to %o", st.Mode().Perm()) + } + if st, _ := os.Stat(dir); st.Mode().Perm()&0o005 != 0 { + t.Errorf("the folder holding it was opened to %o", st.Mode().Perm()) + } +} -- 2.54.0