Bug fix: Generic Worker: Process absolute paths in task payload corre…

…ctly

Generic Worker now evaluates absolute paths inside `mounts` (properties
`directory` and `file`) and artifacts (property `path`) correctly.  Previously
Generic Worker would effectively strip leading path separators and treat them
as relative paths inside the task directory. For example, `/tmp` would be
resolved as the relative path `tmp` from inside the task directory.  This has
now been fixed, so that both absolute and relative paths are evaluated
correctly.

changelog/issue-6689.md

@@ -0,0 +1,17 @@
+audience: users
+level: major
+reference: issue 6689
+---
+Breaking change: Generic Worker now evaluates absolute paths inside `mounts`
+(properties `directory` and `file`) and artifacts (property `path`) correctly.
+Previously Generic Worker would effectively strip leading path separators and
+treat them as relative paths inside the task directory. For example, `/tmp`
+would be resolved as the relative path `tmp` from inside the task directory.
+
+Although this is technically a bug fix, it does change the behaviour of Generic
+Worker when absolute paths are specified in task payloads. We have examined
+production tasks on both the Community and Firefox CI deployments of
+taskcluster, and are reasonably confident that this change should not have
+adverse affects on existing tasks. However we are bumping the major version
+number of the taskcluster release, in recognition of the backward
+incompatibility.

workers/generic-worker/artifacts.go

