feat: support Pro2 host asset packages

[wabicai]

Summary

• package Pro2 NFT and wallpaper payloads as unsigned resource containers with raw LZ4 blocks for firmware 1.0.1+
• retain the existing upload flow for earlier firmware versions
• allow Pro2 and Neo structured Safe TypedData payloads up to 1536 bytes
• bump publishable SDK packages to 1.2.2-alpha.0

Validation

• core test suite: 87 suites passed, 1222 tests passed
• focused NFT/LZ4 tests: 18 passed
• focused Safe TypedData tests: 32 passed
• targeted ESLint passed
• full SDK build passed

[sidmorizon]

@codex review

[sidmorizon]

@codex security review

[sidmorizon]

@cursoragent review

[chatgpt-codex-connector]

You have reached your Codex usage limits for security reviews. Please try again later.

[cursor]

Reviewed PR #900 (feat/pro2-nft-wallpaper-upload-v2) as a read-only pass over Pro2/Neo Protocol V2 NFT/wallpaper packaging and Safe TypedData routing. Full notes are on the PR.

What looks solid: unlock policy, vol1 path confinement, pre-I/O JPEG/Base64 checks, legacy < 1.0.1 upload path, supported_messages gating, and no NftUpdate replay. Structured 1536-byte Safe TypedData for Pro2/Neo is the safer signing route if firmware can parse it.

Proven issues

• The 1.0.1-beta.1 reject test is bypassed in production: getCurrentFirmwareVersionString() strips prerelease to 1.0.1.
• Firmware 1.0.1+ writes *.okpkg but NFT responses still return .bin / _m.bin / .json paths.
• Host-side 10-NFT cap is skipped on the package path; docs still say Core enforces it.
• Wallpaper fileName is validated, then silently replaced with wallpaper.okpkg.
• Current-fact docs (pro2-device-management.md, evm.md) still describe the old flows.

Residual risk: host OKPP omits official RESC flexible_metadata; confirm firmware’s unpacker does not require it. Branch is also 1 commit behind onekey (v1.2.1).

No code changes in this pass. I can turn the proven items into a follow-up patch if you want that.

  

[cursor]

Review scope

• Refs: feat/pro2-nft-wallpaper-upload-v2 (df728888) vs onekey (23da0e26)
• Runtime / transport: Core host-side packaging; all transports only move the resulting bytes
• Device / protocol: Pro2 and Neo, Protocol V2, call phase (deviceUploadNft, deviceUploadWallpaper, evmSignTypedData)
• Threat boundary: untrusted JPEG/Base64 and TypedData input → validation → host packaging or signing-route selection → device confirmation / filesystem write

This is a read-only review. Findings below are split into proven issues and hypotheses.

What looks correct

• Unlock policy stays unlock-before-run with DeviceSessionPinType.Any; wallet Session handling stays off.
• JPEG/Base64, size, title/subtitle, and wallpaper fileName charset checks still happen before device I/O.
• Writes stay under vol1:/nft and vol1:/wallpapers; archive entry names are SDK-generated, not caller paths.
• Protocol messages are still gated by ProtocolInfo.supported_messages. The new version check only chooses payload layout, not a new command.
• Firmware < 1.0.1 keeps the existing raw .bin / triplet upload path.
• NftUpdate is still sent once and is not retried on timeout or business failure.
• Safe TypedData 1536-byte routing for Pro2/Neo prefers structured device display over hash/blind-sign. That is the safer signing path if firmware can parse the payload.
• Local raw-LZ4 encoder is isolated, dependency-free, and the golden test matches the expected single-block token layout.
• Published package versions are bumped together to 1.2.2-alpha.0.

Proven findings

1. Package-format gate is weaker than its own unit test

supportsPro2HostAssetPackage('1.0.1-beta.1') is tested as false, but production never sees that string.

  getCurrentFirmwareVersionString() {
    return parseDeviceVersion(this.state?.versions.firmware).join('.');
  }

parseDeviceVersion() keeps only major.minor.patch. A device reporting 1.0.1-beta.1 becomes "1.0.1", and the upload path then builds an .okpkg.

