fix: upgrade sdk 1.1.13 & fix security issue #89
Conversation
WalkthroughUpdated package.json with dependency bumps (internal SDKs and axios), added Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant HW as getHardwareSDKInstance
participant SDK as Hardware SDK
App->>HW: request instance
alt First call or re-init
HW->>SDK: init({ connectSrc: "https://jssdk.onekey.so/1.1.13/", ... })
SDK-->>HW: initialized SDK instance
HW-->>App: return instance
else Already initialized
HW-->>App: return cached instance
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
package.json(3 hunks)src/hardware/instance.ts(1 hunks)
🔇 Additional comments (3)
package.json (2)
182-191: Enforce Yarn 1.22.22 and document resolution pins
- yarn.lock is present and contains the pinned versions (protobufjs 6.11.4, sha.js 2.4.12, cipher-base 1.0.6, @babel/traverse 7.28.4, base-x 3.0.11, cross-spawn 7.0.6, webpack-dev-middleware 5.3.4).
- Action: enforce Yarn v1.22.22 in CI and dev environments (add "engines" to package.json and/or pin/validate Yarn in CI jobs) and add a short rationale for each resolution (security/bug/compat) in package.json or a DEPENDENCIES.md.
33-33: Verify Axios 1.x migration — repo scan failed, please re-run checksrg errored with "unrecognized file type: tsx" so I couldn't confirm usages. Run these from the repo root (no --type flags) and paste results:
rg -n -S '\baxios\b' -C2 .
rg -n -S 'CancelToken|isCancel|axios.all(|axios.spread(|interceptors.(request|response).use(' -C2 .
rg -n -S 'paramsSerializer|toFormData|FormData|Content-Type|headers.common' -C2 .
rg -n -S "from\s+'axios'|require(['"]axios['"])" -C2 .Quick migration checklist to act on if matches appear:
- Replace CancelToken usage (use AbortController) and remove isCancel reliance.
- Ensure default import style (import axios from 'axios') or correct require interop.
- Update error handling to use axios.isAxiosError and adjust interceptor types.
- Audit FormData uploads and explicitly set/omit Content-Type; use toFormData if needed.
- Stop relying on request.headers.common.*; use the flattened header object.
- Update paramsSerializer shape or pick a 1.x that restores function option.
Attach the rg outputs and I will verify findings.
src/hardware/instance.ts (1)
18-23: Avoid version skew: don't hardcode connectSrc.You bumped SDK deps to ^1.1.13 but hardcoded connectSrc to 1.1.13. A later minor/patch (e.g., 1.1.14) will mismatch and can break runtime. Derive the URL from the installed SDK version (allow an env override), or pin deps exactly.
Apply this diff:
- connectSrc: 'https://jssdk.onekey.so/1.1.13/', + // Keep JSSDK in lockstep with the installed SDK (override via env if needed) + connectSrc: `https://jssdk.onekey.so/${(process.env.REACT_APP_ONEKEY_JSSDK_VERSION ?? getSDKVersion()).split('-')[0]}/`,Docs show the JSSDK uses versioned iframe URLs — confirm the CDN serves https://jssdk.onekey.so//iframe.html for the versions you expect (stable and prerelease). (developer.onekey.so)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json(3 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: Socket Security: Pull Request Alerts
🔇 Additional comments (4)
package.json (4)
181-181: No behavior change in eslintConfig block.Looks fine.
12-14: Approve: OneKey SDKs pinned to 1.1.13 — verification passed.
No 1.1.11 references found. Confirmed jssdk.onekey.so/1.1.13 at src/hardware/instance.ts:20.
183-191: Resolutions applied in yarn.lock — document reasons and verify compatibility.yarn.lock shows the resolutions took effect: protobufjs → 6.11.4; sha.js → 2.4.12; cipher-base → 1.0.6; @babel/traverse → 7.28.4; base-x → 3.0.11; cross-spawn → 7.0.6; webpack-dev-middleware → 5.3.4.
- Document why each pin is required (link each to the advisory/PR). Prefer bumping direct deps or scoped resolutions instead of broad overrides.
- Babel: @babel/traverse is forced to 7.28.4 while other @babel/* versions vary in yarn.lock — verify @babel/core/@babel/parser compatibility and run the build/tests.
- webpack-dev-middleware: confirm compatibility with webpack-dev-server 4.x (check package.json) and smoke-test the dev server.
File: package.json (resolutions block lines 183–191).
33-33: Axios 1.x bump — verify CancelToken → AbortController and progress handlersrg returned "No files were searched", so I couldn't verify. Run and paste results:
rg -nP --hidden --no-ignore '\bCancelToken\b|axios.CancelToken|axios.isCancel' -g '!/node_modules/' || true
rg -nP --hidden --no-ignore '\bon(Upload|Download)Progress\b' -g '!/node_modules/' || trueIf matches are found, migrate CancelToken/axios.isCancel to AbortController-based cancellation and test onUploadProgress/onDownloadProgress and any adapter/polyfill assumptions.
Summary by CodeRabbit