BUG: Patch Maintainers Annotations

[gabibguti]

Describe the bug
Some things still need to be addressed in Maintainers Annotation feature (regarding PR #3905):

• In checker/check_result.go, rename IsExempted to Annotations ( ⚠️ Add initial Maintainers Annotation parsing #3905 (comment))
• In config/annotations.go, change IsValidReason to private method as isValidReason ( ⚠️ Add initial Maintainers Annotation parsing #3905 (comment)).
• similarly, these errors can probably be private?

  scorecard/config/config.go

  Lines 28 to 31 in 19f8ae1

  	var (
  	ErrInvalidCheck = errors.New("check is not valid")
  	ErrInvalidReason = errors.New("reason is not valid")
  	)

• Clarify the desired types for annotations. Define if Paths should be under a Reason or under an Annotation and same thing for Probes. We need to understand the better way for users to fill in the annotations ( ⚠️ Add initial Maintainers Annotation parsing #3905 (comment)).
• Define if one invalid check or reason during the config parsing should invalidate the whole config, or if it should be logged and skipped ( ⚠️ Add initial Maintainers Annotation parsing #3905 (comment) and ⚠️ Add initial Maintainers Annotation parsing #3905 (comment)).
• Define if in the case a check is annotated it should not be removed from SARIF result runs. Or, if is working as expected and we should not populate runs if there are no relevant results for that check ( ⚠️ Add initial Maintainers Annotation parsing #3905 (comment)).
• Do we need to pass in checks here? Also this doesn't work if you only run a subset of checks but there's an annotation for a check which isn't being run.

  scorecard/config/config.go

  Lines 73 to 74 in 19f8ae1

  	// Parse reads the configuration file from the repo, stored in scorecard.yml, and returns a `Config`.
  	func Parse(r io.Reader, checks []string) (Config, error) {

• Closing the file with t.Cleanup which is opened here:

  scorecard/config/config_test.go

  Lines 134 to 138 in 19f8ae1

  	r, err := os.Open(tt.configPath)
  	if err != nil {
  	t.Fatalf("Could not open config test file: %s", tt.configPath)
  	}
  	result, err := config.Parse(r, allChecks)

• Make all output format options pointers for consistency? AsJSON2, AsString, etc:

  scorecard/pkg/json.go

  Lines 131 to 134 in 19f8ae1

  	// AsJSON2 exports results as JSON for new detail format.
  	func (r *ScorecardResult) AsJSON2(writer io.Writer,
  	checkDocs docs.Doc, o AsJSON2ResultOption,
  	) error {

• Support more config file locations (e.g. .scorecard.yml, .github/.scorecard.yml, ...)
• Update the config README, it uses outdated yaml tags / values.
  • Document config file locations

[gabibguti]

Related to: #1907

[raghavkaul]

Do we need to pass in checks here? Also this doesn't work if you only run a subset of checks but there's an annotation for a check which isn't being run.

We should not pass checks []string here, but there's an import cycle b/w checks and config which means we can't use checks.GetAll in this file to validate the YAML against available checks in Scorecard. Looking into this.

Define if one invalid check or reason during the config parsing should invalidate the whole config, or if it should be logged and skipped.

I think this should be all-or-nothing: maintainers should validate the annotation file by running scorecard on their repo locally before committing it. Closing.

When might a valid annotation file become invalid over time?

• valid reason is removed
• check is removed

Neither of these cases is typical.

[spencerschrock]

• Define if in the case a check is annotated it should not be removed from SARIF result runs. Or, if is working as expected and we should not populate runs if there are no relevant results for that check

Based on this comment, I think the runs field should continue to be populated:

scorecard/pkg/sarif.go

Lines 644 to 649 in 9cd1fb8

	// We need to create a run entry even if the check is disabled or the policy is satisfied.
	// The reason is the following: if a check has findings and is later fixed by a user,
	// the absence of run for the check will indicate that the check was *not* run,
	// so GitHub would keep the findings in the dashboard. We don't want that.
	category, err := computeCategory(sarifCheckName, doc.GetSupportedRepoTypes())
	if err != nil {