chore: Adjust release process after II canister split#3648
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for releasing and proposing upgrades for the new internet_identity_frontend canister alongside the existing backend (internet_identity) and archive flow.
Changes:
- Extend
scripts/verify-hashto optionally verify the frontend WASM hash (internet_identity_frontend.wasm.gz). - Update
scripts/make-upgrade-proposalto support proposing backend-only, frontend-only, or combined upgrades, generating separate proposal markdown and args as needed. - Add
--frontendsupport toscripts/docker-buildand update the release-notes action to include frontend verification in the published instructions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/verify-hash | Adds optional frontend hash verification via a new CLI flag and builds frontend WASM when requested. |
| scripts/make-upgrade-proposal | Introduces upgrade-type selection and splits proposal generation/deploy steps for backend vs frontend canisters. |
| scripts/docker-build | Adds --frontend flag to build the internet_identity_frontend canister WASM via Docker. |
| .github/actions/release/run.sh | Updates generated release notes to include frontend hash verification in the verification command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aterga
changed the title
aterga
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| local upgrade_type="$(checklist_must_get 'Upgrade type')" | ||
| local cmd_parts=() | ||
|
|
||
| if should_upgrade_backend "$upgrade_type"; then | ||
| II_WASM_SHA=$(sha256sum "internet_identity.wasm.gz" | awk '{print $1}') | ||
| ARCHIVE_WASM_SHA=$(sha256sum "archive.wasm.gz" | awk '{print $1}') | ||
| cmd_parts+=("--ii-hash $II_WASM_SHA --archive-hash $ARCHIVE_WASM_SHA") | ||
| fi | ||
|
|
||
| if should_upgrade_frontend "$upgrade_type"; then | ||
| FRONTEND_WASM_SHA=$(sha256sum "internet_identity_frontend.wasm.gz" | awk '{print $1}') | ||
| cmd_parts+=("--iife-hash $FRONTEND_WASM_SHA") | ||
| fi | ||
|
|
||
| echo "Run the following command in another terminal to verify the hashes" | ||
| echo "./scripts/verify-hash --ii-hash $II_WASM_SHA --archive-hash $ARCHIVE_WASM_SHA" | ||
| echo "./scripts/verify-hash ${cmd_parts[@]}" | ||
| } |
There was a problem hiding this comment.
When Upgrade type is set to frontend, this function will print a ./scripts/verify-hash command that only includes --iife-hash. However, scripts/verify-hash currently exits if --ii-hash is not provided, so the suggested verification command will fail for frontend-only upgrades. Consider either updating scripts/verify-hash to allow verifying only the frontend (make --ii-hash optional when --iife-hash is present and skip the backend build), or ensure this script always provides --ii-hash (and downloads/computes it) even in frontend-only mode.
| ./scripts/docker-build | ||
| prod_build_sha256="$(shasum -a 256 ./internet_identity.wasm.gz | cut -d ' ' -f1)" | ||
|
|
||
| ii_match=false | ||
| iife_match=false | ||
| archive_match=false |
There was a problem hiding this comment.
verify-hash now potentially invokes ./scripts/docker-build up to three separate times (default backend build, then --frontend, then --archive). This makes local verification noticeably slower and increases the chance of inconsistent outputs if build-time inputs change between invocations. Consider computing which canisters need building based on the provided expected hashes and calling docker-build once with all required flags (it supports multiple canisters).
| git pull # to ensure you have the latest changes. | ||
| git checkout $GITHUB_SHA | ||
| ./scripts/verify-hash --ii-hash $(shasum -a 256 "$PRODUCTION_ASSET" | cut -d ' ' -f1) | ||
| ./scripts/verify-hash --ii-hash $(shasum -a 256 "$PRODUCTION_ASSET" | cut -d ' ' -f1) --iife-hash $(shasum -a 256 internet_identity_frontend.wasm.gz | cut -d ' ' -f1) |
There was a problem hiding this comment.
The release-notes verification command hard-codes internet_identity_frontend.wasm.gz without quoting and without making it clear that the file must be present in the working directory. Since this ends up in published release notes, it’s easy for users to copy/paste and hit a "file not found" or globbing issues. Consider quoting the filename and/or referencing it via an input/variable (similar to $PRODUCTION_ASSET), or explicitly stating that both WASM assets must be downloaded before running the command.
| --use-hsm \ | ||
| --key-id "01" \ | ||
| --slot 0 \ | ||
| --pin "$(read -p 'HSM PIN? '; echo "$REPLY")" \ |
There was a problem hiding this comment.
The HSM PIN is again passed via --pin on the command line, leaking this secret to process listings and logs and allowing a local attacker to recover it and misuse the HSM to submit forged governance proposals. This repeats the same issue as the backend dry-run: secrets should never be passed as CLI arguments. Use a secure input channel (e.g., an interactive prompt handled by ic-admin or another mechanism that does not expose the PIN in process arguments).
| --use-hsm \ | ||
| --key-id "01" \ | ||
| --slot 0 \ | ||
| --pin "$(read -p 'HSM PIN? '; echo "$REPLY")" \ |
There was a problem hiding this comment.
Here the HSM PIN for the real backend proposal is passed in --pin on the command line, which exposes it to any local user able to inspect running processes and to logs/history, enabling compromise of the NNS HSM. This is especially critical because this path is used for actual governance proposals, not just a dry run. Refactor this to supply the PIN through a secure, non-argument mechanism so it never appears in process arguments or shell history.
| --use-hsm \ | ||
| --key-id "01" \ | ||
| --slot 0 \ | ||
| --pin "$(read -p 'HSM PIN? '; echo "$REPLY")" \ |
There was a problem hiding this comment.
The frontend proposal path also passes the HSM PIN via --pin on the command line, leaking this secret to process listings and logs and allowing local attackers to recover it and control the HSM. This is the same high-impact issue as in the backend proposal logic. Avoid passing the PIN as a CLI argument and instead use a secure input method that keeps the PIN out of process arguments and persistent logs.
| --use-hsm \ | ||
| --key-id "01" \ | ||
| --slot 0 \ | ||
| --pin "$(read -p 'HSM PIN? '; echo "$REPLY")" \ |
There was a problem hiding this comment.
The ic-admin --pin invocation passes the HSM PIN on the command line, which exposes this secret to other local users via process listings (e.g., ps) and potentially to shell history or logs. An attacker with local access to the release engineer’s machine could capture the PIN and use the HSM to authorize arbitrary NNS canister changes. Instead, ensure the HSM PIN is provided via a secure, non-echoed input mechanism that is not placed in process arguments (for example, letting ic-admin prompt securely or using a protected file descriptor/OS key store).
Motivation
We need a release process for the new internet_identity_frontend canister (uqzsh-gqaaa-aaaaq-qaada-cai).
Changes