Skip to content

Commit 5f9bff6

Browse files
committed
Merge branch 'develop' into release/7.14.2-rc31
Three conflicts, plus one semantic clash the auto-merge hid. .github/workflows/ci.yml -- took develop's pipeline (PR #474 cut it from ~7:20 to ~2:00 and folded check-submodules, build-emulator, unit-tests and python-integration-tests into build-and-test) and re-applied what this branch adds on top: - permissions: contents: read at the workflow level. develop has no permissions block, so code-scanning rule actions/missing-workflow-permissions had one alert open per job. - generate-test-report restored as its OWN job rather than folded into build-and-test. develop's fold is correct for develop's report script, which only needs directories that job already has. This branch's generate-test-report.py also consumes test-reports/dylib-junit.xml and the ARM manifest -- produced by other jobs, on other runners -- and fail()s hard when either is missing. Folded, the report could never succeed. - the report's provenance env (both source SHAs, run URL, both PR URLs). develop passes only KK_BUILD_LABEL, which this branch's script does not read; the fields it does read are release gates for 7.14.2. - the report artifact is a directory again. The script emits test-report/test-report.pdf beside test-report-manifest.json and test-report.pdf.sha256; develop uploaded a bare test-report.pdf, which would have found no files. - release-evidence-gate, rewired to develop's folded job names, so a release still has one required check that every evidence-producing job succeeded. .gitleaks.toml -- union. Kept develop's header and its docs-SHA allowlist, which is broader than and subsumes this branch's python-keepkey-pin rule, plus this branch's U2F/vendored-deps allowlist and the note recording why no first-party test tree is exempted. scripts/emulator/python-keepkey.Dockerfile -- took develop's parameterised deps-stage version wholesale. scripts/emulator/Dockerfile did NOT conflict: only this branch had changed it (base -> digest), so git kept the digest silently. That defeats develop's GHCR mirror, which tags the mirrored image as kktech/firmware:v15 -- a digest FROM ignores that tag and pulls from Docker Hub on the heaviest job in the run. Parameterised it the way python-keepkey.Dockerfile already is, defaulting to the digest so a release build still names an immutable base, with CI passing the resolved mirror. kktech/firmware:v15 resolves to exactly that digest today. Verified: emulator image builds both with the default and with an explicit --build-arg; 153/153 unit tests pass; gitleaks 8.30.1 finds no leaks with the merged config; clang-format clean.
2 parents e1f5e99 + fc1e937 commit 5f9bff6

8 files changed

Lines changed: 730 additions & 311 deletions

File tree

.github/copilot-instructions.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Copilot code review instructions
2+
3+
How reviews on this repository should be conducted. These govern the review
4+
process itself, not what counts as good code.
5+
6+
## Report everything in one pass
7+
8+
Report every finding you have in a single review. Do not hold findings back for a
9+
later round.
10+
11+
- Review the complete diff — every changed file, every changed hunk. Do not stop
12+
partway through.
13+
- Finding several problems early is not a reason to end the review. It is a reason
14+
to keep going.
15+
- Do not sample. If a file has twelve problems, report twelve.
16+
- Comment on every instance of a repeated problem, not just the first. If the same
17+
mistake appears in six places, mark all six — the author fixes what is marked, so
18+
"and similar issues elsewhere" leaves five defects in the branch.
19+
20+
## Include minor and uncertain findings
21+
22+
Do not filter down to only the findings you are most confident about.
23+
24+
- Report low-severity and minor issues alongside significant ones.
25+
- Report a finding you are less than certain about, and say plainly what you are
26+
unsure of. A finding the author dismisses in ten seconds costs far less than an
27+
entire extra review cycle.
28+
- Label each finding's severity so the author can triage quickly instead of having
29+
to weigh every comment equally.
30+
31+
## Converge in as few rounds as possible
32+
33+
Every additional review round costs the author a full cycle. Target:
34+
35+
- **One round** — ideal. Everything surfaced on the first review.
36+
- **Two or three rounds** — acceptable.
37+
- **Five rounds** — the absolute ceiling.
38+
39+
A later round should raise only problems introduced by the previous round's fixes.
40+
A problem that was present in the original diff and went unmentioned until round
41+
three is a review failure, not a thorough review.
42+
43+
## Do not trade thoroughness for brevity
44+
45+
None of the above is a reason to scrutinize less carefully. The goal is more
46+
findings per review, not faster reviews. Depth of analysis stays the same; what
47+
changes is that all of it arrives at once.

.github/workflows/ci.yml

Lines changed: 544 additions & 294 deletions
Large diffs are not rendered by default.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Mirrors the firmware build base image from Docker Hub into this org's GHCR.
2+
#
3+
# Why: every CI job that compiles anything starts by pulling ~650 MB of base
4+
# image. Served from Docker Hub that measured ~34s per job; GHCR serves it to
5+
# GitHub-hosted runners over the same network and does not apply Docker Hub's
6+
# anonymous pull limits.
7+
#
8+
# CI treats the mirror as optional -- ci.yml falls back to Docker Hub with a
9+
# warning if the pull fails -- so this workflow never becomes a hard
10+
# dependency of the build. Run it once to populate the mirror, and again
11+
# whenever BASE_IMAGE in ci.yml is bumped.
12+
name: Mirror base image
13+
14+
on:
15+
workflow_dispatch:
16+
inputs:
17+
source_image:
18+
description: 'Docker Hub image to mirror (must match BASE_IMAGE in ci.yml)'
19+
required: true
20+
type: string
21+
default: 'kktech/firmware:v15'
22+
23+
permissions:
24+
contents: read
25+
packages: write
26+
27+
jobs:
28+
mirror:
29+
runs-on: ubuntu-latest
30+
timeout-minutes: 20
31+
steps:
32+
- name: Log in to GHCR
33+
uses: docker/login-action@v4
34+
with:
35+
registry: ghcr.io
36+
username: ${{ github.actor }}
37+
password: ${{ secrets.GITHUB_TOKEN }}
38+
39+
- name: Pull, tag and push
40+
env:
41+
SOURCE: ${{ inputs.source_image }}
42+
run: |
43+
set -euo pipefail
44+
45+
# The source is constrained to the upstream base image, and the
46+
# destination name is fixed rather than derived from the input.
47+
#
48+
# Both matter. ci.yml pulls BASE_IMAGE_MIRROR and builds firmware
49+
# from it, so whatever lands at ghcr.io/<owner>/firmware:<tag> is
50+
# trusted by every subsequent build. Deriving the destination from
51+
# the input -- as this previously did, via ${SOURCE##*/} -- meant a
52+
# dispatch of `anyone/firmware:v15` would resolve to that same
53+
# destination and overwrite the image CI trusts. Dispatch needs
54+
# write access, but "a writer can typo" and "a writer can silently
55+
# replace the firmware build base" are different blast radii.
56+
if [[ ! "${SOURCE}" =~ ^kktech/firmware:[A-Za-z0-9._-]+$ ]]; then
57+
echo "::error::refusing to mirror '${SOURCE}'. This workflow only mirrors kktech/firmware:<tag>, because ci.yml builds firmware from whatever it publishes."
58+
exit 1
59+
fi
60+
61+
TAG="${SOURCE##*:}"
62+
DEST="ghcr.io/${{ github.repository_owner }}/firmware:${TAG}"
63+
echo "mirroring ${SOURCE} -> ${DEST}"
64+
docker pull "${SOURCE}"
65+
docker tag "${SOURCE}" "${DEST}"
66+
docker push "${DEST}"
67+
echo "::notice::Mirrored ${SOURCE} to ${DEST}"
68+
echo "Set BASE_IMAGE_MIRROR in ci.yml to ${DEST} if it differs."

.gitignore

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,8 @@ build
33
.DS_Store
44
.vscode/
55

6+
.claude/
7+
8+
# cppcheck output (static-analysis writes this at repo root in CI)
9+
cppcheck_report.txt
10+
cppcheck_annotations.txt

.gitleaks.toml

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
1-
title = "KeepKey firmware gitleaks configuration"
1+
# gitleaks configuration for keepkey-firmware
2+
#
3+
# Extends the stock gitleaks rule set — every default rule stays active. This
4+
# file only adds narrowly-scoped allowlists for false positives that recur
5+
# across commits, which .gitleaksignore cannot express: its entries are keyed
6+
# by commit SHA, so a rebased, cherry-picked, or merged commit reintroduces the
7+
# same finding under a new fingerprint. Prefer .gitleaksignore for a genuine
8+
# one-off; add here only when the same content keeps coming back.
29

310
[extend]
411
useDefault = true
512

