Bugfix: Allow serve url to be set without materializing

actions/vql_test.go

@@ -130,27 +130,31 @@ func (self *ClientVQLTestSuite) TestMaxRows() {
 }
 
 func (self *ClientVQLTestSuite) TestMaxWait() {
-	resp := responder.TestResponderWithFlowId(self.ConfigObj, "TestMaxWait")
+	assert.True(self.T(), test_utils.Retry(self.T(), 5, time.Millisecond*500,
+		func(r *test_utils.R) {
+			resp := responder.TestResponderWithFlowId(self.ConfigObj, "TestMaxWait")
+
+			actions.VQLClientAction{}.StartQuery(self.ConfigObj, self.Sm.Ctx, resp,
+				&actions_proto.VQLCollectorArgs{
+					MaxRow:  1000,
+					MaxWait: 1,
+					Query: []*actions_proto.VQLRequest{
+						{
+							Name: "Query",
+							VQL:  "SELECT sleep(ms=400) FROM range(end=4)",
+						},
+					},
+				})
 
-	actions.VQLClientAction{}.StartQuery(self.ConfigObj, self.Sm.Ctx, resp,
-		&actions_proto.VQLCollectorArgs{
-			MaxRow:  1000,
-			MaxWait: 1,
-			Query: []*actions_proto.VQLRequest{
-				{
-					Name: "Query",
-					VQL:  "SELECT sleep(ms=400) FROM range(end=4)",
-				},
-			},
-		})
+			var responses []*crypto_proto.VeloMessage
 
-	var responses []*crypto_proto.VeloMessage
-	vtesting.WaitUntil(5*time.Second, self.T(), func() bool {
-		responses = resp.Drain.Messages()
-		payloads := getResponsePacketCounts(responses)
-		// Message will be split into 2 packets 2 messages in each
-		return len(payloads) == 2 && payloads[0] == 2 && payloads[1] == 2
-	})
+			vtesting.WaitUntil(5*time.Second, r, func() bool {
+				responses = resp.Drain.Messages()
+				payloads := getResponsePacketCounts(responses)
+				// Message will be split into 2 packets 2 messages in each
+				return len(payloads) == 2 && payloads[0] == 2 && payloads[1] == 2
+			})
+		}))
 }
 
 func getLogs(responses []*crypto_proto.VeloMessage) string {

api/tools.go

@@ -63,7 +63,7 @@ func (self *ApiServer) SetToolInfo(ctx context.Context,
 		return nil, Status(self.verbose, err)
 	}
 
-	// Clear internally managed tools the user should not be allowed
+	// Clear internally managed fields the user should not be allowed
 	// to set.
 	in.Versions = nil
 	in.ServeUrl = ""

api/upload.go

@@ -209,8 +209,10 @@ func formUploadHandler() http.Handler {
 			org_config_obj, form_desc.Filename)
 
 		pathspec, file_store_factory, err := path_manager.Path()
-		returnError(w, 403, fmt.Sprintf("Error: %v", err))
-		return
+		if err != nil {
+			returnError(w, 403, fmt.Sprintf("Error: %v", err))
+			return
+		}
 
 		form_desc.Url = path_manager.URL()
 

artifacts/testdata/server/testcases/binary_blobs.out.yaml

@@ -37,7 +37,7 @@ SELECT * FROM switch( b={SELECT Complete FROM execve(argv=["rm", "-f", "/tmp/aut
   "materialize": false,
   "artifact": "",
   "filestore_path": "351b4f6d59a4266cc7a2eab9cedf959eb6a4c924746044e6edeabdd1a477643e",
-  "serve_url": "",
+  "serve_url": "https://storage.googleapis.com/go.velocidex.com/winpmem_v3.3.rc3.exe",
   "serve_path": "",
   "filename": "winpmem_v3.3.rc3.exe",
   "hash": "",

artifacts/testdata/server/testcases/tools.out.yaml

@@ -31,6 +31,7 @@ SELECT inventory_add(tool='Autorun_amd64', url='https://storage.googleapis.com/g
     "url": "https://storage.googleapis.com/go.velocidex.com/autorunsc.exe",
     "admin_override": true,
     "filestore_path": "affbb298885d99a7569ef9ec463bbfbf9db112a11cec65cf78ec8657ab78950a",
+    "serve_url": "https://storage.googleapis.com/go.velocidex.com/autorunsc.exe",
     "filename": "autorunsc_x64.exe",
     "hash": "083d7eee4ed40a3e5a35675503b0b6be0cb627b4cb1009d185a558a805f64153",
     "versions": [

docs/references/vql.yaml

@@ -5421,7 +5421,7 @@
     type: string
     description: SQL Connection String
   - name: file
-    type: string
+    type: accessors.OSPath
     description: Required if using sqlite driver
   - name: accessor
     type: string
@@ -5439,7 +5439,7 @@
   type: Plugin
   args:
   - name: file
-    type: string
+    type: accessors.OSPath
     required: true
   - name: accessor
     type: string
@@ -6560,7 +6560,7 @@
   type: Plugin
   args:
   - name: filename
-    type: string
+    type: accessors.OSPath
     description: CSV files to open
     required: true
   - name: accessor
@@ -6578,7 +6578,7 @@
   type: Plugin
   args:
   - name: filename
-    type: string
+    type: accessors.OSPath
     description: CSV files to open
     required: true
   - name: accessor

file_store/test_utils/retry.go

@@ -0,0 +1,99 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// This file from
+// https://github.com/googlecloudplatform/golang-samples/blob/5465a7a4f5b2/internal/testutil/retry.go
+
+package test_utils
+
+import (
+	"bytes"
+	"fmt"
+	"path/filepath"
+	"runtime"
+	"strconv"
+	"testing"
+	"time"
+)
+
+// Retry runs function f for up to maxAttempts times until f returns successfully, and reports whether f was run successfully.
+// It will sleep for the given period between invocations of f.
+// Use the provided *testutil.R instead of a *testing.T from the function.
+func Retry(t *testing.T, maxAttempts int, sleep time.Duration, f func(r *R)) bool {
+	for attempt := 1; attempt <= maxAttempts; attempt++ {
+		r := &R{Attempt: attempt, log: &bytes.Buffer{}}
+
+		f(r)
+
+		if !r.failed {
+			if r.log.Len() != 0 {
+				t.Logf("Success after %d attempts:%s", attempt, r.log.String())
+			}
+			return true
+		}
+
+		if attempt == maxAttempts {
+			t.Logf("FAILED after %d attempts:%s", attempt, r.log.String())
+			t.Fail()
+		}
+
+		time.Sleep(sleep)
+	}
+	return false
+}
+
+// R is passed to each run of a flaky test run, manages state and accumulates log statements.
+type R struct {
+	// The number of current attempt.
+	Attempt int
+
+	failed bool
+	log    *bytes.Buffer
+}
+
+// Fail marks the run as failed, and will retry once the function returns.
+func (r *R) Fail() {
+	r.failed = true
+}
+
+// Errorf is equivalent to Logf followed by Fail.
+func (r *R) Errorf(s string, v ...interface{}) {
+	r.logf(s, v...)
+	r.Fail()
+}
+
+func (r *R) Fatalf(s string, v ...interface{}) {
+	r.logf(s, v...)
+	r.Fail()
+}
+
+// Logf formats its arguments and records it in the error log.
+// The text is only printed for the final unsuccessful run or the first successful run.
+func (r *R) Logf(s string, v ...interface{}) {
+	r.logf(s, v...)
+}
+
+func (r *R) logf(s string, v ...interface{}) {
+	fmt.Fprint(r.log, "\n")
+	fmt.Fprint(r.log, lineNumber())
+	fmt.Fprintf(r.log, s, v...)
+}
+
+func lineNumber() string {
+	_, file, line, ok := runtime.Caller(3) // logf, public func, user function
+	if !ok {
+		return ""
+	}
+	return filepath.Base(file) + ":" + strconv.Itoa(line) + ": "
+}

gui/velociraptor/src/components/notebooks/notebook-cell-renderer.jsx

@@ -176,11 +176,13 @@ export default class NotebookCellRenderer extends React.Component {
 
     componentDidMount() {
         this.source = CancelToken.source();
+        this.update_source = CancelToken.source();
         this.fetchCellContents();
     }
 
     componentWillUnmount() {
         this.source.cancel();
+        this.update_source.cancel();
     }
 
     componentDidUpdate = (prevProps, prevState, rootNode) => {
@@ -288,19 +290,27 @@ export default class NotebookCellRenderer extends React.Component {
 
     recalculate = () => {
         let cell = this.state.cell;
-        cell.output = "Loading";
+        cell.output = T("Loading");
         cell.timestamp = 0;
+        cell.calculating = true;
+        cell.messages = [];
+
         this.setState({cell: cell});
 
+        // Reset any inflight calls.
+        this.update_source.cancel();
+        this.update_source = CancelToken.source();
+
         api.post('v1/UpdateNotebookCell', {
             notebook_id: this.props.notebook_id,
             cell_id: this.state.cell.cell_id,
             type: this.state.cell.type || "Markdown",
             env: this.state.cell.env,
             currently_editing: false,
             input: this.state.cell.input,
-        }, this.source.token).then( (response) => {
+        }, this.update_source.token).then( (response) => {
             if (response.cancel) {
+                this.setState({currently_editing: false});
                 return;
             }
 
@@ -310,6 +320,7 @@ export default class NotebookCellRenderer extends React.Component {
                 api.error(response.data.error);
                 keep_editing = true;
             }
+
             this.setState({cell: response.data,
                            currently_editing: keep_editing});
         });
@@ -334,9 +345,16 @@ export default class NotebookCellRenderer extends React.Component {
     saveCell = () => {
         let cell = this.state.cell;
         cell.input = this.state.ace.getValue();
-        cell.output = "Loading";
+        cell.output = T("Loading");
         cell.timestamp = 0;
-        this.setState({cell: cell});
+        cell.calculating = true;
+        cell.messages = [];
+
+        this.setState({cell: cell, currently_editing: false});
+
+        // Reset any inflight calls.
+        this.update_source.cancel();
+        this.update_source = CancelToken.source();
 
         api.post('v1/UpdateNotebookCell', {
             notebook_id: this.props.notebook_id,
@@ -345,8 +363,9 @@ export default class NotebookCellRenderer extends React.Component {
             env: this.state.cell.env,
             currently_editing: false,
             input: cell.input,
-        }, this.source.token).then( (response) => {
+        }, this.update_source.token).then( (response) => {
             if (response.cancel) {
+                this.setState({currently_editing: false});
                 return;
             }
 

gui/velociraptor/src/components/tools/tool-viewer.jsx

@@ -128,11 +128,18 @@ export default class ToolViewer extends React.Component {
     setServeUrl = url=>{
         let tool = Object.assign({}, this.state.tool);
         tool.url = this.state.remote_url;
-        tool.hash = "";
+
+        // Preserve the hash if we already know it. This avoids us
+        // having to refetch the file all the time.
+        // tool.hash = "";
         tool.filename = "";
         tool.github_project = "";
         tool.serve_locally = false;
-        tool.materialize = true;
+
+        // Do not force a materialize - it is possible that the server
+        // has no egress access and it is not possible to materialize
+        // the tool from the server.
+        tool.materialize = false;
         this.setToolInfo(tool);
     };
 

services/inventory/inventory.go

@@ -483,6 +483,8 @@ func (self *InventoryService) AddTool(
 	tool := proto.Clone(tool_request).(*artifacts_proto.Tool)
 	tool.FilestorePath = paths.ObfuscateName(config_obj, tool.Name)
 
+	// No client config so we dont know any server urls - therefore we
+	// can not serve locally at all.
 	if tool.ServeLocally && config_obj.Client == nil {
 		tool.ServeLocally = false
 	}
@@ -492,6 +494,10 @@ func (self *InventoryService) AddTool(
 			return errors.New("No server URLs configured!")
 		}
 		tool.ServeUrl = config_obj.Client.ServerUrls[0] + "public/" + tool.FilestorePath
+	} else {
+		// If we dont serve the tool, the clients will directly get
+		// the tool from its upstream URL.
+		tool.ServeUrl = tool.Url
 	}
 
 	// Set the filename to something sensible so it is always valid.

services/sanity/fixtures/TestUpgradeTools.golden

@@ -6,13 +6,15 @@
     "filename": "www.company.com",
     "filestore_path": "95b097fd386adb06d90ff35d45f18fb507675b076522e7993b5be4a695f4247a",
     "name": "Tool1",
+    "serve_url": "https://www.company.com",
     "url": "https://www.company.com"
    },
    {
     "artifact": "TestArtifact",
     "filename": "www.example2.com",
     "filestore_path": "9a0dbc5804c7e06fc2e371a936bbe1d90b206aa9b8fbad7601f4a7f70fb596e1",
     "name": "Tool2",
+    "serve_url": "https://www.example2.com/",
     "url": "https://www.example2.com/"
    }
   ],

vtesting/helpers.go

@@ -31,6 +31,10 @@ import (
 	"www.velocidex.com/golang/vfilter/types"
 )
 
+type TestFailer interface {
+	Fatalf(format string, args ...interface{})
+}
+
 func ReadFile(t *testing.T, filename string) []byte {
 	result, err := ioutil.ReadFile(filename)
 	if err != nil {
@@ -62,7 +66,7 @@ func ContainsString(expected string, watched []string) bool {
 	return false
 }
 
-func WaitUntil(deadline time.Duration, t *testing.T, cb func() bool) {
+func WaitUntil(deadline time.Duration, t TestFailer, cb func() bool) {
 	end_time := time.Now().Add(deadline)
 
 	for end_time.After(time.Now()) {