feat(config): parse relative date in --before

[kaezone]

This PR implements npm/rfcs issue #844, adding support for relative dates in the --before config arg (e.g. --before=24h, --before=7d, etc).

[wraithgar]

The current hint for this field is [--before <date>], this is customized via the hint attribute on any given definition. Is <date> still sufficient here?

[kaezone]

Couldn't really think of a better hint attribute that was more descriptive without being too wordy - <date/relative> maybe? I think date is still fairly applicable as long as the rest of the doc string notes using relative dates.

[wraithgar]

I think date is still fairly applicable as long as the rest of the doc string notes using relative dates.

That seems fine to me, just wanted to bring it up in case I was missing something obvious.

[kaezone]

Just curious, would this be able to fit in the 11.7.0 release (if accepted of course) or is it a bit late?

[wraithgar]

The reason this hasn't been merged yet is because of hesitation with the "loosely defined human parsable date range" part of the parameter. Historically things like this feel helpful but are not. As long as folks have a consistent way to set a minimum age I think being more rigorous in what this value can be is in order.

Defining this as an integer value of hours may be sufficient. I think it's worth waiting on and considering input from others with UX knowledge.

[wraithgar]

Prior art here is:

• pnpm, which defines this as an integer of minutes
• dependabot, which explicitly names the parameter as "days" and accepts integer values

[kaezone]

Defining this as an integer value of hours may be sufficient.

Yeah I agree that'd probably be more consistent (and easier to explain in docs) - would matching pnpm with an integer of minutes be ideal?

Will wait for input before doing any more tweaking on this.

[wraithgar]

Will wait for input before doing any more tweaking on this.

Best option, thanks!

[karlhorky]

As other users mentioned in the alternative PR #8825, this approach of extending --before misses the feature of excluding packages, patterns and specific versions of packages, which is possible with pnpm's minimumReleaseAgeExclude:

• feat: add minimum package age policy to prevent supply chain attacks #8825 (comment)

Maybe it makes sense to reconsider this alternative PR #8825, as it follows the standard patterns introduced by pnpm minimumReleaseAge and Bun minimumReleaseAge (although original minimumReleaseAge naming credit goes to Renovate).

[kaezone]

FWIW I'm not particularly attached to my PR, we're just looking for any variation of this feature at my workplace.

[wraithgar]

Thanks @kaezone it is still a good PR and we're glad you made it. This is a topic that's obviously top of mind for a lot of folks and this PR has become a great focal point for the discussion.

[AsdarhTheBest]

As other users mentioned in the alternative PR #8825, this approach of extending --before misses the feature of excluding packages, patterns and specific versions of packages, which is possible with pnpm's minimumReleaseAgeExclude:

• feat: add minimum package age policy to prevent supply chain attacks #8825 (comment)

Maybe it makes sense to reconsider this alternative PR #8825, as it follows the standard patterns introduced by pnpm and Bun (although original minimumReleaseAge naming credit goes to Renovate).

Yuuuup, I made a Github account only for joining this discussion cuz was interesting, I was searching for if npm has a minimum age release rule or something and found this.

I comply with @karlhorky . @wraithgar you should reconsider re-opening #8825 PR. Implements a minimum release age policy very similar as pnpm's and Bun's.

Not pretending to ofend, this PR is really good, but I don't think that is what the community wants to see on NPM. After all, the final usser/developer is the one who wants to code without caring about vulnerabilities in dependencies/sub-dependencies and set a minimum required age for the releases of the packages and with that, they can be "safer" in some way.

As many people say in #8825, the feature implemented at this PR, has no form of excluding packages from this policy.

In my point of view as a developer, I would prefer having the possibility of excluding some packages.
Example:

• Express: Minimum release age
• React: Maybe, I dont want to include this policy, so exclude the package.

I believe both implementations should be implemented, but for a more complete implementation, in my opinion and that of several users and even contributors, the best option currently would be to implement PR #8825. However, I also don't want this PR to be closed, as it can also be improved beforehand, allowing both implementations to work harmoniously together.

[kaezone]

I'm generally in agreement here as well - the other PR looks quite a bit more robust.

This PR was intended as the minimum necessary to extend the current functionality to help reduce a risk I'm working on (and presumably one many other dev shops face), but the other one provides quite a bit more in terms of QOL.

For what it's worth, I primarily work in the .NET appsec and GRC spaces, so I'd tend to lean towards the opinion of those who spend more time in the JS/TS ecosystem :)

[wraithgar]

Naming of the parameter aside, the approach in #8825 is not correct. This is a parameter that is parsed in npm-pick-manifest. The cli itself just gets the intent from the user and passes it down. npm-pick-manifest is where the "get me a package based on this spec" logic lives, and this before param then limits the time frame of available versions it can "see". Arborist, pacote, and the cli don't care about any of that.

This is one of the reasons extending before makes sense. It already does what we want.

