Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False alarm for “Found fs.readFile with non literal argument at index 0”? #65

Closed
hongbo-miao opened this issue Aug 5, 2020 · 5 comments · Fixed by #109
Closed

Comments

@hongbo-miao
Copy link

Originally asked at https://stackoverflow.com/questions/63262683/how-to-fix-found-fs-readfile-with-non-literal-argument-at-index-0

Copy to here:


I am trying to add eslint-plugin-security in a TypeScript project. However, for these codes

import { promises as fsp } from 'fs';
import fs from 'fs';
import path from 'path';

const index = await fsp.readFile(path.resolve(__dirname, './index.html'), 'utf-8');
const key = fs.readFileSync(path.join(__dirname, './ssl.key'));
await fsp.writeFile(path.resolve(__dirname, './sitemap.xml'), sitemap);

I got many these ESLint warnings:

warning Found fs.readFile with non literal argument at index 0   security/detect-non-literal-fs-filename
warning Found fs.readFileSync with non literal argument at index 0  security/detect-non-literal-fs-filename
warning Found fs.writeFile with non literal argument at index 0  security/detect-non-literal-fs-filename

I found the document about this ESLint error at https://github.com/nodesecurity/eslint-plugin-security#detect-non-literal-fs-filename

But I still have no idea how to fix it. Any guide will be helpful! Thanks


UPDATE:

Found out as long as using passing the path returned by path.join or path.resolve will show this ESLint issue.

If I change to absolute path, the ESLint issue is gone. However, this loose the benefit of the relative path by path.join or path.resolve.

fs.readFileSync('/Users/me/project/ssl.key');

Looking for an alternative / better way if exists.

@Tjerk-Haaye-Henricus
Copy link

Tjerk-Haaye-Henricus commented Sep 3, 2020

Same for fs.link and fs.exists. I've created my own lib which have a .link and .exists property and I'm getting those issues

@Deepesh316
Copy link

Any fix for this?

@ankurnarkhede
Copy link

Any update on this?

@Elindorath
Copy link

I think your example could be treated as a false alarm.

But the rule works as expected. It basically reports all variables usage in a file path argument.
This should be a valid warning only if the variables used are holding some user input but eslint can't do much better with code static analysis.
As showned in the README.md, this plugin might raise a lot of false positive.

This is why the call fs.readFileSync('/Users/me/project/ssl.key'); is not reported. But you should definitely stick to using the path package.

If you are sure that no user input can reach your fs method calls, you should disable the rule for the offending line with :

/* eslint-disable-next-line security/detect-non-literal-fs-filename -- Safe as no value holds user input */
const index = await fsp.readFile(path.resolve(__dirname, './index.html'), 'utf-8');

@kamal250
Copy link

I have another example. I am trying to open MatDialog with a component in Angular so I have to disable the rule for this line:

const dialogRef = this.dialog.open(ConfirmationPopupComponent, {
  data: {
    title: 'DELETE MOCKUPS',
    message: ' Are you sure you want to delete the mockups? ',
    buttonOneName: this.translation.translate('DELETE'),
    buttonTwoName: this.translation.translate('CANCEL'),
    from: 'mockup',
  },
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants