[BRE-138] - Fix Workflow Name Underscore Error and Enforce Action Ver…

…sion Comment (#30)

* fix workflow name underscore error and enforce action version comment

* Update tests/rules/test_name_capitalized.py

* Update src/bitwarden_workflow_linter/rules/underscore_outputs.py

---------

Co-authored-by: Vince Grassia <[email protected]>

src/bitwarden_workflow_linter/actions.py

@@ -11,11 +11,13 @@
 
 from .utils import Colors, Settings, Action
 
+
 class GitHubApiSchemaError(Exception):
     """A generic Exception to catch redefinitions of GitHub Api Schema changes."""
 
     pass
 
+
 class ActionsCmd:
     """Command to manage the pre-approved list of Actions
 

src/bitwarden_workflow_linter/cli.py

@@ -11,6 +11,7 @@
 
 local_settings = Settings.factory()
 
+
 def main(input_args: Optional[List[str]] = None) -> int:
     """CLI utility to lint GitHub Action Workflows.
 
@@ -48,5 +49,6 @@ def main(input_args: Optional[List[str]] = None) -> int:
 
     return -1
 
+
 if __name__ == "__main__":
     sys.exit(main())

src/bitwarden_workflow_linter/lint.py

@@ -10,6 +10,7 @@
 from .load import WorkflowBuilder, Rules
 from .utils import LintFinding, Settings
 
+
 class LinterCmd:
     """Command to lint GitHub Action Workflow files
 

src/bitwarden_workflow_linter/load.py

@@ -15,11 +15,13 @@
 
 yaml = YAML()
 
+
 class WorkflowBuilderError(Exception):
     """Exception to indicate an error with the WorkflowBuilder."""
 
     pass
 
+
 class WorkflowBuilder:
     """Collection of methods to build Workflow objects."""
 
@@ -82,11 +84,13 @@ def build(
             "The workflow must either be built from a file or from a CommentedMap"
         )
 
+
 class LoadRulesError(Exception):
     """Exception to indicate an error with loading rules."""
 
     pass
 
+
 class Rules:
     """A collection of all of the types of rules.
 

src/bitwarden_workflow_linter/models/job.py

@@ -8,6 +8,7 @@
 
 from .step import Step
 
+
 @dataclass_json(undefined=Undefined.EXCLUDE)
 @dataclass
 class Job:

src/bitwarden_workflow_linter/models/step.py

@@ -6,6 +6,7 @@
 from dataclasses_json import config, dataclass_json, Undefined
 from ruamel.yaml.comments import CommentedMap
 
+
 @dataclass_json(undefined=Undefined.EXCLUDE)
 @dataclass
 class Step:

src/bitwarden_workflow_linter/rule.py

@@ -7,11 +7,13 @@
 from .models.step import Step
 from .utils import LintFinding, LintLevels, Settings
 
+
 class RuleExecutionException(Exception):
     """Exception for the Base Rule class."""
 
     pass
 
+
 class Rule:
     """Base class of a Rule to extend to create a linting Rule."""
 

src/bitwarden_workflow_linter/rules/job_environment_prefix.py

@@ -6,6 +6,7 @@
 from ..rule import Rule
 from ..utils import LintLevels, Settings
 
+
 class RuleJobEnvironmentPrefix(Rule):
     """Rule to enforce specific prefixes for environment variables.
 

src/bitwarden_workflow_linter/rules/name_capitalized.py

@@ -8,6 +8,7 @@
 from ..rule import Rule
 from ..utils import LintLevels, Settings
 
+
 class RuleNameCapitalized(Rule):
     """Rule to enforce all 'name' values start with a capital letter.
 
@@ -50,6 +51,12 @@ def fn(self, obj: Union[Workflow, Job, Step]) -> Tuple[bool, str]:
         capitalized names. This Rule DOES NOT enforce that the name exists.
         It only enforces capitalization IF it does.
         """
-        if obj.name:
-            return obj.name[0].isupper(), self.message
-        return True, ""  # Force passing if obj.name doesn't exist
+        if isinstance(obj, Workflow):
+            if obj.name:
+                if obj.name[0] != "_":
+                    return obj.name[0].isupper(), self.message
+        else:
+            if obj.name:
+                return obj.name[0].isupper(), self.message
+
+        return True, ""  # Force passing

src/bitwarden_workflow_linter/rules/name_exists.py

@@ -8,6 +8,7 @@
 from ..rule import Rule
 from ..utils import LintLevels, Settings
 
+
 class RuleNameExists(Rule):
     """Rule to enforce a 'name' key exists for every object in GitHub Actions.
 

src/bitwarden_workflow_linter/rules/pinned_job_runner.py

@@ -6,6 +6,7 @@
 from ..rule import Rule
 from ..utils import LintLevels, Settings
 
+
 class RuleJobRunnerVersionPinned(Rule):
     """Rule to enforce pinned Runner OS versions.
 

src/bitwarden_workflow_linter/rules/step_approved.py

@@ -6,6 +6,7 @@
 from ..rule import Rule
 from ..utils import LintLevels, Settings
 
+
 class RuleStepUsesApproved(Rule):
     """Rule to enforce that all Actions have been pre-approved.
 

src/bitwarden_workflow_linter/rules/step_pinned.py

@@ -6,6 +6,7 @@
 from ..rule import Rule
 from ..utils import LintLevels, Settings
 
+
 class RuleStepUsesPinned(Rule):
     """Rule to contain the enforcement logic for pinning Actions versions.
 
@@ -94,4 +95,7 @@ def fn(self, obj: Step) -> Tuple[bool, str]:
         if len(ref) != 40:
             return False, "Please use the full commit sha to pin the action"
 
+        if not obj.uses_comment:
+            return False, "Please comment the version of the action commit sha"
+
         return True, ""

src/bitwarden_workflow_linter/rules/underscore_outputs.py

@@ -1,3 +1,5 @@
+""" Rule to enforce all GitHub outputs with more than one words use an underscore."""
+
 import re
 
 from typing import Optional, Union, Tuple
@@ -8,8 +10,9 @@
 from ..models.step import Step
 from ..utils import LintLevels, Settings
 
+
 class RuleUnderscoreOutputs(Rule):
-    """Rule to enforce all GitHub 'outputs' more than one words contain an underscore.
+    """Rule to enforce all GitHub outputs with more than one word use an underscore.
 
     A simple standard to ensure uniformity in naming.
     """
@@ -98,9 +101,11 @@ def fn(self, obj: Union[Workflow, Job, Step]) -> Tuple[bool, str]:
 
         if isinstance(obj, Step):
             if obj.run:
-                outputs.extend(re.findall(
-                    r"\b([a-zA-Z0-9_-]+)\s*=\s*[^=]*>>\s*\$GITHUB_OUTPUT",
-                    obj.run))
+                outputs.extend(
+                    re.findall(
+                        r"\b([a-zA-Z0-9_-]+)\s*=\s*[^=]*>>\s*\$GITHUB_OUTPUT", obj.run
+                    )
+                )
 
         for output_name in outputs:
             if "-" in output_name:

src/bitwarden_workflow_linter/utils.py

@@ -12,6 +12,7 @@
 
 yaml = YAML()
 
+
 @dataclass
 class Colors:
     """Class containing color codes for printing strings to output."""
@@ -25,20 +26,23 @@ class Colors:
     cyan = "36m"
     white = "37m"
 
+
 @dataclass
 class LintLevel:
     """Class to contain the numeric level and color of linting."""
 
     code: int
     color: Colors
 
+
 class LintLevels(LintLevel, Enum):
     """Collection of the different types of LintLevels available."""
 
     NONE = 0, Colors.white
     WARNING = 1, Colors.yellow
     ERROR = 2, Colors.red
 
+
 class LintFinding:
     """Represents a problem detected by linting."""
 
@@ -57,6 +61,7 @@ def __str__(self) -> str:
             f"{self.description}"
         )
 
+
 @dataclass
 class Action:
     """Collection of the metadata associated with a GitHub Action."""
@@ -93,13 +98,16 @@ def __ne__(self, other: Self) -> bool:
         """
         return not self.__eq__(other)
 
+
 class SettingsError(Exception):
     """Custom Exception to indicate an error with loading Settings."""
 
     pass
 
+
 SettingsFromFactory = TypeVar("SettingsFromFactory", bound="Settings")
 
+
 class Settings:
     """Class that contains configuration-as-code for any portion of the app."""
 

tests/rules/test_name_capitalized.py

@@ -49,6 +49,24 @@ def fixture_incorrect_workflow():
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
 
+@pytest.fixture(name="underscore_name_workflow")
+def fixture_underscore_name_workflow():
+    workflow = """\
+name: _test
+on:
+  workflow_dispatch:
+
+jobs:
+  job-key:
+    name: test
+    runs-on: ubuntu-latest
+    steps:
+      - name: test
+        run: echo test
+"""
+    return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
+
+
 @pytest.fixture(name="missing_name_workflow")
 def fixture_missing_name_workflow():
     workflow = """\
@@ -86,6 +104,11 @@ def test_rule_on_incorrect_workflow_name(rule, incorrect_workflow):
     assert result is False
 
 
+def test_rule_on_underscore_name_workflow(rule, underscore_name_workflow):
+    result, _ = rule.fn(underscore_name_workflow)
+    assert result is True
+
+
 def test_rule_on_incorrect_job_name(rule, incorrect_workflow):
     result, _ = rule.fn(incorrect_workflow.jobs["job-key"])
     assert result is False

tests/rules/test_step_pinned.py

@@ -47,6 +47,9 @@ def fixture_incorrect_workflow():
   job-key:
     runs-on: ubuntu-22.04
     steps:
+      - name: Test 3rd Party Action without version pinned
+        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
+
       - name: Test External Branch
         uses: actions/checkout@main
 
@@ -78,20 +81,26 @@ def test_rule_on_correct_workflow(rule, correct_workflow):
     assert result is True
 
 
-def test_rule_on_incorrect_workflow_external_branch(rule, incorrect_workflow):
+def test_rule_on_incorrect_workflow_version_comment(rule, incorrect_workflow):
     result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[0])
     assert result is False
+    assert "the version of the action commit sha" in message
+
+
+def test_rule_on_incorrect_workflow_external_branch(rule, incorrect_workflow):
+    result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[1])
+    assert result is False
     assert "Please pin the action" in message
 
 
 def test_rule_on_incorrect_workflow_hex(rule, incorrect_workflow):
-    result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[1])
+    result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[2])
     assert result is False
     assert "Please use the full commit sha" in message
 
 
 def test_rule_on_incorrect_workflow_internal_commit(rule, incorrect_workflow):
-    result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[2])
+    result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[3])
     assert result is False
     assert "Please pin to main" in message
 

tests/rules/test_underscore_output.py

@@ -141,6 +141,7 @@ def fixture_push_only_workflow():
 """
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
+
 @pytest.fixture(name="misc_workflow")
 def fixture_misc_workflow():
     workflow = """\
@@ -164,6 +165,7 @@ def fixture_misc_workflow():
 """
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
+
 @pytest.fixture(name="no_output_workflow")
 def fixture_no_output_workflow():
     workflow = """\
@@ -184,7 +186,6 @@ def fixture_no_output_workflow():
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
 
-
 @pytest.fixture(name="rule")
 def fixture_rule():
     return RuleUnderscoreOutputs()