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

Escape pipe char of commit msg in markdown table #515

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions src/lib/github/v3/createStatusComment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,34 @@ describe('getCommentBody', () => {
targetPullRequestStates: [],
},
},
{
formatted: 'some-formatted-text',
commit: {
author: {
email: '[email protected]',
name: 'Matthias Polman-Wilhelm',
},
sourceBranch: 'master',
sourcePullRequest: {
labels: [],
number: 44,
title: 'Antarctica commit | with pipeline char',
url: 'url-to-pr-45',
mergeCommit: {
sha: '',
message: 'Antarctica commit | with pipeline char',
},
},
suggestedTargetBranches: [],
sourceCommit: {
branchLabelMapping: {},
committedDate: '',
sha: '',
message: 'Antarctica commit | with pipeline char',
},
targetPullRequestStates: [],
},
},
],
}),
},
Expand All @@ -365,27 +393,27 @@ describe('getCommentBody', () => {
it('posts a comment when `publishStatusCommentOnFailure = true`', () => {
const params = getParams({ publishStatusCommentOnFailure: true });
expect(getCommentBody(params)).toMatchInlineSnapshot(`
"## 💔 Some backports could not be created
"## 💔 Some backports could not be created

| Status | Branch | Result |
|:------:|:------:|:------|
|✅|7.x|[<img src="https://img.shields.io/github/pulls/detail/state/elastic/kibana/55">](url-to-pr-55)|
|❌|7.1|**Backport failed because of merge conflicts**<br><br>You might need to backport the following PRs to 7.1:<br> - [New Zealand commit message](url-to-pr-5)<br> - [Australia commit](url-to-pr-44)|
|❌|7.2|Backport failed because of merge conflicts|
| Status | Branch | Result |
|:------:|:------:|:------|
|✅|7.x|[<img src="https://img.shields.io/github/pulls/detail/state/elastic/kibana/55">](url-to-pr-55)|
|❌|7.1|**Backport failed because of merge conflicts**<br><br>You might need to backport the following PRs to 7.1:<br> - [New Zealand commit message](url-to-pr-5)<br> - [Australia commit](url-to-pr-44)<br> - [Antarctica commit \\| with pipeline char](url-to-pr-45)|
|❌|7.2|Backport failed because of merge conflicts|

Note: Successful backport PRs will be merged automatically after passing CI.
Note: Successful backport PRs will be merged automatically after passing CI.

### Manual backport
To create the backport manually run:
\`\`\`
node scripts/backport --pr 55
\`\`\`
### Manual backport
To create the backport manually run:
\`\`\`
node scripts/backport --pr 55
\`\`\`

### Questions ?
Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)
### Questions ?
Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

<!--- Backport version: 1.2.3-mocked -->"
`);
<!--- Backport version: 1.2.3-mocked -->"
`);
});

it('does not post a comment when `publishStatusCommentOnFailure = false`', () => {
Expand Down
7 changes: 4 additions & 3 deletions src/lib/github/v3/createStatusComment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,10 @@
) {
const unmergedBackports =
result.error.errorContext.commitsWithoutBackports.map((c) => {
return ` - [${getFirstLine(c.commit.sourceCommit.message)}](${
c.commit.sourcePullRequest?.url
})`;
const msg = getFirstLine(c.commit.sourceCommit.message);
// make sure to escape the pipe character to ensure the markdown table is correct
const msgEscaped = msg.replace(/\|/g, '\\|');

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sorenlouv I don't know why this is raised, looked around but didn't find answers, IMO opinion this should be correct in Markdown, I don't know why this is a failure?

return ` - [${msgEscaped}](${c.commit.sourcePullRequest?.url})`;
});

const unmergedBackportsSection =
Expand Down
Loading