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

Feature: Make existing results more structured #1874

Open
laurentsimon opened this issue Apr 27, 2022 · 15 comments
Open

Feature: Make existing results more structured #1874

laurentsimon opened this issue Apr 27, 2022 · 15 comments
Labels
kind/enhancement New feature or request

Comments

@laurentsimon
Copy link
Contributor

We have severity at the level of a check today.
However, within a check, severity may vary. For example, contents: write is more critical than status: write, for example.

This could then be exposed in SARIF in the action ossf/scorecard-action#210

@laurentsimon laurentsimon added the kind/enhancement New feature or request label Apr 27, 2022
@laurentsimon
Copy link
Contributor Author

@laurentsimon
Copy link
Contributor Author

friendly ping: would love to hear comments or feedback. I think this would help users prioritize their remediation. It's also something thats been asked at least a couple times in the issues

@azeemshaikh38
Copy link
Contributor

SGTM.

@justaugustus
Copy link
Member

SGTM!

@laurentsimon
Copy link
Contributor Author

laurentsimon commented May 13, 2022

I think to be even broader, we need a flag that is either positive or negative. For example, some results are positive Info() and neutral Debug().
These can be reported also in the raw results. So I think we need something like an enum to the effect of:

PositiveBonus, PositiveLow, PositiveMedium, PositiveHigh, PositiveCritical,
Neutral, NotApplicable,
NegativeLow, NegativeMedium, NegativeHigh, NegativeCritical

@laurentsimon
Copy link
Contributor Author

laurentsimon commented May 25, 2022

A first use case is to lower the severity of contents: write declared at the top-level if there's only one job and it's is a release job (needs the permission). Today score is 0 in this case, even though

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Jun 2, 2022

In addition, we should expose the File structure in the details, to allow consumer likes deps.dev to provide links to the original source code.
Another field is for developer annotation, e.g. #1907

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Jul 7, 2022

Below is how I think we could augment the existing results to be more structured:

type Impact string // PositiveLow, etc - see https://github.com/ossf/scorecard/issues/1874#issuecomment-1126265840
type Path struct{
	Type // File, URL, etc
	Value // will let you create links to original repo file, e.g. deps.dev when they show the results
	LineStart *uint
	LineEnd *uint
}

type RemediationEffort string // Minutes, Hours, Days, Weeks, etc. or Low/Medium/High/etc?

// Note: we have started work towards this in https://github.com/ossf/scorecard/issues/1850
type Remediation struct{
	Text string	// For human
        TextMarkdown string // For human
	Patch string	// For machines, e.g to create a pull request
	Effort RemediationEffort
}

type DetailID struct{
	Short string // short description
	URL string
	ID string // Will help with policy/configuration, e.g. to disable a type of detail results or to take an action, e.g. in GH Action https://github.com/ossf/scorecard-action/issues/729
}

type DevAnnotation struct {
   Text string //  string provided by developers about the check results, e.g., why they don't pass it
   Path Path // location of the annotation
}

type CheckResult struct {
  	Name    string
	Impact *Impact // Useful to deps.dev and Action https://github.com/ossf/scorecard-action/issues/729
	Remediation *Remediation
	DevAnnotation *DevAnnotation 
        Details [] struct{
	   Path Path
           Impact *Impact
	   Remediation *Remediation
           ID *DetailID 
	   Score int // no change
           DevAnnotation *DevAnnotation
       }
}

@laurentsimon
Copy link
Contributor Author

/cc @raghavkaul

@laurentsimon laurentsimon changed the title Feature: add severity to each Warn result Feature: Make existing results more structured Jul 7, 2022
@naveensrinivasan
Copy link
Member

Below is how I think we could augment the existing results to be more structured:

type Outcome string // PositiveLow, etc - see https://github.com/ossf/scorecard/issues/1874#issuecomment-1126265840
type Path struct{
	Type // File, URL, etc
	Value // will let you create links to original repo file, e.g. deps.dev when they show the results
	LineStart *uint
	LineEnd *uint
}

type RemediationEffort string // Minutes, Hours, Days, Weeks, etc. or Low/Medium/High/etc?

// Note: we have started work towards this in https://github.com/ossf/scorecard/issues/1850
type Remediation struct{
	Text string	// For human
	Patch string	// For machines, e.g to create a pull request
	Effort RemediationEffort
}

type DetailID struct{
	Short string // short description
	URL string
	ID string // Will help with policy/configuration, e.g. to disable a type of detail results or to take an action, e.g. in GH Action https://github.com/ossf/scorecard-action/issues/729
}

type DevAnnotation struct {
   Text string //  string provided by developers about the check results, e.g., why they don't pass it
   Path Path // location of the annotation
}

type CheckResult struct {
  	Name    string
	Outcome *Outcome // Useful to deps.dev and Action https://github.com/ossf/scorecard-action/issues/729
	Remediation *Remediation
	DevAnnotation *DevAnnotation 
        Details [] struct{
	   Path Path
           Outcome *Outcome
	   Remediation *Remediation
           ID *DetailID 
	   Score int // no change
           DevAnnotation *DevAnnotation
       }
}

We probably need to define the functionality and do some writeup and then define the structure/implementation. Thoughts?

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Jul 7, 2022

In terms of implementation, Path is already available (it's not exposed in cron or via json format but should be easy to add).

We already have a remediation field as well (added recently), so it should be easy to add to the final results too.

I would start the rest of the implementation by building support for each detail's Outcome. I have not thought a lot about implementation, but maybe having a details.yml (like for checks.yml) where we list all detail ID, their Outcome, short sentence description, the check they belong, and remediation information (remediation will need to support simple templating, e.g. to generate links that depend on workflow names, etc).
Or we augment the existing checks.yaml, but I'm worried it may look awful until we have finished populating all detail information. So maybe this is something we'd do once everything else is done?

I'm not sure sure how we want to expose this in the CLI: part of --format json? Or something else? I'm wondering if this format may be overwhelming for human or not.

@leec94
Copy link
Contributor

leec94 commented Feb 13, 2023

hi @laurentsimon , if you need testers in this work to make more structured results, i'm happy to help. can continue this conversation here or on slack, whatever works better

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Apr 25, 2023

Here's the short-term plan.

  1. I will provide a new option --details-format for user to decide how the existing results should be displayed. Either the existing string-based details, or a more structured JSON structure.
  2. In the meantime, we will engage with more external teams to validate - or improve - the design of a flexible approach to create checks via config files. The config files will take as input the results from step (1) and combine them to create checks. This step may provide a new result structure altogether.
  3. Hopefully, we will implement the outcome of (2).

I will share more in the next bi-weekly meeting about the current design. I also have a branch for folks to try it out on a subset of checks.

@github-actions
Copy link

github-actions bot commented Oct 1, 2023

This issue is stale because it has been open for 60 days with no activity.

@laurentsimon
Copy link
Contributor Author

don't close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

7 participants