advanced-security / policy-as-code

GitHub Advanced Security Policy as Code
MIT License
71 stars 14 forks source link

licensing - total dependency graph 0 on pull-request workflow #54

Open eidottermihi opened 1 year ago

eidottermihi commented 1 year ago

Describe the bug

When the policy-is-code Action is run on a pull-request (scanning the non-default branch), License checking does not work because the dependency graph includes 0 dependencies.

When the action is run on the default branch, License checking works.

To Reproduce

Workflow

jobs:
  compliance:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v3
      - name: Advance Security Policy as Code
        uses: advanced-security/policy-as-code@v2.4.0
        with:
          policy-path: .github/policies/custom.yaml
          token: ${{ secrets.GITHUB_TOKEN }}
          argvs: "--disable-dependabot --disable-secret-scanning --disable-code-scanning --display"

Policy:

name: custom

# Dependency Licensing Alerts
licensing:
  conditions:
    ids:
      - GPL-*
      - LGPL-*
      - AGPL-*

  warnings:
    # Warning is the dependency isn't known
    ids:
      - Other
      - NA

Open a PR and check the output of the action.

Expected behavior

Dependency Graph should be calculated with >0 dependencies.

Screenshots

action on PR (see https://github.com/it-at-m/appswitcher-vue/actions/runs/5711495958/job/15473216682)

grafik

action on main branch (see https://github.com/it-at-m/appswitcher-vue/actions/runs/5711336691/job/15472815170)

grafik

Additional context

I noticed Dependencies from Pull Request in the action log - does this mean the scan on a PR detects "changed dependencies" and therefore just scans these dependencies?

GeekMasher commented 1 year ago

@eidottermihi thank you for raising this issue, can you please confirm you have enabled GitHub Advanced Security on this repository (api used)? I just checked and if there is an error getting the diff list, we just return a blank list of dependencies.

I'm about to add better logging to this so an end user can see the issues.

eidottermihi commented 1 year ago

@GeekMasher thanks for looking into this issue!

can you please confirm you have enabled GitHub Advanced Security on this repository (api used)?

I did not find a specific setting for "GitHub Advanced Security" in repo settings, I suspect "Dependency graph" is needed? This is enabled.

grafik

Maybe this is a problem on pull request workflows because the GITHUB_TOKEN maybe has no sufficient rights? However some error handling or logging would be nice, as you mentioned ;)

GeekMasher commented 1 year ago

@eidottermihi can you try and update the permissions to include security-events?

permissions:
  contents: read
  security-events: read

https://github.com/it-at-m/appswitcher-vue/actions/runs/5711495958/workflow#L13

Getting the dependencies from the API for Pull Requests requires access to security-events if I'm not mistaken. I will be requesting a docs update to make this clear.

There will be 2 things I'll work on to make this better:

  1. Better logging will be added soon - https://github.com/GeekMasher/ghastoolkit/pull/95
  2. Better policy-as-code docs and example updates
GeekMasher commented 1 year ago

I have created this discussion for now to help with this https://github.com/advanced-security/policy-as-code/discussions/56

eidottermihi commented 1 year ago

@GeekMasher I added the permission (see https://github.com/it-at-m/appswitcher-vue/pull/74/files ) - the Action then reported 1 "Total dependencies in graph" -> this was advanced-security/policy-as-code, which was also updated in this PR (2.4.0 -> 2.4.1)

So still looks to me as only "changed" dependencies are analyzed? This would be fine IMHO.

image

https://github.com/it-at-m/appswitcher-vue/actions/runs/5718094968/job/15493206133

GeekMasher commented 1 year ago

@eidottermihi You are correct, the policy-as-code looks at the diff in the PR versus all the dependencies in a PR.

https://github.com/GeekMasher/ghastoolkit/blob/main/src/ghastoolkit/octokit/dependencygraph.py#L120-L121

Glad to see it's working now, this will show warnings next time when the feature / permissions are missing.