Fix handling of symlinks in artifact recording

* Records source path instead of destination path for symlinks
* Adds explicit parameter to decide if symlinks to directories must be
  followed

Signed-off-by: Aditya Sirish <aditya@saky.in>

cmd/record.go

@@ -126,6 +126,15 @@ operating systems. It is done by replacing all line separators
 with a new line character.`,
 	)
 
+	recordCmd.PersistentFlags().BoolVar(
+		&followSymlinkDirs,
+		"follow-symlink-dirs",
+		false,
+		`Follow symlinked directories to their targets. Note: this parameter
+toggles following linked directories only, linked files are always
+recorded independently of this parameter.`,
+	)
+
 	recordCmd.MarkPersistentFlagRequired("name")
 
 	// Record Start Command
@@ -156,7 +165,7 @@ command is executed. Symlinks are followed.`,
 }
 
 func recordStart(cmd *cobra.Command, args []string) error {
-	block, err := intoto.InTotoRecordStart(recordStepName, recordMaterialsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization)
+	block, err := intoto.InTotoRecordStart(recordStepName, recordMaterialsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization, followSymlinkDirs)
 	if err != nil {
 		return fmt.Errorf("failed to create start link file: %w", err)
 	}
@@ -179,7 +188,7 @@ func recordStop(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to load start link file at %s: %w", prelimLinkName, err)
 	}
 
-	linkMb, err := intoto.InTotoRecordStop(prelimLinkMb, recordProductsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization)
+	linkMb, err := intoto.InTotoRecordStop(prelimLinkMb, recordProductsPaths, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization, followSymlinkDirs)
 	if err != nil {
 		return fmt.Errorf("failed to create stop link file: %w", err)
 	}

cmd/root.go

@@ -23,6 +23,7 @@ var (
 	exclude           []string
 	outDir            string
 	lineNormalization bool
+	followSymlinkDirs bool
 )
 
 var rootCmd = &cobra.Command{

cmd/run.go

@@ -140,6 +140,15 @@ with a new line character.`,
 		`Indicate that there is no command to be executed for the step.`,
 	)
 
+	runCmd.PersistentFlags().BoolVar(
+		&followSymlinkDirs,
+		"follow-symlink-dirs",
+		false,
+		`Follow symlinked directories to their targets. Note: this parameter
+toggles following linked directories only, linked files are always
+recorded independently of this parameter.`,
+	)
+
 	runCmd.Flags().StringVar(
 		&spiffeUDS,
 		"spiffe-workload-api-path",
@@ -158,7 +167,7 @@ func run(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("no command arguments passed, please specify or use --no-command option")
 	}
 
-	block, err := intoto.InTotoRun(stepName, runDir, materialsPaths, productsPaths, args, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization)
+	block, err := intoto.InTotoRun(stepName, runDir, materialsPaths, productsPaths, args, key, []string{"sha256"}, exclude, lStripPaths, lineNormalization, followSymlinkDirs)
 	if err != nil {
 		return fmt.Errorf("failed to create link metadata: %w", err)
 	}

doc/in-toto_record.md

@@ -18,6 +18,9 @@ failure and zero otherwise.
   -e, --exclude stringArray               Path patterns to match paths that should not be recorded as 
                                           ‘materials’ or ‘products’. Passed patterns override patterns defined
                                           in environment variables or config files. See Config docs for details.
+      --follow-symlink-dirs               Follow symlinked directories to their targets. Note: this parameter
+                                          toggles following linked directories only, linked files are always
+                                          recorded independently of this parameter.
   -h, --help                              help for record
   -k, --key string                        Path to a private key file to sign the resulting link metadata.
                                           The keyid prefix is used as an infix for the link metadata filename,

doc/in-toto_record_start.md

@@ -30,6 +30,9 @@ in-toto record start [flags]
   -e, --exclude stringArray               Path patterns to match paths that should not be recorded as 
                                           ‘materials’ or ‘products’. Passed patterns override patterns defined
                                           in environment variables or config files. See Config docs for details.
+      --follow-symlink-dirs               Follow symlinked directories to their targets. Note: this parameter
+                                          toggles following linked directories only, linked files are always
+                                          recorded independently of this parameter.
   -k, --key string                        Path to a private key file to sign the resulting link metadata.
                                           The keyid prefix is used as an infix for the link metadata filename,
                                           i.e. ‘<name>.<keyid prefix>.link’. See ‘–key-type’ for available

doc/in-toto_record_stop.md

@@ -31,6 +31,9 @@ in-toto record stop [flags]
   -e, --exclude stringArray               Path patterns to match paths that should not be recorded as 
                                           ‘materials’ or ‘products’. Passed patterns override patterns defined
                                           in environment variables or config files. See Config docs for details.
+      --follow-symlink-dirs               Follow symlinked directories to their targets. Note: this parameter
+                                          toggles following linked directories only, linked files are always
+                                          recorded independently of this parameter.
   -k, --key string                        Path to a private key file to sign the resulting link metadata.
                                           The keyid prefix is used as an infix for the link metadata filename,
                                           i.e. ‘<name>.<keyid prefix>.link’. See ‘–key-type’ for available

doc/in-toto_run.md

@@ -22,6 +22,9 @@ in-toto run [flags]
   -e, --exclude stringArray               Path patterns to match paths that should not be recorded as 0
                                           ‘materials’ or ‘products’. Passed patterns override patterns defined
                                           in environment variables or config files. See Config docs for details.
+      --follow-symlink-dirs               Follow symlinked directories to their targets. Note: this parameter
+                                          toggles following linked directories only, linked files are always
+                                          recorded independently of this parameter.
   -h, --help                              help for run
   -k, --key string                        Path to a PEM formatted private key file used to sign
                                           the resulting link metadata.

in_toto/runlib.go

@@ -92,10 +92,10 @@ the following format:
 If recording an artifact fails the first return value is nil and the second
 return value is the error.
 */
-func RecordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (evalArtifacts map[string]interface{}, err error) {
+func RecordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (evalArtifacts map[string]interface{}, err error) {
 	// Make sure to initialize a fresh hashset for every RecordArtifacts call
 	visitedSymlinks = NewSet()
-	evalArtifacts, err = recordArtifacts(paths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization)
+	evalArtifacts, err = recordArtifacts(paths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs)
 	// pass result and error through
 	return evalArtifacts, err
 }
@@ -118,7 +118,7 @@ the following format:
 If recording an artifact fails the first return value is nil and the second
 return value is the error.
 */
-func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (map[string]interface{}, error) {
+func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (map[string]interface{}, error) {
 	artifacts := make(map[string]interface{})
 	for _, path := range paths {
 		err := filepath.Walk(path,
@@ -160,18 +160,35 @@ func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns
 					if err != nil {
 						return err
 					}
+					info, err := os.Stat(evalSym)
+					if err != nil {
+						return err
+					}
+					targetIsDir := false
+					if info.IsDir() {
+						if !followSymlinkDirs {
+							// We don't follow symlinked directories
+							return nil
+						}
+						targetIsDir = true
+					}
 					// add symlink to visitedSymlinks set
 					// this way, we know which link we have visited already
 					// if we visit a symlink twice, we have detected a symlink cycle
 					visitedSymlinks.Add(path)
-					// We recursively call RecordArtifacts() to follow
+					// We recursively call recordArtifacts() to follow
 					// the new path.
-					evalArtifacts, evalErr := recordArtifacts([]string{evalSym}, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization)
+					evalArtifacts, evalErr := recordArtifacts([]string{evalSym}, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs)
 					if evalErr != nil {
 						return evalErr
 					}
 					for key, value := range evalArtifacts {
-						artifacts[key] = value
+						if targetIsDir {
+							symlinkPath := filepath.Join(path, strings.TrimPrefix(key, evalSym))
+							artifacts[symlinkPath] = value
+						} else {
+							artifacts[path] = value
+						}
 					}
 					return nil
 				}
@@ -189,8 +206,7 @@ func recordArtifacts(paths []string, hashAlgorithms []string, gitignorePatterns
 					}
 				}
 				// Check if path is unique
-				_, existingPath := artifacts[path]
-				if existingPath {
+				if _, exists := artifacts[path]; exists {
 					return fmt.Errorf("left stripping has resulted in non unique dictionary key: %s", path)
 				}
 				artifacts[path] = artifact
@@ -295,10 +311,10 @@ return value is an empty Metablock and the second return value is the error.
 */
 func InTotoRun(name string, runDir string, materialPaths []string, productPaths []string,
 	cmdArgs []string, key Key, hashAlgorithms []string, gitignorePatterns []string,
-	lStripPaths []string, lineNormalization bool) (Metablock, error) {
+	lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (Metablock, error) {
 	var linkMb Metablock
 
-	materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization)
+	materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs)
 	if err != nil {
 		return linkMb, err
 	}
@@ -312,7 +328,7 @@ func InTotoRun(name string, runDir string, materialPaths []string, productPaths
 		}
 	}
 
-	products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization)
+	products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs)
 	if err != nil {
 		return linkMb, err
 	}
@@ -347,9 +363,9 @@ in order to provide evidence for supply chain steps that cannot be carries out
 by a single command.  InTotoRecordStart collects the hashes of the materials
 before any commands are run, signs the unfinished link, and returns the link.
 */
-func InTotoRecordStart(name string, materialPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (Metablock, error) {
+func InTotoRecordStart(name string, materialPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (Metablock, error) {
 	var linkMb Metablock
-	materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization)
+	materials, err := RecordArtifacts(materialPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs)
 	if err != nil {
 		return linkMb, err
 	}
@@ -380,7 +396,7 @@ created by InTotoRecordStart and records the hashes of any products creted by
 commands run between InTotoRecordStart and InTotoRecordStop.  The resultant
 finished link metablock is then signed by the provided key and returned.
 */
-func InTotoRecordStop(prelimLinkMb Metablock, productPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool) (Metablock, error) {
+func InTotoRecordStop(prelimLinkMb Metablock, productPaths []string, key Key, hashAlgorithms, gitignorePatterns []string, lStripPaths []string, lineNormalization bool, followSymlinkDirs bool) (Metablock, error) {
 	var linkMb Metablock
 	if err := prelimLinkMb.VerifySignature(key); err != nil {
 		return linkMb, err
@@ -391,7 +407,7 @@ func InTotoRecordStop(prelimLinkMb Metablock, productPaths []string, key Key, ha
 		return linkMb, errors.New("invalid metadata block")
 	}
 
-	products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization)
+	products, err := RecordArtifacts(productPaths, hashAlgorithms, gitignorePatterns, lStripPaths, lineNormalization, followSymlinkDirs)
 	if err != nil {
 		return linkMb, err
 	}

in_toto/runlib_test.go

@@ -104,7 +104,7 @@ func TestGitPathSpec(t *testing.T) {
 		"*.pub",           // Match all .pub files (even the ones in subdirectories)
 		"beta/foo.tar.gz", // Match full path
 		"alpha/**",        // Match all directories and files beneath alpha
-	}, nil, testOSisWindows())
+	}, nil, testOSisWindows(), false)
 	if !reflect.DeepEqual(result, expected) {
 		t.Errorf("RecordArtifacts returned '(%s, %s)', expected '(%s, nil)'",
 			result, err, expected)
@@ -128,11 +128,11 @@ func TestSymlinkToFile(t *testing.T) {
 	}
 
 	expected := map[string]interface{}{
-		"foo.tar.gz": map[string]interface{}{
+		"foo.tar.gz.sym": map[string]interface{}{
 			"sha256": "52947cb78b91ad01fe81cd6aef42d1f6817e92b9e6936c1e5aabb7c98514f355",
 		},
 	}
-	result, err := RecordArtifacts([]string{"foo.tar.gz.sym"}, []string{"sha256"}, nil, nil, testOSisWindows())
+	result, err := RecordArtifacts([]string{"foo.tar.gz.sym"}, []string{"sha256"}, nil, nil, testOSisWindows(), false)
 	if !reflect.DeepEqual(result, expected) {
 		t.Errorf("RecordArtifacts returned '(%s, %s)', expected '(%s, nil)'",
 			result, err, expected)
@@ -171,7 +171,7 @@ func TestIndirectSymlinkCycles(t *testing.T) {
 	}
 
 	// provoke "symlink cycle detected" error
-	_, err = RecordArtifacts([]string{"symTestA/linkToB.sym", "symTestB/linkToA.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows())
+	_, err = RecordArtifacts([]string{"symTestA/linkToB.sym", "symTestB/linkToA.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows(), true)
 	if !errors.Is(err, ErrSymCycle) {
 		t.Errorf("we expected: %s, we got: %s", ErrSymCycle, err)
 	}
@@ -214,13 +214,13 @@ func TestSymlinkToFolder(t *testing.T) {
 		t.Errorf("could not write symTmpfile: %s", err)
 	}
 
-	result, err := RecordArtifacts([]string{"symTmpfile.sym"}, []string{"sha256"}, nil, nil, testOSisWindows())
+	result, err := RecordArtifacts([]string{"symTmpfile.sym"}, []string{"sha256"}, nil, nil, testOSisWindows(), true)
 	if err != nil {
 		t.Error(err)
 	}
 
 	expected := map[string]interface{}{
-		p: map[string]interface{}{
+		filepath.Join("symTmpfile.sym", "symTmpfile"): map[string]interface{}{
 			"sha256": "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad",
 		},
 	}
@@ -266,7 +266,7 @@ func TestSymlinkCycle(t *testing.T) {
 	}
 
 	// provoke "symlink cycle detected" error
-	_, err = RecordArtifacts([]string{"symlinkCycle/symCycle.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows())
+	_, err = RecordArtifacts([]string{"symlinkCycle/symCycle.sym", "foo.tar.gz"}, []string{"sha256"}, nil, nil, testOSisWindows(), true)
 	if !errors.Is(err, ErrSymCycle) {
 		t.Errorf("we expected: %s, we got: %s", ErrSymCycle, err)
 	}
@@ -289,7 +289,7 @@ func TestRecordArtifacts(t *testing.T) {
 		t.Errorf("could not write tmpfile: %s", err)
 	}
 	result, err := RecordArtifacts([]string{"foo.tar.gz",
-		"tmpdir/tmpfile"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows())
+		"tmpdir/tmpfile"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows(), false)
 	expected := map[string]interface{}{
 		"foo.tar.gz": map[string]interface{}{
 			"sha256": "52947cb78b91ad01fe81cd6aef42d1f6817e92b9e6936c1e5aabb7c98514f355",
@@ -307,7 +307,7 @@ func TestRecordArtifacts(t *testing.T) {
 		t.Errorf("could not write tmpfile: %s", err)
 	}
 	_, err = RecordArtifacts([]string{"foo.tar.gz",
-		"tmpdir/foo.tar.gz"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows())
+		"tmpdir/foo.tar.gz"}, []string{"sha256"}, nil, []string{"tmpdir/"}, testOSisWindows(), false)
 	if err == nil {
 		t.Error("duplicated path error expected")
 	}
@@ -317,7 +317,7 @@ func TestRecordArtifacts(t *testing.T) {
 	}
 
 	// Test error by recording nonexistent artifact
-	result, err = RecordArtifacts([]string{"file-does-not-exist"}, []string{"sha256"}, nil, nil, testOSisWindows())
+	result, err = RecordArtifacts([]string{"file-does-not-exist"}, []string{"sha256"}, nil, nil, testOSisWindows(), false)
 	if !os.IsNotExist(err) {
 		t.Errorf("RecordArtifacts returned '(%s, %s)', expected '(nil, %s)'",
 			result, err, os.ErrNotExist)
@@ -464,7 +464,7 @@ func TestInTotoRun(t *testing.T) {
 	}
 
 	for _, table := range tablesCorrect {
-		result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows())
+		result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false)
 		if !reflect.DeepEqual(result, table.result) {
 			t.Errorf("InTotoRun returned '(%s, %s)', expected '(%s, nil)'", result, err, table.result)
 		} else {
@@ -508,7 +508,7 @@ func TestInTotoRun(t *testing.T) {
 	}
 
 	for _, table := range tablesInvalid {
-		result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows())
+		result, err := InTotoRun(linkName, "", table.materialPaths, table.productPaths, table.cmdArgs, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false)
 		if err == nil {
 			t.Errorf("InTotoRun returned '(%s, %s)', expected error",
 				result, err)
@@ -578,10 +578,10 @@ func TestInTotoRecord(t *testing.T) {
 	}
 
 	for _, table := range tablesCorrect {
-		result, err := InTotoRecordStart(linkName, table.materialPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows())
+		result, err := InTotoRecordStart(linkName, table.materialPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false)
 		assert.Nil(t, err, "unexpected error while running record start")
 		assert.Equal(t, table.startResult, result, "result from record start did not match expected result")
-		stopResult, err := InTotoRecordStop(result, table.productPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows())
+		stopResult, err := InTotoRecordStop(result, table.productPaths, table.key, table.hashAlgorithms, nil, nil, testOSisWindows(), false)
 		assert.Nil(t, err, "unexpected error while running record stop")
 		assert.Equal(t, table.stopResult, stopResult, "result from record stop did not match expected result")
 	}

in_toto/verifylib.go

@@ -52,7 +52,7 @@ func RunInspections(layout Layout, runDir string, lineNormalization bool) (map[s
 		}
 
 		linkMb, err := InTotoRun(inspection.Name, runDir, paths, paths,
-			inspection.Run, Key{}, []string{"sha256"}, nil, nil, lineNormalization)
+			inspection.Run, Key{}, []string{"sha256"}, nil, nil, lineNormalization, false)
 
 		if err != nil {
 			return nil, err