Skip to content

Chore/trezor vendor sync v26.7.4 - #901

Open
ByteZhang1024 wants to merge 32 commits into
onekeyfrom
chore/trezor-vendor-sync-v26.7.4
Open

Chore/trezor vendor sync v26.7.4#901
ByteZhang1024 wants to merge 32 commits into
onekeyfrom
chore/trezor-vendor-sync-v26.7.4

Conversation

@ByteZhang1024

Copy link
Copy Markdown
Contributor

No description provided.

… relay bridge

Trezor: verifyAuthenticityProof now takes an explicit proofType so an
Optiga proof can no longer be validated against Tropic's root keys/
algorithm (or vice versa), closing a proof-substitution gap. deviceId
now always derives from sha3_256(deviceCertPubKey) instead of mixing in
the X.509 serial number.

Ledger: removes the server-owned APDU relay bridge
(startDeviceAttestationApduBridge/exchangeDeviceAttestationApdu) and its
supporting state, since device-reward claiming is moving out of the app
and onto a web page that calls the SDK directly. The genuine-check path
keeps using DMK's own deviceId output without a local re-derivation
cross-check.

Updates HANDOFF-device-attestation.md and
docs/device-attestation-voucher-backend.md to describe the current
verified interface instead of the investigation history.
@socket-security

socket-security Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​bignumber.js@​11.1.510010010087100

View full report

Comment thread packages/hwk-ledger-adapter/src/connector/LedgerConnectorBase.ts Outdated
Comment thread packages/hwk-trezor-transport/src/transports/bridge.ts
Comment thread packages/hwk-trezor-adapter/src/deviceAuthenticity/verifyAuthenticityProof.ts Outdated
@originalix

originalix commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Automated code review found blocking issues

@ByteZhang1024

Reviewed commit 5311691511af. This report always reflects the latest reviewed changes.

Review summary

This PR vendors the Trezor transport-common and browser layers, refreshes protobuf and protocol definitions, adds streamed Trezor device-authenticity verification, and adds Ledger genuine-check relay configuration while updating package wiring and examples.

What needs attention: Remove the newly introduced uncontrolled network and logging paths, then rerun browser, transport, and package validation.

Issues to address

  • P1 · WebUSB initialization allows an arbitrary sessions-background fetch
    • Impact: sessionsBackgroundUrl is caller-controlled and init() issues a fetch to it without origin or path validation. This expands the SDK's outbound network surface and permits arbitrary hosts to receive client requests during transport startup.
    • Suggested action: Restrict the URL to the same-origin generated worker asset (or a tightly defined allowlist), reject external origins, and use credentials omission for the asset probe.
  • P1 · Protocol errors log raw transport payload bytes
    • Impact: When decoding fails, the new transport utility writes the first response buffer as hexadecimal to console.warn. Device responses can contain transaction/signing material and other sensitive protocol data, so malformed input or device errors can expose it in browser or application logs.
    • Suggested action: Remove the payload dump and log only non-sensitive metadata such as message type, length, and error code.
  • P1 · Ledger genuine-check adds an unapproved outbound WebSocket path
    • Impact: Calling getDeviceGenuineCheck constructs Ledger's DMK action, which opens the documented wss://scriptrunner.api.live.ledger.com/update/genuine backend by default. This is a new external network path carrying an attestation transcript and device metadata outside the repository's approved network surface.
    • Suggested action: Route the operation through a validated OneKey-owned relay or obtain an explicit network-policy exception with enforced endpoint and data-flow controls before exposing this API.

Validation gaps

The focused package build could not run because the workspace lacks the tsup executable (/bin/sh: tsup: command not found). Browser bundling, transport integration, and real-device attestation paths therefore still need to be validated after dependencies are installed.

Comment thread packages/hwk-ledger-adapter/src/connector/LedgerConnectorBase.ts Outdated
Comment thread packages/hwk-trezor-transport/src/transports/bridge.ts
Comment thread packages/hwk-trezor-adapter/src/deviceAuthenticity/verifyAuthenticityProof.ts Outdated
P1: add an SDK-owned host allowlist for the Ledger genuine-check relay
URL. LedgerConnectorBase.configure() previously accepted any wss: URL,
letting a caller redirect the device-attestation transcript to an
untrusted server. Now validates hostname (against an explicit
allowlist), path shape, token format, and rejects userinfo/query/
fragment/non-default ports.

P1: restore the legacy-bridge protocol-message capability negotiation
that the v26.7.4 vendor sync silently dropped from
hwk-trezor-transport's BridgeTransport. init() now reads back
protocolMessages from trezord's own response, and getProtocol/
getRequestBody/createProtocolMessage/validateProtocolMessage fall back
to the raw (non-JSON-wrapped) legacy framing when a bridge doesn't
advertise support, matching this package's pre-sync behavior exactly
(verified by diffing against the vendored source and against the
pre-sync onekey commit).

P2: reject Trezor attestation certificates whose validity window has
lapsed. verifyAuthenticityProof only checked that the CA certificate's
notBefore wasn't in the future; it never checked notAfter for the CA or
the device leaf, so an expired chain could still verify. Both
certificates' full validity intervals are now checked against the
current time.
Confirmed with the OneKey team: attestation.onekey.com is the
production relay for rebate.onekey.com, alongside the staging
attestation.onekeytest.com already allowed.
…ned subdomain

The exact-subdomain allowlist assumed the relay always lives at
attestation.<root>, but that subdomain name is an operational detail
that shouldn't require an SDK code change if it differs. Accept any
host equal to, or a subdomain of, onekeytest.com / onekey.com instead,
with tests covering lookalike-domain rejection (notonekeytest.com,
onekeytest.com.attacker.example) alongside the relaxed match.
Comment thread packages/hwk-ledger-adapter/src/connector/LedgerConnectorBase.ts
The root "build" script was copy-pasted from "publish-packages", which
correctly excludes these internal-only vendored packages from
independent npm publishing. But "build" needs them compiled too:
hwk-trezor-connector depends on hwk-trezor-schema-utils's built
dist/index.mjs (via hwk-trezor-utils), which never gets produced when
the package is skipped, so esbuild fails to resolve it. Every ignored
package already has a working "build": "tsup" script; there was no
reason to skip them here.

Verified locally: `yarn build` now builds all 33 packages successfully.
This is the root cause of both CI's "build" and "lint" job failures on
PR #901 (lint runs a build step first).
Comment thread packages/hwk-trezor-adapter/src/deviceAuthenticity/index.ts
Comment thread packages/hwk-ledger-adapter/src/connector/LedgerConnectorBase.ts
Not required to maintain going forward; the backend-facing spec lives
in app-monorepo's BACKEND-API-HANDOFF.md.
Comment thread packages/hwk-ledger-adapter/src/connector/LedgerConnectorBase.ts
@ByteZhang1024
ByteZhang1024 enabled auto-merge (squash) August 26, 2026 05:20
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