chore: address review feedback

Signed-off-by: Ben Selwyn-Smith <benselwynsmith@googlemail.com>

src/macaron/__main__.py

@@ -187,14 +187,6 @@ def perform_action(action_args: argparse.Namespace) -> None:
             sys.exit(verify_policy(action_args))
 
         case "analyze":
-            # If the GitHub token has not already been read, try to read it here.
-            if not global_config.gh_token:
-                gh_token = os.environ.get("GITHUB_TOKEN")
-                if not gh_token:
-                    logger.error("GitHub access token not set.")
-                    sys.exit(os.EX_USAGE)
-                global_config.gh_token = gh_token
-
             # TODO: Here we should try to statically analyze the config before
             # actually running the analysis.
             try:
@@ -227,10 +219,9 @@ def main(argv: list[str] | None = None) -> None:
     if os.path.exists(token_file):
         with open(token_file, encoding="utf-8") as file:
             for line in file:
-                print(f"Line: {line}")
                 if not line or "=" not in line:
                     continue
-                key, value = line.rstrip().split("=")
+                key, value = line.rstrip().split("=", 1)
                 if key and value:
                     if key == "GITHUB_TOKEN":
                         global_config.gh_token = value
@@ -240,7 +231,20 @@ def main(argv: list[str] | None = None) -> None:
                         global_config.gl_self_host_token = value
         # Overwrite file contents as deleting won't work when in a container.
         with open(token_file, "w", encoding="utf-8") as file:
-            file.write("")
+            pass
+    else:
+        # If there is no token file, try to read from environment variables instead.
+        gh_token = os.environ.get("GITHUB_TOKEN")
+        if not gh_token:
+            logger.error("GitHub access token not set.")
+            sys.exit(os.EX_USAGE)
+        global_config.gh_token = gh_token
+        gl_token = os.environ.get("MCN_GITLAB_TOKEN")
+        if gl_token:
+            global_config.gl_token = gl_token
+        gl_self_host_token = os.environ.get("MCN_SELF_HOSTED_GITLAB_TOKEN")
+        if gl_self_host_token:
+            global_config.gl_self_host_token = gl_self_host_token
 
     main_parser = argparse.ArgumentParser(prog="macaron")
 

src/macaron/slsa_analyzer/git_service/gitlab.py

@@ -19,7 +19,6 @@
 """
 
 import logging
-import os
 from abc import abstractmethod
 from collections.abc import Callable
 from urllib.parse import ParseResult, urlunparse
@@ -38,10 +37,9 @@
 class GitLab(BaseGitService):
     """This class contains the spec of the GitLab service."""
 
-    def __init__(self, access_token_env_name: str, token_function: Callable[[], str]) -> None:
+    def __init__(self, token_function: Callable[[], str]) -> None:
         """Initialize instance."""
         super().__init__("gitlab")
-        self.access_token_env_name = access_token_env_name
         self.token_function = token_function
 
     @abstractmethod
@@ -85,8 +83,6 @@ def construct_clone_url(self, url: str) -> str:
         # Construct clone URL from ``urlparse`` result, with or without an access token.
         # https://docs.gitlab.com/ee/gitlab-basics/start-using-git.html#clone-using-a-token
         access_token: str | None = self.token_function()
-        if not access_token:
-            access_token = os.environ.get(self.access_token_env_name)
         if access_token:
             clone_url_netloc = f"oauth2:{access_token}@{self.hostname}"
         else:
@@ -232,7 +228,7 @@ class SelfHostedGitLab(GitLab):
 
     def __init__(self) -> None:
         """Initialize instance."""
-        super().__init__("MCN_SELF_HOSTED_GITLAB_TOKEN", lambda: global_config.gl_self_host_token)
+        super().__init__(lambda: global_config.gl_self_host_token)
 
     def load_defaults(self) -> None:
         """Load the values for this git service from the ini configuration and environment variables.
@@ -253,9 +249,9 @@ def load_defaults(self) -> None:
         if not self.hostname:
             return
 
-        if not (os.environ.get(self.access_token_env_name) or self.token_function()):
+        if not self.token_function():
             raise ConfigurationError(
-                f"Environment variable '{self.access_token_env_name}' is not set "
+                f"Environment variable for '{self.__class__}' is not set "
                 + f"for private GitLab service '{self.hostname}'."
             )
 
@@ -265,7 +261,7 @@ class PubliclyHostedGitLab(GitLab):
 
     def __init__(self) -> None:
         """Initialize instance."""
-        super().__init__("MCN_GITLAB_TOKEN", lambda: global_config.gl_token)
+        super().__init__(lambda: global_config.gl_token)
 
     def load_defaults(self) -> None:
         """Load the values for this git service from the ini configuration and environment variables.

tests/slsa_analyzer/git_service/test_gitlab.py

@@ -26,7 +26,7 @@
 )
 def test_construct_clone_url_without_token(repo_url: str) -> None:
     """Test if the ``construct_clone_url`` method produces proper clone URLs without the access token."""
-    with mock.patch.dict(os.environ, {"MCN_GITLAB_TOKEN": ""}):
+    with mock.patch("macaron.config.global_config.global_config.gl_token", ""):
         clone_url = repo_url
         gitlab = PubliclyHostedGitLab()
         gitlab.load_defaults()
@@ -48,7 +48,7 @@ def test_construct_clone_url_without_token(repo_url: str) -> None:
 )
 def test_construct_clone_url_with_token(repo_url: str, clone_url: str) -> None:
     """Test if the ``construct_clone_url`` method produces proper clone URLs with the access token."""
-    with mock.patch.dict(os.environ, {"MCN_GITLAB_TOKEN": "abcxyz"}):
+    with mock.patch("macaron.config.global_config.global_config.gl_token", "abcxyz"):
         gitlab = PubliclyHostedGitLab()
         gitlab.load_defaults()
         assert gitlab.construct_clone_url(repo_url) == clone_url
@@ -89,7 +89,7 @@ def test_construct_clone_url_for_self_hosted_gitlab(
     # ``setup_test`` fixture.
     load_defaults(user_config_path)
 
-    with mock.patch.dict(os.environ, {"MCN_SELF_HOSTED_GITLAB_TOKEN": "abcxyz"}):
+    with mock.patch("macaron.config.global_config.global_config.gl_self_host_token", "abcxyz"):
         gitlab = SelfHostedGitLab()
         gitlab.load_defaults()
         assert gitlab.construct_clone_url(repo_url) == clone_url
@@ -109,8 +109,7 @@ def test_self_hosted_gitlab_without_env_set(tmp_path: Path) -> None:
     # pollution here, since we reload the ``defaults`` object before every test with the
     # ``setup_test`` fixture.
     load_defaults(user_config_path)
-
-    with mock.patch.dict(os.environ, {"MCN_SELF_HOSTED_GITLAB_TOKEN": ""}):
+    with mock.patch("macaron.config.global_config.global_config.gl_self_host_token", ""):
         gitlab = SelfHostedGitLab()
 
         with pytest.raises(ConfigurationError):
@@ -190,7 +189,7 @@ def test_origin_remote_url_masking(self_hosted_gitlab: Git, expected_origin_url:
     # ``setup_test`` fixture.
     load_defaults(user_config_path)
 
-    with mock.patch.dict(os.environ, {"MCN_SELF_HOSTED_GITLAB_TOKEN": "abcxyz"}):
+    with mock.patch("macaron.config.global_config.global_config.gl_self_host_token", "abcxyz"):
         gitlab = SelfHostedGitLab()
         gitlab.load_defaults()