plugins/bundle: Escape reserved chars used in persisted bundle directory name

In Windows there are some reserved characters that cannot be used in the names
of files and directories (eg. ?, *). If a bundle name contains these and if it's
configured to be persisted, the operation will fail on Windows. This change attempts
to fix this on Windows systems by escaping any encountered reserved characters before
using them in the bundle persistence path.

Fixes: #6915

Signed-off-by: Ashutosh Narkar <[email protected]>

Author: ashutosh-narkar

plugins/bundle/plugin.go

@@ -14,6 +14,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"runtime"
 	"strings"
 	"sync"
 	"time"
@@ -41,6 +42,8 @@ import (
 // successfully activate.
 const maxActivationRetry = 10
 
+var goos = runtime.GOOS
+
 // Loader defines the interface that the bundle plugin uses to control bundle
 // loading via HTTP, disk, etc.
 type Loader interface {
@@ -697,7 +700,9 @@ func (p *Plugin) configDelta(newConfig *Config) (map[string]*Source, map[string]
 
 func (p *Plugin) saveBundleToDisk(name string, raw io.Reader) error {
 
-	bundleDir := filepath.Join(p.bundlePersistPath, name)
+	bundleName := getNormalizedBundleName(name)
+
+	bundleDir := filepath.Join(p.bundlePersistPath, bundleName)
 	bundleFile := filepath.Join(bundleDir, "bundle.tar.gz")
 
 	tmpFile, saveErr := saveCurrentBundleToDisk(bundleDir, raw)
@@ -723,10 +728,12 @@ func saveCurrentBundleToDisk(path string, raw io.Reader) (string, error) {
 }
 
 func (p *Plugin) loadBundleFromDisk(path, name string, src *Source) (*bundle.Bundle, error) {
+	bundleName := getNormalizedBundleName(name)
+
 	if src != nil {
-		return bundleUtils.LoadBundleFromDiskForRegoVersion(p.manager.ParserOptions().RegoVersion, path, name, src.Signing)
+		return bundleUtils.LoadBundleFromDiskForRegoVersion(p.manager.ParserOptions().RegoVersion, path, bundleName, src.Signing)
 	}
-	return bundleUtils.LoadBundleFromDiskForRegoVersion(p.manager.ParserOptions().RegoVersion, path, name, nil)
+	return bundleUtils.LoadBundleFromDiskForRegoVersion(p.manager.ParserOptions().RegoVersion, path, bundleName, nil)
 }
 
 func (p *Plugin) log(name string) logging.Logger {
@@ -756,6 +763,33 @@ func (p *Plugin) getBundlesCpy() map[string]*Source {
 	return bundlesCpy
 }
 
+// getNormalizedBundleName returns a version of the input with
+// invalid file and directory name characters on Windows escaped.
+// It returns the input as-is for non-Windows systems.
+func getNormalizedBundleName(name string) string {
+	if goos != "windows" {
+		return name
+	}
+
+	sb := new(strings.Builder)
+	for i := 0; i < len(name); i++ {
+		if isReservedCharacter(rune(name[i])) {
+			sb.WriteString(fmt.Sprintf("\\%c", name[i]))
+		} else {
+			sb.WriteByte(name[i])
+		}
+	}
+
+	return sb.String()
+}
+
+// isReservedCharacter checks if the input is a reserved character on Windows that should not be
+// used in file and directory names
+// For details, see https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions.
+func isReservedCharacter(r rune) bool {
+	return r == '<' || r == '>' || r == ':' || r == '"' || r == '/' || r == '\\' || r == '|' || r == '?' || r == '*'
+}
+
 type fileLoader struct {
 	name             string
 	path             string

plugins/bundle/plugin_test.go

@@ -1911,6 +1911,85 @@ func TestLoadAndActivateBundlesFromDisk(t *testing.T) {
 	}
 }
 
+func TestLoadAndActivateBundlesFromDiskReservedChars(t *testing.T) {
+
+	ctx := context.Background()
+	manager := getTestManager()
+
+	dir := t.TempDir()
+
+	goos = "windows"
+
+	bundleName := "test?bundle=opa" // bundle name contains reserved characters
+	bundleSource := Source{
+		Persist: true,
+	}
+
+	bundles := map[string]*Source{}
+	bundles[bundleName] = &bundleSource
+
+	plugin := New(&Config{Bundles: bundles}, manager)
+	plugin.bundlePersistPath = filepath.Join(dir, ".opa")
+
+	plugin.loadAndActivateBundlesFromDisk(ctx)
+
+	// persist a bundle to disk and then load it
+	module := "package foo\n\ncorge=1"
+
+	b := bundle.Bundle{
+		Manifest: bundle.Manifest{Revision: "quickbrownfaux"},
+		Data:     util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}}`)).(map[string]interface{}),
+		Modules: []bundle.ModuleFile{
+			{
+				URL:    "/foo/bar.rego",
+				Path:   "/foo/bar.rego",
+				Parsed: ast.MustParseModule(module),
+				Raw:    []byte(module),
+			},
+		},
+	}
+
+	b.Manifest.Init()
+
+	var buf bytes.Buffer
+	if err := bundle.NewWriter(&buf).UseModulePath(true).Write(b); err != nil {
+		t.Fatal("unexpected error:", err)
+	}
+
+	err := plugin.saveBundleToDisk(bundleName, &buf)
+	if err != nil {
+		t.Fatalf("unexpected error %v", err)
+	}
+
+	plugin.loadAndActivateBundlesFromDisk(ctx)
+
+	txn := storage.NewTransactionOrDie(ctx, manager.Store)
+	defer manager.Store.Abort(ctx, txn)
+
+	ids, err := manager.Store.ListPolicies(ctx, txn)
+	if err != nil {
+		t.Fatal(err)
+	} else if len(ids) != 1 {
+		t.Fatal("Expected 1 policy")
+	}
+
+	bs, err := manager.Store.GetPolicy(ctx, txn, ids[0])
+	exp := []byte("package foo\n\ncorge=1")
+	if err != nil {
+		t.Fatal(err)
+	} else if !bytes.Equal(bs, exp) {
+		t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
+	}
+
+	data, err := manager.Store.Read(ctx, txn, storage.Path{})
+	expData := util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test?bundle=opa": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}}}`))
+	if err != nil {
+		t.Fatal(err)
+	} else if !reflect.DeepEqual(data, expData) {
+		t.Fatalf("Bad data content. Exp:\n%v\n\nGot:\n\n%v", expData, data)
+	}
+}
+
 func TestLoadAndActivateBundlesFromDiskV1Compatible(t *testing.T) {
 	type update struct {
 		modules map[string]string
@@ -6089,6 +6168,58 @@ func TestPluginManualTriggerWithTimeout(t *testing.T) {
 	}
 }
 
+func TestGetNormalizedBundleName(t *testing.T) {
+	cases := []struct {
+		input string
+		goos  string
+		exp   string
+	}{
+		{
+			input: "foo",
+			exp:   "foo",
+		},
+		{
+			input: "foo=bar",
+			exp:   "foo=bar",
+			goos:  "windows",
+		},
+		{
+			input: "c:/foo",
+			exp:   "c:/foo",
+		},
+		{
+			input: "c:/foo",
+			exp:   "c\\:\\/foo",
+			goos:  "windows",
+		},
+		{
+			input: "file:\"<>c:/a",
+			exp:   "file\\:\\\"\\<\\>c\\:\\/a",
+			goos:  "windows",
+		},
+		{
+			input: "|a?b*c",
+			exp:   "\\|a\\?b\\*c",
+			goos:  "windows",
+		},
+		{
+			input: "a?b=c",
+			exp:   "a\\?b=c",
+			goos:  "windows",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.input, func(t *testing.T) {
+			goos = tc.goos
+			actual := getNormalizedBundleName(tc.input)
+			if actual != tc.exp {
+				t.Fatalf("Want %v but got: %v", tc.exp, actual)
+			}
+		})
+	}
+}
+
 func writeTestBundleToDisk(t *testing.T, srcDir string, signed bool) bundle.Bundle {
 	t.Helper()