Skip to content

Commit 2c67a55

Browse files
efraespadaefraespada
committed
feature-296-bugbot-autofix: Enhance regex handling in BranchRepository and ThinkUseCase by introducing a utility function for escaping special characters. Update related logic to ensure safe processing of user input and improve regex pattern matching. Add tests to verify correct behavior when handling special characters in user mentions and version extraction.
1 parent 6fc9e3a commit 2c67a55

File tree

10 files changed

+155
-17
lines changed

10 files changed

+155
-17
lines changed

‎build/cli/index.js‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49858,6 +49858,7 @@ class BranchRepository {
4985849858
if (baseBranchName.indexOf('tags/') > -1) {
4985949859
ref = baseBranchName;
4986049860
}
49861+
const refForGraphQL = ref.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
4986149862
const octokit = github.getOctokit(token);
4986249863
const { repository } = await octokit.graphql(`
4986349864
query($repo: String!, $owner: String!, $issueNumber: Int!) {
@@ -49866,7 +49867,7 @@ class BranchRepository {
4986649867
issue(number: $issueNumber) {
4986749868
id
4986849869
}
49869-
ref(qualifiedName: "refs/${ref}") {
49870+
ref(qualifiedName: "refs/${refForGraphQL}") {
4987049871
target {
4987149872
... on Commit {
4987249873
oid
@@ -56658,7 +56659,8 @@ class ThinkUseCase {
5665856659
}));
5665956660
return results;
5666056661
}
56661-
const question = commentBody.replace(new RegExp(`@${param.tokenUser}`, 'gi'), '').trim();
56662+
const escapedUsername = param.tokenUser.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
56663+
const question = commentBody.replace(new RegExp(`@${escapedUsername}`, 'gi'), '').trim();
5666256664
if (!question) {
5666356665
results.push(new result_1.Result({
5666456666
id: this.taskId,
@@ -59428,14 +59430,19 @@ exports.PROMPTS = {};
5942859430

5942959431
Object.defineProperty(exports, "__esModule", ({ value: true }));
5943059432
exports.injectJsonAsMarkdownBlock = exports.extractChangelogUpToAdditionalContext = exports.extractReleaseType = exports.extractVersion = void 0;
59433+
function escapeRegexLiteral(s) {
59434+
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
59435+
}
5943159436
const extractVersion = (pattern, text) => {
59432-
const versionPattern = new RegExp(`###\\s*${pattern}\\s+(\\d+\\.\\d+\\.\\d+)`, 'i');
59437+
const escaped = escapeRegexLiteral(pattern);
59438+
const versionPattern = new RegExp(`###\\s*${escaped}\\s+(\\d+\\.\\d+\\.\\d+)`, 'i');
5943359439
const match = text.match(versionPattern);
5943459440
return match ? match[1] : undefined;
5943559441
};
5943659442
exports.extractVersion = extractVersion;
5943759443
const extractReleaseType = (pattern, text) => {
59438-
const releaseTypePattern = new RegExp(`###\\s*${pattern}\\s+(Patch|Minor|Major)`, 'i');
59444+
const escaped = escapeRegexLiteral(pattern);
59445+
const releaseTypePattern = new RegExp(`###\\s*${escaped}\\s+(Patch|Minor|Major)`, 'i');
5943959446
const match = text.match(releaseTypePattern);
5944059447
return match ? match[1] : undefined;
5944159448
};
@@ -59448,7 +59455,7 @@ const extractChangelogUpToAdditionalContext = (body, sectionTitle) => {
5944859455
if (body == null || body === '') {
5944959456
return 'No changelog provided';
5945059457
}
59451-
const escaped = sectionTitle.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
59458+
const escaped = escapeRegexLiteral(sectionTitle);
5945259459
const pattern = new RegExp(`(?:###|##)\\s*${escaped}\\s*\\n\\n([\\s\\S]*?)` +
5945359460
`(?=\\n(?:###|##)\\s*Additional Context\\s*|$)`, 'i');
5945459461
const match = body.match(pattern);

‎build/cli/src/data/repository/__tests__/branch_repository.createLinkedBranch.test.d.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/**
2+
* Unit tests for createLinkedBranch: GraphQL ref escaping so branch names with " or \ do not break the query.
3+
*/
4+
export {};

‎build/github_action/index.js‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44947,6 +44947,7 @@ class BranchRepository {
4494744947
if (baseBranchName.indexOf('tags/') > -1) {
4494844948
ref = baseBranchName;
4494944949
}
44950+
const refForGraphQL = ref.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
4495044951
const octokit = github.getOctokit(token);
4495144952
const { repository } = await octokit.graphql(`
4495244953
query($repo: String!, $owner: String!, $issueNumber: Int!) {
@@ -44955,7 +44956,7 @@ class BranchRepository {
4495544956
issue(number: $issueNumber) {
4495644957
id
4495744958
}
44958-
ref(qualifiedName: "refs/${ref}") {
44959+
ref(qualifiedName: "refs/${refForGraphQL}") {
4495944960
target {
4496044961
... on Commit {
4496144962
oid
@@ -51966,7 +51967,8 @@ class ThinkUseCase {
5196651967
}));
5196751968
return results;
5196851969
}
51969-
const question = commentBody.replace(new RegExp(`@${param.tokenUser}`, 'gi'), '').trim();
51970+
const escapedUsername = param.tokenUser.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
51971+
const question = commentBody.replace(new RegExp(`@${escapedUsername}`, 'gi'), '').trim();
5197051972
if (!question) {
5197151973
results.push(new result_1.Result({
5197251974
id: this.taskId,
@@ -54736,14 +54738,19 @@ exports.PROMPTS = {};
5473654738

5473754739
Object.defineProperty(exports, "__esModule", ({ value: true }));
5473854740
exports.injectJsonAsMarkdownBlock = exports.extractChangelogUpToAdditionalContext = exports.extractReleaseType = exports.extractVersion = void 0;
54741+
function escapeRegexLiteral(s) {
54742+
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
54743+
}
5473954744
const extractVersion = (pattern, text) => {
54740-
const versionPattern = new RegExp(`###\\s*${pattern}\\s+(\\d+\\.\\d+\\.\\d+)`, 'i');
54745+
const escaped = escapeRegexLiteral(pattern);
54746+
const versionPattern = new RegExp(`###\\s*${escaped}\\s+(\\d+\\.\\d+\\.\\d+)`, 'i');
5474154747
const match = text.match(versionPattern);
5474254748
return match ? match[1] : undefined;
5474354749
};
5474454750
exports.extractVersion = extractVersion;
5474554751
const extractReleaseType = (pattern, text) => {
54746-
const releaseTypePattern = new RegExp(`###\\s*${pattern}\\s+(Patch|Minor|Major)`, 'i');
54752+
const escaped = escapeRegexLiteral(pattern);
54753+
const releaseTypePattern = new RegExp(`###\\s*${escaped}\\s+(Patch|Minor|Major)`, 'i');
5474754754
const match = text.match(releaseTypePattern);
5474854755
return match ? match[1] : undefined;
5474954756
};
@@ -54756,7 +54763,7 @@ const extractChangelogUpToAdditionalContext = (body, sectionTitle) => {
5475654763
if (body == null || body === '') {
5475754764
return 'No changelog provided';
5475854765
}
54759-
const escaped = sectionTitle.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
54766+
const escaped = escapeRegexLiteral(sectionTitle);
5476054767
const pattern = new RegExp(`(?:###|##)\\s*${escaped}\\s*\\n\\n([\\s\\S]*?)` +
5476154768
`(?=\\n(?:###|##)\\s*Additional Context\\s*|$)`, 'i');
5476254769
const match = body.match(pattern);

‎build/github_action/src/data/repository/__tests__/branch_repository.createLinkedBranch.test.d.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/**
2+
* Unit tests for createLinkedBranch: GraphQL ref escaping so branch names with " or \ do not break the query.
3+
*/
4+
export {};

‎src/data/repository/__tests__/branch_repository.createLinkedBranch.test.ts‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* Unit tests for createLinkedBranch: GraphQL ref escaping so branch names with " or \ do not break the query.
3+
*/
4+
5+
import { BranchRepository } from "../branch_repository";
6+
7+
jest.mock("../../../utils/logger", () => ({
8+
logDebugInfo: jest.fn(),
9+
logError: jest.fn(),
10+
}));
11+
12+
const mockGraphql = jest.fn();
13+
jest.mock("@actions/github", () => ({
14+
getOctokit: () => ({
15+
graphql: (...args: unknown[]) => mockGraphql(...args),
16+
}),
17+
}));
18+
19+
describe("createLinkedBranch", () => {
20+
const repo = new BranchRepository();
21+
22+
beforeEach(() => {
23+
mockGraphql.mockReset();
24+
});
25+
26+
it("escapes double quote in ref when baseBranchName contains quote", async () => {
27+
mockGraphql
28+
.mockResolvedValueOnce({
29+
repository: {
30+
id: "R_1",
31+
issue: { id: "I_1" },
32+
ref: { target: { oid: "abc123" } },
33+
},
34+
})
35+
.mockResolvedValueOnce({ createLinkedBranch: { linkedBranch: { id: "LB_1" } } });
36+
37+
await repo.createLinkedBranch(
38+
"o",
39+
"r",
40+
'feature"injection',
41+
"feature/42-foo",
42+
42,
43+
undefined,
44+
"token"
45+
);
46+
47+
expect(mockGraphql).toHaveBeenCalledTimes(2);
48+
const queryString = mockGraphql.mock.calls[0][0] as string;
49+
expect(queryString).toContain('refs/heads/feature\\"injection');
50+
expect(queryString).not.toMatch(/qualifiedName:\s*"refs\/heads\/feature"[^\\]/);
51+
});
52+
53+
it("escapes backslash in ref when baseBranchName contains backslash", async () => {
54+
mockGraphql
55+
.mockResolvedValueOnce({
56+
repository: {
57+
id: "R_1",
58+
issue: { id: "I_1" },
59+
ref: { target: { oid: "abc123" } },
60+
},
61+
})
62+
.mockResolvedValueOnce({ createLinkedBranch: { linkedBranch: { id: "LB_1" } } });
63+
64+
await repo.createLinkedBranch(
65+
"o",
66+
"r",
67+
"feature\\branch",
68+
"feature/42-foo",
69+
42,
70+
undefined,
71+
"token"
72+
);
73+
74+
expect(mockGraphql).toHaveBeenCalledTimes(2);
75+
const queryString = mockGraphql.mock.calls[0][0] as string;
76+
expect(queryString).toContain("refs/heads/feature\\\\branch");
77+
});
78+
});

‎src/data/repository/branch_repository.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,11 @@ export class BranchRepository {
288288
try {
289289
logDebugInfo(`Creating linked branch ${newBranchName} from ${oid ?? baseBranchName}`)
290290

291-
let ref = `heads/${baseBranchName}`
291+
let ref = `heads/${baseBranchName}`;
292292
if (baseBranchName.indexOf('tags/') > -1) {
293-
ref = baseBranchName
293+
ref = baseBranchName;
294294
}
295+
const refForGraphQL = ref.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
295296

296297
const octokit = github.getOctokit(token);
297298
const {repository} = await octokit.graphql<RepositoryResponse>(`
@@ -301,7 +302,7 @@ export class BranchRepository {
301302
issue(number: $issueNumber) {
302303
id
303304
}
304-
ref(qualifiedName: "refs/${ref}") {
305+
ref(qualifiedName: "refs/${refForGraphQL}") {
305306
target {
306307
... on Commit {
307308
oid

‎src/usecase/steps/common/__tests__/think_use_case.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,24 @@ describe('ThinkUseCase', () => {
194194
expect(results[0].executed).toBe(true);
195195
});
196196

197+
it('strips mention correctly when tokenUser contains regex-special chars', async () => {
198+
mockGetDescription.mockResolvedValue(undefined);
199+
mockAskAgent.mockResolvedValue({ answer: 'OK' });
200+
mockAddComment.mockResolvedValue(undefined);
201+
const param = baseParam({
202+
tokenUser: 'bot.',
203+
issue: { ...baseParam().issue, commentBody: '@bot. what is 2+2?' },
204+
});
205+
206+
const results = await useCase.invoke(param);
207+
208+
expect(mockAskAgent).toHaveBeenCalledTimes(1);
209+
const prompt = mockAskAgent.mock.calls[0][2];
210+
expect(prompt).toContain('Question: what is 2+2?');
211+
expect(results[0].success).toBe(true);
212+
expect(results[0].executed).toBe(true);
213+
});
214+
197215
it('includes issue description in prompt when getDescription returns content', async () => {
198216
mockGetDescription.mockResolvedValue('Implement login feature for the app.');
199217
mockAskAgent.mockResolvedValue({ answer: 'Sure, here is how...' });

‎src/usecase/steps/common/think_use_case.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ export class ThinkUseCase implements ParamUseCase<Execution, Result[]> {
6969
return results;
7070
}
7171

72-
const question = commentBody.replace(new RegExp(`@${param.tokenUser}`, 'gi'), '').trim();
72+
const escapedUsername = param.tokenUser.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
73+
const question = commentBody.replace(new RegExp(`@${escapedUsername}`, 'gi'), '').trim();
7374
if (!question) {
7475
results.push(
7576
new Result({

‎src/utils/__tests__/content_utils.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ describe('content_utils', () => {
2626
expect(extractVersion('Release Version', '### Release Version 1.2')).toBeUndefined();
2727
expect(extractVersion('Release Version', '### Release Version abc')).toBeUndefined();
2828
});
29+
30+
it('escapes regex-special chars in pattern (no ReDoS or over-matching)', () => {
31+
expect(extractVersion('Release (Version)', '### Release (Version) 1.2.3')).toBe('1.2.3');
32+
expect(extractVersion('.*', '### .* 1.2.3')).toBe('1.2.3');
33+
expect(extractVersion('.*', '### x 1.2.3')).toBeUndefined();
34+
expect(extractVersion('x.y', '### x.y 9.8.7')).toBe('9.8.7');
35+
});
2936
});
3037

3138
describe('extractReleaseType', () => {
@@ -44,6 +51,11 @@ describe('content_utils', () => {
4451
expect(extractReleaseType('Release Type', 'No type here')).toBeUndefined();
4552
expect(extractReleaseType('Other', '### Release Type Patch')).toBeUndefined();
4653
});
54+
55+
it('escapes regex-special chars in pattern', () => {
56+
expect(extractReleaseType('Release (Type)', '### Release (Type) Minor')).toBe('Minor');
57+
expect(extractReleaseType('Patch|Minor', '### Patch|Minor Major')).toBe('Major');
58+
});
4759
});
4860

4961
describe('extractChangelogUpToAdditionalContext', () => {

‎src/utils/content_utils.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1+
function escapeRegexLiteral(s: string): string {
2+
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
3+
}
4+
15
export const extractVersion = (pattern: string, text: string): string | undefined => {
2-
const versionPattern = new RegExp(`###\\s*${pattern}\\s+(\\d+\\.\\d+\\.\\d+)`, 'i');
6+
const escaped = escapeRegexLiteral(pattern);
7+
const versionPattern = new RegExp(`###\\s*${escaped}\\s+(\\d+\\.\\d+\\.\\d+)`, 'i');
38
const match = text.match(versionPattern);
49
return match ? match[1] : undefined;
510
};
611

712
export const extractReleaseType = (pattern: string, text: string): string | undefined => {
8-
const releaseTypePattern = new RegExp(`###\\s*${pattern}\\s+(Patch|Minor|Major)`, 'i');
13+
const escaped = escapeRegexLiteral(pattern);
14+
const releaseTypePattern = new RegExp(`###\\s*${escaped}\\s+(Patch|Minor|Major)`, 'i');
915
const match = text.match(releaseTypePattern);
1016
return match ? match[1] : undefined;
1117
};
@@ -21,7 +27,7 @@ export const extractChangelogUpToAdditionalContext = (
2127
if (body == null || body === '') {
2228
return 'No changelog provided';
2329
}
24-
const escaped = sectionTitle.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
30+
const escaped = escapeRegexLiteral(sectionTitle);
2531
const pattern = new RegExp(
2632
`(?:###|##)\\s*${escaped}\\s*\\n\\n([\\s\\S]*?)` +
2733
`(?=\\n(?:###|##)\\s*Additional Context\\s*|$)`,

0 commit comments

Comments
 (0)