quintilesims / slackbot

Slack bot for IQVIA
MIT License
2 stars 0 forks source link

Add !alias command #26

Closed zpatrick closed 6 years ago

zpatrick commented 6 years ago

Adds a !alias command and behavior to the slackbot.

TODO

tlake commented 6 years ago

For visibility, I'm gonna summarize some conversation about !alias I had with @zpatrick in a PM.

Dangerous Glob

Given a usage like !alias add "PATTERN" "TEMPLATE", I would caution very strongly against glob matching on PATTERN. That creates a pretty big security (is that the right term?) breach, in that someone could write this alias:

!alias add "*" "!gif something nsfw"

and then every single message posted across every channel in the slack team gets matched to that alias and replaced with an nswf gif. Even !alias commands like !alias rm would match that, so you couldn't even find the offending alias and remove it.

One suggestion was to skip glob matching if the message received starts with !alias. That way, you could never overwrite the !alias command with an alias. Even then, although you could find and remove bogus aliases, all of slack is still nswf gifs until someone takes the time to fix it.

Issues with .Text

We could try and keep !alias super simple and do no glob matching at all, which is certainly safe, but very, very limiting. It could be just a simple substitution feature that passes through the rest of user input:

!alias add "!wfh *" "<@{{ .User }}> is working from home today! {{ .Text }}"

but that would probably require a bit of backend work on .Text, since at the moment it's the full message text and that would include the !alias as well:

!wfh cc person1 person2

output:
<@tlake> is working from home today! !wfh cc person1 person2

Maybe something like name, input := strings.SplitN(msg.Text, " ", 2) somewhere on the backend?

zpatrick commented 6 years ago

merging for now, can always come back and re-address