fix(cli): make uipro update actually upgrade the CLI via npm (supersedes #326)#389
Conversation
`uipro update` only printed manual instructions when a newer release existed; it never upgraded the package, so users stayed on stale assets. It now runs the global install itself when behind: - execFileSync with an explicit args array (no shell string to expand), per the review on nextlevelbuilder#326 — not execSync. - Windows uses npm.cmd with shell:true (Node won't spawn a .cmd without a shell post-CVE-2024-27980); POSIX runs npm with no shell at all. - The release version is validated as semver before use, so nothing unexpected can reach the Windows shell. - On failure it falls back to manual instructions; on success it tells the user to rerun `uipro init --ai <platform> --force`. README updated to describe the new behavior. Supersedes nextlevelbuilder#326. Credit to @alfredo-petri for the original fix. Co-authored-by: alfredo-petri <146951566+alfredo-petri@users.noreply.github.com>
|
@mrgoonie more clean up duty |
mrgoonie
left a comment
There was a problem hiding this comment.
Summary: approved. This PR fixes the stale updater path with the safer shape requested on #326: POSIX uses execFileSync with an explicit argv array and no shell, Windows is limited to npm.cmd with shell:true, and the release tag is normalized/semver-gated before it can reach that Windows shell path. The README now clearly says update the global CLI first, then rerun init with --force.nnEvidence checked: reviewed cli/src/commands/update.ts and cli/README.md diff; compared against the prior #326 review; searched related PRs/issues (#326, #353, #362); verified the current code keeps the already-current case refreshing local bundled assets.nnLocal verification note: npm run typecheck could not run in this cron workspace because node_modules/tsc is not installed, so I did not treat that as a code failure.nnVerdict: APPROVE.
What & why
uipro updatechecked the latest release but, when the installed CLI was behind, only printednpm install -g uipro-cli@…and returned — it never actually upgraded. Users stayed on stale assets (the root of #353 for npm consumers). This makesuipro updateperform the upgrade itself.Change
When the installed version is behind the latest release,
updateCommandnow runs the global install:Addressing the review on #326:
execFileSyncwith an explicit args array instead ofexecSync('npm install -g …')— no shell string is built or expanded on the common path.npmisnpm.cmd, which Node refuses to spawn without a shell since CVE-2024-27980, so Windows usesnpm.cmd+shell: true; POSIX runs with no shell at all.latestVersionis validated as semver (/^\d+\.\d+\.\d+([.-][0-9A-Za-z.-]+)?$/) before it can reach the Windows shell — e.g. a tag like2.7.0; rm -rf /is rejected and falls back to manual instructions.uipro init --ai <platform> --force(the maintainer's suggestion). README updated to match.Verification
npm run typecheckpasses.2.7.0,2.8.0-beta.1; rejects2.7.0; rm -rf /,latest && evil, empty.(Can't exercise a real global install in CI without side effects; the install path is
execFileSyncwith a validated static arg array.)Supersedes #326 (resolves the
execSync/shell review finding + Windows.cmdhandling + version validation). Credit to @alfredo-petri for the original fix.