I realize we are getting bogged down a bit in the naming of the parameter, and not the implementation. I don't think anyone involved doesn't want this, it's just a matter of having it work where it is supposed to. There is also the matter of the exclusion option, which is a bit out of scope for this initial extension of the feature.

[tounsils]

New to the repo here — really appreciate the thoughtful discussion on this PR. Extending --before at the npm-pick-manifest level makes sense architecturally, and it’s helpful to see the tradeoffs discussed so clearly. I agree that starting with a well-defined, consistent minimum-age mechanism and iterating later (e.g. exclusions) feels like a pragmatic path forward. Thanks to everyone involved for the transparency and context.

[kaezone]

Heyo, just want to ask if there's anything I could do to move this forward - all good if it's still just a point for discussion though :)

[sean-kintone]

Seems like this isn't going to get implemented, according to this comment. Which is a shame, as enterprise orgs using npm would certainly benefit from the option. Any option which gives the end user an increased chance of avoiding such attacks is beneficial, and thus beneficial to "the ecosystem" overall in my opinion.

edit: to clarify, not "due" to the comment linked, but rather due to the reasoning that the linked comment contains.

[ljharb]

@sean-kintone i don't work for npm and am not a maintainer of the cli.

[paulbrimicombe]

@wraithgar is there anything we can do to move this forwards please?

Defining this as an integer value of hours may be sufficient. I think it's worth waiting on and considering input from others with UX knowledge.

I agree that an integer value of minutes / hours / days is probably easier to understand / document etc.

--before already supports some integer values but interprets them differently (e.g. try npm_config_before=-2 npm config get before or npm_config_before=2 npm config get before) but only for inputs between "-12" and "12" inclusive (try npm_config_before=13 npm config get before). I think the options are probably something like:

• Use --before and treat any integer input (positive or negative) as an offset relative to the current date
  • I suspect the most intuitive units would be milliseconds since that is the standard timestamp in JavaScript even though that's slightly unreadable / unwieldy
  • It would have to be accepted that this (in the strictest definition) would be a breaking change.
• Add a new CLI flag such as --before-days / --before-hours etc. that accepts an integer value.
  • Should this accept a negative integer since it is a negative time offset compared to the current date?
• Support a specific format for --before that isn't a valid date (e.g. -7d) but be more strict about its formatting than this PR (i.e. only support hours / only support days etc).

I'm very happy to work on PRs etc. if that's helpful.

[wraithgar]

Thank you all for your patience on this PR. Other priorities came up that took the focus of cli work and this was one of the things that we didn't have the time to give the attention it was due.

My (biased) opinion is that if we allowed an integer value, either in days or minutes (based on prior art from pnpm or dependabot), then this is good to go.

The exclusion option is also still out of scope, and would need to be a separate PR or discussion.

I believe the problem to solve then would be a consistent way to distinguish between date strings and integers, since the cli only sees strings when parsing argv.

[kaezone]

Thanks for the update @wraithgar, I appreciate you've probably been pretty busy.

I have it running locally with both a fixed minutes value (e.g --before=1440 parsed as 1 day prior) and the fixed timestamp, but it does result in a change in behavior.

There's ambiguity for strings like 2020, where it could either be parsed as 2020 minutes, or the year 2020 - I'm dealing with this by taking the integer first, so trying to do "before the year 2020" would require specifying --before=2020-01-01 or similar, rather than just --before=2020 as you can do now. Effectively anything that's just an integer is parsed as minutes, but if there's a hypen/slash/etc etc like 2020-01 or 2020/01 it's parsed as a date.

Let me know your thoughts, if it's acceptable I can either push here or base off a new branch+PR, or if not I'm happy to look at another option e.g. a separate --before-minutes.

[valentijnscholten]

At first sight it doens't look really inituive to have a parameter named --before and put a number of seconds in there. If we can't have a new parameter like --min-release-age <seconds> it might be worth to have a minus sign in front of the second to make it more clear that it is relative to today, i.e. --before -1440 or --before -1440s or --before "-1440s". Or does this look crazy?

[wraithgar]

At first sight it doens't look really inituive to have a parameter named --before and put a number of seconds in there.

Very valid point.

it might be worth to have a minus sign in front of the second to make it more clear that it is relative to today

If the problem is 'being clear' then a wholly separate parameter is in order.

[wraithgar]

If we want a new flag then I would humbly propose that the npm config library do all of the work to translate it into a before. Second-level precision is likely not needed for this, only day-level.

The new flag could be an integer day value, and we can follow the pattern of the various include/omit flags and package-lock-only on how to consistently have two flags that overlap.

We would also want to make them exclusive with each other.

[wraithgar]

The advantage of flattening this down to --before is that it does not require any new config in any of the libraries that look for that value. It would be able to be a single change in the cli itself, translating user intent into pre-existing flags.

[wraithgar]

A good argument for NOT overloading --before is how npm parses it today:

$ npm config list --before=2024

; "cli" config from command line options

before = "2024-01-01T00:00:00.000Z"

[wraithgar]

#8965

[kaezone]

Superseded by #8965.