Panic in SASTToolInCheckRuns #135

moorereason · 2021-01-18T16:02:00Z

I ran into an odd issue today:

$ ./scorecard --repo=github.com/adnanh/webhook --show-details
Starting [Active]
Starting [Branch-Protection]
Starting [CI-Tests]
Starting [CII-Best-Practices]
Starting [Code-Review]
Starting [Contributors]
Starting [Frozen-Deps]
Starting [Fuzzing]
Starting [Packaging]
Starting [Pull-Requests]
Starting [SAST]
Starting [Security-Policy]
Starting [Signed-Releases]
Starting [Signed-Tags]
Finished [Fuzzing]
Finished [CII-Best-Practices]
Finished [Branch-Protection]
Finished [Packaging]
Finished [Frozen-Deps]
Finished [Signed-Tags]
Finished [Signed-Releases]
Finished [Contributors]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x7696f1]

goroutine 29 [running]:
github.com/ossf/scorecard/checks.SASTToolInCheckRuns(0x9a53c0, 0xc00009c000, 0xc0000e0840, 0xc0000b00d0, 0xc000097620, 0xc00009a103, 0x6, 0xc00009a10a, 0x7, 0xc000032320, ...)
        /src/github.com/ossf/scorecard/checks/sast.go:54 +0x231
github.com/ossf/scorecard/checker.MultiCheck.func1(0x9a53c0, 0xc00009c000, 0xc0000e0840, 0xc0000b00d0, 0xc000097620, 0xc00009a103, 0x6, 0xc00009a10a, 0x7, 0xc000032320, ...)
        /src/github.com/ossf/scorecard/checker/check.go:57 +0xf8
github.com/ossf/scorecard/checks.SAST(0x9a53c0, 0xc00009c000, 0xc0000e0840, 0xc0000b00d0, 0xc000097620, 0xc00009a103, 0x6, 0xc00009a10a, 0x7, 0xc000032320, ...)
        /src/github.com/ossf/scorecard/checks/sast.go:32 +0xd5
