Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Misc ZAP Updates #914

Merged
merged 50 commits into from
Feb 4, 2025
Merged

feat: Misc ZAP Updates #914

merged 50 commits into from
Feb 4, 2025

Conversation

afwilcox
Copy link
Collaborator

@afwilcox afwilcox commented Jan 30, 2025

Description

Please provide a summary of the change and the issue fixed. Please include relevant context. List dependency changes.

Made some changes to the Caddy File and Libraries in order to address the following vulnerabilities:
- CSP: Wildcard Directive
- CSP: script-src unsafe-eval
- CSP: script-src unsafe-inline
- Dangerous JS Function (Upgraded library)
- Deprecated Feature Policy Header Set

Updated the GitHub actions to do the following:

  • Removed ZAP from scheduled scans and added into PR workflow
  • Fail Action if a High Severity issue is found

The following ZAP reports were unable to be resolved:

  • CSP: style-src unsafe-inline: This is part of font awesome, and doesn't seem to work without it. See this GitHub Issue: SVG Symbols violate strict CSP style-src directives FortAwesome/Font-Awesome#16827
  • Hidden File Found: This is because our custom error page is triggered which does not return 404 error code
  • Proxy Disclosure: Suspect this is possibly coming from OpenShift is there is nothing sensitive in the headers, and attempting to disable OPTIONS, TRACE and TRACK in Caddy did not resolve the issue
  • Cookie with SameSite Attribute None: This is a Keycloak Cookie
  • Insufficient Site Isolation Against Spectre Vulnerability: Patching this will allow the authentication redirects to not function properly.
  • Timestamp Disclosure: This is a false positive as a result of some code in the Keycloak library
  • Information Disclosure: Suspicious Comments: - this is coming from a third party library where a comment contains the word 'bug'
  • Modern Web Application: Yes, this is a modern web application...
  • Non Storable Content: This is intentional as we don't want to allow anything to be cached
  • Sec-Fetch-Dest Header is Missing: Adding Header did not resolve issue, possibly being flagged from something else?
  • Sec-Fetch-Mode Header is Missing: Adding Header did not resolve issue, possibly being flagged from something else?
  • Sec-Fetch-Site Header is Missing: Adding Header did not resolve issue, possibly being flagged from something else?
  • Sec-Fetch-User Header is Missing: Adding Header did not resolve issue, possibly being flagged from something else?
  • Session Management Response Identified: As per ZAP this is informational only

Fixes # (issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Further comments


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:

Copy link

sonarqubecloud bot commented Feb 4, 2025

Copy link
Collaborator

@jon-funk jon-funk left a comment

Choose a reason for hiding this comment

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

Really appreciate the work on this, always nice to do cleanup, improve standards and workflows.

Reading through the leftover ZAPs, none stick out as concerning. As with most policy tools you get some noise.

target: https://${{ env.PREFIX }}-frontend.${{ env.DOMAIN }}/api
- name: Check for Back End High Risk Alerts
run: |
HIGH_RISK_COUNT=$(jq '[.site[].alerts[] | select(.riskcode == "3")] | length' report_json.json)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really clever use of jq in the zap report output!

@jon-funk jon-funk merged commit e7a0838 into release/1.0.2 Feb 4, 2025
20 checks passed
@jon-funk jon-funk deleted the CE-1378-zap-updates branch February 4, 2025 18:14
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.

2 participants