Skip to content

Add Build Backend Module CI and OWASP dependency check#522

Merged
dkayiwa merged 4 commits intoopenmrs:masterfrom
wikumChamith:gh
Feb 22, 2026
Merged

Add Build Backend Module CI and OWASP dependency check#522
dkayiwa merged 4 commits intoopenmrs:masterfrom
wikumChamith:gh

Conversation

@wikumChamith
Copy link
Member

  • Add OWASP dependency check workflow and integrate it into the CI pipeline
  • Fix ChromeHeadless sandbox issue in GitHub Actions by switching to a custom ChromeHeadlessNoSandbox launcher
  • Disable Node Audit Analyzer due to incompatible npm-shrinkwrap.json

@@ -0,0 +1,86 @@
name: OWASP Dependency Check

on:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to duplicate this in every module?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. We can simplify this after the PR in the openmrs/openmrs-contrib-gha-workflows gets merged.

name: Build with Maven

on:
  push:
    branches: [ main ]
  pull_request:
    branches: [ main ]
  workflow_dispatch:

jobs:
  build:
    uses: openmrs/openmrs-contrib-gha-workflows/.github/workflows/build-backend-module.yml@main
    with:
      java_versions: '[8, 11, 17, 21]'
      main_java_version: '8'
    secrets:
      NVD_API_KEY: ${{ secrets.NVD_API_KEY }}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkayiwa My goal here was to have a smaller blast radius to look at the effects this will have. My concern is that there will be vulnerable dependencies marked in this module, but which are included as part of openmrs-core and not directly as part of this module. This is is a useful testbed because coreapps literally has no dependencies that it bundles itself, so everything is transitive and should be excluded.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh i see. 😊

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wikumChamith However, can we inline this so that the build can be done on the PR? That way we don't need to merge this in to get a result.

Copy link
Member Author

@wikumChamith wikumChamith Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibacher Since the PR for the reusable OWASP check has been merged, we can use that here instead. We can remove the OWASP build from this module 🙂

openmrs/openmrs-contrib-gha-workflows@ba65e5f

@wikumChamith wikumChamith force-pushed the gh branch 2 times, most recently from f23bffe to 55637f2 Compare February 18, 2026 10:18
- Add OWASP dependency check workflow and integrate it into the CI pipeline
- Fix ChromeHeadless sandbox issue in GitHub Actions by switching to a custom ChromeHeadlessNoSandbox launcher
- Disable Node Audit Analyzer due to incompatible npm-shrinkwrap.json
@wikumChamith
Copy link
Member Author

wikumChamith commented Feb 18, 2026

@ibacher, @dkayiwa to resolve the error we’re currently seeing in the OWASP Dependency Check, we need to pass the --disableNodeAudit flag.

Error: NodeAuditAnalyzer failed on /github/workspace/omod/npm-shrinkwrap.json

To do this, we’ll need to get this PR merged first.

@wikumChamith
Copy link
Member Author

@ibacher, @dkayiwa to resolve the error we’re currently seeing in the OWASP Dependency Check, we need to pass the --disableNodeAudit flag.

Error: NodeAuditAnalyzer failed on /github/workspace/omod/npm-shrinkwrap.json

To do this, we’ll need to get this PR merged first.

Also, we could just disable the OWASP check in build-backend-module (with run_owasp_check) and invoke it directly from here with the required arguments.

I’ll leave it to you two to decide which approach makes more sense 🙂

@ibacher
Copy link
Member

ibacher commented Feb 18, 2026

Oh... I don't really love that as --disableNodeAudit means that we're not actually checking frontend dependencies which would seem like a real issue...

That said, I think the approach in the PR makes the most sense.

@wikumChamith
Copy link
Member Author

Oh... I don't really love that as --disableNodeAudit means that we're not actually checking frontend dependencies which would seem like a real issue...

That said, I think the approach in the PR makes the most sense.

Oh... I don't really love that as --disableNodeAudit means that we're not actually checking frontend dependencies which would seem like a real issue...

That said, I think the approach in the PR makes the most sense.

@ibacher Could this be because NodeAuditAnalyzer doesn’t support lockfileVersion: 1?

I updated it to version 2, but npm-shrinkwrap.json contains very old packages, and the upgrade is running into multiple errors.

This feels like it would be a significant dependency update effort on its own.

@ibacher
Copy link
Member

ibacher commented Feb 18, 2026

This feels like it would be a significant dependency update effort on its own.

Oh, yes, it definitely will be, but there's not a lot of value in a dependency check that skips something just because it's flagging a lot of things.

@wikumChamith
Copy link
Member Author

This feels like it would be a significant dependency update effort on its own.

Oh, yes, it definitely will be, but there's not a lot of value in a dependency check that skips something just because it's flagging a lot of things.

Since the report is still being generated successfully, can we ignore this error for now?

@dkayiwa
Copy link
Member

dkayiwa commented Feb 18, 2026

Since the report is still being generated successfully, can we ignore this error for now?

This is simply because we told it to generate the report even if it failed with errors.

@wikumChamith
Copy link
Member Author

Since the report is still being generated successfully, can we ignore this error for now?

This is simply because we told it to generate the report even if it failed with errors.

@dkayiwa, @ibacher I updated the lockfileVersion, and now the check runs without that error.

@ibacher
Copy link
Member

ibacher commented Feb 20, 2026

Alright... I'm satisfied. The report is just flagging the massively out of date Javascript files, so I guess that's ok.

@dkayiwa dkayiwa merged commit aca7065 into openmrs:master Feb 22, 2026
1 of 2 checks passed
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 this pull request may close these issues.

3 participants