From def3ccefe3c8f26a4bb2dc6f95ba6efd9949bb6d Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:51:48 +0000 Subject: [PATCH] Refuse templates read through a symlink anywhere under files/ A symlinked files/ or a symlinked directory under it let module check, the merge gate and module add read a template from outside the repository tree, and a symlink no template names passed them while the build's packing refuses it (review of mesh-controller#313). Every entry under files/ must now be a plain directory or file, and templates are read through an os.Root of the module's directory. Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-controller/template_test.go | 27 +++++++++++ internal/catalogue/template.go | 72 +++++++++++++++++++++++++--- internal/catalogue/template_test.go | 65 +++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 6 deletions(-) diff --git a/cmd/mesh-controller/template_test.go b/cmd/mesh-controller/template_test.go index 60ca0e74..edcc66a9 100644 --- a/cmd/mesh-controller/template_test.go +++ b/cmd/mesh-controller/template_test.go @@ -62,6 +62,18 @@ func TestModuleCheckReadsTemplatesFromTheTreeBesideTheManifest(t *testing.T) { t.Fatalf("a template whose text is refused inline passed:\n%s", out.String()) } + // A symlink under files/ that no template names: the build's packing refuses it, so the check does too. + linked := t.TempDir() + aTemplatedModule(t, linked, templatedNotifier, map[string]string{"files/notifier.conf": "volume = 3\n"}) + if err := os.Symlink("/etc/hostname", filepath.Join(linked, "files", "elsewhere")); err != nil { + t.Fatal(err) + } + out.Reset() + if err := moduleCheck([]string{filepath.Join(linked, "module.json")}, &out); err == nil || + !strings.Contains(out.String(), "files/elsewhere is a symlink") { + t.Fatalf("a symlink under files/ passed the check:\n%s", out.String()) + } + // And a template the tree lacks, by name. missing := aTemplatedModule(t, t.TempDir(), templatedNotifier, nil) out.Reset() @@ -139,6 +151,21 @@ func TestTheMergeGateJudgesAChangeToATemplateAlone(t *testing.T) { t.Fatalf("a template the controller refuses passed the gate:\n%s", v.Report()) } + // A symlinked files/ fails: nothing is read from outside the tree. + tree := aTree(t, withEdit(manifests, "notifier", "")) + elsewhere := t.TempDir() + if err := os.WriteFile(filepath.Join(elsewhere, "notifier.conf"), []byte("volume = 3\n"), 0o644); err != nil { + t.Fatal(err) + } + aTemplatedModule(t, filepath.Join(tree, "modules", "notifier"), templatedNotifier, nil) + if err := os.Symlink(elsewhere, filepath.Join(tree, "modules", "notifier", "files")); err != nil { + t.Fatal(err) + } + v = gateJudged(t, f, tree, notifierTemplate) + if v.Verdict != "fail" || !strings.Contains(v.Report(), "files/ is a symlink") { + t.Fatalf("a symlinked files/ passed the gate:\n%s", v.Report()) + } + // And the template removed from the tree fails, naming it. v = gateJudged(t, f, aTemplatedTree(t, manifests, "", false), notifierTemplate) if v.Verdict != "fail" || !strings.Contains(v.Report(), "files/notifier.conf") { diff --git a/internal/catalogue/template.go b/internal/catalogue/template.go index ac785e2a..2a363bf2 100644 --- a/internal/catalogue/template.go +++ b/internal/catalogue/template.go @@ -180,11 +180,18 @@ func fillTemplates(module string, resources []map[string]any, text func(string) } // ReadTemplates is the text of each template the manifest names, read from the module's directory in a repository -// tree, by template path. A missing template, or one that is not a plain file (a symlink is refused by the build's -// packing too), is refused by name. +// tree, by template path. Refused by name: a template the directory lacks, and anything under files/ the build's +// packing refuses — files/ itself, or any directory or file under it, that is a symlink or not a plain file — so +// nothing judged here can be read from outside the tree, and no tree passes here that the build would refuse. Each +// template is read through an os.Root of the module's directory besides, which no path element can leave. func ReadTemplates(m Manifest, dir string) (map[string]string, error) { + problems := templatesTreeProblems(m.Module, dir) + root, err := os.OpenRoot(dir) + if err != nil { + return nil, fmt.Errorf("%s: the module's directory cannot be opened to read its templates: %w", m.Module, err) + } + defer root.Close() texts := map[string]string{} - var problems []string for _, r := range m.Resources { name, has, wrong := templateOf(m.Module, r) if !has { @@ -197,8 +204,7 @@ func ReadTemplates(m Manifest, dir string) (map[string]string, error) { if _, read := texts[name]; read { continue } - full := filepath.Join(dir, filepath.FromSlash(name)) - info, err := os.Lstat(full) + info, err := root.Lstat(name) switch { case errors.Is(err, fs.ErrNotExist): problems = append(problems, fmt.Sprintf("%s: %v names the template %q, and the module's directory "+ @@ -213,7 +219,7 @@ func ReadTemplates(m Manifest, dir string) (map[string]string, error) { "the build packs only files", m.Module, r["id"], name)) continue } - body, err := os.ReadFile(full) + body, err := root.ReadFile(name) if err != nil { problems = append(problems, fmt.Sprintf("%s: %v names the template %q, which cannot be read: %v", m.Module, r["id"], name, err)) @@ -227,6 +233,60 @@ func ReadTemplates(m Manifest, dir string) (map[string]string, error) { return texts, nil } +// templatesTreeProblems is everything under the module's files/ that the build's packing refuses: files/ itself not +// a plain directory, and any entry under it that is neither a directory nor a plain file — a symlink, to a file or a +// directory, a device, a pipe. Each said by its path. A files/ that does not exist is no problem here: each template +// it would have held is refused as missing. +func templatesTreeProblems(module, dir string) []string { + top := filepath.Join(dir, TemplatesDir) + info, err := os.Lstat(top) + if errors.Is(err, fs.ErrNotExist) { + return nil + } + if err != nil { + return []string{fmt.Sprintf("%s: %s/ cannot be read: %v", module, TemplatesDir, err)} + } + if !info.IsDir() { + return []string{fmt.Sprintf("%s: %s/ is %s, not a directory of the module's own; its templates are read "+ + "only from inside the repository, and the build packs only directories and files", module, TemplatesDir, + kindOf(info.Mode()))} + } + var problems []string + err = filepath.WalkDir(top, func(at string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || d.Type().IsRegular() { + return nil + } + rel, _ := filepath.Rel(dir, at) + problems = append(problems, fmt.Sprintf("%s: %s is %s; the build packs only directories and files, so "+ + "%s/ holds nothing else", module, filepath.ToSlash(rel), kindOf(d.Type()), TemplatesDir)) + return nil + }) + if err != nil { + problems = append(problems, fmt.Sprintf("%s: %s/ cannot be read: %v", module, TemplatesDir, err)) + } + return problems +} + +// kindOf says what a file that is not a plain file is. +func kindOf(mode fs.FileMode) string { + switch { + case mode&fs.ModeSymlink != 0: + return "a symlink" + case mode&fs.ModeDevice != 0: + return "a device" + case mode&fs.ModeNamedPipe != 0: + return "a named pipe" + case mode&fs.ModeSocket != 0: + return "a socket" + case mode.IsDir(): + return "a directory" + } + return "not a plain file" +} + // WithTemplatesFrom is the manifest with each template's text read from the module's directory in a repository // tree as its content: what `module check`, the merge gate and `module add` judge, where nothing was built. Read // again strictly afterwards, so every rule about a file's text judges the template's text exactly as it judges diff --git a/internal/catalogue/template_test.go b/internal/catalogue/template_test.go index 85c81896..a439acb0 100644 --- a/internal/catalogue/template_test.go +++ b/internal/catalogue/template_test.go @@ -256,3 +256,68 @@ func TestAManifestStillNamingATemplateIsNotHeld(t *testing.T) { t.Fatalf("an inline manifest is refused: %s", problem) } } + +// A symlinked files/, or a symlinked directory under it, would read a file outside the tree: refused by name, +// as is any symlink or special file under files/ that no template names, which the build's packing refuses. +func TestASymlinkAnywhereUnderFilesIsRefusedByName(t *testing.T) { + m, err := ParseManifest([]byte(templatedTwin)) + if err != nil { + t.Fatal(err) + } + outside := t.TempDir() + if err := os.MkdirAll(filepath.Join(outside, "deeper"), 0o755); err != nil { + t.Fatal(err) + } + for _, name := range []string{"tidy.conf", "deeper/extra.conf", "secret"} { + if err := os.WriteFile(filepath.Join(outside, filepath.FromSlash(name)), []byte("outside"), 0o644); err != nil { + t.Fatal(err) + } + } + write := func(t *testing.T, dir, name string) { + t.Helper() + full := filepath.Join(dir, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte("inside"), 0o644); err != nil { + t.Fatal(err) + } + } + link := func(t *testing.T, target, at string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(at), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, at); err != nil { + t.Fatal(err) + } + } + for _, c := range []struct { + name string + make func(t *testing.T, dir string) + names string + }{ + {"files/ itself", func(t *testing.T, dir string) { + link(t, outside, filepath.Join(dir, "files")) + }, "files/ is a symlink"}, + {"a directory under files/", func(t *testing.T, dir string) { + write(t, dir, "files/tidy.conf") + link(t, filepath.Join(outside, "deeper"), filepath.Join(dir, "files", "deeper")) + }, "files/deeper is a symlink"}, + {"a file no template names", func(t *testing.T, dir string) { + write(t, dir, "files/tidy.conf") + write(t, dir, "files/deeper/extra.conf") + link(t, filepath.Join(outside, "secret"), filepath.Join(dir, "files", "unnamed")) + }, "files/unnamed is a symlink"}, + } { + dir := t.TempDir() + c.make(t, dir) + _, err := WithTemplatesFrom(m, dir) + if err == nil || !strings.Contains(err.Error(), c.names) { + t.Errorf("a symlinked %s was not refused by name: %v", c.name, err) + } + if err != nil && strings.Contains(err.Error(), "outside") { + t.Errorf("a symlinked %s was read: %v", c.name, err) + } + } +}