ci: pin gitleaks, allowlist pre-submodule test history, and add an aggregate gate - #474
Merged
BitHighlander merged 2 commits intoAug 17, 2026
Merged
Conversation
secret-scan went red across the whole repository with no change to it. The install step resolved GITLEAKS_VERSION from releases/latest, and a newer generic-api-key rule began flagging published BIP32 test vectors in history: 12 findings across tests/test_ecies.py (2014, inherited from Trezor), tests/test_msg_eos_signtx.py and tests/test_msg_ethereum_erc20_signtx_exchange.py (2018). Zero of them are in any commit in 1af2ffe..HEAD. The flagged lines are WIF private keys printed next to their own address and public key as documentation. They are upstream test fixtures, published for a decade, and secure nothing. Two changes, because the failure had two causes. Allowlist ^tests/. That path does not exist in the working tree -- the python suite lived at the repository root before it became deps/python-keepkey -- but CI scans with fetch-depth 0, so gitleaks still reads those commits. Same situation and same remedy as the existing ^deps/ exemption. Scoped by path, not by regex: the findings are real key material by format, so a pattern exemption would suppress genuine secrets elsewhere in the tree. The cost is recorded in the config rather than left implicit: a real secret committed under tests/ would not be caught, which is acceptable only while the directory does not exist. Negative-controlled both ways -- a freshly generated EC private key planted under lib/firmware/ is still detected, the same key under tests/ is ignored. Pin gitleaks to 8.30.0. Tracking releases/latest means an upstream ruleset change can turn this gate red with no change here, and because every build job declares `needs: [.., secret-scan]`, the entire build and test graph is SKIPPED rather than failed -- silence that looks identical to success. That is the third instance of this class on this release (see #471 for lint-format, and the earlier gitleaks breakage). Partially addresses #424. Verified: gitleaks detect --source . --verbose --redact reports "no leaks found" and exits 0. Refs #424 #471
… jobs Every build and test job declares `needs: [lint-format, static-analysis, check-submodules, secret-scan]`. When one of those gate jobs fails, GitHub marks the whole downstream graph SKIPPED rather than failed -- and a skipped job is not a red check. The run summary shows green ticks on whatever finished, which reads as healthy unless someone opens the job list and notices that the ARM build, the unit tests and both python suites produced nothing. That has happened three times on this release line: gitleaks failing on develop, lint-format timing out inside its apt.llvm.org install (#471), and gitleaks again after an unpinned upstream bump (the commit before this one). Each time the absence of signal was indistinguishable from success. ci-gate needs every required job, runs `if: always()` so it executes even when they skip, and fails unless each reports exactly `success`. failure, cancelled and skipped are all not-success, because for a required job they are. publish-emulator is deliberately excluded: it is workflow_dispatch-only and is legitimately skipped on every push and pull_request. Point branch protection at this job rather than the individual ones. It is the only check whose green means "the entire graph ran and passed". Verified against the two shapes that actually occurred, plus the obvious ones: all success -> PASS one skipped -> FAIL one cancelled -> FAIL one failure -> FAIL python-integration-tests cancelled-> FAIL (today's release/7.14.2) lint-format cancelled + 2 skipped -> FAIL (the #471 run) Consequence, stated up front: release/7.14.2 will now report RED until #466 is fixed, because python-integration-tests is killed at its 30-minute timeout on every run. That is the honest state of the branch and has been all along; this job only stops it being hidden. Closes #471
BitHighlander
changed the title
This was referenced Aug 17, 2026
BitHighlander
added a commit
that referenced
this pull request
Aug 17, 2026
…riant The integration suite is red, the quickest route to green is to stop showing a screen, and the commit that does it looks like a test fix. Writing the rule down is the cheapest defence against that, because the pressure arrives disguised as housekeeping and the result would look like progress. Tests adapt to newly required disclosure. Firmware disclosure is never removed to satisfy a stale test. If a test fails because the device now shows more, the test is wrong. The extra THORChain screens disclose affiliate-fee bytes that were previously signed invisibly. Also replaces the known-red section, which was wrong in two ways. It said the integration stall was pre-existing at 4406197 -- a controlled run (same test files, only firmware varying) showed 1af2ffe passes and head hangs, so this release introduced it. And it could not enumerate the failures at all, because the job was killed before producing JUnit XML. With #477 the suite completes in 134s and the list is finite: 22 failed, 362 passed, 47 skipped, every failure a stale pinned expectation and none a firmware defect. Recorded as a table with an owner per row, since that is now the concrete work list for the repin. Adds the two measurement traps that cost real time here: do not run the suite sequentially against one emulator, and treat the PINNED submodule commit as the test source of truth rather than the working tree -- reading master's copy of test__sign_transformERC20 produced a confident and wrong "this breaks nothing". Refs #466 #474 #477
BitHighlander
added a commit
that referenced
this pull request
Aug 20, 2026
The emulator test script detected the firmware version with:
FW_VERSION=$(sed -n '/^project/,/)/p' CMakeLists.txt \
| grep -oP '\d+\.\d+\.\d+' || echo "7.14.0")
`grep -oP` is a GNU extension. This container's grep is BusyBox, which has no
-P, so that command ALWAYS failed and the fallback silently supplied 7.14.0 --
on the 7.14.2 release branch. CI has been reporting
"Detected FW_VERSION=7.14.0 from CMakeLists.txt" while CMakeLists says 7.14.2.
Everything downstream keys off that number. SECTIONS entries are selected by
ver_ge(fw_version, min_fw), so at 7.14.0 every test gated to 7.14.1 or later was
excluded from BOTH the screenshot filter and validate_junit. That is the root
cause of the gap #487 set out to fix: the suites this release changed captured
no screens because the report system did not believe those tests existed yet.
It also means the per-test screenshot audit added in #487 passed vacuously --
with no 7.14.2 sections active, nothing declared screens, so nothing could be
missing. The audit was correct; it was being asked about the wrong release.
Two changes:
- read VERSION with sed alone, portable to BusyBox
- FAIL if it cannot be read, rather than defaulting. A wrong version here is
invisible and silently narrows what CI checks -- exactly the failure mode
ci-gate (#474) and the screenshot audit were added to prevent.
Same class as those two: the check ran, went green, and was measuring something
other than what its name implied.
BitHighlander
added a commit
that referenced
this pull request
Aug 21, 2026
…at all
The alpha<-develop merge left .gitleaks.toml with BOTH allowlist forms — alpha's
three `[[allowlists]]` and develop's one `[allowlist]`. gitleaks refuses that
outright:
FTL Failed to load config
error="[allowlist] is deprecated, it cannot be used alongside [[allowlists]]"
secret-scan is a stage-1 gate, so its failure SKIPPED the entire build graph:
build-emulator, build-arm-firmware, unit-tests, python-integration-tests,
python-dylib-tests and generate-test-report all reported "skipped", and the run
looked like one red job rather than a release with no evidence behind it. This
is the silent-skip class #474 added the aggregate `CI gate` for, and that gate
did its job — it failed and named all seven.
Fix is presentational only: develop's `[allowlist]` becomes a fourth
`[[allowlists]]` entry. No path, regex or rule changed, and neither side's
exemptions are dropped — alpha's three are narrow (path AND regex, for the AES
test vectors, ed25519 type names and the rc21 doc pin) and develop's is broad
path scoping (U2F attestation artifacts, deps/, docs/, tests/) with its
rationale intact.
Verified locally with gitleaks 8.30.0, CI's exact command:
gitleaks detect --source . --verbose --redact
-> 7586 commits scanned, no leaks found, exit 0
And the negative control develop's own comment claims, re-run because a merge
is exactly when an exemption silently widens — a fresh prime256v1 key planted in
two places, JSON report:
private-key -> lib/firmware/NEGCTRL_planted.pem CAUGHT
(same key under deps/) exempt, as documented
Neither gate in tools/ could have caught this. merge_symbol_gate.py reads C
symbols; merge_direction_gate.py only flags a file taken verbatim from one side,
and this file was genuinely merged — the union was syntactically valid TOML and
semantically invalid to the tool that consumes it. Config files merged from both
sides need the consuming tool run against them, not a diff read.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI hardening in two independently revertable commits. Together they make the pipeline deterministic and structurally incapable of hiding a required-job failure.
Commit 1 —
52a78b525scanner determinismsecret-scanwent red across the entire repository with no change to it, taking the whole build and test graph down.Run
32075116969on #472: failed in 9s, everything downstream SKIPPED. Reproduced locally with CI's exact command — 12 findings, none from this release:tests/test_ecies.pytests/test_msg_eos_signtx.pytests/test_msg_ethereum_erc20_signtx_exchange.pygit rev-list 1af2ffe7de..HEADintersected with the findings: 0.The flagged lines are published BIP32 test vectors — WIF keys printed beside their own address and pubkey as documentation:
Root cause:
GITLEAKS_VERSIONresolved fromreleases/latest. A newergeneric-api-keyrule started flagging historical fixtures. Nothing here changed.Two fixes. Allowlist
^tests/— that path does not exist in the working tree (the python suite lived at the repo root before becomingdeps/python-keepkey), but CI scans withfetch-depth: 0. Identical situation and remedy to the existing^deps/exemption, and scoped by path rather than regex because the findings are real key material by format. And pin gitleaks to 8.30.0.The cost is written into the config rather than left implicit: a genuine secret under
tests/would not be caught, which is acceptable only while the directory doesn't exist — if it returns, the exemption goes in the same commit.Negative-controlled both directions:
lib/firmware/leaks found: 1)tests/gitleaks detect --source . --verbose --redact→no leaks found, exit 0.Commit 2 —
4e8d74667the aggregate gate (closes #471)A skipped job is not a red check. When a Stage-1 gate fails, GitHub marks the downstream graph skipped, the summary shows green ticks on whatever finished, and the absence of signal is indistinguishable from success.
Three occurrences on this release line: gitleaks on develop,
lint-formattiming out inside itsapt.llvm.orginstall (#471), and gitleaks again (commit 1 above).ci-gateneeds every required job, runsif: always(), and fails unless each reports exactlysuccess.failure,cancelledandskippedare all not-success — for a required job, they are.publish-emulatoris excluded asworkflow_dispatch-only.Verified against the shapes that actually occurred:
python-integration-testscancelled — today's branchlint-formatcancelled + 2 skipped — the #471 runPoint branch protection at
CI gaterather than the individual jobs. It is the only check whose green means the entire graph ran and passed.Consequence, stated up front
release/7.14.2will report RED after this merges, becausepython-integration-testsis killed at its 30-minute timeout on every run (#466). That has been the branch's true state all along — this PR only stops it being hidden. #466 becomes the unambiguous next blocker to a genuinely green graph.This PR's own
ci-gatewill therefore also be red, for exactly that reason. That is the gate working, not failing.