diff --git a/scripts/lib/github-coordination/gh-api.js b/scripts/lib/github-coordination/gh-api.js index dd2cb4397..874c247f2 100644 --- a/scripts/lib/github-coordination/gh-api.js +++ b/scripts/lib/github-coordination/gh-api.js @@ -12,8 +12,9 @@ function normalizeRepo(repo) { } function normalizeIssueNumber(value) { - const parsed = Number.parseInt(String(value), 10); - if (!Number.isFinite(parsed) || parsed <= 0) { + const text = String(value).trim(); + const parsed = Number(text); + if (!/^\+?\d+$/.test(text) || !Number.isSafeInteger(parsed) || parsed <= 0) { throw new Error(`Invalid issue number: ${value}`); } return parsed; diff --git a/tests/lib/github-coordination-gh-api.test.js b/tests/lib/github-coordination-gh-api.test.js new file mode 100644 index 000000000..558d93f17 --- /dev/null +++ b/tests/lib/github-coordination-gh-api.test.js @@ -0,0 +1,96 @@ +/** + * Tests for scripts/lib/github-coordination/gh-api.js — normalizeIssueNumber + * + * Guards against silent truncation of malformed issue numbers + * (https://github.com/affaan-m/ECC/issues/3514): parseInt-style prefix + * parsing accepted "42oops" as 42, "42.9" as 42, and "4e2" as 4, while + * integers beyond 2^53 lost precision (9007199254740993 became ...992). + * The same helper also parses --limit, so malformed values there must be + * rejected too. Whitespace, leading plus, and leading zero tolerance is + * preserved. + * + * Run with: node tests/lib/github-coordination-gh-api.test.js + */ + +'use strict'; + +const assert = require('assert'); + +const { + normalizeIssueNumber, +} = require('../../scripts/lib/github-coordination/gh-api'); +const { test, banner, section, summary } = require('./helpers/mini-test-runner'); + +let passed = 0; +let failed = 0; + +banner('normalizeIssueNumber — accepted forms:'); + +if (test('accepts a plain positive integer', () => { + assert.strictEqual(normalizeIssueNumber('42'), 42); +})) passed++; else failed++; + +if (test('preserves surrounding whitespace tolerance', () => { + assert.strictEqual(normalizeIssueNumber(' 7 '), 7); +})) passed++; else failed++; + +if (test('preserves leading plus tolerance', () => { + assert.strictEqual(normalizeIssueNumber('+7'), 7); +})) passed++; else failed++; + +if (test('preserves leading zero tolerance', () => { + assert.strictEqual(normalizeIssueNumber('007'), 7); +})) passed++; else failed++; + +if (test('accepts MAX_SAFE_INTEGER exactly', () => { + assert.strictEqual(normalizeIssueNumber('9007199254740991'), 9007199254740991); +})) passed++; else failed++; + +if (test('accepts numeric (non-string) input', () => { + assert.strictEqual(normalizeIssueNumber(42), 42); +})) passed++; else failed++; + +section('normalizeIssueNumber — malformed tokens are rejected, not truncated:'); + +for (const [value, label] of [ + ['42oops', 'trailing garbage after digits'], + ['42.9', 'decimal fraction'], + ['4e2', 'exponent notation'], + ['0x10', 'hex notation'], + ['1e999', 'exponent beyond Number range'], + ['12 34', 'internal whitespace'], + ['-.', 'punctuation only'], +]) { + if (test(`rejects ${JSON.stringify(value)} (${label})`, () => { + assert.throws(() => normalizeIssueNumber(value), /Invalid issue number/); + })) passed++; else failed++; +} + +section('normalizeIssueNumber — unsafe integers are rejected:'); + +for (const value of [ + '9007199254740992', // 2^53 + '9007199254740993', // 2^53 + 1 (rounds to 2^53 as a Number) + '999999999999999999999999', +]) { + if (test(`rejects ${value}`, () => { + assert.throws(() => normalizeIssueNumber(value), /Invalid issue number/); + })) passed++; else failed++; +} + +section('normalizeIssueNumber — non-positive and empty inputs:'); + +for (const value of ['0', '-3', '', ' ', 'abc']) { + if (test(`rejects ${JSON.stringify(value)}`, () => { + assert.throws(() => normalizeIssueNumber(value), /Invalid issue number/); + })) passed++; else failed++; +} + +if (test('error message preserves the original input', () => { + assert.throws( + () => normalizeIssueNumber('42oops'), + (error) => error.message === 'Invalid issue number: 42oops' + ); +})) passed++; else failed++; + +summary(passed, failed); diff --git a/tests/scripts/github-coordination.test.js b/tests/scripts/github-coordination.test.js index 73394a1ef..006406658 100644 --- a/tests/scripts/github-coordination.test.js +++ b/tests/scripts/github-coordination.test.js @@ -93,6 +93,28 @@ async function runTests() { let passed = 0; let failed = 0; + if ( + await test('rejects malformed issue numbers during argument parsing', async () => { + const { parseArgs } = require(SCRIPT); + + for (const argv of [ + ['node', 'github-coordination.js', 'claim', '42oops', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--issue', '42.9', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--issue', '4e2', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--issue', '9007199254740993', '--repo', 'example/project'], + ['node', 'github-coordination.js', 'claim', '--limit', '42.9', '--repo', 'example/project'], + ]) { + assert.throws(() => parseArgs(argv), /Invalid issue number/); + } + + const valid = parseArgs(['node', 'github-coordination.js', 'claim', '7', '--repo', 'example/project']); + assert.strictEqual(valid.issueNumber, 7); + assert.strictEqual(valid.limit, 100); + }) + ) + passed++; + else failed++; + if ( await test('claims an epic issue, updates GitHub state, and caches a work item', async () => { const rootDir = createTempDir('github-coordination-claim-');