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

Dependencies: Upgrade file-system-cache #23253

Closed
wants to merge 7 commits into from
Closed

Dependencies: Upgrade file-system-cache #23253

wants to merge 7 commits into from

Conversation

seriouz
Copy link
Contributor

@seriouz seriouz commented Jun 29, 2023

Reverts #23221 as the issue has now been fixed upstream.

What I did

Removed the pinned dependency file-system-cache with 2.3.0 and set it to the fixed version 2.4.1.
Links:

This is the result of yarn why file-system-cache in code:

├─ @storybook/core-common@workspace:lib/core-common
│  └─ file-system-cache@npm:2.4.1 (via npm:^2.4.1)
│
└─ @storybook/types@workspace:lib/types
   └─ file-system-cache@npm:2.4.1 (via npm:^2.4.1)

How to test

Must be tested in a case-sensitive operating system like Linux - not MacOS or Windows.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@JReinhold JReinhold changed the title Dependencies: Set file-system-cache to 2.4.1 Dependencies: Upgrade file-system-cache Jul 3, 2023
@JReinhold JReinhold added dependencies patch:yes Bugfix & documentation PR that need to be picked to main branch labels Jul 3, 2023
@JReinhold
Copy link
Contributor

Thanks for the PR!

It appears that v2.4.1 introduces some type errors which causes CI to fail. I've filed a bug report upstream: philcockfield/file-system-cache#32

We could do skipLibCheck: true, but I'd rather not, as that makes our types less strict.

@seriouz
Copy link
Contributor Author

seriouz commented Jul 3, 2023

@JReinhold Thanks for bringing this up in philcockfield/file-system-cache#32.
Lets wait and see what @philcockfield has to say about this.

@JReinhold
Copy link
Contributor

@seriouz could you try with v2.4.2 here and see if that fixes the issue?

philcockfield/file-system-cache#32 (comment)

@seriouz
Copy link
Contributor Author

seriouz commented Jul 9, 2023

ramda has ts-toolbelt as dependency.

@seriouz
Copy link
Contributor Author

seriouz commented Jul 9, 2023

@JReinhold
Copy link
Contributor

Now the pipeline fails saying fs-extra is missing. But its in the dependencies #23253 (files)

I see, looks to be the same problem as with @types/rambda. Installing it as a dependency won't help because it will land in node_modules/@types/fs-extra while TypeScript is looking in node_modules/file-system-cache/node_modules/@types/fs-extra (I believe) - so file-system-cache needs to install it.

philcockfield/file-system-cache#32 (comment)

@kasperpeulen kasperpeulen removed the patch:yes Bugfix & documentation PR that need to be picked to main branch label Aug 9, 2023
@JReinhold
Copy link
Contributor

@seriouz seems like this has now been fixed in 2.4.3 of file-system-cache. Could you upgrade to that here, I think that should make this mergable.

@yannbf
Copy link
Member

yannbf commented Sep 19, 2023

Hey @seriouz I could not push updates to your branch, could you please update your branch and fix the merge conflicts? Thanks!

@ndelangen ndelangen self-assigned this Sep 19, 2023
@ndelangen
Copy link
Member

@yannbf It's hard to update branches when they are named next because git will say the ref is ambiguous.

It's possible though, if you rename the branch name locally, and point it to track the branch of the PR.

@socket-security
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@types/picomatch 2.3.0 None +0 18.4 kB types
file-system-cache 2.4.4 None +9 1.62 MB philcockfield

🚮 Removed packages: @vitejs/[email protected], [email protected], [email protected]

@socket-security
Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue Package Version Note Source
New author @babel/preset-typescript 7.10.4

Next steps

What is new author?

A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package.

Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

@seriouz
Copy link
Contributor Author

seriouz commented Sep 19, 2023

I created a new PR with the latest "next" branch: #24232
I think this PR can be closed.

@ndelangen ndelangen closed this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants