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

Shell export not working when password has special characters #131

Closed
kpetremann opened this issue Aug 14, 2022 · 4 comments
Closed

Shell export not working when password has special characters #131

kpetremann opened this issue Aug 14, 2022 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@kpetremann
Copy link
Contributor

kpetremann commented Aug 14, 2022

Expected Behavior

The password when executing teller sh should be enclosed in quote to avoid interpreting special characters such as parentheses.

Current Behavior

Currently passwords are not escaped, resulting in the following error:
bash: syntax error near unexpected token '('

Possible Solution

Enclose password with quote and substitute single quote already in the variable
strings.ReplaceAll(v.Value, "'", "'\"'\"'")

I've a commit ready to be submitted as a PR: kpetremann@7610091

Steps to Reproduce

  1. have this kind of secret: ()"';@ \(\)\"\'\;\@
  2. eval "$(teller sh)"

Context

Secrets can have special characters which should not be interpreted by the shell

Specifications

  • Version: 1.5.3
  • Platform: Ubuntu 22.04
@kpetremann kpetremann added the bug Something isn't working label Aug 14, 2022
@kaplanelad
Copy link
Contributor

kaplanelad commented Aug 14, 2022

Hey @kpetreman, thanks for reporting this issue.
Do you want to open a pull request with your fix? We're accepting PRs

It will good also if your will create a e2e test that test this scenario. just add a yaml file to https://github.com/tellerops/teller/tree/master/e2e/tests

@kaplanelad kaplanelad self-assigned this Aug 14, 2022
@kpetremann
Copy link
Contributor Author

sure, I'll add it and then send the PR!

@kpetremann
Copy link
Contributor Author

@kaplanelad I added the e2e test in the PR #132, don't hesitate if it needs adjustments

@kpetremann
Copy link
Contributor Author

kpetremann commented Aug 25, 2022

closing the issue as the PR has been merged and released.

Thanks for having merged the PR :)

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants