Update with linter findings for RuleStepUsesApproved

lint-workflow/src/rules/job_environment_prefix.py

@@ -1,10 +1,10 @@
 """A Rule to enforce prefixes environment variables."""
 from typing import Union, Tuple, List
 
-from ..rule import Rule
 from ..models.job import Job
 from ..models.workflow import Workflow
 from ..models.step import Step
+from ..rule import Rule
 from ..utils import LintLevels, Settings
 
 

lint-workflow/src/rules/name_capitalized.py

@@ -1,10 +1,10 @@
 """A Rule to enforce all 'name' values start with a capital letter."""
-from typing import Union, Tuple
+from typing import Tuple, Union
 
-from ..rule import Rule
 from ..models.workflow import Workflow
 from ..models.job import Job
 from ..models.step import Step
+from ..rule import Rule
 from ..utils import LintLevels, Settings
 
 

lint-workflow/src/rules/name_exists.py

@@ -1,10 +1,10 @@
 """A Rule to enforce that a 'name' exists."""
-from typing import Union, Tuple
+from typing import Tuple, Union
 
-from ..rule import Rule
 from ..models.workflow import Workflow
 from ..models.job import Job
 from ..models.step import Step
+from ..rule import Rule
 from ..utils import LintLevels, Settings
 
 

lint-workflow/src/rules/pinned_job_runner.py

@@ -1,14 +1,28 @@
-from typing import Union, Tuple
+"""A Rule to enforce pinning runners to a specific OS version."""
+from typing import List, Tuple, Union
 
-from ..rule import Rule
 from ..models.job import Job
 from ..models.workflow import Workflow
 from ..models.step import Step
+from ..rule import Rule
 from ..utils import LintLevels, Settings
 
 
 class RuleJobRunnerVersionPinned(Rule):
+    """Rule to enforce pinned Runner OS versions.
+
+    `*-latest` versions updating without knowing has broken all of our worklfows
+    in the past. To avoid this and prevent a single event from breaking the majority of
+    our pipelines, we pin the versions.
+    """
     def __init__(self, settings: Settings = None) -> None:
+        """Constructor for RuleJobRunnerVersionPinned to override Rule class.
+
+        Args:
+          settings:
+            A Settings object that contains any default, overriden, or custom settings
+            required anywhere in the application.
+        """
         self.message = "Workflow runner must be pinned"
         self.on_fail: LintLevels = LintLevels.ERROR
         self.compatibility: List[Union[Workflow, Job, Step]] = [Job]

lint-workflow/src/rules/step_approved.py

@@ -1,15 +1,22 @@
-from typing import Union, Tuple
+"""A Rule to enforce the use of a list of pre-approved Actions."""
+from typing import List, Tuple, Union
 
-from ..rule import Rule
 from ..models.job import Job
 from ..models.workflow import Workflow
 from ..models.step import Step
+from ..rule import Rule
 from ..utils import LintLevels, Settings
 
 
 class RuleStepUsesApproved(Rule):
+    """Rule to enforce that all Actions have been pre-approved.
+
+    To limit the surface area of a supply chain attack in our pipelines, all Actions
+    are required to pass a security review and be added to the pre-approved list to
+    check against.
+    """
     def __init__(self, settings: Settings = None) -> None:
-        self.message = f"error"
+        self.message = "error"
         self.on_fail: LintLevels = LintLevels.WARNING
         self.compatibility: List[Union[Workflow, Job, Step]] = [Step]
         self.settings: Settings = settings
@@ -69,19 +76,14 @@ def fn(self, obj: Step) -> Tuple[bool, str]:
         if self.skip(obj):
             return True, ""
 
-        path, hash = obj.uses.split("@")
-
         # Actions in bitwarden/gh-actions are auto-approved
