Skip to content

Commit 1386c70

Browse files
authored
fix: validate subcommand name before dynamic import (#4620)
Validation for subcommand names in `execute_subcommand` ensures that only alphanumeric characters and underscores are processed. While checking the command execution flow, I noticed that `import_module` was receiving values directly from the scenario configuration without sanitization. This change prevents potential unintended module loading attempts through special characters or path traversal sequences in the subcommand string. Existing tests for command execution remain unchanged, and I added a new test case to verify that invalid subcommand names are correctly rejected with a `MoleculeError`. Verified the changes locally with `ruff` to ensure compliance with the project's linting standards.
1 parent 7a686d0 commit 1386c70

File tree

3 files changed

+27
-2
lines changed

3 files changed

+27
-2
lines changed

src/molecule/command/base.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import fnmatch
2929
import importlib
3030
import logging
31+
import re
3132
import shutil
3233
import subprocess
3334

@@ -416,10 +417,18 @@ def execute_subcommand(
416417
current_config: An instance of a Molecule config.
417418
subcommand_and_args: A string representing the subcommand and arguments.
418419
420+
Raises:
421+
MoleculeError: If an invalid subcommand name is provided.
422+
419423
Returns:
420424
The result of the subcommand.
421425
"""
422426
(subcommand, *args) = subcommand_and_args.split(" ")
427+
428+
if not re.fullmatch(r"[a-zA-Z0-9_-]+", subcommand):
429+
msg = f"Invalid subcommand name: {subcommand}"
430+
raise MoleculeError(msg)
431+
423432
command_module = importlib.import_module(f"molecule.command.{subcommand}")
424433
command = getattr(command_module, text.camelize(subcommand))
425434

src/molecule/exceptions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def __init__(
2929
message: The message to display about the problem.
3030
code: Exit code to use when exiting.
3131
"""
32-
super().__init__()
32+
super().__init__(message)
3333
self.code = code
3434
self.message = message
3535

tests/unit/command/test_base.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131

3232
from molecule import config, util
3333
from molecule.command import base
34-
from molecule.exceptions import ImmediateExit, ScenarioFailureError
34+
from molecule.exceptions import ImmediateExit, MoleculeError, ScenarioFailureError
3535
from molecule.shell import main
3636

3737

@@ -361,6 +361,22 @@ def test_execute_subcommand(config_instance: config.Config) -> None:
361361
assert config_instance.action == "list"
362362

363363

364+
def test_execute_subcommand_invalid(config_instance: config.Config) -> None:
365+
"""Ensure execute_subcommand raises MoleculeError for invalid subcommands.
366+
367+
Args:
368+
config_instance: Mocked config_instance fixture.
369+
370+
Returns:
371+
None
372+
"""
373+
with pytest.raises(MoleculeError, match="Invalid subcommand name"):
374+
base.execute_subcommand(config_instance, "invalid.command")
375+
376+
with pytest.raises(MoleculeError, match="Invalid subcommand name"):
377+
base.execute_subcommand(config_instance, "../../malicious")
378+
379+
364380
def test_execute_scenario(
365381
mocker: MockerFixture,
366382
patched_execute_subcommand: MagicMock,

0 commit comments

Comments
 (0)