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

Add option to mask command #32

Closed
s0rthak opened this issue Jan 21, 2021 · 1 comment · Fixed by #33
Closed

Add option to mask command #32

s0rthak opened this issue Jan 21, 2021 · 1 comment · Fixed by #33

Comments

@s0rthak
Copy link
Contributor

s0rthak commented Jan 21, 2021

Hi,

It is possible to add an option to hide the command information in case of errors thrown?

In the execute method, right now if there is an error, due to the way node child_process.exec works, the library returns the full command plus whatever was in stdout/stderr. This can be a security risk as the full command would contain the password as well.

execute(cmd, cmdArgs, workingDir) {
    const fullCmd = wrap(util.format("%s %s", cmd, cmdArgs));
    const command = [
      "smbclient",
      this.getSmbClientArgs(fullCmd).join(" "),
    ].join(" ");

    const options = {
      cwd: workingDir || "",
    };

    return new Promise((resolve, reject) => {
      exec(command, options, function (err, stdout, stderr) {
        const allOutput = stdout + stderr;

        if (err) {
          err.message += allOutput;
          return reject(err); // Has information about the full command
        }

        return resolve(allOutput);
      });
    });
  }

I'll be happy to contribute a pull request to add maskCmd as an additional option and if it is set to true, only return the stdout/stderr output instead of the whole command.

Let me know, thanks!

@nateevans
Copy link

@s0rthak sound good to me. Shoot a PR over and we'll check it out. Thanks

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 a pull request may close this issue.

2 participants