Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRename and standardize ZAP baseline scan job/step names in CI workflow, change scan options to include a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
.zap-rules.tsv
Outdated
There was a problem hiding this comment.
Yes I removed all those ids which can be a actual security lapse
- 10010 Cookie No HttpOnly Flag
- 10011 Cookie Without Secure Flag
- 10054 Cookie without SameSite Attribute
- 10015 Re-examine Cache-control Directives due to
https://nest.owasp.dev/csrf/
.github/workflows/run-ci-cd.yaml
Outdated
| allow_issue_writing: false | ||
| fail_action: false | ||
| cmd_options: '-a -r zap-report.html' | ||
| rules_file_name: 'zap_rules.tsv' |
There was a problem hiding this comment.
| rules_file_name: 'zap_rules.tsv' | |
| rules_file_name: '.zap-rules.tsv' |
There was a problem hiding this comment.
As recommended I changed file name to .zap-rules.tsv in commit
f282877
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
This PR doesn't address the issue scope fully. The FPs suppression was the easy part whereas real findings haven't been addressed.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @.github/workflows/run-ci-cd.yaml:
- Around line 945-967: The run-production-zap-baseline-scan job is missing a
checkout step so the referenced .zapconfig in the cmd_options won't be
available; add a step using actions/checkout before the "Run baseline scan" step
(i.e., insert a step with name "Checkout repository" that uses actions/checkout)
so the ZAP action can read .zapconfig and the report upload step can find
report.html.
- Around line 604-627: The run-staging-zap-baseline-scan job references
.zapconfig via the cmd_options but never checks out the repository, so add a
checkout step before the "Run baseline scan" step; specifically, insert an
actions/checkout@vX step (or reuse existing checkout step) in the
run-staging-zap-baseline-scan job so the .zapconfig file is present for the
zaproxy/action-baseline action that uses cmd_options: '-a -c .zapconfig -r
report.html '.
🧹 Nitpick comments (2)
.github/workflows/run-ci-cd.yaml (2)
619-619: Optional: Remove trailing space in cmd_options.There's a trailing space after
report.htmlwhich is unnecessary but harmless.🔎 Proposed fix
- cmd_options: '-a -c .zapconfig -r report.html ' + cmd_options: '-a -c .zapconfig -r report.html'
960-960: Optional: Remove trailing space in cmd_options.Same trailing space issue as in the staging job.
🔎 Proposed fix
- cmd_options: '-a -c .zapconfig -r report.html ' + cmd_options: '-a -c .zapconfig -r report.html'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/run-ci-cd.yaml.zapconfigcspell/custom-dict.txt
✅ Files skipped from review due to trivial changes (1)
- .zapconfig
🧰 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
⏰ 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: Run frontend e2e tests
🔇 Additional comments (2)
cspell/custom-dict.txt (1)
148-148: LGTM!The addition of "zapconfig" to the custom dictionary is correct and prevents spell-check failures for the new
.zapconfigfile..github/workflows/run-ci-cd.yaml (1)
605-605: Good naming improvements for consistency.The job and step names are now more concise and consistent with the rest of the workflow.
Also applies to: 612-612, 621-621
|
hello @arkid15r If you don’t mind, could you please help me understand why this approach doesn’t address the real findings?? |
Your code doesn't solve real-world problems the report has, e.g. cookie flags, missing headers, not sure whether other are false positives. Each warning needs to be addressed -- either ignored or fixed. |
* fixed zap baseline scan * fix:removed false positives * lint/format * fix:removed false positives * update:followed recommendation * lint/format * Update code * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
2 participants
Resolves #3183
report_html.htmlChecklist
make check-testlocally and all tests passed