-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: allow users to run custom commands before exit #3
Conversation
|
||
finish () { | ||
exit_code=$?; | ||
if [ ! -z "${CALLBACK_ON_SUCCESS:-}" ] && [ "$exit_code" = 0 ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit torn on whether this should be a single CALLBACK_COMMAND
that gets passed the exit code as an argument, but considering this gets passed as an env var, it's probably a bit annoying to start adding conditions to that single line string...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I can totally see that; probably easiest to do this
finish () { | ||
exit_code=$?; | ||
if [ ! -z "${CALLBACK_ON_SUCCESS:-}" ] && [ "$exit_code" = 0 ]; then | ||
sh -c "$CALLBACK_ON_SUCCESS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered if we should worry at all about cleverly escaping this then reminded myself that really we don't want to worry since this is basically arbitrary remote code execution by design anyway.
Maybe if we were worried about this we would instead provide a webhook type URL rather than totally arbitrary command
@@ -36,6 +37,18 @@ The OAuth access token used for authenticating against the target wiki. | |||
|
|||
The OAuth access secret used for authenticating against the target wiki. | |||
|
|||
## Running commands after the script has finished |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prasie: thanks for diligently adding the docs here; that's super nice
Ticket https://phabricator.wikimedia.org/T369261