Repository navigation
fix(init): reject template hooks the installed SAM CLI cannot run - #9211
Conversation
Cookiecutter runs a Python hook with sys.executable. In the packaged macOS and Linux SAM CLI that is the sam executable rather than an interpreter, so the hook is not executed and cookiecutter is told it succeeded -- the project is generated without whatever the hook was meant to do, and sam init exits 0. Refuse instead, naming the hook and how to unblock. Templates without a Python hook are unaffected, as is the Windows installer, which ships a real interpreter.
The packaged macOS and Linux SAM CLI now rejects Python hooks, so the assertion that their output is captured only holds where hooks can run.
|
Thanks for digging into this — the diagnosis looks right to me, and I confirmed the patch targets against the pinned cookiecutter 2.7.1: all three attributes exist, Two things I'd like to see addressed before merge, both about how the refusal actually reaches the user. 1. On
The gap is pre-existing for any cookiecutter failure on the pipeline path, but this change is the first to route an intentional, user-actionable error through it, so an expected refusal would invite bug reports. Converting at the pipeline call site (or having 2. cookiecutter's Happy to be corrected on either if you're seeing different output locally. |
|
One more, as a follow-up rather than something for this PR — the
if cmd is None and not ctx.resilient_parsing:
if split_opt(cmd_name)[0]:
self.parse_args(ctx, ctx.args)
ctx.fail(f"No such command {cmd_name!r}.")
That means the silent-success problem isn't specific to cookiecutter — a script doing |
On sam pipeline init the refusal was wrapped by a catch-all into GenerateProjectFailedError, which is not a UserException, so it reached the user as an unhandled exception with a stack trace and a file-a-bug link. Convert it there the way init_generator already does. Cookiecutter 2.7.1 also logs a hook failure with logger.exception before re-raising, putting a traceback ahead of the remedy, so quiet that logger for the duration of the guard.
Thanks for catching, fixed and updated on 1,2. The exit code change is at a much larger scale so not doing that in this PR. |
![aws-sam-tooling-bot[bot]](https://avatars.githubusercontent.com/b/1290?s=60&v=4){kind=small}
…overed Raising the cookiecutter.hooks logger to CRITICAL silenced every hook log on a bundle, losing which hook failed for shell hooks and all --debug hook diagnostics. Filter on the in-flight exception instead. Also split out a hook-free --location test, so that the project directory being taken from cookiecutter's return value stays covered on the packaged CLI.
The skip was keyed on SAM_CLI_DEV alone, which also covered the Windows installer -- where hooks do run, since it ships an embedded interpreter rather than a bundle. Narrow it so that platform keeps its coverage. Also the hook lives in the template, not in the generated project.
The existing tests call the patched attributes directly, so they would keep passing if a cookiecutter upgrade reached its hook runners some other way and the guard silently stopped firing. Build a real template and assert cookiecutter() itself refuses, for all three hooks including pre_prompt, which is dispatched through cookiecutter.main rather than cookiecutter.hooks.
…e init Only the pipeline path had the clause, so on sam init --location the remedy was buried behind "An error occurred while generating this project : " -- with an empty project name -- and telemetry recorded it as a generic cookiecutter failure rather than this refusal.
They drove the real cookiecutter() with no config_file, so it read ~/.cookiecutterrc -- whose default_context can decide project_name -- and wrote a replay file to ~/.cookiecutter_replay. Point it at a config inside the temp directory instead, so these only fail when the guard stops firing.
The backstop raises the same exception for a temporary copy, so a type-only assertion would pass while the message degraded to tmpXXXX.py -- losing the name that tells the user which hook to remove.
The wrappers narrowed the types to str, so a Path would have raised AttributeError from endswith instead of refusing. Normalise with os.fspath.
Which issue(s) does this change fix?
N/A
Why is this change necessary?
sam init --location(andsam pipeline initwith a custom template) silently ignores a template's Python hooks on the packaged macOS and Linux SAM CLI.Cookiecutter runs a Python hook as
[sys.executable, hook.py]. In a PyInstaller bundlesys.executableis thesamexecutable rather than an interpreter, so the command becomessam /tmp/xxxx.py. That path starts with/, which click treats as option-like: it prints help and exits 0. Cookiecutter reads 0 as success, so generation continues and the user gets a project missing whatever the hook was meant to do, withsam initreporting success.Reproduced on a released binary:
This does not affect the Windows installer, which ships an embedded CPython (
sam.cmdrunspython.exe -m samcli), sosys.executablethere is a real interpreter and hooks run correctly. Verified on Windows CI: hook ran,sys.executable=...\AWSSAMCLI\runtime\python.exe, exit 0.sys._MEIPASSis absent there, so this change is a no-op on Windows.How does it address the issue?
Refuses rather than pretending to succeed, naming the hook and how to unblock:
A context manager wraps the
cookiecutter()call at both call sites, active only whenis_pyinstaller_bundle(). Three cookiecutter entry points are covered:run_script_with_contextforpre_gen_project/post_gen_project, since it still knows the hook's real name rather than the temporary copyrun_scriptreceives;run_pre_prompt_hookup front, because cookiecutter otherwise replaces apre_promptfailure with a message of its own that would hide the remedy; andrun_scriptas a backstop.The error subclasses cookiecutter's
FailedHookExceptionso cookiecutter still removes the partially generated project, leaving nothing behind to clean up before retrying.What side effects does this change have?
Nothing that works today starts failing. Verified behaviour with the bundle simulated:
hooks/directoryhooks/with no matching hook filehooks/post_gen_project.shhooks/post_gen_project.pyhooks/pre_gen_project.pyhooks/pre_prompt.pyCookiecutter never calls
run_scriptwhen a template has no hook —run_hookreturns early onfind_hook— so a hookless--locationtemplate cannot be affected. Shell hooks are delegated untouched and keep working on POSIX. Windows and pip installs are unaffected.Neither official template repository uses hooks: 0 hook files in
aws-sam-cli-app-templates(6702 paths at the pinned commit) and 0 inaws-sam-cli-pipeline-init-templates, so the defaultsam initandsam pipeline initflows are untouched.Also verified that removing the
withblock at either call site fails the corresponding new guard test and nothing else — without those guards the wrapper could be dropped silently, since it is a no-op outside a bundle.make prpasses (9413 unit tests, 94.10% coverage).Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.