github.com/ossf/scorecard/checker.(*Runner).Run(0xc0001fbea8, 0x933ba0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /src/github.com/ossf/scorecard/checker/checker.go:54 +0x1b5
github.com/ossf/scorecard/pkg.RunScorecards.func1(0xc00009c414, 0x9a53c0, 0xc00009c000, 0xc0000e0840, 0xc0000b00d0, 0xc000097620, 0xc00009a103, 0x6, 0xc00009a10a, 0x7, ...)
        /src/github.com/ossf/scorecard/pkg/scorecard.go:107 +0xd4
created by github.com/ossf/scorecard/pkg.RunScorecards
        /src/github.com/ossf/scorecard/pkg/scorecard.go:104 +0x353

If I limit the checks to just SAST, I haven't seen it panic yet. I added some debugging print statements before the range loop to see what the github package is returning. It looks something like this:

pr     = https://github.com/adnanh/webhook/pull/463
error  = <nil>
resp   = &{0xc000778d80 0 0 0 0  github.Rate{Limit:5000, Remaining:3034, Reset:github.Timestamp{2021-01-18 10:01:53 -0600 CST}}}
status = 200 OK
crs    = (*github.ListCheckRunsResults)(nil)

It would be easy to add a nil-check before accessing csr, but it seems like the github package should be returning an error or a non-nil response.

$ go version
go version go1.16beta1 linux/amd64

$ git log -1 --oneline
c00aa4b (HEAD -> main, origin/main, origin/HEAD) Add e2e tests for remaining checks.

I tried updating to github/v33, but the problem remains.

The text was updated successfully, but these errors were encountered:

naveensrinivasan · 2021-01-18T19:01:56Z

I tried it and it didn't fail for me.

./scorecard --repo=github.com/adnanh/webhook --show-details
Starting [Active]
Starting [Branch-Protection]
Starting [CI-Tests]
Starting [CII-Best-Practices]
Starting [Code-Review]
Starting [Contributors]
Starting [Frozen-Deps]
Starting [Fuzzing]
Starting [Packaging]
Starting [Pull-Requests]
Starting [SAST]
Starting [Security-Policy]
Starting [Signed-Releases]
Starting [Signed-Tags]
Finished [Fuzzing]
Finished [Frozen-Deps]
Finished [CII-Best-Practices]
Finished [Packaging]
Finished [Branch-Protection]
Finished [Signed-Tags]
Finished [Signed-Releases]
Finished [Contributors]
Finished [Security-Policy]
Finished [SAST]
Finished [Active]
Finished [Pull-Requests]
Finished [CI-Tests]
Finished [Code-Review]

RESULTS
-------
Active: Pass 10
    commits in last 90 days: 18
Branch-Protection: Fail 10
    !! branch protection not enabled
CI-Tests: Fail 8
    CI test found: pr: 489, context: github-actions, url: https://api.github.com/repos/adnanh/webhook/check-runs/1504710886
    CI test found: pr: 486, context: github-actions, url: https://api.github.com/repos/adnanh/webhook/check-runs/1454553541
    CI test found: pr: 485, context: github-actions, url: https://api.github.com/repos/adnanh/webhook/check-runs/1451381442
    CI test found: pr: 484, context: github-actions, url: https://api.github.com/repos/adnanh/webhook/check-runs/1450531646
    CI test found: pr: 479, context: github-actions, url: https://api.github.com/repos/adnanh/webhook/check-runs/1435955645
    CI test found: pr: 477, context: github-actions, url: https://api.github.com/repos/adnanh/webhook/check-runs/1414707563
    CI test found: pr: 472, context: github-actions, url: https://api.github.com/repos/adnanh/webhook/check-runs/1302651275
    !! found merged PR without CI test: 469
    !! found merged PR without CI test: 465
    !! found merged PR without CI test: 463
    !! found merged PR without CI test: 462
    !! found merged PR without CI test: 461
    !! found merged PR without CI test: 460
    !! found merged PR without CI test: 449
    !! found merged PR without CI test: 447
    !! found merged PR without CI test: 446
    !! found merged PR without CI test: 445
    !! found merged PR without CI test: 432
    !! found merged PR without CI test: 431
    !! found merged PR without CI test: 427
    !! found merged PR without CI test: 426
    !! found merged PR without CI test: 420
    !! found merged PR without CI test: 417
    !! found merged PR without CI test: 415
    !! found merged PR without CI test: 413
    found CI tests for 7 of 25 merged PRs
CII-Best-Practices: Fail 10
    no badge found
Code-Review: Pass 10
    found pr with committer different than author: 489
    found pr with committer different than author: 486
    found pr with committer different than author: 485
    found pr with committer different than author: 484
    found review approved pr: 479
    found review approved pr: 477
    found pr with committer different than author: 472
    found pr with committer different than author: 469
    found pr with committer different than author: 465
    found pr with committer different than author: 463
    found pr with committer different than author: 462
    found pr with committer different than author: 461
    found pr with committer different than author: 460
    found pr with committer different than author: 449
    found pr with committer different than author: 447
    found pr with committer different than author: 446
    found pr with committer different than author: 445
    found pr with committer different than author: 432
    found pr with committer different than author: 431
    found pr with committer different than author: 427
    found pr with committer different than author: 426
    found pr with committer different than author: 420
    found pr with committer different than author: 417
    found pr with committer different than author: 415
    found pr with committer different than author: 413
    github code reviews found
Contributors: Pass 10
    companies found: saburly,medevit,voxmedia
Frozen-Deps: Pass 10
    go modules found: go.mod
Fuzzing: Fail 10
Packaging: Fail 10
    !! not a packaging workflow: .github/workflows/build.yml
Pull-Requests: Pass 9
    !! found commit without PR: b1f69564a3fa41b856258cb668ce0032c29294c7, committer: adnanh
    !! found commit without PR: 159cb4a911d6499b70a9bab0d4ab16d142bf2af9, committer: adnanh
    found commit with PR: b5af9a396881aa9a103c0daeed6060e7959add57
    found commit with PR: 2e4aea4cbc6e43b8aab4a063c784f6b017a0115c
    found commit with PR: b6e5b11174d01d4c53b3c24cdd3c63dbe7feb24d
    found commit with PR: 9dec52c727e41866fda3bdbb6a8f8b3bbf862517
    found commit with PR: f2b536dbad4a7cb8da1fbef6f506b226203c6997
    found commit with PR: 62f9c01cab97598969456faa1148f3cc32c4f817
    found commit with PR: 6d2f26d95283a77a97722adb57a2c6a2574576b3
    found commit with PR: c2ffd465c4db38bb256606bcb8b955ec9392a442
    found commit with PR: 3e18a060ae290e7fdd35d080aab5af4b3ecd7f35
    found commit with PR: 6f5962f8f2592b75ccf55900eb7d27512f99c74a
    found commit with PR: 346c761ef6f504362bb0869eae5a894ad39f2f1f
    found commit with PR: e513eb4bf46885046be32d348d4c823e93e62121
    found commit with PR: 22c8a1670b92a879580eebe48397edd343dab12c
    !! found commit without PR: 9c7f8c1ac4993c56658c4afacda5f3d40630d11e, committer: web-flow
    found commit with PR: 4fadb1171f22c0ef350f02b2e8a93eacb6c39530
    found commit with PR: dc184d2737ff15e76a953cb76b34576e055e3ee9
    found commit with PR: 7467933680e8e8b6b45ee5c8bb66d3d1fb37a256
    found commit with PR: fd50118712af7d82729de9e4c9c619d0aeca6f5f
    found commit with PR: 67c317e741b6a91dcde1d69a51ee8c180022cf20
    found commit with PR: ab3ff0343ecd53ba872edc8c2704037bb98401de
    found commit with PR: f007fa52809edc645cddb27ce688ce73f58aad92
    found commit with PR: a904537367e0c7d36976e723f1c1a596c3498670
    found commit with PR: 0814b10a16f8e53280d9e2138ae3a7b95381514a
    found commit with PR: d2795059303521cac3fdd0a1c4feeec406512514
    found commit with PR: 0f4bbfac9f871fd3b6c1c14250186a3ac30a3655
    found commit with PR: 6bbf14f7d9fc0f354ef0a313e76e69e93f83a6d2
    found commit with PR: 6797bf7cf71bbc5e87418126be7bf19b79e01564
    found commit with PR: c6603894c1215ded1e717594b05dfb7cdd2dd81b
    found PRs for 27 out of 30 commits
SAST: Fail 10
Security-Policy: Fail 10
Signed-Releases: Fail 10
    release found: 2.8.0
    !! release 2.8.0 has no signed artifacts
    release found: 2.7.0
    !! release 2.7.0 has no signed artifacts
    release found: 2.6.11
    !! release 2.6.11 has no signed artifacts
    release found: 2.6.10
    !! release 2.6.10 has no signed artifacts
    release found: 2.6.9
    !! release 2.6.9 has no signed artifacts
    release found: 2.6.8
    !! release 2.6.8 has no signed artifacts
    found signed artifacts for 0 out of 6 releases
Signed-Tags: Fail 0
    error, retrying: GET https://api.github.com/repos/adnanh/webhook/git/tags/4e1719d9660a6eb6d9da3eb7967c9658b8c8638b: 404 Not Found []

go version
go version go1.15.6 darwin/amd64

c00aa4b (HEAD -> fix/panic-sast, origin/main, origin/HEAD, main, feat/docker-static-builds) Add e2e tests for remaining checks.

moorereason · 2021-01-18T19:09:25Z

@naveensrinivasan,
How many times did you try it? I forgot to mention that it's intermittent.

I also tried with go1.15.6, and it still panics.

naveensrinivasan · 2021-01-18T19:19:47Z

@naveensrinivasan,
How many times did you try it? I forgot to mention that it's intermittent.

I also tried with go1.15.6, and it still panics.

You are right it fails when I try it within a loop. Thanks for that information 😄 .

Fixed the panic by doing a nil check. Fixes #135

naveensrinivasan added a commit that referenced this issue Jan 18, 2021

fix - panic on nil

0b68fb1

Fixed the panic by doing a nil check. Fixes #135

naveensrinivasan mentioned this issue Jan 18, 2021

fix - panic on nil #136

Merged

1 task

naveensrinivasan closed this as completed in #136 Jan 18, 2021

naveensrinivasan added a commit that referenced this issue Jan 18, 2021

fix - panic on nil

33e9189

Fixed the panic by doing a nil check. Fixes #135

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Panic in SASTToolInCheckRuns #135

Panic in SASTToolInCheckRuns #135

moorereason commented Jan 18, 2021

naveensrinivasan commented Jan 18, 2021 •

edited

Loading

moorereason commented Jan 18, 2021

naveensrinivasan commented Jan 18, 2021

Panic in SASTToolInCheckRuns #135

Panic in SASTToolInCheckRuns #135

Comments

moorereason commented Jan 18, 2021

naveensrinivasan commented Jan 18, 2021 • edited Loading

moorereason commented Jan 18, 2021

naveensrinivasan commented Jan 18, 2021

naveensrinivasan commented Jan 18, 2021 •

edited

Loading