Skip to content

refactor(calm-hub): use centralized strict sanitization policy to prevent XSS#1860

Merged
jpgough-ms merged 3 commits intofinos:mainfrom
rocketstack-matt:fix/sanitization-security-check
Nov 27, 2025
Merged

refactor(calm-hub): use centralized strict sanitization policy to prevent XSS#1860
jpgough-ms merged 3 commits intofinos:mainfrom
rocketstack-matt:fix/sanitization-security-check

Conversation

@rocketstack-matt
Copy link
Member

Description

Security advisory https://github.com/finos/architecture-as-code/security/dependabot/112 highlights a potential XSS issue, analysis shows this is not an issue with the current implementation, but also that there are a couple of small improvements we could make, which are in this PR.

Refactors CoreSchemaResource to use ResourceValidationConstants.STRICT_SANITIZATION_POLICY
instead of creating a new policy instance. This ensures consistent application of security policies.

Adds a regression test TestSanitizationSecurityShould to verify immunity against
OWASP HtmlPolicyBuilder XSS vulnerability involving noscript and style tags.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Code style/formatting changes
  • ♻️ Refactoring (no functional changes)
  • ⚡ Performance improvements
  • ✅ Test additions or updates
  • 🔧 Chore (maintenance, dependencies, CI, etc.)

Affected Components

  • CLI (cli/)
  • Shared (shared/)
  • CALM Widgets (calm-widgets/)
  • CALM Hub (calm-hub/)
  • CALM Hub UI (calm-hub-ui/)
  • Documentation (docs/)
  • VS Code Extension (calm-plugins/vscode/)
  • Dependencies
  • CI/CD

Commit Message Format ✅

Testing

  • I have tested my changes locally
  • I have added/updated unit tests
  • All existing tests pass

Checklist

  • My commits follow the conventional commit format
  • I have updated documentation if necessary
  • I have added tests for my changes (if applicable)
  • My changes follow the project's coding standards

…vent XSS

Refactors CoreSchemaResource to use ResourceValidationConstants.STRICT_SANITIZATION_POLICY
instead of creating a new policy instance. This ensures consistent application of
security policies.

Adds a regression test TestSanitizationSecurityShould to verify immunity against
OWASP HtmlPolicyBuilder XSS vulnerability involving noscript and style tags.
Copilot AI review requested due to automatic review settings November 27, 2025 11:47
@github-actions github-actions bot added the calm-hub The Calm Hub Product label Nov 27, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the security sanitization approach in the CALM Hub to use a centralized policy, improving maintainability and consistency. The changes address a potential XSS vulnerability highlighted in a security advisory by ensuring all sanitization uses the same strict policy configuration defined in ResourceValidationConstants.

Key Changes:

  • Refactored CoreSchemaResource to reference the centralized ResourceValidationConstants.STRICT_SANITIZATION_POLICY instead of creating a local policy instance
  • Added comprehensive regression tests verifying immunity against OWASP HtmlPolicyBuilder XSS vulnerabilities involving noscript and style tags

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
calm-hub/src/main/java/org/finos/calm/resources/CoreSchemaResource.java Refactored to use centralized sanitization policy constant
calm-hub/src/test/java/org/finos/calm/resources/TestSanitizationSecurityShould.java Added security regression tests for XSS attack vectors

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

…onSecurityShould.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jpgough-ms jpgough-ms merged commit 01d4cb7 into finos:main Nov 27, 2025
13 checks passed
@rocketstack-matt rocketstack-matt deleted the fix/sanitization-security-check branch November 27, 2025 13:55
rocketstack-matt added a commit to rocketstack-matt/architecture-as-code that referenced this pull request Nov 28, 2025
…vent XSS (finos#1860)

* refactor(calm-hub): use centralized strict sanitization policy to prevent XSS

Refactors CoreSchemaResource to use ResourceValidationConstants.STRICT_SANITIZATION_POLICY
instead of creating a new policy instance. This ensures consistent application of
security policies.

Adds a regression test TestSanitizationSecurityShould to verify immunity against
OWASP HtmlPolicyBuilder XSS vulnerability involving noscript and style tags.

* chore: Remove blank lines between imports in CoreSchemaResource.java

* Update calm-hub/src/test/java/org/finos/calm/resources/TestSanitizationSecurityShould.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

calm-hub The Calm Hub Product

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants