[pylint] Test No Secrets

tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md

@@ -1,5 +1,8 @@
 # Release History
 
+## 0.5.8 (Unreleased)
+- Add `do-not-store-secrets-in-test-variables` check
+
 ## 0.5.7 (2025-07-15)
 - Bug fix for `do-not-use-logging-exception` checker
 

tools/pylint-extensions/azure-pylint-guidelines-checker/README.md

@@ -2,7 +2,7 @@
 
 ## Overview
 
-This package contains custom pylint checkers for Azure SDK guidelines. The custom checkers have message codes in the range C4717 - C4772.
+This package contains custom pylint checkers for Azure SDK guidelines. The custom checkers have message codes in the range C4717 - C4773.
 You can identify a custom checker by the link to the guidelines included in its message.
 
 
@@ -65,7 +65,7 @@ You can identify a custom checker by the link to the guidelines included in its
 | C4770 | missing-logging-policy | You should include a LoggingPolicy in your HTTP pipeline. | # pylint:disable=missing-logging-policy | [Network operations](https://azure.github.io/azure-sdk/python_implementation.html#network-operations) | [Example](code_examples.md#missing-logging-policy) |
 | C4771 | missing-retry-policy | You should include a RetryPolicy in your HTTP pipeline. | # pylint:disable=missing-retry-policy | [Network operations](https://azure.github.io/azure-sdk/python_implementation.html#network-operations) | [Example](code_examples.md#missing-retry-policy) |
 | C4772 | missing-distributed-tracing-policy | You should include a DistributedTracingPolicy in your HTTP pipeline. | # pylint:disable=missing-distributed-tracing-policy | [Network operations](https://azure.github.io/azure-sdk/python_implementation.html#network-operations) | [Example](code_examples.md#missing-distributed-tracing-policy) |
-| C4773 | do-not-use-logging-exception | Do not use Exception level logging, this is an error log and may leak sensitive information. Use another logging level instead. | # pylint:disable=do-not-use-logging-exception | No Link. | [Example](code_examples.md#do-not-use-logging-exception) |
+| C4773 | do-not-store-secrets-in-test-variables | Do not assign secrets to local variables in test files. They will be exposed if the test fails. Use the secret directly in the function call instead i.e foo(credential=bar.secret). | # pylint:disable=do-not-store-secrets-in-test-variables | [Security](https://azure.github.io/azure-sdk/python_implementation.html#security) | [Example](code_examples.md#do-not-store-secrets-in-test-variables) |
 
 ## How to disable a pylint error (do not do this without permission from an azure sdk team member)
 

tools/pylint-extensions/azure-pylint-guidelines-checker/dev_requirements.txt

@@ -1,3 +1,3 @@
-pylint==3.3.6
+pylint==4.0.4
 azure-core
 pytest==7.1.1

tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py

@@ -4,9 +4,9 @@
 # ------------------------------------
 
 """
-Pylint custom checkers for SDK guidelines: C4717 - C4767
+Pylint custom checkers for SDK guidelines: C4717 - C4773
 """
-
+import os
 import logging
 import astroid
 from pylint.checkers import BaseChecker
@@ -3266,6 +3266,101 @@ def visit_functiondef(self, node):
     # this line makes it work for async functions
     visit_asyncfunctiondef = visit_functiondef
 
+
+class DoNotStoreSecretsInTestVariables(BaseChecker):
+    """Rule to check that secrets are not assigned to variables in test files."""
+
+    name = "do-not-store-secrets-in-test-variables"
+    priority = -1
+    msgs = {
+        "C4773": (
+            "Do not assign secrets to local variables in test files. They will be exposed if the test fails. Use the secret directly in the function call instead i.e foo(credential=bar.secret).",
+            "do-not-store-secrets-in-test-variables",
+            "Do not assign secrets to local variables in test files. They will be exposed if the test fails. Use the secret directly in the function call instead i.e foo(credential=bar.secret).",
+        ),
+    }
+
+    def __init__(self, linter=None):
+        super(DoNotStoreSecretsInTestVariables, self).__init__(linter)
+        self.secret_variables = set()  # Track variables that hold secrets
+
+    def _is_test_file(self, node):
+        """Check if the current file is a test file."""
+        try:
+            filename = node.root().file
+            if not filename:
+                return False
+
+            # Get just the basename (filename without path) for testing
+            basename = os.path.basename(filename).lower()
+
+            # Check if it's a test file - specifically files that start with test_
+            is_test = basename.startswith('test_')
+            return is_test
+        except (AttributeError, TypeError, ImportError):
+            return False
+
+    def _is_secret_access(self, node):
+        """Check if a node accesses a .secret attribute."""
+        try:
+            if hasattr(node, 'attrname') and node.attrname == 'secret':
+                return True
+        except:
+            logging.info("Failed to determine if node accesses a secret attribute.")
+        return False
+
+    def visit_assign(self, node):
+        """Check for variable assignments that store secrets."""
+        if not self._is_test_file(node):
+            return
+
+        try:
+            # Check if we're assigning a secret to a variable
+            if self._is_secret_access(node.value):
+                # Get the variable name being assigned to
+                for target in node.targets:
+                    if hasattr(target, 'name'):
+                        var_name = target.name
+                        self.secret_variables.add(var_name)
+                        self.add_message(
+                            msgid="do-not-store-secrets-in-test-variables",
+                            node=node,
+                            confidence=None,
+                        )
+        except:
+            logging.info("Failed to check for secret variable assignment.")
+
+    def visit_call(self, node):
+        """Check for usage of secret variables in function calls."""
+        if not self._is_test_file(node):
+            return
+
+        try:
+            # Check function arguments for secret variables
+            for arg in node.args:
+                if hasattr(arg, 'name') and arg.name in self.secret_variables:
+                    self.add_message(
+                        msgid="do-not-store-secrets-in-test-variables", 
+                        node=arg,
+                        confidence=None,
+                    )
+
+            # Check keyword arguments
+            for keyword in node.keywords:
+                if hasattr(keyword.value, 'name') and keyword.value.name in self.secret_variables:
+                    self.add_message(
+                        msgid="do-not-store-secrets-in-test-variables",
+                        node=keyword.value,
+                        confidence=None,
+                    )
+        except AttributeError:
+            logging.info("Failed to check for secret variable usage in function.")
+
+    def leave_module(self, node):
+        """Reset secret variables when leaving a module."""
+        self.secret_variables.clear()
+
+
 class DoNotUseLoggingException(BaseChecker):
     """Rule to check that exceptions aren't logged using exception method"""
 
