defenseunicorns / uds-cli

Apache License 2.0
24 stars 11 forks source link

feat: Validate and format bundle names to exclude special characters #914

Open catsby opened 2 weeks ago

catsby commented 2 weeks ago

Description

Prevent special characters in bundle names, which ultimately affect the tarball filename but are in general probably not a good idea to allow anyway.

In this PR if a bundle's name contains non letters, numbers, or hyphens, uds-cli will error. If the name is valid then we remove all leading and trailing spaces (" "), lowercase everything, and replace any remaining spaces with hyphens.

Ex:

worDpress -> wordpress " Word pRess " -> word-press

It's currently possible to use a bundle name with odd characters or even spaces which result in tarball artifacts with either the special characters or spaces in the name.

Example:

The name here is unlikely but still valid

kind: UDSBundle
metadata:
  name: "a exAmple   ??}doOm\n"
  description: an example UDS bundle
  version: 0.0.1

Output (trimmed):

 ➜ uds create
  🎁 BUNDLE DEFINITION

kind: UDSBundle
metadata:
  name: |
    a exAmple   ??}doOm
  description: an example UDS bundle
  version: 0.0.1
packages:
- name: dos-game
  repository: ghcr.io/zarf-dev/packages/dos-games
  ref: 1.1.0
[...]
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
[...]
? Create this bundle? Yes
  βœ”  Bundle Validated
[...]
  🚧 Building Bundle
[...]
  βœ”  Created bundle archive at: /Users/clint/go/github.com/defenseunicorns/uds-cli/uds-bundle-a exAmple   ??}doOm
       -arm64-0.0.1.tar.zst

Generates this file:

-rw-r--r--@  1 clint  staff  3547344 Sep  9 11:39 uds-bundle-a exAmple   ??}doOm?-arm64-0.0.1.tar.zst

Note:

This is currently a draft for review and feedback on next steps. It's not entirely clear if we should outright error and abort initially, or if we should at first output some kind of warning that the name is not valid and in the future we'll error, to enable a sort of grace period.

Related Issue

Fixes #886

Type of change

Checklist before merging

decleaver commented 2 weeks ago

Did someone external create an issue for this or was it something you found? I would say we just error out and abort if someone tries to use a bad name

catsby commented 2 weeks ago

@decleaver it came from me. I was working through onboarding things and for reasons I don't recall I had a space in my bundle name ("example doom game" or something) and noticed the resulting tarball had spaces in the file name that had to be escaped if I wanted to reference it

UncleGedd commented 2 weeks ago

+1 to aborting and erroring out, but when we do, show the user how they can fix the error

UncleGedd commented 1 week ago

"I want to not break things and not be terribly surprising" - @catsby