rfcs
rfcs copied to clipboard
RFC: Add configurable data to HTTP header
What / Why
A proposition to add npm-app-id
to the HTTP header of npm tool.
Some questions.
Is APP_ID
an opaque machine-generated token, or something with meaning that the user sets? If it's machine-generated, can the registry provide it, rather than the CLI?
Can we put it in package.json instead of package-lock.json, since package-lock.json is more ephemeral? (Users sometimes blow the package-lock away to get a fresh install. Maybe they'll do that less in npm v7, but who knows, habits die hard.)
Does it need to be (a) unique to each project, (b) unique to each top level project (but shared among workspaces in a monorepo, for example), (c) shared among each package in a user-defined concept of what an "app" is?
Can we spell it appid
instead of APP_ID
?
I think it'd be a nice simple thing for a registry to provide a npm-app-id
HTTP header in the response if it doesn't get an appid
, saying "this is the appid you should use", and then the CLI could just stash it in package.json if it gets one, and repeat it back on subsequent requests. A registry server would have to be smart enough to assign the same appid for each new request in a single session (since the CLI sends a bunch of requests out all in parallel without waiting for their responses), but we have the npm-session-id
header, so that would be possible.
And if the user does send an npm-app-id
header, then the registry would just echo it back.
This would also allow us to do other interesting things in the future, too, especially if an app could be associated with an org or user, and different types of access attached to that indicator. Since it's user-input, we couldn't trust it on its own, but in combination with a login token, we could say that a given appid associated with your organization can only be installed in your production IP block by certain user accounts, so you don't accidentally push to production when you thought you were doing a staging/local build, or something.
Hi, @isaacs here my answers:
APP_ID
sets a user while using the third-party app: Nexus Repository Manager only for npm audit
. It is similar to the NODE_ENV
and how it is used in Express.
npm audit
provides package-lock.json
in the request body, so we can't use package.json
(a) - It may be unique to each project.
Yes, we can use 'appid' instead of APP_ID
.
We can also provide npm-app-id
HTTP header like npm-session-id
. It is no matter how it will be fetched: from the package-lock.json
or HTTP header. For HTTP header I don't know how much effort it will take to implement such functionality, but maybe it the most correct way from the npm CLI perspective. With metadata, we just need to add one line of code: meta.app_id = process.env.APP_ID
And if the user does send an npm-app-id header
User should set npm-app-id
in the env var or in the package.json
and after npm audit
this value should appear in the HTTP header.
I agree it would be nice to implement it as much as useful for CLI tool and provide more flexibility for npm users.
npm audit provides package-lock.json in the request body, so we can't use package.json
That isn't really relevant. We modify the lockfile data that we send anyway, and have the package.json data readily at hand at that time, so it can reside in either place. I feel like "what app is this" is more appropriate for package.json
to define than package-lock.json
, which is a definition of the dependency package tree as it's laid out on disk.
Also, we're not likely to add a feature like this in npm v6, and we're considering a much more streamlined API for fetching bulk advisories in npm v7 anyway. So, sky's the limit :)
@isaacs yes, I agree that package-lock.json
is not the right place to add such custom variable. I'm not a much familiar with npm tool. So what do you think if we move app-id
to the HTTP header (which will appear in npm install
/ npm audit
) and this value will be taken from package.json
or env variable?
@isaacs I've updated RFC. Can someone approve these changes to start working under the implementation?
I have no objection to the basic idea, but it does need some work to get over the finish line. (Your continued engagement is encouraged and welcome, but not required, if you have other things to do :)
The next step is to discuss it in our weekly Open RFC meeting (which you're invited to attend!) to quickly discuss some of the ramifications of the change, decide next steps, and eventually fit it into our roadmap. I would like to drive towards some more clarity around implementation, alternatives, and prior art, just to have some confidence that we're making the right choices and not opening the door to supporting a mistake forever. Since this is going to be an interface between the client and server, it's important to get the spec as detailed as possible.
The implementation itself (as proposed, anyway) is relatively straightforward, and I don't expect it'll have much impact for other parts of the system, so it should be pretty easy to get landed once we nail down the spec.
Summary from the OpenRFC Meeting:
-
app-id
may be added to the HTTP header and can be provided by a user from the npm command parameter or env variable or npm config file - Provide a hook for assigning metadata to an application. Make more general mechanism to add some custom user’s info to the npm commands.
The first idea is very simple to implement and have no risk for npm-registry/npm-tool. The second one is more generic and allow a user to add some custom metadata which can be used by third-party apps and maybe by npm-tool itself.
@isaacs From my perspective of view, it is not clear how to add such a hook mechanism to the npm tool. Also not clear how a user will add such metadata info: from the npm console or npm config file.
Will be it stored in the package.json/package-lock.json
or will it be provided in the HTTP header?
@jlstephens89 You can also watch the full discussion, if you wanna feel like you were there :) https://www.youtube.com/watch?v=W6XbJ5e3xrA
Hi @isaacs I've created the short Demo https://youtu.be/aMFbEYgmnEk what I expect to get from this feature and how a user can set up/configure their project. We can discuss it more in the upcoming Meeting.
Several remarks here:
- A user may use the same
app-id
along with the different projects (in that case a user can set upapp-id
in the global .npmrc file). Otherwise, it can be set up along with the project. Soapp-id
may not be unique. - To make it more generalized (not only Nexus products can use it) we can think about the new field in the
packaje.json
, for example,user_metadata
which value will be injected in the request header. I don't know is it possible in npm tool and what risks it will take. Another issue from a user side - how this value can be changed for existing project. While runningnpm audit
npm tool can access only to thepackage-lock.json
. And it is not a good idea to set up some data into this file manually. Thus we also have to provide some general command for it - likenpm set metadata
which will reflectpackage.json/package-lock.json
- Also It would be cool to set user metadata via
npm config
command, for example:npm config set metadata_app-id myAppId
npm config set metadata_project-name myProjectName
In that case, I expect to see in the request header following parameters:app-id
with valuemyAppId
project-name
with valuemyProjectName
Thus the suffix name after themetadata_
will go the to HTTP request header name with the corresponding value. But I don't know if it is possible to apply some regex to npm registry to fetch this info or not.
If we can get suggestions on how to make this more generic and benefit the wider npm community (not just those that use NXRM), then I'd be happy to make it a priority and carve out some time for my team work on it.
Guys, who can help to find the best approach of implementing custom user metadata into the npm registry. Like I wrote above - the best way is:
- from a user side write:
npm config set metadata_app-id myAppId
npm config set metadata_project-name myProjectName
- while npm commands like
npm audit
in the HTTP header get the new field:user_metadata
with value:app-id=myAppId, project-name=myProjectName
or with the new fields:user_metadata-app-id
with valuemyAppId
and so on.
So the main issue/question of how to regex user-metadata_(.[\w|\-]+)
from the registry and map these values to HTTP header. Is it possible or we have to find another solution?
I would like to see if the PR (https://github.com/npm/cli/pull/1674) I have put together satisfies the discussion held during the rfc meeting.
In short, the PR allows a generic way to add metadata to the http headers sent as part of the audit command. I think this was asked during the meeting. The metadata
to send is retrieved from a metadata
object in the package.json file.
Is there an agenda to (re)discuss rfc's? I would be happy to attend
I have updated the RFC as discussed at the last meeting. Is this at a point where it can be re-reviewed / merged?
Its possibly a bit late but could this be added to the agenda (9th September)? Or how can I go about taking this discussion further again? /cc @isaacs
Other options to explore, discussed in 2020-09-16 RFC call:
{
"headers": {
"app-id": "call it headers instead of metadata"
}
}
{
"appId": "top-level field"
}
; project-level .npmrc
headers[]="npm-app-id: a config value"
headers[]="Authorization: custom-thing whatever idk"
Leaning towards "add headers list to config", and then we can later add an RFC to put a npm-app-id
header from the registry is saved to ./.npmrc
by default.
@isaacs I started to take a look at the implementation of this RFC to get ahead of the game and I found out that the npmrc already caters for additional header info. It is not as elegant is this proposal but it does suggest that there might be a backwards compatibility problem if this RFC is introduced.
https://github.com/npm/cli/blob/5587ac01ffd0d2ea830a6bbb67bb34a611ffc409/node_modules/npm-registry-fetch/config.js#L23 has an entry for headers.
I tested by creating something like the following in a local .npmrc file:
; headers
headers[]="app_id: test"
note that this is identical to the proposal.
What is sent across the wire is:
"1": "app_id: test"
So, although not ideal, this is something I can actually work with. Additionally, 'fixing' it so that it is a correct key/pair sent across the wire my break anyone already using this capability.
This was tested on the latest version 6. I did not check version 7
Ohhhhh ok. I wonder if we can do something like this then?
[headers]
some-key = some value
I'll look more closely into it soon.
$ cat .npmrc
foo = bar
[headers]
key = value
otherkey = othervalue
bar = baz
npm/arborist isaacs/do-not-collide-on-matching-peer-set-members tau:~/dev/npm/arborist/headers [email protected]
$ npm config get headers
{ otherkey: 'othervalue', bar: 'baz' }
So this is weird? Seems like we drop the first one for some reason.
If you try:
foo = bar
[headers]
key = value
key = value
otherkey = othervalue
bar = baz
does key
show up in the headers?
Ahhhhh, the problem is that I had a trailing space, which the ini
package treats as invalid for some reason, and causes it to skip the line.
Yes, this works as expected, as far as I can tell.
@isaacs @doddi can we close this given that headers
looks to already be supported/configurable? Should we instead just document this so the functionality isn't lost in translation?
@isaacs @doddi can we close this given that
headers
looks to already be supported/configurable? Should we instead just document this so the functionality isn't lost in translation?
I am happy to close if this same functionality is transferred over to v7. That would be ideal because we are already making use of this feature. However, I think @isaacs expressed some concern that this was not intentional behaviour.
I am happy to close if this same functionality is transferred over to v7. That would be ideal because we are already making use of this feature. However, I think isaacs expressed some concern that this was not intentional behaviour.
This [headers]
functionality still works in v6 but was not transferred over to v7 or v8, for anyone wondering.
Is this still being worked on?
- For my own use case, I'd want the additional option to limit headers to certain registries, e.g. for headers used for authentication
- If official support were to be reintroduced, maybe
[//my.registry.com/path:headers]
to override or merge the default headers override if the user's registry isregistry = http://my.registry.com/path
. See other notes in #645 that I'd made before realizing this PR was open
It seems like npm 9 will be dropping support for global configs entirely for _auth
, _authToken
, always-auth
, etc, and only allowing those configs to be scoped to a given registry.
e.g. https://github.com/npm/cli/pull/5696
So maybe there should be no global [headers]
section in an existing or new RFC to do this, and only a section such as this:
[//my.registry.com/my/prefix/path:headers]
my-xyz = xyz
[//registry.npmjs.org/:headers]
something-else = xyz
The last rfc meeting was in August - was that because there had been nothing to discuss, or another reason? https://github.com/npm/rfcs/issues?q=is%3Aissue+sort%3Aupdated-desc+is%3Aclosed+label%3Ameeting (I'm unfamiliar with the rfc process and didn't see anything about it in the last meeting notes)