feat(loading): sandbox local loading with WithRoot (GHSA-v2xp-g8xf-22pf)

loading/doc.go

@@ -2,4 +2,24 @@
 // SPDX-License-Identifier: Apache-2.0
 
 // Package loading provides tools to load a file from http or from a local file system.
+//
+// # Security
+//
+// By default, the local loader reads any path the process can access, including absolute
+// paths and "file://" URIs (for example "file:///etc/passwd"). Applications that pass
+// untrusted input to [LoadFromFileOrHTTP], [JSONDoc] (or to downstream consumers such as
+// go-openapi/loads) must confine local loading to a trusted directory.
+//
+// Use [WithRoot] to do so: it resolves every requested path relative to a chosen directory
+// and rejects anything that escapes it, including via symlink. It is built on [os.Root]
+// and is therefore safer than passing an [os.DirFS] to [WithFS], which does not block
+// symlink escapes.
+//
+// Remote loading uses a standard [net/http] client. By default it follows redirects and
+// performs no destination filtering — exactly like [net/http.DefaultClient]. A
+// caller-controlled URL may therefore reach internal services or cloud metadata endpoints
+// (server-side request forgery). This package does not, and should not, embed a network
+// policy: when the URL may derive from untrusted input, supply a restricted client with
+// [WithHTTPClient] whose transport rejects unwanted destinations at dial time — which also
+// covers redirects and DNS rebinding. See the example on [LoadFromFileOrHTTP].
 package loading

loading/example_test.go

@@ -0,0 +1,64 @@
+// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
+// SPDX-License-Identifier: Apache-2.0
+
+package loading_test
+
+import (
+	"errors"
+	"fmt"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"syscall"
+
+	"github.com/go-openapi/swag/loading"
+)
+
+// errForbiddenAddr is returned by the dial guard when a destination is not allowed.
+var errForbiddenAddr = errors.New("blocked dial to a forbidden address")
+
+// ExampleLoadFromFileOrHTTP_restrictNetwork shows how to confine remote spec loading so a
+// caller-controlled URL cannot reach loopback, private, or link-local (cloud metadata)
+// addresses.
+//
+// The [net.Dialer] Control hook runs after DNS resolution and before connect, on every
+// connection, so the check also covers HTTP redirects and DNS rebinding — neither of which
+// a URL-string allowlist can defend against. Here a loopback test server stands in for an
+// internal endpoint that the guard must refuse to reach.
+func ExampleLoadFromFileOrHTTP_restrictNetwork() {
+	control := func(_, address string, _ syscall.RawConn) error {
+		host, _, err := net.SplitHostPort(address)
+		if err != nil {
+			return err
+		}
+		addr, err := netip.ParseAddr(host)
+		if err != nil {
+			return err
+		}
+		if a := addr.Unmap(); a.IsLoopback() || a.IsPrivate() || a.IsLinkLocalUnicast() || a.IsUnspecified() {
+			return errForbiddenAddr
+		}
+
+		return nil
+	}
+
+	client := &http.Client{
+		Transport: &http.Transport{
+			DialContext: (&net.Dialer{Control: control}).DialContext,
+		},
+	}
+
+	// An internal service the application must not let untrusted input reach.
+	internal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte("internal secret"))
+	}))
+	defer internal.Close()
+
+	// internal.URL is a loopback address (the untrusted URL in a real attack).
+	_, err := loading.LoadFromFileOrHTTP(internal.URL, loading.WithHTTPClient(client))
+	fmt.Println("blocked:", errors.Is(err, errForbiddenAddr))
+
+	// Output:
+	// blocked: true
+}

loading/loading.go

@@ -17,7 +17,11 @@ import (
 	"strings"
 )
 