613
# Published U2F attestation material and vendored third-party code.
714
#
815
# Deliberately NOT allowlisted: any first-party test tree. An earlier revision
9-
# carried '''^tests/''' here, which turned off secret detection for every
10-
# first-party test present or future -- a real credential committed under it
11-
# would have passed CI. No such directory exists in this repository, so the
12-
# entry exempted nothing and only stood ready to exempt something later. If a
13-
# fixture ever does trip a rule, allowlist that path or its fingerprint, not
14-
# the tree above it.
16+
# of this branch carried '''^tests/''' here, which turned off secret detection
17+
# for every first-party test present or future -- a real credential committed
18+
# under it would have passed CI. No such directory exists in this repository,
19+
# so the entry exempted nothing and only stood ready to exempt something later.
20+
# If a fixture ever does trip a rule, allowlist that path or its fingerprint,
21+
# not the tree above it.
1522
[[allowlists]]
1623
description = "Public device material and vendored third-party code"
1724
paths = [
@@ -20,10 +27,17 @@ paths = [
2027
'''^deps/''',
2128
]
2229

30+
# Release/security docs record the exact submodule commits a build pins, e.g.
31+
# - python-keepkey: `c406a1ba9120da410c356dbff7f4d4bd1e1758fa`;
32+
# A 40-char lowercase hex git SHA has enough entropy to trip generic-api-key.
33+
# Scoped with condition = "AND" so this only exempts SHA-shaped strings inside
34+
# markdown docs — a real credential in docs/ is still reported.
35+
#
36+
# This supersedes an earlier, narrower rule on the release branch that matched
37+
# only python-keepkey pins under generic-api-key; every string that one
38+
# exempted is a subset of this one.
2339
[[allowlists]]
24-
description = "Documented python-keepkey commit pins"
25-
targetRules = ["generic-api-key"]
40+
description = "Git commit SHAs quoted in markdown docs"
2641
condition = "AND"
27-
paths = ['''^docs/''']
28-
regexes = ['''python-keepkey[^\n]{0,16}`[0-9a-f]{40}`''']
29-
regexTarget = "match"
42+
paths = ['''(^|/)docs/.*\.md$''']
43+
regexes = ['''\b[0-9a-f]{40}\b''']

scripts/emulator/Dockerfile

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
1-
FROM kktech/firmware@sha256:7438e53933d47d53157ed6d96d864cb208597e62dce26235ace09d1063427fa2
1+
# Parameterised for the same reason as python-keepkey.Dockerfile: CI resolves
2+
# the base through a GHCR mirror and must be able to point FROM at it, while a
3+
# plain `docker build` keeps working unchanged.
4+
#
5+
# The DEFAULT is a digest, not the :v15 tag develop carries. A release build
6+
# has to name an immutable base -- kktech/firmware:v15 resolves to exactly this
7+
# digest today, but a tag can be moved and a signed release should not depend
8+
# on that. CI overrides it with the mirror it resolved, which is the same image
9+
# either way.
10+
ARG BASE_IMAGE=kktech/firmware@sha256:7438e53933d47d53157ed6d96d864cb208597e62dce26235ace09d1063427fa2
11+
FROM ${BASE_IMAGE}
212

313
WORKDIR /kkemu
414
COPY ./ /kkemu

scripts/emulator/docker-compose.yml

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,22 @@ services:
1313
- "127.0.0.1:5000:5000"
1414
healthcheck:
1515
test: ["CMD-SHELL", "python3 -c \"import urllib.request; urllib.request.urlopen('http://localhost:5000/health')\" || exit 1"]
16-
interval: 3s
16+
# The emulator is listening well inside a second; a 3s interval meant
17+
# the integration suite sat waiting on poll granularity, not on the
18+
# service. retries is raised alongside so shortening the interval does
19+
# not also shorten how long a slow start is tolerated -- a failing check
20+
# can burn up to `timeout` before the next `interval`, so the ceiling is
21+
# roughly start_period + retries * (timeout + interval), not
22+
# retries * interval.
23+
interval: 1s
1724
timeout: 3s
18-
retries: 20
19-
start_period: 10s
25+
retries: 40
26+
start_period: 3s
2027
python-keepkey:
28+
# Named so CI can build it once (`docker compose build python-keepkey`)
29+
# and then `up` without --build, which would otherwise also force a
30+
# rebuild of kkemu — the expensive one.
31+
image: kktech/kkemu-tests:latest
2132
build:
2233
context: '../../'
2334
dockerfile: 'scripts/emulator/python-keepkey.Dockerfile'
@@ -29,6 +40,9 @@ services:
2940
kkemu:
3041
condition: service_healthy
3142
firmware-unit:
43+
# Local-dev convenience only — CI runs `make xunit` directly against the
44+
# image it just built, rather than paying for a second compose build.
45+
image: kktech/kkemu:latest
3246
build:
3347
context: '../../'
3448
dockerfile: 'scripts/emulator/Dockerfile'

scripts/emulator/python-keepkey.Dockerfile

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
FROM kktech/firmware@sha256:7438e53933d47d53157ed6d96d864cb208597e62dce26235ace09d1063427fa2
1+
# Split into a `deps` stage on purpose. The dependency layers below are
2+
# stable across commits and worth caching; the COPY layer beneath them ships
3+
# the whole build context and is invalidated by every commit, so exporting it
4+
# to a layer cache is pure cost with no possible hit. CI caches `deps` only.
5+
# Parameterised because buildx's docker-container driver resolves FROM
6+
# against a registry rather than the local daemon, so CI must be able to
7+
# point it at the GHCR mirror explicitly. The default keeps plain
8+
# `docker build` and local use working unchanged.
9+
ARG BASE_IMAGE=kktech/firmware:v15
10+
FROM ${BASE_IMAGE} AS deps
211

312
# Extra Python deps needed by tests that aren't in the shared base image.
413
# - rlp + eth-keys + eth-utils: build the canonical EIP-1559 type-2 pre-image
@@ -10,6 +19,8 @@ FROM kktech/firmware@sha256:7438e53933d47d53157ed6d96d864cb208597e62dce26235ace0
1019
RUN apk add --no-cache python3-dev gcc musl-dev
1120
RUN python3 -m pip install --no-cache-dir rlp eth-keys eth-utils pycryptodome
1221

22+
FROM deps
23+
1324
WORKDIR /kkemu
1425
COPY ./ /kkemu
1526

0 commit comments

Comments
 (0)