Skip to content

Conversation

@larsewi
Copy link
Contributor

@larsewi larsewi commented Nov 28, 2025

@larsewi
Copy link
Contributor Author

larsewi commented Nov 28, 2025

@cf-bottom Jenkins with exotics please :)

@larsewi larsewi marked this pull request as ready for review November 28, 2025 15:00
@cf-bottom
Copy link

@larsewi
Copy link
Contributor Author

larsewi commented Nov 29, 2025

@cf-bottom Jenkins with exotics please :)

@cf-bottom
Copy link

@larsewi
Copy link
Contributor Author

larsewi commented Dec 1, 2025

^ Known failure ENT-12953

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

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

nice

* # Comment character, can truncate commands
* \n \r Newlines can inject additional commands
*/
const char *shell_metacharacters = ";|&`$(){}[]<>!#*?~\\'\"\n\r";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@olehermanse maybe this is too strict?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I do think the tilde is likely too strict and needed for some package names. I remember @nickanderson mentioning it I think. I checked OpenBSD and Debian and your list seems fine for those based on a simple package name search, but not on the other ways to specify package names with meta information maybe like versions and such.

Maybe a better way here is to have a default in C code that is maybe too strict and add a common attribute that can override that default so folks can make a choice without changing C code.

Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
I also refactored the code to use a dynamic buffer when assembling the
commands. Estimating the buffer size needed is error prone and can
quickly lead to buffer overflows.

Ticket: ENT-13535
Changelog: Title
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
@larsewi
Copy link
Contributor Author

larsewi commented Dec 4, 2025

@cf-bottom Jenkins please :)

@cf-bottom
Copy link

@craigcomstock
Copy link
Contributor

@larsewi regarding the CI failure, a small fix might fix it: #5981 (I couldn't push to the PR so a separate temporary PR is there with a VERY tiny fix. :) )

… quoting arguments

Ticket: ENT-13535
Changelog: none
(cherry picked from commit 6f97d0a)
@larsewi
Copy link
Contributor Author

larsewi commented Dec 5, 2025

@cf-bottom Jenkins please :)

@cf-bottom
Copy link

@larsewi larsewi merged commit 9525710 into cfengine:master Dec 7, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants