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

fix(yarnlock): trim the trialing : in the header #406

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

cuixq
Copy link
Contributor

@cuixq cuixq commented Jan 23, 2025

Without trimming the trialing : in the header, if the header is just the name, the name will be parsed wrongly with : in it.

@cuixq cuixq requested a review from erikvarga January 23, 2025 01:00
@cuixq cuixq marked this pull request as ready for review January 23, 2025 01:00
@cuixq
Copy link
Contributor Author

cuixq commented Jan 23, 2025

@G-Rath not sure if you have yarn knowledge - do you know if this applies to yarn.lock v2?

@G-Rath
Copy link
Collaborator

G-Rath commented Jan 23, 2025

@cuixq could you provide more details on how you produced this lockfile? afaik this is invalid

@cuixq
Copy link
Contributor Author

cuixq commented Jan 23, 2025

so only having the package name in the header is not allowed?

we have a yarn.lock file in this format:

package-name:
  version "x.x.x"
  resolved "xxxxx"
  integrity xxxxxxx

and osv-scalibr extracts the package name as package-name: which causes error when fetching vulnerabilities from OSV.dev.

@G-Rath
Copy link
Collaborator

G-Rath commented Jan 23, 2025

so only having the package name in the header is not allowed?

afaik there's no official specification for any version of yarn.lock, so I can't say for sure it's invalid - but among other things at Ackama we've used Yarn for a while due to it being the js package manager for Rails and I've never seen that format.

So ideally it'd be good if you could come up with a reproduction for that lockfile - happy to take this "offline" if that'd be easier (I assume you've not shared more info as its a lockfile in a private codebase).

@oliverchang
Copy link
Collaborator

so only having the package name in the header is not allowed?

afaik there's no official specification for any version of yarn.lock, so I can't say for sure it's invalid - but among other things at Ackama we've used Yarn for a while due to it being the js package manager for Rails and I've never seen that format.

So ideally it'd be good if you could come up with a reproduction for that lockfile - happy to take this "offline" if that'd be easier (I assume you've not shared more info as its a lockfile in a private codebase).

Indeed, this is from a real case in a private codebase.

@cuixq
Copy link
Contributor Author

cuixq commented Jan 23, 2025

Considering this change does not cause any trouble with the existing fixtures, it may be fine to merge it anyway.

@G-Rath
Copy link
Collaborator

G-Rath commented Jan 23, 2025

(to be clear, I'm not super against landing this, I'd just really like to try and figure out how it's generated in case there are other things we need to change 🙂)

@cuixq
Copy link
Contributor Author

cuixq commented Jan 23, 2025

Understandable. Considering my limited knowledge about yarn, I would like some insights on whether this is very wrong.

@copybara-service copybara-service bot merged commit afdaa52 into google:main Jan 23, 2025
8 checks passed
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 this pull request may close these issues.

3 participants