Skip to content

Disable CLI comprehension evaluation#555

Open
fallintoplace wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
fallintoplace:fix/disable-cli-comprehension-eval
Open

Disable CLI comprehension evaluation#555
fallintoplace wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
fallintoplace:fix/disable-cli-comprehension-eval

Conversation

@fallintoplace

Copy link
Copy Markdown

What changed

  • stop auto-detecting and evaluating comprehension syntax in PythonicParser.parse_value()
  • make parse_comprehension() reject comprehension input instead of executing it
  • remove the now-unused AST evaluator that could reach raw eval(...)
  • replace the old comprehension success tests with rejection coverage and a regression that proves a comprehension payload stays inert

Why

CLI config values are user-controlled strings. The previous comprehension path parsed the AST and then executed list, dict, and set comprehensions through raw eval(...), which allowed arbitrary code execution from CLI input.

Impact

Comprehension syntax is no longer supported in CLI values. Suspicious comprehension-looking strings now remain inert unless the caller handles them explicitly, and direct calls to parse_comprehension() fail with a clear error.

Checks

  • uv run -- pytest test/cli/test_cli_parser.py
  • uv run --group lint -- ruff check nemo_run/cli/cli_parser.py test/cli/test_cli_parser.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant