Bugfix: Only update notebooks atomically (Velocidex#3439)

Notebook updates were allowed to be done without locking the notebook. This resulted in the notebook being corrupted when updates were initiated during times where a cell was calculated, resulting in cell corruption. This PR ensures the GUI does not attempt to edit the notebook while its state is not in sync with the server. As well as that the notebook manager ensures the notebook is updated under locks. Also fixed memory leak regression with group by
Genio40921 · Apr 21, 2024 · 1af93b1 · 1af93b1
1 parent 314110f
commit 1af93b1
Show file tree

Hide file tree

Showing 21 changed files with 256 additions and 77 deletions.
diff --git a/api/notebooks.go b/api/notebooks.go
@@ -195,10 +195,6 @@ func (self *ApiServer) UpdateNotebook(
 		return nil, InvalidStatus("Notebook is not shared with user.")
 	}
 
-	if old_notebook.ModifiedTime != in.ModifiedTime {
-		return nil, InvalidStatus("Edit clash detected.")
-	}
-
 	// When updating an existing notebook only certain fields may
 	// be changed by the user - definitely not the creator, created time or notebookId.
 	in.ModifiedTime = time.Now().Unix()

diff --git a/artifacts/definitions/Server/Import/CuratedSigma.yaml b/artifacts/definitions/Server/Import/CuratedSigma.yaml
@@ -19,10 +19,10 @@ parameters:
       - Velociraptor Hayabusa Ruleset
       - Velociraptor Hayabusa Live Detection
       - Velociraptor ChopChopGo Ruleset (Linux)
+      - Velociraptor Curated Windows Ruleset
 
   - name: Prefix
     description: Add artifacts with this prefix
-    default: Sigma.
 
 sources:
   - query: |

diff --git a/artifacts/definitions/Server/Internal/ToolDependencies.yaml b/artifacts/definitions/Server/Internal/ToolDependencies.yaml
@@ -7,19 +7,19 @@ description: |
 
 tools:
   - name: VelociraptorWindows
-    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72-rc2-windows-amd64.exe
+    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72.0-windows-amd64.exe
     serve_locally: true
-    version: 0.72-rc2
+    version: 0.72.0
 
   - name: VelociraptorWindows_x86
-    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72-rc2-windows-386.exe
+    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72.0-windows-386.exe
     serve_locally: true
-    version: 0.72-rc2
+    version: 0.72.0
 
   - name: VelociraptorLinux
-    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72-rc2-linux-amd64-musl
+    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72.0-linux-amd64-musl
     serve_locally: true
-    version: 0.72-rc2
+    version: 0.72.0
 
   # On MacOS we can not embed the config in the binary so we use a
   # shell script stub instead. See
@@ -31,11 +31,11 @@ tools:
     serve_locally: true
 
   - name: VelociraptorWindowsMSI
-    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72-rc2-windows-amd64.msi
+    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72.0-windows-amd64.msi
     serve_locally: true
-    version: 0.72-rc2
+    version: 0.72.0
 
   - name: VelociraptorWindows_x86MSI
-    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72-rc2-windows-386.msi
+    url: https://github.com/Velocidex/velociraptor/releases/download/v0.72/velociraptor-v0.72.0-windows-386.msi
     serve_locally: true
-    version: 0.72-rc2
+    version: 0.72.0
diff --git a/docs/wix/README.md b/docs/wix/README.md
@@ -11,9 +11,10 @@ are happy with the default settings it is easier to just use the
 official distributed MSI packages.
 
 To build MSI packages you will need to download and install the WIX
-distribution from the github page (it requires .NET 3.5):
+distribution from the github page. We currently recommend the 3.14
+release series:
 
-http://wixtoolset.org/releases/
+https://github.com/wixtoolset/wix3/releases/
 
 Next, follow these steps:
 

diff --git a/docs/wix/build_amd64.bat b/docs/wix/build_amd64.bat
@@ -1,3 +1,3 @@
-"c:\Program Files (x86)\WiX Toolset v3.11\bin\candle.exe" velociraptor_amd64.xml -arch x64 -ext "c:\Program Files (x86)\WiX Toolset v3.11\bin\WixUtilExtension.dll"
+"c:\Program Files (x86)\WiX Toolset v3.14\bin\candle.exe" velociraptor_amd64.xml -arch x64 -ext "c:\Program Files (x86)\WiX Toolset v3.14\bin\WixUtilExtension.dll"
 
-"c:\Program Files (x86)\WiX Toolset v3.11\bin\light.exe" velociraptor_amd64.wixobj -ext "c:\Program Files (x86)\WiX Toolset v3.11\bin\WixUtilExtension.dll"
+"c:\Program Files (x86)\WiX Toolset v3.14\bin\light.exe" velociraptor_amd64.wixobj -ext "c:\Program Files (x86)\WiX Toolset v3.14\bin\WixUtilExtension.dll"
diff --git a/docs/wix/build_x86.bat b/docs/wix/build_x86.bat
@@ -1,3 +1,3 @@
-"c:\Program Files (x86)\WiX Toolset v3.11\bin\candle.exe" velociraptor_x86.xml -arch x86 -ext "c:\Program Files (x86)\WiX Toolset v3.11\bin\WixUtilExtension.dll"
+"c:\Program Files (x86)\WiX Toolset v3.14\bin\candle.exe" velociraptor_x86.xml -arch x86 -ext "c:\Program Files (x86)\WiX Toolset v3.14\bin\WixUtilExtension.dll"
 
-"c:\Program Files (x86)\WiX Toolset v3.11\bin\light.exe" velociraptor_x86.wixobj -ext "c:\Program Files (x86)\WiX Toolset v3.11\bin\WixUtilExtension.dll"
+"c:\Program Files (x86)\WiX Toolset v3.14\bin\light.exe" velociraptor_x86.wixobj -ext "c:\Program Files (x86)\WiX Toolset v3.14\bin\WixUtilExtension.dll"
diff --git a/go.mod b/go.mod
@@ -99,7 +99,7 @@ require (
 	www.velocidex.com/golang/go-prefetch v0.0.0-20220801101854-338dbe61982a
 	www.velocidex.com/golang/oleparse v0.0.0-20230217092320-383a0121aafe
 	www.velocidex.com/golang/regparser v0.0.0-20240404115756-2169ac0e3c09
-	www.velocidex.com/golang/vfilter v0.0.0-20240417120216-f7fa24ce744e
+	www.velocidex.com/golang/vfilter v0.0.0-20240421183637-1454295e8546
 )
 
 require (

diff --git a/go.sum b/go.sum
@@ -1344,7 +1344,7 @@ www.velocidex.com/golang/oleparse v0.0.0-20230217092320-383a0121aafe h1:o9jQWSwK
 www.velocidex.com/golang/oleparse v0.0.0-20230217092320-383a0121aafe/go.mod h1:R7IisRzDO7q5LVRJsCQf1xA50LrIavsPWzAjVE4THyY=
 www.velocidex.com/golang/regparser v0.0.0-20240404115756-2169ac0e3c09 h1:G1RWYBXP2lSzxKcrAU1YhiUlBetZ7hGIzIiWuuazvfo=
 www.velocidex.com/golang/regparser v0.0.0-20240404115756-2169ac0e3c09/go.mod h1:pxSECT5mWM3goJ4sxB4HCJNKnKqiAlpyT8XnvBwkLGU=
-www.velocidex.com/golang/vfilter v0.0.0-20240417120216-f7fa24ce744e h1:FhnWGS3SjGaUh4mGkVhJ/CakqQgWVAKwToRVB5s7dAM=
-www.velocidex.com/golang/vfilter v0.0.0-20240417120216-f7fa24ce744e/go.mod h1:P50KPQr2LpWVAu7ilGH8CBLBASGtOJ2971yA9YhR8rY=
+www.velocidex.com/golang/vfilter v0.0.0-20240421183637-1454295e8546 h1:XWhIhCSFt/qEgmrYiUSB5J4KaGmBK0R20P/jzenwV+E=
+www.velocidex.com/golang/vfilter v0.0.0-20240421183637-1454295e8546/go.mod h1:P50KPQr2LpWVAu7ilGH8CBLBASGtOJ2971yA9YhR8rY=
 www.velocidex.com/golang/vtypes v0.0.0-20240123105603-069d4a7f435c h1:rL/It+Ig+mvIhmy9vl5gg5b6CX2J12x0v2SXIT2RoWE=
 www.velocidex.com/golang/vtypes v0.0.0-20240123105603-069d4a7f435c/go.mod h1:tjaJNlBWbvH4cEMrEu678CFR2hrtcdyPINIpRxrOh4U=
diff --git a/gui/velociraptor/src/components/core/api-service.jsx b/gui/velociraptor/src/components/core/api-service.jsx
@@ -53,8 +53,24 @@ function retryDelay(retryNumber = 0) {
 }
 
 function isRetryableError(error) {
-    return error.code !== 'ECONNABORTED' && (!error.response || (
-        error.response.status >= 500 && error.response.status <= 599));
+    if (error.code === 'ECONNABORTED') {
+        return false;
+    }
+
+    if (!error.response) {
+        return true;
+    }
+
+    // This represents a real server error no need to retry it.
+    if (error.response.data && error.response.data.code) {
+        return false;
+    }
+
+    if (error.response.status >= 500 && error.response.status <= 599) {
+        return true;
+    }
+
+    return false;
 }
 
 function isNetworkError(error) {

diff --git a/gui/velociraptor/src/components/notebooks/notebook-cell-renderer.jsx b/gui/velociraptor/src/components/notebooks/notebook-cell-renderer.jsx
@@ -147,6 +147,11 @@ export default class NotebookCellRenderer extends React.Component {
         selected_cell_id: PropTypes.string,
         setSelectedCellId: PropTypes.func,
 
+        // Manage notebook lock state. The notebook tries to avoid
+        // confusing by locking updates while any cell is calculating.
+        notebookLocked: PropTypes.number,
+        incNotebookLocked: PropTypes.func,
+
         upCell: PropTypes.func,
         downCell: PropTypes.func,
         deleteCell: PropTypes.func,
@@ -254,13 +259,18 @@ export default class NotebookCellRenderer extends React.Component {
         this.source.cancel();
         this.source = CancelToken.source();
 
-        let cell_version = (this.props.cell_metadata && this.props.cell_metadata.current_version);
+        let cell_version = this.props.cell_metadata &&
+            this.props.cell_metadata.current_version;
+
+        this.props.incNotebookLocked(1);
 
         api.get("v1/GetNotebookCell", {
             notebook_id: this.props.notebook_id,
             cell_id: this.props.cell_metadata.cell_id,
             version: cell_version,
         }, this.source.token).then((response) => {
+            this.props.incNotebookLocked(-1);
+
             if (response.cancel) {
                 return;
             }
@@ -271,7 +281,17 @@ export default class NotebookCellRenderer extends React.Component {
                                input: cell.input,
                                loading: false});
             }
-        });
+
+
+        }).catch((e) => {
+            this.props.incNotebookLocked(-1);
+
+            let message = e.response && e.response.data &&
+                e.response.data.message;
+            let cell = Object.assign({}, this.props.cell_metadata || {});
+            cell.messages = [message];
+            this.setState({loading: false, cell: cell});
+        });;
     };
 
     setEditing = (edit) => {
@@ -281,10 +301,10 @@ export default class NotebookCellRenderer extends React.Component {
     getPlaceholder = () => {
         let type = this.ace_type(this.state.cell && this.state.cell.type);
         if (type === "vql") {
-            return "Type VQL to evaluate on the server (press ? for help)";
+            return T("Type VQL to evaluate on the server (press ? for help)");
         };
         if (type === "markdown") {
-            return "Enter markdown text to render in the notebook";
+            return T("Enter markdown text to render in the notebook");
         };
         return type;
     }
@@ -337,6 +357,9 @@ export default class NotebookCellRenderer extends React.Component {
         this.update_source.cancel();
         this.update_source = CancelToken.source();
 
+        // Lock the notebook until we refresh the notebook.
+        this.props.incNotebookLocked(1);
+
         api.post('v1/UpdateNotebookCell', {
             notebook_id: this.props.notebook_id,
             cell_id: this.state.cell.cell_id,
@@ -345,6 +368,8 @@ export default class NotebookCellRenderer extends React.Component {
             currently_editing: false,
             input: this.state.cell.input,
         }, this.update_source.token).then( (response) => {
+            this.props.incNotebookLocked(-1);
+
             if (response.cancel) {
                 this.setState({currently_editing: false});
                 return;
@@ -357,9 +382,17 @@ export default class NotebookCellRenderer extends React.Component {
                 keep_editing = true;
             }
 
-            this.setState({cell: response.data,
-                           loading: false,
-                           currently_editing: keep_editing});
+            let cell = response.data;
+            if (cell.cell_id === this.props.cell_metadata.cell_id) {
+                this.setState({cell: response.data,
+                               loading: false,
+                               currently_editing: keep_editing});
+            } else {
+                this.fetchCellContents();
+            }
+
+        }).catch(e=>{
+            this.props.incNotebookLocked(-1);
         });
 
     }
@@ -397,14 +430,18 @@ export default class NotebookCellRenderer extends React.Component {
         this.update_source.cancel();
         this.update_source = CancelToken.source();
 
+        this.props.incNotebookLocked(1);
+
         api.post('v1/UpdateNotebookCell', {
             notebook_id: this.props.notebook_id,
             cell_id: cell.cell_id,
-            type: cell.type || "Markdown",
+            type: cell.type || T("Markdown"),
             env: this.state.cell.env,
             currently_editing: false,
             input: cell.input,
         }, this.update_source.token).then( (response) => {
+            this.props.incNotebookLocked(-1);
+
             if (response.cancel) {
                 this.setState({currently_editing: false});
                 return;
@@ -416,11 +453,23 @@ export default class NotebookCellRenderer extends React.Component {
                 api.error(response.data.error);
                 keep_editing = true;
             }
-            this.setState({cell: response.data,
-                           currently_editing: keep_editing});
 
-            if(this.state.ace && this.state.ace.setValue && response.data.input) {
-                this.state.ace.setValue(response.data.input);
+            let cell = response.data;
+            if (cell.cell_id === this.props.cell_metadata.cell_id) {
+                this.setState({cell: response.data,
+                               loading: false,
+                               currently_editing: keep_editing});
+
+                // Update the ACL edit box if needed.
+                if (this.state.ace &&
+                    this.state.ace.setValue &&
+                    response.data.input) {
+
+                    this.state.ace.setValue(response.data.input);
+                }
+
+            } else {
+                this.fetchCellContents();
             }
         });
     };
@@ -703,6 +752,7 @@ export default class NotebookCellRenderer extends React.Component {
               <Button data-tooltip={T("Up Cell")}
                       data-position="right"
                       className="btn-tooltip"
+                      disabled={this.props.notebookLocked}
                       onClick={() => {
                           this.props.upCell(this.state.cell.cell_id);
                       }}
@@ -713,6 +763,7 @@ export default class NotebookCellRenderer extends React.Component {
               <Button data-tooltip={T("Down Cell")}
                       data-position="right"
                       className="btn-tooltip"
+                      disabled={this.props.notebookLocked}
                       onClick={() => {
                           this.props.downCell(this.state.cell.cell_id);
                       }}