npm finding for git package on yarn.lock - false positive?

[nirhaas]

Hi. I ran osv-scanner on the firebase-js-sdk repository and encountered a potential false positive match.

The project's yarn.lock includes a direct git dependency pointing to Google's official repository:

"closure-net@git+[https://github.com/google/closure-net.git#6f48f578d3e80fe7a85e530a5d95b9351433d135](https://github.com/google/closure-net.git#6f48f578d3e80fe7a85e530a5d95b9351433d135)":
  version "0.0.0"
  resolved "git+[https://github.com/google/closure-net.git#6f48f578d3e80fe7a85e530a5d95b9351433d135](https://github.com/google/closure-net.git#6f48f578d3e80fe7a85e530a5d95b9351433d135)"

However, osv-scanner flags this against MAL-2026-276, which is a malicious package advisory intended for the closure-net npm registry package.

Since yarn is resolving this dependency directly from a specific commit on a public github repository (google/closure-net) rather than fetching a package from the npm registry, it seems like osv-scanner might be incorrectly mapping the package name collision.

Could you please clarify if osv-scanner is intended to match git dependencies this way, or if this is an issue with how the ecosystem lookup is handled for direct git URLs?

If this is not a bug - does that mean that firebase-js-sdk users are considered compromised?

Thanks

[G-Rath]

I can't find the issues right now, but I believe the general reasons are:

• the npm ecosystem represents packages in the NPM repository
• the npm ecosystem does not get commits enumerated, in part because of the first point

It's been a while since I've looked at the code, but I'm also pretty sure we're not even going to be submitting the commit because there's a version, and if you hit the API with the commit you'll see no results:

❯ curl -d '{"commit": "6f48f578d3e80fe7a85e530a5d95b9351433d135"}' "https://api.osv.dev/v1/query"
{}

Generally for these situations you can use overrides to tell the scanner that particular package should not be treated as being from the npm ecosystem (i.e. by having it ignored).

[nirhaas]

@G-Rath thanks for responding. In this particular case, is the github repository the source for the npm package? can we prove that?

Who should put the override? me as a user of osv-scanner, or the maintainers of firebase-js-sdk somewhere? because it may pop up for other users of osv-scanner as well.

[G-Rath]

In this particular case, is the github repository the source for the npm package? can we prove that?

I would assume that the git repository is not related, it was someone attempting to leverage the name overlap but I couldn't say how you can prove it - you could try making an issue on the github repo asking for a maintainer to confirm?

Who should put the override? me as a user of osv-scanner, or the maintainers of firebase-js-sdk somewhere? because it may pop up for other users of osv-scanner as well.

I think that lives somewhere "around" the firebase-js-sdk and depends on the context: if you're working on that package then the most you can really do is suggest it to the maintainers that they include a config with the override but if they're not keen you could add the config locally and ignore it using an external / global .gitignore.

If you're doing a form of vendored dependency scanning (i.e. explicitly scanning the lockfiles of dependencies, in addition to your own) then you should do the override in a root level config in your codebase

[nirhaas]

Isn't firebase-js-sdk just an instance of this type of issue?

It seems like osv-scanner (specifically osv-scalibr) is detecting direct git dependencies as npm ecosystem dependencies. This can cause widespread false positives, considering that the git repository and the npm package name are not necessarily related.

[G-Rath]

I think "wide spread" is a bit of an exaggeration: the vast majority of package.json dependences are related to the npm registry which is why we assume all git dependencies are the sources or forks of packages with the same name published on the npm registry.

The opposite would be more strictly correct but then you'd have the issue of false negatives which we consider a far worse issue

(We also don't have anyway to tell the scanner to include a package, only to exclude packages).

[nirhaas]

I agree "wide spread" was not the right choice of word.. This is pretty rare I suppose.

How would you suggest the users to use osv-scanner considering the issue though? for every npm finding, manually check if the source was git ecosystem? would labeling the finding as potentially fp / add a configuration help here?

[G-Rath]

I would expect it to come up as part of the triage process you do when any tool reports a vulnerability - the only time you might skip that is if there is a fixed version that you can successfully apply 🤷

I can't think of any labeling we could add that wouldn't risk weakening our reporting e.g. if we had a "might be false positive" label there's a risk people will just start ignoring those or want a flag to have us not report them entirely, giving a nice vector for bad stuff to slip through.

The fact that the version is 0.0.0 does already give a subtle indication that the package is special

[nirhaas]

Honestly, it would have been real nice to have such a flag to reduce the false positive on this case. Considering the lock file references a specific commit hash on github, unless that commit hash is affected (and in this case shouldnt the git ecosystem be used?), this is a false positive. I can't imagine the vector attack here considering the package manager (yarn in this case) goes to github in the first place and not the malicious npm package.

That being said, for the time being 0.0.0 will be a good indication for this. I would love to keep discussing it here for the ideal solution (imo) but if you think that it's ok as is - I will not push further. Thanks for being responsive.

[another-rex]

Hmm I do see that if it's pinned to a git commit it's highly unlikely that it's malicious. If we make it default not report anything for pinned git repos, it's not really adding a new an attack vector for people adding malicious packages, as they can just name the package anything if they pin it to a repository anyway.

If it's not part of the npm registry, we should not query osv.dev with the npm ecosystem for it, instead just do a commit query

[johnsaurabh]

I've looked at the code, looks like the yarnlock extractor sets ecosystem=npm for git-pinned deps too and never populates SourceCode. Commit, so the commit query path never triggers. Might be worth post-processing to detect git URLs and extract the commit before querying. Can I be assigned to work on this?

[G-Rath]

@johnsaurabh go for it!

[G-Rath]

noting though I think this change should be in the extractor, not in scanner

[johnsaurabh]

@G-Rath Yes, I traced it to osv-scalibr: extractor/filesystem/language/javascript/yarnlock/yarnlock.go:

return &extractor.Package{
    Name:     name,
    Version:  version,
    PURLType: purl.TypeNPM,
    SourceCode: &extractor.SourceCodeIdentifier{
        Commit: commitextractor.TryExtractCommit(resolution),
    },
}

PURLType is always set to npm regardless of the resolved URL, so a git-pinned dep like git+https://github.com/google/closure-net.git#6f48f578 gets queried against the npm registry, hitting the unrelated package with the same name.

Fix would be to only set purl.TypeNPM when resolution actually points to the npm registry. But the osv-scalibr is in a different repo, we'll have to fix it there. We could also patch it in osv-scanner by doing a commit query when ecosystem is npm and a commit is already set. Fixing it in the extractor seems like the right call though. What do you think?