Skip to content

Conversation

fflorent
Copy link
Contributor

@fflorent fflorent commented Aug 7, 2025

Context

Passing the dir option to tmp.dir() results in a strange error:

> var tmp = require('./lib/tmp')
undefined
> var os = require('os')
undefined
> tmp.dir({dir: os.tmpdir()}, (err, val) => { console.log(err, val); })
undefined
> [Error: ENOENT: no such file or directory, lstat '/home/florentpro/projects/node-tmp/[object Object]'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'lstat',
  path: '/home/florentpro/projects/node-tmp/[object Object]'
} undefined

Fix proposal

  1. When reading the code, I could find that this line is the culprit (path is defined but refers to the node:path library):
    fs.realpath(path, cb);
  2. When running the tests, istanbul suggests that this line is only executed 2 times, and only for errors, I add a test case to inbandStandard to check passing the dir option

My guess is that the variable that should have been used is pathToResolve like in the implementation of _resolvePathSync:

return fs.realpathSync(pathToResolve);

The new tests fail without the fix and pass with the fix.

Copy link
Owner

@raszi raszi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! Thank you!

@raszi raszi merged commit e162828 into raszi:master Aug 8, 2025
8 checks passed
@fflorent fflorent deleted the fix-tmp-dir-with-dir branch August 8, 2025 21:20
@raszi
Copy link
Owner

raszi commented Aug 8, 2025

Released as v2.0.5

paulfitz pushed a commit to gristlabs/grist-core that referenced this pull request Aug 11, 2025
## Context

See #1753 

But the tests fail.

It fixes a vulnerability (with a low score):
GHSA-52f5-9888-hmc6

EDIT: I also have found that the CI does not pass due to a regression by
the `tmp` library:
raszi/node-tmp#309

## Proposed solution

1. Also upgrade the type definition
2. Adapt the tests to make it compile
3. See whether the CI passes 👀
4. Fix the node-tmp library ([Pull-Request
accepted](raszi/node-tmp#309))
5. Remove any type override of `tmp`, remove use of Bluebirds'
`promisifyAll` (I am not very comfortable with altering the prototype
globally) and rather use the already imported library `tmp-promise`.

## Has this been tested?

<!-- Put an `x` in the box that applies: -->

- [x] 👍 yes, I added tests to the test suite
- [ ] 💭 no, because this PR is a draft and still needs work
- [ ] 🙅 no, because this is not relevant here
- [ ] 🙋 no, because I need help <!-- Detail how we can help you -->

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants