Skip to content

fix: swap arguments to isNotSpecial in tidy() to correctly preserve bin/ and current#1290

Merged
jfeingold35 merged 1 commit intooclif:mainfrom
khaled4vokalz:fix/tidy-swapped-args
Mar 24, 2026
Merged

fix: swap arguments to isNotSpecial in tidy() to correctly preserve bin/ and current#1290
jfeingold35 merged 1 commit intooclif:mainfrom
khaled4vokalz:fix/tidy-swapped-args

Conversation

@khaled4vokalz
Copy link
Contributor

What

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

How to test

  • Install a CLI that uses @oclif/plugin-update from this branch, probably using yalc
  • touch -t $(date -v-43d +%Y%m%d%H%M.%S) ~/.local/share/myapp/client/bin
  • Trigger an update (myapp update or via autoupdate)
  • Observe that client/bin/ is deleted by tidy()

…in/ 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: oclif#1289
@jfeingold35 jfeingold35 merged commit 97bec25 into oclif:main Mar 24, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tidy() deletes all old entries including bin/ due to swapped arguments in isNotSpecial

2 participants