My original concern still remains. I think what you have is valuable but would prefer that it be activated in an independent build that notifies the interested parties.
On Wed, Nov 1, 2017 at 11:13 AM, Vlad Rozov <vro...@apache.org> wrote: > Any other concerns regarding merging the PR? By looking at the active PRs > on the apex core the entire conversation looks to be at the moot point. > > Thank you, > > Vlad > > > On 10/30/17 18:50, Vlad Rozov wrote: > >> On 10/30/17 17:30, Pramod Immaneni wrote: >> >>> On Sat, Oct 28, 2017 at 7:47 AM, Vlad Rozov <vro...@apache.org> wrote: >>> >>> Don't we use unit test to make sure that PR does not break an existing >>>> functionality? For that we use CI environment that we do not control >>>> and do >>>> not introduce any changes to, but for example Apache infrastructure team >>>> may decide to upgrade Jenkins and that may impact Apex builds. The same >>>> applies to CVE. It is there to prevent dependencies with severe >>>> vulnerabilities. >>>> >>>> Infrastructure changes are quite different, IMO, from this proposal. >>> While >>> they are not in our control, in majority of the cases, the changes >>> maintain >>> compatibility so everything on top will continue to run the same. In this >>> case a CVE will always fail all PRs, the code changes have nothing to do >>> with introducing the CVE. I did make the exception that if a PR is >>> bringing >>> in the CVE yes do fail it. >>> >> There were just two recent changes, one on Travis CI side and another on >> Jenkins side that caused builds for all PRs to fail and none of them was >> caused by code changes in any of open PRs, so I don't see how it is >> different. >> >> A code change may or may not have relation to CVE introduced. For newly >> introduced dependencies, there may be known CVEs. In any case I don't think >> it is important to differentiate how CVE is introduced, it is important to >> eliminate dependencies with known CVEs. >> >>> >>> >>> There is no "stick" in a failed build or keeping PR open until dependency >>>> issue is resolved or unit test failure is fixed. Unless an employer >>>> punishes its employee for not delivering PR based on that vendor >>>> priority, >>>> there is no "stick". As we already discussed, the community does not >>>> have a >>>> deadline for a PR merge or for a release to go out. In a case there is a >>>> problematic dependency (with CVE or category X license) you as a PMC >>>> suppose to -1 a release (at least I will). Will you consider -1 as a >>>> "stick"? For me, it is not about punishing an individual contributor, >>>> it is >>>> a priority and focus shift for the entire community, not a "stick" for >>>> an >>>> individual contributor. >>>> >>>> The stick I am referring to is failing all PRs hoping that will make >>> people >>> address CVEs. It's got nothing to do with an employer, people >>> contributing >>> to open source can't expect they can control what the outcome will be or >>> what form it will take. You may be confusing this with some other issue. >>> In >>> some of the arguments, you are assuming this scenario is similar to build >>> failures from failing unit tests and I am arguing that premise. I don't >>> think we should bring regular development to a halt whenever a matching >>> CVE >>> is discovered, unless there is some other secondary reason like merging a >>> PR will make it difficult for a CVE fix to be made. Have you given a >>> thought to what I said about having a separate build that will notify >>> about >>> CVEs. >>> >> As I mentioned, there is no stick, no deadlines and no issues keeping PRs >> open until builds can be verified on CI environment. Fixing a failed build >> is a priority for the *community* not a stick for an individual contributor. >> >> I don't see why keeping PRs open (for whatever reason) brings regular >> development to a halt. Nobody is going to put github repo offline. >> Contributors may continue to open new PR, collaborate on existing PRs and >> add more changes (and need to be patient for those changes to be reviewed >> and accepted). The regular development will continue with the only >> exception that the next commit to be merged must address the build issue >> (whether it is a failed unit test or newly found CVE). >> >> I don't see much value in a separate build and do not plan to put effort >> in that direction. Additionally, will not a separate build that only checks >> for CVE will trigger your initial concern of disclosing CVE in public? >> >>> >>> Thank you, >>> >>>> Vlad >>>> >>>> >>>> On 10/27/17 14:28, Pramod Immaneni wrote: >>>> >>>> On Fri, Oct 27, 2017 at 11:51 AM, Vlad Rozov <vro...@apache.org> wrote: >>>>> >>>>> On 10/26/17 11:46, Pramod Immaneni wrote: >>>>> >>>>>> On Thu, Oct 26, 2017 at 10:39 AM, Vlad Rozov <vro...@apache.org> >>>>>> wrote: >>>>>> >>>>>>> I guess you are mostly concerned regarding new CVE in an existing >>>>>>> >>>>>>> dependency. Once such CVE is added to a database, will it be better >>>>>>>> to >>>>>>>> know >>>>>>>> about it or postpone discovery till we cut release candidate? In >>>>>>>> case >>>>>>>> it >>>>>>>> is >>>>>>>> reported only during release cycle, there is much less time for the >>>>>>>> community to take an action and it still needs to be taken (as a PMC >>>>>>>> member >>>>>>>> you are responsible for preventing release with severe security >>>>>>>> issue >>>>>>>> going >>>>>>>> out). If it is reported once the information becomes available, >>>>>>>> there >>>>>>>> is >>>>>>>> more time to research and either upgrade the dependency with newly >>>>>>>> found >>>>>>>> CVE, agree that it does not impact the project. >>>>>>>> >>>>>>>> This would be the more commonly occurring scenario. We can always >>>>>>>> know >>>>>>>> >>>>>>>> about the CVEs because of your changes. We don't need to fail >>>>>>> builds to >>>>>>> do >>>>>>> that. I am not asking you to remove the reporting. There is no set >>>>>>> time >>>>>>> for >>>>>>> a release so having less time during release for addressing relevant >>>>>>> CVEs >>>>>>> does not come up. There is also nothing preventing anyone from >>>>>>> looking >>>>>>> at >>>>>>> these reports and taking action earlier. >>>>>>> >>>>>>> I don't see why it will be more commonly occurring scenario, but I >>>>>>> think >>>>>>> >>>>>> it is equally important to prevent new dependency with severe >>>>>> vulnerabilities being introduced into the project and check existing >>>>>> dependencies for newly discovered severe vulnerabilities. >>>>>> >>>>>> How will we know about CVE if it is removed from CI build? Why require >>>>>> manual verification when it can be done during CI build and does not >>>>>> affect >>>>>> builds done by individual contributors? >>>>>> >>>>>> While there is no set time for a release, there is no set time for a >>>>>> PR >>>>>> merge as well. >>>>>> >>>>>> Yes, nothing prevents anyone from looking at the dependency >>>>>> vulnerabilities, but there is a better chance that "anyone" does not >>>>>> mean >>>>>> nobody if CI build fails. >>>>>> >>>>>> >>>>>> I still do not understand why you value an individual contributor and >>>>>>> PR >>>>>>> >>>>>>> over the community and the project itself. Once there is a severe >>>>>>>> security >>>>>>>> vulnerability, it affects everyone who cares about the project, >>>>>>>> including >>>>>>>> all contributors. I don't see a problem with a PR being in a pending >>>>>>>> (not >>>>>>>> merged) open state till a build issue is resolved. >>>>>>>> >>>>>>>> That is a mischaracterization that you have stated before as well. A >>>>>>>> >>>>>>>> project cannot grow without contributions and without policies that >>>>>>> create >>>>>>> a supportive enviroment where that can happen, I don't see the need >>>>>>> to >>>>>>> put >>>>>>> unnecessary obstacles for legitimate contributions that are not the >>>>>>> cause >>>>>>> of a problem. Everytime there is a matching CVE the PRs are going to >>>>>>> get >>>>>>> blocked till that CVE is addressed and I am not confident we have the >>>>>>> bandwidth in the community to address this expediently. It is also >>>>>>> inaccurate to equate this to PR not being merged till build issues >>>>>>> are >>>>>>> resolved as it derives from an assumption that CVE is same as a build >>>>>>> failure. >>>>>>> >>>>>>> While project can't grow without individual contributions, project >>>>>>> is a >>>>>>> >>>>>> result of a large number of contributions from a number of >>>>>> contributors. >>>>>> Some of those contributors are not active anymore and will not provide >>>>>> any >>>>>> fixes should a vulnerability be found in their contribution. It >>>>>> becomes a >>>>>> shared responsibility of all currently active community members and >>>>>> those >>>>>> who submit PR are part of the community and share that responsibility, >>>>>> are >>>>>> not they? If a contributor considers him/herself as part of a >>>>>> community, >>>>>> why he or she can't wait for the build issue to be resolved or better >>>>>> take >>>>>> an action on resolving the issue? The only possible explanation that I >>>>>> see >>>>>> is the one that I already mentioned on this thread. >>>>>> >>>>>> If you see this as unnecessary obstacles for legitimate contributions, >>>>>> why >>>>>> to enforce code style, it is also unnecessary obstacle. Unit test? >>>>>> Should >>>>>> it be considered to be optional for a PR to pass unit tests as well? >>>>>> What >>>>>> if an environment change on CI side causes build to fail similar to >>>>>> what >>>>>> happened recently? Should we disable CI builds too and rely on a >>>>>> committer >>>>>> or a release manager to run unit tests? If CI build fails for >>>>>> whatever >>>>>> reason, how can you be sure that if it fails for another PR as well, >>>>>> that >>>>>> they both fail for the same reason and there is no any other reasons >>>>>> that >>>>>> will cause a problem with a PR? >>>>>> >>>>>> I don't know how failing PRs because of CVE, which we don't introduce, >>>>> don't control, no idea of and possibly unrelated would fall in the same >>>>> bucket as unit tests. I am at a loss of words for that. There is no >>>>> reason >>>>> to block legitimate development for this. This can be handled separtely >>>>> and >>>>> in parallel. Maybe there is a way we can setup an independent job on a >>>>> build server that runs nightly, fails if there are new CVEs discovered >>>>> and >>>>> sends an email out to the security or dev group. You could even reduce >>>>> the >>>>> CVE threshold for this. I don't believe in a stick approach (carrot and >>>>> stick metaphor) and believe in proportional measures. >>>>> >>>>> >>>>> >>>>> Thank you, >>>>>> >>>>>>> Vlad >>>>>>>> >>>>>>>> >>>>>>>> On 10/26/17 09:42, Pramod Immaneni wrote: >>>>>>>> >>>>>>>> On Thu, Oct 26, 2017 at 12:08 AM, Vlad Rozov <vro...@apache.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>> There is a way to add an exception, but it needs to be discussed on >>>>>>>>> a >>>>>>>>> case >>>>>>>>> >>>>>>>>> by case basis. Note that CVEs are not published until a fix is >>>>>>>>> >>>>>>>>>> available. >>>>>>>>>> For severity 8 CVEs I expect patches to become available for the >>>>>>>>>> reported >>>>>>>>>> version unless it is an obsolete version in which case, the >>>>>>>>>> upgrade >>>>>>>>>> to >>>>>>>>>> a >>>>>>>>>> supported version is already overdue. >>>>>>>>>> >>>>>>>>>> I think we should retain the ability to make that choice of what >>>>>>>>>> and >>>>>>>>>> when >>>>>>>>>> >>>>>>>>>> to upgrade rather than hard enforce it. Like I mentioned the CVE >>>>>>>>>> may >>>>>>>>>> >>>>>>>>> not >>>>>>>>> apply to us (it has happened before), even though it may be >>>>>>>>> beneficial >>>>>>>>> upgrade generally when its not applicable, there may not be the >>>>>>>>> bandwidth >>>>>>>>> in community to do the necessary changes to upgrade to a newer >>>>>>>>> version >>>>>>>>> especially if those dependencies don't follow semver (has happened >>>>>>>>> before >>>>>>>>> as well, remember effort with ning). My caution comes from >>>>>>>>> experiencing >>>>>>>>> this situation before. >>>>>>>>> >>>>>>>>> >>>>>>>>> I don't see how reporting helps. If a build succeeds, I don't >>>>>>>>> expect >>>>>>>>> >>>>>>>>> anyone to look into the report, it is only when CI build fails, >>>>>>>>> >>>>>>>>>> committers >>>>>>>>>> and reviewers look into the details. >>>>>>>>>> >>>>>>>>>> We can add a mandatory step during release that we need to assess >>>>>>>>>> CVEs >>>>>>>>>> >>>>>>>>>> matching this criteria before proceeding with the release. This >>>>>>>>>> could >>>>>>>>>> >>>>>>>>> end >>>>>>>>> up requiring upgrade of some dependencies and in other cases it may >>>>>>>>> not >>>>>>>>> be >>>>>>>>> needed. This assessment can also happen more often in adhoc fashion >>>>>>>>> offline >>>>>>>>> before release based upon interest from community. I am also open >>>>>>>>> to >>>>>>>>> making >>>>>>>>> it mandatory with every PR, in future, like you are suggesting, if >>>>>>>>> we >>>>>>>>> see >>>>>>>>> sufficient uptake in community on these issues. From experience >>>>>>>>> this >>>>>>>>> is >>>>>>>>> not >>>>>>>>> there currently and hence I don't want to do this now. >>>>>>>>> >>>>>>>>> >>>>>>>>> IMO, it does not matter how CVE is introduced. It may be a new >>>>>>>>> dependency >>>>>>>>> >>>>>>>>> with an existing CVE or it can be a new CVE for an existing >>>>>>>>> >>>>>>>>>> dependency. >>>>>>>>>> In >>>>>>>>>> both cases, dependency with the CVE needs to be fixed. >>>>>>>>>> >>>>>>>>>> In one case the PR is not directly responsible for the issue and >>>>>>>>>> hence >>>>>>>>>> >>>>>>>>>> we >>>>>>>>> should avoid penalizing them or block them. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Thank you, >>>>>>>>> >>>>>>>>> Vlad >>>>>>>>>> >>>>>>>>>> On 10/25/17 11:58, Pramod Immaneni wrote: >>>>>>>>>> >>>>>>>>>> Thanks that sounds mostly fine except what happens if there is a >>>>>>>>>> cve >>>>>>>>>> >>>>>>>>>> matching that severity in a dependency but it doesnt affect us >>>>>>>>>> >>>>>>>>>>> because >>>>>>>>>>> let's say we don't exercise that part of functionality *and* >>>>>>>>>>> there >>>>>>>>>>> isn't a >>>>>>>>>>> fix available or there is a fix but the upgrade requires >>>>>>>>>>> significant >>>>>>>>>>> effort >>>>>>>>>>> (for example if we need to move to a new major version of the >>>>>>>>>>> dependency >>>>>>>>>>> or >>>>>>>>>>> something like that). Is there a way to add an exception like we >>>>>>>>>>> did >>>>>>>>>>> for >>>>>>>>>>> checkstyle in the interim. How about reporting instead of failing >>>>>>>>>>> the >>>>>>>>>>> builds. One of the steps in release process could be to >>>>>>>>>>> investigate >>>>>>>>>>> these >>>>>>>>>>> reports before proceeding with the release. If a PR introduces >>>>>>>>>>> new >>>>>>>>>>> cves >>>>>>>>>>> by >>>>>>>>>>> virtue of adding a new dependency or changing an existing >>>>>>>>>>> version, >>>>>>>>>>> that >>>>>>>>>>> would be of interest and can be subject to failure. Is there a >>>>>>>>>>> way >>>>>>>>>>> to >>>>>>>>>>> distinguish that? >>>>>>>>>>> >>>>>>>>>>> On Wed, Oct 25, 2017 at 8:52 AM Vlad Rozov <vro...@apache.org> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> A CVE (should there be a vulnerability in existing or a newly >>>>>>>>>>> introduced >>>>>>>>>>> >>>>>>>>>>> dependency) will not be exposed during the CI build, but the >>>>>>>>>>> build >>>>>>>>>>> >>>>>>>>>>> will >>>>>>>>>>>> fail if the CVE has severity 8 or above. To get the details, it >>>>>>>>>>>> will >>>>>>>>>>>> be >>>>>>>>>>>> necessary to run dependency check manually. >>>>>>>>>>>> >>>>>>>>>>>> Thank you, >>>>>>>>>>>> >>>>>>>>>>>> Vlad >>>>>>>>>>>> >>>>>>>>>>>> On 10/24/17 16:27, Pramod Immaneni wrote: >>>>>>>>>>>> >>>>>>>>>>>> There was a lot of discussion on this but looks like there was >>>>>>>>>>>> no >>>>>>>>>>>> final >>>>>>>>>>>> >>>>>>>>>>>> agreement. Can you summarize what your PR does? Are we >>>>>>>>>>>> disclosing >>>>>>>>>>>> >>>>>>>>>>>>> the >>>>>>>>>>>>> actual vulnerabilities as part of the automated build for every >>>>>>>>>>>>> PR? >>>>>>>>>>>>> That >>>>>>>>>>>>> would be a no-no for me. If it is something that requires >>>>>>>>>>>>> manual >>>>>>>>>>>>> steps, >>>>>>>>>>>>> >>>>>>>>>>>>> for >>>>>>>>>>>>> >>>>>>>>>>>>> example as part of a release build, that would be fine. >>>>>>>>>>>>> >>>>>>>>>>>> On Mon, Oct 23, 2017 at 1:16 PM, Vlad Rozov <vro...@apache.org> >>>>>>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Please see https://github.com/apache/apex-core/pull/585 and >>>>>>>>>>>>> APEXCORE-790. >>>>>>>>>>>>> Thank you, >>>>>>>>>>>>> >>>>>>>>>>>>> Vlad >>>>>>>>>>>>> >>>>>>>>>>>>> On 9/14/17 09:35, Vlad Rozov wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> Do you expect anything else from the community to recognize a >>>>>>>>>>>>>> >>>>>>>>>>>>>> contribution other than committing it to the code line? Once >>>>>>>>>>>>>> there >>>>>>>>>>>>>> >>>>>>>>>>>>>> is >>>>>>>>>>>>>>> a >>>>>>>>>>>>>>> steady flow of quality contributions, the community/PMC will >>>>>>>>>>>>>>> recognize >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> a >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> contributor by making that contributor a committer. >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Thank you, >>>>>>>>>>>>> >>>>>>>>>>>>> Vlad >>>>>>>>>>>>>> >>>>>>>>>>>>>>> On 9/12/17 13:05, Sanjay Pujare wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> For a vendor too, quality ought to be as important as >>>>>>>>>>>>>>> security >>>>>>>>>>>>>>> so >>>>>>>>>>>>>>> I >>>>>>>>>>>>>>> don't >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> think we disagree on the cost benefit analysis. But I get >>>>>>>>>>>>>>> your >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> drift. >>>>>>>>>>>>>> By "creative incentive" I didn't imply any material incentive >>>>>>>>>>>>>> >>>>>>>>>>>>>> (although a >>>>>>>>>>>>>> >>>>>>>>>>>>>>> gift card would be nice :-)) but more along the lines of >>>>>>>>>>>>>>> what a >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> community >>>>>>>>>>>>>> can do to recognize such contribution. >>>>>>>>>>>>>> Sanjay >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Tue, Sep 12, 2017 at 8:10 AM, Vlad Rozov < >>>>>>>>>>>>>> vro...@apache.org> >>>>>>>>>>>>>> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I guess we have a different view on the benefit and cost >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> definition. >>>>>>>>>>>>>> For >>>>>>>>>>>>>> me the benefit of fixing CI build, flaky unit test, severe >>>>>>>>>>>>>> security >>>>>>>>>>>>>> issue >>>>>>>>>>>>>> >>>>>>>>>>>>>> is huge for the community and is possibly small (except for a >>>>>>>>>>>>>> >>>>>>>>>>>>>>> security >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> issues) for a vendor. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> By "creative" I hope you don't mean that other community >>>>>>>>>>>>>>> >>>>>>>>>>>>>> members, >>>>>>>>>>>>>> >>>>>>>>>>>>>> users >>>>>>>>>>>>>> >>>>>>>>>>>>>>> and customers send a contributor a gift cards to compensate >>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> cost >>>>>>>>>>>>>>> >>>>>>>>>>>>>> :). For me PR that is blocked on a failed CI build is >>>>>>>>>>>>>> >>>>>>>>>>>>>>> sufficiently >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> incentive for a contributor to look into why it fails and >>>>>>>>>>>>>>> fixing >>>>>>>>>>>>>>> >>>>>>>>>>>>>> it. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thank you, >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Vlad >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On 9/11/17 23:58, Sanjay Pujare wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> I don't want to speak for others and I don't want to >>>>>>>>>>>>>>>>> generalize. >>>>>>>>>>>>>>>>> But >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> obvious answer could be "cost-benefit analysis". >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> In any case we should come up with a creative way to >>>>>>>>>>>>>>> >>>>>>>>>>>>>> "incentivize" >>>>>>>>>>>>>> >>>>>>>>>>>>>> members >>>>>>>>>>>>>> >>>>>>>>>>>>>>> to do these tasks. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >> >
