search

ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source
https://scorecard.dev
Apache License 2.0
4.36k stars 478 forks source link

SAST false positive: CodeQL steps in main workflow not detected #1580

Open mjpieters opened 2 years ago

mjpieters commented 2 years ago

My current build workflow clearly uses CodeQL steps:

      # Initializes the CodeQL tools for scanning.
      - name: Initialize CodeQL
        uses: github/codeql-action/init@1a927e9307bc11970b2c679922ebc4d03a5bd980 # v1
        with:
          languages: ${{ matrix.language }}

      - name: Perform CodeQL Analysis
        uses: github/codeql-action/analyze@1a927e9307bc11970b2c679922ebc4d03a5bd980 # v1

but this is not being detected:

SAST tool detected but not run on all commmits:  
Warn: 0 commits out of 6 are checked with a SAST tool

Perhaps the check needs to be updated?

To be explicit: PRs require the CodeQL checks to pass before they can be merged.

Possible cause: I removed the github/codeql-action/autobuild action as my repository doesn't require a compilation step.

laurentsimon commented 2 years ago

I ran scorecard on your repo now and got:

"details": [
        "Warn: 1 commits out of 7 are checked with a SAST tool",
        "Info: SAST tool detected: CodeQL"
      ],
      "score": 7,
      "reason": "SAST tool detected but not run on all commits",

Until you get all last 30 commits checked with CodeQL, you'll see the alert. Once the last 30 commits have CodeQL run on them, the alert will disappear.

Is the message SAST tool detected but not run on all commits what's causing confusion? Shall it be updated to SAST tool (CodeQL) detected but not run on all commits?

mjpieters commented 2 years ago

The '1 commit' is there because I added LGTM later on. The other 6 commits before that are all covered by CodeQL.

The issue is that all 7 commits are covered by a SAST tool, not just the last one.

mjpieters commented 2 years ago

Looking over the SAST step, I see it relies on Github search to detect workflows using the github/codeql-action/analyze action. Unfortunately, search is unreliable, e.g. the specific search gives no results on my repository, even though I clearly am using the action.

At fault is the / in the search text, which GitHub search seems to trip over, a search for the three individual words without slashes succeeds.

Perhaps, instead of using code search, the plugin should load the workflow definition for each run attached to a PR (using the head_ref to get the right revision) and check if there is a use: line in the yaml definition for the action?

laurentsimon commented 2 years ago

Good catch. This will be fixed in https://github.com/ossf/scorecard/pull/1487 which parses the action's yaml instead of using the search API.

heitorlessa commented 1 year ago

Just run into this - been using CodeQL (and LGTM prior to that) for ~2 years and Scorecard thinks 0 commits have been scanned.

Workflow: https://github.com/aws-powertools/powertools-lambda-python/blob/develop/.github/workflows/codeql-analysis.yml#L37

Any recommendation?

spencerschrock commented 1 year ago

Just run into this - been using CodeQL (and LGTM prior to that) for ~2 years and Scorecard thinks 0 commits have been scanned.

Scorecard only looks back 30 commits, so it wouldn't detect that far back.

Workflow: https://github.com/aws-powertools/powertools-lambda-python/blob/develop/.github/workflows/codeql-analysis.yml#L37

Any recommendation?

My guess is it's due to running when merged, but not in the PR before being merged (which as you point out can be slow):

# NOTE: This is our slowest workflow hence it only runs on code merged.
#
# Always triggered on PR merge when source code changes.

on:
  push:
    branches: [develop]

The way scorecard looks for codeql in commits is by looking at the checks at the head commit of the associated PR.

The checks currently looks for known Github apps such as CodeQL (github-code-scanning) or SonarCloud in the recent (~30) merged PRs, or the use of "github/codeql-action" in a GitHub workflow.

There may be a way for us to grab the data from the merge commit, but I don't have the time to play around with it right this second.

heitorlessa commented 1 year ago

Thanks a lot for clarifying @spencerschrock!

I (wrongly?) assumed that with our policy to squash and merge PRs then run CodeQL on them (squashed & merged commit) was sufficient for scorecard.

I'm not a Go person but happy to learn and try helping if you could point out where to start/look ;-)

Thanks!!

heitorlessa commented 1 year ago

Did some initial digging and the head commit associated to the PR has all the appropriate SAST checks, however the Slug is not what scorecard expects.

Notes as I dig into this: https://gist.github.com/heitorlessa/04a4f1bd6b9c9185455c85ce77f13147

Areas that I need investigate

Command used to fetch check runs

gh api \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/aws-powertools/powertools-lambda-python/commits/ec9c45148a4228ddfb4717259246881c35d7261e/check-runs

Excerpt of CodeQL Run

{
    "id": 14848337843,
    "name": "Analyze",
    "node_id": "CR_kwDODTo4k88AAAADdQensw",
    "head_sha": "ec9c45148a4228ddfb4717259246881c35d7261e",
    "external_id": "b9afb22f-0837-5d02-bc24-6c30aee08bcf",
    "url": "https://api.github.com/repos/aws-powertools/powertools-lambda-python/check-runs/14848337843",
    "html_url": "https://github.com/aws-powertools/powertools-lambda-python/actions/runs/5482980956/jobs/9988864866",
    "details_url": "https://github.com/aws-powertools/powertools-lambda-python/actions/runs/5482980956/jobs/9988864866",
    "status": "completed",
    "conclusion": "success",
    "started_at": "2023-07-07T05:09:37Z",
    "completed_at": "2023-07-07T05:43:05Z",
    "output": {
        "title": null,
        "summary": null,
        "text": null,
        "annotations_count": 0,
        "annotations_url": "https://api.github.com/repos/aws-powertools/powertools-lambda-python/check-runs/14848337843/annotations"
    },
    "check_suite": {
        "id": 14124947525
    },
    "app": {
        "id": 15368,
        "slug": "github-actions",
        "node_id": "MDM6QXBwMTUzNjg=",
        "owner": {
            "login": "github",
            "id": 9919,
            "node_id": "MDEyOk9yZ2FuaXphdGlvbjk5MTk=",
            "avatar_url": "https://avatars.githubusercontent.com/u/9919?v=4",
            "gravatar_id": "",
            "url": "https://api.github.com/users/github",
            "html_url": "https://github.com/github",
            "followers_url": "https://api.github.com/users/github/followers",
            "following_url": "https://api.github.com/users/github/following{/other_user}",
            "gists_url": "https://api.github.com/users/github/gists{/gist_id}",
            "starred_url": "https://api.github.com/users/github/starred{/owner}{/repo}",
            "subscriptions_url": "https://api.github.com/users/github/subscriptions",
            "organizations_url": "https://api.github.com/users/github/orgs",
            "repos_url": "https://api.github.com/users/github/repos",
            "events_url": "https://api.github.com/users/github/events{/privacy}",
            "received_events_url": "https://api.github.com/users/github/received_events",
            "type": "Organization",
            "site_admin": false
        },
        "name": "GitHub Actions",
        "description": "Automate your workflow from idea to production",
        "external_url": "https://help.github.com/en/actions",
        "html_url": "https://github.com/apps/github-actions",
        "created_at": "2018-07-30T09:30:17Z",
        "updated_at": "2019-12-10T19:04:12Z",
        "permissions": {
            "actions": "write",
            "administration": "read",
            "checks": "write",
            "contents": "write",
            "deployments": "write",
            "discussions": "write",
            "issues": "write",
            "merge_queues": "write",
            "metadata": "read",
            "packages": "write",
            "pages": "write",
            "pull_requests": "write",
            "repository_hooks": "write",
            "repository_projects": "write",
            "security_events": "write",
            "statuses": "write",
            "vulnerability_alerts": "read"
        },
        "events": [
            "branch_protection_rule",
            "check_run",
            "check_suite",
            "create",
            "delete",
            "deployment",
            "deployment_status",
            "discussion",
            "discussion_comment",
            "fork",
            "gollum",
            "issues",
            "issue_comment",
            "label",
            "merge_group",
            "milestone",
            "page_build",
            "project",
            "project_card",
            "project_column",
            "public",
            "pull_request",
            "pull_request_review",
            "pull_request_review_comment",
            "push",
            "registry_package",
            "release",
            "repository",
            "repository_dispatch",
            "status",
            "watch",
            "workflow_dispatch",
            "workflow_run"
        ]
    },
    "pull_requests": [
        {
            "url": "https://api.github.com/repos/Pandinosaurus/aws-lambda-powertools-python/pulls/1",
            "id": 1323586820,
            "number": 1,
            "head": {
                "ref": "develop",
                "sha": "50949b2d0af3933e88fb6f8623926edab7a39ee1",
                "repo": {
                    "id": 221919379,
                    "url": "https://api.github.com/repos/aws-powertools/powertools-lambda-python",
                    "name": "powertools-lambda-python"
                }
            },
            "base": {
                "ref": "develop",
                "sha": "0c16011a1c0486d3a9fcff16c283e465b0f3ffcd",
                "repo": {
                    "id": 630920868,
                    "url": "https://api.github.com/repos/Pandinosaurus/aws-lambda-powertools-python",
                    "name": "aws-lambda-powertools-python"
                }
            }
        },
        {
            "url": "https://api.github.com/repos/pecigonzalo/aws-lambda-powertools-python/pulls/29",
            "id": 995612880,
            "number": 29,
            "head": {
                "ref": "develop",
                "sha": "50949b2d0af3933e88fb6f8623926edab7a39ee1",
                "repo": {
                    "id": 221919379,
                    "url": "https://api.github.com/repos/aws-powertools/powertools-lambda-python",
                    "name": "powertools-lambda-python"
                }
            },
            "base": {
                "ref": "develop",
                "sha": "6e37fcefdf239ea15d955c42d29209dcfb25eac7",
                "repo": {
                    "id": 410796572,
                    "url": "https://api.github.com/repos/pecigonzalo/aws-lambda-powertools-python",
                    "name": "aws-lambda-powertools-python"
                }
            }
        }
    ]
}
spencerschrock commented 1 year ago

Did some initial digging and the head commit associated to the PR (ec9c45148a4228ddfb4717259246881c35d7261e)

Note: ec9c45148a4228ddfb4717259246881c35d7261e is the merge commit, not the head commit of the PR: which was d92fe09efddbffd18d8cc403681bc76ab58c3b18

When I re-run the api call with that commit sha (which is what scorecard currently does), the Analyze job isn't part of it:

We could likely add something that checked the merge commit too, but the difference in slug name would also need to be investigated:

Expected slug: github-code-scanning