-// LoadFromFileOrHTTP loads the bytes from a file or a remote http server based on the path passed in
+// LoadFromFileOrHTTP loads the bytes from a file or a remote http server based on the path passed in.
+//
+// Security: by default a local path is read with no confinement, so a caller-controlled path
+// (including a "file://" URI or an absolute path) may read any file the process can access.
+// When the path may derive from untrusted input, confine local loading with [WithRoot].
 func LoadFromFileOrHTTP(pth string, opts ...Option) ([]byte, error) {
 	o := optionsWithDefaults(opts)
 	return LoadStrategy(pth, o.ReadFileFunc(), loadHTTPBytes(opts...), opts...)(pth)
@@ -54,11 +58,14 @@ func LoadFromFileOrHTTP(pth string, opts ...Option) ([]byte, error) {
 // - `file:///c:/folder/file` becomes `C:\folder\file`
 // - `file://c:/folder/file` is tolerated (without leading `/`) and becomes `c:\folder\file`
 func LoadStrategy(pth string, local, remote func(string) ([]byte, error), opts ...Option) func(string) ([]byte, error) {
-	if strings.HasPrefix(pth, "http") {
+	if hasHTTPScheme(pth) {
 		return remote
 	}
 	o := optionsWithDefaults(opts)
 	_, isEmbedFS := o.fs.(embed.FS)
+	// any loader backed by an fs.FS or an os.Root consumes forward-slash paths on every
+	// platform, so it must not go through the windows-native file:// preprocessing below.
+	isFSBacked := o.fs != nil || o.root != ""
 
 	return func(p string) ([]byte, error) {
 		upth, err := url.PathUnescape(p)
@@ -67,14 +74,20 @@ func LoadStrategy(pth string, local, remote func(string) ([]byte, error), opts .
 		}
 
 		cpth, hasPrefix := strings.CutPrefix(upth, "file://")
-		if !hasPrefix || isEmbedFS || runtime.GOOS != "windows" {
+		if !hasPrefix || isFSBacked || runtime.GOOS != "windows" {
 			// crude processing: trim the file:// prefix. This leaves full URIs with a host with a (mostly) unexpected result
 			// regular file path provided: just normalize slashes
 			if isEmbedFS {
-				// on windows, we need to slash the path if FS is an embed FS.
+				// embed.FS always uses "/" as separator, even on windows, and rejects leading "./" or "/".
 				return local(strings.TrimLeft(filepath.ToSlash(cpth), "./")) // remove invalid leading characters for embed FS
 			}
 
+			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.
+				return local(filepath.ToSlash(cpth))
+			}
+
 			return local(filepath.FromSlash(cpth))
 		}
 
@@ -113,6 +126,21 @@ func LoadStrategy(pth string, local, remote func(string) ([]byte, error), opts .
 	}
 }
 
+// hasHTTPScheme reports whether pth is an absolute URL with an http or https scheme,
+// selecting the remote loader. The comparison is case-insensitive, as URL schemes are.
+//
+// Requiring the "://" separator (rather than a bare "http" prefix) avoids misrouting a
+// local file whose name merely starts with "http" (e.g. "httpbin.json") to the remote loader.
+func hasHTTPScheme(pth string) bool {
+	for _, scheme := range [...]string{"http://", "https://"} {
+		if len(pth) >= len(scheme) && strings.EqualFold(pth[:len(scheme)], scheme) {
+			return true
+		}
+	}
+
+	return false
+}
+
 func loadHTTPBytes(opts ...Option) func(path string) ([]byte, error) {
 	o := optionsWithDefaults(opts)
 

loading/loading_test.go

@@ -221,6 +221,28 @@ func TestLoadStrategy(t *testing.T) {
 		b, _ := ldr("")
 		assert.EqualT(t, thisIsNotIt, string(b))
 	})
+
+	t.Run("should serve remote strategy with an uppercase scheme", func(t *testing.T) {
+		// URL schemes are case-insensitive
+		for _, pth := range []string{"HTTP://blah", "HTTPS://blah", "HtTp://blah"} {
+			t.Run(pth, func(t *testing.T) {
+				ldr := LoadStrategy(pth, loader, remLoader)
+				b, _ := ldr("")
+				assert.EqualT(t, thisIsNotIt, string(b))
+			})
+		}
+	})
+
+	t.Run("should serve local strategy for a local file named like an http URL", func(t *testing.T) {
+		// a bare "http" prefix must not misroute a local file to the remote loader
+		for _, pth := range []string{"httpbin.json", "http2-spec.yaml", "http"} {
+			t.Run(pth, func(t *testing.T) {
+				ldr := LoadStrategy(pth, loader, remLoader)
+				b, _ := ldr("")
+				assert.YAMLEqT(t, string(yamlPetStore), string(b))
+			})
+		}
+	})
 }
 
 func TestLoadStrategyFile(t *testing.T) {

loading/options.go

@@ -4,6 +4,7 @@
 package loading
 
 import (
+	"errors"
 	"io/fs"
 	"net/http"
 	"os"
@@ -23,7 +24,8 @@ type (
 	}
 
 	fileOptions struct {
-		fs fs.ReadFileFS
+		fs   fs.ReadFileFS
+		root string // when non-empty, local reads are confined to this directory via os.Root
 	}
 
 	options struct {
@@ -33,6 +35,20 @@ type (
 )
 
 func (fo fileOptions) ReadFileFunc() func(string) ([]byte, error) {
+	if fo.root != "" {
+		root := fo.root
+
+		return func(name string) ([]byte, error) {
+			r, err := os.OpenRoot(root)
+			if err != nil {
+				return nil, errors.Join(err, ErrLoader)
+			}
+			defer func() { _ = r.Close() }()
+
+			return r.ReadFile(name)
+		}
+	}
+
 	if fo.fs == nil {
 		return os.ReadFile
 	}
@@ -87,8 +103,15 @@ func WithHTTPClient(client *http.Client) Option {
 // By default, the file system is the one provided by the os package.
 //
 // For example, this may be set to consume from an embedded file system, or a rooted FS.
+//
+// WithFS and [WithRoot] are mutually exclusive: the last one applied wins.
+//
+// Security note: a file system built from [os.DirFS] confines paths but does NOT protect
+// against symlinks that escape the root. To load from a directory derived from untrusted
+// input, prefer [WithRoot], which is symlink-escape resistant.
 func WithFS(filesystem fs.FS) Option {
 	return func(o *options) {
+		o.root = "" // last-wins vs WithRoot
 		if rfs, ok := filesystem.(fs.ReadFileFS); ok {
 			o.fs = rfs
 
@@ -98,6 +121,28 @@ 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.
+//
+// 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
+// (http/https) loading. WithRoot and [WithFS] are mutually exclusive: the last one applied
+// wins.
+//
+// Note: [os.Root] confines path resolution but does not, by itself, protect against
+// traversal of mount/bind boundaries, /proc special files, or device files. Point WithRoot
+// at a directory that holds only the documents you intend to expose.
+func WithRoot(dir string) Option {
+	return func(o *options) {
+		o.root = dir
+		o.fs = nil // last-wins vs WithFS
+	}
+}
+
 type readFileFS struct {
 	fs.FS
 }

loading/withroot_test.go

@@ -0,0 +1,114 @@
+// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
+// SPDX-License-Identifier: Apache-2.0
+
+package loading
+
+import (
+	"io/fs"
+	"os"
+	"path/filepath"
+	"testing"
+	"testing/fstest"
+
+	"github.com/go-openapi/testify/v2/assert"
+	"github.com/go-openapi/testify/v2/require"
+)
+
+func TestWithRoot(t *testing.T) {
+	const (
+		inside = "inside the root"
+		nested = "nested in the root"
+		secret = "this is a secret outside the root"
+	)
+
+	// layout:
+	//   <parent>/secret.txt        (outside the root)
+	//   <parent>/root/api.yaml
+	//   <parent>/root/sub/api.yaml
+	parent := t.TempDir()
+	root := filepath.Join(parent, "root")
+	require.NoError(t, os.MkdirAll(filepath.Join(root, "sub"), 0o750))
+	require.NoError(t, os.WriteFile(filepath.Join(parent, "secret.txt"), []byte(secret), 0o600))
+	require.NoError(t, os.WriteFile(filepath.Join(root, "api.yaml"), []byte(inside), 0o600))
+	require.NoError(t, os.WriteFile(filepath.Join(root, "sub", "api.yaml"), []byte(nested), 0o600))
+
+	t.Run("should load paths confined to the root", func(t *testing.T) {
+		for _, pth := range []string{
+			"api.yaml",
+			"./api.yaml",
+			"file://api.yaml",
+			"sub/../api.yaml",
+		} {
+			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 path", func(t *testing.T) {
+			b, err := LoadFromFileOrHTTP("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
+			filepath.Join(parent, "secret.txt"), // absolute path to an existing sibling file
+			"../secret.txt",                     // traversal
+			"file://../secret.txt",              // traversal via file:// URI
+		} {
+			t.Run(pth, func(t *testing.T) {
+				b, err := LoadFromFileOrHTTP(pth, WithRoot(root))
+				require.Error(t, err)
+				// the rejected read must not leak any bytes
+				assert.Empty(t, b)
+			})
+		}
+	})
+
+	t.Run("should reject a symlink escaping the root", func(t *testing.T) {
+		// <root>/escape.yaml -> <parent>/secret.txt  (escapes the root)
+		if err := os.Symlink(filepath.Join(parent, "secret.txt"), filepath.Join(root, "escape.yaml")); err != nil {
+			t.Skipf("symlinks not supported on this platform/filesystem: %v", err)
+		}
+
+		b, err := LoadFromFileOrHTTP("escape.yaml", WithRoot(root))
+		require.Error(t, err)
+		assert.Empty(t, b)
+		// this is exactly the case os.DirFS would NOT block
+		assert.NotContains(t, string(b), secret)
+	})
+
+	t.Run("should surface an error for a missing root", func(t *testing.T) {
+		b, err := LoadFromFileOrHTTP("api.yaml", WithRoot(filepath.Join(parent, "does-not-exist")))
+		require.Error(t, err)
+		require.ErrorIs(t, err, ErrLoader)
+		assert.Empty(t, b)
+	})
+
+	t.Run("WithRoot and WithFS are mutually exclusive (last wins)", func(t *testing.T) {
+		mapfs := fstest.MapFS{"api.yaml": &fstest.MapFile{Data: []byte("from map fs"), Mode: fs.ModePerm}}
+
+		t.Run("WithRoot after WithFS uses the root", func(t *testing.T) {
+			b, err := LoadFromFileOrHTTP("api.yaml", WithFS(mapfs), WithRoot(root))
+			require.NoError(t, err)
+			assert.EqualT(t, inside, string(b))
+		})
+
+		t.Run("WithFS after WithRoot uses the fs", func(t *testing.T) {
+			b, err := LoadFromFileOrHTTP("api.yaml", WithRoot(root), WithFS(mapfs))
+			require.NoError(t, err)
+			assert.EqualT(t, "from map fs", string(b))
+		})
+	})
+
+	t.Run("default loader (no WithRoot) is unchanged and reads outside any root", func(t *testing.T) {
+		// regression guard: the fix is opt-in and must not change default behavior
+		b, err := LoadFromFileOrHTTP(filepath.Join(parent, "secret.txt"))
+		require.NoError(t, err)
+		assert.EqualT(t, secret, string(b))
+	})
+}