crytic / slither-action

GNU Affero General Public License v3.0
128 stars 20 forks source link

foundry compilation fails being unable to resolve dependencies #75

Closed eugenPtr closed 7 months ago

eugenPtr commented 8 months ago

Hi, I need a bit of support figuring out the following issue:

The slither-action task complains about a file in the compilation phase. I confirm that the file exists in the CI env - it is generated when the LayerZero-v2 is built in a previous step of the workflow.

Note that the job runs successfully with act in my local env.

More details about the error in the image below.

image

Here is my remapping: "@layerzerolabs/lz-evm-protocol-v2/=lib/LayerZero-v2/oapp/node_modules/@layerzerolabs/lz-evm-protocol-v2/"

Here is my workflow file:

name: Run Slither Analysis

on:
  pull_request:
    branches: [main]

jobs:
  run-slither:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v3
        with:
          submodules: recursive

      - name: Install Foundry
        uses: foundry-rs/foundry-toolchain@v1
        with:
          version: nightly

      - name: Set up Node.js v20
        uses: actions/setup-node@v3
        with:
          node-version: '20.x'

      - name: Set up Yarn
        run: |
          corepack enable

      - name: Build LayerZero V2
        run: |
          cd lib/LayerZero-v2
          yarn install && yarn build

      # This is able to find AddressCast.sol at the location
      - run: |
          ls 
          ls lib/LayerZero-v2/oapp/node_modules/@layerzerolabs/lz-evm-protocol-v2/contracts/libs

      - name: Run Slither
        uses: crytic/slither-action@v0.3.1
        id: slither
        with:
          fail-on: none
          slither-args: --exclude-dependencies --exclude-optimization --exclude-informational --exclude-low --checklist --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/

      - name: Create/update checklist as PR comment
        uses: actions/github-script@v6
        env:
          REPORT: ${{ steps.slither.outputs.stdout }}
        with:
          github-token: ${{ secrets.GITHUB_TOKEN }}
          script: |
            const script = require('.github/scripts/comment')
            const header = '# Slither report'
            const body = process.env.REPORT
            await script({ github, context, header, body })

I also tried building the project in the preceding step and using the ignore-compile: true option. That didn't work either and outputted the part starting from line 251 in the above image.

Any hints on what might be the issue here?

elopez commented 8 months ago

Hi @eugenPtr! I'm thinking this could be Yarn generating symlinks and causing the issue. The Slither action is a container action and as such, it runs within a Docker container that only has the workspace mapped into /github/workspace. If your yarn build && yarn install step produces some sort of absolute symlinks, those may not be resolvable inside the action container.

We do attempt to create a link that fixes most of these issues (you should see Applied compatibility link:... on your action log) but maybe it's not enough in this case.

Could you run the following right after yarn install && yarn build and see if that's what might be causing trouble? It should print any symlinks present in that directory.

find -type l -exec ls -la {} \;
eugenPtr commented 8 months ago

Hi @elopez! Thank you for being prompt as always!

The command printed a bunch of results covering all the remappings in my repo. Here is the one related to the error

lrwxrwxrwx 1 runner docker 111 Mar 6 10:00 ./oapp/node_modules/@layerzerolabs/lz-evm-protocol-v2 -> /home/runner/work/my-oapp/my-oapp/lib/LayerZero-v2/protocol

What do you make of this? Is the issue that ./oapp/node_modules/.... is not mapped to /github/workspace? If that so what could be a possible fix for this?

elopez commented 8 months ago

I think the issue could happen if /home/runner/work/my-oapp/my-oapp/lib/LayerZero-v2/protocol is not resolvable inside the container, as that's an absolute link. The action has some code to create a link like /home/runner/work/my-oapp/my-oapp -> /github/workspace to alleviate some of these issues.

Can you confirm if you see [-] Applied compatibility link: /home/runner/work/my-oapp/my-oapp -> /github/workspace on your action log? Looking at it now, I think we only create the link if you use ignore-compile: true, but as you're doing something hybrid maybe that's not being applied.

If you don't see it, can you try your workflow with the patched action version in the dev-always-compat-link branch? It should be enough to specify uses: crytic/slither-action@dev-always-compat-link on your workflow to try it out.

eugenPtr commented 8 months ago

Could not see the applied compatibility link part in the logs, but I do see it when using dev-always-compat-link and it did the trick! Seems like this was indeed the issue. Thank you so much for your support @elopez!

Regarding ignore-compile:true, I modified my workflow to use this option and do the compilation separately in a different step, as you can see below:

- name: Build LayerZero V2
  run: |
    cd lib/LayerZero-v2
    yarn install && yarn build

- name: Run Forge build
  run: |
    forge --version
    forge build --sizes
  id: build

- name: Run Slither
  uses: crytic/slither-action@v0.3.1
  id: slither
  with:
    ignore-compile: true
    fail-on: none
    slither-args: --exclude-dependencies --exclude-optimization --exclude-informational --exclude-low --checklist --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/

This time it applied the compatibility link but the job still failed with this message:

image

Any clue what the cause of this might be?

Also, would it be possible for you to include the patch in dev-always-compat-link in a future release?

elopez commented 8 months ago

Could not see the applied compatibility link part in the logs, but I do see it when using dev-always-compat-link and it did the trick! Seems like this was indeed the issue. Thank you so much for your support @elopez!

Great! 🎉

Regarding ignore-compile:true, I modified my workflow to use this option and do the compilation separately in a different step, as you can see below:

- name: Run Forge build
  run: |
    forge --version
    forge build --sizes
  id: build

This time it applied the compatibility link but the job still failed with this message: (snip)

Any clue what the cause of this might be?

Slither invokes forge build with the --build-info flag, it's possible you just need to add that to your command.

Also, would it be possible for you to include the patch in dev-always-compat-link in a future release?

Sure, I'll open a pull request to track it 👍