@@ -20,6 +20,7 @@ import (
 	tcclient "github.com/taskcluster/taskcluster/v57/clients/client-go"
 	"github.com/taskcluster/taskcluster/v57/clients/client-go/tcqueue"
 	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/artifacts"
+	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/fileutil"
 )
 
 var (
@@ -84,7 +85,7 @@ func (task *TaskRun) PayloadArtifacts() []artifacts.TaskArtifact {
 				// cause the task to fail, and the cause to be preserved in the
 				// error artifact.
 				case incomingErr != nil:
-					fullPath := filepath.Join(taskContext.TaskDir, subPath)
+					fullPath := fileutil.AbsFrom(taskContext.TaskDir, subPath)
 					payloadArtifacts = append(
 						payloadArtifacts,
 						&artifacts.ErrorArtifact{
@@ -105,7 +106,7 @@ func (task *TaskRun) PayloadArtifacts() []artifacts.TaskArtifact {
 			}
 			// Any error returned here should already have been handled by
 			// walkFn, so should be safe to ignore.
-			_ = filepath.Walk(filepath.Join(taskContext.TaskDir, basePath), walkFn)
+			_ = filepath.Walk(fileutil.AbsFrom(taskContext.TaskDir, basePath), walkFn)
 		}
 	}
 	return payloadArtifacts
@@ -121,7 +122,7 @@ func (task *TaskRun) PayloadArtifacts() []artifacts.TaskArtifact {
 // "invalid-resource-on-worker" ErrorArtifact
 // TODO: need to also handle "too-large-file-on-worker"
 func resolve(base *artifacts.BaseArtifact, artifactType string, path string, contentType string, contentEncoding string) artifacts.TaskArtifact {
-	fullPath := filepath.Join(taskContext.TaskDir, path)
+	fullPath := fileutil.AbsFrom(taskContext.TaskDir, path)
 	fileReader, err := os.Open(fullPath)
 	if err != nil {
 		// cannot read file/dir, create an error artifact

workers/generic-worker/artifacts_test.go

@@ -1032,3 +1032,32 @@ func TestObjectArtifact(t *testing.T) {
 	td := testTask(t)
 	_ = submitAndAssert(t, td, payload, "completed", "completed")
 }
+
+func TestFileArtifactWithAbsolutePath(t *testing.T) {
+
+	setup(t)
+	validateArtifacts(t,
+
+		// what appears in task payload
+		[]Artifact{
+			{
+				Expires: inAnHour,
+				Path:    filepath.Join(cwd, filepath.Join("testdata", "SampleArtifacts", "b", "c", "d.jpg")),
+				Type:    "file",
+				Name:    "public/build/firefox.exe",
+			},
+		},
+
+		// what we expect to discover on file system
+		[]artifacts.TaskArtifact{
+			&artifacts.S3Artifact{
+				BaseArtifact: &artifacts.BaseArtifact{
+					Name:    "public/build/firefox.exe",
+					Expires: inAnHour,
+				},
+				ContentType:     "image/jpeg",
+				ContentEncoding: "identity",
+				Path:            filepath.Join(cwd, filepath.Join("testdata", "SampleArtifacts", "b", "c", "d.jpg")),
+			},
+		})
+}

workers/generic-worker/chain_of_trust.go

@@ -127,15 +127,15 @@ func (feature *ChainOfTrustTaskFeature) Stop(err *ExecutionErrors) {
 	if feature.disabled {
 		return
 	}
-	logFile := filepath.Join(taskContext.TaskDir, logPath)
-	certifiedLogFile := filepath.Join(taskContext.TaskDir, certifiedLogPath)
-	unsignedCert := filepath.Join(taskContext.TaskDir, unsignedCertPath)
-	ed25519SignedCert := filepath.Join(taskContext.TaskDir, ed25519SignedCertPath)
+	logFile := fileutil.AbsFrom(taskContext.TaskDir, logPath)
+	certifiedLogFile := fileutil.AbsFrom(taskContext.TaskDir, certifiedLogPath)
+	unsignedCert := fileutil.AbsFrom(taskContext.TaskDir, unsignedCertPath)
+	ed25519SignedCert := fileutil.AbsFrom(taskContext.TaskDir, ed25519SignedCertPath)
 	copyErr := copyFileContents(logFile, certifiedLogFile)
 	if copyErr != nil {
 		panic(copyErr)
 	}
-	err.add(feature.task.uploadLog(certifiedLogName, filepath.Join(taskContext.TaskDir, certifiedLogPath)))
+	err.add(feature.task.uploadLog(certifiedLogName, fileutil.AbsFrom(taskContext.TaskDir, certifiedLogPath)))
 	artifactHashes := map[string]ArtifactHash{}
 	for _, artifact := range feature.task.Artifacts {
 		// make sure SHA256 is calculated
@@ -188,7 +188,7 @@ func (feature *ChainOfTrustTaskFeature) Stop(err *ExecutionErrors) {
 	if e != nil {
 		panic(e)
 	}
-	err.add(feature.task.uploadLog(unsignedCertName, filepath.Join(taskContext.TaskDir, unsignedCertPath)))
+	err.add(feature.task.uploadLog(unsignedCertName, fileutil.AbsFrom(taskContext.TaskDir, unsignedCertPath)))
 
 	// create detached ed25519 chain-of-trust.json.sig
 	sig := ed25519.Sign(feature.ed25519PrivKey, certBytes)
@@ -202,8 +202,8 @@ func (feature *ChainOfTrustTaskFeature) Stop(err *ExecutionErrors) {
 				Name:    ed25519SignedCertName,
 				Expires: feature.task.Definition.Expires,
 			},
-			filepath.Join(taskContext.TaskDir, ed25519SignedCertPath),
-			filepath.Join(taskContext.TaskDir, ed25519SignedCertPath),
+			fileutil.AbsFrom(taskContext.TaskDir, ed25519SignedCertPath),
+			fileutil.AbsFrom(taskContext.TaskDir, ed25519SignedCertPath),
 			"application/octet-stream",
 			"gzip",
 		),

workers/generic-worker/fileutil/filepath.go

@@ -0,0 +1,15 @@
+package fileutil
+
+import (
+	"path/filepath"
+)
+
+// AbsFrom returns path if an absolute path, otherwise the result of joining it
+// to the parent path. This mimics filepath.Abs(path) without restricting
+// parent to the current working directory.
+func AbsFrom(parent string, path string) string {
+	if filepath.IsAbs(path) {
+		return path
+	}
+	return filepath.Join(parent, path)
+}

workers/generic-worker/helper_test.go

@@ -193,7 +193,7 @@ func ensureDirContainsNFiles(t *testing.T, dir string, n int) {
 
 func LogText(t *testing.T) string {
 	t.Helper()
-	bytes, err := os.ReadFile(filepath.Join(taskContext.TaskDir, logPath))
+	bytes, err := os.ReadFile(fileutil.AbsFrom(taskContext.TaskDir, logPath))
 	if err != nil {
 		t.Fatalf("Error when trying to read log file: %v", err)
 	}

workers/generic-worker/main.go

@@ -890,7 +890,7 @@ func (task *TaskRun) kill() {
 }
 
 func (task *TaskRun) createLogFile() *os.File {
-	absLogFile := filepath.Join(taskContext.TaskDir, logPath)
+	absLogFile := fileutil.AbsFrom(taskContext.TaskDir, logPath)
 	logFileHandle, err := os.Create(absLogFile)
 	if err != nil {
 		panic(err)
@@ -947,10 +947,10 @@ func (task *TaskRun) Run() (err *ExecutionErrors) {
 		}
 		task.closeLog(logHandle)
 		if task.Payload.Features.BackingLog {
-			err.add(task.uploadLog(task.Payload.Logs.Backing, filepath.Join(taskContext.TaskDir, logPath)))
+			err.add(task.uploadLog(task.Payload.Logs.Backing, fileutil.AbsFrom(taskContext.TaskDir, logPath)))
 		}
 		if config.CleanUpTaskDirs {
-			_ = os.Remove(filepath.Join(taskContext.TaskDir, logPath))
+			_ = os.Remove(fileutil.AbsFrom(taskContext.TaskDir, logPath))
 		}
 	}()
 
@@ -1151,7 +1151,7 @@ func PrepareTaskEnvironment() (reboot bool) {
 	if PlatformTaskEnvironmentSetup(taskDirName) {
 		return true
 	}
-	logDir := filepath.Join(taskContext.TaskDir, filepath.Dir(logPath))
+	logDir := fileutil.AbsFrom(taskContext.TaskDir, filepath.Dir(logPath))
 	err := os.MkdirAll(logDir, 0700)
 	if err != nil {
 		panic(err)

workers/generic-worker/main_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/mcuadros/go-defaults"
 	"github.com/stretchr/testify/require"
 	"github.com/taskcluster/slugid-go/slugid"
+	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/fileutil"
 )
 
 // Test failure should resolve as "failed"
@@ -152,7 +153,7 @@ func TestExecutionErrorsText(t *testing.T) {
 func TestNonExecutableBinaryFailsTask(t *testing.T) {
 	setup(t)
 	commands := copyTestdataFile("ed25519_public_key")
-	commands = append(commands, singleCommandNoArgs(filepath.Join(taskContext.TaskDir, "ed25519_public_key"))...)
+	commands = append(commands, singleCommandNoArgs(fileutil.AbsFrom(taskContext.TaskDir, "ed25519_public_key"))...)
 	payload := GenericWorkerPayload{
 		Command:    commands,
 		MaxRunTime: 10,

workers/generic-worker/mounts.go

@@ -504,7 +504,7 @@ func (f *FileMount) FSContent() (FSContent, error) {
 }
 
 func (w *WritableDirectoryCache) Mount(taskMount *TaskMount) error {
-	target := filepath.Join(taskContext.TaskDir, w.Directory)
+	target := fileutil.AbsFrom(taskContext.TaskDir, w.Directory)
 	// cache already there?
 	if _, dirCacheExists := directoryCaches[w.CacheName]; dirCacheExists {
 		// bump counter
@@ -560,7 +560,7 @@ func (w *WritableDirectoryCache) Mount(taskMount *TaskMount) error {
 func (w *WritableDirectoryCache) Unmount(taskMount *TaskMount) error {
 	cache := directoryCaches[w.CacheName]
 	cacheDir := cache.Location
-	taskCacheDir := filepath.Join(taskContext.TaskDir, w.Directory)
+	taskCacheDir := fileutil.AbsFrom(taskContext.TaskDir, w.Directory)
 	taskMount.Infof("Preserving cache: Moving %q to %q", taskCacheDir, cacheDir)
 	err := RenameCrossDevice(taskCacheDir, cacheDir)
 	if err != nil {
@@ -612,7 +612,7 @@ func (r *ReadOnlyDirectory) Mount(taskMount *TaskMount) error {
 	if err != nil {
 		return fmt.Errorf("Not able to retrieve FSContent: %v", err)
 	}
-	dir := filepath.Join(taskContext.TaskDir, r.Directory)
+	dir := fileutil.AbsFrom(taskContext.TaskDir, r.Directory)
 	err = extract(c, r.Format, dir, taskMount)
 	if err != nil {
 		return err
@@ -631,7 +631,7 @@ func (f *FileMount) Mount(taskMount *TaskMount) error {
 		return err
 	}
 
-	file := filepath.Join(taskContext.TaskDir, f.File)
+	file := fileutil.AbsFrom(taskContext.TaskDir, f.File)
 	err = decompress(fsContent, f.Format, file, taskMount)
 	if err != nil {
 		return err

workers/generic-worker/mounts_test.go

@@ -671,7 +671,8 @@ func TestMounts(t *testing.T) {
 			}`),
 		},
 		&FileMount{
-			File: filepath.Join("preloaded", "shasums"),
+			// use absolute path of file
+			File: filepath.Join(taskContext.TaskDir, "preloaded", "shasums"),
 			Content: json.RawMessage(`{
 				"url": "https://raw.githubusercontent.com/taskcluster/testrepo/db12070fc7ea6e5d21797bf943c0b9466fb4d65e/generic-worker/shasums"
 			}`),
@@ -730,9 +731,9 @@ func TestMounts(t *testing.T) {
 			Format: "zip",
 		},
 
-		// read only directory from url
+		// read only directory from url with absolute path for directory
 		&ReadOnlyDirectory{
-			Directory: filepath.Join("my-task-caches", "package"),
+			Directory: filepath.Join(taskContext.TaskDir, "my-task-caches", "package"),
 			Content: json.RawMessage(`{
 				"url": "https://github.com/taskcluster/logserver/raw/53134a5b9cbece05752c0ecc1a6c6d7c2fbf6580/node_modules/express/node_modules/connect/node_modules/multiparty/test/fixture/file/binaryfile.tar.gz"
 			}`),

workers/generic-worker/multiuser_windows.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"syscall"
 
+	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/fileutil"
 	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/host"
 	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/process"
 	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/runtime"
@@ -59,7 +60,7 @@ func deleteDir(path string) error {
 
 func (task *TaskRun) generateCommand(index int) error {
 	commandName := fmt.Sprintf("command_%06d", index)
-	wrapper := filepath.Join(taskContext.TaskDir, commandName+"_wrapper.bat")
+	wrapper := fileutil.AbsFrom(taskContext.TaskDir, commandName+"_wrapper.bat")
 	log.Printf("Creating wrapper script: %v", wrapper)
 	command, err := process.NewCommand([]string{wrapper}, taskContext.TaskDir, nil, taskContext.pd)
 	if err != nil {
@@ -75,11 +76,11 @@ func (task *TaskRun) generateCommand(index int) error {
 func (task *TaskRun) prepareCommand(index int) *CommandExecutionError {
 	// In order that capturing of log files works, create a custom .bat file
 	// for the task which redirects output to a log file...
-	env := filepath.Join(taskContext.TaskDir, "env.txt")
-	dir := filepath.Join(taskContext.TaskDir, "dir.txt")
+	env := fileutil.AbsFrom(taskContext.TaskDir, "env.txt")
+	dir := fileutil.AbsFrom(taskContext.TaskDir, "dir.txt")
 	commandName := fmt.Sprintf("command_%06d", index)
-	wrapper := filepath.Join(taskContext.TaskDir, commandName+"_wrapper.bat")
-	script := filepath.Join(taskContext.TaskDir, commandName+".bat")
+	wrapper := fileutil.AbsFrom(taskContext.TaskDir, commandName+"_wrapper.bat")
+	script := fileutil.AbsFrom(taskContext.TaskDir, commandName+".bat")
 	contents := ":: This script runs command " + strconv.Itoa(index) + " defined in TaskId " + task.TaskID + "..." + "\r\n"
 	contents += "@echo off\r\n"
 

workers/generic-worker/rdp_windows.go

@@ -83,7 +83,7 @@ func (l *RDPTask) createRDPArtifact() {
 		Username: taskContext.User.Name,
 		Password: taskContext.User.Password,
 	}
-	rdpInfoFile := filepath.Join(taskContext.TaskDir, rdpInfoPath)
+	rdpInfoFile := fileutil.AbsFrom(taskContext.TaskDir, rdpInfoPath)
 	err := fileutil.WriteToFileAsJSON(l.info, rdpInfoFile)
 	// if we can't write this, something seriously wrong, so cause worker to
 	// report an internal-error to sentry and crash!
@@ -100,8 +100,8 @@ func (l *RDPTask) uploadRDPArtifact() *CommandExecutionError {
 				// RDP info expires one day after task
 				Expires: tcclient.Time(time.Now().Add(time.Hour * 24)),
 			},
-			filepath.Join(taskContext.TaskDir, rdpInfoPath),
-			filepath.Join(taskContext.TaskDir, rdpInfoPath),
+			fileutil.AbsFrom(taskContext.TaskDir, rdpInfoPath),
+			fileutil.AbsFrom(taskContext.TaskDir, rdpInfoPath),
 			"application/json",
 			"gzip",
 		),

workers/generic-worker/user_creation_multiuser_test.go

@@ -8,6 +8,8 @@ import (
 	"runtime"
 	"strings"
 	"testing"
+
+	"github.com/taskcluster/taskcluster/v57/workers/generic-worker/fileutil"
 )
 
 func TestRunAfterUserCreation(t *testing.T) {
@@ -28,7 +30,7 @@ func TestRunAfterUserCreation(t *testing.T) {
 			t.Fatalf("Problem deleting old tasks: %v", err)
 		}
 	}()
-	fileContents, err := os.ReadFile(filepath.Join(taskContext.TaskDir, "run-after-user.txt"))
+	fileContents, err := os.ReadFile(fileutil.AbsFrom(taskContext.TaskDir, "run-after-user.txt"))
 	if err != nil {
 		t.Fatalf("Got error when looking for file run-after-user.txt: %v", err)
 	}