From 0b48dbc5c1b28c2387bb791f2304d79dae765a90 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 24 Aug 2026 11:20:05 +0200 Subject: [PATCH 1/2] feat(genapp): add WithRoot to confine written files to a directory WithRoot(dir) confines every file a GoGenApp writes to dir, built on os.Root. The output path must sit at or below dir, and dir must exist: openRoot reports a missing root instead of building a tree under a mistyped path. Without the option the root is the output path itself, so one code path serves both cases. RenderFile now rejects an absolute target, one that climbs out with "..", and a Windows volume or UNC prefix, whether or not WithRoot is set. A spec supplies the target names, so those checks are not optional. Writes: - MkdirAll, the temporary file, chmod and rename all go through os.Root. os.MkdirAll walked through a symlinked directory and created files outside the output path. - scratchLink removes a symbolic link at the target instead of following it, and refuses a directory, device, socket or fifo. os.Root refuses a link leaving the root but follows one that stays inside it, so the root alone does not cover this. - InitModule wrote with os.WriteFile, which follows a symbolic link at go.mod and writes through it. It now writes through a temporary file and renames, so a go.mod appears whole or not at all, and checkModuleAbsent reads the entry with Lstat, so a dangling link counts as present. os.Root has no CreateTemp, so createTemp opens a random name with O_EXCL and tries another when that name is taken. Holding the *os.Root on GoGenApp would give the type a lifetime and a Close method it has never had. openRoot opens one per call instead, which keeps Render and RenderFile concurrent, at one openat per file. TidyModule stays outside this: it runs the go command, which writes go.mod and go.sum itself. PackagePath and EnclosingModule walk up past any root by design and stay unconfined. The temp-and-rename write in writeFile already replaced a symbolic link rather than following it, and already broke a hard link rather than writing through the shared inode. That code is unchanged. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- genapp/doc.go | 29 ++++ genapp/genapp.go | 63 ++++--- genapp/module.go | 73 ++++++-- genapp/options.go | 37 +++++ genapp/paths.go | 76 +++++++++ genapp/root.go | 178 ++++++++++++++++++++ genapp/rooted_test.go | 375 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 793 insertions(+), 38 deletions(-) create mode 100644 genapp/paths.go create mode 100644 genapp/root.go create mode 100644 genapp/rooted_test.go diff --git a/genapp/doc.go b/genapp/doc.go index 9b009cb..f3acad8 100644 --- a/genapp/doc.go +++ b/genapp/doc.go @@ -47,6 +47,35 @@ // [WithSkipFormatFunc] to decide differently, or [WithSkipFormat] to write every target unformatted, // which is worth doing when a template is misbehaving and the parse error hides the output. // +// # Writing outside the output path +// +// A generator names its targets after the models and operations of a spec, so the spec decides what +// [GoGenApp.RenderFile] writes. RenderFile therefore refuses an absolute target, and one that climbs +// out of the output path with "..", whether or not the caller asks to be confined. +// +// Writes go through [os.Root], which checks each symbolic link as it walks the path. RenderFile +// removes a link standing at the target instead of following it, so the file it pointed at keeps its +// content, and refuses a link on the way to the target. Renaming replaces a name rather than the +// file behind it, so another hard link to the target keeps its own content. A directory, a device, +// a socket or a named pipe at the target is refused rather than overwritten. +// +// [WithRoot] widens the boundary from the output path to a directory above it, for a generator +// writing into several directories of one tree: +// +// app, err := genapp.New( +// genapp.WithTemplates(templates), +// genapp.WithOutputPath("./gen/models"), +// genapp.WithRoot("./gen"), +// ) +// +// Two things sit outside this. [GoGenApp.TidyModule] runs the go command, which writes go.mod and +// go.sum itself, and no root reaches into another process. Reads run unconfined too: +// [GoGenApp.PackagePath] and [GoGenApp.EnclosingModule] walk up from the output path looking for a +// go.mod, and that go.mod usually sits above any root worth setting. +// +// [os.Root] confines path resolution and no more. It does not stop traversal of a bind mount, a +// /proc special file or a device file. +// // # Where the code lands // // A generator has to write the imports that reach the code it produces, and that means knowing the diff --git a/genapp/genapp.go b/genapp/genapp.go index 4dafa46..8b0a8f4 100644 --- a/genapp/genapp.go +++ b/genapp/genapp.go @@ -96,6 +96,11 @@ func (g *GoGenApp) Render(w io.Writer, name string, data any) error { // reports a line and a column, and reading them means reading the source they came from; that // source would otherwise be gone. func (g *GoGenApp) RenderFile(target, name string, data any) error { + cleaned, err := checkedTarget(target) + if err != nil { + return err + } + rendered := shared.BorrowBuffer() defer shared.RedeemBuffer(rendered) @@ -103,12 +108,26 @@ func (g *GoGenApp) RenderFile(target, name string, data any) error { return err } - path := filepath.Join(g.outputPath, filepath.FromSlash(target)) - if err := os.MkdirAll(filepath.Dir(path), dirPerm); err != nil { - return fmt.Errorf("cannot create the directory for %q: %w: %w", target, err, ErrGenApp) + root, down, err := g.openRoot() + if err != nil { + return err + } + defer func() { _ = root.Close() }() + + written := destination{ + root: root, + rel: filepath.Join(down, filepath.FromSlash(cleaned)), + target: target, + displayed: filepath.Join(g.outputPath, filepath.FromSlash(cleaned)), + } + + if dir := written.dir(); dir != "." { + if err := root.MkdirAll(dir, dirPerm); err != nil { + return fmt.Errorf("cannot create the directory for %q: %w: %w", target, err, ErrGenApp) + } } - return g.writeFile(path, target, name, rendered) + return g.writeFile(written, name, rendered) } // execute renders one template into the buffer it is given. @@ -147,8 +166,8 @@ const unformattedSuffix = ".unformatted" // // The returned error carries the formatting failure and adds the path. It attaches no second // [ErrGenApp]: the cause already carries one. -func dumpUnformatted(file *os.File, path string, rendered *bytes.Buffer, cause error) error { - dumped := path + unformattedSuffix +func dumpUnformatted(d destination, file *os.File, temporary string, rendered *bytes.Buffer, cause error) error { + dumped := d.sibling(unformattedSuffix) written := func() error { if err := file.Truncate(0); err != nil { @@ -171,14 +190,16 @@ func dumpUnformatted(file *os.File, path string, rendered *bytes.Buffer, cause e return err } - return os.Rename(file.Name(), dumped) + return commit(dumped, temporary) }() if written != nil { - return fmt.Errorf("could not keep the unformatted output at %q (%w): %w", dumped, written, cause) + return fmt.Errorf( + "could not keep the unformatted output at %q (%w): %w", dumped.displayed, written, cause, + ) } - return fmt.Errorf("the unformatted output is kept at %q: %w", dumped, cause) + return fmt.Errorf("the unformatted output is kept at %q: %w", dumped.displayed, cause) } // format writes the formatted render, and hands the imports report to the caller's sink. @@ -200,10 +221,10 @@ func (g *GoGenApp) format(w io.Writer, name string, rendered *bytes.Buffer) erro } // writeFile writes the target through a temporary file in the same directory, then renames. -func (g *GoGenApp) writeFile(path, target, name string, rendered *bytes.Buffer) (err error) { - temporary, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*") +func (g *GoGenApp) writeFile(d destination, name string, rendered *bytes.Buffer) (err error) { + temporary, temporaryRel, err := createTemp(d.root, d.dir(), filepath.Base(d.rel)) if err != nil { - return fmt.Errorf("cannot create a temporary file for %q: %w: %w", target, err, ErrGenApp) + return fmt.Errorf("cannot create a temporary file for %q: %w: %w", d.target, err, ErrGenApp) } keep := false @@ -214,31 +235,27 @@ func (g *GoGenApp) writeFile(path, target, name string, rendered *bytes.Buffer) } _ = temporary.Close() - _ = os.Remove(temporary.Name()) + _ = d.root.Remove(temporaryRel) }() - if g.skipsFormat(target) { + if g.skipsFormat(d.target) { if _, err = temporary.Write(rendered.Bytes()); err != nil { - return fmt.Errorf("cannot write %q: %w: %w", target, err, ErrGenApp) + return fmt.Errorf("cannot write %q: %w: %w", d.target, err, ErrGenApp) } } else if formatErr := g.format(temporary, name, rendered); formatErr != nil { keep = true - err = dumpUnformatted(temporary, path, rendered, formatErr) + err = dumpUnformatted(d, temporary, temporaryRel, rendered, formatErr) return err } if err = temporary.Chmod(filePerm); err != nil { - return fmt.Errorf("cannot set the mode of %q: %w: %w", target, err, ErrGenApp) + return fmt.Errorf("cannot set the mode of %q: %w: %w", d.target, err, ErrGenApp) } if err = temporary.Close(); err != nil { - return fmt.Errorf("cannot close %q: %w: %w", target, err, ErrGenApp) + return fmt.Errorf("cannot close %q: %w: %w", d.target, err, ErrGenApp) } - if err = os.Rename(temporary.Name(), path); err != nil { - return fmt.Errorf("cannot write %q: %w: %w", target, err, ErrGenApp) - } - - return nil + return commit(d, temporaryRel) } diff --git a/genapp/module.go b/genapp/module.go index cdd7332..b9904ab 100644 --- a/genapp/module.go +++ b/genapp/module.go @@ -7,13 +7,12 @@ import ( "errors" "fmt" "io/fs" - "os" "path/filepath" "golang.org/x/mod/modfile" ) -// goModFile is the name the go command gives a module definition. +// goModFile is "go.mod". The go command reads a module definition from a file of that name. const goModFile = "go.mod" // InitModule writes a go.mod in the output path, as "go mod init" would. @@ -35,41 +34,85 @@ func (g *GoGenApp) InitModule(opts ...ModOption) error { return err } - path := filepath.Join(g.outputPath, goModFile) - - if err := g.checkModuleAbsent(path, o); err != nil { + content, err := buildModFile(o) + if err != nil { return err } - content, err := buildModFile(o) + root, down, err := g.openRoot() if err != nil { return err } + defer func() { _ = root.Close() }() + + written := destination{ + root: root, + rel: filepath.Join(down, goModFile), + target: filepath.Join(g.outputPath, goModFile), + displayed: filepath.Join(g.outputPath, goModFile), + } + + if err := checkModuleAbsent(written, o); err != nil { + return err + } + + if down != "." { + if err := root.MkdirAll(down, dirPerm); err != nil { + return fmt.Errorf("cannot create the module directory %q: %w: %w", g.outputPath, err, ErrGenApp) + } + } + + return writeModFile(written, content) +} - if err := os.MkdirAll(g.outputPath, dirPerm); err != nil { - return fmt.Errorf("cannot create the module directory %q: %w: %w", g.outputPath, err, ErrGenApp) +// writeModFile writes a go.mod through a temporary file, then renames. +// +// It writes through a temporary file and renames, as [GoGenApp.RenderFile] does, so a go.mod +// appears whole or not at all, and a symbolic link standing at the path is removed rather than +// written through. +func writeModFile(d destination, content []byte) (err error) { + temporary, temporaryRel, err := createTemp(d.root, d.dir(), goModFile) + if err != nil { + return fmt.Errorf("cannot create a temporary file for %q: %w: %w", d.target, err, ErrGenApp) } - if err := os.WriteFile(path, content, filePerm); err != nil { - return fmt.Errorf("cannot write %q: %w: %w", path, err, ErrGenApp) + defer func() { + if err == nil { + return + } + + _ = temporary.Close() + _ = d.root.Remove(temporaryRel) + }() + + if _, err = temporary.Write(content); err != nil { + return fmt.Errorf("cannot write %q: %w: %w", d.target, err, ErrGenApp) } - return nil + if err = temporary.Close(); err != nil { + return fmt.Errorf("cannot close %q: %w: %w", d.target, err, ErrGenApp) + } + + return commit(d, temporaryRel) } // checkModuleAbsent reports an existing go.mod, unless the caller asked to replace it. -func (g *GoGenApp) checkModuleAbsent(path string, o modOptions) error { +// +// It reads the entry with [os.Root.Lstat] rather than Stat, so a symbolic link at the path counts +// as something already there. Stat follows the link and calls a dangling one absent, and InitModule +// would then write over a link the caller never put there. +func checkModuleAbsent(d destination, o modOptions) error { if o.replace { return nil } - switch _, err := os.Stat(path); { + switch _, err := d.root.Lstat(d.rel); { case err == nil: - return fmt.Errorf("%q exists, see WithReplaceExisting: %w: %w", path, fs.ErrExist, ErrGenApp) + return fmt.Errorf("%q exists, see WithReplaceExisting: %w: %w", d.target, fs.ErrExist, ErrGenApp) case errors.Is(err, fs.ErrNotExist): return nil default: - return fmt.Errorf("cannot read %q: %w: %w", path, err, ErrGenApp) + return fmt.Errorf("cannot read %q: %w: %w", d.target, err, ErrGenApp) } } diff --git a/genapp/options.go b/genapp/options.go index 1c5fe98..486c96d 100644 --- a/genapp/options.go +++ b/genapp/options.go @@ -18,6 +18,7 @@ type ( options struct { templates *repo.Repository outputPath string + root string formatOptions []formatting.Option importsReporter func(string, *formatting.ImportsReport) skipFormat bool @@ -56,6 +57,42 @@ func WithOutputPath(path string) Option { } } +// WithRoot confines every file a [GoGenApp] writes to dir. +// +// The output path must sit at or below dir, and dir must exist. The caller declares the root, so +// WithRoot reports a missing one instead of creating it. Nothing outside dir is written, whether a +// target climbs out with "..", names an absolute path, or reaches a symbolic link pointing away. +// [os.Root] checks each link as it walks the path, where a prefix test on the name alone would miss +// a link halfway down. +// +// app, err := genapp.New( +// genapp.WithTemplates(templates), +// genapp.WithOutputPath("./gen/models"), +// genapp.WithRoot("./gen"), +// ) +// +// Use it when a spec supplies the target names, such as its operation and model names. +// +// Without it, writes still stay under the output path and the checks on a target still run. +// WithRoot widens the boundary past the output path, for a generator writing into several +// directories of one tree. +// +// It covers this package's writes and no more. [GoGenApp.TidyModule] runs the go command, which +// writes go.mod and go.sum itself, and no root reaches into another process. Reads run unconfined +// too: [GoGenApp.PackagePath] and [GoGenApp.EnclosingModule] walk up from the output path looking +// for a go.mod, and that go.mod usually sits above any root worth setting. +// +// [os.Root] confines path resolution and no more. It does not stop traversal of a bind mount, a +// /proc special file or a device file, so point WithRoot at a directory that holds only generated +// output. +func WithRoot(dir string) Option { + return func(o options) options { + o.root = dir + + return o + } +} + // WithFormatOptions configures the formatter. // // Grouping, gofumpt and the rest are settled by diff --git a/genapp/paths.go b/genapp/paths.go new file mode 100644 index 0000000..11a1f9e --- /dev/null +++ b/genapp/paths.go @@ -0,0 +1,76 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package genapp + +import ( + "fmt" + "path" + "path/filepath" + "strings" +) + +// checkedTarget cleans a target into a slash-separated relative name, and rejects one that would +// write outside the output path. +// +// A target names a file under the output path, so checkedTarget refuses an absolute path instead of +// rebasing it. A generator renders no file addressed from the root of the file system, so a target +// that starts there came from somewhere else. +// +// [github.com/go-openapi/swag/loading.WithRoot] rebases such a path instead, because +// github.com/go-openapi/spec normalizes every $ref it loads to an absolute path. RenderFile reads +// no such input. +// +// The checks run whether or not [WithRoot] is set. A spec supplies the target names, so RenderFile +// keeps writing under the output path even when nothing else confines it. +func checkedTarget(target string) (string, error) { + if strings.TrimSpace(target) == "" { + return "", fmt.Errorf("the target names no file: %w", ErrGenApp) + } + + // filepath.IsAbs reads "/x" as relative on Windows, and a volume name covers "C:x" and + // "\\\\server\\share" alike, neither of which filepath.IsAbs reports on its own. + if filepath.IsAbs(target) || path.IsAbs(filepath.ToSlash(target)) || filepath.VolumeName(target) != "" { + return "", fmt.Errorf( + "the target %q is an absolute path, RenderFile writes under the output path: %w", target, ErrGenApp, + ) + } + + cleaned := path.Clean(filepath.ToSlash(target)) + + if cleaned == ".." || strings.HasPrefix(cleaned, "../") { + return "", fmt.Errorf("the target %q climbs out of the output path: %w", target, ErrGenApp) + } + + if cleaned == "." { + return "", fmt.Errorf("the target %q names a directory, not a file: %w", target, ErrGenApp) + } + + return cleaned, nil +} + +// within returns the way down from base to target, and rejects a target that does not sit below it. +// +// [filepath.Abs] resolves both paths first, so a relative base still compares against an absolute +// target. [filepath.Rel] then follows ".." and ".", where a prefix test would match the names as +// text and miss the traversal. +func within(base, target string) (string, error) { + absoluteBase, err := filepath.Abs(base) + if err != nil { + return "", fmt.Errorf("cannot resolve the root %q: %w: %w", base, err, ErrGenApp) + } + + absoluteTarget, err := filepath.Abs(target) + if err != nil { + return "", fmt.Errorf("cannot resolve the output path %q: %w: %w", target, err, ErrGenApp) + } + + down, err := filepath.Rel(absoluteBase, absoluteTarget) + if err != nil || down == ".." || strings.HasPrefix(down, ".."+string(filepath.Separator)) { + return "", fmt.Errorf( + "the output path %q is outside the root %q: %w", target, base, ErrGenApp, + ) + } + + return down, nil +} diff --git a/genapp/root.go b/genapp/root.go new file mode 100644 index 0000000..5fbd8d6 --- /dev/null +++ b/genapp/root.go @@ -0,0 +1,178 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package genapp + +import ( + "errors" + "fmt" + "io/fs" + "math/rand/v2" + "os" + "path/filepath" + "strconv" +) + +// tempAttempts caps how many names createTemp tries before it fails. +const tempAttempts = 1000 + +// destination addresses one file three ways, because a write needs each of them. +type destination struct { + root *os.Root + + // rel addresses the file from the root. Every [os.Root] method takes this form. + rel string + + // target is the caller's argument to [GoGenApp.RenderFile]. Error messages quote it. + target string + + // displayed prefixes target with the output path, so a reader can open it. Only the messages + // naming a file left on disk need it: a relative target alone does not locate that file. + displayed string +} + +// sibling names a file beside this one, suffixed in every form at once. +func (d destination) sibling(suffix string) destination { + d.rel += suffix + d.target += suffix + d.displayed += suffix + + return d +} + +// dir returns the directory holding the file, addressed from the root. +func (d destination) dir() string { return filepath.Dir(d.rel) } + +// openRoot opens the directory that confines every write, and returns the way down to the output +// path from there. +// +// Without [WithRoot] the root is the output path itself, so one code path serves both cases. +// openRoot creates that directory first, as [GoGenApp.RenderFile] has always created its output +// directory. +// +// With [WithRoot] the root has to exist. The caller declares the root, so openRoot reports a +// missing one instead of building a tree under a mistyped path. +// +// openRoot opens the root per call, and the caller closes it. A [GoGenApp] holds no state between +// calls, which is what lets [GoGenApp.Render] and [GoGenApp.RenderFile] run concurrently. An open +// root would add a lifetime to the type, and a Close method with it. The cost is one openat per +// file. +func (g *GoGenApp) openRoot() (*os.Root, string, error) { + output := g.outputPath + if output == "" { + output = "." + } + + if g.root == "" { + if err := os.MkdirAll(output, dirPerm); err != nil { + return nil, "", fmt.Errorf("cannot create the output path %q: %w: %w", output, err, ErrGenApp) + } + + root, err := os.OpenRoot(output) + if err != nil { + return nil, "", fmt.Errorf("cannot open the output path %q: %w: %w", output, err, ErrGenApp) + } + + return root, ".", nil + } + + down, err := within(g.root, output) + if err != nil { + return nil, "", err + } + + root, err := os.OpenRoot(g.root) + if err != nil { + return nil, "", fmt.Errorf("cannot open the root %q, see WithRoot: %w: %w", g.root, err, ErrGenApp) + } + + return root, down, nil +} + +// createTemp opens a temporary file beside the target, under an unused name. +// +// [os.Root] has no CreateTemp, so this opens a random name with O_EXCL and tries another when that +// name is taken. O_EXCL fails on a name that already exists, so the suffix only has to avoid +// collisions rather than be unguessable. +func createTemp(root *os.Root, dir, base string) (*os.File, string, error) { + for range tempAttempts { + name := filepath.Join(dir, "."+base+"."+strconv.FormatUint(rand.Uint64(), 36)) //nolint:gosec // O_EXCL names it + + file, err := root.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, filePerm) + switch { + case err == nil: + return file, name, nil + case errors.Is(err, fs.ErrExist): + continue + default: + return nil, "", err + } + } + + return nil, "", fmt.Errorf("no free temporary name beside %q after %d tries: %w", base, tempAttempts, fs.ErrExist) +} + +// scratchLink clears the path so that a rename lands on a regular file. +// +// A symbolic link is removed rather than followed. [os.Root] refuses a link pointing out of the +// root but follows one that stays inside it, so the root alone does not cover this case: a link +// inside a generated tree still redirects the write. +// +// A regular file is left where it is, since the rename replaces it. Anything else — a directory, a +// device, a socket, a fifo — is refused: a generator rendering a file over one of those has been +// pointed at the wrong path. +// +// Removing the link rather than the file it points at also keeps the rename safe for a hard link. +// A rename replaces a name, so another name for the same file keeps the content it had. +func scratchLink(d destination) error { + info, err := d.root.Lstat(d.rel) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil + } + + return fmt.Errorf("cannot read %q: %w: %w", d.target, err, ErrGenApp) + } + + switch mode := info.Mode(); { + case mode&fs.ModeSymlink != 0: + if err := d.root.Remove(d.rel); err != nil { + return fmt.Errorf("cannot remove the symbolic link at %q: %w: %w", d.target, err, ErrGenApp) + } + + return nil + case mode.IsRegular(): + return nil + default: + return fmt.Errorf("%q is a %s, which genapp does not overwrite: %w", d.target, modeName(mode), ErrGenApp) + } +} + +// modeName names a file type as an error message says it. +func modeName(mode fs.FileMode) string { + switch { + case mode.IsDir(): + return "directory" + case mode&fs.ModeDevice != 0: + return "device" + case mode&fs.ModeNamedPipe != 0: + return "named pipe" + case mode&fs.ModeSocket != 0: + return "socket" + default: + return "not a regular file" + } +} + +// commit moves a temporary file onto the target, once nothing but a regular file stands there. +func commit(d destination, temporary string) error { + if err := scratchLink(d); err != nil { + return err + } + + if err := d.root.Rename(temporary, d.rel); err != nil { + return fmt.Errorf("cannot write %q: %w: %w", d.target, err, ErrGenApp) + } + + return nil +} diff --git a/genapp/rooted_test.go b/genapp/rooted_test.go new file mode 100644 index 0000000..ec13f79 --- /dev/null +++ b/genapp/rooted_test.go @@ -0,0 +1,375 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package genapp_test + +import ( + "net" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" + + "github.com/go-openapi/codegen/genapp" +) + +// skipWithoutSymlinks skips a test on a runner that cannot create one. +// +// Windows grants the privilege to an administrator or to a machine in developer mode, and the CI +// runner is neither, so the symlink cases are skipped there rather than failing on a setup step. +func skipWithoutSymlinks(t *testing.T, dir string) { + t.Helper() + + if runtime.GOOS != "windows" { + return + } + + probe := filepath.Join(dir, ".symlink-probe") + if err := os.Symlink(filepath.Join(dir, ".absent"), probe); err != nil { + t.Skip("this runner cannot create symbolic links") + } + + require.NoError(t, os.Remove(probe)) +} + +// assertAbsent reports a path that a confined write must not have created. +func assertAbsent(t *testing.T, path string, msgAndArgs ...any) { + t.Helper() + + _, err := os.Lstat(path) + assert.ErrorIs(t, err, os.ErrNotExist, msgAndArgs...) +} + +// outside builds a directory beside the output path, holding a file a test tries to reach. +func outside(t *testing.T) (dir, secret string) { + t.Helper() + + dir = t.TempDir() + secret = filepath.Join(dir, "secret.txt") + require.NoError(t, os.WriteFile(secret, []byte("SECRET"), 0o600)) + + return dir, secret +} + +func TestTargetIsConfined(t *testing.T) { + t.Parallel() + + t.Run("should refuse a target that climbs out of the output path", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + app := newApp(t, genapp.WithOutputPath(filepath.Join(dir, "out"))) + + err := app.RenderFile("../../escaped.go", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assert.Contains(t, err.Error(), "climbs out of the output path") + assertAbsent(t, filepath.Join(dir, "escaped.go")) + }) + + t.Run("should refuse an absolute target", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + app := newApp(t, genapp.WithOutputPath(filepath.Join(dir, "out"))) + + absolute := filepath.Join(dir, "absolute.go") + + err := app.RenderFile(absolute, "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assert.Contains(t, err.Error(), "is an absolute path") + assertAbsent(t, absolute) + }) + + t.Run("should refuse a rooted target on every platform", func(t *testing.T) { + t.Parallel() + + app := newApp(t, genapp.WithOutputPath(t.TempDir())) + + err := app.RenderFile("/etc/passwd", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assert.Contains(t, err.Error(), "is an absolute path", + `filepath.IsAbs reads "/etc/passwd" as relative on Windows, so path.IsAbs decides too`) + }) + + t.Run("should refuse a target naming no file", func(t *testing.T) { + t.Parallel() + + app := newApp(t, genapp.WithOutputPath(t.TempDir())) + + for _, target := range []string{"", " ", ".", "a/.."} { + err := app.RenderFile(target, "model", pet) + require.ErrorIs(t, err, genapp.ErrGenApp, "target %q", target) + } + }) + + t.Run("should keep a target that stays under the output path", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + app := newApp(t, genapp.WithOutputPath(dir)) + + require.NoError(t, app.RenderFile("models/./nested/../pet.go", "model", pet)) + assert.FileExists(t, filepath.Join(dir, "models", "pet.go")) + }) +} + +func TestSymlinkIsNotFollowed(t *testing.T) { + t.Parallel() + + t.Run("should scratch a symbolic link standing at the target", func(t *testing.T) { + t.Parallel() + + dir, secret := outside(t) + skipWithoutSymlinks(t, dir) + + out := filepath.Join(dir, "out") + require.NoError(t, os.MkdirAll(out, 0o750)) + require.NoError(t, os.Symlink(secret, filepath.Join(out, "pet.go"))) + + app := newApp(t, genapp.WithOutputPath(out)) + require.NoError(t, app.RenderFile("pet.go", "model", pet)) + + kept, err := os.ReadFile(secret) + require.NoError(t, err) + assert.Equal(t, "SECRET", string(kept), "the link was replaced, not written through") + + info, err := os.Lstat(filepath.Join(out, "pet.go")) + require.NoError(t, err) + assert.True(t, info.Mode().IsRegular(), "a regular file stands where the link did") + }) + + t.Run("should refuse a symbolic link on the way to the target", func(t *testing.T) { + t.Parallel() + + dir, _ := outside(t) + skipWithoutSymlinks(t, dir) + + out := filepath.Join(dir, "out") + require.NoError(t, os.MkdirAll(out, 0o750)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "elsewhere"), 0o750)) + require.NoError(t, os.Symlink(filepath.Join(dir, "elsewhere"), filepath.Join(out, "models"))) + + app := newApp(t, genapp.WithOutputPath(out)) + + err := app.RenderFile("models/pet.go", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assertAbsent(t, filepath.Join(dir, "elsewhere", "pet.go"), + "os.MkdirAll would have walked through the link and written there") + }) + + t.Run("should break a hard link rather than write through it", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + out := filepath.Join(dir, "out") + require.NoError(t, os.MkdirAll(out, 0o750)) + + target := filepath.Join(out, "pet.go") + require.NoError(t, os.WriteFile(target, []byte("SHARED"), 0o600)) + + other := filepath.Join(out, "other.go") + if err := os.Link(target, other); err != nil { + t.Skip("this file system does not support hard links") + } + + app := newApp(t, genapp.WithOutputPath(out)) + require.NoError(t, app.RenderFile("pet.go", "model", pet)) + + kept, err := os.ReadFile(other) + require.NoError(t, err) + assert.Equal(t, "SHARED", string(kept), "the rename replaced a name, not the file behind it") + + written, err := os.ReadFile(target) + require.NoError(t, err) + assert.Contains(t, string(written), "package models") + }) + + t.Run("should refuse to overwrite a socket", func(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("unix domain sockets in the file system are not the same thing here") + } + + dir := t.TempDir() + + var config net.ListenConfig + + listener, err := config.Listen(t.Context(), "unix", filepath.Join(dir, "pet.go")) + require.NoError(t, err) + defer func() { _ = listener.Close() }() + + app := newApp(t, genapp.WithOutputPath(dir)) + + err = app.RenderFile("pet.go", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assert.Contains(t, err.Error(), "socket") + }) + + t.Run("should refuse to overwrite something that is not a regular file", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "pet.go"), 0o750)) + + app := newApp(t, genapp.WithOutputPath(dir)) + + err := app.RenderFile("pet.go", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assert.Contains(t, err.Error(), "directory") + }) +} + +func TestWithRoot(t *testing.T) { + t.Parallel() + + t.Run("should write under a root wider than the output path", func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + app := newApp(t, + genapp.WithRoot(root), + genapp.WithOutputPath(filepath.Join(root, "gen", "models")), + ) + + require.NoError(t, app.RenderFile("pet.go", "model", pet)) + assert.FileExists(t, filepath.Join(root, "gen", "models", "pet.go")) + }) + + t.Run("should refuse an output path outside the root", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + app := newApp(t, + genapp.WithRoot(filepath.Join(dir, "root")), + genapp.WithOutputPath(filepath.Join(dir, "elsewhere")), + ) + + err := app.RenderFile("pet.go", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assert.Contains(t, err.Error(), "outside the root") + }) + + t.Run("should refuse a root that does not exist", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + root := filepath.Join(dir, "absent") + app := newApp(t, + genapp.WithRoot(root), + genapp.WithOutputPath(filepath.Join(root, "gen")), + ) + + err := app.RenderFile("pet.go", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assert.Contains(t, err.Error(), "WithRoot", "the message points at the option that set it") + assertAbsent(t, root, "a mistyped root is reported, not created") + }) + + t.Run("should refuse a symbolic link that leaves the root", func(t *testing.T) { + t.Parallel() + + dir, _ := outside(t) + skipWithoutSymlinks(t, dir) + + root := filepath.Join(dir, "root") + require.NoError(t, os.MkdirAll(filepath.Join(root, "gen"), 0o750)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "elsewhere"), 0o750)) + require.NoError(t, os.Symlink(filepath.Join(dir, "elsewhere"), filepath.Join(root, "gen", "models"))) + + app := newApp(t, + genapp.WithRoot(root), + genapp.WithOutputPath(filepath.Join(root, "gen", "models")), + ) + + err := app.RenderFile("pet.go", "model", pet) + + require.ErrorIs(t, err, genapp.ErrGenApp) + assertAbsent(t, filepath.Join(dir, "elsewhere", "pet.go")) + }) +} + +func TestInitModuleIsConfined(t *testing.T) { + t.Parallel() + + t.Run("should report a live symbolic link at go.mod as existing", func(t *testing.T) { + t.Parallel() + + dir, secret := outside(t) + skipWithoutSymlinks(t, dir) + + out := filepath.Join(dir, "out") + require.NoError(t, os.MkdirAll(out, 0o750)) + require.NoError(t, os.Symlink(secret, filepath.Join(out, "go.mod"))) + + app := newApp(t, genapp.WithOutputPath(out)) + + err := app.InitModule(genapp.WithModulePath("example.com/gen")) + + require.ErrorIs(t, err, os.ErrExist) + assert.Contains(t, err.Error(), "WithReplaceExisting") + + kept, readErr := os.ReadFile(secret) + require.NoError(t, readErr) + assert.Equal(t, "SECRET", string(kept)) + }) + + t.Run("should report a dangling symbolic link at go.mod as existing", func(t *testing.T) { + t.Parallel() + + dir, _ := outside(t) + skipWithoutSymlinks(t, dir) + + out := filepath.Join(dir, "out") + require.NoError(t, os.MkdirAll(out, 0o750)) + require.NoError(t, os.Symlink(filepath.Join(dir, "absent"), filepath.Join(out, "go.mod"))) + + app := newApp(t, genapp.WithOutputPath(out)) + + err := app.InitModule(genapp.WithModulePath("example.com/gen")) + + require.ErrorIs(t, err, os.ErrExist, + "os.Stat follows the link and calls it absent, os.Root.Lstat reads the link itself") + }) + + t.Run("should scratch the link when asked to replace", func(t *testing.T) { + t.Parallel() + + dir, secret := outside(t) + skipWithoutSymlinks(t, dir) + + out := filepath.Join(dir, "out") + require.NoError(t, os.MkdirAll(out, 0o750)) + require.NoError(t, os.Symlink(secret, filepath.Join(out, "go.mod"))) + + app := newApp(t, genapp.WithOutputPath(out)) + + require.NoError(t, app.InitModule( + genapp.WithModulePath("example.com/gen"), + genapp.WithReplaceExisting(true), + )) + + kept, err := os.ReadFile(secret) + require.NoError(t, err) + assert.Equal(t, "SECRET", string(kept), "the link was removed, not written through") + + info, err := os.Lstat(filepath.Join(out, "go.mod")) + require.NoError(t, err) + assert.True(t, info.Mode().IsRegular()) + + written, err := os.ReadFile(filepath.Join(out, "go.mod")) + require.NoError(t, err) + assert.Contains(t, string(written), "module example.com/gen") + }) +} From 20454a1f73fa4546b8a28d512e286789c2277be6 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 24 Aug 2026 11:59:31 +0200 Subject: [PATCH 2/2] test(genapp): shorten the socket path to fit darwin's sun_path limit t.TempDir() names the directory after the test, so the socket bound by TestSymlinkIsNotFollowed/should_refuse_to_overwrite_a_socket ran to roughly 130 bytes on the macOS runner and bind failed with EINVAL. sun_path holds 104 bytes on darwin and 108 on linux. The fixture takes a short name from the same temp root instead, and skips when the path still exceeds 100 bytes. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- genapp/rooted_test.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/genapp/rooted_test.go b/genapp/rooted_test.go index ec13f79..b25a5fd 100644 --- a/genapp/rooted_test.go +++ b/genapp/rooted_test.go @@ -16,6 +16,12 @@ import ( "github.com/go-openapi/codegen/genapp" ) +// maxSocketPath bounds the path a unix domain socket can be bound to. +// +// sun_path holds 104 bytes on darwin and 108 on linux, and the address has to fit whole. 100 leaves +// room under the smaller of the two. +const maxSocketPath = 100 + // skipWithoutSymlinks skips a test on a runner that cannot create one. // // Windows grants the privilege to an administrator or to a machine in developer mode, and the CI @@ -198,11 +204,21 @@ func TestSymlinkIsNotFollowed(t *testing.T) { t.Skip("unix domain sockets in the file system are not the same thing here") } - dir := t.TempDir() + // t.TempDir() names the directory after the test, which puts the socket well past the 104 + // bytes darwin allows in sun_path. Take a short name from the same temp root instead. + //nolint:usetesting // t.TempDir() overflows sun_path here, see the comment above + dir, err := os.MkdirTemp("", "gs") + require.NoError(t, err) + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + + socket := filepath.Join(dir, "pet.go") + if len(socket) > maxSocketPath { + t.Skipf("%q is longer than the %d bytes sun_path holds here", socket, maxSocketPath) + } var config net.ListenConfig - listener, err := config.Listen(t.Context(), "unix", filepath.Join(dir, "pet.go")) + listener, err := config.Listen(t.Context(), "unix", socket) require.NoError(t, err) defer func() { _ = listener.Close() }()