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

Migrate vul chain finder to data processing platform #11

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

ashkboos
Copy link
Contributor

@ashkboos ashkboos commented Mar 4, 2022

In this PR we migrate the proof of concept implementation of VulChainFinder in the old repository to the data-processing platform.


public static String createStrForSelectVulCallablesWhereModuleIdIs(final Long moduleId) {
return "SELECT packages.package_name, package_versions.version, callables.fasten_uri, " +
"callables.metadata -> 'vulnerabilities' " +
Copy link
Contributor

@mir-am mir-am Mar 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, you can also use JOOQ's methods to get vulnerable callables given a module id instead of using a raw SQL query.

@mir-am
Copy link
Contributor

mir-am commented May 11, 2022

I have talked to Sebastian (@proksch) and we have decided that I can work on this feature branch, address the above comments, and deploy it on monster for the vulnerability paper(s).
That said, we are not going to merge it until the final code review and your approval.
I would like to ask you, Mehdi (@ashkboos ), whether you are also okay with this, i.e, me working on this feature branch.

@ashkboos
Copy link
Contributor Author

I have talked to Sebastian (@proksch) and we have decided that I can work on this feature branch, address the above comments, and deploy it on monster for the vulnerability paper(s). That said, we are not going to merge it until the final code review and your approval. I would like to ask you, Mehdi (@ashkboos ), whether you are also okay with this, i.e, me working on this feature branch.

Yes sure, no need to even ask Amir. And sorry for not addressing the comments so far, I was waiting for the code review session that we planned to do on this PR to look at it together in person but unfortunately, it got postponed multiple times.

@proksch proksch force-pushed the mehdi/migrate-vul-chain-finder branch from 3e5d0f4 to 936e533 Compare May 19, 2022 21:30
@proksch
Copy link
Contributor

proksch commented May 19, 2022

I have updated (and rebased) the branch to include the latest changes that were necessary to adopt changes from core. The build should work again and use all up-to-date classes/plugins.

While updating, I also checked the implementation a bit... maybe we should make this plugin subject of a pair programming Monday to clarify some things about the loader platform and to talk about recurring issues that I found in several places in the code.

I am also not particularly sure about the introduction of the new Error class for exception handling... I would like to discuss this with you in person before we release this plugin.

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.

None yet

3 participants