@@ -3408,3 +3503,4 @@ def register(linter):
     # linter.register_checker(ClientLROMethodsUseCorePolling(linter))
     # linter.register_checker(ClientLROMethodsUseCorrectNaming(linter))
     linter.register_checker(DoNotUseLoggingException(linter))
+    linter.register_checker(DoNotStoreSecretsInTestVariables(linter))

tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py

@@ -6,7 +6,7 @@
 
 setup(
     name="azure-pylint-guidelines-checker",
-    version="0.5.7",
+    version="0.5.8",
     url="http://github.com/Azure/azure-sdk-for-python",
     license="MIT License",
     description="A pylint plugin which enforces azure sdk guidelines.",

tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/__init__.py

@@ -1,7 +1,11 @@
 # Test for CheckNamingMismatchGeneratedCode
 
-from something import Something
-from something2 import something2 as somethingTwo
+# Dummy imports for testing purposes
+class Something:
+    pass
+
+class somethingTwo:
+    pass
 
 __all__ = (
     Something,

tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_files/test_do_not_store_secrets_in_test_variables.py

@@ -0,0 +1,107 @@
+# Test file for DoNotStoreSecretsInTestVariables checker
+
+# Mock objects for testing
+class MockClient:
+    @property
+    def secret(self):
+        return "mock_secret"
+
+    def get_data(self):
+        return "mock_data"
+
+    @property 
+    def secret_config(self):
+        return "mock_config"
+
+class MockAuth:
+    @property
+    def secret(self):
+        return "mock_auth_secret"
+
+class MockService:
+    @property
+    def secret(self):
+        return "mock_service_secret"
+
+class MockItem:
+    @property
+    def secret(self):
+        return "mock_item_secret"
+
+class MockConfig:
+    def __init__(self):
+        self.auth = MockAuth()
+
+# Mock variables
+my_client = MockClient()
+client = MockClient()
+auth = MockAuth()
+config = MockConfig()
+service = MockService()
+condition = True
+items = [MockItem(), MockItem()]
+
+# Mock functions
+def some_function(arg):
+    pass
+
+def other_function(param=None):
+    pass
+
+def function_call(arg1, arg2):
+    pass
+
+def another_call(param=None):
+    pass
+
+def process(arg):
+    pass
+
+def handle(arg):
+    pass
+
+# This should trigger the rule
+def test_bad_secret_usage():
+    # This assigns a secret to a variable (should be flagged)
+    secret_value = my_client.secret
+
+    # Using the secret variable (should also be flagged) 
+    some_function(secret_value)
+
+    # Also check keyword args
+    other_function(param=secret_value)
+
+# Multiple secret assignments
+def test_multiple_secrets():
+    secret1 = client.secret
+    secret2 = auth.secret
+    secret3 = config.auth.secret
+
+    # Using the secret variables
+    function_call(secret1, secret2)
+    another_call(param=secret3)
+
+# Secret assignments in different contexts
+def test_secret_in_contexts():
+    # In if statement
+    if condition:
+        temp_secret = service.secret
+        process(temp_secret)
+
+    # In for loop
+    for item in items:
+        loop_secret = item.secret
+        handle(loop_secret)
+
+# This should NOT trigger the rule
+def test_good_secret_usage():
+    # Direct usage is preferred
+    some_function(my_client.secret)
+    other_function(param=my_client.secret)
+
+    # Non-secret variables are fine
+    normal_value = my_client.get_data()
+    some_function(normal_value)
+
+    # Other attributes ending with 'secret' but not actually secret
+    not_secret = my_client.secret_config  # This won't trigger since it's not .secret