From 3cf0e709d596351e05c299dc19cceae836126386 Mon Sep 17 00:00:00 2001 From: akushonkamen Date: Mon, 12 Oct 2026 01:25:09 +0800 Subject: [PATCH] fix(github-coordination): reject malformed issue numbers instead of truncating them normalizeIssueNumber parsed with Number.parseInt, which stops at the first non-digit ("42oops" -> 42, "42.9" -> 42, "4e2" -> 4) and loses precision past 2^53 (9007199254740993 became 9007199254740992). The CLI shares this helper for --issue, --limit, and the positional issue number, so malformed values silently targeted the wrong issue before any integer guard downstream could notice. Validate the entire trimmed decimal token and require Number.isSafeInteger so malformed or out-of-range values throw "Invalid issue number" during argument parsing, before the state store is opened or GitHub is contacted. Whitespace, leading plus, and leading zero tolerance is preserved. Regression coverage added at both the library level (tests/lib/github-coordination-gh-api.test.js) and the CLI argument parser (tests/scripts/github-coordination.test.js). Fixes #3514 --- scripts/lib/github-coordination/gh-api.js | 5 +- tests/lib/github-coordination-gh-api.test.js | 96 ++++++++++++++++++++ tests/scripts/github-coordination.test.js | 22 +++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 tests/lib/github-coordination-gh-api.test.js 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-');