Bug 1414035

Summary: [RFE] npm plugin should collect npm configuration
Product: Red Hat Enterprise Linux 7 Reporter: Miroslav Hradílek <mhradile>
Component: sosAssignee: Pavel Moravec <pmoravec>
Status: CLOSED ERRATA QA Contact: Miroslav Hradílek <mhradile>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.3CC: agk, bmr, gavin, jpopelka, mhradile, plambri, sbradley
Target Milestone: rcKeywords: FutureFeature
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
URL: https://github.com/sosreport/sos/pull/917
Whiteboard:
Fixed In Version: sos-3.4-1.el7 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
: 1414036 (view as bug list) Environment:
Last Closed: 2017-08-01 23:08:12 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1414036    

Description Miroslav Hradílek 2017-01-17 15:11:15 UTC
Description of problem:
Currently npm plugin does not collect npm configuration and npmrc files. It would be useful to know how npm is configured globally and what user and project specific modifications were taken.

For example whether user is using different registry would be good to know.

I believe collecting output of this command "npm config list -l" would suffice.

Version-Release number of selected component (if applicable):
sos-3.3-5.el7_3.noarch


Actual results:
No npm configuration information is collected.

Expected results:
Output of "npm config list -l" is collected.

Additional info:
It should call the command when project is specified to pick up project specific changes made in project npmrc or package.json.

Comment 1 Pavel Moravec 2017-01-18 08:26:00 UTC
I am bit confused now.

Per my understanding from [1], "npm config list -l" shows all the config settings in a long format. Sounds reasonable to collect (cant there be a secret information we should obfuscate?).

> It should call the command when project is specified to pick up project specific changes made in project npmrc or package.json.

So there shall be a trigger / condition before the command execution? Or shall be the npm config list executed every time?


[1] https://docs.npmjs.com/cli/config

Comment 2 Miroslav Hradílek 2017-01-18 10:29:04 UTC
> So there shall be a trigger / condition before the command execution? Or shall be the npm config list executed every time?

Currently in the plugin "npm ls -g" is collected every time as "npm-ls-global" and if project path is specified "npm ls" within project directory is collected in addition as "npm-ls-project".

I would mirror this behavior and called "npm config list -l" every time and called the same command within project directory if specified. There will be redundant data but the projject directory variant will contain the project specific changes if there were any.

> cant there be a secret information we should obfuscate?

Good idea. I need to investigate this, not sure where npm repository credentials for uploading packages are stored.

Comment 4 Miroslav Hradílek 2017-01-18 14:09:05 UTC
So I tried to login to the repository and I was required to provide credentials every time. This created auth token in my ~/.npmrc which is hidden from "mpm config ls -l".

Documentation refered to some old repositories with different auth that stores credentials. I could not find the npmrc entries documented but i found this for example
https://github.com/npm/npm/issues/1300#issuecomment-1886999

Those credentials should be omitted from "npm config ls" anyway. I tried to inject "_auth = bnBtOm5wbQ==" in my .npmrc and it was ignored by "npm config ls" while it was consumed by "npm login" prefilling the credentials.

Bottom line: I guess that nothing sensitive should be contained in "npm config ls -l"

Comment 5 Pavel Moravec 2017-01-19 20:20:31 UTC
OK thanks. So a patch like:

diff --git a/sos/plugins/npm.py b/sos/plugins/npm.py
index 634325f..dcba164 100644
--- a/sos/plugins/npm.py
+++ b/sos/plugins/npm.py
@@ -90,7 +90,11 @@ class Npm(Plugin, RedHatPlugin, DebianPlugin, UbuntuPlugin, SuSEPlugin):
                 self.get_option("project_path")))
             self._get_npm_output("npm ls --json", "npm-ls-project",
                                  working_directory=project_path)
+            self._get_npm_output("npm config list -l", "npm-config-list-project",
+                                 working_directory=project_path)
+
         self._get_npm_output("npm ls -g --json", "npm-ls-global")
+        self._get_npm_output("npm config list -l", "npm-config-list-global")
         self._find_modules_in_npm_cache()
 

correct?

(maybe shorten npm-config-list-* to npm-config-* ?)

Comment 6 Miroslav Hradílek 2017-01-20 07:17:16 UTC
Yep. "ls" is a valid abbreviation so it could be "npm-config-ls" but I would keep the full form for clarity.

Thanks Pavel.

Comment 7 Bryn M. Reeves 2017-01-20 10:46:54 UTC
Normally files in sos_commands use '_' as the 'space replacement', and '-' is only used when it appears in option strings.

The names chosen should be as desired for the component being collected (so 'npm_config_list_*' is fine), but they should conform to the normal formatting so as not to confuse automated report parsers.

Comment 8 Pavel Moravec 2017-01-21 11:30:15 UTC
(In reply to Bryn M. Reeves from comment #7)
> Normally files in sos_commands use '_' as the 'space replacement', and '-'
> is only used when it appears in option strings.
> 
> The names chosen should be as desired for the component being collected (so
> 'npm_config_list_*' is fine), but they should conform to the normal
> formatting so as not to confuse automated report parsers.

https://github.com/sosreport/sos/pull/917 created.

Mirek, pls. comment in the PR or here if you wish the output files shall have '_' instead of '-' in their names, and if so I will update the PR accordingly.

Comment 9 Miroslav Hradílek 2017-01-23 11:13:21 UTC
It does not matter to me. It can be separated with underscores. I was just copying the format used for currently collected commands.

Whether you are supposed to change the old commands too is up to the original reporters.

Comment 10 Pavel Moravec 2017-03-12 21:49:25 UTC
POSTed to upstream in  	581277b

Comment 13 errata-xmlrpc 2017-08-01 23:08:12 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:2203