FrankChen021 commented on code in PR #20126:
URL: https://github.com/apache/druid/pull/20126#discussion_r4036866005


##########
.github/workflows/cron-job-its.yml:
##########
@@ -45,20 +50,76 @@ jobs:
           distribution: 'zulu'
           cache: maven
 
+      - name: Restore Dependency-Check data
+        id: dependency-check-cache
+        uses: actions/cache/restore@v4
+        with:
+          path: ~/.cache/dependency-check
+          key: dependency-check-data-${{ runner.os }}-odc-${{ 
env.DEPENDENCY_CHECK_CACHE_VERSION }}-${{ github.run_id }}
+          restore-keys: |
+            dependency-check-data-${{ runner.os }}-odc-${{ 
env.DEPENDENCY_CHECK_CACHE_VERSION }}-
+
+      - name: Report Dependency-Check cache status
+        run: |
+          echo "Dependency-Check cache-hit=${{ 
steps.dependency-check-cache.outputs.cache-hit }}"
+          echo "Dependency-Check cache-matched-key=${{ 
steps.dependency-check-cache.outputs.cache-matched-key }}"
+
       - name: maven build # needed to rebuild in case of maven snapshot 
resolution fails
         run: mvn clean install -P dist -P skip-static-checks,skip-tests 
-Dmaven.javadoc.skip=true -Dcyclonedx.skip=true -Dweb.console.skip=true
 
+      - name: Update Dependency-Check data
+        id: dependency-check-update
+        env:
+          NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
+        run: |
+          if [ -n "$NVD_API_KEY" ]; then
+            echo "NVD API key is available to the update"
+          else
+            echo "NVD API key is unavailable to the update"
+          fi
+          mvn -B dependency-check:update-only \
+            -DdataDirectory="$HOME/.cache/dependency-check" \
+            -DnvdValidForHours=24
+
       - name: security vulnerabilities check
+        if: steps.dependency-check-update.outcome == 'success'
         env:
           OSS_INDEX_USERNAME: ${{ secrets.OSS_INDEX_USERNAME }}
           OSS_INDEX_PASSWORD: ${{ secrets.OSS_INDEX_PASSWORD }}
           NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
         run: |
-          mvn -B dependency-check:purge dependency-check:check 
-DnvdApiKey=$NVD_API_KEY -DossIndexUsername=$OSS_INDEX_USERNAME 
-DossIndexPassword=$OSS_INDEX_PASSWORD || { echo "
-          The OWASP dependency check has found security vulnerabilities. 
Please use a newer version
-          of the dependency that does not have vulnerabilities. To see a 
report run
-          `mvn dependency-check:check`
-          If the analysis has false positives,
-          they can be suppressed by adding entries to 
owasp-dependency-check-suppressions.xml (for more
-          information, see 
https://jeremylong.github.io/DependencyCheck/general/suppression.html).
-          " && false; }
+          if [ -n "$NVD_API_KEY" ]; then
+            echo "NVD API key is available to the scan"
+          else
+            echo "NVD API key is unavailable to the scan"
+          fi
+          # --fail-at-end skips modules that depend on a failed module. 
Continue through the
+          # entire reactor, then aggregate the module results so CI still 
fails when findings exist.
+          dependency_check_log="$RUNNER_TEMP/dependency-check.log"
+          set +e
+          mvn -B --fail-never dependency-check:check -DautoUpdate=false 
-DdataDirectory="$HOME/.cache/dependency-check" 
-DossIndexUsername="$OSS_INDEX_USERNAME" 
-DossIndexPassword="$OSS_INDEX_PASSWORD" 2>&1 | tee "$dependency_check_log"
+          pipeline_status=("${PIPESTATUS[@]}")
+          set -e
+          maven_status=${pipeline_status[0]}
+          tee_status=${pipeline_status[1]}
+          if [ "$maven_status" -ne 0 ] || [ "$tee_status" -ne 0 ] \
+             || grep -Eq '^\[INFO\] BUILD FAILURE$|^\[ERROR\] Failed to 
execute goal org\.owasp:dependency-check-maven:' "$dependency_check_log"; then
+            echo "
+            The OWASP dependency check has found security vulnerabilities or 
failed in one or more modules. Please use a newer version
+            of the dependency that does not have vulnerabilities. To see a 
report run
+            mvn dependency-check:check
+            If the analysis has false positives,
+            they can be suppressed by adding entries to 
owasp-dependency-check-suppressions.xml (for more
+            information, see 
https://jeremylong.github.io/DependencyCheck/general/suppression.html).
+            "
+            exit 1
+          fi
+
+      - name: Save Dependency-Check data
+        # A pull_request run uses an isolated merge-ref cache that cannot be
+        # reused by branch, scheduled, or workflow_dispatch runs.
+        if: steps.dependency-check-update.outcome == 'success' && 
github.event_name != 'pull_request'

Review Comment:
   [P1] Persist the database after scan failures
   
   **Finding:** When Dependency-Check finds a vulnerability, the preceding 
security-vulnerabilities step exits nonzero. Because this cache-save step has 
no always() (or equivalent) condition, GitHub Actions applies its implicit 
success() guard and skips the save even though the update step succeeded. The 
refreshed NVD database is then discarded, so the next scheduled or manual run 
restores an older cache and repeats a full update.
   
   **Suggestion:** Make the save step run whenever the update succeeded and the 
job was not cancelled, including when the vulnerability scan fails.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to