Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

catsby
Copy link
Collaborator

@catsby catsby commented Sep 9, 2024

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@decleaver
Copy link
Collaborator

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
Copy link
Collaborator Author

catsby commented Sep 9, 2024

@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
Copy link
Collaborator

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

@UncleGedd
Copy link
Collaborator

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

@UncleGedd UncleGedd added this to the v0.16.0 milestone Sep 16, 2024
@catsby catsby modified the milestones: v0.16.0, v0.17.0 Sep 20, 2024
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.

Sanitize Bundle Name
3 participants