Skip to content

Commit 7529855

Browse files
committed
fix: swap arguments to isNotSpecial in tidy() to correctly preserve bin/ and current
The `isNotSpecial` function in `tidy()` was called with swapped arguments: `isNotSpecial(this.config.version, f.path)` instead of `isNotSpecial(f.path, this.config.version)`. This caused `isNotSpecial` to always return `true` for every entry, meaning the only protection against deletion was the 42-day age check. After 42 days, `tidy()` would delete all entries including `bin/` and `current`, breaking the CLI installation. The `bin/` directory is particularly vulnerable because `createBin()` overwrites the file inside it (via `writeFile`) which does not update the directory's mtime — so the directory retains its original mtime from installation and eventually exceeds the 42-day threshold. fixes: #1289
1 parent 8c84095 commit 7529855

File tree

2 files changed

+93
-2
lines changed

2 files changed

+93
-2
lines changed

src/update.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ ${binPathEnvVar}="\$DIR/${bin}" ${redirectedEnvVar}=1 "$DIR/../${version}/bin/${
216216

217217
await Promise.all(
218218
files
219-
.filter((f) => isNotSpecial(this.config.version, f.path) && isOld(f.stat))
219+
.filter((f) => isNotSpecial(f.path, this.config.version) && isOld(f.stat))
220220
.map((f) => rm(f.path, {force: true, recursive: true})),
221221
)
222222
} catch (error: unknown) {

test/update.test.ts

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {expect} from 'chai'
33
import {got} from 'got'
44
import nock from 'nock'
55
import {existsSync} from 'node:fs'
6-
import {mkdir, rm, symlink, writeFile} from 'node:fs/promises'
6+
import {mkdir, rm, symlink, utimes, writeFile} from 'node:fs/promises'
77
import path from 'node:path'
88
import zlib from 'node:zlib'
99
import sinon from 'sinon'
@@ -42,6 +42,33 @@ function initUpdater(config: Config): Updater {
4242
return updater
4343
}
4444

45+
const setOldMtime = async (filePath: string): Promise<void> => {
46+
const oldDate = new Date()
47+
oldDate.setDate(oldDate.getDate() - 43)
48+
await utimes(filePath, oldDate, oldDate)
49+
}
50+
51+
const setupTidyClientRoot = async (config: Interfaces.Config): Promise<string> => {
52+
const root = config.scopedEnvVar('OCLIF_CLIENT_HOME') || path.join(config.dataDir, 'client')
53+
await mkdir(root, {recursive: true})
54+
55+
// Create current version directory (matches config.version = '2.0.0')
56+
const versionDir = path.join(root, '2.0.0')
57+
await mkdir(path.join(versionDir, 'bin'), {recursive: true})
58+
await writeFile(path.join(versionDir, 'bin', config.bin), 'binary', 'utf8')
59+
60+
// Create bin/ directory with launcher script
61+
await mkdir(path.join(root, 'bin'), {recursive: true})
62+
await writeFile(path.join(root, 'bin', config.bin), '../2.0.0/bin', 'utf8')
63+
64+
// Create current symlink
65+
if (!existsSync(path.join(root, 'current'))) {
66+
await symlink(path.join(root, '2.0.0'), path.join(root, 'current'))
67+
}
68+
69+
return root
70+
}
71+
4572
describe('update plugin', () => {
4673
let config: Config
4774
let updater: Updater
@@ -298,4 +325,68 @@ describe('update plugin', () => {
298325
const stdout = stripAnsi(collector.stdout.join(' '))
299326
expect(stdout).to.matches(/Updating to a specific version will not update the channel/)
300327
})
328+
329+
describe('tidy', () => {
330+
it('should preserve bin, current, and active version directories during tidy', async () => {
331+
clientRoot = await setupTidyClientRoot(config)
332+
333+
// Create old version directory that should be cleaned up
334+
const oldVersionDir = path.join(clientRoot, '1.0.0-abc1234')
335+
await mkdir(path.join(oldVersionDir, 'bin'), {recursive: true})
336+
await writeFile(path.join(oldVersionDir, 'bin', 'example-cli'), 'old version', 'utf8')
337+
338+
// Backdate the old version directory to be older than 42 days
339+
await setOldMtime(oldVersionDir)
340+
341+
// Also backdate bin/ and current to verify they are preserved even when old
342+
await setOldMtime(path.join(clientRoot, 'bin'))
343+
await setOldMtime(path.join(clientRoot, 'current'))
344+
345+
// Backdate the active version directory too - it should still be preserved
346+
await setOldMtime(path.join(clientRoot, '2.0.0'))
347+
348+
// Trigger tidy via runUpdate (already on same version, but tidy still runs)
349+
const manifestRegex = new RegExp(
350+
`channels\\/stable\\/example-cli-${config.platform}-${config.arch}-buildmanifest`,
351+
)
352+
nock(/oclif-staging.s3.amazonaws.com/)
353+
.get(manifestRegex)
354+
.reply(200, {version: '2.0.0'})
355+
356+
updater = initUpdater(config)
357+
await updater.runUpdate({autoUpdate: false})
358+
359+
// Verify bin/ and current survived even though they are old
360+
expect(existsSync(path.join(clientRoot, 'bin'))).to.be.true
361+
expect(existsSync(path.join(clientRoot, 'current'))).to.be.true
362+
363+
// Verify active version directory survived even though it is old
364+
expect(existsSync(path.join(clientRoot, '2.0.0'))).to.be.true
365+
366+
// Verify old version was cleaned up
367+
expect(existsSync(oldVersionDir)).to.be.false
368+
})
369+
370+
it('should not delete entries newer than 42 days', async () => {
371+
clientRoot = await setupTidyClientRoot(config)
372+
373+
// Create a recent version directory (should survive)
374+
const recentVersionDir = path.join(clientRoot, '1.9.0-def5678')
375+
await mkdir(path.join(recentVersionDir, 'bin'), {recursive: true})
376+
await writeFile(path.join(recentVersionDir, 'bin', 'example-cli'), 'recent version', 'utf8')
377+
378+
const manifestRegex = new RegExp(
379+
`channels\\/stable\\/example-cli-${config.platform}-${config.arch}-buildmanifest`,
380+
)
381+
nock(/oclif-staging.s3.amazonaws.com/)
382+
.get(manifestRegex)
383+
.reply(200, {version: '2.0.0'})
384+
385+
updater = initUpdater(config)
386+
await updater.runUpdate({autoUpdate: false})
387+
388+
// Recent version should survive
389+
expect(existsSync(recentVersionDir)).to.be.true
390+
})
391+
})
301392
})

0 commit comments

Comments
 (0)