Skip to content

OpenBSD: service.enable needs python_shell=true#25201

Closed
ajacoutot wants to merge 1 commit intosaltstack:developfrom
ajacoutot:openbsd
Closed

OpenBSD: service.enable needs python_shell=true#25201
ajacoutot wants to merge 1 commit intosaltstack:developfrom
ajacoutot:openbsd

Conversation

@ajacoutot
Copy link
Copy Markdown
Contributor

That's because it uses the double ampersand to combine commands.

That's because it uses the double ampersand to combine commands.
@thatch45
Copy link
Copy Markdown
Contributor

thatch45 commented Jul 7, 2015

@jfindlay please take a look at doing this in a more elegant way since we don't want to use a shell=True here

@jfindlay jfindlay self-assigned this Jul 7, 2015
@jfindlay jfindlay added the pending-changes The pull request needs additional changes before it can be merged label Jul 7, 2015
@ajacoutot
Copy link
Copy Markdown
Contributor Author

Hi. I am the developer of the rcctl(1) utility, I can try and find a better way to achieve the same if needed.

@jfindlay
Copy link
Copy Markdown
Contributor

jfindlay commented Jul 7, 2015

@ajacoutot, how do you feel about #25216? One other issue is that the introduction of the && is also in the 2015.8 branch, so I've submitted that pull request there.

@jfindlay
Copy link
Copy Markdown
Contributor

jfindlay commented Jul 7, 2015

Also the issue with using python_shell=True in execution modules is that this can expose salt to shell injection vulnerabilities in some circumstances.

@ajacoutot
Copy link
Copy Markdown
Contributor Author

@jfindlay #25216 is definitely the way to go, thanks a lot.
Successfully tested here.

@rallytime
Copy link
Copy Markdown
Contributor

Thanks for confirming @ajacoutot. I am going to close this pull request in favor of #25216 so this one doesn't get accidentally merged. Thanks!

@rallytime rallytime closed this Jul 8, 2015
@jfindlay
Copy link
Copy Markdown
Contributor

jfindlay commented Jul 8, 2015

Thanks, @ajacoutot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending-changes The pull request needs additional changes before it can be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants