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

✨ Update message for org-level security policy files #1939

Merged
merged 35 commits into from
May 26, 2022
Merged

✨ Update message for org-level security policy files #1939

merged 35 commits into from
May 26, 2022

Conversation

aidenwang9867
Copy link
Contributor

@aidenwang9867 aidenwang9867 commented May 22, 2022

What kind of change does this PR introduce?

Introduce a non-breaking feature.

What is the current behavior?

As described in #1908, the org-defined security policy file is reported as SECURITY.md instead of github.com/org/.github/SECURITY.md.

What is the new behavior (if this is a feature change)?**

Scorecards can report more granular information/messages for an org-defined (global) security policy as github.com/ORG_NAME/.github/SECURITY.md in the detailed logger.

  • Tests for the changes have been added (for bug fixes/features)
    No new unit tests have been added for this specific bug fix since (1) the changes don't affect the original correct judgment logic of the security policy file; (2) the root cause for this bug is trivial: when an org-defined security policy is detected, it doesn't set the file.Type to 4 (an URL) and the file.Path is not filled with the client URI (instead, it is just a file path in the repo with file.Type=1)

Manual testcases like go run . --repo=aidenwang9867/scorecard --format json --checks Security-Policy --show-details --verbosity debug | jq (security policy in repo), go run . --repo=aidenwang9867/vld --format json --checks Security-Policy --show-details --verbosity debug | jq (security policy in org) can be used to verify the correctness of my changes.

  • Additional unit testcases have been added (see changes in checks/security_policy_test.go)
  1. case insensitive testing for a security policy file name

  2. testing for the failure of a non-security policy filename (error triggered, might be a potential bug, see in issue 🐛 BUG found in the Security-Policy unit test & more testcases are needed #1954)

Which issue(s) this PR fixes

#1908

Special notes for your reviewer

N/A

Does this PR introduce a user-facing change?

No.

Update message for org-level security policy files

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 22, 2022 09:24 Inactive
@github-actions
Copy link

Integration tests success for
[e8065ec]
(https://github.com/ossf/scorecard/actions/runs/2365971254)

@codecov
Copy link

codecov bot commented May 22, 2022

Codecov Report

Merging #1939 (57722f1) into main (d1714a2) will increase coverage by 3.06%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main    #1939      +/-   ##
==========================================
+ Coverage   50.79%   53.85%   +3.06%     
==========================================
  Files          83       83              
  Lines        6719     6729      +10     
==========================================
+ Hits         3413     3624     +211     
+ Misses       3078     2870     -208     
- Partials      228      235       +7     

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 23, 2022 18:08 Inactive
@github-actions
Copy link

Integration tests success for
[af8019c]
(https://github.com/ossf/scorecard/actions/runs/2373110585)

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 23, 2022 18:53 Inactive
@github-actions
Copy link

Integration tests success for
[2255b84]
(https://github.com/ossf/scorecard/actions/runs/2373318645)

checks/evaluation/security_policy.go Outdated Show resolved Hide resolved
checks/evaluation/security_policy.go Outdated Show resolved Hide resolved
checks/evaluation/security_policy.go Outdated Show resolved Hide resolved
checks/raw/security_policy.go Outdated Show resolved Hide resolved
checks/raw/security_policy.go Outdated Show resolved Hide resolved
checks/security_policy_test.go Outdated Show resolved Hide resolved
checks/raw/security_policy.go Outdated Show resolved Hide resolved
@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 24, 2022 04:59 Inactive
@github-actions
Copy link

Integration tests success for
[4a78687]
(https://github.com/ossf/scorecard/actions/runs/2375569588)

@github-actions
Copy link

Integration tests success for
[eed9854]
(https://github.com/ossf/scorecard/actions/runs/2386523878)

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 25, 2022 19:17 Inactive
@github-actions
Copy link

Integration tests success for
[c8e5a96]
(https://github.com/ossf/scorecard/actions/runs/2386619957)

checks/raw/security_policy.go Show resolved Hide resolved
@laurentsimon laurentsimon enabled auto-merge (squash) May 25, 2022 19:37
@laurentsimon laurentsimon temporarily deployed to integration-test May 25, 2022 19:37 Inactive
@laurentsimon laurentsimon enabled auto-merge (squash) May 25, 2022 19:45
@laurentsimon
Copy link
Contributor

Some pre-submits are failing, PTAL https://github.com/ossf/scorecard/runs/6598997286?check_suite_focus=true

If you see errors like

protoc --go_out=../../../ cron/data/request.proto
/bin/bash: protoc: command not found
make: *** [Makefile:108: cron/data/request.pb.go] Error 127
Warning: Attempt 1 failed. Reason: Child_process exited with error code 2

this is a known problem. Let me know if all other pre-submits are failing and I'll merge in

@github-actions
Copy link

Integration tests success for
[99581f3]
(https://github.com/ossf/scorecard/actions/runs/2386717239)

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 25, 2022 22:59 Inactive
@github-actions
Copy link

Integration tests success for
[a459ac2]
(https://github.com/ossf/scorecard/actions/runs/2387605463)

auto-merge was automatically disabled May 25, 2022 23:51

Head branch was pushed to by a user without write access

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 25, 2022 23:51 Inactive
@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 25, 2022 23:52 Inactive
@github-actions
Copy link

Integration tests success for
[0196433]
(https://github.com/ossf/scorecard/actions/runs/2387789799)

@github-actions
Copy link

Integration tests success for
[e870c10]
(https://github.com/ossf/scorecard/actions/runs/2387792789)

@laurentsimon laurentsimon enabled auto-merge (squash) May 26, 2022 00:11
auto-merge was automatically disabled May 26, 2022 04:04

Head branch was pushed to by a user without write access

@aidenwang9867 aidenwang9867 temporarily deployed to integration-test May 26, 2022 04:04 Inactive
@github-actions
Copy link

Integration tests success for
[57722f1]
(https://github.com/ossf/scorecard/actions/runs/2388575071)

@laurentsimon laurentsimon enabled auto-merge (squash) May 26, 2022 15:13
@laurentsimon laurentsimon merged commit 3e2c0fa into ossf:main May 26, 2022
@aidenwang9867 aidenwang9867 deleted the issue1908 branch May 26, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants