Skip to content
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

fix: Prevent error when using quotation marks in BuildMessage #75

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

colindean
Copy link
Contributor

When a PR title or commit summary contains quotation marks, they were not escaped when injected into the JSON attachment. This caused a message like

unable to unmarshal webhook message: invalid character 't' after object key:value pair

when used inside of a template containing this test:

{
  "text": "*Status*: FAILURE\n*Repo*: {{ .RepositoryName }} | *Branch*: {{ .BuildBranch }} | *Author*: {{ .BuildAuthor }}\n*Build*: {{ .BuildLink }}\n*Message*: {{ .BuildMessage }}"
}

This change safens this use case. Other typical fields shouldn't have quotation marks in them, but it might be prudent down the line to cleanse all fields as they are injected into the JSON or provide a method usable inside of the template that escapes correctly.

When a PR title or commit summary contains quotation marks, they were
not escaped when injected into the JSON attachment. This caused
a message like

    unable to unmarshal webhook message: invalid character 't' after object key:value pair

when used inside of a template containing this test:

```json
{
  "text": "*Status*: FAILURE\n*Repo*: {{ .RepositoryName }} | *Branch*: {{ .BuildBranch }} | *Author*: {{ .BuildAuthor }}\n*Build*: {{ .BuildLink }}\n*Message*: {{ .BuildMessage }}"
}
```

This change safens this use case. Other typical fields _shouldn't_ have
quotation marks in them, but it might be prudent down the line to
cleanse _all_ fields as they are injected into the JSON or provide
a method usable inside of the template that escapes correctly.
@colindean colindean requested a review from a team as a code owner June 27, 2024 18:27
ecrupper
ecrupper previously approved these changes Jun 27, 2024
cmd/vela-slack/plugin.go Outdated Show resolved Hide resolved
Copy link
Member

@wass3rw3rk wass3rw3rk left a comment

Choose a reason for hiding this comment

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

thanks for providing a fix. minor suggestion and we're good to go

cmd/vela-slack/plugin.go Outdated Show resolved Hide resolved
wass3r
wass3r previously approved these changes Jun 28, 2024
@ecrupper ecrupper merged commit 379b0ba into go-vela:main Jul 17, 2024
@colindean colindean deleted the safen_quotes branch July 17, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants