Check the aarch64 target in CI - #15
Conversation
The Raspberry Pi 5 is aarch64 and nothing had ever compiled this crate for it. Every other job builds x86-64, and scripts/pi/bootstrap.sh warns that a non-aarch64 host is not Raspberry Pi evidence, so the target the product ships on was the one target no check covered. A linker or target-library failure would have surfaced with the hardware on the desk, at the same native-host step that a missing default-run key had already broken. Adds a cross job that installs the target and gcc-aarch64-linux-gnu, runs clippy with -D warnings over all targets, links vcg-host, and asserts the artifact really is an ARM aarch64 ELF rather than a silently native build. Verified locally before adding it: check, clippy, and link are all clean for aarch64, and the resulting binary runs under qemu-user with `vcg-host doctor` reporting `target: aarch64-linux`. The job deliberately does not run cargo test. Under qemu-user the suite fails 17 of 393, and every failure sits in child-process supervision or journal file locking rather than in crypto, parsing, or catalog logic. A test that re-executes itself cannot exec an aarch64 child on an x86-64 kernel, and registering a binfmt handler inside WSL2 did not change it. Those failures are consistent with emulation limits but are not proven to be only that; emulated results would be noise rather than evidence. The suite runs natively on the Pi during bring-up, which is where that question gets answered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe CI workflow adds an Ubuntu job for aarch64 cross-compilation. The job installs the Rust target and linker, runs Clippy, builds ChangesARM64 CI validation
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/workflows/ci.yml:
- Around line 167-168: Configure the protected branch rules to require the exact
status-check context “native / aarch64 cross” after the workflow’s first report,
using the job name defined by native-aarch64. Ensure ARM64 validation is treated
as a blocking required check.
- Line 173: Update the actions/checkout@v4 step in the native-aarch64 job to set
persist-credentials to false, ensuring the checkout token is not stored in
.git/config.
- Line 173: Update all GitHub Action references in the workflow, including
actions/checkout, actions/setup-node, actions/upload-artifact, and
Swatinem/rust-cache, to reviewed immutable 40-character commit SHA pins instead
of mutable `@v4` or `@v2` tags. Preserve each action’s existing version and
configuration.
- Around line 167-169: Add a job-level permissions block to the native-aarch64
job, alongside its name and runs-on settings, granting only contents: read. Do
not add permissions for the cache action or alter other workflow jobs.
- Around line 199-202: Update the “Artifact really is aarch64” workflow step to
make its file-type assertion require all three markers: ELF, executable, and ARM
aarch64. Replace the current grep condition while preserving the existing
artifact path and validation command flow.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: dd2ba4c8-e45c-4f43-af2e-0c79f5ac28cf
📒 Files selected for processing (1)
.github/workflows/ci.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: native / windows-latest
- GitHub Check: node / ubuntu-latest
- GitHub Check: e2e / windows-latest
- GitHub Check: e2e / ubuntu-latest
- GitHub Check: node / windows-latest
- GitHub Check: pi bring-up script
- GitHub Check: native / aarch64 cross
🧰 Additional context used
🪛 zizmor (1.28.0)
.github/workflows/ci.yml
[warning] 173-178: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 167-209: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 173-173: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 185-185: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
170-171: LGTM!Also applies to: 175-183, 186-197, 204-209
| native-aarch64: | ||
| name: native / aarch64 cross |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial
Make this status check required after its first report.
After the first run reports, add the exact context native / aarch64 cross to the protected branch's required status checks. Otherwise, ARM64 validation remains non-blocking.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 167-209: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 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 @.github/workflows/ci.yml around lines 167 - 168, Configure the protected
branch rules to require the exact status-check context “native / aarch64 cross”
after the workflow’s first report, using the job name defined by native-aarch64.
Ensure ARM64 validation is treated as a blocking required check.
| native-aarch64: | ||
| name: native / aarch64 cross | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
repo="$(gh repo view --json nameWithOwner -q .nameWithOwner)"
gh api "repos/${repo}/actions/permissions" \
--jq '.default_workflow_permissions'Repository: Randroids-Dojo/VCG-Console
Length of output: 404
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- workflow section ---'
cat -n .github/workflows/ci.yml | sed -n '1,230p'
printf '%s\n' '--- permission and action references ---'
rg -n '^[[:space:]]*permissions:|^[[:space:]]+[A-Za-z0-9_-]+:[[:space:]]*(read|write|none)|uses:' .github/workflows/ci.ymlRepository: Randroids-Dojo/VCG-Console
Length of output: 10224
🌐 Web query:
GitHub Actions permissions contents read actions/cache save required permissions
💡 Result:
Using the official actions/cache action does not require you to explicitly configure permissions: actions: write in your workflow file. The action itself manages the necessary communication with the GitHub Actions service [1][2]. However, there are important restrictions on when a workflow is permitted to save a cache: 1. Permission to Save: Some workflow triggers, such as those initiated by pull requests from forks or specific events like pull_request_target, issue_comment, and workflow_run, are granted only read-only access to the cache [3]. These runs can restore existing caches but cannot create or overwrite them [3]. 2. Behavior when Read-Only: If a workflow run is restricted to read-only access, the actions/cache (and actions/cache/save) action will not fail the job if a save is attempted [1][2]. Instead, it will emit a warning in the logs (e.g., "Failed to save:... cache write denied:...") and complete the step successfully without performing a write [1][2][4]. 3. Security Context: GitHub automatically enforces these cache access restrictions to prevent "cache poisoning," where unauthorized users might write malicious caches that could be restored and executed by more privileged workflows [3]. If you are using the gh CLI's native gh cache command directly (rather than the actions/cache action), you may need to grant the actions: write permission to your workflow token to perform cache operations [5]. For standard usage of the actions/cache GitHub Action, no extra permissions are required [1][2].
Citations:
- 1: https://github.com/actions/cache/?tab=readme-ov-file
- 2: https://github.com/actions/cache/blob/main/README.md
- 3: https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching
- 4: https://github.com/actions/cache
- 5: Document that
actions: read/writepermissions are required to use gh-actions-cache in gh actions CI actions/gh-actions-cache#85
Declare least-privilege permissions for this job.
Set the job-level permission boundary to contents: read. This prevents repository or workflow defaults from granting unnecessary write access. The cache action does not require an additional permission.
permissions:
contents: read🧰 Tools
🪛 zizmor (1.28.0)
[warning] 167-209: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 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 @.github/workflows/ci.yml around lines 167 - 169, Add a job-level permissions
block to the native-aarch64 job, alongside its name and runs-on settings,
granting only contents: read. Do not add permissions for the cache action or
alter other workflow jobs.
Source: Linters/SAST tools
| env: | ||
| CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER: aarch64-linux-gnu-gcc | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 \
'actions/checkout@|persist-credentials|git (fetch|push|clone)' \
.github/workflows/ci.ymlRepository: Randroids-Dojo/VCG-Console
Length of output: 1588
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Raspberry Pi job ---'
sed -n '160,215p' .github/workflows/ci.yml
printf '%s\n' '--- Git and checkout references in the workflow ---'
rg -n -C 3 \
'actions/checkout@|persist-credentials|(^|[[:space:]])git([[:space:]]|$)|GITHUB_TOKEN|github.token|GH_TOKEN' \
.github/workflows/ci.ymlRepository: Randroids-Dojo/VCG-Console
Length of output: 3972
Disable checkout credential persistence.
The native-aarch64 job has no later authenticated Git operation. Set persist-credentials: false on actions/checkout@v4 to prevent the token from being stored in .git/config.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 173-178: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 173-173: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yml at line 173, Update the actions/checkout@v4 step in
the native-aarch64 job to set persist-credentials to false, ensuring the
checkout token is not stored in .git/config.
Source: Linters/SAST tools
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
if rg -nP '^\s*-\s*uses:\s*[^@\s]+@(?![0-9a-fA-F]{40}(?:\s|$))\S+' \
.github/workflows/ci.yml; then
echo "Found an action reference that is not pinned to a full commit SHA." >&2
exit 1
fiRepository: Randroids-Dojo/VCG-Console
Length of output: 660
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- workflow context ---'
sed -n '160,190p' .github/workflows/ci.yml
printf '%s\n' '--- all action references ---'
rg -n '^\s*-\s*uses:' .github/workflows/ci.yml
printf '%s\n' '--- repository policy references ---'
rg -n -i 'pin|sha|checkout|rust-cache|action' .github .github* 2>/dev/null || true
printf '%s\n' '--- current tag targets ---'
for spec in 'actions/checkout:tags/v4' 'Swatinem/rust-cache:tags/v2'; do
repo=${spec%%:*}
ref=${spec#*:}
printf '%s\n' "$repo $ref"
curl -fsSL "https://api.github.com/repos/$repo/git/ref/$ref" |
jq -r '.object | "type=\(.type) sha=\(.sha)"'
doneRepository: Randroids-Dojo/VCG-Console
Length of output: 4946
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
resolve_tag() {
local repo=$1 tag=$2
local ref_json object_type object_sha commit_sha
ref_json=$(curl -fsSL "https://api.github.com/repos/${repo}/git/ref/tags/${tag}")
object_type=$(jq -r '.object.type' <<<"$ref_json")
object_sha=$(jq -r '.object.sha' <<<"$ref_json")
if [ "$object_type" = tag ]; then
commit_sha=$(curl -fsSL \
"https://api.github.com/repos/${repo}/git/tags/${object_sha}" |
jq -r '.object.sha')
else
commit_sha=$object_sha
fi
curl -fsSL "https://api.github.com/repos/${repo}/commits/${commit_sha}" |
jq -r --arg repo "$repo" --arg tag "$tag" --arg type "$object_type" \
'{repo: $repo, tag: $tag, ref_object_type: $type, commit: .sha}'
}
resolve_tag actions/checkout v4
resolve_tag actions/setup-node v4
resolve_tag actions/upload-artifact v4
resolve_tag Swatinem/rust-cache v2Repository: Randroids-Dojo/VCG-Console
Length of output: 713
Pin every GitHub Action to a full commit SHA.
Replace the mutable @v4 and @v2 tags throughout .github/workflows/ci.yml, including lines 173 and 185, with reviewed 40-character commit SHAs. The workflow contains mutable references to actions/checkout, actions/setup-node, actions/upload-artifact, and Swatinem/rust-cache.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 173-178: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 173-173: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yml at line 173, Update all GitHub Action references in
the workflow, including actions/checkout, actions/setup-node,
actions/upload-artifact, and Swatinem/rust-cache, to reviewed immutable
40-character commit SHA pins instead of mutable `@v4` or `@v2` tags. Preserve each
action’s existing version and configuration.
Source: Linters/SAST tools
| - name: Artifact really is aarch64 | ||
| run: | | ||
| file target/aarch64-unknown-linux-gnu/debug/vcg-host | ||
| file target/aarch64-unknown-linux-gnu/debug/vcg-host | grep -q 'ARM aarch64' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- workflow excerpt ---'
sed -n '180,210p' .github/workflows/ci.yml
printf '%s\n' '--- repository references ---'
rg -n --glob '!target/**' 'Artifact really is aarch64|vcg-host|aarch64-unknown-linux-gnu' .Repository: Randroids-Dojo/VCG-Console
Length of output: 26026
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY' |
import struct
import sys
# Emit minimal ELF64 little-endian headers for AArch64 files with different
# ELF file types. The bytes are sent through stdin; no repository files are
# created or executed.
for name, elf_type in (("relocatable", 1), ("executable", 2), ("shared-object", 3)):
header = bytearray(64)
header[:4] = b"\x7fELF"
header[4] = 2 # ELFCLASS64
header[5] = 1 # ELFDATA2LSB
header[6] = 1 # EV_CURRENT
struct.pack_into("<H", header, 16, elf_type)
struct.pack_into("<H", header, 18, 183) # EM_AARCH64
struct.pack_into("<I", header, 20, 1)
sys.stdout.buffer.write(header)
sys.stdout.buffer.flush()
# Separate each input for the shell loop.
sys.stdout.buffer.write(b"\n")
PY
while IFS= read -r line; do
printf '%s\n' "$line"
doneRepository: Randroids-Dojo/VCG-Console
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -e
command -v file
printf '%s\n' '--- representative file(1) descriptions ---'
for type in relocatable executable shared-object; do
python3 - "$type" <<'PY' | file -b -
import struct
import sys
elf_type = {"relocatable": 1, "executable": 2, "shared-object": 3}[sys.argv[1]]
header = bytearray(64)
header[:4] = b"\x7fELF"
header[4:7] = bytes((2, 1, 1))
struct.pack_into("<H", header, 16, elf_type)
struct.pack_into("<H", header, 18, 183) # EM_AARCH64
struct.pack_into("<I", header, 20, 1)
sys.stdout.buffer.write(header)
PY
done
printf '%s\n' '--- predicate exit status ---'
for type in relocatable executable shared-object; do
output="$(
python3 - "$type" <<'PY' | file -b -
import struct
import sys
elf_type = {"relocatable": 1, "executable": 2, "shared-object": 3}[sys.argv[1]]
header = bytearray(64)
header[:4] = b"\x7fELF"
header[4:7] = bytes((2, 1, 1))
struct.pack_into("<H", header, 16, elf_type)
struct.pack_into("<H", header, 18, 183)
struct.pack_into("<I", header, 20, 1)
sys.stdout.buffer.write(header)
PY
)"
current=0
proposed=0
printf '%s\n' "$output" | grep -q 'ARM aarch64' && current=1 || :
printf '%s\n' "$output" | grep -Eq 'ELF .*executable.*ARM aarch64' && proposed=1 || :
printf '%s current=%s proposed=%s\n' "$type" "$current" "$proposed"
doneRepository: Randroids-Dojo/VCG-Console
Length of output: 164
🌐 Web query:
file(1) output examples AArch64 ELF executable relocatable ARM aarch64
💡 Result:
The file(1) utility identifies AArch64 ELF files by analyzing their headers and reporting characteristics such as file type (relocatable, executable, shared object), architecture, endianness, and linking status [1][2][3]. For an AArch64 ELF relocatable object file (typically a.o file), the output typically appears as: hello.o: ELF 64-bit LSB relocatable, ARM aarch64, version 1 (SYSV), not stripped [4][2][3] For an AArch64 ELF executable, the output typically appears as: hello: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=..., for GNU/Linux 3.7.0, not stripped [3] Key components of this output include: - ELF 64-bit: Indicates the 64-bit Executable and Linkable Format [2][3]. - LSB: Indicates Least Significant Byte (little-endian) byte order [2][3]. - relocatable: Identifies the file type ET_REL, common for object files that have not yet been linked into an executable [2][3]. - executable (or pie executable): Identifies the file type ET_EXEC or ET_DYN, indicating a binary ready for execution [2][3]. - ARM aarch64: Specifies the machine architecture [1][5]. - version 1 (SYSV): The ELF version [2][3]. - dynamically/statically linked: Describes how the binary handles library dependencies [3][6].
Citations:
- 1: https://github.com/file/file/blob/4264364d4a46d632ceb095e8cef56339f592931d/magic/Magdir/elf
- 2: https://duetorun.com/blog/20230627/elf-obj-tour/
- 3: https://noise.getoto.net/2023/11/17/how-to-execute-an-object-file-part-4-aarch64-edition/
- 4: https://stackoverflow.com/questions/77622611/gcc-errors-on-aarch64-rk3568-soc-board-from-buildroot
- 5: https://github.com/file/file/blob/master/magic/Magdir/elf
- 6: https://github.com/joebobmiles/ARMv8ViaLinuxCommandline
Assert the complete artifact type.
A relocatable AArch64 ELF also contains ARM aarch64, so the current check can pass it. Require ELF, executable, and ARM aarch64.
Proposed artifact check
- file target/aarch64-unknown-linux-gnu/debug/vcg-host | grep -q 'ARM aarch64'
+ file -b target/aarch64-unknown-linux-gnu/debug/vcg-host | grep -Eq 'ELF .*executable.*ARM aarch64'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Artifact really is aarch64 | |
| run: | | |
| file target/aarch64-unknown-linux-gnu/debug/vcg-host | |
| file target/aarch64-unknown-linux-gnu/debug/vcg-host | grep -q 'ARM aarch64' | |
| - name: Artifact really is aarch64 | |
| run: | | |
| file target/aarch64-unknown-linux-gnu/debug/vcg-host | |
| file -b target/aarch64-unknown-linux-gnu/debug/vcg-host | grep -Eq 'ELF .*executable.*ARM aarch64' |
🤖 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 @.github/workflows/ci.yml around lines 199 - 202, Update the “Artifact really
is aarch64” workflow step to make its file-type assertion require all three
markers: ELF, executable, and ARM aarch64. Replace the current grep condition
while preserving the existing artifact path and validation command flow.
The Raspberry Pi 5 is aarch64 and nothing had ever compiled this crate for it. Every other job builds x86-64, and
scripts/pi/bootstrap.shwarns that a non-aarch64 host is not Raspberry Pi evidence — so the target the product actually ships on was the one target no check covered.A linker or target-library failure would have surfaced with the hardware on the desk, at the same native-host step a missing
default-runkey had already broken.The job
Installs the target and
gcc-aarch64-linux-gnu, then:clippy --all-targets --target aarch64 -- -D warningscargo build --target aarch64 -p vcg-hostcargo checkcannot provefile ... | grep -q 'ARM aarch64'Verified before adding it
cargo check— clean for aarch64, whole crypto stack (ed25519/curve25519-dalek, sha2, rustix, fs4, tar)clippy -D warnings— cleanELF 64-bit LSB pie executable, ARM aarch64vcg-host doctorreportstarget: aarch64-linuxWhy there is no
cargo testhereUnder qemu-user the suite fails 17 of 393, and every failure sits in child-process supervision or journal file locking — none in crypto, parsing, or catalog logic. The one I traced re-executes itself as a child, which cannot exec an aarch64 binary on an x86-64 kernel; registering a binfmt handler inside WSL2 did not change it.
That is consistent with emulation limits, but I did not prove it is only that. Emulated results would be noise rather than evidence, so the job omits them and the comment in the workflow says why. The suite runs natively on the Pi during bring-up, which is where the question actually gets answered.
Follow-up
Once this job has reported once, its context needs adding to the branch-protection required checks, or it is advisory only. I will do that after merge.
🤖 Generated with Claude Code