Fix: Improve PR number validation and warning messages in input handling (#939)

* Fix: Improve PR number validation and warning messages in input handling

* Fix: Enhance PR number validation with improved sanitization and warning messages

* Fix: Change sanitizeForWarning to a local function for better encapsulation

__tests__/main.test.ts

@@ -441,6 +441,197 @@ describe('run', () => {
     );
   });
 
+  it('(with pr-number: non-numeric string) warns and makes no API call', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['abc']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'abc' is not a valid pull request number"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: negative number) warns and makes no API call', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['-1']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'-1' is not a valid pull request number"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: zero) warns and makes no API call', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['0']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'0' is not a valid pull request number"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: number with internal space) warns and makes no API call', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['10 4']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'10 4' is not a valid pull request number"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: number with trailing non-numeric chars) warns and makes no API call', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['104abc']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'104abc' is not a valid pull request number"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: valid number with surrounding whitespace) trims and processes correctly', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': [' 104 ']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+    mockGitHubResponseChangedFiles('foo.pdf');
+    getPullMock.mockResolvedValue(<any>{data: {labels: []}});
+
+    await run();
+
+    expect(getPullMock).toHaveBeenCalled();
+    expect(coreWarningMock).not.toHaveBeenCalledWith(
+      expect.stringContaining('is not a valid pull request number')
+    );
+  });
+
+  it('(with pr-number: string with carriage return) sanitizes CR as \\x0d in warning', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['abc\rdef']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'abc\\x0ddef' is not a valid pull request number (non-printable characters were escaped as \\xNN)"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: string with tab) sanitizes tab as \\x09 in warning', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['abc\tdef']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'abc\\x09def' is not a valid pull request number (non-printable characters were escaped as \\xNN)"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: string with ANSI escape sequence) sanitizes ESC byte as \\x1b in warning', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['abc\x1b[31mINJECTED\x1b[0m']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'abc\\x1b[31mINJECTED\\x1b[0m' is not a valid pull request number (non-printable characters were escaped as \\xNN)"
+    );
+    expect(getPullMock).not.toHaveBeenCalled();
+    expect(setLabelsMock).not.toHaveBeenCalled();
+  });
+
+  it('(with pr-number: mix of valid and invalid) processes valid, skips invalid with warning', async () => {
+    configureInput({
+      'repo-token': 'foo',
+      'configuration-path': 'bar',
+      'pr-number': ['104', 'abc']
+    });
+
+    usingLabelerConfigYaml('only_pdfs.yml');
+    mockGitHubResponseChangedFiles('foo.pdf');
+
+    getPullMock.mockResolvedValue(<any>{
+      data: {labels: []}
+    });
+
+    await run();
+
+    expect(coreWarningMock).toHaveBeenCalledWith(
+      "'abc' is not a valid pull request number"
+    );
+    expect(setLabelsMock).toHaveBeenCalledTimes(1);
+    expect(setLabelsMock).toHaveBeenCalledWith({
+      owner: 'monalisa',
+      repo: 'helloworld',
+      issue_number: 104,
+      labels: ['touched-a-pdf-file']
+    });
+  });
+
   it('does not add labels to PRs that have no changed files', async () => {
     usingLabelerConfigYaml('only_pdfs.yml');
     mockGitHubResponseChangedFiles();

dist/index.js

@@ -995,6 +995,9 @@ exports.getPrNumbers = void 0;
 const core = __importStar(__nccwpck_require__(7484));
 const github = __importStar(__nccwpck_require__(3228));
 const getPrNumberFromContext = () => { var _a; return (_a = github.context.payload.pull_request) === null || _a === void 0 ? void 0 : _a.number; };
+const sanitizeForWarning = (value) => {
+    return value.replace(/[\x00-\x1F\x7F-\x9F]/g, c => `\\x${c.charCodeAt(0).toString(16).padStart(2, '0')}`);
+};
 const getPrNumbers = () => {
     const prInput = core.getMultilineInput('pr-number');
     if (!(prInput === null || prInput === void 0 ? void 0 : prInput.length)) {
@@ -1002,9 +1005,14 @@ const getPrNumbers = () => {
     }
     const result = [];
     for (const line of prInput) {
-        const prNumber = parseInt(line, 10);
+        const trimmed = line.trim();
-        if (isNaN(prNumber) && prNumber <= 0) {
+        const prNumber = parseInt(trimmed, 10);
-            core.warning(`'${prNumber}' is not a valid pull request number`);
+        if (isNaN(prNumber) || prNumber <= 0 || String(prNumber) !== trimmed) {
+            const sanitized = sanitizeForWarning(line);
+            const hint = sanitized !== line
+                ? ' (non-printable characters were escaped as \\xNN)'
+                : '';
+            core.warning(`'${sanitized}' is not a valid pull request number${hint}`);
             continue;
         }
         result.push(prNumber);

src/get-inputs/get-pr-numbers.ts

@@ -4,6 +4,13 @@ import * as github from '@actions/github';
 const getPrNumberFromContext = () =>
   github.context.payload.pull_request?.number;
 
+const sanitizeForWarning = (value: string): string => {
+  return value.replace(
+    /[\x00-\x1F\x7F-\x9F]/g,
+    c => `\\x${c.charCodeAt(0).toString(16).padStart(2, '0')}`
+  );
+};
+
 export const getPrNumbers = (): number[] => {
   const prInput = core.getMultilineInput('pr-number');
 
@@ -14,10 +21,16 @@ export const getPrNumbers = (): number[] => {
   const result: number[] = [];
 
   for (const line of prInput) {
-    const prNumber = parseInt(line, 10);
+    const trimmed = line.trim();
+    const prNumber = parseInt(trimmed, 10);
 
-    if (isNaN(prNumber) && prNumber <= 0) {
+    if (isNaN(prNumber) || prNumber <= 0 || String(prNumber) !== trimmed) {
-      core.warning(`'${prNumber}' is not a valid pull request number`);
+      const sanitized = sanitizeForWarning(line);
+      const hint =
+        sanitized !== line
+          ? ' (non-printable characters were escaped as \\xNN)'
+          : '';
+      core.warning(`'${sanitized}' is not a valid pull request number${hint}`);
       continue;
     }