-        if not path in self.settings.approved_actions:
-            return (
-                False,
-                (
-                    f"New Action detected: {path}\n"
-                    "For security purposes, actions must be reviewed and on the pre-approved list"
-                ),
+        if obj.uses and not obj.uses_path in self.settings.approved_actions:
+            return False, (
+                f"New Action detected: {obj.uses_path}\nFor security purposes, "
+                "actions must be reviewed and on the pre-approved list"
             )
 
-        action = self.settings.approved_actions[path]
+        action = self.settings.approved_actions[obj.uses_path]
 
         if obj.uses_version != action.version or obj.uses_ref != action.sha:
             return False, (

lint-workflow/src/rules/step_pinned.py

@@ -1,9 +1,9 @@
 from typing import Union, Tuple
 
-from ..rule import Rule
 from ..models.job import Job
 from ..models.workflow import Workflow
 from ..models.step import Step
+from ..rule import Rule
 from ..utils import LintLevels, Settings
 
 

lint-workflow/tests/rules/test_pinned_job_runner.py

@@ -1,18 +1,16 @@
+"""Test src/rules/pinned_job_runner.py."""
 import pytest
 
 from ruamel.yaml import YAML
 
-from ..conftest import FIXTURE_DIR
-from ..context import src
-
 from src.load import WorkflowBuilder
 from src.rules.pinned_job_runner import RuleJobRunnerVersionPinned
 
 yaml = YAML()
 
 
-@pytest.fixture
-def correct_runner():
+@pytest.fixture(name="correct_runner")
+def fixture_correct_runner():
     workflow = """\
 ---
 on:
@@ -27,8 +25,8 @@ def correct_runner():
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
 
-@pytest.fixture
-def incorrect_runner():
+@pytest.fixture(name="incorrect_runner")
+def fixture_incorrect_runner():
     workflow = """\
 ---
 on:
@@ -43,16 +41,16 @@ def incorrect_runner():
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
 
-@pytest.fixture
-def rule():
+@pytest.fixture(name="rule")
+def fixture_rule():
     return RuleJobRunnerVersionPinned()
 
 
 def test_rule_on_correct_runner(rule, correct_runner):
-    result, message = rule.fn(correct_runner.jobs["job-key"])
-    assert result == True
+    result, _ = rule.fn(correct_runner.jobs["job-key"])
+    assert result is True
 
 
 def test_rule_on_incorrect_runner(rule, incorrect_runner):
-    result, message = rule.fn(incorrect_runner.jobs["job-key"])
-    assert result == False
+    result, _ = rule.fn(incorrect_runner.jobs["job-key"])
+    assert result is False

lint-workflow/tests/rules/test_step_approved.py

@@ -1,10 +1,8 @@
+"""Test src/rules/step_approved.py."""
 import pytest
 
 from ruamel.yaml import YAML
 
-from ..conftest import FIXTURE_DIR
-from ..context import src
-
 from src.load import WorkflowBuilder
 from src.rules.step_approved import RuleStepUsesApproved
 from src.utils import Settings
@@ -13,8 +11,8 @@
 yaml = YAML()
 
 
-@pytest.fixture
-def settings():
+@pytest.fixture(name="settings")
+def fixture_settings():
     return Settings(
         approved_actions={
             "actions/checkout": {
@@ -31,8 +29,8 @@ def settings():
     )
 
 
-@pytest.fixture
-def correct_workflow():
+@pytest.fixture(name="correct_workflow")
+def fixture_correct_workflow():
     workflow = """\
 ---
 on:
@@ -57,8 +55,8 @@ def correct_workflow():
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
 
-@pytest.fixture
-def incorrect_workflow():
+@pytest.fixture(name="incorrect_workflow")
+def fixture_incorrect_workflow():
     workflow = """\
 ---
 on:
@@ -77,32 +75,32 @@ def incorrect_workflow():
     return WorkflowBuilder.build(workflow=yaml.load(workflow), from_file=False)
 
 
-@pytest.fixture
-def rule(settings):
+@pytest.fixture(name="rule")
+def fixture_rule(settings):
     return RuleStepUsesApproved(settings=settings)
 
 
 def test_rule_on_correct_workflow(rule, correct_workflow):
-    result, message = rule.fn(correct_workflow.jobs["job-key"].steps[0])
-    assert result == True
+    result, _ = rule.fn(correct_workflow.jobs["job-key"].steps[0])
+    assert result is True
 
-    result, message = rule.fn(correct_workflow.jobs["job-key"].steps[1])
-    assert result == True
+    result, _ = rule.fn(correct_workflow.jobs["job-key"].steps[1])
+    assert result is True
 
-    result, message = rule.fn(correct_workflow.jobs["job-key"].steps[2])
-    assert result == True
+    result, _ = rule.fn(correct_workflow.jobs["job-key"].steps[2])
+    assert result is True
 
-    result, message = rule.fn(correct_workflow.jobs["job-key"].steps[3])
-    assert result == True
+    result, _ = rule.fn(correct_workflow.jobs["job-key"].steps[3])
+    assert result is True
 
 
 def test_rule_on_incorrect_workflow(rule, incorrect_workflow):
     result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[0])
-    assert result == False
+    assert result is False
     assert "New Action detected" in message
 
     result, message = rule.fn(incorrect_workflow.jobs["job-key"].steps[1])
-    assert result == False
+    assert result is False
     assert "Action is out of date" in message