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

template: allow & in arg string #809

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tschettervictor
Copy link
Collaborator

@tschettervictor tschettervictor commented Jan 14, 2025

#412

The PR will make sure an escape character \ is added in front of the & character in 'arg' string ONLY if it is escaped like "\&".

It fixes #412, but I'm not sure if it will break something else down the road. It shouldn't as it only looks for \& and makes sure it stays that way.

@michael-o
@tobiastom

@tschettervictor
Copy link
Collaborator Author

tschettervictor commented Jan 14, 2025

As long as the "&" is escaped like \& it will treat it literally now. But if it is on its own like "&" then it will treat it like a special character.

This is the Bastille file I'm using.

ARG NAME

CMD echo "My name is ${NAME}" > /root/my-name
CMD "cat /root/my-name

With --arg NAME="me \& you"

@michael-o
Copy link
Contributor

The question is how to properly document that for the user since it is not obvious that you have to pass "\&". I not 100% convinced that it is an ideal solution. How is the user supposed to know that this goes through sed?

@tschettervictor
Copy link
Collaborator Author

tschettervictor commented Jan 15, 2025

Good point.

I think personally it should be a general point that if we are using special characters (anywhere) we need to try to escape them.

This keeps things lined up with the rest of the BSD world/code for the most part.

The other question is, Is there ever a time when the user will want to use the "specialness" of that character? And if they do, we definitely need to have a way to do both, and escaping it is the best option. IMO.

@tschettervictor
Copy link
Collaborator Author

How so?

All it does is add one additional clans to the "parse_arg_value" function.

And that is, if it finds \&, it will make sure it stays.

@bmac2
Copy link
Collaborator

bmac2 commented Jan 15, 2025

ok I am good with it.

@tschettervictor
Copy link
Collaborator Author

tschettervictor commented Jan 15, 2025

I've added docs saying that it needs to be escaped if it is to be treated literally.

The question is, does it work for you?

@michael-o
Copy link
Contributor

I have now tried the following:

bastille template deblndw013x4j dw/base-complete \
  --arg LOCALE="en_GB.UTF-8" \
  --arg ROOT_FULLNAME="Michael '\\&' Osipov" \
  --arg ROOT_AUTHORIZED_KEYS=/var/tmp/deblndw013x4j/authorized_keys \
  --arg ROOT_K5LOGIN=/var/tmp/deblndw013x4j/k5login \
  --arg ROOT_FORWARD=/var/tmp/deblndw013x4j/forward \
  --arg INSTALL_SOFTWARE_FROM="packages"

It does not work:

# finger root
Login: root                             Name: Michael '' Osipov
Directory: /root                        Shell: /bin/sh
On since Thu 16 Jan 10:10 (CET) on pts/1
No Mail.
No Plan.

I also need to note that ROOT_FULLNAME is passed to another template first:
From base-complete:

...
ARG ROOT_FULLNAME
ARG ROOT_AUTHORIZED_KEYS
ARG ROOT_K5LOGIN
ARG ROOT_FORWARD
ARG INSTALL_SOFTWARE_FROM="packages"

CMD echo "Starting base jail configuration"
...
INCLUDE dw/root-config --arg FULLNAME="${ROOT_FULLNAME}" --arg AUTHORIZED_KEYS="${ROOT_AUTHORIZED_KEYS}" --arg K5LOGIN="${ROOT_K5LOGIN}" --arg FORWARD="${ROOT_FORWARD}"
CMD touch /etc/.base-complete
CMD echo "Base jail complete"

and root-config contains:

...
CMD pw usermod root -c "${FULLNAME}"
...

@tschettervictor
Copy link
Collaborator Author

You are escaping it twice there. It should only be escaped once, and without the single quotes.

@bmac2
Copy link
Collaborator

bmac2 commented Jan 18, 2025

@michael-o @tschettervictor what is the status of this one. Looking at what can be closed out this weekend but not sure where we are. Last posting from Michael was he found an error.

@yaazkal

@michael-o
Copy link
Contributor

@michael-o @tschettervictor what is the status of this one. Looking at what can be closed out this weekend but not sure where we are. Last posting from Michael was he found an error.

@yaazkal

Will do more tests on Monday.

@michael-o
Copy link
Contributor

You are escaping it twice there. It should only be escaped once, and without the single quotes.

Why no single quotes? So if single quotes aren't allowed then one issue is fixed and then something else will be broken.

@tschettervictor
Copy link
Collaborator Author

Did you try with the single quotes? Perhaps it does work. I didn't try that.

@michael-o
Copy link
Contributor

The only thing which works is:

  --arg ROOT_FULLNAME="Michael \\& Osipov" \

but with single quotes it does not work. It also requires two backslashes, from a user PoV the behavior is unclear and not really usable.

@bmac2
Copy link
Collaborator

bmac2 commented Jan 20, 2025

what triggered this PR with a & in it? Is this something we are trying to code around a one off occurance, or is there a real use case behind it? Just curious since this regex is seeming to get complicated.

@tschettervictor @michael-o

@tschettervictor
Copy link
Collaborator Author

#412

@michael-o
Copy link
Contributor

what triggered this PR with a & in it? Is this something we are trying to code around a one off occurance, or is there a real use case behind it? Just curious since this regex is seeming to get complicated.

@tschettervictor @michael-o

See my example by providing GECOS and have the system resolve '&' for the username.

@tschettervictor
Copy link
Collaborator Author

So, what if we would require it to be inside single quotes for it to be recognized literally?

@michael-o
Copy link
Contributor

So, what if we would require it to be inside single quotes for it to be recognized literally?

Can you rephrase with an example? I don't understand the question.

@tschettervictor
Copy link
Collaborator Author

'&' will treat it literally, where not having the single quotes will treat it as a special character.

@michael-o
Copy link
Contributor

'&' will treat it literally, where not having the single quotes will treat it as a special character.

Then how to combine both? Two singlequotes?

@tschettervictor
Copy link
Collaborator Author

First off, what is your suggestion? What should work and be acceptable? Then we can go from there.

@michael-o
Copy link
Contributor

First off, what is your suggestion? What should work and be acceptable? Then we can go from there.

My expectation would be the following that these work:

bastille apply foo/bar --arg ME="foo & bar" --arg YOU="foo '&' bar" --arg US="foo \"&\" bar"

if any of these requires escaping then it should be properly documented.

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.

[BUG] Ampersand mangled when used in an ARG
3 participants