Test vectors for scores have been recently introduced. Now we can tune scores separately from ratings. That gives more flexibility.

The RatingRepository loads an instance of OssSecurityRating from a file including OssSecurityScore. The OssScores also provides an instance of OssSecurityRating which has all weights set to 1.0. That's not correct. Instead, the RatingRepository should load scores from files. Then, the RatingRepository should offer the loaded scores to the client code.

Here is what needs to be done:

• Update the RatingRepository class to load scores.
• Store OssSecurityScore to a file.
• Remove OssScores class.

Executing mvn clean package

We face with below-mentioned warnings during the build

[WARNING] Configuration options: 'appendAssemblyId' is set to false, and 'classifier' is missing. Instead of attaching the assembly file: /target/${oversize}.jar, it will become the file for main project artifact.
NOTE: If multiple descriptors or descriptor-formats are provided for this project, the value of this file will be non-deterministic!

[WARNING] Replacing pre-existing project main-artifact file: /target/fosstars-rating-core.jar with assembly file: /target/${oversize}.jar

The test fails when the GitHub API rate limit exceeded or some networking issues occur. That's why the test is currently disabled.

The test needs to be re-written to avoid connecting to the GitHub API.

Rating calculation for https://github.com/apache/spamassassin fails with the following output:

$ java -jar target/fosstars-github-rating-calc.jar --no-questions --token $TOKEN --url https://github.com/apache/spamassassin
[+] Project: https://github.com/apache/spamassassin
[+] Let's get info about the project and calculate a security rating
[+] Counting how many commits have been done in the last three months ...
[+] Counting how many people contributed to the project in the last three months ...
[+] Counting how many stars the project has ...
[+] Counting how many watchers the project has ...
[+] Figuring out when the project started ...
[+] Figuring out if the project has a security team ...
[+] Figuring out if the project is supported by a company ...
[+] Figuring out if the project has a security policy ...
[+] Figuring out if any security review has been done for the project ...
[+] Figuring out if the project has any unpatched vulnerability ...
[+] Looking for vulnerabilities in NVD ...
[+] Figuring out if the project belongs to the Apache Software Foundation ...
[+] Figuring out if the project belongs to the Eclipse Software Foundation ...
[+] Figuring out if the project uses OWASP Dependency Check ...
[+] Figuring out if the project uses verified signed commits ...
[+] Here is what we know about the project:
[+]    If an open-source project belongs to Eclipse Foundation: false
[+]    If a project uses verified signed commits: false
[+]    If an open-source project is regularly scanned for vulnerable dependencies: false
[+]    If an open-source project has a security team: true
[+]    If a project uses LGTM: true
[+]    Number of watchers for a GitHub repository: 24
[+]    If an open-source project has a security policy: false
[+]    Info about vulnerabilities in open-source project: 11 vulnerabilities
[+]    Number of stars for a GitHub repository: 161
[+]    Number of contributors in the last three months: 0
[+]    When a project started: Fri Apr 20 20:47:31 CEST 2001
[+]    Security reviews for an open-source project: 0 security reviews
[+]    The worst LGTM grade of a project: D
[+]    Number of commits in the last three months: 229
[+]    If an open-source project belongs to Apache Foundation: true
[+]    If an open-source project is supported by a company: false
[x] Something went wrong!
java.lang.IllegalArgumentException: Commits without contributor! How is that possible?
	at com.sap.sgs.phosphor.fosstars.model.score.oss.ProjectActivityScore.check(ProjectActivityScore.java:192)
	at com.sap.sgs.phosphor.fosstars.model.score.oss.ProjectActivityScore.calculate(ProjectActivityScore.java:121)
	at com.sap.sgs.phosphor.fosstars.model.score.AbstractScore.calculate(AbstractScore.java:85)
	at com.sap.sgs.phosphor.fosstars.model.score.AbstractScore.calculateIfNecessary(AbstractScore.java:201)
	at com.sap.sgs.phosphor.fosstars.model.score.WeightedCompositeScore.calculate(WeightedCompositeScore.java:124)
	at com.sap.sgs.phosphor.fosstars.model.score.AbstractScore.calculate(AbstractScore.java:85)
	at com.sap.sgs.phosphor.fosstars.model.rating.AbstractRating.calculate(AbstractRating.java:65)
	at com.sap.sgs.phosphor.fosstars.tool.github.SingleSecurityRatingCalculator.calculateFor(SingleSecurityRatingCalculator.java:82)
	at com.sap.sgs.phosphor.fosstars.tool.github.SingleSecurityRatingCalculator.calculateFor(SingleSecurityRatingCalculator.java:41)
	at com.sap.sgs.phosphor.fosstars.tool.github.SecurityRatingCalculator.processUrl(SecurityRatingCalculator.java:151)
	at com.sap.sgs.phosphor.fosstars.tool.github.SecurityRatingCalculator.run(SecurityRatingCalculator.java:126)
	at com.sap.sgs.phosphor.fosstars.tool.github.SecurityRatingCalculator.main(SecurityRatingCalculator.java:57)
