mesh-build-node: write the install's registry credential in the account's home, never beneath the opened cache, and refuse to open a toolchain holding one (issue 1891 review, ADR 0533 §2)

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")
+	// The credential is written in the account's own home (0700), never beneath the cache, whose toolchains are
-	if err := os.WriteFile(npmrc, []byte(reg.Npmrc()), 0o600); err != nil {
+	// 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
 	}

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())
+	}
+}