[ARVADOS] updated: a340487a7d406e73e51479a765a3d08bdb92b8d0
Git user
git at public.curoverse.com
Tue May 16 09:13:06 EDT 2017
Summary of changes:
services/crunch-run/crunchrun.go | 69 ++++++++++++++++++++++-------------
services/crunch-run/crunchrun_test.go | 30 +++++++++++++--
2 files changed, 70 insertions(+), 29 deletions(-)
via a340487a7d406e73e51479a765a3d08bdb92b8d0 (commit)
via 94b92f075dbfb60a25fbe28e5741a553ac4985fd (commit)
from eee2c470dfa879c769eccb515861419a6b900101 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit a340487a7d406e73e51479a765a3d08bdb92b8d0
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Tue May 16 09:12:58 2017 -0400
11693: Detect and error on symlinks pointing to locations outside the output directory.
diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 5a89f6a..4676a4f 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -942,40 +942,57 @@ func (runner *ContainerRunner) CaptureOutput() error {
return nil
}
// read link to get container internal path
+ // only support 1 level of symlinking here.
var tgt string
tgt, err = os.Readlink(path)
if err != nil {
return err
}
- if !strings.HasPrefix(tgt, "/") {
- // Link is relative, don't handle it
- return nil
- }
- // go through mounts and reverse map to collection reference
- for _, bind := range binds {
- mnt := runner.Container.Mounts[bind]
- if tgt == bind || strings.HasPrefix(tgt, bind+"/") && mnt.Kind == "collection" {
- // get path relative to bind
- sourceSuffix := tgt[len(bind):]
- // get path relative to output dir
- bindSuffix := path[len(runner.HostOutputDir):]
-
- // Copy mount and adjust the path to add path relative to the bind
- adjustedMount := mnt
- adjustedMount.Path = filepath.Join(adjustedMount.Path, sourceSuffix)
-
- // get manifest text
- var m string
- m, err = runner.getCollectionManifestForPath(adjustedMount, bindSuffix)
- if err != nil {
- return err
+
+ outputSuffix := path[len(runner.HostOutputDir):]
+
+ if strings.HasPrefix(tgt, "/") {
+ // go through mounts and try reverse map to collection reference
+ for _, bind := range binds {
+ mnt := runner.Container.Mounts[bind]
+ if tgt == bind || strings.HasPrefix(tgt, bind+"/") && mnt.Kind == "collection" {
+ // get path relative to bind
+ targetSuffix := tgt[len(bind):]
+ // get path relative to output dir
+ outputSuffix := path[len(runner.HostOutputDir):]
+
+ // Copy mount and adjust the path to add path relative to the bind
+ adjustedMount := mnt
+ adjustedMount.Path = filepath.Join(adjustedMount.Path, targetSuffix)
+
+ // get manifest text
+ var m string
+ m, err = runner.getCollectionManifestForPath(adjustedMount, outputSuffix)
+ if err != nil {
+ return err
+ }
+ manifestText = manifestText + m
+ // delete symlink so WriteTree won't try to to dereference it.
+ os.Remove(path)
+ return nil
}
- manifestText = manifestText + m
- // delete symlink so WriteTree won't try to to dereference it.
- os.Remove(path)
- return nil
}
}
+
+ // Not a link to a mount. Must be dereferencible and
+ // point into the output directory.
+ tgt, err = filepath.EvalSymlinks(path)
+ if err != nil {
+ os.Remove(path)
+ return err
+ }
+
+ // Symlink target must be within the output directory otherwise it's an error.
+ if !strings.HasPrefix(tgt, runner.HostOutputDir+"/") {
+ os.Remove(path)
+ return fmt.Errorf("Output directory symlink %q points to invalid location %q, must point to mount or output directory.",
+ outputSuffix, tgt)
+ }
return nil
})
if err != nil {
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 64eed2c..37fe32a 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -1491,7 +1491,7 @@ func (s *TestSuite) TestOutputError(c *C) {
extraMounts := []string{}
api, _, _ := FullRunHelper(c, helperRecord, extraMounts, 0, func(t *TestDockerClient) {
- os.Symlink("/does/not/exist", t.realTemp+"/2/baz")
+ os.Symlink("/etc/hosts", t.realTemp+"/2/baz")
t.logWriter.Close()
})
commit 94b92f075dbfb60a25fbe28e5741a553ac4985fd
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Mon May 15 16:58:05 2017 -0400
11693: Add test that error capturing output results in cancelled container.
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 3c32944..64eed2c 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -10,7 +10,6 @@ import (
"fmt"
"io"
"io/ioutil"
- "log"
"net"
"os"
"os/exec"
@@ -640,7 +639,9 @@ func FullRunHelper(c *C, record string, extraMounts []string, exitCode int, fn f
}
err = cr.Run()
- c.Check(err, IsNil)
+ if api.CalledWith("container.state", "Complete") != nil {
+ c.Check(err, IsNil)
+ }
c.Check(api.WasSetRunning, Equals, true)
c.Check(api.Content[api.Calls-1]["container"].(arvadosclient.Dict)["log"], NotNil)
@@ -1448,7 +1449,6 @@ func (s *TestSuite) TestOutputSymlinkToInput(c *C) {
}
api, _, _ := FullRunHelper(c, helperRecord, extraMounts, 0, func(t *TestDockerClient) {
- log.Printf("realtemp is %v", t.realTemp)
os.Symlink("/keep/foo/sub1file2", t.realTemp+"/2/baz")
os.Symlink("/keep/foo2/subdir1/file2_in_subdir1.txt", t.realTemp+"/2/baz2")
os.Symlink("/keep/foo2/subdir1", t.realTemp+"/2/baz3")
@@ -1474,6 +1474,30 @@ func (s *TestSuite) TestOutputSymlinkToInput(c *C) {
}
}
+func (s *TestSuite) TestOutputError(c *C) {
+ helperRecord := `{
+ "command": ["/bin/sh", "-c", "echo $FROBIZ"],
+ "container_image": "d4ab34d3d4f8a72f5c4973051ae69fab+122",
+ "cwd": "/bin",
+ "environment": {"FROBIZ": "bilbo"},
+ "mounts": {
+ "/tmp": {"kind": "tmp"}
+ },
+ "output_path": "/tmp",
+ "priority": 1,
+ "runtime_constraints": {}
+ }`
+
+ extraMounts := []string{}
+
+ api, _, _ := FullRunHelper(c, helperRecord, extraMounts, 0, func(t *TestDockerClient) {
+ os.Symlink("/does/not/exist", t.realTemp+"/2/baz")
+ t.logWriter.Close()
+ })
+
+ c.Check(api.CalledWith("container.state", "Cancelled"), NotNil)
+}
+
func (s *TestSuite) TestStdinCollectionMountPoint(c *C) {
helperRecord := `{
"command": ["/bin/sh", "-c", "echo $FROBIZ"],
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list