Skip to content

Commit 83ec7ea

Browse files
committed
fix(cli): treat non-numeric version cores as not comparable, never "ahead"
Copilot review catch on #669: compareVersions parsed core segments with Number.parseInt and NaN !== NaN made any malformed version (v1.0.0, 1.0.x, garbage from a corrupt manifest) deterministically compare as GREATER — classifying it 'ahead' and suppressing the align/reinstall guidance exactly where it's needed. Guard the parse: either side with a NaN core returns 0 (not comparable), which callers classify as 'behind' — the safe direction. Tests at both layers (comparator + versionSkew classification). Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
1 parent d803914 commit 83ec7ea

3 files changed

Lines changed: 32 additions & 0 deletions

File tree

packages/cli/src/__tests__/runtime-versions.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,14 @@ describe('compareVersions', () => {
116116
expect(compareVersions('1.0.0-rc.1', '1.0.0-rc.1')).toBe(0)
117117
})
118118

119+
it('treats a non-numeric core as not comparable (never "ahead")', () => {
120+
// A corrupt manifest version must not NaN-poison the order into 1
121+
// ("installed is newer") — that would suppress the align guidance.
122+
expect(compareVersions('v1.0.0', '1.0.0')).toBe(0)
123+
expect(compareVersions('1.0.x', '1.0.0')).toBe(0)
124+
expect(compareVersions('garbage', '1.0.0')).toBe(0)
125+
})
126+
119127
it('numeric identifiers sort below alphanumeric; shorter below longer', () => {
120128
expect(compareVersions('1.0.0-1', '1.0.0-alpha')).toBe(-1)
121129
expect(compareVersions('1.0.0-rc', '1.0.0-rc.1')).toBe(-1)

packages/cli/src/commands/init/steps/__tests__/install-deps.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,23 @@ describe('versionSkew', () => {
316316
expect(versionSkew(['@no-map/package'], {})).toEqual([])
317317
})
318318

319+
it('classifies a garbage installed version as behind (align guidance), not ahead', () => {
320+
present('@cipherstash/stack')
321+
resolvedVersions({ '@cipherstash/stack': 'not-a-version' })
322+
expect(
323+
versionSkew(['@cipherstash/stack'], {
324+
'@cipherstash/stack': '9.9.9-test.1',
325+
}),
326+
).toEqual([
327+
{
328+
pkg: '@cipherstash/stack',
329+
installed: 'not-a-version',
330+
expected: '9.9.9-test.1',
331+
direction: 'behind',
332+
},
333+
])
334+
})
335+
319336
it('flags an installed package with an unreadable manifest', () => {
320337
present('@cipherstash/stack')
321338
resolvedVersions({ '@cipherstash/stack': undefined })

packages/cli/src/runtime-versions.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,12 @@ export function pinnedSpec(
103103
* metadata — just enough to order the versions this repo publishes
104104
* (`x.y.z` and `x.y.z-rc.n`), so the skew warning can tell "behind" from
105105
* "ahead" without adding a dependency to the CLI.
106+
*
107+
* A version whose core isn't numeric (`v1.0.0`, `1.0.x`, garbage from a
108+
* corrupt manifest) is NOT COMPARABLE: return `0` rather than a
109+
* NaN-poisoned order. Callers classify non-ahead as behind, so an
110+
* unparseable installed version gets the safe treatment (align/reinstall
111+
* guidance) instead of being silently promoted to "newer, leave it".
106112
*/
107113
export function compareVersions(a: string, b: string): -1 | 0 | 1 {
108114
const parse = (v: string) => {
@@ -114,6 +120,7 @@ export function compareVersions(a: string, b: string): -1 | 0 | 1 {
114120
}
115121
const pa = parse(a)
116122
const pb = parse(b)
123+
if (pa.core.some(Number.isNaN) || pb.core.some(Number.isNaN)) return 0
117124
for (let i = 0; i < 3; i++) {
118125
const da = pa.core[i] ?? 0
119126
const db = pb.core[i] ?? 0

0 commit comments

Comments
 (0)