[+] Bye!

The repository is a mirror of Apache SpamAssassin. Looks like authors of commits are not connected to existing GitHub users. That's why the NumberOfContributors data provider returns 0. Let's see if the data provider may become smarter or the score may be updated.

The test fails when the GitHub API rate limit exceeded or some networking issues occur. That's why the test is currently disabled.

The test needs to be re-written to avoid connecting to the GitHub API.

The UnpatchedVulnerabilitiesScoreTestVectors notepad defines the following test vectors:

# TODO: should it be less than 10?
test_vector_list.register(
    TestVector()
        .vulnerabilities(all_vulnerabilities_fixed_slow)
        .score_from(7.0)
        .score_to(10.0)
)

# TODO: should it be less than 9?
test_vector_list.register(
    TestVector()
        .vulnerabilities(one_minor_unpatched_vulnerability)
        .score_from(5.0)
        .score_to(9.0)
)

# TODO: should it be less than 8?
test_vector_list.register(
    TestVector()
        .vulnerabilities(one_major_unpatched_vulnerability)
        .score_from(4.0)
        .score_to(8.0)
)

...

# TODO: should it be less than 8?
test_vector_list.register(
    TestVector()
        .vulnerabilities(two_minor_unpatched_vulnerabilities)
        .score_from(4.0)
        .score_to(8.0)
)

# TODO: should it be less than 6?
test_vector_list.register(
    TestVector()
        .vulnerabilities(two_major_unpatched_vulnerabilities)
        .score_from(3.0)
        .score_to(6.0)
)

The expected score ranges look to high (see the TODOs). We need to consider making the score a bit stricter.

It's hard to gather data for the SECURITY_REVIEWS_DONE in an automated way. Currently we maintain a list of security reviews in a JSON file.

