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.

Sign up for GitHub

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

Jump to bottom

✨ [experimental] Create probes within findings #2919

Merged
merged 12 commits into from
May 3, 2023
Merged

✨ [experimental] Create probes within findings #2919

merged 12 commits into from
May 3, 2023

Conversation

laurentsimon
Copy link
Contributor

This PR introduces the concept of probes, which will have a unique ID. These used to be called "rules".

Create probes within findings

@laurentsimon laurentsimon changed the title ✨ Create probes within findings ✨ [experimental] Create probes within findings Apr 26, 2023
@laurentsimon laurentsimon temporarily deployed to integration-test April 26, 2023 23:53 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Apr 27, 2023 •
edited
Loading

Codecov Report

Merging #2919 (ff212a8) into main (700faf1) will decrease coverage by 0.12%.
The diff coverage is 53.21%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2919      +/-   ##
==========================================
- Coverage   52.67%   52.55%   -0.12%     
==========================================
  Files         158      159       +1     
  Lines       12066    12202     +136     
==========================================
+ Hits         6356     6413      +57     
- Misses       5339     5414      +75     
- Partials      371      375       +4

@laurentsimon laurentsimon temporarily deployed to integration-test April 27, 2023 00:27 — with GitHub Actions Inactive
@laurentsimon laurentsimon temporarily deployed to integration-test April 27, 2023 00:34 — with GitHub Actions Inactive
spencerschrock
spencerschrock reviewed Apr 28, 2023
View reviewed changes
Copy link
Member

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to skim over some parts but wanted to get the partial review out before I travel a bit.

checks/evaluation/permissions/permissions.go Outdated Show resolved Hide resolved
checks/evaluation/permissions/permissions.go Show resolved Hide resolved
checks/evaluation/permissions/permissions.go Show resolved Hide resolved
checks/evaluation/permissions/permissions.go Show resolved Hide resolved
finding/probe/testdata/invalid-risk.yml Outdated Show resolved Hide resolved
finding/finding.go Show resolved Hide resolved
finding/finding_test.go Outdated Show resolved Hide resolved
finding/finding.go Outdated Show resolved Hide resolved
finding/finding.go Outdated Show resolved Hide resolved
finding/finding.go Outdated Show resolved Hide resolved
@laurentsimon
Copy link
Contributor Author

Thanks for the review! Made the changes, PTAL

@laurentsimon laurentsimon temporarily deployed to integration-test April 28, 2023 22:29 — with GitHub Actions Inactive
spencerschrock
spencerschrock approved these changes May 3, 2023
View reviewed changes
Copy link
Member

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall LGTM as experimental changes.

I think we should do a thorough review before GA, as there will likely be a lot of TODOs left. Thanks for opening #2928 as it helps track things.

checks/evaluation/permissions/permissions.go Outdated Show resolved Hide resolved
finding/finding.go Outdated Show resolved Hide resolved
finding/probe/probe_test.go Show resolved Hide resolved
finding/finding_test.go Outdated Show resolved Hide resolved
finding/finding.go Outdated Show resolved Hide resolved
@laurentsimon
Copy link
Contributor Author

Still think the const approach is a good step for now.

I've made the changes.

@laurentsimon
update
f97a034 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
11d7c7c 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
33d9954 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
c8c6cbf 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
bb66160 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
155360a 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
f23475c 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
741bb4c 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
bc0bae6 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
ee21c74 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
064ebe2 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
update
ff212a8 
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon laurentsimon enabled auto-merge (squash) May 3, 2023 00:30
@laurentsimon laurentsimon temporarily deployed to integration-test May 3, 2023 00:30 — with GitHub Actions Inactive
@laurentsimon laurentsimon merged commit a4da39a into ossf:main May 3, 2023
raghavkaul pushed a commit to raghavkaul/scorecard that referenced this pull request May 3, 2023
@laurentsimon @raghavkaul
✨ [experimental] Create probes within findings (ossf#2919)
4f93a00 
* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

---------

Signed-off-by: laurentsimon <[email protected]>
balteravishay pushed a commit to balteravishay/scorecard that referenced this pull request May 29, 2023
@laurentsimon @balteravishay
✨ [experimental] Create probes within findings (ossf#2919)
770f97e 
* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

---------

Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: Avishay <[email protected]>
@spencerschrock spencerschrock mentioned this pull request Apr 5, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spencerschrock spencerschrock spencerschrock approved these changes

@azeemshaikh38 azeemshaikh38 Awaiting requested review from azeemshaikh38

@justaugustus justaugustus Awaiting requested review from justaugustus

@naveensrinivasan naveensrinivasan Awaiting requested review from naveensrinivasan

@raghavkaul raghavkaul Awaiting requested review from raghavkaul

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@laurentsimon @spencerschrock