Add T3W1 emulator certificates#6917
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughReplaces shell-based certificate header generation with a new Python CLI Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Make as Makefile
participant Gen as core/tools/generate_certificates.py
participant FS as Filesystem
Dev->>Make: make certs / make gen
Make->>Gen: run script (with or without --check)
Gen->>FS: read DER files (optiga certs + secret cert)
Gen->>Gen: format DER -> C byte arrays + <name>_size constants
alt check mode
Gen->>FS: compare generated content to existing headers
FS-->>Gen: match / mismatch
Gen-->>Make: exit non-zero on mismatch
else write mode
Gen->>FS: write/update header files
FS-->>Gen: write success
Gen-->>Make: success
end
Make-->>Dev: report success or failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
| model | device_test | click_test | persistence_test |
|---|---|---|---|
| T2T1 | test(all) main(all) ![]() |
test(all) main(all) ![]() |
test(all) main(all) ![]() |
| T3B1 | test(all) main(all) ![]() |
test(all) main(all) ![]() |
test(all) main(all) ![]() |
| T3T1 | test(all) main(all) ![]() |
test(all) main(all) ![]() |
test(all) main(all) ![]() |
| T3W1 | test(all) main(all) ![]() |
test(all) main(all) ![]() |
test(all) main(all) ![]() |
Latest CI run: 25859031075
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/embed/sec/secret/unix/certs/prodtest.h`:
- Line 1: The declaration static unsigned char mcu_device_cert[] = {}; uses an
empty braced initializer which is non-portable in C11/C17; change it to use an
explicit size and a valid initializer (e.g. static unsigned char
mcu_device_cert[1] = {0};) or, if you intend no data, make it a pointer
initialized to NULL (static unsigned char *mcu_device_cert = NULL;), updating
any code that assumes array semantics; locate the mcu_device_cert symbol to
apply the fix.
In `@core/embed/sec/secret/unix/secret.c`:
- Around line 212-223: In secret_mcu_device_cert_write, silence unused-parameter
warnings in the non-PRODTEST (`#else`) branch by explicitly casting the parameters
to void (i.e., add (void)cert and (void)cert_size) before returning secfalse;
this preserves the function signature and documents intent without changing
behavior.
In `@core/tools/generate_certificates.py`:
- Around line 11-13: Fix the typo in the generated banner string: update the
COMMENT constant in generate_certificates.py so the third line reads "in the
root directory" (replace "direcotry" with "directory"), ensuring you preserve
the existing f-string interpolation using Path(__file__).relative_to(ROOT) and
the rest of the message verbatim.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 450a3733-395f-4852-b68d-78b6286d1919
⛔ Files ignored due to path filters (3)
tests/tropic_model/root_cert.pemis excluded by!**/*.pemtests/tropic_model/root_key.pemis excluded by!**/*.pemtests/tropic_model/tropic_key.pemis excluded by!**/*.pem
📒 Files selected for processing (19)
Makefilecommon/hsm_keys.jsoncore/embed/projects/prodtest/cmd/hsm_keys.hcore/embed/sec/optiga/unix/certs/T2B1.hcore/embed/sec/optiga/unix/certs/T3B1.hcore/embed/sec/optiga/unix/certs/T3T1.hcore/embed/sec/optiga/unix/certs/T3W1.hcore/embed/sec/optiga/unix/certs/gen.shcore/embed/sec/secret/unix/certs/T3W1.dercore/embed/sec/secret/unix/certs/T3W1.hcore/embed/sec/secret/unix/certs/gen.shcore/embed/sec/secret/unix/certs/prodtest.hcore/embed/sec/secret/unix/secret.ccore/tools/generate_certificates.pycore/tools/generate_tropic_model_config.pypython/src/trezorlib/_root_keys.pytests/tropic_model/README.mdtests/tropic_model/base_config.ymltests/tropic_model/config.yml
💤 Files with no reviewable changes (2)
- core/embed/sec/secret/unix/certs/gen.sh
- core/embed/sec/optiga/unix/certs/gen.sh
d5183f3 to
58cc3c4
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 202-210: The Makefile targets certs, certs_check, gen, and
gen_check are not declared phony so make may skip running their recipes if files
or dirs with those names exist; add a .PHONY declaration listing these targets
(e.g., .PHONY: certs certs_check gen gen_check) near the top or grouped with
other phony targets so make always executes the recipes for the certs,
certs_check, gen, and gen_check targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56f1d0f3-f394-4eb8-98a8-d4276e1f7385
⛔ Files ignored due to path filters (3)
tests/tropic_model/root_cert.pemis excluded by!**/*.pemtests/tropic_model/root_key.pemis excluded by!**/*.pemtests/tropic_model/tropic_key.pemis excluded by!**/*.pem
📒 Files selected for processing (20)
Makefilecommon/hsm_keys.jsoncore/embed/projects/prodtest/cmd/hsm_keys.hcore/embed/projects/prodtest/cmd/prodtest_secrets.ccore/embed/sec/optiga/unix/certs/T2B1.hcore/embed/sec/optiga/unix/certs/T3B1.hcore/embed/sec/optiga/unix/certs/T3T1.hcore/embed/sec/optiga/unix/certs/T3W1.hcore/embed/sec/optiga/unix/certs/gen.shcore/embed/sec/secret/unix/certs/T3W1.dercore/embed/sec/secret/unix/certs/T3W1.hcore/embed/sec/secret/unix/certs/gen.shcore/embed/sec/secret/unix/certs/prodtest.hcore/embed/sec/secret/unix/secret.ccore/tools/generate_certificates.pycore/tools/generate_tropic_model_config.pypython/src/trezorlib/_root_keys.pytests/tropic_model/README.mdtests/tropic_model/base_config.ymltests/tropic_model/config.yml
💤 Files with no reviewable changes (2)
- core/embed/sec/secret/unix/certs/gen.sh
- core/embed/sec/optiga/unix/certs/gen.sh
✅ Files skipped from review due to trivial changes (4)
- core/embed/sec/secret/unix/certs/prodtest.h
- tests/tropic_model/base_config.yml
- core/embed/sec/optiga/unix/certs/T3B1.h
- core/embed/sec/secret/unix/certs/T3W1.h
🚧 Files skipped from review as they are similar to previous changes (7)
- core/embed/sec/secret/unix/secret.c
- core/embed/sec/optiga/unix/certs/T3W1.h
- core/tools/generate_certificates.py
- python/src/trezorlib/_root_keys.py
- core/tools/generate_tropic_model_config.py
- tests/tropic_model/config.yml
- core/embed/sec/optiga/unix/certs/T3T1.h
58cc3c4 to
59299e6
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
[no changelog]
[no changelog]
[no changelog]
[no changelog]
[no changelog]
f7e9c8d to
5420374
Compare
|
I’m waiting for #6816 to be merged. Then I'll resolve the merge conflicts in |




































No description provided.