more refactoring of the TaskEnv.ClientPath abstraction

client/allocrunner/taskrunner/getter/getter.go

@@ -31,7 +31,6 @@ const (
 // is usually satisfied by taskenv.TaskEnv.
 type EnvReplacer interface {
 	ReplaceEnv(string) string
-	ReplaceEnvClient(string) string
 	ClientPath(string) (string, bool)
 }
 

client/allocrunner/taskrunner/getter/getter_test.go

@@ -26,28 +26,23 @@ type noopReplacer struct {
 	taskDir string
 }
 
-func (noopReplacer) ReplaceEnv(s string) string {
-	return s
-}
-
-func (r noopReplacer) ReplaceEnvClient(s string) string {
-	return r.ReplaceEnv(s)
+func clientPath(taskDir, path string) (string, bool) {
+	if !filepath.IsAbs(path) {
+		path = filepath.Join(taskDir, path)
+	}
+	path = filepath.Clean(path)
+	if taskDir != "" && !helper.PathEscapesSandbox(taskDir, path) {
+		return path, false
+	}
+	return path, true
 }
 
-func checkEscape(taskDir, testPath string) bool {
-	if taskDir != "" && !helper.PathEscapesSandbox(taskDir, testPath) {
-		return false
-	}
-	return true
+func (noopReplacer) ReplaceEnv(s string) string {
+	return s
 }
 
 func (r noopReplacer) ClientPath(p string) (string, bool) {
-	path := r.ReplaceEnv(p)
-	if !filepath.IsAbs(path) {
-		path = filepath.Join(r.taskDir, path)
-	}
-	path = filepath.Clean(path)
-	escapes := checkEscape(r.taskDir, path)
+	path, escapes := clientPath(r.taskDir, r.ReplaceEnv(p))
 	return path, escapes
 }
 
@@ -67,16 +62,8 @@ func (upperReplacer) ReplaceEnv(s string) string {
 	return strings.ToUpper(s)
 }
 
-func (u upperReplacer) ReplaceEnvClient(s string) string {
-	return u.ReplaceEnv(s)
-}
-
 func (u upperReplacer) ClientPath(p string) (string, bool) {
-	path := u.ReplaceEnv(p)
-	if !filepath.IsAbs(path) {
-		path = filepath.Join(u.taskDir, path)
-	}
-	escapes := checkEscape(u.taskDir, path)
+	path, escapes := clientPath(u.taskDir, u.ReplaceEnv(p))
 	return path, escapes
 }
 

client/allocrunner/taskrunner/template/template.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"math/rand"
 	"os"
-	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
@@ -725,10 +724,8 @@ func loadTemplateEnv(tmpls []*structs.Template, taskDir string, taskEnv *taskenv
 			continue
 		}
 
-		dest := taskEnv.ReplaceEnvClient(t.DestPath)
-		if !filepath.IsAbs(dest) {
-			dest = filepath.Join(taskDir, dest)
-		}
+		// we checked escape before we rendered the file
+		dest, _ := taskEnv.ClientPath(t.DestPath)
 		f, err := os.Open(dest)
 		if err != nil {
 			return nil, fmt.Errorf("error opening env template: %v", err)

client/taskenv/env.go

@@ -306,16 +306,18 @@ func (t *TaskEnv) ReplaceEnv(arg string) string {
 	return hargs.ReplaceEnv(arg, t.EnvMap, t.NodeAttrs)
 }
 
-// ReplaceEnvClient takes an arg and replaces all occurrences of client-specific
+// replaceEnvClient takes an arg and replaces all occurrences of client-specific
 // environment variables and Nomad variables.  If the variable is found in the
 // passed map it is replaced, otherwise the original string is returned.
 // The difference from ReplaceEnv client is potentially difference values for
 // the following variables:
 // * NOMAD_ALLOC_DIR
 // * NOMAD_TASK_DIR
 // * NOMAD_SECRETS_DIR
+// and anything that was interpolated using them.
+//
 // See https://github.com/hashicorp/nomad/pull/9668
-func (t *TaskEnv) ReplaceEnvClient(arg string) string {
+func (t *TaskEnv) replaceEnvClient(arg string) string {
 	return hargs.ReplaceEnv(arg, t.EnvMapClient, t.NodeAttrs)
 }
 
@@ -332,7 +334,7 @@ func (t *TaskEnv) checkEscape(testPath string) bool {
 }
 
 func (t *TaskEnv) ClientPath(rawPath string) (string, bool) {
-	path := t.ReplaceEnvClient(rawPath)
+	path := t.replaceEnvClient(rawPath)
 	if !filepath.IsAbs(path) {
 		path = filepath.Join(t.clientTaskDir, path)
 	}