Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion loading/loading.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ func LoadStrategy(pth string, local, remote func(string) ([]byte, error), opts .

if isFSBacked {
// other fs.FS (e.g. os.DirFS) and os.Root loaders also use "/" on every platform.
// Escaping paths (absolute, "..", escaping symlinks) are rejected by the loader, not rewritten here.
// Path confinement is enforced by the loader, not here: the os.Root loader rebases
// absolute in-root paths and rejects escaping paths ("..", out-of-root absolute,
// escaping symlinks); an fs.FS loader rejects what its file system does not allow.
return local(filepath.ToSlash(cpth))
}

Expand Down
45 changes: 40 additions & 5 deletions loading/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io/fs"
"net/http"
"os"
"path/filepath"
"time"
)

Expand Down Expand Up @@ -39,13 +40,21 @@ func (fo fileOptions) ReadFileFunc() func(string) ([]byte, error) {
root := fo.root

return func(name string) ([]byte, error) {
// os.Root only accepts paths relative to the root, but callers (and this package's
// own file:// handling) routinely produce absolute paths. Rebase an absolute path
// onto the root before handing it to os.Root.
rel, err := rootRelative(root, name)
if err != nil {
return nil, errors.Join(err, ErrLoader)
}

r, err := os.OpenRoot(root)
if err != nil {
return nil, errors.Join(err, ErrLoader)
}
defer func() { _ = r.Close() }()

return r.ReadFile(name)
return r.ReadFile(rel)
}
}

Expand All @@ -56,6 +65,30 @@ func (fo fileOptions) ReadFileFunc() func(string) ([]byte, error) {
return fo.fs.ReadFile
}

// rootRelative expresses name as a path relative to root, so that it can be resolved by os.Root.
//
// A relative name is returned unchanged: os.Root confines it directly (including "../" traversal
// and symlink escapes, which it rejects at open time).
//
// An absolute name is rebased onto root. If it cannot be expressed relative to root — for
// example because it lives on a different volume on Windows — filepath.Rel returns an error,
// which is propagated so the read is rejected rather than silently escaping the root. An
// absolute path that lexically escapes root yields a "../" prefix here and is then rejected by
// os.Root.
func rootRelative(root, name string) (string, error) {
osName := filepath.FromSlash(name)
if !filepath.IsAbs(osName) {
return name, nil
}

absRoot, err := filepath.Abs(filepath.FromSlash(root))
if err != nil {
return "", err
}

return filepath.Rel(absRoot, osName)
}

// WithTimeout sets a timeout for the remote file loader.
//
// The default timeout is 30s.
Expand Down Expand Up @@ -123,10 +156,12 @@ func WithFS(filesystem fs.FS) Option {

// WithRoot confines local file loading to dir.
//
// Every requested path is resolved relative to dir, and any path that would escape dir —
// whether through an absolute path, ".." traversal, or a symlink pointing outside dir — is
// rejected. This is built on [os.Root] and is therefore resistant to the symlink escapes
// that a plain [os.DirFS] does not prevent.
// Every requested path is resolved within dir. A relative path is resolved against dir; an
// absolute path is rebased onto dir (so a caller that normalizes references to absolute paths,
// such as github.com/go-openapi/spec, still resolves correctly). Any path that would escape dir
// — through ".." traversal, an absolute path pointing outside dir, or a symlink pointing outside
// dir — is rejected. This is built on [os.Root] and is therefore resistant to the symlink
// escapes that a plain [os.DirFS] does not prevent.
//
// WithRoot is the recommended option when loading specs from a location derived from
// untrusted input. It applies to local loading only and has no effect on remote
Expand Down
50 changes: 50 additions & 0 deletions loading/withroot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/fs"
"os"
"path/filepath"
"strings"
"testing"
"testing/fstest"

Expand Down Expand Up @@ -53,6 +54,29 @@ func TestWithRoot(t *testing.T) {
})
})

t.Run("should load absolute paths confined to the root", func(t *testing.T) {
// Callers such as go-openapi/spec normalize $ref targets to absolute paths before
// loading. These must resolve when they point within the root (they are rebased onto it),
// not be rejected merely for being absolute.
abs := filepath.Join(root, "api.yaml")
for _, pth := range []string{
abs, // absolute path within the root
"file://" + filepath.ToSlash(abs), // absolute path within the root, via a file:// URI
} {
t.Run(pth, func(t *testing.T) {
b, err := LoadFromFileOrHTTP(pth, WithRoot(root))
require.NoError(t, err)
assert.EqualT(t, inside, string(b))
})
}

t.Run("nested absolute path", func(t *testing.T) {
b, err := LoadFromFileOrHTTP(filepath.Join(root, "sub", "api.yaml"), WithRoot(root))
require.NoError(t, err)
assert.EqualT(t, nested, string(b))
})
})

t.Run("should reject paths escaping the root", func(t *testing.T) {
for _, pth := range []string{
"file:///etc/passwd", // absolute via file:// URI
Expand Down Expand Up @@ -112,3 +136,29 @@ func TestWithRoot(t *testing.T) {
assert.EqualT(t, secret, string(b))
})
}

func TestRootRelative(t *testing.T) {
root := filepath.Join(t.TempDir(), "root")

t.Run("relative names are returned unchanged", func(t *testing.T) {
// os.Root confines relative names directly, so they are passed through verbatim.
for _, name := range []string{"api.yaml", "sub/api.yaml", "sub/../api.yaml", "../escape.yaml"} {
got, err := rootRelative(root, name)
require.NoError(t, err)
assert.EqualT(t, name, got)
}
})

t.Run("absolute in-root names are rebased onto the root", func(t *testing.T) {
got, err := rootRelative(root, filepath.Join(root, "sub", "api.yaml"))
require.NoError(t, err)
assert.EqualT(t, filepath.Join("sub", "api.yaml"), got)
})

t.Run("absolute escaping names yield a traversal path (rejected later by os.Root)", func(t *testing.T) {
got, err := rootRelative(root, filepath.Join(filepath.Dir(root), "secret.txt"))
require.NoError(t, err)
assert.TrueT(t, strings.HasPrefix(got, ".."+string(filepath.Separator)),
"expected a traversal path, got %q", got)
})
}
20 changes: 20 additions & 0 deletions loading/withroot_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
// SPDX-License-Identifier: Apache-2.0

//go:build windows

package loading

import (
"testing"

"github.com/go-openapi/testify/v2/require"
)

func TestRootRelativeVolumeMismatch(t *testing.T) {
// A path on a different volume cannot be expressed relative to the root. filepath.Rel
// returns an error, which rootRelative must propagate so the read is rejected rather than
// silently escaping the root.
_, err := rootRelative(`C:\root`, `D:\secret.txt`)
require.Error(t, err)
}
Loading