Once we have more features, which contribute to the ProjectSecurityTestingScore (see for example #24), we can remove the SECURITY_REVIEWS_DONE from the OssSecurityRating until we get enough data to fill it out.

At the moment, only the MutableWeight class implements the Weight interface. Therefore, all ratings, which are returned by the RatingRepository, use this implementation. The MutableWeight is a mutable class which makes it possible to modify the ratings returned by the RatingRepository. The ratings may be modified either intentionally or accidentally. To prevent this from happening. the RatingReposotory class should return only immutable ratings.

The project uses Jackson libraries of version 2.10.1. However, never versions are available. We need to update to the latest version.

It seems to be possible that there are more contributors than commits (in the last three months).
In the sample project https://github.com/jdesboeufs/connect-mongo we encountered above situation

• "name":"Number of commits last three months","n":2
• "name":"Number of contributors last three months","n":3

Which leads to a java.lang.IllegalArgumentException: More commits than contributors! How is that possible?

Must the check in the ProjectActivityScore changed/removed?

Currently, the ProjectSecurityTestingScore is a feature-based score. It uses the SECURITY_REVIEWS_DONE and SCANS_FOR_VULNERABLE_DEPENDENCIES features. However, it needs to be updated to use the LgtmScore, see #24. Therefore, it would be better to make the ProjectSecurityTestingScore to use only scores. In order to implement that, the SECURITY_REVIEWS_DONE is going to be removed from the score, see #25. Second, the SCANS_FOR_VULNERABLE_DEPENDENCIES feature needs to be wrapped in to a new DependencyScanScore.

Currently the RatingReposotiry class maintains only ratings. The class may be updated to maintain a list of registered scores as well. It would make it possible to use scores independently.

Here is a list of things to do:

• Add register(...) and score(...) methods to the RatingRepository class.
• Introduce a version for a score and add a new Score.version() method.
• Updated the verification tests for the existing scores to get the score directly from the RatingRepository.

docs/oss_security_rating.md contains several references to source code which become broken links in GitHub pages.

OssSecurityRatingTestVectors.ipynb defines test vectors for the open-source security rating. The notepad currently displays the test vectors only in a table. It may be helpful to add some visualization for data in the test vectors. That would make it easier to analyze what kind of values are covered by the test vectors. For example, we can consider adding histograms, correlation matrices, etc.

Data Providers like:

• FirstCommit
• ProjectStarted
• NumberOfCommits
• UsesSignedCommits

Use commit history. It takes a toll on the performance when fosstars-rating-core try to hit the GitHub API again and again for each data provider to get the same data.

To avoid this redundant work, merge the data gathering method for all the above-mentioned data providers.

The test fails when the GitHub API rate limit exceeded or some networking issues occur. That's why the test is currently disabled.

The test needs to be re-written to avoid connecting to the GitHub API.

UsesOwaspDependencyCheck sometimes throws an NPE:

[+] Figuring out if the project uses OWASP Dependency Check ...
[!] Holy Moly, one of the data providers failed!
[!] The last thing that it said was: null

Looks like it happens for C/C++ projects which don't have a pom.xml file.

The test loads test vectors form a CSV file and runs tuning with CMA-ES algorithm. Each time when a new feature is added to the open-source security score, the CSV file needs to be updates. We need to split the test to two cases:

• first one should test loading data from a CSV file
• second one should test turing (actually the existing simpleTestVectors() test case may be enough)

That, we won't need to update the CSV file each time a new feature is added.

Meanwhile, the test is ignored.

When the SecurityRatingCalculator class create an instance of GitHub, it doesn't specify any timeout for connections. As a result, the tool can hang while connecting to the GitHub APIs. The class needs to be updated to use a timeout.

Currently, there is very few tests for the demo tool. We need to add more tests to verify if it works fine. For example, the test suite may be based on shell scripts. Ideally, the new test suite should be integrated to the build process. However, if it takes long, it may be run separately (in this case, the docs need to be updated to remind contributors to run the test suite).

Currently, the OssSecurityScore is a feature-based score. Since the LgtmScore is going to be added to the OssSecurityScore (see #24), the score needs to be updated to use sub-scores.

NVDTest is currently disabled since it requires downloading data from the NVD. The test needs to be updated to test only parsing data. It should not try to download data from the NVD.

Here is a test vector for the VulnerabilityLifetimeScore which shows how the score behaves if vulnerabilities are fixed slowly:

test_vector_list.register(
    TestVector()
        .vulnerabilities(all_vulnerabilities_fixed_slow)
        .when_project_started(five_years_ago)
        .when_first_commit_was_done(five_years_ago)
        .score_from(5.0)
        .score_to(9.5)
)

9.5 looks too high. The score needs to be updated to be stricter.

More test vectors should be added for the score as well.

The ObjectMapper.enableDefaultTyping() method has been deprecated, but the RatingRepository class still uses it:

  /**
   * An ObjectMapper for serialization and deserialization ratings to JSON.
   */
  private static final ObjectMapper MAPPER = new ObjectMapper();

  static {
    MAPPER.enableDefaultTyping();
  }

Instead, the RatingRepository class should use the new version of the enableDefaultTyping() method which allows specifying a validator.

LGTM is a service for static analysis which is free for open-source projects. The service also looks for security issues.

Let's consider adding a score which shows if and how an open-source project uses LTGM. Here is a couple of features which can be introduced:

• If a project uses LGTM
• The worse LGTM grade

The method writes unvalidated input into JSON. This call could allow an attacker to inject arbitrary elements or attributes into the JSON entity.

There are some classes which contain Sysouts. They should better use a Logger if possible.

The ProjectStarted data provider uses a date of the first commit to estimate when the project started. This info comes from the FirstCommit data provider. Sometimes the FirstCommit data provider fails to find the first commit and returns an unknown value. The ProjectStarted data provider doesn't expect that and fails with an exception. Let's make it a bit smarter.

The OssSecurityRatingMonteCarloTest fails sometimes with the following output:

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.211 s <<< FAILURE! - in com.sap.sgs.phosphor.fosstars.model.rating.oss.OssSecurityRatingMonteCarloTest
[ERROR] run(com.sap.sgs.phosphor.fosstars.model.rating.oss.OssSecurityRatingMonteCarloTest)  Time elapsed: 0.211 s  <<< ERROR!
java.lang.IllegalArgumentException: Weight 2.42861286636753E-17 doesn't below to the interval (0.0, 1.0]
	at com.sap.sgs.phosphor.fosstars.model.Weight.check(Weight.java:54)
	at com.sap.sgs.phosphor.fosstars.model.weight.MutableWeight.value(MutableWeight.java:29)
	at com.sap.sgs.phosphor.fosstars.model.tuning.MonteCarloWeightsOptimization.optimize(MonteCarloWeightsOptimization.java:53)
	at com.sap.sgs.phosphor.fosstars.model.tuning.AbstractWeightsOptimization.run(AbstractWeightsOptimization.java:80)
	at com.sap.sgs.phosphor.fosstars.model.rating.oss.OssSecurityRatingMonteCarloTest.run(OssSecurityRatingMonteCarloTest.java:124)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:383)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:344)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:125)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:417)

