feat: guided remediation dependency resolution #432
Conversation
michaelkedar
force-pushed
the
guided-remediation/resolve
branch
from
d50b67f to
47998ea
Compare
michaelkedar
changed the title
| // OSVRecord is a representation of an OSV record. | ||
| // TODO: replace with https://github.com/ossf/osv-schema/pull/333 | ||
| type OSVRecord struct { |
There was a problem hiding this comment.
I should be able to do this soon
There was a problem hiding this comment.
internally we have a policy that all TODOs need to have a bug attached - can you create a github issue for moving Guided Remediation and refer to it here?
| ) | ||
|
|
||
| // Temporarily internal while migration is in progress. | ||
| // Will need to be moved to publicly accessible location once external interface is created. |
There was a problem hiding this comment.
Maybe add a TODO here and link to a github issue about moving guided remediation
| // OSVRecord is a representation of an OSV record. | ||
| // TODO: replace with https://github.com/ossf/osv-schema/pull/333 | ||
| type OSVRecord struct { |
There was a problem hiding this comment.
internally we have a policy that all TODOs need to have a bug attached - can you create a github issue for moving Guided Remediation and refer to it here?
| } `yaml:"affected,omitempty"` | ||
| } | ||
|
|
||
| type OSVEvent struct { |
|
|
||
| type mockVulnerabilityMatcher []*matcher.OSVRecord | ||
|
|
||
| func (mvc mockVulnerabilityMatcher) MatchVulnerabilities(ctx context.Context, invs []*extractor.Inventory) ([][]*matcher.OSVRecord, error) { |
| return mockVulnerabilityMatcher(vulns.Vulns) | ||
| } | ||
|
|
||
| // TODO: similar logic will need to be used elsewhere in guided remediation. |
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // Package resolution provides dependency graph resolution and vulnerability findings |
There was a problem hiding this comment.
Our internal linters only expect this comment for just one of the package's files (so it'll complain if it's there for multiple files)
There was a problem hiding this comment.
LGTM - probably need to patch #442 (sorry for the trouble!)
Agree with Erik's suggestions on code styles. maybe we should find a way to implement the linter equivalent to the internal one, so we don't rely on manual review on these.
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // Package maven provides the manifest parsing and writing for the npm package.json format. |
| mavenResolve "deps.dev/util/resolve/maven" | ||
| npmResolve "deps.dev/util/resolve/npm" |
There was a problem hiding this comment.
I think the convention is mavenresolve and npmresolve - though not sure whether the internal linter will alarm this or not...
Beginning the move of guided remediation from osv-scanner. Started with the dependency resolution & vulnerability detection.
I've also removed the
DependencyClientinterface, since we only really need the deps.devresolve.Client(this will conflict with the change in #441, I'll fix it here after that's merged).Trying to keep this stuff mostly self-contained in
internal/guidedremediationas I work on it, but it I will have to move some things around eventually.