feat: sign-firmware action (Ed25519, Phase 1)#7
Conversation
Adds workflow-templates action that signs Intel HEX firmware images with Ed25519 and patches the 64-byte signature into a fixed region at app_end - 68. Leaves the last 4 bytes (CRC region) untouched so the existing CRC step runs unchanged after signing. Key resolution: FIRMWARE_SIGNING_PRIVATE_KEY env var (CI), then --private-key-file, with a --allow-dummy-key fallback that uses a committed non-secret keypair under tests/ for local pipeline runs. Includes 10 pytest cases covering roundtrip, determinism, tamper detection on payload and signature, CRC-region preservation, sparse hex padding, and all three CLI key-resolution paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds two new GitHub composite actions and supporting tooling. actions/sign-firmware: composite action metadata and entrypoint (action.yml, run.sh), a Python CLI (actions/sign-firmware/sign_firmware.py) that signs an Intel HEX image with Ed25519 and writes a 64-byte signature into the trailer, pytest tests (actions/sign-firmware/tests/test_sign_firmware.py), and a .gitleaksignore entry for the test private key. actions/extract-memory-map: composite action (action.yml) and a C extractor (actions/extract-memory-map/extract.c) that emits memory-map symbols. Also updates actions/build-stm32 to accept/forward a private-key input and adds python3-cryptography/python3-intelhex to its Dockerfile. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,3 @@ | |||
| -----BEGIN PRIVATE KEY----- | |||
There was a problem hiding this comment.
🛑 Gitleaks has detected a secret with rule-id private-key in commit 6f4c6ff.
If this secret is a true positive, please rotate the secret ASAP.
If this secret is a false positive, you can add the fingerprint below to your .gitleaksignore file and commit the change to this branch.
echo 6f4c6ffaf316f20ecc539f8e437ea9ce629e5cda:actions/sign-firmware/tests/dummy_private_key.pem:private-key:1 >> .gitleaksignore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@actions/sign-firmware/action.yml`:
- Around line 29-39: Replace the floating action and loose package installs in
the "Install signing dependencies" step by pinning the GitHub Action to its full
commit SHA (instead of actions/setup-python@v5) and move the pip installs (the
lines calling "python -m pip install --upgrade pip" and "python -m pip install
"cryptography>=41" "intelhex>=2.3"") into a requirements.txt with exact versions
and SHA256 hashes; generate that requirements.txt with pip-compile
--generate-hashes from a requirements.in and update the workflow to run "python
-m pip install -r $GITHUB_ACTION_PATH/requirements.txt" so the signing runtime
is reproducible and verifiable.
In `@actions/sign-firmware/sign_firmware.py`:
- Around line 61-63: Wrap file reads that assign pem (the blocks reading
Path(args.private_key_file).read_text() and the dummy key read) in try/except
catching OSError (FileNotFoundError, PermissionError, UnicodeDecodeError) and
re-raise as SigningError with a clear message including the source variable
(e.g., args.private_key_file or the dummy key source). Likewise wrap the call to
serialization.load_pem_private_key(...) in a try/except catching ValueError,
TypeError, and cryptography.exceptions.UnsupportedAlgorithm and re-raise a
SigningError with context about which key was being loaded (use the same source
name). Ensure you reference the existing SigningError type so the main() handler
will format the error consistently.
🪄 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: 285f76ad-0039-4c69-9179-506a27c6ec7e
⛔ Files ignored due to path filters (2)
actions/sign-firmware/tests/dummy_private_key.pemis excluded by!**/*.pemactions/sign-firmware/tests/dummy_public_key.pemis excluded by!**/*.pem
📒 Files selected for processing (3)
actions/sign-firmware/action.ymlactions/sign-firmware/sign_firmware.pyactions/sign-firmware/tests/test_sign_firmware.py
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.x" | ||
|
|
||
| - name: Install signing dependencies | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| python -m pip install --upgrade pip | ||
| python -m pip install "cryptography>=41" "intelhex>=2.3" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "action.yml" -type f | head -20Repository: minimalx/.github
Length of output: 543
🏁 Script executed:
cat -n ./actions/sign-firmware/action.ymlRepository: minimalx/.github
Length of output: 2081
🏁 Script executed:
ls -la ./actions/sign-firmware/Repository: minimalx/.github
Length of output: 358
🏁 Script executed:
find . -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" | grep -v node_modulesRepository: minimalx/.github
Length of output: 42
🏁 Script executed:
rg "actions/setup-python" --type yaml | head -10Repository: minimalx/.github
Length of output: 274
🏁 Script executed:
cat ./actions/sign-firmware/sign_firmware.py | head -50Repository: minimalx/.github
Length of output: 1514
🏁 Script executed:
rg "cryptography|intelhex" ./actions/sign-firmware/ -A 2 -B 2Repository: minimalx/.github
Length of output: 1612
🌐 Web query:
GitHub Actions SHA pinning setup-python commit reference syntax best practices
💡 Result:
To set up SHA pinning for the setup-python action in GitHub Actions, use the full commit SHA instead of a tag like v4 or v5 in the 'uses' statement. This is the recommended best practice for security to prevent supply chain attacks where tags could be rewritten or repositories compromised [1][2][3]. Syntax: uses: actions/setup-python@ # vX.Y.Z Example for a pinned version (replace with current SHA): - name: Set up Python uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v4.0.0 [67] with: python-version: '3.13' cache: 'pip' Note: This SHA is from an older example; always verify the latest [4]. How to find the SHA for a tag: 1. Go to https://github.com/actions/setup-python/releases/tag/v6.2.0 (latest as of 2026-01-22 [5]). 2. Click the commit SHA next to the tag (or use git ls-remote --tags https://github.com/actions/setup-python | grep v6.2.0) [6][7]. 3. Copy the full 40-character SHA. 4. Verify it's from the official repo, not a fork [2]. 5. Add a comment like # v6.2.0 for readability [8][7]. Best practices: - Pin to full SHA (not short SHA, insecure) [2][3]. - Use tools like Hyland/github-actions-ensure-sha-pinned-actions to automate [8]. - Enable org/repo policies to enforce SHA pinning [1]. - Update periodically via Dependabot or manual checks, testing workflows after [9][7]. - For python-version input, specify explicitly (e.g., '3.13') rather than relying on PATH [10][11]. Official docs emphasize pinning actions to SHA for immutability [2][3]. Current latest is v6.2.0 [4][5].
Citations:
- 1: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 2: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions?learn=getting_started&learnProduct=actions
- 3: https://docs.github.com/enterprise-server@3.0/actions/security-guides/security-hardening-for-github-actions
- 4: https://github.com/actions/setup-python/releases
- 5: https://github.com/actions/setup-python/releases/tag/v6.2.0
- 6: https://stackoverflow.com/questions/78903499/how-do-i-pin-an-action-to-a-specific-sha
- 7: Pin all GitHub Actions to immutable SHA commits for supply chain security github/gh-aw#5691
- 8: https://github.com/Hyland/github-actions-ensure-sha-pinned-actions
- 9: Feature Request: Improve compatibility with SHA pinning best practices github/vscode-github-actions#514
- 10: https://github.com/actions/setup-python/
- 11: https://github.com/actions/setup-python?tab=readme-ov-file
🌐 Web query:
pip install --require-hashes requirements.txt Python dependency locking best practices
💡 Result:
Using pip install --require-hashes -r requirements.txt is a best practice for secure, reproducible Python dependency installations. It enables pip's hash-checking mode, which verifies cryptographic hashes (typically SHA256) for every package and its transitive dependencies before installation, protecting against tampering, supply chain attacks, network corruption, or malicious changes on PyPI [1][2][3]. This mode is all-or-nothing: all requirements must be pinned (using ==, URL, or path) and have hashes; missing any triggers an error [1][3]. To create a suitable requirements.txt: 1. List direct dependencies (with version ranges if desired) in requirements.in, e.g.: requests>=2.31.0 flask==3.0.0 2. Install pip-tools (pip install pip-tools) [4][5][6]. 3. Generate the locked file: pip-compile --generate-hashes -o requirements.txt requirements.in. This resolves all transitive dependencies, pins exact versions, adds multiple hashes per package (for different wheels), and includes # via comments showing dependency trees [4][7][8][5][9][6][10]. Example output line: requests==2.32.3 \ --hash=sha256:abc... \ --hash=sha256:def... # via -r requirements.in For installation in production/CI: pip install --require-hashes --no-deps -r requirements.txt - --no-deps prevents fetching unlisted dependencies [1][2][11]. - Optionally add --only-binary :all: to avoid source builds [1][11]. Workflow: - Edit requirements.in for changes. - Re-run pip-compile --generate-hashes (add --upgrade to update all, or --upgrade-package=pkg for specific) [5][6][12]. - Commit both .in and .txt files [13][4][14]. - Use pip-sync requirements.txt (from pip-tools) to sync environments exactly [5][12]. This ensures reproducibility across machines, detects tampering (hash mismatch errors) [15], and is recommended for deployments over plain pinning [2][7]. Avoid pip freeze > requirements.txt as it includes unrelated packages and lacks hashes [13][4]. For modern projects, consider tools like Poetry or PDM, but pip-tools remains lightweight for requirements.txt workflows [16][17][18].
Citations:
- 1: https://pip.pypa.io/en/stable/topics/secure-installs.html
- 2: https://pip.pypa.io/en/stable/topics/repeatable-installs.html
- 3: https://pip.pypa.io/en/stable/topics/secure-installs/
- 4: https://lincolnloop.com/blog/python-dependency-locking-pip-tools/
- 5: https://pip-tools.readthedocs.io/en/stable
- 6: https://github.com/jazzband/pip-tools/blob/7.3.0/README.md
- 7: https://docs.bswen.com/blog/2026-04-02-python-hash-pinning-requirements
- 8: https://pip-compile-multi.readthedocs.io/en/latest/features.html
- 9: https://calmcode.io/course/pip-tools/hashing
- 10: https://docs.bswen.com/blog/2026-04-02-python-hash-pinning-requirements/
- 11: https://www.b-list.org/weblog/2023/dec/07/pip-install-safely/
- 12: https://pydevtools.com/handbook/reference/pip-tools/index.md
- 13: https://stackoverflow.com/questions/61536466/pips-requirements-txt-best-practice
- 14: https://codegive.com/blog/pip_compile_requirements_txt.php
- 15: https://stackoverflow.com/questions/71435874/pip-these-packages-do-not-match-the-hashes-from-the-requirements-file
- 16: https://dev.to/tlaloces/pyprojecttoml-modern-python-dependency-management-1a04
- 17: https://thelinuxcode.com/managing-python-dependencies-in-2026-the-real-production-feature/
- 18: https://thelinuxcode.com/managing-python-dependencies-in-2026-a-practical-real-world-playbook/
Pin GitHub Actions and Python dependencies to ensure firmware-signing reproducibility and meet supply-chain security standards.
Floating action versions (@v5) and Python package ranges (>=41, >=2.3) can silently change the signing runtime across CI runs. Following established security practices, immutable pinning is essential for firmware provenance and traceability. Pin the action to its full commit SHA and lock Python dependencies with verified hashes.
Suggested approach
- uses: actions/setup-python@v5
+ uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.x.xCreate $GITHUB_ACTION_PATH/requirements.txt with pinned versions and SHA256 hashes (using pip-tools):
- python -m pip install "cryptography>=41" "intelhex>=2.3"
+ python -m pip install --require-hashes -r "$GITHUB_ACTION_PATH/requirements.txt"Generate hashes with: pip-compile --generate-hashes requirements.in -o requirements.txt
🤖 Prompt for 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.
In `@actions/sign-firmware/action.yml` around lines 29 - 39, Replace the floating
action and loose package installs in the "Install signing dependencies" step by
pinning the GitHub Action to its full commit SHA (instead of
actions/setup-python@v5) and move the pip installs (the lines calling "python -m
pip install --upgrade pip" and "python -m pip install "cryptography>=41"
"intelhex>=2.3"") into a requirements.txt with exact versions and SHA256 hashes;
generate that requirements.txt with pip-compile --generate-hashes from a
requirements.in and update the workflow to run "python -m pip install -r
$GITHUB_ACTION_PATH/requirements.txt" so the signing runtime is reproducible and
verifiable.
| if not pem and args.private_key_file: | ||
| pem = Path(args.private_key_file).read_text() | ||
| source = f"file {args.private_key_file}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd actions/sign-firmware && wc -l sign_firmware.pyRepository: minimalx/.github
Length of output: 81
🏁 Script executed:
cd actions/sign-firmware && head -80 sign_firmware.py | tail -30 && echo "---" && sed -n '60,85p' sign_firmware.py && echo "---" && sed -n '170,185p' sign_firmware.pyRepository: minimalx/.github
Length of output: 2672
🏁 Script executed:
cd actions/sign-firmware && cat -n sign_firmware.py | grep -A5 -B5 "def main\|SigningError\|load_pem_private_key" | head -80Repository: minimalx/.github
Length of output: 3286
🏁 Script executed:
cd actions/sign-firmware && cat -n sign_firmware.pyRepository: minimalx/.github
Length of output: 7548
🏁 Script executed:
python3 << 'EOF'
# Check what exceptions load_pem_private_key can raise
from cryptography.hazmat.primitives import serialization
# Test invalid PEM
test_cases = [
("invalid_pem", "not a valid PEM"),
("empty_pem", ""),
("wrong_format", "-----BEGIN RSA PRIVATE KEY-----\ninvalid\n-----END RSA PRIVATE KEY-----"),
]
exception_types = set()
for name, pem in test_cases:
try:
serialization.load_pem_private_key(pem.encode(), password=None)
except Exception as e:
exception_types.add(type(e).__name__)
print(f"{name}: {type(e).__module__}.{type(e).__name__}: {e}")
print(f"\nException types raised: {exception_types}")
EOFRepository: minimalx/.github
Length of output: 614
🏁 Script executed:
python3 << 'EOF'
# Check what Path.read_text can raise
from pathlib import Path
import tempfile
import os
# Test 1: Non-existent file
try:
Path("/nonexistent/file.txt").read_text()
except Exception as e:
print(f"Non-existent: {type(e).__module__}.{type(e).__name__}")
# Test 2: Permission denied
with tempfile.TemporaryDirectory() as tmpdir:
test_file = Path(tmpdir) / "test.txt"
test_file.write_text("test")
os.chmod(test_file, 0o000)
try:
test_file.read_text()
except Exception as e:
print(f"Permission denied: {type(e).__module__}.{type(e).__name__}")
finally:
os.chmod(test_file, 0o644)
# Test 3: Invalid UTF-8
with tempfile.TemporaryDirectory() as tmpdir:
test_file = Path(tmpdir) / "invalid.txt"
test_file.write_bytes(b'\x80\x81\x82')
try:
test_file.read_text()
except Exception as e:
print(f"Invalid UTF-8: {type(e).__module__}.{type(e).__name__}")
print("\nAll above inherit from OSError or ValueError")
EOFRepository: minimalx/.github
Length of output: 234
🏁 Script executed:
rg "UnsupportedAlgorithm" --type pyRepository: minimalx/.github
Length of output: 42
🏁 Script executed:
grep -n "read_text\|load_pem_private_key" actions/sign-firmware/sign_firmware.pyRepository: minimalx/.github
Length of output: 229
Wrap key-loading failures in SigningError for consistent CLI error handling.
Currently, Path.read_text() at lines 62 and 68 can raise FileNotFoundError, PermissionError, or UnicodeDecodeError (all OSError subclasses). Similarly, serialization.load_pem_private_key() at line 82 can raise ValueError, TypeError, or UnsupportedAlgorithm. These exceptions bypass the SigningError handler in main() (lines 177–179) and escape as raw tracebacks rather than the intended sign-firmware: ... format. Converting them to SigningError ensures deterministic, user-friendly error messages and maintains a robust error contract.
Proposed fix
from cryptography.exceptions import InvalidSignature
+from cryptography.exceptions import UnsupportedAlgorithm
@@
if not pem and args.private_key_file:
- pem = Path(args.private_key_file).read_text()
+ try:
+ pem = Path(args.private_key_file).read_text(encoding="utf-8")
+ except OSError as exc:
+ raise SigningError(
+ f"Unable to read private key file {args.private_key_file}: {exc}"
+ ) from exc
source = f"file {args.private_key_file}"
@@
elif not pem and args.allow_dummy_key:
dummy = Path(__file__).parent / "tests" / "dummy_private_key.pem"
if not dummy.exists():
raise SigningError(f"Dummy key not found at {dummy}")
- pem = dummy.read_text()
+ try:
+ pem = dummy.read_text(encoding="utf-8")
+ except OSError as exc:
+ raise SigningError(
+ f"Unable to read dummy private key: {exc}"
+ ) from exc
@@
- key = serialization.load_pem_private_key(pem.encode(), password=None)
+ try:
+ key = serialization.load_pem_private_key(pem.encode(), password=None)
+ except (ValueError, TypeError, UnsupportedAlgorithm) as exc:
+ raise SigningError(
+ "Invalid private key PEM. Provide an unencrypted Ed25519 private key."
+ ) from excAlso applies to: line 68 (dummy key read).
🤖 Prompt for 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.
In `@actions/sign-firmware/sign_firmware.py` around lines 61 - 63, Wrap file reads
that assign pem (the blocks reading Path(args.private_key_file).read_text() and
the dummy key read) in try/except catching OSError (FileNotFoundError,
PermissionError, UnicodeDecodeError) and re-raise as SigningError with a clear
message including the source variable (e.g., args.private_key_file or the dummy
key source). Likewise wrap the call to serialization.load_pem_private_key(...)
in a try/except catching ValueError, TypeError, and
cryptography.exceptions.UnsupportedAlgorithm and re-raise a SigningError with
context about which key was being loaded (use the same source name). Ensure you
reference the existing SigningError type so the main() handler will format the
error consistently.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Compiles a small C extractor against a caller-supplied app_bootloader_interface.h and writes the resolved slot addresses to GITHUB_ENV. Lets every ECU CI workflow derive memory-map values from the same C header the firmware uses. Exports MAIN_APP_*, MAIN_SIG_*, MAIN_CRC_ADDR, BL_APP_*, BL_SIG_*, BL_CRC_ADDR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 `@actions/extract-memory-map/action.yml`:
- Around line 27-30: Replace the fixed /tmp/_extract_memmap binary with a
securely created unique temp path (e.g., via mktemp or similar) when compiling
GITHUB_ACTION_PATH/extract.c and then execute that unique path; ensure you
capture the temp filename in a variable, run the binary via that variable, and
remove the temp binary on exit to avoid collisions and TOCTOU risks associated
with the literal '/tmp/_extract_memmap'.
🪄 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: 5c3f7da8-797d-4187-8528-738774c6bd80
📒 Files selected for processing (2)
actions/extract-memory-map/action.ymlactions/extract-memory-map/extract.c
| gcc -I "${{ inputs.include-dir }}" \ | ||
| "$GITHUB_ACTION_PATH/extract.c" \ | ||
| -o /tmp/_extract_memmap | ||
| /tmp/_extract_memmap | tee -a "$GITHUB_ENV" |
There was a problem hiding this comment.
Use a unique temp binary path instead of a fixed /tmp filename
Line 29 uses a predictable executable path (/tmp/_extract_memmap), then runs it on Line 30. This is a reliability/security footgun on shared runners (collision/TOCTOU). Secure temp-file handling standards recommend unique paths plus cleanup.
Suggested patch
set -euo pipefail
+ tmp_bin="$(mktemp /tmp/extract-memmap.XXXXXX)"
+ trap 'rm -f "$tmp_bin"' EXIT
gcc -I "${{ inputs.include-dir }}" \
"$GITHUB_ACTION_PATH/extract.c" \
- -o /tmp/_extract_memmap
- /tmp/_extract_memmap | tee -a "$GITHUB_ENV"
+ -o "$tmp_bin"
+ "$tmp_bin" | tee -a "$GITHUB_ENV"🤖 Prompt for 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.
In `@actions/extract-memory-map/action.yml` around lines 27 - 30, Replace the
fixed /tmp/_extract_memmap binary with a securely created unique temp path
(e.g., via mktemp or similar) when compiling GITHUB_ACTION_PATH/extract.c and
then execute that unique path; ensure you capture the temp filename in a
variable, run the binary via that variable, and remove the temp binary on exit
to avoid collisions and TOCTOU risks associated with the literal
'/tmp/_extract_memmap'.
…ainer Lets the .cproject post-build step sign the linked image inside the build container (so signing and CRC happen as one IDE post-build pass instead of being re-done in CI). Adds python3-cryptography and python3-intelhex to the image, plus a private-key input that gets forwarded to docker run as FIRMWARE_SIGNING_PRIVATE_KEY. Default is empty so callers that don't sign keep working unchanged; the sign script falls back to its committed dummy key when the env var is missing (local builds). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 `@actions/build-stm32/action.yml`:
- Around line 23-26: The action currently allows an empty private-key
(private-key) which enables a silent dummy-key fallback; change the action to
fail closed by making the signing key an explicit required input and/or
validating it at runtime: remove the empty default for "private-key" and set
required: true in action.yml (or add a separate explicit opt-in flag like
"allow-dummy-key" that must be true to permit the test key), and add runtime
validation in the action entrypoint to error/exit if private-key is missing or
empty; do the same for the other input entries referenced around lines 51-54 to
ensure no silent fallback is possible.
🪄 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: 5648a564-ca30-474d-9c29-2955918df170
📒 Files selected for processing (2)
actions/build-stm32/Dockerfileactions/build-stm32/action.yml
✅ Files skipped from review due to trivial changes (1)
- actions/build-stm32/Dockerfile
| private-key: | ||
| description: 'PEM-encoded Ed25519 private key for firmware signing in the post-build step. Leave empty to fall back to the dummy test key.' | ||
| required: false | ||
| default: '' |
There was a problem hiding this comment.
Fail closed when the signing key is missing.
The current default (private-key: '') plus documented dummy fallback makes the signing path fail-open in CI. If the secret is absent/miswired, the pipeline can still produce “signed” artifacts with a known test key. Secure CI coding standards usually require explicit opt-in for dummy keys and otherwise hard-fail.
Suggested hardening diff
inputs:
private-key:
- description: 'PEM-encoded Ed25519 private key for firmware signing in the post-build step. Leave empty to fall back to the dummy test key.'
+ description: 'PEM-encoded Ed25519 private key for firmware signing in the post-build step.'
required: false
default: ''
+ allow-dummy-key:
+ description: 'Allow fallback to dummy signing key (local/dev only).'
+ required: false
+ default: 'false'
@@
- name: Build firmware
shell: bash
env:
PROJECT_PATH: ${{ inputs.project-path }}
PROJECT_TARGET: ${{ inputs.project-target }}
FIRMWARE_SIGNING_PRIVATE_KEY: ${{ inputs.private-key }}
+ ALLOW_DUMMY_KEY: ${{ inputs.allow-dummy-key }}
run: |
+ if [[ -z "${FIRMWARE_SIGNING_PRIVATE_KEY}" && "${ALLOW_DUMMY_KEY}" != "true" ]]; then
+ echo "ERROR: Missing firmware signing key. Provide 'private-key' or explicitly set 'allow-dummy-key=true' for local/dev."
+ exit 1
+ fi
docker run --rm \
-v "${GITHUB_WORKSPACE}:/workspace" \
-w /workspace \
-e FIRMWARE_SIGNING_PRIVATE_KEY \
stm32cubeide-builder \
"${PROJECT_PATH}" "${PROJECT_TARGET}"Also applies to: 51-54
🤖 Prompt for 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.
In `@actions/build-stm32/action.yml` around lines 23 - 26, The action currently
allows an empty private-key (private-key) which enables a silent dummy-key
fallback; change the action to fail closed by making the signing key an explicit
required input and/or validating it at runtime: remove the empty default for
"private-key" and set required: true in action.yml (or add a separate explicit
opt-in flag like "allow-dummy-key" that must be true to permit the test key),
and add runtime validation in the action entrypoint to error/exit if private-key
is missing or empty; do the same for the other input entries referenced around
lines 51-54 to ensure no silent fallback is possible.
Adds run.sh as the single entry point for sign_firmware.py. If the caller's python3 already imports cryptography and intelhex (CI container, dev machine with deps installed) the script execs the signer directly. Otherwise it creates a per-action .venv next to the script, installs the deps once, and runs from there. This unblocks STM32CubeIDE local builds whose post-build step calls sign_firmware.py via system python3 that lacks the modules. The composite action is updated to invoke run.sh, dropping its explicit pip-install step. .gitignore added so .venv/ and __pycache__/ don't pollute consumers' submodule working trees. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
actions/sign-firmware/action.yml (1)
29-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin action ref and Python minor version for immutable signing environment.
Line 29 and Line 31 are floating (
@v5,3.x), which weakens provenance guarantees for firmware signing. Pinactions/setup-pythonto a full commit SHA and pin an explicit minor version.Suggested change
- uses: actions/setup-python@v5 + uses: actions/setup-python@<full-40-char-sha> # v5.x.x with: - python-version: "3.x" + python-version: "3.12"🤖 Prompt for 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. In `@actions/sign-firmware/action.yml` around lines 29 - 31, The workflow currently uses a floating action ref and Python version (actions/setup-python@v5 and python-version: "3.x"); replace the floating ref with a specific commit SHA for actions/setup-python (use the GitHub Actions repository commit SHA instead of `@v5`) and pin python-version to an explicit minor release (e.g., "3.11") in the action.yml to ensure an immutable, reproducible signing environment; update the uses entry and the python-version value accordingly (look for the uses: actions/setup-python@v5 and python-version: "3.x" lines to modify).actions/sign-firmware/run.sh (1)
22-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLock dependency versions for signing determinism and supply-chain safety.
Line 22 and Line 23 install floating ranges (
>=), which allows silent runtime drift in a firmware-signing path. Please install from a checked-in lock file with exact pins (preferably hash-verified) to keep signing behavior reproducible.Suggested change
- "$VENV/bin/pip" install --quiet "cryptography>=41" "intelhex>=2.3" + "$VENV/bin/pip" install --quiet --require-hashes -r "$SCRIPT_DIR/requirements.txt"And add
actions/sign-firmware/requirements.txtwith pinned versions + hashes.🤖 Prompt for 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. In `@actions/sign-firmware/run.sh` around lines 22 - 23, Replace the floating pip installs that call "$VENV/bin/pip" install --quiet "cryptography>=41" "intelhex>=2.3" with installing from a checked-in requirements.txt that pins exact package versions and hashes; create a requirements.txt in the action directory with exact versions and pip hash entries for cryptography and intelhex (and any other deps), update the script to run "$VENV/bin/pip install --require-hashes -r requirements.txt" (or equivalent pinned install command), and ensure the virtualenv bootstrap still upgrades pip before installing from the lock file.
🤖 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 `@actions/sign-firmware/action.yml`:
- Around line 40-45: The workflow currently interpolates expressions like `${{
inputs.hex-path }}`, `${{ inputs.app-start }}`, `${{ inputs.app-end }}`, and
`${{ inputs.output-path }}` directly into the shell command in action.yml (see
the args+=(...) lines); replace these direct interpolations by first mapping
each input to an environment variable (e.g., export HEX_PATH="${{
inputs.hex-path }}" etc.) in the step's env or by using the action's env
mapping, then update the shell invocation to reference the safe shell variables
(e.g., "$HEX_PATH") when building args (e.g., args+=(--hex "$HEX_PATH") and
args+=(--output "$OUTPUT_PATH")), and audit other occurrences in action.yml for
similar direct `${{ inputs.* }}` usage to harden against injection.
---
Duplicate comments:
In `@actions/sign-firmware/action.yml`:
- Around line 29-31: The workflow currently uses a floating action ref and
Python version (actions/setup-python@v5 and python-version: "3.x"); replace the
floating ref with a specific commit SHA for actions/setup-python (use the GitHub
Actions repository commit SHA instead of `@v5`) and pin python-version to an
explicit minor release (e.g., "3.11") in the action.yml to ensure an immutable,
reproducible signing environment; update the uses entry and the python-version
value accordingly (look for the uses: actions/setup-python@v5 and
python-version: "3.x" lines to modify).
In `@actions/sign-firmware/run.sh`:
- Around line 22-23: Replace the floating pip installs that call "$VENV/bin/pip"
install --quiet "cryptography>=41" "intelhex>=2.3" with installing from a
checked-in requirements.txt that pins exact package versions and hashes; create
a requirements.txt in the action directory with exact versions and pip hash
entries for cryptography and intelhex (and any other deps), update the script to
run "$VENV/bin/pip install --require-hashes -r requirements.txt" (or equivalent
pinned install command), and ensure the virtualenv bootstrap still upgrades pip
before installing from the lock file.
🪄 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: fd2e8f4e-8c09-484e-84a4-02730042fcff
📒 Files selected for processing (3)
actions/sign-firmware/.gitignoreactions/sign-firmware/action.ymlactions/sign-firmware/run.sh
✅ Files skipped from review due to trivial changes (1)
- actions/sign-firmware/.gitignore
| --hex "${{ inputs.hex-path }}" | ||
| --app-start "${{ inputs.app-start }}" | ||
| --app-end "${{ inputs.app-end }}" | ||
| ) | ||
| if [ -n "${{ inputs.output-path }}" ]; then | ||
| args+=(--output "${{ inputs.output-path }}") |
There was a problem hiding this comment.
Avoid direct expression interpolation inside shell commands.
Line 40–Line 45 interpolate ${{ inputs.* }} directly into bash source. That pattern is fragile and can become command/script injection if inputs are ever derived from untrusted workflow data. Map inputs to env vars first, then reference normal shell variables.
Suggested hardening
- name: Sign firmware
shell: bash
env:
FIRMWARE_SIGNING_PRIVATE_KEY: ${{ inputs.private-key }}
+ HEX_PATH: ${{ inputs.hex-path }}
+ APP_START: ${{ inputs.app-start }}
+ APP_END: ${{ inputs.app-end }}
+ OUTPUT_PATH: ${{ inputs.output-path }}
run: |
set -euo pipefail
args=(
- --hex "${{ inputs.hex-path }}"
- --app-start "${{ inputs.app-start }}"
- --app-end "${{ inputs.app-end }}"
+ --hex "$HEX_PATH"
+ --app-start "$APP_START"
+ --app-end "$APP_END"
)
- if [ -n "${{ inputs.output-path }}" ]; then
- args+=(--output "${{ inputs.output-path }}")
+ if [ -n "$OUTPUT_PATH" ]; then
+ args+=(--output "$OUTPUT_PATH")
fi
sh "$GITHUB_ACTION_PATH/run.sh" "${args[@]}"🤖 Prompt for 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.
In `@actions/sign-firmware/action.yml` around lines 40 - 45, The workflow
currently interpolates expressions like `${{ inputs.hex-path }}`, `${{
inputs.app-start }}`, `${{ inputs.app-end }}`, and `${{ inputs.output-path }}`
directly into the shell command in action.yml (see the args+=(...) lines);
replace these direct interpolations by first mapping each input to an
environment variable (e.g., export HEX_PATH="${{ inputs.hex-path }}" etc.) in
the step's env or by using the action's env mapping, then update the shell
invocation to reference the safe shell variables (e.g., "$HEX_PATH") when
building args (e.g., args+=(--hex "$HEX_PATH") and args+=(--output
"$OUTPUT_PATH")), and audit other occurrences in action.yml for similar direct
`${{ inputs.* }}` usage to harden against injection.
The previous .gitleaksignore entry pinned the dummy test private key by commit fingerprint, which broke whenever history was rewritten (squash merge of #7 invalidated 6f4c6ff -> 8a37048, which is what triggered the post-merge failure on main). Replace with a .gitleaks.toml that extends the default rules and allowlists the path of the committed test fixture. This stays correct across squash merges, rebases, and force-pushes because it does not reference commit hashes. .gitleaksignore is removed; .gitleaks.toml fully supersedes it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) The previous .gitleaksignore entry pinned the dummy test private key by commit fingerprint, which broke whenever history was rewritten (squash merge of #7 invalidated 6f4c6ff -> 8a37048, which is what triggered the post-merge failure on main). Replace with a .gitleaks.toml that extends the default rules and allowlists the path of the committed test fixture. This stays correct across squash merges, rebases, and force-pushes because it does not reference commit hashes. .gitleaksignore is removed; .gitleaks.toml fully supersedes it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
actions/sign-firmware/composite action that signs an Intel HEX firmware image with Ed25519 and patches a 64-byte signature into a fixed region atapp_end - 68.--allow-dummy-keyuses a committed non-secret keypair undertests/so pipelines can be exercised locally without access toFIRMWARE_SIGNING_PRIVATE_KEY.This is Phase 1 of the firmware signing rollout (TCU-Stack OTA). It is not yet wired into any ECU firmware repo — that's Phase 3+.
Image layout
Signature covers
[app_start, app_end - 68), gaps padded with0xFF(erased flash). CRC unchanged in algorithm; will now cover the signature bytes too because they're written before the CRC step.Inputs
hex-pathapp-start0x08020000app-end0x08040000output-pathprivate-key${{ secrets.FIRMWARE_SIGNING_PRIVATE_KEY }}Note on the algorithm name
Plain Ed25519 (RFC 8032) internally uses SHA-512 — the hash function isn't a parameter of the scheme. Earlier discussion mentioned "Ed25519 + SHA-256" which was inaccurate; the action just uses Ed25519 with no separate hash step (the library does it).
Test plan
python -m pytest tests/— 10/10 passing locally (roundtrip, determinism, payload tamper, signature tamper, CRC preservation, sparse-hex padding, three CLI key-resolution paths, app-slot-too-small)--allow-dummy-keyagainst a synthetic 4 KiB hex; verified offline against the dummy public keypytest tests/on PR — follow-upLocal usage
🤖 Generated with Claude Code
✨ New Features
Sign firmware
Automate memory-map extraction
🔧 Improvements
Better CI signing workflow
📚 Documentation & Resources
Tests added for firmware signing