Skip to content

[Snyk] Security upgrade @thoughtspot/ts-chart-sdk from 0.0.2-alpha.3 to 1.0.0#212

Open
mnk-blr wants to merge 1 commit intomainfrom
snyk-fix-31a6e0cf751d3c4f3ad76aee84ed6432
Open

[Snyk] Security upgrade @thoughtspot/ts-chart-sdk from 0.0.2-alpha.3 to 1.0.0#212
mnk-blr wants to merge 1 commit intomainfrom
snyk-fix-31a6e0cf751d3c4f3ad76aee84ed6432

Conversation

@mnk-blr
Copy link
Copy Markdown

@mnk-blr mnk-blr commented Feb 28, 2026

snyk-top-banner

Snyk has created this PR to fix 1 vulnerabilities in the npm dependencies of this project.

Snyk changed the following file(s):

  • example/custom-leaflet-chart/package.json
  • example/custom-leaflet-chart/package-lock.json

Vulnerabilities that will be fixed with an upgrade:

Issue Score
high severity Directory Traversal
SNYK-JS-ROLLUP-15340920
  234  

Important

  • Check the changes in this PR to ensure they won't cause issues with your project.
  • Max score is 1000. Note that the real score may have changed since the PR was raised.
  • This PR was automatically created by Snyk using the credentials of a real user.

Note: You are seeing this because you or someone else with access to this repository has authorized Snyk to open fix PRs.

For more information:
🧐 View latest project report
📜 Customise PR templates
🛠 Adjust project settings
📚 Read about Snyk's upgrade logic


Learn how to fix vulnerabilities with free interactive lessons:

🦉 Directory Traversal

…et-chart/package-lock.json to reduce vulnerabilities

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-ROLLUP-15340920
@github-actions
Copy link
Copy Markdown

File Coverage
All files 88%
src/main/custom-chart-context.ts 86%
src/main/logger.ts 88%
src/main/post-message-event-bridge.ts 77%
src/react/use-custom-chart-context.tsx 86%
src/react/mocks/custom-chart-context-mock.ts 96%
src/utils/chart-config.ts 82%
src/utils/date-formatting.ts 82%
src/utils/formatting-util.ts 89%
src/utils/conditional-formatting/conditional-formatting.ts 92%
src/utils/globalize-Initializer/globalize-utils.ts 95%
src/utils/number-formatting/number-formatting-utils.ts 98%
src/utils/number-formatting/number-formatting.ts 90%

Minimum allowed coverage is 0%

Generated by 🐒 cobertura-action against 14eec5a

},
"dependencies": {
"@thoughtspot/ts-chart-sdk": "^0.0.2-alpha.3",
"@thoughtspot/ts-chart-sdk": "^1.0.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚨 Critical Version Mismatch

This upgrade creates a severe version inconsistency:

  • This example: Upgrading to ^1.0.0
  • Main SDK package: Currently at 2.9.2
  • Other examples: Using 2.7.6

Issues:

  1. The example will be using an outdated major version (1.0.0) while the current SDK is at 2.9.2
  2. This creates compatibility issues and misleading developer experience
  3. May not work with current ThoughtSpot platform features

Recommendation:
Change to "^2.9.2" to match the current SDK version, or verify that 1.0.0 is truly the correct target version for this security fix.

@@ -8,7 +8,7 @@
"name": "custom-leaflet-chart",
"version": "0.0.1",
"dependencies": {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📋 Missing Test Coverage & Security Concerns

This SDK upgrade introduces several concerns:

1. Test Coverage Gap:

  • This example has zero test coverage despite being upgraded to a major new version
  • The security fix should have regression tests to prevent future vulnerabilities

2. Security Dependencies:

  • New dependency cldr-data includes install scripts that should be reviewed
  • Multiple new dependencies increase attack surface

Recommendation:

  • Add basic functionality tests for this example
  • Review install scripts in new dependencies for security implications

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Feb 28, 2026

Security Upgrade Review Summary

Status: ❌ NOT READY TO MERGE - Critical issues must be resolved

🚨 Critical Issues

1. Major Version Mismatch

  • This example upgrades to ^1.0.0 but the main SDK is at 2.9.2
  • Creates severe compatibility issues and misleading developer experience
  • Action Required: Update to current SDK version or verify 1.0.0 is correct target

2. Example Version Inconsistency

  • Different examples now use different SDK versions (1.0.0, 2.7.6, various alphas)
  • Violates project consistency and confuses developers
  • Action Required: Standardize all example versions

⚠️ Additional Concerns

  • Missing Test Coverage: This example has zero tests despite major upgrade
  • Security Dependencies: New cldr-data package includes install scripts requiring review
  • Documentation: TSDoc version tags need updates to reflect current versions

🔒 Security Analysis

Positive: Addresses SNYK-JS-ROLLUP-15340920 (directory traversal vulnerability)
⚠️ Concern: Using outdated major version may introduce other unpatched vulnerabilities

Recommendations

  1. Before Merge: Update SDK version to ^2.9.2 or verify 1.0.0 is correct
  2. Before Merge: Ensure all examples use consistent SDK versions
  3. Optional: Add basic tests for this example
  4. Optional: Add security regression tests for the vulnerability fix

The security fix is important, but the version management issues create more problems than they solve in the current state.

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