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

upload-sarif action seems to always use GITHUB_WORKSPACE Git information to compute commit_oid #952

Closed
daniel-beck opened this issue Feb 26, 2022 · 7 comments · Fixed by #956
Assignees

Comments

@daniel-beck
Copy link
Contributor

Uploading SARIF files using the upload-sarif@v1 action can fail with:

Processing sarif files: ["scan.sarif"]
Uploading results
Error: commit not found
RequestError [HttpError]: commit not found

The problem here seems to be that the commit_oid parameter in the uploaded JSON is from an entirely different repository: the repository that's the "main" checkout (the one with empty path parameter to actions/checkout@v2), even with checkout_path specified.

Full log output
 Processing sarif files: ["jenkins-security-scan.sarif"]
 Uploading results
 Error: commit not found
 RequestError [HttpError]: commit not found
     at /home/runner/work/_actions/github/codeql-action/v1/node_modules/@octokit/request/dist-node/index.js:66:23
     at processTicksAndRejections (internal/process/task_queues.js:93:5)
     at async Job.doExecute (/home/runner/work/_actions/github/codeql-action/v1/node_modules/bottleneck/light.js:405:18) {
   name: 'HttpError',
   status: 404,
   headers: {
     'access-control-allow-origin': '*',
     'access-control-expose-headers': 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset',
     connection: 'close',
     'content-encoding': 'gzip',
     'content-security-policy': "default-src 'none'",
     'content-type': 'application/json; charset=utf-8',
     date: 'Fri, 25 Feb 2022 10:29:15 GMT',
     'referrer-policy': 'origin-when-cross-origin, strict-origin-when-cross-origin',
     server: 'GitHub.com',
     'strict-transport-security': 'max-age=31536000; includeSubdomains; preload',
     'transfer-encoding': 'chunked',
     vary: 'Accept-Encoding, Accept, X-Requested-With',
     'x-content-type-options': 'nosniff',
     'x-frame-options': 'deny',
     'x-github-media-type': 'github.v3; format=json',
     'x-github-request-id': '0403:3D83:83138D:1507C61:6218AF7B',
     'x-ratelimit-limit': '1000',
     'x-ratelimit-remaining': '995',
     'x-ratelimit-reset': '1645785350',
     'x-ratelimit-resource': 'code_scanning_upload',
     'x-ratelimit-used': '5',
     'x-xss-protection': '0'
   },
   request: {
     method: 'PUT',
     url: 'https://api.github.com/repos/daniel-beck-org/sample-plugin/code-scanning/analysis',
     headers: {
       accept: 'application/vnd.github.v3+json',
       'user-agent': 'CodeQL-Action/1.1.3 octokit-core.js/3.1.2 Node.js/12.13.1 (linux; x64)',
       authorization: 'token [REDACTED]',
       'content-type': 'application/json; charset=utf-8'
     },
     body: '{"commit_oid":"92e0b0945a6334eeeb4c65a78c2be5a7767e3cc9","ref":"refs/heads/main","analysis_key":".github/workflows/jss.yaml:scan","analysis_name":"Jenkins Security Scan","sarif":"...","workflow_run_id":1898119060,"checkout_uri":"file:///home/runner/work/sample-plugin/sample-plugin","environment":"null","started_at":"2022-02-25T10:29:14.509Z","tool_names":["Jenkins Security Scan"]}',
     request: { agent: [Agent], hook: [Function: bound bound register] }
   },
   documentation_url: 'https://docs.github.com/rest'
 }

If no repository is checked out at this location (i.e. every actions/checkout@v2 has a path), then the error is the same as in #944 (but at least the upload still happens, at least for a non-PR upload).

@aibaars
Copy link
Collaborator

aibaars commented Feb 26, 2022

Have you tried setting the ref: and sha: properties?

ref:
description: "The ref where results will be uploaded. If not provided, the Action will use the GITHUB_REF environment variable. If provided, the sha input must be provided as well. This input is not available in pull requests from forks."
required: false
sha:
description: "The sha of the HEAD of the ref where results will be uploaded. If not provided, the Action will use the GITHUB_SHA environment variable. If provided, the ref input must be provided as well. This input is not available in pull requests from forks."
required: false

@daniel-beck
Copy link
Contributor Author

daniel-beck commented Feb 26, 2022

Have you tried setting the ref: and sha: properties?

I have. Made no difference.

Log excerpt
Run github/codeql-action/upload-sarif@v1
  with:
    sarif_file: jenkins-security-scan.sarif
    category: Jenkins Security Scan
    ref: refs/heads/main
    sha: 14e4c7a1ad0fc1cb6f428f70cf22d9d7d15306ca
    checkout_path: /home/runner/work/sample-plugin/sample-plugin
    token: ***
    matrix: null
    wait-for-processing: false
  env:
    JAVA_HOME: /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.14-101/x64
Uploading results
  Processing sarif files: ["jenkins-security-scan.sarif"]
  Uploading results
  Error: commit not found
  RequestError [HttpError]: commit not found
      at /home/runner/work/_actions/github/codeql-action/v1/node_modules/@octokit/request/dist-node/index.js:66:23
      at processTicksAndRejections (internal/process/task_queues.js:93:5)
      at async Job.doExecute (/home/runner/work/_actions/github/codeql-action/v1/node_modules/bottleneck/light.js:405:18) {
    name: 'HttpError',
    status: 404,
    headers: {
      (irrelevant)
    },
    request: {
      method: 'PUT',
      url: 'https://api.github.com/repos/daniel-beck-org/sample-plugin/code-scanning/analysis',
      headers: {
        accept: 'application/vnd.github.v3+json',
        'user-agent': 'CodeQL-Action/1.1.3 octokit-core.js/3.1.2 Node.js/12.13.1 (linux; x64)',
        authorization: 'token [REDACTED]',
        'content-type': 'application/json; charset=utf-8'
      },
      body: '{"commit_oid":"b029bbc98973e672e9d717a449efa77c468d2a85","ref":"refs/heads/main","analysis_key":".github/workflows/jss.yaml:scan","analysis_name":"Jenkins Security Scan","sarif":"...","workflow_run_id":1898079034,"checkout_uri":"file:///home/runner/work/sample-plugin/sample-plugin","environment":"null","started_at":"2022-02-25T10:20:21.497Z","tool_names":["Jenkins Security Scan"]}',

Also, since these are not working for PRs from forks that wouldn't be a solution, as that's the only reason I'm using this action.

@daniel-beck
Copy link
Contributor Author

but at least the upload still happens, at least for a non-PR upload

I only did fairly superficial checks before trying to work around the problem, but IIUC, PRs also fail to upload results, since the PR merge ref cannot be checked out.

@aeisenberg
Copy link
Contributor

aeisenberg commented Feb 28, 2022

If I understand correctly, the workflow is trying to upload some SARIF to a different repository than the one currently runing the workflow?

The codeql-action was not designed to operate in this manner. If this is what you are trying to do, you will need to avoid using the codeql-action at all and use the codeql CLI directly to perform the upload. essentially, you will need to treat the upload as coming from a third party CI system.

There is a command, codeql github upload-results that you can invoke to do the upload.

It will be a bit more finicky since you will need to specify the correct SHA, head, and base refs, and repository. But I think it should work.

Also, you will need to make sure that the PAT used to upload will have enough privileges to upload security results to the appropriate repository. In order to do this, you will need to create a new repository secret for a PAT you have generated that will have upload access to this repo.

Let me know if you have any difficulties with this and I can go into more detail.

@daniel-beck
Copy link
Contributor Author

daniel-beck commented Feb 28, 2022

If I understand correctly, the workflow is trying to upload some SARIF to a different repository than the one currently runing the workflow?

No, it's the correct destination repository, but it specifies a wrong SHA for the commit_oid parameter of the undocumented API it uploads to, resulting in the server rejecting the upload (AFAICT, undocumented!).

Basically, while a checkout_path parameter exists, that is ignored when determining various parameters of the /code-scanning/analysis API. This is non-fatal in cases like #944 (when there is no checkout to the top-level directory), but if there is a wrong repo there, or we're building a PR, then the git commands will fail (for PR) or produce an unexpected result (for non-PR) and the upload aborts.

Since my use case includes scan results in PRs from forks in a workflow, this action is the only (documented and safe) solution. I don't know whether I'd violate the ToS of GitHub by just calling this API directly, so I've since completely restructured my workflow to make it work by checking out the component being scanned without a path argument to actions/checkout@v2.

IMO this behavior is a bug: It is both unexpected (existence of checkout_path indicates support for checkouts into subdirs) and undocumented.

@aeisenberg
Copy link
Contributor

OK. Thanks for explaining. That does sound like a bug. The checkout_path is not being used to determine the commit_oid. It shouldn't be difficult to address this.

That being said, my previous comment would still be a viable workaround until there is a fix. You can still call the codeql github upload-results directly.

@daniel-beck
Copy link
Contributor Author

daniel-beck commented Feb 28, 2022

You can still call the codeql github upload-results directly.

I'm not that familiar with the GitHub actions security model, but AFAIUI even PR authors can edit workflows and therefore access any secrets made available to them, which is why GITHUB_TOKEN has fewer permissions when run from PRs. The GitHub CLI docs specifically point out that a token with security_events: write permission is needed.

If I wanted that I could have had it with my curl-based approach I used originally, but that seemed unsafe. Apparently /code-scanning/analysis doesn't need this permission, but it's undocumented and I didn't want to risk a ToS violation -- so here I am, with no real options other than this action 😉

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 a pull request may close this issue.

3 participants