Merge pull request #676 from ajeddeloh/fix-symlinks-again

Fix symlinks again
coreos · Dec 6, 2018 · be256c1 · be256c1
2 parents 2a88cd9 + 91bab0c
commit be256c1
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 3 deletions.
diff --git a/internal/exec/util/file.go b/internal/exec/util/file.go
@@ -153,7 +153,7 @@ func (u Util) PerformFetch(f *FetchOp) error {
 		// guarantees here. If the user explicitly doesn't want us to overwrite
 		// preexisting nodes, check the target path and fail if something's
 		// there.
-		_, err := os.Stat(path)
+		_, err := os.Lstat(path)
 		switch {
 		case os.IsNotExist(err):
 			break
@@ -282,7 +282,7 @@ func (u Util) PathExists(path string) (bool, error) {
 		return false, err
 	}
 
-	_, err = os.Stat(path)
+	_, err = os.Lstat(path)
 	switch {
 	case os.IsNotExist(err):
 		return false, nil

diff --git a/internal/exec/util/util.go b/internal/exec/util/util.go
@@ -56,11 +56,14 @@ func wantsToEscape(p string) bool {
 // u.DestDir. This means that the resulting path will always be under
 // u.DestDir. If u.IsRoot is false, it fails if a symlink resolves such
 // that it would escape u.DestDir.
+// The last element of the path is never followed.
 func (u Util) JoinPath(path ...string) (string, error) {
 	components := []string{}
 	for _, tmp := range path {
 		components = append(components, splitPath(tmp)...)
 	}
+	last := components[len(components)-1]
+	components = components[:len(components)-1]
 
 	realpath := "/"
 	for _, component := range components {
@@ -94,5 +97,5 @@ func (u Util) JoinPath(path ...string) (string, error) {
 		realpath = filepath.Join(realpath, symlinkPath)
 	}
 
-	return filepath.Join(u.DestDir, realpath), nil
+	return filepath.Join(u.DestDir, realpath, last), nil
 }
diff --git a/tests/negative/files/link.go b/tests/negative/files/link.go
@@ -22,6 +22,7 @@ import (
 func init() {
 	register.Register(register.NegativeTest, WriteThroughRelativeSymlink())
 	register.Register(register.NegativeTest, WriteThroughAbsoluteSymlink())
+	register.Register(register.NegativeTest, WriteOverBrokenSymlink())
 }
 func WriteThroughRelativeSymlink() types.Test {
 	name := "Write Through Relative Symlink off the Root Filesystem"
@@ -114,3 +115,38 @@ func WriteThroughAbsoluteSymlink() types.Test {
 		ConfigMinVersion: configMinVersion,
 	}
 }
+
+func WriteOverBrokenSymlink() types.Test {
+	name := "Write Over Broken Symlink at end of path"
+	in := types.GetBaseDisk()
+	out := types.GetBaseDisk()
+	config := `{
+	  "ignition": { "version": "$version" },
+	  "storage": {
+	    "files": [{
+	      "filesystem": "root",
+	      "path": "/etc/file",
+	      "overwrite": false,
+	      "mode": 420
+	    }]
+	  }
+	}`
+	in[0].Partitions.AddLinks("ROOT", []types.Link{
+		{
+			Node: types.Node{
+				Name:      "file",
+				Directory: "etc",
+			},
+			Target: "/usr/rofile",
+		},
+	})
+	configMinVersion := "2.2.0"
+
+	return types.Test{
+		Name:             name,
+		In:               in,
+		Out:              out,
+		Config:           config,
+		ConfigMinVersion: configMinVersion,
+	}
+}
diff --git a/tests/positive/files/link.go b/tests/positive/files/link.go
@@ -27,6 +27,8 @@ func init() {
 	register.Register(register.PositiveTest, WriteThroughAbsoluteSymlink())
 	register.Register(register.PositiveTest, ForceLinkCreation())
 	register.Register(register.PositiveTest, ForceHardLinkCreation())
+	register.Register(register.PositiveTest, WriteOverSymlink())
+	register.Register(register.PositiveTest, WriteOverBrokenSymlink())
 }
 
 func CreateHardLinkOnRoot() types.Test {
@@ -396,3 +398,113 @@ func ForceHardLinkCreation() types.Test {
 		ConfigMinVersion: configMinVersion,
 	}
 }
+
+func WriteOverSymlink() types.Test {
+	name := "Write Over Symlink at end of path"
+	in := types.GetBaseDisk()
+	out := types.GetBaseDisk()
+	// note this abuses the order in which ignition writes links and will break with 3.0.0
+	// Also tests that Ignition does not try to resolve symlink targets
+	config := `{
+	  "ignition": { "version": "$version" },
+	  "storage": {
+	    "files": [{
+	      "filesystem": "root",
+	      "path": "/etc/file",
+	      "mode": 420
+	    }]
+	  }
+	}`
+	in[0].Partitions.AddLinks("ROOT", []types.Link{
+		{
+			Node: types.Node{
+				Name:      "file",
+				Directory: "etc",
+			},
+			Target: "/usr/rofile",
+		},
+	})
+	in[0].Partitions.AddFiles("ROOT", []types.File{
+		{
+			Node: types.Node{
+				Name:      "rofile",
+				Directory: "usr",
+			},
+			Contents: "",
+			Mode:     420,
+		},
+	})
+	out[0].Partitions.AddFiles("ROOT", []types.File{
+		{
+			Node: types.Node{
+				Name:      "rofile",
+				Directory: "usr",
+			},
+			Contents: "",
+			Mode:     420,
+		},
+		{
+			Node: types.Node{
+				Name:      "file",
+				Directory: "etc",
+			},
+			Contents: "",
+			Mode:     420,
+		},
+	})
+	configMinVersion := "2.1.0"
+
+	return types.Test{
+		Name:             name,
+		In:               in,
+		Out:              out,
+		Config:           config,
+		ConfigMinVersion: configMinVersion,
+	}
+}
+
+func WriteOverBrokenSymlink() types.Test {
+	name := "Write Over Broken Symlink at end of path"
+	in := types.GetBaseDisk()
+	out := types.GetBaseDisk()
+	// note this abuses the order in which ignition writes links and will break with 3.0.0
+	// Also tests that Ignition does not try to resolve symlink targets
+	config := `{
+	  "ignition": { "version": "$version" },
+	  "storage": {
+	    "files": [{
+	      "filesystem": "root",
+	      "path": "/etc/file",
+	      "mode": 420
+	    }]
+	  }
+	}`
+	in[0].Partitions.AddLinks("ROOT", []types.Link{
+		{
+			Node: types.Node{
+				Name:      "file",
+				Directory: "etc",
+			},
+			Target: "/usr/rofile",
+		},
+	})
+	out[0].Partitions.AddFiles("ROOT", []types.File{
+		{
+			Node: types.Node{
+				Name:      "file",
+				Directory: "etc",
+			},
+			Contents: "",
+			Mode:     420,
+		},
+	})
+	configMinVersion := "2.1.0"
+
+	return types.Test{
+		Name:             name,
+		In:               in,
+		Out:              out,
+		Config:           config,
+		ConfigMinVersion: configMinVersion,
+	}
+}