dependency-review-action icon indicating copy to clipboard operation
dependency-review-action copied to clipboard

[BUG] Action fails in merge queue with v4.3.5

Open kylebjordahl opened this issue 1 year ago • 2 comments

Describe the bug Action fails in merge queue with unhelpful error (looks like a zod or schema validation?)

Run actions/dependency-review-action@v4
Error: [
  {
    "code": "invalid_type",
    "expected": "object",
    "received": "undefined",
    "path": [],
    "message": "Required"
  }
]

Showed up within minutes of the release of 4.3.5 (which we got due to a floating version pin of v4). Pinning to 4.3.4 seems to resolve the issue

To Reproduce Unfortunately this is in a private repo, so cannot share a full repro. We use a GitHub merge queue, but have had no issues before the release of 4.3.5

Expected behavior Action should run.

Action version Showed up within minutes of the release of 4.3.5 (which we got due to a floating version pin of v4). Pinning to 4.3.4 seems to resolve the issue

Screenshots Screenshot 2024-10-21 at 4 33 24 PM

kylebjordahl avatar Oct 21 '24 23:10 kylebjordahl

HI @kylebjordahl, thank you for reporting this issue!

I'm not able to reproduce the problem with the action in merge queue using default configurations. Can you share the configs that you are using for the action and/or the configuration file, if you're using any.

Ahmed3lmallah avatar Oct 22 '24 21:10 Ahmed3lmallah

Hi @Ahmed3lmallah - here is our action config yaml (pretty vanilla)

name: 'Dependency review'
on:
  pull_request:
  merge_group:
    types: ['checks_requested']

# If using a dependency submission action in this workflow this permission will need to be set to:
#
# permissions:
#   contents: write
#
# https://docs.github.com/en/enterprise-cloud@latest/code-security/supply-chain-security/understanding-your-software-supply-chain/using-the-dependency-submission-api
permissions:
  contents: read
  # Write permissions for pull-requests are required for using the `comment-summary-in-pr` option, comment out if you aren't using this option
  pull-requests: write

jobs:
  dependency-review:
    name: Dependency Review
    runs-on: ubuntu-latest
    steps:
      - name: 'Checkout repository'
        uses: actions/checkout@v4

      - name: 'Dependency Review'
        # RN-3003: Pinned due to issue with 4.3.5, can probably revert to v4 once a fix is out
        uses: actions/[email protected]
        # Commonly enabled options, see https://github.com/actions/dependency-review-action#configuration-options for all available options.
        with:
          comment-summary-in-pr: always
          fail-on-severity: moderate
          base-ref: ${{ github.event.pull_request.base.ref || github.event.merge_group.base_ref || 'main'}}
          head-ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref || github.ref}}
          # TODO: review these in mid-sept 2024, see RN-1974 for details
          allow-ghsas: ## REDACTED, THERE ARE TWO
        #   deny-licenses: GPL-1.0-or-later, LGPL-2.0-or-later
        #   retry-on-snapshot-warnings: true

kylebjordahl avatar Oct 23 '24 23:10 kylebjordahl

Here's my analysis of the issue:

In git-refs.ts, line 17 fails because there's no pull_request field in the event. That passes undefined into the schema parser, which fails to validate the input as a an object.

The code likely needs to do the following:

  • Move if (context.eventName == 'merge_group') to a separate else if block
  • Add a new MergeGroupSchema
  • The merge_group field needs to be pulled out of the context. Right now this is done for pull_request in the function declaration, but this starts to get a bit weird if these fields are not always present. May need some refactoring.
  •  base_ref = base_ref | merge_group.base_sha
     head_ref = head_ref | merge_group.head_sha
    

The short version is that merge_group and pull_request have completely different event structures, so some additional logic was needed beyond allowing the event type. 🙂

ebickle avatar Oct 27 '24 02:10 ebickle

This bug should be fixed now in the latest release: v4.4.0

Thank you @kylebjordahl for reporting the bug and @ebickle for your analysis of the issue!

Ahmed3lmallah avatar Oct 28 '24 18:10 Ahmed3lmallah