• Precondition: firmware DeviceInfo version includes a prerelease tag below the real host-asset unpacker.
• Impact: SDK writes a package the device cannot unpack, or skips the legacy storage-limit path on a build that still needs it.
• Remediation: compare the raw state.versions.firmware string (or fail closed on prerelease). Add a test that goes through getCurrentFirmwareVersionString(), not only the helper.

2. NFT public paths no longer match the file that was written

On firmware 1.0.1+, Core writes vol1:/nft/<basename>.okpkg but still returns:

• imagePath: vol1:/nft/<basename>.bin
• thumbnailPath: …_m.bin
• metadataPath: …json

The new test locks this in. Callers that persist those paths for later read/delete/display will miss the uploaded object unless firmware unpacks the archive to those exact names.

• Remediation: return the written .okpkg path (or both written and extracted paths), and document the firmware unpack contract.

3. Host-side NFT cap is removed on the new path

assertStorageCapacity() is skipped whenever the package path is selected. getCompletePro2NftBasenames() also cannot see .okpkg files.

The published contract in docs/business/pro2-device-management.md is still: count complete NFT triplets, throw NftStorageLimitReached at 10, do not ask firmware to delete the oldest item.

• Precondition: firmware 1.0.1 does not enforce the same cap.
• Impact: device user storage can be filled; NftStorageLimitReached no longer reaches the App.
• Remediation: keep the host check (and teach it .okpkg), or document and test that firmware now owns the cap and the error mapping.

4. Wallpaper fileName is accepted, then ignored

fileName is still a public parameter and is still charset-validated in init(). On firmware 1.0.1+, run() overwrites this.path with the fixed vol1:/wallpapers/wallpaper.okpkg.

That is a silent compatibility break for callers that used hashed/custom names to keep more than one wallpaper.

• Remediation: document the single-package contract, or keep fileName as the .okpkg basename. Add a test that a valid fileName is either honored or rejected explicitly.

5. Current-fact docs were not updated

The only “docs” commit comments pro2HostAssetPackage.ts. These current-fact docs still describe the old flow:

• docs/business/pro2-device-management.md — three NFT files, FilesystemDirList, host-side cap, hashed wallpaper-*.bin
• docs/business/evm.md — Touch/Pro 1.5KB rule only; Pro2/Neo 1536-byte structured SafeTx is missing

Repo rule: Core mapping, tests, and relevant docs stay in the same change.

Hypotheses / residual risk

Host OKPP omits official RESC flexible_metadata

parseProtocolV2ResourcePackageHeader() requires a non-empty device path at 0x6c. Host packages leave that region zeroed and set payload_version to 1 ([0,0,1] in the official packed-version reading).

If firmware’s host-asset unpacker shares that parser, apply/unpack fails after the file is already written. Confirm against firmware-pro2 payload_package and, if the path is required, write the Filesystem write path into flexible_metadata.

Unsigned RESOURCE containers

The writer emits a full OKPP RESC header, SHA3-512 hashes, and the Ed25519 algorithm discriminator, with empty signature slots. That is acceptable only if firmware has a distinct unsigned host-asset path and never treats these bytes as authenticated vol0 resources. SDK writes are confined to vol1, which is the right host boundary.

TypedData 1536-byte change has no Pro2 firmware floor

hasBiggerData() now gives every model_pro2 device the 1536-byte structured route, including firmware 1.0.0 in tests. That is safer than blind-sign when firmware can parse the payload. If some shipping Pro2/Neo build cannot, signing fails or the device is asked to parse more than it can display. Confirm the firmware floor or add a version gate.

Test / process gaps

• No test that a 1.0.1 upload still reports NftStorageLimitReached (or that firmware now returns the equivalent).
• No test that wallpaper fileName is honored or explicitly ignored on 1.0.1.
• LZ4 coverage is one 9-byte block. A multi-block / last-block-remainder vector would better lock the 8-byte descriptor layout.
• buildPro2HostAssetPackage() throws generic Error rather than CallMethodInvalidParameter.
• This branch is 1 commit behind onekey (v1.2.1). The version bump is 1.2.0 → 1.2.2-alpha.0; rebase before merge so the alpha sits on 1.2.1.

I did not request changes in this pass. Happy to turn the proven items into a follow-up patch if you want that.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df728888c1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".