From 4590c5538069c230166bd7dce14626908738bea4 Mon Sep 17 00:00:00 2001 From: Eric Brown Date: Thu, 28 Mar 2024 10:10:57 -0700 Subject: [PATCH] Check for no action passed in argparse rule The default value for action when no value is passed is "store" which is the exact value we are trying to detect for issues. So if a program creates CLI arguments via add_argument with api-key or password arg and unset action, it needs to surface this as an issue. Signed-off-by: Eric Brown --- .../python/stdlib/argparse_sensitive_info.py | 2 +- .../argparse_add_argument_default_action.py | 17 +++++++++++++++++ .../argparse/test_argparse_sensitive_info.py | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/unit/rules/python/stdlib/argparse/examples/argparse_add_argument_default_action.py diff --git a/precli/rules/python/stdlib/argparse_sensitive_info.py b/precli/rules/python/stdlib/argparse_sensitive_info.py index a885f3e8..33290323 100644 --- a/precli/rules/python/stdlib/argparse_sensitive_info.py +++ b/precli/rules/python/stdlib/argparse_sensitive_info.py @@ -100,7 +100,7 @@ def analyze_call(self, context: dict, call: Call) -> Result: if ( "--password" in [arg0.value_str, arg1.value_str] or "--api-key" in [arg0.value_str, arg1.value_str] - ) and action.value_str == "store": + ) and (action.value is None or action.value_str == "store"): return Result( rule_id=self.id, location=Location(node=call.node), diff --git a/tests/unit/rules/python/stdlib/argparse/examples/argparse_add_argument_default_action.py b/tests/unit/rules/python/stdlib/argparse/examples/argparse_add_argument_default_action.py new file mode 100644 index 00000000..088bf95f --- /dev/null +++ b/tests/unit/rules/python/stdlib/argparse/examples/argparse_add_argument_default_action.py @@ -0,0 +1,17 @@ +# level: ERROR +# start_line: 13 +# end_line: 17 +# start_column: 0 +# end_column: 1 +import argparse + + +parser = argparse.ArgumentParser( + prog="ProgramName", + description="What the program does", +) +parser.add_argument( + "--api-key", + dest="api_key", + help="API key to connect to the server", +) diff --git a/tests/unit/rules/python/stdlib/argparse/test_argparse_sensitive_info.py b/tests/unit/rules/python/stdlib/argparse/test_argparse_sensitive_info.py index 715fa560..85d96cee 100644 --- a/tests/unit/rules/python/stdlib/argparse/test_argparse_sensitive_info.py +++ b/tests/unit/rules/python/stdlib/argparse/test_argparse_sensitive_info.py @@ -39,6 +39,7 @@ def test_rule_meta(self): @parameterized.expand( [ "argparse_add_argument_api_key.py", + "argparse_add_argument_default_action.py", "argparse_add_argument_password.py", "argparse_add_argument_password_file.py", "argparse_add_argument_password_store_true.py",