Repository navigation
Fix audited defects: code generation escaping, env template injection, unsafe file writes, cli exit codes - #54
Merged
Merged
Conversation
Requirements were parsed by hand: one operator, matched by substring, and the installed versions came from `pip freeze`. As a result - `PyYAML>=6` raised KeyError (the name was lowercased for one lookup only), - `requests>=2,<3` raised InvalidVersion, - `click~=8.0`, `typer[all]>=0.9`, `rich_click` and requirements with an environment marker were reported as missing, - packages that `pip freeze` does not print as `name==version` (editable installs) or environments without pip could not be validated at all. Requirements are now parsed with packaging.requirements.Requirement and checked against importlib.metadata, with PEP 503 name normalization. The error message also gets its missing line break. parse_requirement, compare_versions, RequirementSpec and OPERATOR_MAP are replaced by check_requirement; their tests are ported to it case by case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Help texts, versions, vars, defaults, aliases and command config were pasted between quotes in the generated source. A quote, a backslash or a line break in any of them produced a SyntaxError, silently changed the value (`C:\temp\new` became a tab and a line break), or ran as code when the CLI was imported. Every string now goes through py_str()/repr(). Names are turned into identifiers that are safe to define: - commands named like a keyword, a builtin or a name the generated module uses itself (`import`, `list`, `print`, `cli`, `typer`, `2fa`) no longer break the module or shadow what other commands call, - groups and click aliases with a dash (`my-group.sub`, `rm-all`) work, - a param that is not a valid Python variable name is rejected with a clear message instead of a SyntaxError at load time. Param handling: - global_params are applied to commands without params of their own, - a default may contain `=` (`--who: str = "a=b"`), - `name: str = typer.Option(...)` keeps its typer definition instead of being reduced to `name: str`, - `--dry-run|-d` keeps the option name `--dry-run` (was `--dry_run`), - a non-str default of a str param no longer raises AttributeError. Greedy `(*)` commands now get their template and `|` aliases, are attached to the right nested group, and accept non-str defaults. Click generator: `--name|-n`, `name: --who`, dashed options and arguments and global params generate working decorators; bool options are `--flag/--no-flag` switches as in the Typer generator. ClickParser now extends Parser instead of copying it. The base imports are written in sorted order, so the generated code no longer differs from run to run. A script-less `a.b` command that only carries the help of the nested group `a.b` no longer generates an empty function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The manifest `name` becomes the file name of the generated module, of the entry script in the interpreter's bin directory and of the metadata file, and it is written into the import line of that script. Nothing checked it: `name: ../../evil` wrote files outside those directories, and a name with a space, a dot or a line break produced a script that cannot run. Names are now limited to letters, digits, `_` and `-`, starting with a letter or `_`, which covers every name that could be loaded and run so far. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ifests
`env` was put into the vars that are interpolated on every load, so the
whole of os.environ was rendered by Jinja as if it were a template:
- an environment variable holding `{{ ... }}` was evaluated, which is
template injection through the environment of whoever runs `cli load`,
- one holding `{%` (a zsh prompt, for example) made every manifest fail
with TemplateSyntaxError,
- the result replaced `env` with a string, so `{{ env['X'] }}` only
worked inside `vars` and rendered empty everywhere else.
`env` is now handed to the templates as the mapping it is, for `vars` and
for the rest of the manifest alike.
An empty manifest, a manifest that is a list or a scalar, and an empty
`vars:` section now end in an error message instead of an
AttributeError/TypeError traceback. The manifest is read as UTF-8.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… cycles - Relative include paths were resolved against the working directory, so a manifest only loaded from the directory it was written for. They are now looked up next to the including manifest first; the working directory stays as a fallback for existing manifests. - The includes went through a set() before merging. The first manifest to define a key wins, so with conflicting includes the result depended on the hash seed of the process. The documented order is now kept. - Two manifests including each other recursed until RecursionError. The chain of manifests being resolved is tracked and a cycle is reported. - A missing include is reported with the manifest that asked for it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Files were opened with the locale encoding. With LANG=C (containers, cron, CI) `cli load` failed with UnicodeEncodeError as soon as a manifest held a non-ASCII character, and a manifest could be decoded differently by the two passes that read it. Python reads the generated source as UTF-8, so that is what gets written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `cli rm '*'` (or any name with glob characters) matched the metadata of other CLIs and deleted it, leaving their scripts and modules behind with nothing left to remove them by. Names are now matched literally. - remove_metadata raised OSError when the metadata directory held another file (a .DS_Store, for example) after the metadata file was already deleted. The directory is only removed once it is empty. - get_metadata_bypath never closed the metadata file. - Metadata and manifests are read as UTF-8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`cli load` writes an entry script named after the CLI into the bin directory of the interpreter. A manifest named `python`, `pip` or after any other installed tool replaced that executable: bin/python is a symlink, so the write went through it and overwrote the interpreter itself, and `cli rm` deleted it afterwards. Loading now stops with an error when the target exists and is not a cliffy entry script, and unloading only removes cliffy entry scripts. unload_cli also wrapped both removals in one suppress(): when the script was already gone, the generated module was never removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A zipapp built from a `use_click` manifest only bundled typer and failed with "No module named 'rich_click'" on any machine that does not have rich-click installed. The framework is now read from the generated code. - The temporary script was only deleted on success: a failed build or a generated module that failed to import left it in the temp directory. - `cli build -o a/b` failed when `a` did not exist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `cli test` exited 0 when tests failed, also with `-x`, so it could not gate anything in CI. It now exits 1 unless every test passed. - `cli test` errored on every test of a `use_click` manifest: typer's runner cannot invoke a click group. The click runner is used for those. - `cli validate` exited 0 for a manifest that failed code generation. - `cli build`, `cli update` and `cli remove` exited 0 after printing an error for a CLI that is not loaded or a build that failed. They still process the remaining arguments, then exit 1. - `cli build <loaded-cli>` looked for `my-cli.py`, but the module of a loaded CLI is stored as `my_cli.py`. - `cli docs <loaded-cli>` validated the raw manifest text as a model and always failed; an unknown name printed nothing and exited 0. - `cli init` printed a traceback for a name the template rejects, and `--raw` accepted names that can never be loaded. - The tester left its temporary module behind when the import failed. - `exit()` (a site builtin that is missing under `python -S`) is no longer used; `__main__` had an unreachable one. Four existing assertions that pinned exit code 0 for those error cases are updated to 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The changed path was compared with endswith(), so watching `x.yaml` also reloaded on changes to `other-x.yaml` in the same directory. Paths are compared in full. - reload() never closed the manifest it opened, once per reload. - The observer thread was only stopped on KeyboardInterrupt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without rich-click, Table.__str__ popped the column styles while rendering, so a second str() of the same table raised IndexError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`cli docs -f html` pasted help texts, params and examples into the page as-is, so `<src>` vanished and markup in a manifest ended up in the page. An example given as a plain string has no description and was followed by the text "None" in all three formats. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideThis audit hardens Cliffy's manifest-to-code pipeline by escaping all data-derived source, isolating environment values, validating names and filesystem operations, and repairing parser behavior; it also makes dependency/include resolution, encoding, cleanup, documentation, testing, builds, reloads, and CLI exit codes reliable, with broad regression coverage. Sequence diagram for safe manifest transformation and code generationsequenceDiagram
participant Manifest as Manifest
participant Transformer
participant Jinja
participant Commander
participant GeneratedCLI
Manifest->>Transformer: load_manifest()
Transformer->>Jinja: render manifest with env mapping
Jinja-->>Transformer: rendered manifest data
Transformer->>Commander: generate_cli()
Commander->>Commander: py_str() and to_identifier()
Commander-->>Transformer: escaped generated source
Transformer->>GeneratedCLI: import generated source
Sequence diagram for collision-safe CLI loading and unloadingsequenceDiagram
participant User
participant Loader
participant Bin as InterpreterBin
participant CLIFile as CliffyCLIFile
User->>Loader: load_from_cli(cli)
Loader->>Bin: check target with lexists()
alt foreign executable exists
Bin-->>Loader: existing non-Cliffy file
Loader-->>User: exit_err()
else target is absent or Cliffy-owned
Loader->>Bin: write entry script
Loader->>CLIFile: write generated module
end
User->>Loader: unload_cli(cli_name)
Loader->>Bin: is_cli_script()
alt Cliffy entry script
Loader->>Bin: remove script
else foreign executable
Loader-->>Bin: keep file
end
Flow diagram for reliable CLI command outcomesflowchart TD
A[CLI command invoked] --> B{Operation succeeds?}
B -->|Yes| C[Print success]
B -->|No| D[Print error]
D --> E[Exit with status 1]
C --> F[Exit with status 0]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 8 issues
Fixed security issues:
- Command injection from untrusted input passed to OS command execution (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="cliffy/homer.py" line_range="103" />
<code_context>
Optional[str]: CLI metadata path
"""
- if cli_metadata_path := glob.glob(f"{CLIFFY_METADATA_DIR}/*/{cli_name}.json"):
+ if cli_metadata_path := glob.glob(f"{glob.escape(str(CLIFFY_METADATA_DIR))}/*/{glob.escape(cli_name)}.json"):
return cli_metadata_path[0]
return None
</code_context>
<issue_to_address>
**CLI removal escapes managed directories**
When a caller supplies a CLI name containing traversal components and a matching `.json` file exists at the resulting outside path, `glob.escape` does not escape path separators or `..`, so a crafted name can make `get_metadata_path` find a matching JSON file outside the metadata directory. `remove` then calls `remove_metadata` and `Loader.unload_cli` with that name, deleting files outside Cliffy's managed directories.
Validate CLI names at CLI entry points and perform removal using the exact metadata record's validated name and paths rather than reconstructing paths from raw arguments.
Also at `cliffy/homer.py:49`, `cliffy/loader.py:37`.
</issue_to_address>
### Comment 2
<location path="cliffy/loader.py" line_range="46-50" />
<code_context>
os.remove(cls.get_cli_path(cli_name))
+ @staticmethod
+ def is_cli_script(script_path: str) -> bool:
+ """Whether the file is an entry script written by `deploy_script`"""
+ try:
+ with open(script_path, encoding="utf-8") as script:
+ return "from cliffy.clis." in script.read(4096)
+ except (OSError, UnicodeDecodeError):
+ return False
</code_context>
<issue_to_address>
**Foreign executables are overwritten**
When a foreign target or symlink target contains `from cliffy.clis.` in its first 4096 characters, `Loader.is_cli_script` treats the marker substring as proof of ownership, so `deploy_script` overwrites the target and `unload_cli` can delete it; the caller loses a foreign executable.
Verify launcher ownership using a reliable check that rejects foreign files and symlinks instead of accepting a marker substring.
</issue_to_address>
### Comment 3
<location path="cliffy/loader.py" line_range="21" />
<code_context>
def deploy_script(self) -> str:
script_path = Loader.get_cli_script_path(self.cli.name)
+ if os.path.lexists(script_path) and not Loader.is_cli_script(script_path):
+ # i.e. a manifest named `python` or `pip`: the script would replace that executable
+ exit_err(
</code_context>
<issue_to_address>
**Ownership checks race with writes**
When another process replaces the launcher path after the ownership check and before the write or removal, `deploy_script` or `unload_cli` uses the path after checking it, so a replacement executable or symlink target is overwritten or deleted.
Make launcher ownership checks and writes or removals race-safe, and do not follow symlinks.
Also at `cliffy/loader.py:39-41`.
</issue_to_address>
### Comment 4
<location path="cliffy/loader.py" line_range="38-41" />
<code_context>
@classmethod
def unload_cli(cls, cli_name: str) -> None:
+ script_path = cls.get_cli_script_path(cli_name)
+ if cls.is_cli_script(script_path):
+ with contextlib.suppress(FileNotFoundError):
+ os.remove(script_path)
with contextlib.suppress(FileNotFoundError):
- os.remove(cls.get_cli_script_path(cli_name))
</code_context>
<issue_to_address>
**Launcher is left unable to run**
When an existing launcher is edited or cannot be read during unload, `unload_cli` skips removing the launcher but still deletes its module; `remove` can also delete the metadata before this check. The remaining launcher cannot import its module, and the metadata no longer provides a normal way to clean it up.
Only delete the module and metadata after the launcher has been safely removed, or otherwise keep the remaining launcher runnable and manageable.
</issue_to_address>
### Comment 5
<location path="cliffy/builder.py" line_range="86" />
<code_context>
+ interpreter=interpreter,
+ )
+ finally:
+ delete_temp_files()
- delete_temp_files()
</code_context>
<issue_to_address>
**Concurrent runs delete each other's files**
When two `run_cli` calls overlap while one is writing or importing its temporary script, `delete_temp_files` closes and unlinks every file in the process-wide `TEMP_FILES` list, so one run's cleanup removes another run's script and its import or use fails.
Track and clean up only the temporary files belonging to the current run.
Also at `cliffy/builder.py:34-35`, `cliffy/tester.py:52-54`.
</issue_to_address>
### Comment 6
<location path="cliffy/reloader.py" line_range="28" />
<code_context>
def on_modified(self, event: FileSystemEvent) -> None:
- if event.is_directory or not str(event.src_path).endswith(self.manifest_path):
+ # compared as full paths: `endswith` also matched other-x.yaml when watching x.yaml
+ if event.is_directory or os.path.realpath(os.fsdecode(event.src_path)) != self.manifest_realpath:
return
</code_context>
<issue_to_address>
**Older reload overwrites newer state**
When a reload of an older manifest state runs slowly while a later edit is reloaded and committed first, `on_modified()` starts an independent reload thread for each eligible event. An older thread can finish after a newer one and overwrite the newer generated module and metadata, leaving the loaded CLI at an older manifest version.
Serialize reloads or attach a generation/version to each reload and prevent an older generation from committing after a newer one.
</issue_to_address>
### Comment 7
<location path="tests/test_codegen.py" line_range="214" />
<code_context>
+ "['a', 'b']",
+ id="builtin_command",
+ ),
+ pytest.param(BASE + "commands:\n print: print('printed')\n", ["--version"], "t, 1", id="print_command"),
+ pytest.param(BASE + "commands:\n my-group.sub: print(1)\n", ["my-group", "sub"], "1", id="dash_group"),
+ pytest.param(BASE + "commands:\n 2fa.setup: print(1)\n", ["2fa", "setup"], "1", id="digit_group"),
</code_context>
<issue_to_address>
**Builtin command test skips command**
When the generated CLI's version callback works but its `print` command is missing or broken, `test_names_become_valid_identifiers` uses the `print_command` case at line 214 with `args=["--version"]`, so it never dispatches the generated `print` command; the version output still passes if that command is missing or broken.
Invoke the `print` command in this case and assert its command output.
</issue_to_address>
### Comment 8
<location path="tests/test_loader.py" line_range="27" />
<code_context>
if __name__ == "__main__":
main()
</code_context>
<issue_to_address>
**Symlink setup fails on Windows**
When the test runs on Windows without Developer Mode or symlink-creation privileges, `test_load_does_not_overwrite_foreign_executable` reaches `Path.symlink_to` at line 26 before calling `Loader`; on Windows without symlink privileges, that raises `OSError` and fails the test even when the loader is correct.
Skip this test when creating the symlink is unsupported, while keeping the symlink-based assertions on systems that permit it.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 8 findings to address first, and if an unrelated executable in the bin directory contains from cliffy.clis. in its first 4096 bytes, the ownership check can treat it as Cliffy’s and overwrite it during load or delete it during unload. That file loss persists after reverting and requires restoring the executable separately.
Blocking findings: cliffy/homer.py:103, cliffy/loader.py:50, cliffy/loader.py:21, cliffy/loader.py:41, cliffy/builder.py:86, and 3 more
|
|
||
| def deploy_script(self) -> str: | ||
| script_path = Loader.get_cli_script_path(self.cli.name) | ||
| if os.path.lexists(script_path) and not Loader.is_cli_script(script_path): |
There was a problem hiding this comment.
🔴 Critical · Ownership checks race with writes
When another process replaces the launcher path after the ownership check and before the write or removal, deploy_script or unload_cli uses the path after checking it, so a replacement executable or symlink target is overwritten or deleted.
Make launcher ownership checks and writes or removals race-safe, and do not follow symlinks.
Also at cliffy/loader.py:39-41.
Prompt for AI agents
In `cliffy/loader.py` at line 21:
**Ownership checks race with writes**
When another process replaces the launcher path after the ownership check and before the write or removal, `deploy_script` or `unload_cli` uses the path after checking it, so a replacement executable or symlink target is overwritten or deleted.
Make launcher ownership checks and writes or removals race-safe, and do not follow symlinks.
Also at `cliffy/loader.py:39-41`.
Owner
Author
There was a problem hiding this comment.
Mostly fixed in #60 (1459ad2): launchers and modules are written to a temp file in the same directory and renamed, a new launcher is created exclusively, reads use lstat and O_NOFOLLOW, and the ownership check runs right before unlink. A window remains: replacing or deleting an existing cliffy launcher is still check-then-rename/unlink, so a file another process puts at that exact path in between is replaced or removed. Links are never followed, so nothing outside the bin directory is touched. This is recorded in known-issues.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit of
cliffy/*.pyandcliffy/commanders/*.py(notai.py,blueprint.pyordocs/), stacked onchore/python-3.11-3.15. Every fix below was reproduced first and has a regression test. The suite grows from 188 to 360 tests and passes with ruff andmypy --stricton Python 3.11 and 3.15.The three most serious findings:
os.environwas rendered by Jinja on each manifest load. An environment variable containing{{ ... }}was evaluated, and one containing{%(a zsh prompt, for example) made every manifest fail to load.cli loadcould overwrite the interpreter. A manifest namedpythonorpipwrote its entry script through thebin/pythonsymlink, andcli rmthen deleted it.Fixes
Severity is my assessment. Tests are in
tests/.{%).{{ env.X }}also rendered empty outsidevars.envis passed to templates as a mapping and never rendered.test_transformer.py::test_environment_values_are_not_rendered_as_templates,::test_env_is_available_in_the_whole_manifestcli loadoverwrote, andcli rmdeleted, any executable in the interpreter's bin directory with the CLI's name.test_loader.py::test_load_does_not_overwrite_foreign_executable,::test_unload_keeps_foreign_executablecli_options, defaults, aliases, alias table) were interpolated between quotes in generated code.py_str()/repr().test_codegen.py::test_manifest_text_is_escaped(19 cases),::test_manifest_text_is_not_executed(5),::test_py_str_round_tripsname: ../../evilwrote the module, script and metadata outside their directories._and-, starting with a letter or_.test_transformer.py::test_cli_name_must_be_a_safe_file_name,::test_cli_name_validimport,list,print,cli,typer,2fa) broke the module or shadowed what other commands call. Groups and click aliases with a dash were SyntaxErrors.to_identifier()produces a safe function or variable name. An invalid param name is rejected with a clear error.test_codegen.py::test_names_become_valid_identifiers,::test_keyword_param_name_is_rejected,::test_to_identifierglobal_paramswere ignored for commands without params of their own.test_codegen.py::test_params[global_params_without_own_params],[click_global_params_without_own_params]name: str = typer.Option(...)was reduced toname: str, soexamples/todo.yaml's prompt was lost. A default containing=was truncated.--dry-run|-dbecame--dry_run. A non-str default on astrparam raised AttributeError.parser.pyandmanifest.py.test_codegen.py::test_params(typer cases),::test_manifest_text_is_escaped[param_default_int_for_str](*)commands ignored their template and|aliases, were attached to the wrong nested group, and crashed on non-str defaults.rsplit(".", 1).test_codegen.py::test_greedy_commands(5 cases)--name|-n,name: --who(became----who), dashed options/arguments and global params produced broken code; bool options needed a value.ClickParserrewritten on top ofParser.test_codegen.py::test_params(click cases)a.bcommand carrying help for nested groupa.bgenerated an empty function (SyntaxError, Typer).test_codegen.py::test_names_become_valid_identifiers[nested_group_help]requiresvalidation:PyYAML>=6raised KeyError,requests>=2,<3raised InvalidVersion, and~=, extras,rich_clickand markers were reported as missing.packagingand checked againstimportlib.metadata, with normalized names.test_transformer.py::test_requires_met,::test_requires_unmet;test_helper.py::test_check_requirement(18 cases)set(); a cycle ended in RecursionError.test_transformer.py::test_includes_*,::test_circular_includes,::test_missing_includecli testexited 0 when tests failed (also with-x) and errored on every test of ause_clickmanifest.test_tester.py::test_manifest_tests_*use_clickmanifest did not bundle rich-click.test_builder.py::test_build_installs_the_framework_the_cli_importsLANG=C,cli loadfailed with UnicodeEncodeError on any non-ASCII manifest text.test_encoding.py(2 tests, run in a C-locale subprocess)cli docs <loaded-cli>always failed;cli build <loaded-cli>looked formy-cli.pyinstead ofmy_cli.py.Loader.get_cli_path.test_cli.py::test_docs_for_loaded_cli,::test_build_loaded_cli_with_dashed_namecli validate,build,update,removeanddocsexited 0 after printing an error.test_cli.py::test_validate_reports_generation_errors_with_exit_code,::test_build_failure_exit_code,::test_docs_for_unknown_cli, plus 4 updated assertionscli rm '*'deleted other CLIs' metadata;remove_metadataraised if the directory held another file; a metadata file handle was leaked.rmdir, context manager.test_homer.py::test_remove_metadata_*,::test_get_metadata_bypath_closes_fileunload_clinever removed the module when the script was already gone.suppressblocks.test_loader.py::test_unload_removes_module_when_script_is_missingcli build -o a/bfailed ifadid not exist.try/finally;os.makedirs.test_builder.py::test_*_removes_temp_file_*,::test_build_creates_nested_output_dir;test_tester.py::test_tester_removes_temp_file_when_import_failsx.yamlalso reloaded onother-x.yaml; the manifest was never closed; the observer was only stopped on Ctrl-C.finally.test_reloader.py::test_on_modified_only_reloads_the_watched_manifest,::test_reload_closes_manifest,::test_watch_stops_observer_on_unexpected_errorvars:ended in AttributeError/TypeError tracebacks.test_transformer.py::test_manifest_must_be_a_mapping,::test_empty_vars_sectioncli docs -f htmldid not escape manifest text; examples without a description printed "None".html.escape; empty string.test_doc.py::test_generate_html_escapes_manifest_text,::test_example_without_descriptionTable(no rich-click) could only be rendered once.test_rich.py::test_fallback_table_can_be_rendered_twicecli initprinted a traceback for a name the template rejects;--rawaccepted names that cannot be loaded.test_cli.py::test_init_invalid_name,::test_init_template_name_error_is_not_a_tracebacktest_codegen.py::test_generated_imports_are_sortedEach new test was also run against the base commit's code to confirm it fails there. A handful pass on base on purpose: they guard behavior that must not change (for example
test_load_replaces_its_own_script,test_params[click_quoted_default],test_cli_name_valid).test_generated_imports_are_sortedfailed on base in my runs, but because the old order was hash-dependent it could pass there by chance.Breaking changes
[A-Za-z_][A-Za-z0-9_-]*. Names with spaces, dots or slashes are rejected.cli testexits 1 when any test fails.cli validate,build,update,removeanddocsexit 1 on error. Four existing assertions that pinned exit code 0 for these cases were changed to 1 (test_cli.py::test_update_nonexistent_cli,::test_remove_command,::test_build_command,test_examples.py::test_cli_build_fails)._(list->list_). Run blocks that call such a command's function by name must be updated. Command names on the command line are unchanged.global_paramsnow apply to every command, including those without params of their own.--flag/--no-flagswitches (was--flag true). A bare str default (str = World) is a string.--dry-run, was--dry_run). Simple params written as a fulltyper.Option(...)/typer.Argument(...)are now honored, sotodo createprompts as its manifest says.{{ env.X }}works outsidevars(it rendered empty before).requiresentries must be valid PEP 508 requirements. Markers are evaluated.cli loadrefuses a name that collides with an existing non-cliffy executable in the interpreter's bin directory.parse_requirement,compare_versions,RequirementSpecandOPERATOR_MAPare removed in favor ofcheck_requirement.get_installed_package_versionsreturns normalized names fromimportlib.metadata.Transformer.resolve_include_by_pathis replaced byresolve_include. The tests for the removed helpers were ported case by case totest_check_requirement.Looked at and left alone
imports,functions,types, raw param strings, simple-param defaults and thename: {expr: null}var form are code by design.Undefined. The manifest author is trusted, andStrictUndefinedwould break the documented{{ env['X'] or 'default' }}pattern.yaml.safe_loadeverywhere.builder.pycalls pip and shiv with argument lists (no shell) and cleans itsTemporaryDirectory.tester.pyexecs the manifest's assert scripts and shell lines by design.build_cli(deps=None)is handled.$transform was not audited beyond the existing example tests.How verified
Local, macOS arm64, uv 0.13.0, one venv per Python synced with
uv sync --locked --all-extras:mypy --strictpass on all 13. Ruff passes on 12; the first commit has three unused imports that the second commit starts using.🤖 Generated with Claude Code
Summary by Sourcery
Harden manifest processing, code generation, CLI lifecycle operations, and command outcomes against audited defects and unsafe input.
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: