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

fix: validate key names dont have spaces (#1449) #1490

Merged
merged 3 commits into from
Apr 6, 2022
Merged

Conversation

BruceMacD
Copy link
Collaborator

  • validate key names dont have spaces
  • migrate existing keys with spaces
  • validate key name in CLI before create

Summary

Working with access keys that have spaces in the name in the CLI is a pain. Fix this by not allowing spaces in the access key name.

Checklist

  • Wrote appropriate unit tests
  • Considered security implications of the change
  • Updated associated docs where necessary
  • Updated associated configuration where necessary
  • Change is backwards compatible if it needs to be (user can upgrade without manual steps?)
  • Nothing sensitive logged
  • Commit message conforms to Conventional Commit
  • GitHub Actions are passing
  • Considered data migrations for smooth upgrades

Related Issues

Resolves #1449

- validate key names dont have spaces
- migrate existing keys with spaces
- validate key name in CLI before create
- only do migration is access_keys table present
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This definitely addresses the most common case. I think we would have the same problem with any other whitespace characters, right? (tab, vertical tab, newline, carriage return, etc).

I wonder if there is a way to restrict the characters to only "visible" characters. Anything that returns true for https://pkg.go.dev/unicode#IsSpace should probably be invalid.

Not blocking, but maybe something for us to investigate later.

@BruceMacD BruceMacD merged commit b080ca2 into main Apr 6, 2022
@BruceMacD BruceMacD deleted the no-key-space branch April 6, 2022 18:03
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.

Key names should not have spaces
3 participants