diff --git a/packages/cli/src/commands/scale.test.ts b/packages/cli/src/commands/scale.test.ts index e4c3b6bb..ccf5c722 100644 --- a/packages/cli/src/commands/scale.test.ts +++ b/packages/cli/src/commands/scale.test.ts @@ -11,6 +11,9 @@ import { saveFleet, loadRollouts, saveRollouts, + parsePositiveSafeInteger, + parseNonNegativeSafeInteger, + parsePositiveFiniteNumber, } from './scale.js'; // Helper to create a temp dir and override CREDS_FILE path @@ -28,6 +31,30 @@ afterEach(() => { } }); +describe('scale numeric option parsers', () => { + it('accepts safe integer counts and rejects invalid count values', () => { + expect(parsePositiveSafeInteger('3')).toBe(3); + for (const invalid of ['nope', '1.5', '0', '-1', 'Infinity', '9007199254740992']) { + expect(() => parsePositiveSafeInteger(invalid)).toThrow('positive safe integer'); + } + }); + + it('allows zero only for non-negative safe integer options', () => { + expect(parseNonNegativeSafeInteger('0')).toBe(0); + expect(parseNonNegativeSafeInteger('4')).toBe(4); + for (const invalid of ['-1', '1.5', 'NaN', 'Infinity']) { + expect(() => parseNonNegativeSafeInteger(invalid)).toThrow('non-negative safe integer'); + } + }); + + it('accepts positive finite prices and rejects non-finite or non-positive values', () => { + expect(parsePositiveFiniteNumber('1.25')).toBe(1.25); + for (const invalid of ['nope', '0', '-1', 'Infinity', '-Infinity']) { + expect(() => parsePositiveFiniteNumber(invalid)).toThrow('positive finite number'); + } + }); +}); + // --------------------------------------------------------------------------- // getNextId // --------------------------------------------------------------------------- diff --git a/packages/cli/src/commands/scale.ts b/packages/cli/src/commands/scale.ts index 5b433a10..7d2e52e1 100644 --- a/packages/cli/src/commands/scale.ts +++ b/packages/cli/src/commands/scale.ts @@ -1,4 +1,4 @@ -import { Command } from 'commander'; +import { Command, InvalidArgumentError } from 'commander'; import kleur from 'kleur'; import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs'; import { homedir } from 'node:os'; @@ -12,6 +12,30 @@ import { deployCmd } from './deploy.js'; const CREDS_FILE = join(homedir(), '.sh1pt', 'credentials.json'); const ROLLOUTS_FILE = join(homedir(), '.sh1pt', 'rollouts.json'); +export function parsePositiveSafeInteger(value: string): number { + const parsed = Number(value); + if (!Number.isSafeInteger(parsed) || parsed < 1) { + throw new InvalidArgumentError('must be a positive safe integer'); + } + return parsed; +} + +export function parseNonNegativeSafeInteger(value: string): number { + const parsed = Number(value); + if (!Number.isSafeInteger(parsed) || parsed < 0) { + throw new InvalidArgumentError('must be a non-negative safe integer'); + } + return parsed; +} + +export function parsePositiveFiniteNumber(value: string): number { + const parsed = Number(value); + if (!Number.isFinite(parsed) || parsed <= 0) { + throw new InvalidArgumentError('must be a positive finite number'); + } + return parsed; +} + export interface FleetEntry { id: string; provider: string; @@ -165,9 +189,9 @@ scaleCmd.addCommand(deployCmd); scaleCmd .command('up') .description('Buy more instances of the current SKU (via sh1pt deploy under the hood)') - .option('--instances ', 'how many to add', Number, 1) + .option('--instances ', 'how many to add', parsePositiveSafeInteger, 1) .option('--provider ', 'which cloud provider to add to (default: same as existing fleet, or first in pricing table)') - .option('--max-hourly-price ', 'abort if the new instances would push above this total/hr', Number) + .option('--max-hourly-price ', 'abort if the new instances would push above this total/hr', parsePositiveFiniteNumber) .option('--dry-run', 'show the plan without modifying state') .action((opts: { instances: number; @@ -262,7 +286,7 @@ scaleCmd scaleCmd .command('down') .description('Tear down instances (cheapest / least-healthy first)') - .option('--instances ', 'number of instances to destroy', Number, 1) + .option('--instances ', 'number of instances to destroy', parsePositiveSafeInteger, 1) .option('--provider ', 'only remove instances from this cloud provider') .option('--dry-run', 'show the plan without modifying state') .option('--json', 'machine-readable output') @@ -371,10 +395,10 @@ scaleCmd scaleCmd .command('auto') .description('Set auto-scale rules (sh1pt cloud polls metrics and runs scale up/down on your behalf)') - .option('--min ', 'minimum instances', Number, 1) - .option('--max ', 'maximum instances', Number, 10) - .option('--target-cpu ', 'target CPU utilization to maintain', Number, 70) - .option('--cooldown ', 'minimum time between scale events', Number, 300) + .option('--min ', 'minimum instances', parseNonNegativeSafeInteger, 1) + .option('--max ', 'maximum instances', parsePositiveSafeInteger, 10) + .option('--target-cpu ', 'target CPU utilization to maintain', parsePositiveSafeInteger, 70) + .option('--cooldown ', 'minimum time between scale events', parsePositiveSafeInteger, 300) .option('--status', 'show current auto-scale rules') .option('--dry-run', 'show the rules without saving') .option('--json', 'machine-readable output') @@ -497,7 +521,7 @@ scaleCmd .description('Wire round-robin DNS so traffic spreads across the fleet') .requiredOption('--provider ', 'dns-porkbun | dns-cloudflare') .requiredOption('--domain ', 'e.g. api.example.com') - .option('--ttl ', 'TTL for DNS records', Number, 60) + .option('--ttl ', 'TTL for DNS records', parsePositiveSafeInteger, 60) .option('--proxied', 'cloudflare only — route through the CF edge (orange cloud)') .option('--dry-run', 'show the DNS records that would be created/updated') .option('--json', 'machine-readable output') @@ -609,7 +633,7 @@ scaleCmd .description('Stage a new version across the fleet (canary / blue-green / rolling)') .requiredOption('--version ', 'version identifier to deploy (e.g. v2.1.0)') .option('--strategy ', 'canary | blue-green | rolling', 'canary') - .option('--percent ', 'canary only — start at N% of traffic', Number, 5) + .option('--percent ', 'canary only — start at N% of traffic', parsePositiveSafeInteger, 5) .option('--dry-run', 'show the plan without modifying state') .option('--status', 'show active rollouts and their state') .option('--rollback ', 'roll back a previously completed rollout by ID')