[Proposal] Rules Severities for Marketplace Technical Review #14
Comments
lenaorobei
added
proposal
lenaorobei
changed the title
|
Hi @lenaorobei, Also, I recommend using the Checks to build a score and publish on Marketplace in these 3 categories. Categories:
Socre and Bagde:
The Badge and Score can be used to find nice Extension also, it can be used for Marketplace Ranking and Trust. I think most extension provider will update their Normal Extension to get a better ranking in the Marketplace. |
|
@larsroettig thanks for the feedback! Answering to your concerns about security - they are possible issues, so I didn't assign severity 10 to them. For example
I'm not sure we are ready now to reject extensions because of wrong annotation. Once we'll bring this message to developers and spread the idea about Magento coding standard, we can make rules stricter. I like the idea you've described very much! Scores and badges is definitely what we're aiming to. To make it happen we need to define severities and start using this coding standard. Then we will be able to move forward with more complex calculations. |
|
That possible security issues are not rejected got my attention at first sight as well. I am not 100% sure what the current security-related sniffs check and how many false positives they produce. However, if rejecting extensions "because of wrong annotation" is your only concern, I would not care. Even if this is quite strict, it is probably easy to fix and hence, a rejection is not a big issue as long as it leads to better security in Magento extensions in general. |
|
@sprankhub I see your point and the rejection of extensions with XSS issues is what we are striving to, but I think we should start with explanation and popularization the idea of how to write good code and warn that every 9, 8 or 7 severity issue might became 10 in future. |
|
A warning for possible security issues is not enough IMO. I participate in the bug bounty program and I actively look for Things that have been added to the code and marked as "secure enough" are often broken and used in attack chains. If it is manually reviewed by a human who thinks "this is secure and can't be broken" there is still the chance they are mistaken. I know some popular modules still use the php |
|
@larsroettig Thanks for the scoring suggestion - it aligns well with a plan we had to encourage extension developers to fix the warnings too. The scoring mechanism will be incorporated along with this consolidation work as part of the enhancement of phpcs tests in EQP 2 with a plan for a beta release initially not affecting any submissions, with plans to give developers enough time to be ready when it is fully activated. |
- changed rules severities based on #14
lenaorobei
added
accepted
|
Resolved in ##37 |
…-coding-standard-211 [Imported] #210: Fixed HtmlDirectiveSniff.php from causing the Fatal Error for c…
Marketplace Technical Review
PHP CodeSniffer is one of the tools Magento Marketplace uses for extensions verification. Find more details about current approach here.
Severities is the way to determine how strict the rule is. The higher is the severity the stricter the rule is.
Proposed Severities Definition
Proposed Rules Severities
The Magento standard contains 96 sniffs.
Fixable column identifies whether issue can be fixed by
phpcbftoolMage::) classes constructions.finalclasses and methods is prohibited.goto.===and!===operators when checkingsrtpos,stripos,array_searchexit,echo, backquotes).evalis discouraged.newkeyword.ObjectManagerusage.Exception.$thisvariable files.varlanguage construction.+operator.globalkeyword.else ifstatements (elseifshould be used instead).The text was updated successfully, but these errors were encountered: