create-theme validates the theme name after creating the folder · Issue #771 · rendercv/rendercv · GitHub
Skip to content

create-theme validates the theme name after creating the folder #771

Description

@Exyons

rendercv create-theme checks the theme name only after it has already copied the templates onto disk. If the name is invalid, the folder has been created by then, and nothing removes it when validation fails. The same missing check also lets the theme name point outside the current directory.

There is a related problem with the name pattern itself: it accepts names that produce an __init__.py Python cannot parse, and in that case the command reports success.

Tested on RenderCV v2.8 (main, 1d4b87b), Python 3.13, Linux.

1. An invalid name leaves a populated folder behind

$ rendercv create-theme "My Theme!"
╭─ Error ──────────────────────────────────────────────────────────────╮
│ The custom theme name should only contain lowercase letters and       │
│ digits. The provided value is `My Theme!`.                            │
╰──────────────────────────────────────────────────────────────────────╯
$ echo $?
1
$ find "My Theme!" -type f | wc -l
13

The command exits 1, but ./My Theme!/ is still there with all 13 template files in it, and the user has to delete it by hand.

2. The theme name can escape the working directory

$ mkdir work && cd work
$ rendercv create-theme ../escaped
╭─ Error ──────────────────────────────────────────────────────────────╮
│ The custom theme name should only contain lowercase letters and       │
│ digits. The provided value is `../escaped`.                           │
╰──────────────────────────────────────────────────────────────────────╯
$ ls ..
escaped/  work/
$ find ../escaped -type f | wc -l
13

The folder lands in the parent directory, outside the directory the command was run in.

Where both come from

src/rendercv/cli/create_theme_command/create_theme_command.py:32-41:

new_theme_folder = pathlib.Path.cwd() / theme_name   # unvalidated name -> path

if new_theme_folder.exists():
    ...

copy_templates("typst", new_theme_folder)            # creates the folder

create_init_file_for_theme(theme_name, new_theme_folder / "__init__.py")

The name check lives inside create_init_file_for_theme (create_init_file_for_theme.py:19-24), which runs after copy_templates has already called shutil.copytree. When the check then fails, nothing cleans up.

3. A name starting with a digit produces a broken theme and no error

custom_theme_name_pattern = re.compile(r"^[a-z0-9]+$") (src/rendercv/schema/models/design/design.py:17) allows a leading digit, but the class name is built with f"{theme_name.capitalize()}Theme" (create_init_file_for_theme.py:37), which then isn't a valid Python identifier:

$ rendercv create-theme 123theme
╭─ Theme created ──────────────────────────────────────────────────────╮
│ ✓ Created your custom theme: ./123theme                               │
...
$ echo $?
0
$ sed -n '866p' 123theme/__init__.py
class 123themeTheme(BaseModelWithoutExtraKeys):

Nothing goes wrong until the user tries to render, and the message they get points at a file RenderCV wrote for them:

$ rendercv render Test_User_CV.yaml
│ design │ ... │ The custom theme 123theme's __init__.py file          │
│        │     │ has a syntax error. Please fix it.                    │

One caveat if custom_theme_name_pattern looks like the place to fix this: tightening it would also reject template-only themes, which have no __init__.py and are handled by the fallback at design.py:138-144. Those work fine with a leading digit, so the identifier requirement really only applies where the class name is generated.

Possible fix

Validating theme_name at the top of cli_command_create_theme, before the path is built, would cover both 1 and 2, since a rejected name never reaches the path join. For 3, rejecting names that don't start with a lowercase letter inside create_init_file_for_theme would avoid touching the shared pattern.

Why the tests miss this

tests/cli/create_theme_command/test_create_theme_command.py:34-38 checks that typer.Exit is raised for an invalid name, but never checks that the folder wasn't created, so it passes either way.

I'd like to work on this and open a PR.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions