Add sast (#24)

* Add SAST; address findings

* update contrib doc

* increase cookie security

* add custom semgrep rules

* add semgrep rule to require auth on handlers

* bump version

* install semgrep in github action

* semgrep rule: use metavars

.github/workflows/make-test.yml

@@ -17,5 +17,5 @@ jobs:
     - uses: actions/setup-python@v4
       with:
         python-version: '3.10'
-    - run: pip install black build pyright usort
+    - run: pip install black build pyright semgrep usort
     - run: make test

CONTRIBUTING.md

@@ -1,13 +1,15 @@
 # Contributing
 
 ## Bug Reports & Feature Requests
+
 Bug reports and feature requests are really helpful. Head over to
 [Issues](https://github.com/Backblaze/boardwalk/issues), and provide
 plenty of detail and context.
 
 ## Development Guidelines
 
 ### Development Dependencies
+
 In addition to the python version specified in the `setup.cfg` and `pip`, you
 will need:
 
@@ -18,9 +20,11 @@ will need:
 - `podman`
 - `pyenv`
 - `pyright` (`pip3 install pyright`)
+- `semgrep`
 - `usort` (`pip3 install usort`)
 
 #### Makefile
+
 The [Makefile](./Makefile) has some useful targets for typical development
 operations, like formatting, building, and locally installing the module.
 
@@ -32,20 +36,24 @@ See the content of the Makefile for other useful targets.
 ### Code Style
 
 #### Automated Formatting
+
 - Run `make format` to format to this codebase's standard.
 
 #### Not Automated Styling
+
 - The last sentence in a code comments, logs, or error messages should not end
   in a period (`.`).
 - Comments should be used generously.
 
 ### Testing
+
 Automated tests are run with `make test`.
 
 Automated tests should be developed for cases that clearly improve Boardwalk's
 reliability, user and developer experience. Otherwise there is no specific
 enforcement of test coverage.
 
 ### Versioning
+
 The boardwalk pip module uses semantic versioning. Please make sure to update
 the VERSION file along with any changes to the package.

Dockerfile

@@ -5,6 +5,10 @@ RUN make build
 
 FROM python:3.10-slim
 COPY --from=build /build/dist ./dist
-RUN python3 -m pip install ./dist/boardwalk-*.whl && mkdir /var/boardwalk && rm -rf ./dist
+RUN groupadd -g 1000 not_root && useradd -u 1000 -g 1000 not_root \
+    && python3 -m pip install ./dist/boardwalk-*.whl \
+    && rm -rf ./dist
+
+USER not_root
 WORKDIR /var/boardwalk
 ENTRYPOINT ["python3", "-m"]

Makefile

@@ -70,7 +70,7 @@ install-web-deps:
 
 # Runs all available tests
 .PHONY: test
-test: test-black test-pyright test-usort
+test: test-black test-pyright test-semgrep test-usort
 
 # Test that code is formatted with black
 .PHONY: test-black
@@ -82,6 +82,16 @@ test-black:
 test-pyright: develop
 	PYRIGHT_PYTHON_FORCE_VERSION=latest pyright
 
+# Perform security static analysis
+.PHONY: test-semgrep
+test-semgrep:
+	semgrep \
+		--config test/semgrep-rules.yml \
+		--config "p/r2c-security-audit" \
+		--config "p/r2c-bug-scan" \
+		--config "p/secrets" \
+		--config "p/dockerfile"
+
 # Ensure imports are formatted in a uniform way
 .PHONY: test-usort
 test-usort:

VERSION

@@ -1 +1 @@
-0.7.3
+0.7.4

src/boardwalk/cli_run.py

@@ -487,7 +487,7 @@ def handle_workflow_catch(workspace: Workspace, hostname: str):
                 ),
             )
         while workspace.caught():
-            time.sleep(5)
+            time.sleep(5)  # nosemgrep: python.lang.best-practice.sleep.arbitrary-sleep
 
     # Now check if there is a remote catch
     def check_boardwalkd_catch(client: WorkspaceClient) -> bool:
@@ -521,7 +521,7 @@ def check_boardwalkd_catch(client: WorkspaceClient) -> bool:
             )
         )
         while check_boardwalkd_catch(boardwalkd_client):
-            time.sleep(5)
+            time.sleep(5)  # nosemgrep: python.lang.best-practice.sleep.arbitrary-sleep
 
 
 def lock_remote_host(host: Host):

src/boardwalkd/protocol.py

@@ -263,7 +263,7 @@ def workspace_heartbeat_keepalive(self, workspace_name: str) -> None:
                 socket.gaierror,
             ):
                 pass
-            time.sleep(5)
+            time.sleep(5)  # nosemgrep: python.lang.best-practice.sleep.arbitrary-sleep
 
     def workspace_heartbeat_keepalive_connect(
         self, workspace_name: str

src/boardwalkd/server.py

@@ -80,11 +80,14 @@ def ui_method_sort_events_by_date(
 class AnonymousLoginHandler(UIBaseHandler):
     """Handles "logging in" the UI when no auth is actually configured"""
 
+    # nosemgrep: test.boardwalk.python.security.handler-method-missing-authentication
     async def get(self):  # pyright: reportIncompatibleMethodOverride=false
         self.set_secure_cookie(
             "boardwalk_user",
             "[email protected]",
             expires_days=self.settings["auth_expire_days"],
+            samesite="Strict",
+            secure=True,
         )
         return self.redirect(
             self.get_query_argument("next", "/")
@@ -97,6 +100,7 @@ class GoogleOAuth2LoginHandler(UIBaseHandler, tornado.auth.GoogleOAuth2Mixin):
 
     url_encryption_key = Fernet.generate_key()
 
+    # nosemgrep: test.boardwalk.python.security.handler-method-missing-authentication
     async def get(self, *args: Any, **kwargs: Any):
         try:
             # If the request is sent along with a code, then we assume the code
@@ -118,6 +122,8 @@ async def get(self, *args: Any, **kwargs: Any):
                 "boardwalk_user",
                 user["email"],
                 expires_days=self.settings["auth_expire_days"],
+                samesite="Strict",
+                secure=True,
             )
 
             # We attempt to redirect back to the original URL the user was browsing
@@ -381,6 +387,7 @@ class AuthApiDenied(APIBaseHandler):
     """Dedicated handler for redirecting an unauthenticated user to an 'access
     denied' endpoint"""
 
+    # nosemgrep: test.boardwalk.python.security.handler-method-missing-authentication
     def get(self):
         return self.send_error(403)
 
@@ -591,6 +598,7 @@ def make_server(
         "url": url,
         "websocket_ping_interval": 10,
         "xsrf_cookies": True,
+        "xsrf_cookie_kwargs": {"samesite": "Strict", "secure": True},
     }
     if develop:
         settings["debug"] = True

test/semgrep-rules.yml

@@ -0,0 +1,70 @@
+---
+rules:
+- id: boardwalk.python.security.insecure-set-secure-cookie
+  severity: ERROR
+  languages:
+  - python
+  message: |
+    UIBaseHandler.set_secure_cookie() should have additional arguments passed to
+    further enhance the security of cookies
+
+    Example:
+    self.set_secure_cookie("boardwalk_user",
+    "[email protected]",
+    expires_days=self.settings["auth_expire_days"],
+    samesite="Strict", secure=True)
+  patterns:
+  - pattern: |
+      class $C(UIBaseHandler):
+          def $F(...):
+            ...
+            self.set_secure_cookie(...)
+            ...
+  - pattern-not: |
+      class $C(UIBaseHandler):
+        def $F(...):
+          ...
+          self.set_secure_cookie(
+             samesite="Strict",
+             secure=True,
+            expires_days=...,
+            ...
+          )
+          ...
+
+- id: boardwalk.python.security.handler-method-missing-authentication
+  severity: ERROR
+  languages:
+  - python
+  message: |
+    UIBaseHandler or APIBaseHandler HTTP methods must require authentication
+    by using the @tornado.web.authenticated decorator, unless they have been
+    intentionally excluded
+  patterns:
+  - metavariable-pattern:
+      metavariable: $HANDLER
+      patterns:
+      - pattern-either:
+        - pattern: UIBaseHandler
+        - pattern: APIBaseHandler
+  - metavariable-pattern:
+      metavariable: $METHOD
+      patterns:
+      - pattern-either:
+        - pattern: head
+        - pattern: get
+        - pattern: post
+        - pattern: delete
+        - pattern: patch
+        - pattern: put
+        - pattern: options
+  - pattern-inside: |
+      class $C($HANDLER):
+          ...
+  - pattern: |
+      def $METHOD(...):
+          ...
+  - pattern-not: |
+      @tornado.web.authenticated
+      def $METHOD(...):
+          ...