search

advanced-security / maven-dependency-submission-action

GitHub Action for submitting Maven dependencies
MIT License
46 stars 27 forks source link

Incorrect logic for determining dependency scope #72

Open andreas-borglin opened 4 months ago

andreas-borglin commented 4 months ago

Hi there.

We have just started using your action (thanks for the work!) and have found a problem when used in some of our multi-module maven projects.

The logic here, https://github.com/advanced-security/maven-dependency-submission-action/blob/main/src/depgraph.ts#L226,

function getDependencyScopeForMavenScope(mavenScopes: string[] | undefined | null): DependencyScope {
  // Once the API scopes are improved and expanded we should be able to perform better mapping here from Maven to cater for
  // provided, runtime, compile, test, system, etc... in the future.
  if (mavenScopes) {
    if (mavenScopes.includes('test')) { <== THIS HERE SEEMS WRONG
      return 'development';
    }
  }

  // The default scope for now as we only have runtime and development currently
  return 'runtime';
}

does not seem quite right. In our case, we have some dependencies that have test scope for some modules, but runtime for others, but they are now being reported incorrectly as purely development scoped dependencies to Dependabot, despite actually having runtime scope for some modules.

I would assume that the check there should be if (mavenScopes.includes('test') && mavenScopes.length === 1) { or maybe reverse logic that if it does not contain some of the other scopes, it's inferred to be development?

Thanks in advance, Andreas

peter-murray commented 4 months ago

That logic is correct as it stands, as per my intent at the time.

Dependabot does not have the same support for the scopes and granularity that Maven has and as such and so I have to either decide to put things in as runtime or development when submitting to the API.

Whilst in Maven runtime dependencies are provided at runtime and you technically do not have control over that version, these are expected to match your definitions and are being fetched to build against. These have implications that if for instance a new API or contract was defined in the implementation, these are from dependency perspective what is needed once deployed otherwise you will get a runtime error.