From 0601faac50db9f747a3a525c9fa8d3a656a6506a Mon Sep 17 00:00:00 2001 From: Yoland Y <4950057+yoland68@users.noreply.github.com> Date: Thu, 19 Dec 2024 15:21:36 -0800 Subject: [PATCH] refactor: enhance security checks in publish command and update tests Updated the security check implementation in the publish command to use 'ruff' with an '--exit-zero' flag, changing the output handling to display warnings instead of errors. Adjusted the test cases to reflect these changes, ensuring that security warnings are correctly asserted. This improves the clarity of security feedback during the publishing process. --- comfy_cli/command/custom_nodes/command.py | 12 +++++++----- tests/comfy_cli/command/nodes/test_publish.py | 11 ++++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/comfy_cli/command/custom_nodes/command.py b/comfy_cli/command/custom_nodes/command.py index b126476..9e2c8f6 100644 --- a/comfy_cli/command/custom_nodes/command.py +++ b/comfy_cli/command/custom_nodes/command.py @@ -709,14 +709,16 @@ def publish( # Run security checks first typer.echo("Running security checks...") try: - # Run ruff check with security rules - cmd = ["ruff", "check", ".", "--select", "S102,S307"] + # Run ruff check with security rules and --exit-zero to only warn + cmd = ["ruff", "check", ".", "--select", "S102,S307", "--exit-zero"] result = subprocess.run(cmd, capture_output=True, text=True) - if result.returncode != 0: - print("[red]Security issues found:[/red]") + if result.stdout: # Changed from checking returncode to checking if there's output + print("[yellow]Security warnings found:[/yellow]") # Changed from red to yellow to indicate warning print(result.stdout) - raise typer.Exit(code=1) + print("[bold yellow]We will soon disable exec and eval, so this will be an error soon.[/bold yellow]") + # TODO: re-enable exit when we disable exec and eval + # raise typer.Exit(code=1) except FileNotFoundError: print("[red]Ruff is not installed. Please install it with 'pip install ruff'[/red]") diff --git a/tests/comfy_cli/command/nodes/test_publish.py b/tests/comfy_cli/command/nodes/test_publish.py index 7425835..5b9d2c6 100644 --- a/tests/comfy_cli/command/nodes/test_publish.py +++ b/tests/comfy_cli/command/nodes/test_publish.py @@ -13,11 +13,16 @@ def test_publish_fails_on_security_violations(): mock_result.returncode = 1 mock_result.stdout = "S102 Use of exec() detected" - with patch("subprocess.run", return_value=mock_result): + with ( + patch("subprocess.run", return_value=mock_result), + patch("typer.prompt", return_value="test-token"), + ): result = runner.invoke(app, ["publish"]) - assert result.exit_code == 1 - assert "Security issues found" in result.stdout + # TODO: re-enable exit when we disable exec and eval + # assert result.exit_code == 1 + # assert "Security issues found" in result.stdout + assert "Security warnings found" in result.stdout def test_publish_continues_on_no_security_violations():