Conversation
WalkthroughUpdates the ZAP automation configuration to add a new internal API OpenAPI job, set the state API target URL and context, and define an exitStatus job. Removes a pre-run file copy step from the manual scan script; other logic remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CI as CI Runner
participant ZAP as ZAP Automation
participant Ctx as Test Context
participant StateAPI as state-api.test.compactconnect.org
participant InternalAPI as api.test.compactconnect.org
participant Exit as exitStatus
CI->>ZAP: Start automation plan
ZAP->>Ctx: Load "test" context (includePaths updated)
note right of Ctx: Changed: state-api URL added
ZAP->>StateAPI: OpenAPI scan (external)<br/>targetUrl set, context:test, user:""
note right of StateAPI: Changed: targetUrl/context/user configured
ZAP->>InternalAPI: OpenAPI scan (internal)<br/>apiFile (internal spec), context:test
note right of InternalAPI: New: internal OpenAPI job
ZAP->>Exit: Evaluate warnExitValue = 0
Exit-->>CI: Return status
sequenceDiagram
autonumber
actor Dev as Developer
participant Script as manual-scan.sh
participant Auth as Token Provider
participant Docker as Docker/ZAP
Dev->>Script: Run manual scan
%% Old step (removed)
rect rgba(255,230,230,0.5)
note over Script: Removed step
Script-x Script: Copy official API spec to ZAP data
end
Script->>Auth: Retrieve token
Auth-->>Script: Token
Script->>Docker: docker run ... (invoke ZAP)
Docker-->>Script: Exit code
Script-->>Dev: Propagate exit
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/zap-scan-test.yml (1)
10-10: LGTM: run ZAP on PRs to development as well.This broadens coverage appropriately. Optional: to save CI minutes, add a paths filter to only trigger when owasp-zap/**, backend specs, or workflow files change.
Example:
on: pull_request: branches: - main - development + paths: + - 'owasp-zap/**' + - 'backend/compact-connect/docs/**' + - '.github/workflows/zap-scan-test.yml'owasp-zap/manual-scan.sh (1)
5-5: Keep the manual scan plan and the copied OAS path consistent (currently the copy is unused).You copy the internal spec to owasp-zap/data/latest-oas30.json, but test-automation.yml imports from /zap/wrk/backend/compact-connect/docs/internal/api-specification/latest-oas30.json. The copy is dead weight unless the plan reads that location.
Option A — remove the copy line from the manual script.
Option B — point the plan to the copied file for manual runs:
- apiFile: /zap/wrk/backend/compact-connect/docs/internal/api-specification/latest-oas30.json + apiFile: /zap/wrk/owasp-zap/data/latest-oas30.jsonAdditional hardening for the script:
- set -e + set -euo pipefailIf you intend to rely on the new exitStatus job during manual runs, ensure the image has the AF add-on version that supports it; otherwise consider zaproxy/zap-weekly.
Would you like me to align the plan and script in this PR (and add pipefail), or do you prefer to keep the copy for local/manual workflows?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
owasp-zap/authenticator/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
.github/workflows/zap-scan-test.yml(1 hunks)owasp-zap/data/test-automation.yml(2 hunks)owasp-zap/manual-scan.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ZapScan
🔇 Additional comments (2)
owasp-zap/data/test-automation.yml (2)
48-53: LGTM: internal OpenAPI job paths/host align with the new internal spec location.apiFile points to /zap/wrk/backend/.../internal/... and targetUrl to https://api.test.compactconnect.org, which is already in scope via includePaths/urls. No blockers here.
115-117: Verify exitStatus support for zaproxy/action-af and zaproxy/zap-stableBefore relying on the
exitStatusjob to enforce your exit-code gating, please confirm both of the following:
- zaproxy/action-af
• You’re currently onzaproxy/action-af@v0.1.0. Check the Action’s release notes or changelog for the version that introducedexitStatussupport. If v0.1.0 predates that change, bump to the first tag whereexitStatusis documented.- zaproxy/zap-stable Docker image
• The stockzaproxy/zap-stable:latestimage may bundle an older Automation Framework add-on. Inspect its installed AF add-on version (via the ZAP UIHelp → About → Add-onsor the ZAP API) and ensure it matches or exceeds the version that addedexitStatussupport. If not, either switch to the weekly build (zaproxy/zap-weekly) or add a startup step to update the AF add-on.File: owasp-zap/data/test-automation.yml
Lines: 115–117- type: exitStatus parameters: warnExitValue: 0Optional refinements once support is confirmed:
- Explicitly set
infoExitValueanderrorExitValueto make the intended behavior self-documenting.- Add a comment clarifying your gating policy (“warnings don’t fail the build, errors do,” etc.).
jusdino
left a comment
jusdino
left a comment
There was a problem hiding this comment.
Looks good - just want to confirm one item:
|
@isabeleliassen this should be good to merge! |
3 participants
#962 moved the API spec, this fixes it and attempts to use a new job introduced in ZAP's automation framework that supports updating the exit codes: zaproxy/action-af#12
Summary by CodeRabbit