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

PMD sarif report preview unavailable #4522

Open
xezzon opened this issue Jan 18, 2025 · 11 comments · May be fixed by #4571
Open

PMD sarif report preview unavailable #4522

xezzon opened this issue Jan 18, 2025 · 11 comments · May be fixed by #4571
Labels
bug Something isn't working

Comments

@xezzon
Copy link

xezzon commented Jan 18, 2025

Describe the bug

  • MegaLinter's PMD sarif report cannot be previewed.
  • The title is not consistent with the content.

Image

To Reproduce

Here is my configuration:

# .github/workflows/linter.yml

name: Lint

permissions: read-all

on:
  push:
    branches: [ "develop", "master", "main" ]
  pull_request:
    # The branches below must be a subset of the branches above
    branches: [ "develop", "master", "main" ]

jobs:
  linter:
    runs-on: ubuntu-24.04
    permissions:
      contents: read
      issues: write
      pull-requests: write
      security-events: write
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - name: Set Environment
        if: github.base_ref != 'refs/heads/main'
        run: |
          echo "REPOSITORY_TRIVY_DISABLE_ERRORS=true" >> $GITHUB_ENV
          echo "VALIDATE_ALL_CODEBASE=false" >> $GITHUB_ENV
      - name: MegaLinter
        uses: oxsecurity/megalinter@v8
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          MEGALINTER_CONFIG: .github/.mega-linter.yml
      - name: Upload MegaLinter scan results to GitHub Security tab
        if: success() || failure()
        uses: github/codeql-action/upload-sarif@v3
        with:
          sarif_file: 'megalinter-reports/megalinter-report.sarif'
# .github/mega-linter.yml

ENABLE:
  - JAVA
  - PROTOBUF
  - EDITORCONFIG
  - COPYPASTE
  - REPOSITORY
ENABLE_LINTERS:
  - SQL_SQLFLUFF
  - SPELL_CSPELL
DISABLE_LINTERS:
  - REPOSITORY_KICS
  - REPOSITORY_GRYPE
  - REPOSITORY_DEVSKIM
DISABLE_ERRORS_LINTERS:
  - SPELL_CSPELL
  - COPYPASTE_JSCPD
  - REPOSITORY_TRIVY
  - JAVA_PMD
  - JAVA_CHECKSTYLE
  - REPOSITORY_CHECKOV

SARIF_REPORTER: true
SARIF_REPORTER_NORMALIZE_LINTERS_OUTPUT: false

PRE_COMMANDS:
  - command: curl https://raw.githubusercontent.com/checkstyle/checkstyle/refs/heads/master/src/main/resources/google_checks.xml > google_checks.xml

LINTER_RULES_PATH: .github/linters
JAVA_CHECKSTYLE_FILE_NAME: google_checks.xml
SPELL_CSPELL_FILE_EXTENSIONS:
  - .java
  - .proto

Expected behavior

Complies with pmd/pmd-github-action behaviour.

Image

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@xezzon xezzon added the bug Something isn't working label Jan 18, 2025
@nvuillam
Copy link
Member

Please can you show the full path of the following file ?

Image

(you can also find it in SARIF)

MegaLinter does a little bit of post-processing of generated SARIF, but not that much, so I need that info to investigate ^^

@xezzon
Copy link
Author

xezzon commented Jan 18, 2025

Thank you for your reply.

The full path of the file is file://zeroweb-service/zeroweb-service-admin/src/test/java/io/github/xezzon/zeroweb/locale/LocalizedHttpTest.java:60.

And here are the sarif files. It looks like this is because MegaLinter's sarif report has the extra file://. pmd-github-action outputs a sarif that doesn't include it. One more thing, MegaLinter's checkstyle sarif report does not include file:// and its results are normal.

megalinter-report.zip
pmd-report.zip

@nvuillam
Copy link
Member

@xezzon Ok, so basically if we manage to remove the file:// , it should be ok ? :)

@echoix
Copy link
Collaborator

echoix commented Jan 18, 2025

@xezzon Ok, so basically if we manage to remove the file:// , it should be ok ? :)

Is it PMD that adds it? Is it supposed to be there in the sarif spec?

@xezzon
Copy link
Author

xezzon commented Jan 18, 2025

I don't think PMD added it. You can tell this from the sarif report that was unzip from the attachment pmd-report.zip that I uploaded.

@echoix
Copy link
Collaborator

echoix commented Jan 18, 2025

Before going too deep, did you try the beta version of megalinter? I know PMD has had a couple updates since

@nvuillam
Copy link
Member

@janderssonse I see you made the latest updates about SARIF post-processing, do you have some suggestions ? :)

@xezzon
Copy link
Author

xezzon commented Jan 19, 2025

Sorry, I seem to have got one thing wrong. The sarif report for pmd-github-action does not contain file://, but the sarif report for MegaLinter's PMD does. I think it's because pmd-github-action handles this, which I confirmed in its source code.

@echoix
Copy link
Collaborator

echoix commented Jan 19, 2025

But technically there's a way that you can specify (in sarif), the root to use for all the paths in the run (for a single tool).

@nvuillam
Copy link
Member

I'm not a sarif expert, if it's better to remove file:// everywhere that's not complicated to do with a regex in post processing :)

@xezzon
Copy link
Author

xezzon commented Jan 20, 2025

The SARIF specification does not forbid file://. But GitHub recommends converting absolute paths to relative paths.

@xezzon xezzon linked a pull request Jan 23, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants