From f314c7cced2db23a4008622c97a27697c832c664 Mon Sep 17 00:00:00 2001
From: Gareth <garethgeorge97@gmail.com>
Date: Sun, 5 May 2024 07:59:53 -0700
Subject: [PATCH] feat: validate plan ID and repo ID

---
 internal/config/stringutil/stringutil.go | 12 ++++++++++--
 internal/config/validate.go              |  9 +++++++--
 internal/orchestrator/repo/repo.go       | 12 ++++--------
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/internal/config/stringutil/stringutil.go b/internal/config/stringutil/stringutil.go
index 3787961..1171259 100644
--- a/internal/config/stringutil/stringutil.go
+++ b/internal/config/stringutil/stringutil.go
@@ -2,7 +2,15 @@ package stringutil
 
 import "regexp"
 
+var (
+	sanitizeIDRegex = regexp.MustCompile(`[^a-zA-Z0-9_\-\.]+`) // matches invalid characters in an ID
+	idRegex         = regexp.MustCompile(`[a-zA-Z0-9_\-\.]*`)  // matches a valid ID (including empty string)
+)
+
 func SanitizeID(id string) string {
-	reg := regexp.MustCompile(`[^a-zA-Z0-9_\-\.]+`)
-	return reg.ReplaceAllString(id, "_")
+	return sanitizeIDRegex.ReplaceAllString(id, "_")
+}
+
+func ValidateID(id string) bool {
+	return idRegex.MatchString(id)
 }
diff --git a/internal/config/validate.go b/internal/config/validate.go
index f141f04..2065762 100644
--- a/internal/config/validate.go
+++ b/internal/config/validate.go
@@ -63,8 +63,9 @@ func ValidateConfig(c *v1.Config) error {
 
 func validateRepo(repo *v1.Repo) error {
 	var err error
-	if repo.Id == "" {
-		err = multierror.Append(err, errors.New("id is required"))
+
+	if repo.Id == "" || !stringutil.ValidateID(repo.Id) {
+		err = multierror.Append(err, fmt.Errorf("id %q contains invalid characters (or empty)", repo.Id))
 	}
 
 	if repo.Uri == "" {
@@ -88,6 +89,10 @@ func validatePlan(plan *v1.Plan, repos map[string]*v1.Repo) error {
 		err = multierror.Append(err, fmt.Errorf("path is required"))
 	}
 
+	if plan.Id == "" || !stringutil.ValidateID(plan.Id) {
+		err = multierror.Append(err, fmt.Errorf("id %q contains invalid characters (or empty)", plan.Id))
+	}
+
 	for idx, p := range plan.Paths {
 		if p == "" {
 			err = multierror.Append(err, fmt.Errorf("path[%d] cannot be empty", idx))
diff --git a/internal/orchestrator/repo/repo.go b/internal/orchestrator/repo/repo.go
index 6467fda..6ccc85c 100644
--- a/internal/orchestrator/repo/repo.go
+++ b/internal/orchestrator/repo/repo.go
@@ -35,8 +35,10 @@ func NewRepoOrchestrator(config *v1.Config, repoConfig *v1.Repo, resticPath stri
 	opts = append(opts, restic.WithEnviron())
 	opts = append(opts, restic.WithEnv("RESTIC_PROGRESS_FPS=0.5"))
 
-	if len(repoConfig.GetEnv()) > 0 {
-		opts = append(opts, restic.WithEnv(repoConfig.GetEnv()...))
+	if env := repoConfig.GetEnv(); len(env) != 0 {
+		for _, e := range env {
+			opts = append(opts, restic.WithEnv(ExpandEnv(e)))
+		}
 	}
 
 	if p := repoConfig.GetPassword(); p != "" {
@@ -58,12 +60,6 @@ func NewRepoOrchestrator(config *v1.Config, repoConfig *v1.Repo, resticPath stri
 		opts = append(opts, restic.WithFlags("-o", "sftp.args=-oBatchMode=yes"))
 	}
 
-	if env := repoConfig.GetEnv(); len(env) != 0 {
-		for _, e := range env {
-			opts = append(opts, restic.WithEnv(ExpandEnv(e)))
-		}
-	}
-
 	repo := restic.NewRepo(resticPath, repoConfig.GetUri(), opts...)
 
 	return &RepoOrchestrator{