Skip to content

Comments

Fix ZAP baseline scan workflow failure#3186

Closed
anurag2787 wants to merge 2 commits intoOWASP:mainfrom
anurag2787:fix/zap-baseline-scan-issue
Closed

Fix ZAP baseline scan workflow failure#3186
anurag2787 wants to merge 2 commits intoOWASP:mainfrom
anurag2787:fix/zap-baseline-scan-issue

Conversation

@anurag2787
Copy link
Contributor

Proposed change

Fixes the ZAP baseline scan workflow so the generated report is uploaded correctly and the ci job completes successfully

Resolves #3183

Checklist

  • Required: I read and followed the contributing guidelines
  • Required: I ran make check-test locally and all tests passed
  • I used AI for code, documentation, or tests in this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 5, 2026

Summary by CodeRabbit

  • Chores
    • Simplified security scan steps by removing extra command options.
    • Standardized uploaded report handling and renamed the CI report output to a single consistent filename.
    • Updated CI pipeline scan steps for both staging and production to reflect these changes.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Removed cmd_options: '-a -r zap-report.html' and changed the uploaded ZAP report path from zap-report.html to report_html.html in both Run ZAP Baseline Scan blocks (staging and production) of the CI workflow file.

Changes

Cohort / File(s) Summary
CI/CD workflow (ZAP scan edits)
.github/workflows/run-ci-cd.yaml
In both Run ZAP Baseline Scan blocks (staging & production): removed cmd_options: '-a -r zap-report.html'; updated Upload ZAP report step to use report_html.html instead of zap-report.html.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
  • kasya

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing the ZAP baseline scan workflow failure in the CI/CD pipeline.
Description check ✅ Passed The description is related to the changeset, explaining that the fix resolves ZAP baseline scan workflow issues and references the linked issue #3183.
Linked Issues check ✅ Passed The PR changes directly address issue #3183 by fixing the ZAP baseline scan workflow to upload the generated report correctly and resolve the failing CI job.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the ZAP baseline scan workflow as specified in issue #3183; no unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47cd161 and 15dfcab.

📒 Files selected for processing (1)
  • .github/workflows/run-ci-cd.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/run-ci-cd.yaml

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the ci label Jan 5, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/run-ci-cd.yaml (1)

952-967: Consider standardizing ZAP configuration across environments.

The production ZAP baseline scan (lines 952-959) explicitly sets cmd_options: '-a -r zap-report.html' to generate a custom report filename, and the upload path at line 966 matches this filename. However, the staging configuration (lines 604-626) uses the default behavior without cmd_options, relying on the default output filename report_html.html.

For maintainability and consistency, consider:

  1. Removing cmd_options from the production scan (line 959) and updating the upload path to report_html.html (line 966) to match the staging approach, OR
  2. Adding explicit cmd_options to the staging scan to match the production approach

The current inconsistency between environments could lead to confusion or future issues.

🔎 Option 1: Standardize to default behavior (recommended for simplicity)
       - name: Run ZAP Baseline Scan
         uses: zaproxy/action-baseline@de8ad967d3548d44ef623df22cf95c3b0baf8b25
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           target: 'https://nest.owasp.org'
           allow_issue_writing: false
           fail_action: false
-          cmd_options: '-a -r zap-report.html'

       - name: Upload ZAP report
         if: always()
         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f
         with:
           name: zap-baseline-scan-report-${{ github.run_id }}
-          path: zap-report.html
+          path: report_html.html
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 572877b and 47cd161.

📒 Files selected for processing (1)
  • .github/workflows/run-ci-cd.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-21T19:03:59.068Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1139
File: .github/workflows/setup-backend-environment/action.yaml:16-27
Timestamp: 2025-12-21T19:03:59.068Z
Learning: Composite actions (runs: using: composite) execute as steps within the calling job's context and can access the job context, including job.services.* properties (e.g., job.services.<service_id>.id, job.services.<service_id>.ports). Service containers must be defined at the job level, but a composite action's steps can reference them via the job context.

Applied to files:

  • .github/workflows/run-ci-cd.yaml
🔇 Additional comments (2)
.github/workflows/run-ci-cd.yaml (2)

267-267: AI summary contains inaccuracies regarding job structure.

The AI summary incorrectly states that the job was renamed from build-staging-images to build-production-images. Both jobs exist as separate entities serving different environments (staging at line 267, production at line 628). Additionally, the AI summary describes several production ZAP scan changes (target URL change, removed cmd_options, changed artifact path), but these changes are not marked in the provided code with ~ markers, suggesting an inconsistency between the summary and actual changes.

Also applies to: 628-628


625-625: The artifact path report_html.html is correct.

The staging ZAP baseline scan (lines 612-618) runs without custom cmd_options, so it uses the default report generation. ZAP baseline action generates report_html.html by default for HTML reports, making the artifact path change correct.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 5, 2026
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 5, 2026

@anurag2787 anurag2787 marked this pull request as ready for review January 5, 2026 02:17
@arkid15r
Copy link
Collaborator

arkid15r commented Jan 5, 2026

Closing in favor of #3198

@arkid15r arkid15r closed this Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix ZAP baseline scan issues

2 participants