Description
Hi there, thank you for this project. I'm currently working on a server side implementation, that is supposed to receive a webhook and recreate a docker container.
While taking a look at your code, I've found multiple security problems. I don't think they are too bad for most people but still I feel like they should be addressed. I will open an issue for each of them, to help with organizing them, even though they are kind of related to each other. I'm sorry for spamming you with issues, but I think it's worth it.
The current implementation does not verify SSL certificates, if I correctly understand the curl
docs on option -k
. This parameter was introduced in 16b7ccd as a response to #3.
I can see how some people do not have the time, patience or even the option to use verifiable SSL certificates, but I do and so will others.
I believe there are two ways to fix this issue (both using an additional argument verify-ssl: [bool]
similar to silent: [bool]
):
- Remove
-k
-option by default and only add it, if an additional argumentverify-ssl: false
is added to the workflow config. This would have the benefit that the most secure option is used by default, but might break other peoples' workflows if they use unsigned certificates and would require them to update their workflow config. - Add an additional argument
verify-ssl: true
to remove the-k
-option. This would at least allow people like me to verify their SSL certificates, without breaking any existing workflows. If this route is taken, it should at least be mentioned inREADME.md
in my opinion, to warn other users.
Personally I'd prefer option 1 as it will default new users' workflows to verify their SSL certificates, but I do understand if you don't want to break any existing workflows. Just keep in mind that fixing this now might be easier than fixing it when even more people use this workflow and it actually becomes a serious security issue for some.