Uniswap liquidity approval path has no automated coverage anywhere: the only integration tests skip unconditionally on the emulator and the native tests only ever assert refusal

[BitHighlander]

Finding

Severity: high · Dimension: test-integrity · Location: deps/python-keepkey/tests/test_msg_ethereum_erc20_uniswap_liquidity.py:97

CI only ever runs the emulator (kkemu reports firmware_variant "Emulator"/"EmulatorBTC", cf. common.py:193-195), so the guard at line 97 fires on 100% of CI runs and all three tests in this file skip every time. Change zxliquidtx.c:176 back from return true; to return is_self; — the exact release-blocking defect documented in the comment three lines above — and nothing turns red: the four native EXPECT_FALSE assertions still hold (LiquidityCancellationFailsClosed and LiquidityFormatsFullUint256WithoutBlankConfirmation preload a rejection on the FIRST of five confirm screens, so confirmFromAccountMatch is never reached; the other two never call confirm at all), the three python tests skip, validate_junit() accepts 'skip' because test_msg_ethereum_erc20_uniswap_liquidity is not in MUST_RUN_MODULES, and the release PDF prints E17/E18/E19 as "withheld" under a green headline. The user-visible effect of that revert is that every removeLiquidityETH to a third-party address is unsignable and the device reports "Signing cancelled by user" for a transaction the user just approved.

Evidence

test_msg_ethereum_erc20_uniswap_liquidity.py:95-99:
    def test_sign_uni_remove_liquidity_ETH(self):
        self.requires_fullFeature()
        if self.client.features.firmware_variant[0:8] == "Emulator":
            self.skipTest("Skip until emulator issue resolved")
            return
(identical guards at lines 32-38 and 65-67 for the other two tests in the file — all three tests in the file)

unittests/firmware/ethereum.cpp — every assertion on the function under test is negative:
  175:  EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg));
  181:  EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg));
  199:  EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg));
  242:  EXPECT_FALSE(zx_confirmZxLiquidTx(msg.data_initial_chunk.size, &msg));
(the two that drive real confirms, 174 and 241, preload kkconfirm_preload(0, 1) — i.e. reject the FIRST screen, so control never reaches confirmFromAccountMatch, which is the 3rd confirm: zxliquidtx.c:205)

lib/firmware/ethereum_contracts/zxliquidtx.c:164-176 — the regression this file has no test for:
   * `return is_self` refused the transaction AFTER the user approved it, and
   * ethereum.c turns that false into ActionCancelled, so the device reported
   * "Signing cancelled by user" for a transaction the user had just confirmed.
   ...
  if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput,
               "Uniswap Recipient", "%s\n%s",
               is_self ? "this wallet" : "NOT this wallet", address_str))
    return false;
  return true;

deps/python-keepkey/scripts/generate-test-report.py:2522-2525 — skip is accepted for every module except two:
MUST_RUN_MODULES = {
    'test_msg_signtx_taproot',
    'test_msg_getaddress_taproot',
}

Suggested fix

Add a native gtest to unittests/firmware/ethereum.cpp that preloads kkconfirm_preload(5, 0) for an add-liquidity fixture (4 for remove) and asserts EXPECT_TRUE(zx_confirmZxLiquidTx(...)) with EXPECT_EQ(0, kkconfirm_drain()); liquidity_tx() already sets a third-party recipient (0x11*20), so this pins both the screen count and the approve-means-approve semantics without needing the emulator. Separately, add 'test_msg_ethereum_erc20_uniswap_liquidity' to MUST_RUN_MODULES so the disclosed skip becomes visible as a gate failure rather than a green 'withheld' row.

Verification

(not independently verified)

---

Found by an adversarial audit of release/7.15 (628b09257). Each finding was independently re-checked by a separate reviewer instructed to refute it by default; this one survived at confidence ?.

[BitHighlander]

Resolved on alpha by #587 (merge 893c93d). The final immutable head e4ec531 passed both complete CI workflow runs.