Skip to content

Commit

Permalink
Remove symlink, not target, in FileOp.Rm.
Browse files Browse the repository at this point in the history
Before this change, if the path provided to FileOp.Rm was a symlink then
the target of the symlink would be removed instead of the symlink
itself. Now, the symlink will be removed instead. However, any symlinks
present in the parent dirs of the specified path will still be resolved
before calling os.Remove; this change only results in the base of the
specified path not being followed.

Signed-off-by: Erik Sipsma <[email protected]>
  • Loading branch information
sipsma committed Nov 18, 2021
1 parent fce4a32 commit 43a7426
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 1 deletion.
36 changes: 36 additions & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ func TestIntegration(t *testing.T) {
testPullZstdImage,
testMergeOp,
testMergeOpCache,
testRmSymlink,
}, mirrors)

integration.Run(t, []integration.Test{
Expand Down Expand Up @@ -3824,6 +3825,41 @@ func testSourceMapFromRef(t *testing.T, sb integration.Sandbox) {
require.Equal(t, int32(1), srcs[0].Ranges[0].Start.Character)
}

func testRmSymlink(t *testing.T, sb integration.Sandbox) {
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

// Test that if FileOp.Rm is called on a symlink, then
// the symlink is removed rather than the target
mnt := llb.Image("alpine").
Run(llb.Shlex("touch /mnt/target")).
AddMount("/mnt", llb.Scratch())

mnt = llb.Image("alpine").
Run(llb.Shlex("ln -s target /mnt/link")).
AddMount("/mnt", mnt)

def, err := mnt.File(llb.Rm("link")).Marshal(sb.Context())
require.NoError(t, err)

destDir, err := ioutil.TempDir("", "buildkit")
require.NoError(t, err)
defer os.RemoveAll(destDir)

_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterLocal,
OutputDir: destDir,
},
},
}, nil)
require.NoError(t, err)

require.NoError(t, fstest.CheckDirectoryEqualWithApplier(destDir, fstest.CreateFile("target", nil, 0644)))
}

func testProxyEnv(t *testing.T, sb integration.Sandbox) {
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
Expand Down
8 changes: 7 additions & 1 deletion solver/llbsolver/file/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,16 @@ func rm(ctx context.Context, d string, action pb.FileActionRm) error {
}

func rmPath(root, src string, allowNotFound bool) error {
p, err := fs.RootPath(root, filepath.Join("/", src))
src = filepath.Clean(src)
dir, base := filepath.Split(src)
if base == "" {
return errors.New("rmPath: invalid empty path")
}
dir, err := fs.RootPath(root, filepath.Join("/", dir))
if err != nil {
return err
}
p := filepath.Join(dir, base)

if err := os.RemoveAll(p); err != nil {
if errors.Is(err, os.ErrNotExist) && allowNotFound {
Expand Down
7 changes: 7 additions & 0 deletions solver/pb/caps.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const (
CapFileBase apicaps.CapID = "file.base"
CapFileRmWildcard apicaps.CapID = "file.rm.wildcard"
CapFileCopyIncludeExcludePatterns apicaps.CapID = "file.copy.includeexcludepatterns"
CapFileRmNoFollowSymlink apicaps.CapID = "file.rm.nofollowsymlink"

CapConstraints apicaps.CapID = "constraints"
CapPlatform apicaps.CapID = "platform"
Expand Down Expand Up @@ -328,6 +329,12 @@ func init() {
Status: apicaps.CapStatusExperimental,
})

Caps.Init(apicaps.Cap{
ID: CapFileRmNoFollowSymlink,
Enabled: true,
Status: apicaps.CapStatusExperimental,
})

Caps.Init(apicaps.Cap{
ID: CapFileCopyIncludeExcludePatterns,
Enabled: true,
Expand Down

0 comments on commit 43a7426

Please sign in to comment.