Skip to content

Commit 032a9da

Browse files
jdaltonjdalton
committed
Make npm fix on par with pnpm fix
1 parent 9d76169 commit 032a9da

File tree

3 files changed

+117
-47
lines changed

3 files changed

+117
-47
lines changed

‎src/commands/fix/npm-fix.ts‎

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { getManifestData } from '@socketsecurity/registry'
2+
import { arrayUnique } from '@socketsecurity/registry/lib/arrays'
23
import { logger } from '@socketsecurity/registry/lib/logger'
34
import { runScript } from '@socketsecurity/registry/lib/npm'
45
import {
@@ -14,6 +15,7 @@ import {
1415
SafeArborist
1516
} from '../../shadow/npm/arborist/lib/arborist'
1617
import {
18+
findPackageNode,
1719
findPackageNodes,
1820
getAlertsMapFromArborist,
1921
updateNode,
@@ -23,22 +25,23 @@ import { getCveInfoByAlertsMap } from '../../utils/socket-package-alert'
2325

2426
import type { SafeNode } from '../../shadow/npm/arborist/lib/node'
2527
import type { EnvDetails } from '../../utils/package-environment'
28+
import type { PackageJson } from '@socketsecurity/registry/lib/packages'
2629
import type { Spinner } from '@socketsecurity/registry/lib/spinner'
2730

2831
const { CI, NPM } = constants
2932

30-
type SaveAndInstallOptions = {
33+
type InstallOptions = {
3134
cwd?: string | undefined
3235
}
3336

34-
async function saveAndInstall(
37+
async function install(
3538
idealTree: SafeNode,
36-
options: SaveAndInstallOptions
39+
options: InstallOptions
3740
): Promise<void> {
3841
const { cwd = process.cwd() } = {
3942
__proto__: null,
4043
...options
41-
} as SaveAndInstallOptions
44+
} as InstallOptions
4245
const arb2 = new Arborist({ path: cwd })
4346
arb2.idealTree = idealTree
4447
await arb2.reify()
@@ -68,7 +71,7 @@ export async function npmFix(
6871
path: cwd,
6972
...SAFE_ARBORIST_REIFY_OPTIONS_OVERRIDES
7073
})
71-
74+
// Calling arb.reify() creates the arb.diff object and nulls-out arb.idealTree.
7275
await arb.reify()
7376

7477
const alertsMap = await getAlertsMapFromArborist(arb, {
@@ -87,42 +90,48 @@ export async function npmFix(
8790
return
8891
}
8992

90-
await arb.buildIdealTree()
91-
9293
const editablePkgJson = await readPackageJson(cwd, { editable: true })
94+
const { content: pkgJson } = editablePkgJson
9395

94-
for (const { 0: name, 1: infos } of infoByPkg) {
95-
const revertToIdealTree = arb.idealTree!
96-
arb.idealTree = null
96+
await arb.buildIdealTree()
9797

98-
// eslint-disable-next-line no-await-in-loop
99-
await arb.buildIdealTree()
100-
const tree = arb.idealTree!
98+
spinner?.stop()
10199

100+
for (const { 0: name, 1: infos } of infoByPkg) {
102101
const hasUpgrade = !!getManifestData(NPM, name)
103102
if (hasUpgrade) {
104103
spinner?.info(`Skipping ${name}. Socket Optimize package exists.`)
105104
continue
106105
}
107-
108-
const nodes = findPackageNodes(tree, name)
109-
106+
const specs = arrayUnique(
107+
findPackageNodes(arb.idealTree!, name).map(n => `${n.name}@${n.version}`)
108+
)
110109
const packument =
111-
nodes.length && infos.length
110+
specs.length && infos.length
112111
? // eslint-disable-next-line no-await-in-loop
113112
await fetchPackagePackument(name)
114113
: null
115114
if (!packument) {
116115
continue
117116
}
118117

119-
for (const node of nodes) {
118+
for (const spec of specs) {
119+
const lastAtSignIndex = spec.lastIndexOf('@')
120+
const name = spec.slice(0, lastAtSignIndex)
121+
const oldVersion = spec.slice(lastAtSignIndex + 1)
120122
for (const {
121123
firstPatchedVersionIdentifier,
122124
vulnerableVersionRange
123125
} of infos) {
126+
const revertTree = arb.idealTree!
127+
arb.idealTree = null
128+
// eslint-disable-next-line no-await-in-loop
129+
await arb.buildIdealTree()
130+
const node = findPackageNode(arb.idealTree!, name, oldVersion)
131+
if (!node) {
132+
continue
133+
}
124134
spinner?.stop()
125-
const { version: oldVersion } = node
126135
const oldSpec = `${name}@${oldVersion}`
127136
if (
128137
updateNode(
@@ -134,38 +143,54 @@ export async function npmFix(
134143
) {
135144
const targetVersion = node.package.version!
136145
const fixSpec = `${name}@^${targetVersion}`
146+
const revertData = {
147+
...(pkgJson.dependencies
148+
? { dependencies: pkgJson.dependencies }
149+
: undefined),
150+
...(pkgJson.optionalDependencies
151+
? { optionalDependencies: pkgJson.optionalDependencies }
152+
: undefined),
153+
...(pkgJson.peerDependencies
154+
? { peerDependencies: pkgJson.peerDependencies }
155+
: undefined)
156+
} as PackageJson
137157

138158
spinner?.start()
139159
spinner?.info(`Installing ${fixSpec}`)
140160

141161
let saved = false
142162
let installed = false
143163
try {
164+
updatePackageJsonFromNode(editablePkgJson, arb.idealTree!, node)
144165
// eslint-disable-next-line no-await-in-loop
145-
await saveAndInstall(arb.idealTree!, { cwd })
166+
await editablePkgJson.save()
146167
saved = true
168+
169+
// eslint-disable-next-line no-await-in-loop
170+
await install(arb.idealTree!, { cwd })
147171
installed = true
148172

149173
if (test) {
150174
spinner?.info(`Testing ${fixSpec}`)
151175
// eslint-disable-next-line no-await-in-loop
152176
await runScript(testScript, [], { spinner, stdio: 'ignore' })
153177
}
178+
spinner?.info(`Fixed ${name}`)
154179
// Lazily access constants.ENV[CI].
155180
if (constants.ENV[CI]) {
156181
// eslint-disable-next-line no-await-in-loop
157182
await openGitHubPullRequest(name, targetVersion, cwd)
158183
}
159-
updatePackageJsonFromNode(editablePkgJson, tree, node)
160-
// eslint-disable-next-line no-await-in-loop
161-
await editablePkgJson.save()
162-
spinner?.info(`Fixed ${name}`)
163184
} catch {
164185
spinner?.error(`Reverting ${fixSpec}`)
165-
if (saved || installed) {
166-
arb.idealTree = revertToIdealTree
186+
if (saved) {
187+
editablePkgJson.update(revertData)
188+
// eslint-disable-next-line no-await-in-loop
189+
await editablePkgJson.save()
190+
}
191+
if (installed) {
167192
// eslint-disable-next-line no-await-in-loop
168-
await saveAndInstall(arb.idealTree!, { cwd })
193+
await install(revertTree, { cwd })
169194
}
170195
spinner?.stop()
171196
logger.error(`Failed to fix ${oldSpec}`)

‎src/commands/fix/pnpm-fix.ts‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { readWantedLockfile } from '@pnpm/lockfile.fs'
22

33
import { getManifestData } from '@socketsecurity/registry'
4+
import { arrayUnique } from '@socketsecurity/registry/lib/arrays'
45
import { logger } from '@socketsecurity/registry/lib/logger'
56
import { runScript } from '@socketsecurity/registry/lib/npm'
67
import {
@@ -16,13 +17,15 @@ import {
1617
} from '../../shadow/npm/arborist/lib/arborist'
1718
import {
1819
findBestPatchVersion,
20+
findPackageNode,
1921
findPackageNodes,
2022
updatePackageJsonFromNode
2123
} from '../../utils/arborist-helpers'
2224
import { getAlertsMapFromPnpmLockfile } from '../../utils/pnpm-lock-yaml'
2325
import { getCveInfoByAlertsMap } from '../../utils/socket-package-alert'
2426
import { runAgentInstall } from '../optimize/run-agent'
2527

28+
import type { SafeNode } from '../../shadow/npm/arborist/lib/node'
2629
import type { EnvDetails } from '../../utils/package-environment'
2730
import type { PackageJson } from '@socketsecurity/registry/lib/packages'
2831
import type { Spinner } from '@socketsecurity/registry/lib/spinner'
@@ -81,42 +84,47 @@ export async function pnpmFix(
8184
return spinner?.stop()
8285
}
8386

87+
const editablePkgJson = await readPackageJson(cwd, { editable: true })
88+
const { content: pkgJson } = editablePkgJson
89+
8490
const arb = new SafeArborist({
8591
path: cwd,
8692
...SAFE_ARBORIST_REIFY_OPTIONS_OVERRIDES
8793
})
8894
await arb.loadActual()
8995

90-
const editablePkgJson = await readPackageJson(cwd, { editable: true })
91-
const { content: pkgJson } = editablePkgJson
92-
9396
spinner?.stop()
9497

9598
for (const { 0: name, 1: infos } of infoByPkg) {
96-
const tree = arb.actualTree!
97-
9899
if (getManifestData(NPM, name)) {
99100
logger.info(`Skipping ${name}. Socket Optimize package exists.`)
100101
continue
101102
}
102-
103-
const nodes = findPackageNodes(tree, name)
103+
const specs = arrayUnique(
104+
findPackageNodes(arb.actualTree!, name).map(n => `${n.name}@${n.version}`)
105+
)
104106
const packument =
105-
nodes.length && infos.length
107+
specs.length && infos.length
106108
? // eslint-disable-next-line no-await-in-loop
107109
await fetchPackagePackument(name)
108110
: null
109111
if (!packument) {
110112
continue
111113
}
112114

113-
for (const node of nodes) {
115+
for (const spec of specs) {
116+
const lastAtSignIndex = spec.lastIndexOf('@')
117+
const name = spec.slice(0, lastAtSignIndex)
118+
const oldVersion = spec.slice(lastAtSignIndex + 1)
114119
for (const {
115120
firstPatchedVersionIdentifier,
116121
vulnerableVersionRange
117122
} of infos) {
123+
const node = findPackageNode(arb.idealTree!, name, oldVersion)
124+
if (!node) {
125+
continue
126+
}
118127
spinner?.stop()
119-
const { version: oldVersion } = node
120128
const oldSpec = `${name}@${oldVersion}`
121129
const availableVersions = Object.keys(packument.versions)
122130
const targetVersion = findBestPatchVersion(
@@ -180,7 +188,7 @@ export async function pnpmFix(
180188
let installed = false
181189
try {
182190
editablePkgJson.update(updateData)
183-
updatePackageJsonFromNode(editablePkgJson, tree, node)
191+
updatePackageJsonFromNode(editablePkgJson, arb.actualTree!, node)
184192
// eslint-disable-next-line no-await-in-loop
185193
await editablePkgJson.save()
186194
saved = true
@@ -194,14 +202,14 @@ export async function pnpmFix(
194202
// eslint-disable-next-line no-await-in-loop
195203
await runScript(testScript, [], { spinner, stdio: 'ignore' })
196204
}
205+
206+
spinner?.info(`Fixed ${name}`)
207+
197208
// Lazily access constants.ENV[CI].
198209
if (constants.ENV[CI]) {
199210
// eslint-disable-next-line no-await-in-loop
200211
await openGitHubPullRequest(name, targetVersion, cwd)
201212
}
202-
// eslint-disable-next-line no-await-in-loop
203-
await editablePkgJson.save()
204-
spinner?.info(`Fixed ${name}`)
205213
} catch {
206214
spinner?.error(`Reverting ${fixSpec}`)
207215
if (saved) {
@@ -212,6 +220,9 @@ export async function pnpmFix(
212220
if (installed) {
213221
// eslint-disable-next-line no-await-in-loop
214222
await install(pkgEnvDetails, { spinner })
223+
arb.actualTree = null
224+
// eslint-disable-next-line no-await-in-loop
225+
await arb.loadActual()
215226
}
216227
spinner?.stop()
217228
logger.error(`Failed to fix ${oldSpec}`)

‎src/utils/arborist-helpers.ts‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,33 @@ export function findBestPatchVersion(
163163
return semver.maxSatisfying(eligibleVersions, '*')
164164
}
165165

166+
export function findPackageNode(
167+
tree: SafeNode,
168+
name: string,
169+
version?: string | undefined
170+
): SafeNode | undefined {
171+
const queue: Array<{ node: typeof tree }> = [{ node: tree }]
172+
let sentinel = 0
173+
while (queue.length) {
174+
if (sentinel++ === LOOP_SENTINEL) {
175+
throw new Error('Detected infinite loop in findPackageNodes')
176+
}
177+
const { node: currentNode } = queue.pop()!
178+
const node = currentNode.children.get(name)
179+
if (node && (typeof version !== 'string' || node.version === version)) {
180+
return node as unknown as SafeNode
181+
}
182+
const children = [...currentNode.children.values()]
183+
for (let i = children.length - 1; i >= 0; i -= 1) {
184+
queue.push({ node: children[i] as unknown as SafeNode })
185+
}
186+
}
187+
}
188+
166189
export function findPackageNodes(
167190
tree: SafeNode,
168-
packageName: string
191+
name: string,
192+
version?: string | undefined
169193
): SafeNode[] {
170194
const queue: Array<{ node: typeof tree }> = [{ node: tree }]
171195
const matches: SafeNode[] = []
@@ -175,8 +199,8 @@ export function findPackageNodes(
175199
throw new Error('Detected infinite loop in findPackageNodes')
176200
}
177201
const { node: currentNode } = queue.pop()!
178-
const node = currentNode.children.get(packageName)
179-
if (node) {
202+
const node = currentNode.children.get(name)
203+
if (node && (typeof version !== 'string' || node.version === version)) {
180204
matches.push(node as unknown as SafeNode)
181205
}
182206
const children = [...currentNode.children.values()]
@@ -321,39 +345,49 @@ export function updateNode(
321345
// No suitable patch version found.
322346
return false
323347
}
324-
// Use Object.defineProperty to override the version.
348+
// Object.defineProperty is needed to set the version property and replace
349+
// the old value with targetVersion.
325350
Object.defineProperty(node, 'version', {
326351
configurable: true,
327352
enumerable: true,
328353
get: () => targetVersion
329354
})
355+
// Update package.version associated with the node.
330356
node.package.version = targetVersion
331-
// Update resolved and clear integrity for the new version.
357+
// Update node.resolved.
332358
const purlObj = PackageURL.fromString(`pkg:npm/${node.name}`)
333359
node.resolved = `${NPM_REGISTRY_URL}/${node.name}/-/${purlObj.name}-${targetVersion}.tgz`
360+
// Update node.integrity with the targetPackument.dist.integrity value if available
361+
// else delete node.integrity so a new value is resolved for the target version.
334362
const { integrity } = targetPackument.dist
335363
if (integrity) {
336364
node.integrity = integrity
337365
} else {
338366
delete node.integrity
339367
}
340-
if ('deprecated' in targetPackument) {
368+
// Update node.package.deprecated based on targetPackument.deprecated.
369+
if (hasOwn(targetPackument, 'deprecated')) {
341370
node.package['deprecated'] = targetPackument.deprecated as string
342371
} else {
343372
delete node.package['deprecated']
344373
}
374+
// Update node.package.dependencies.
345375
const newDeps = { ...targetPackument.dependencies }
346376
const { dependencies: oldDeps } = node.package
347377
node.package.dependencies = newDeps
348378
if (oldDeps) {
349379
for (const oldDepName of Object.keys(oldDeps)) {
350380
if (!hasOwn(newDeps, oldDepName)) {
381+
// Detach old edges for dependencies that don't exist on the updated
382+
// node.package.dependencies.
351383
node.edgesOut.get(oldDepName)?.detach()
352384
}
353385
}
354386
}
355387
for (const newDepName of Object.keys(newDeps)) {
356388
if (!hasOwn(oldDeps, newDepName)) {
389+
// Add new edges for dependencies that don't exist on the old
390+
// node.package.dependencies.
357391
node.addEdgeOut(
358392
new Edge({
359393
from: node,

0 commit comments

Comments
 (0)