From 28b0ef7332bc92df953f336d87b99441a2b53626 Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Sat, 27 Jun 2026 11:03:56 +0200 Subject: [PATCH 1/2] fix: validate rule path only when used --- quark/cli.py | 13 ++++++ tests/test_cli.py | 100 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 tests/test_cli.py diff --git a/quark/cli.py b/quark/cli.py index 124b82793..3b7a96373 100644 --- a/quark/cli.py +++ b/quark/cli.py @@ -205,6 +205,12 @@ def entry_point( rule_path_list = [rule_filter] else: + if not os.path.exists(rule): + raise click.BadParameter( + f"Path {rule!r} does not exist.", + param_hint="--rule", + ) + rule_path_list = [ os.path.join(dir_path, file) for dir_path, _, file_list in os.walk(rule) @@ -463,6 +469,13 @@ def entry_point( data.close() +# Click validates option defaults before the command body can choose a rule source. +for param in entry_point.params: + if param.name == "rule": + param.type = click.Path(exists=False, file_okay=True, dir_okay=True) + break + + def update_rule_buffer(rule_buffer_list, rule_path_list): for rule_path in rule_path_list: with open(rule_path, "r") as json_file: diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 000000000..a4a39568b --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,100 @@ +import json +from unittest.mock import patch + +from click.testing import CliRunner + +from quark.cli import entry_point + + +def _rule_option(): + for param in entry_point.params: + if param.name == "rule": + return param + raise AssertionError("rule option not found") + + +def _write_rule(path): + path.write_text( + json.dumps( + { + "crime": "test", + "permission": [], + "api": [ + { + "class": "Lfoo/Bar;", + "method": "first", + "descriptor": "()V", + }, + { + "class": "Lfoo/Bar;", + "method": "second", + "descriptor": "()V", + }, + ], + "score": 1, + "label": ["test"], + } + ) + ) + + +def test_custom_rule_does_not_validate_missing_default_rules(tmp_path, monkeypatch): + missing_default_rules = tmp_path / "missing-rules" + custom_rule = tmp_path / "custom_rule.json" + apk = tmp_path / "sample.apk" + _write_rule(custom_rule) + apk.write_text("") + monkeypatch.setattr(_rule_option(), "default", str(missing_default_rules)) + + runner = CliRunner() + + with patch("quark.cli.Quark") as mock_quark: + data = mock_quark.return_value + data.quark_analysis.score_sum = 0 + data.quark_analysis.weight_sum = 0 + data.quark_analysis.summary_report_table = "" + + result = runner.invoke( + entry_point, + ["-a", str(apk), "-s", str(custom_rule)], + ) + + assert result.exit_code == 0 + mock_quark.assert_called_once() + + +def test_missing_rules_path_fails_when_rules_are_loaded(tmp_path, monkeypatch): + missing_rules = tmp_path / "missing-rules" + apk = tmp_path / "sample.apk" + apk.write_text("") + monkeypatch.setattr(_rule_option(), "default", str(missing_rules)) + + runner = CliRunner() + + result = runner.invoke(entry_point, ["-a", str(apk), "-s"]) + + assert result.exit_code != 0 + assert "Path" in result.output + assert str(missing_rules) in result.output + + +def test_existing_rules_directory_is_loaded(tmp_path, monkeypatch): + rules_dir = tmp_path / "rules" + rules_dir.mkdir() + _write_rule(rules_dir / "custom_rule.json") + apk = tmp_path / "sample.apk" + apk.write_text("") + monkeypatch.setattr(_rule_option(), "default", str(rules_dir)) + + runner = CliRunner() + + with patch("quark.cli.Quark") as mock_quark: + data = mock_quark.return_value + data.quark_analysis.score_sum = 0 + data.quark_analysis.weight_sum = 0 + data.quark_analysis.summary_report_table = "" + + result = runner.invoke(entry_point, ["-a", str(apk), "-s"]) + + assert result.exit_code == 0 + mock_quark.assert_called_once() From efa3c5470bf6139c624042ec211f30b85de62b1e Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Sun, 5 Jul 2026 10:03:08 +0200 Subject: [PATCH 2/2] Address review: simplify rule option validation and dedupe test setup - Set exists=False directly on the --rule option instead of patching the parameter after the command is defined. - Use the plain path in the missing-rule error so the assertion is portable on Windows (repr escapes backslashes). - Extract the duplicated Quark mock into a mock_quark fixture. --- quark/cli.py | 11 ++--------- tests/test_cli.py | 39 ++++++++++++++++++++------------------- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/quark/cli.py b/quark/cli.py index 3b7a96373..8d607eaf1 100644 --- a/quark/cli.py +++ b/quark/cli.py @@ -66,7 +66,7 @@ "-r", "--rule", help="Rules directory", - type=click.Path(exists=True, file_okay=True, dir_okay=True), + type=click.Path(exists=False, file_okay=True, dir_okay=True), default=f"{config.DIR_PATH}", required=False, show_default=True, @@ -207,7 +207,7 @@ def entry_point( else: if not os.path.exists(rule): raise click.BadParameter( - f"Path {rule!r} does not exist.", + f"Path {rule} does not exist.", param_hint="--rule", ) @@ -469,13 +469,6 @@ def entry_point( data.close() -# Click validates option defaults before the command body can choose a rule source. -for param in entry_point.params: - if param.name == "rule": - param.type = click.Path(exists=False, file_okay=True, dir_okay=True) - break - - def update_rule_buffer(rule_buffer_list, rule_path_list): for rule_path in rule_path_list: with open(rule_path, "r") as json_file: diff --git a/tests/test_cli.py b/tests/test_cli.py index a4a39568b..221bf1892 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,11 +1,22 @@ import json from unittest.mock import patch +import pytest from click.testing import CliRunner from quark.cli import entry_point +@pytest.fixture +def mock_quark(): + with patch("quark.cli.Quark") as mock: + data = mock.return_value + data.quark_analysis.score_sum = 0 + data.quark_analysis.weight_sum = 0 + data.quark_analysis.summary_report_table = "" + yield mock + + def _rule_option(): for param in entry_point.params: if param.name == "rule": @@ -38,7 +49,9 @@ def _write_rule(path): ) -def test_custom_rule_does_not_validate_missing_default_rules(tmp_path, monkeypatch): +def test_custom_rule_does_not_validate_missing_default_rules( + tmp_path, monkeypatch, mock_quark +): missing_default_rules = tmp_path / "missing-rules" custom_rule = tmp_path / "custom_rule.json" apk = tmp_path / "sample.apk" @@ -48,16 +61,10 @@ def test_custom_rule_does_not_validate_missing_default_rules(tmp_path, monkeypat runner = CliRunner() - with patch("quark.cli.Quark") as mock_quark: - data = mock_quark.return_value - data.quark_analysis.score_sum = 0 - data.quark_analysis.weight_sum = 0 - data.quark_analysis.summary_report_table = "" - - result = runner.invoke( - entry_point, - ["-a", str(apk), "-s", str(custom_rule)], - ) + result = runner.invoke( + entry_point, + ["-a", str(apk), "-s", str(custom_rule)], + ) assert result.exit_code == 0 mock_quark.assert_called_once() @@ -78,7 +85,7 @@ def test_missing_rules_path_fails_when_rules_are_loaded(tmp_path, monkeypatch): assert str(missing_rules) in result.output -def test_existing_rules_directory_is_loaded(tmp_path, monkeypatch): +def test_existing_rules_directory_is_loaded(tmp_path, monkeypatch, mock_quark): rules_dir = tmp_path / "rules" rules_dir.mkdir() _write_rule(rules_dir / "custom_rule.json") @@ -88,13 +95,7 @@ def test_existing_rules_directory_is_loaded(tmp_path, monkeypatch): runner = CliRunner() - with patch("quark.cli.Quark") as mock_quark: - data = mock_quark.return_value - data.quark_analysis.score_sum = 0 - data.quark_analysis.weight_sum = 0 - data.quark_analysis.summary_report_table = "" - - result = runner.invoke(entry_point, ["-a", str(apk), "-s"]) + result = runner.invoke(entry_point, ["-a", str(apk), "-s"]) assert result.exit_code == 0 mock_quark.assert_called_once()