Looks like OssSecurityRatingMonteCarlo doesn't assign correct weights.

Let's add CONTRIBUTING.md and SECURITY.md files.

The RatingValue class needs to be updated to support serialization with Jackson.

Currently a data provider can provide only one feature. But sometimes a data provider may be able to fill out multiple features. To make it possible, the interface DataProvider needs to be updated to support multiple features.

At the moment, it's only possible to tune weights for sub-scores in the WeightedCompositeScore class. It would be good if it were possible to configure scores with more parameters. The parameters may be then tuned as well.

Let's update the artifact ID to fosstars-rating-core and reset the version to 0.1.0.

Let's have a page which displays ratings for well-known projects. It would be useful for demo purposes, and also it can help to catch issues in rating calculation.

Here is a list of things to be done:

1. Write a tool to generate a report for multiple projects.
2. Define a list of projects.
3. Create a report for the defined projects.

Let's add the Maven Checkstyle Plugin to the pom.xml file:

http://maven.apache.org/plugins/maven-checkstyle-plugin/index.html

Since the project uses Google Java Style Guide, the plugin should be configured the corresponding rule set.

Ideally, the plugin should be configured to break Maven build if a error or a warning is found. However, if it now reports to many errors and warnings, we can fix them in the near future and then enable breaking the build.

Currently checkstyle only reports problems but doesn't break the build. Let's fix all warnings and turn the failsOnError option to enforce style checks on build time.

NVDTest is unstable since it tries downloading data from the NVD. It needs to be improved to test only parsing data from the NVD. Meanwhile, the test needs to be disabled.

The ProjectActivityScoreTestVectors notebook has a number of TODOs which suggest to make the ProjectActivityScore a bit stricter.

# TODO: should it be less than 1.0?
test_vector_list.register(
    TestVector()
        .number_of_contributors(1)
        .number_of_commits(5)
        .score_from(0.0)
        .score_to(3.0)
)

# TODO: should it be less than 4.0?
test_vector_list.register(
    TestVector()
        .number_of_contributors(3)
        .number_of_commits(50)
        .score_from(2.0)
        .score_to(5.5)
)

# TODO: should it be less than 2.0?
test_vector_list.register(
    TestVector()
        .number_of_contributors(1)
        .number_of_commits(20)
        .score_from(0.0)
        .score_to(3.5)
)

# it's not too good if a single person maintains a project
# TODO: should the score be less than 5.0?
test_vector_list.register(
    TestVector()
        .number_of_contributors(1)
        .number_of_commits(200)
        .score_from(3.0)
        .score_to(9.0)
)

# TODO: should the score be less than 6.0?
test_vector_list.register(
    TestVector()
        .number_of_contributors(1)
        .number_of_commits(500)
        .score_from(4.0)
        .score_to(9.0)
)

