Merge pull request #374 from CoinFabrik/ci-cd-fixup

CI/CD fixup

detectors/dos-unexpected-revert-with-vector/src/lib.rs

@@ -18,10 +18,10 @@ use rustc_span::Span;
 const LINT_MESSAGE: &str = "This vector operation is called without access control";
 
 dylint_linting::impl_late_lint! {
-    pub UNEXPECTED_REVERT_WARN,
+    pub DOS_UNEXPECTED_REVERT_WITH_VECTOR,
     Warn,
     "",
-    UnexpectedRevertWarn::default(),
+    DosUnexpectedRevertWithVector::default(),
     {
         name: "Unexpected Revert Inserting to Storage",
         long_message: " It occurs by preventing transactions by other users from being successfully executed forcing the blockchain state to revert to its original state.",
@@ -32,14 +32,14 @@ dylint_linting::impl_late_lint! {
 }
 
 #[derive(Default)]
-pub struct UnexpectedRevertWarn {}
-impl UnexpectedRevertWarn {
+pub struct DosUnexpectedRevertWithVector {}
+impl DosUnexpectedRevertWithVector {
     pub fn new() -> Self {
         Self {}
     }
 }
 
-impl<'tcx> LateLintPass<'tcx> for UnexpectedRevertWarn {
+impl<'tcx> LateLintPass<'tcx> for DosUnexpectedRevertWithVector {
     fn check_fn(
         &mut self,
         cx: &LateContext<'tcx>,
@@ -87,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for UnexpectedRevertWarn {
         if uvf_storage.push_def_id.is_some() && !uvf_storage.require_auth {
             span_lint(
                 uvf_storage.cx,
-                UNEXPECTED_REVERT_WARN,
+                DOS_UNEXPECTED_REVERT_WITH_VECTOR,
                 uvf_storage.push_span.unwrap(),
                 LINT_MESSAGE,
             );

detectors/integer-overflow-or-underflow/src/lib.rs

@@ -16,7 +16,7 @@ use utils::ConstantAnalyzer;
 pub const LINT_MESSAGE: &str = "Potential for integer arithmetic overflow/underflow. Consider checked, wrapping or saturating arithmetic.";
 
 dylint_linting::declare_late_lint! {
-    pub INTEGER_OVERFLOW_UNDERFLOW,
+    pub INTEGER_OVERFLOW_OR_UNDERFLOW,
     Warn,
     LINT_MESSAGE,
     {
@@ -30,15 +30,15 @@ dylint_linting::declare_late_lint! {
 enum Type {
     Overflow,
     Underflow,
-    OverflowUnderflow,
+    OverflowAndUnderflow,
 }
 
 impl Type {
     fn message(&self) -> &'static str {
         match self {
             Type::Overflow => "overflow",
             Type::Underflow => "underflow",
-            Type::OverflowUnderflow => "overflow or underflow",
+            Type::OverflowAndUnderflow => "overflow or underflow",
         }
     }
 }
@@ -84,14 +84,14 @@ impl Finding {
         )
     }
 }
-pub struct IntegerOverflowUnderflowVisitor<'a, 'tcx> {
+pub struct IntegerOverflowOrUnderflowVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     findings: Vec<Finding>,
     is_complex_operation: bool,
     constant_analyzer: ConstantAnalyzer<'a, 'tcx>,
 }
 
-impl<'tcx> IntegerOverflowUnderflowVisitor<'_, 'tcx> {
+impl<'tcx> IntegerOverflowOrUnderflowVisitor<'_, 'tcx> {
     pub fn check_pow(&mut self, expr: &Expr<'tcx>, base: &Expr<'tcx>, exponent: &Expr<'tcx>) {
         if self.constant_analyzer.is_constant(base) && self.constant_analyzer.is_constant(exponent)
         {
@@ -137,7 +137,7 @@ impl<'tcx> IntegerOverflowUnderflowVisitor<'_, 'tcx> {
         }
 
         let (finding_type, cause) = if self.is_complex_operation {
-            (Type::OverflowUnderflow, Cause::Multiple)
+            (Type::OverflowAndUnderflow, Cause::Multiple)
         } else {
             match op {
                 BinOpKind::Add => (Type::Overflow, Cause::Add),
@@ -152,7 +152,7 @@ impl<'tcx> IntegerOverflowUnderflowVisitor<'_, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for IntegerOverflowUnderflowVisitor<'a, 'tcx> {
+impl<'a, 'tcx> Visitor<'tcx> for IntegerOverflowOrUnderflowVisitor<'a, 'tcx> {
     fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
         match expr.kind {
             ExprKind::Binary(op, lhs, rhs) | ExprKind::AssignOp(op, lhs, rhs) => {
@@ -178,7 +178,7 @@ impl<'a, 'tcx> Visitor<'tcx> for IntegerOverflowUnderflowVisitor<'a, 'tcx> {
     }
 }
 
-impl<'tcx> LateLintPass<'tcx> for IntegerOverflowUnderflow {
+impl<'tcx> LateLintPass<'tcx> for IntegerOverflowOrUnderflow {
     fn check_fn(
         &mut self,
         cx: &LateContext<'tcx>,
@@ -201,7 +201,7 @@ impl<'tcx> LateLintPass<'tcx> for IntegerOverflowUnderflow {
         constant_analyzer.visit_body(body);
 
         // Analyze the function for integer overflow/underflow
-        let mut visitor = IntegerOverflowUnderflowVisitor {
+        let mut visitor = IntegerOverflowOrUnderflowVisitor {
             cx,
             findings: Vec::new(),
             is_complex_operation: false,
@@ -213,7 +213,7 @@ impl<'tcx> LateLintPass<'tcx> for IntegerOverflowUnderflow {
         for finding in visitor.findings {
             span_lint_and_help(
                 cx,
-                INTEGER_OVERFLOW_UNDERFLOW,
+                INTEGER_OVERFLOW_OR_UNDERFLOW,
                 finding.span,
                 finding.generate_message(),
                 None,

detectors/iterators-over-indexing/src/lib.rs

@@ -27,7 +27,7 @@ const LINT_MESSAGE: &str =
     "Hardcoding an index could lead to panic if the top bound is out of bounds.";
 
 dylint_linting::declare_late_lint! {
-    pub ITERATOR_OVER_INDEXING,
+    pub ITERATORS_OVER_INDEXING,
     Warn,
     LINT_MESSAGE,
     {
@@ -365,7 +365,7 @@ impl<'a, 'b> Visitor<'a> for ForLoopVisitor<'a, 'b> {
     }
 }
 
-impl<'tcx> LateLintPass<'tcx> for IteratorOverIndexing {
+impl<'tcx> LateLintPass<'tcx> for IteratorsOverIndexing {
     fn check_fn(
         &mut self,
         cx: &rustc_lint::LateContext<'tcx>,
@@ -388,7 +388,7 @@ impl<'tcx> LateLintPass<'tcx> for IteratorOverIndexing {
             for span in span_constant {
                 span_lint_and_help(
                     cx,
-                    ITERATOR_OVER_INDEXING,
+                    ITERATORS_OVER_INDEXING,
                     span,
                     LINT_MESSAGE,
                     None,

run-tests.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+#This script is here for convenience only. Do not use it in CI/CD.
+python3 scripts/run-tests2.py

scripts/run-tests.py

@@ -1,6 +1,8 @@
 import os
 import argparse
 import time
+import tempfile
+import json
 
 from utils import (
     parse_json_from_string,
@@ -14,7 +16,6 @@
 GREEN = "\033[92m"
 ENDC = "\033[0m"
 
-
 def run_tests(detector):
     errors = []
     directory = os.path.join("test-cases", detector)
@@ -27,28 +28,31 @@ def run_tests(detector):
         if is_rust_project(root):
             if run_unit_tests(root):
                 errors.append(root)
-            if run_integration_tests(detector, root):
+            if not run_integration_tests(detector, root):
                 errors.append(root)
     return errors
 
+def convert_code(s):
+    return s.replace('_', '-')
 
 def run_unit_tests(root):
     start_time = time.time()
-    returncode, _, stderr = run_subprocess(["cargo", "test", "--all-features"], root)
+    returncode, stdout, _ = run_subprocess(["cargo", "test", "--all-features"], root)
     print_results(
         returncode,
-        stderr,
+        stdout,
         "unit-test",
         root,
         time.time() - start_time,
     )
     return returncode != 0
 
 
+
 def run_integration_tests(detector, root):
     start_time = time.time()
 
-    detectors_path = os.path.join(os.getcwd(), "detectors")
+    local_detectors = os.path.join(os.getcwd(), "detectors")
 
     returncode, stdout, _ = run_subprocess(
         [
@@ -58,12 +62,11 @@ def run_integration_tests(detector, root):
             detector,
             "--metadata",
             "--local-detectors",
-            detectors_path,
+            local_detectors,
         ],
         root,
     )
 
-    #print("stderr: ", stderr.read())
     if stdout is None:
         print(
             f"{RED}Failed to run integration tests in {root} - Metadata returned empty.{ENDC}"
@@ -73,36 +76,73 @@ def run_integration_tests(detector, root):
     detector_metadata = parse_json_from_string(stdout)
 
     if not isinstance(detector_metadata, dict):
-        print("Failed to extract JSON:\n", detector_metadata)
+        print("Failed to extract JSON:", detector_metadata)
         return True
 
     detector_key = detector.replace("-", "_")
     short_message = detector_metadata.get(detector_key, {}).get("short_message")
 
-    returncode, stdout, stderr = run_subprocess(
-        [
-            "cargo",
-            "scout-audit",
-            "--filter",
-            detector,
-            "--local-detectors",
-            os.path.join(os.getcwd(), "detectors"),
-        ],
-        root,
-    )
+    _, tempPath = tempfile.mkstemp(None, f'scout_{os.getpid()}_')
+
+    returncode = None
+    stderr = None
+
+    if detector != "unnecessary-lint-allow":
+        returncode, _, stderr = run_subprocess(
+            [
+                "cargo",
+                "scout-audit",
+                "--filter",
+                detector,
+                "--local-detectors",
+                local_detectors,
+                "--output-format",
+                "raw-json",
+                "--output-path",
+                tempPath,
+            ],
+            root,
+        )
+    else:
+        #We need to handle this case differently, because using filter will
+        #cause other detectors to not run, making the test case invalid.
+        returncode, _, stderr = run_subprocess(
+            [
+                "cargo",
+                "scout-audit",
+                "--local-detectors",
+                local_detectors,
+                "--output-format",
+                "raw-json",
+                "--output-path",
+                tempPath,
+            ],
+            root,
+        )
 
-    should_lint = root.endswith("vulnerable-example")
-    if should_lint and short_message and short_message not in stdout:
-        returncode = 1
+    if returncode != 0:
+        print(f"{RED}Scout failed to run.{ENDC}")
+        return False
+
+    should_fail = "vulnerable" in root
+    did_fail = False
+
+    with open(tempPath) as file:
+        detectors_triggered = {convert_code(json.loads(line.rstrip())['code']['code']) for line in file}
+        did_fail = detector in detectors_triggered
+        if should_fail != did_fail:
+            explanation = "it failed when it shouldn't have" if did_fail else "it didn't fail when it should have"
+            print(f"{RED}Test case {root} didn't pass because {explanation}.{ENDC}")
+            return False
 
     print_results(
         returncode,
-        stdout,
+        stderr,
         "integration-test",
         root,
         time.time() - start_time,
     )
-    return returncode != 0
+    return True
 
 
 if __name__ == "__main__":

scripts/run-tests2.py

@@ -0,0 +1,10 @@
+#This script is here for convenience only. Do not use it in CI/CD.
+import os
+import stat
+
+def is_dir(path):
+    return stat.S_ISDIR(os.stat(path).st_mode)
+
+for name in os.listdir('test-cases'):
+    if is_dir('test-cases/' + name) and name[0:1] != '.' and name != 'target':
+        os.system(f'python3 scripts/run-tests.py --detector={name}')

test-cases/assert-violation/Cargo.toml

@@ -4,7 +4,7 @@ members = ["assert-violation-*/*"]
 resolver = "2"
 
 [workspace.dependencies]
-soroban-sdk = { version = "=21.4.0" }
+soroban-sdk = { version = "=21.7.6" }
 
 [profile.release]
 codegen-units = 1

test-cases/avoid-panic-error/Cargo.toml

@@ -4,7 +4,7 @@ members = ["avoid-panic-error-*/*"]
 resolver = "2"
 
 [workspace.dependencies]
-soroban-sdk = { version = "=21.4.0" }
+soroban-sdk = { version = "=21.7.6" }
 
 [profile.release]
 codegen-units = 1

test-cases/front-running/Cargo.toml

@@ -4,7 +4,8 @@ members = ["front-running-*/*"]
 resolver = "2"
 
 [workspace.dependencies]
-soroban-sdk = { version = "=20.0.0" }
+soroban-sdk = { version = "=21.7.6" }
+soroban-token-sdk = { version = "=21.7.6" }
 
 [profile.release]
 codegen-units = 1

test-cases/front-running/front-running-1/remediated-example/Cargo.toml

@@ -7,11 +7,11 @@ version = "0.1.0"
 crate-type = ["cdylib"]
 
 [dependencies]
-soroban-sdk = { version = "20.0.0" }
-soroban-token-sdk = { version = "20.0.0" }
+soroban-sdk = { workspace = true }
+soroban-token-sdk = { workspace = true }
 
 [dev-dependencies]
-soroban-sdk = { version = "20.0.0", features = ["testutils"] }
+soroban-sdk = { workspace = true, features = ["testutils"] }
 
 [features]
 testutils = ["soroban-sdk/testutils"]