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