We need to evaluate the TODOs and update the vectors and the score accordingly.

Currently a rating just returns a score and a label. But it doesn't explain how the score value was actually calculated. It would be good if ratings could explain how they calculate scores.

Here is several update which may be considered:

• Update ScoreValue to store feature values and sub-score values.
• Add a description to ScoreValue.
• Update scores and ratings to provide info how they calculate values.

The OssSecurityScore notepad defines the following test vectors:

# TODO: should it be stricter?
test_vector_list.register(
    TestVector()
        .security_testing_score(0.0)
        .unpatched_vulnerabilities_score(0.0)
        .vulnerability_lifetime_score(0.0)
        .security_awareness_score(0.0)
        .activity_score(5.0)
        .popularity_score(5.0)
        .community_commitment_score(5.0)
        .score_from(2.0)
        .score_to(3.0)
)

# TODO: should it be stricter?
test_vector_list.register(
    TestVector()
        .security_testing_score(5.0)
        .unpatched_vulnerabilities_score(5.0)
        .vulnerability_lifetime_score(5.0)
        .security_awareness_score(5.0)
        .activity_score(5.0)
        .popularity_score(5.0)
        .community_commitment_score(5.0)
        .score_from(4.0)
        .score_to(5.0)
)

# TODO: should it be stricter?
test_vector_list.register(
    TestVector()
        .security_testing_score(0.0)
        .unpatched_vulnerabilities_score(0.0)
        .vulnerability_lifetime_score(0.0)
        .security_awareness_score(0.0)
        .activity_score(10.0)
        .popularity_score(10.0)
        .community_commitment_score(10.0)
        .score_from(3.0)
        .score_to(5.0)
)

The expected score ranges look to high (see the TODOs). We need to consider making the score a bit stricter.

Currently it's possible to add test vectors only for ratings. If the number of features grows, then it becomes harder to maintain existing test vectors and add new ones. Instead of increasing the number of test vectors for a rating, it may be better to define test vectors for specific scores.

Currently it's possible to add test vectors only for ratings. #14 introduces test vectors for feature-based scores. However, we also have scores which consume other sub-scores. For example, see OssSecurityScore which is based on WeightedCompositeScore. Such scores needs to be covered by test vectors as well.

The Value interface already defines a convenient processIfKnown() method which calls a processor if the value is known. It would be good to adds a new processIfUnknown() which calls a processor if the value is unknown. The new method would help to simplify scores.

The LgtmScore is not currently used in the OssSecurityRating. Consider including the score to the OssSecurityScore directly, or to the SecurityTestingScore.

The LgtmScore may need to be adjusted, and test vectors for the score have to be added.

If only unknown values are passed to the Confidence.make() method, then it returns Confidence.MAX but should return Confidence.MIN. In other words, it produces inverted confidence values.

The ProjectStarted started data provider tries to load a commit history and search for the first commit. Sometimes the history can't be loaded (for example, due to network issues, or the rate limit, etc), but it tries to iterate over it. This results to an NPE.

The difference between GOOD and OKAY looks not that clear (in com.sap.sgs.phosphor.fosstars.model.rating.oss.OssSecurityRating the SecurityLabel enum).

I suggest to rename the OKAY to MODERATE.
IMHO this fits better between GOOD and BAD.
Result would be:

public enum SecurityLabel implements Label {
  BAD, MODERATE, GOOD
}

See related PR #57 which I have created.

Monte Carlo method is not practical. It was implemented only for testing purposes. Now, CMA-ES algorithm is used to tune scores.

GitHub allows signing commits to make sure that the changes come from a trusted store:

• Managing commit signature verification
• Commits API

It may be good to update the open-source security rating to check whether commits are signed or not.

Here is a list of things to be done:

• Introduce a new boolean feature which tell whether a project uses signed commits.
• Include the new feature to the security awareness score unless there is a better place.
• Update the test vectors for the updated score with the new feature. Re-tune the scores if necessary.
• Update the test vectors for the open-source security rating to use the new feature.
• Update one of the data providers to fill out the new feature in the demo tool. Or, create a new one if necessary. Make sure that the demo tool works fine.
• Add new tests.