Skip to content

ENT-13535: Added packages promiser sanitation#5967

Merged
larsewi merged 3 commits into
cfengine:masterfrom
larsewi:packages
Dec 7, 2025
Merged

ENT-13535: Added packages promiser sanitation#5967
larsewi merged 3 commits into
cfengine:masterfrom
larsewi:packages

Conversation

@larsewi

@larsewi larsewi commented Nov 28, 2025

Copy link
Copy Markdown
Contributor

@larsewi

larsewi commented Nov 28, 2025

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown

Comment thread tests/acceptance/07_packages/packages_command.cf Outdated
@larsewi

larsewi commented Nov 29, 2025

Copy link
Copy Markdown
Contributor Author

@cf-bottom Jenkins with exotics please :)

@cf-bottom

Copy link
Copy Markdown

@larsewi

larsewi commented Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

^ Known failure ENT-12953

@craigcomstock craigcomstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice

Comment thread cf-agent/verify_packages.c Outdated
* # Comment character, can truncate commands
* \n \r Newlines can inject additional commands
*/
const char *shell_metacharacters = ";|&`$(){}[]<>!#*?~\\'\"\n\r";

Copy link
Copy Markdown
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
Copy Markdown
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

larsewi commented Dec 4, 2025

Copy link
Copy Markdown
Contributor Author

@cf-bottom Jenkins please :)

@cf-bottom

Copy link
Copy Markdown

@craigcomstock

Copy link
Copy Markdown
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

larsewi commented Dec 5, 2025

Copy link
Copy Markdown
Contributor Author

@cf-bottom Jenkins please :)

@cf-bottom

Copy link
Copy Markdown

@larsewi larsewi merged commit 9525710 into cfengine:master Dec 7, 2025
12 checks passed
@larsewi larsewi deleted the packages branch April 27, 2026 07:15
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