Skip to content

fix(cli): escape metadata in generated scripts - #147

Open
hannibal-lee wants to merge 1 commit into
Andyyyy64:mainfrom
hannibal-lee:fix/escape-generated-script-metadata
Open

fix(cli): escape metadata in generated scripts#147
hannibal-lee wants to merge 1 commit into
Andyyyy64:mainfrom
hannibal-lee:fix/escape-generated-script-metadata

Conversation

@hannibal-lee

Copy link
Copy Markdown
Contributor

Summary

  • encode Hugging Face model IDs, GGUF filenames, and quantization types as
    Python literals before embedding them in generated scripts
  • apply the same escaping to both whichllm run and whichllm snippet
  • add regression tests for metadata containing quotes and Python-like text

Why

whichllm run generates a temporary Python script from Hugging Face metadata
and executes it. Direct string interpolation could produce invalid Python or
allow specially crafted metadata to alter the generated code structure.

Using repr() through the !r conversion preserves the original value while
emitting a correctly escaped Python string literal. Normal model IDs and
filenames keep the same runtime behavior.

Testing

  • uv run --no-dev --with pytest pytest -q (468 passed)
  • uvx ruff check .
  • uvx ruff format --check .

@Andyyyy64 Andyyyy64 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, really sorry this took me so long to get to. Thanks for waiting!

I finally went through this locally. With a safe AST-only payload, main produced an extra top-level assignment from the model ID, while this branch kept the same payload as the model_id string literal.

I also checked both the GGUF and Transformers paths with quotes and newlines. all 468 tests pass, along with Ruff 0.15.22 check and format, and compileall.

nothing blocking on my side. Thanks again for fixing this!

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.

2 participants