Skip to content

Commit 02a5383

Browse files
committed
Port pinned-key check to cargo xtask and fix consistency bugs
Replace the Python pinned-key consistency check with a Rust `cargo xtask verify-pinned-keys` command, keeping the repo all-Rust. The port fixes an unsound equality proxy: the old check used whole-file substring containment, so a wrong pinned constant passed whenever the canonical key body appeared anywhere else in the file (e.g. the NEXT rotation slot). The command now extracts each named pinned constant's own PEM body (shell, PowerShell, and Rust embeddings) and compares it for equality against the canonical docs/keys file, plus the CI signing key cross-check. Close the CI gate hole by adding docs/keys/** to the heavy paths filter so a canonical-key-only change still runs the check, and repoint every CI call site to the xtask command. Teach `cargo xtask verify` to read the public key from ROCM_CLI_SIGNING_PUBLIC_KEY_PEM when no --public-key is given, so release_readiness.py no longer materializes a temp key file. Also quote the openssl path arguments in install.ps1 and drop a stale doc reference from the workflow comments. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>
1 parent 757ef28 commit 02a5383

11 files changed

Lines changed: 677 additions & 378 deletions

File tree

.github/workflows/ci.yml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ jobs:
9494
- '**/*.sh'
9595
- '**/*.ps1'
9696
- 'install*'
97+
# Pinned-key consistency compares docs/keys/* against the installers,
98+
# so a canonical-key-only change must trigger the heavy job that runs
99+
# `cargo xtask verify-pinned-keys`.
100+
- 'docs/keys/**'
97101
- '.github/workflows/**'
98102
# THIRD_PARTY_NOTICES.txt staleness gate: anything that changes the
99103
# dependency tree, the cargo-about config/template, the generator, or
@@ -171,8 +175,8 @@ jobs:
171175
- name: Pinned key consistency (self-test + repo check)
172176
if: needs.changes.outputs.heavy == 'true'
173177
run: |
174-
python scripts/verify_pinned_keys.py --self-test
175-
python scripts/verify_pinned_keys.py
178+
cargo xtask verify-pinned-keys --self-test
179+
cargo xtask verify-pinned-keys
176180
177181
- name: Acceptance install lifecycle
178182
if: needs.changes.outputs.heavy == 'true'
@@ -297,8 +301,8 @@ jobs:
297301
if: needs.changes.outputs.heavy == 'true'
298302
shell: pwsh
299303
run: |
300-
python .\scripts\verify_pinned_keys.py --self-test
301-
python .\scripts\verify_pinned_keys.py
304+
cargo xtask verify-pinned-keys --self-test
305+
cargo xtask verify-pinned-keys
302306
303307
- name: Check PowerShell installer syntax
304308
if: needs.changes.outputs.heavy == 'true'

.github/workflows/nightly.yml

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ env:
1616
# validate_production_trust() and behavior is unchanged. The values are public
1717
# keys and paths (not the private signing key, which stays step-scoped). When
1818
# enabling the gate, the recipe-index inputs also require a step that
19-
# materializes the hosted index + its .sig into the workspace. See
20-
# plans/release-production-trust-rollout.md.
19+
# materializes the hosted index + its .sig into the workspace.
2120
ROCM_CLI_REQUIRE_PRODUCTION_TRUST: ${{ vars.ROCM_CLI_REQUIRE_PRODUCTION_TRUST }}
2221
ROCM_CLI_SIGNING_PUBLIC_KEY_PEM: ${{ secrets.ROCM_CLI_SIGNING_PUBLIC_KEY_PEM }}
2322
ROCM_CLI_METADATA_PUBLIC_KEY_PEM: ${{ secrets.ROCM_CLI_METADATA_PUBLIC_KEY_PEM }}
@@ -36,8 +35,10 @@ jobs:
3635
steps:
3736
- uses: actions/checkout@v6
3837

38+
- uses: actions-rust-lang/setup-rust-toolchain@v1
39+
3940
- name: Verify pinned keys match the signing key
40-
run: python scripts/verify_pinned_keys.py
41+
run: cargo xtask verify-pinned-keys
4142

4243
- name: Check for changes since last nightly
4344
id: check
@@ -196,11 +197,11 @@ jobs:
196197
steps:
197198
- uses: actions/checkout@v6
198199

200+
- uses: actions-rust-lang/setup-rust-toolchain@v1
201+
199202
- name: Verify pinned keys match the signing key
200203
shell: pwsh
201-
run: python .\scripts\verify_pinned_keys.py
202-
203-
- uses: actions-rust-lang/setup-rust-toolchain@v1
204+
run: cargo xtask verify-pinned-keys
204205

205206
- name: Build release binaries
206207
shell: pwsh

.github/workflows/release.yml

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ env:
2020
# validate_production_trust() and behavior is unchanged. The values are public
2121
# keys and paths (not the private signing key, which stays step-scoped). When
2222
# enabling the gate, the recipe-index inputs also require a step that
23-
# materializes the hosted index + its .sig into the workspace. See
24-
# plans/release-production-trust-rollout.md.
23+
# materializes the hosted index + its .sig into the workspace.
2524
ROCM_CLI_REQUIRE_PRODUCTION_TRUST: ${{ vars.ROCM_CLI_REQUIRE_PRODUCTION_TRUST }}
2625
ROCM_CLI_SIGNING_PUBLIC_KEY_PEM: ${{ secrets.ROCM_CLI_SIGNING_PUBLIC_KEY_PEM }}
2726
ROCM_CLI_METADATA_PUBLIC_KEY_PEM: ${{ secrets.ROCM_CLI_METADATA_PUBLIC_KEY_PEM }}
@@ -37,8 +36,10 @@ jobs:
3736
steps:
3837
- uses: actions/checkout@v6
3938

39+
- uses: actions-rust-lang/setup-rust-toolchain@v1
40+
4041
- name: Verify pinned keys match the signing key
41-
run: python scripts/verify_pinned_keys.py
42+
run: cargo xtask verify-pinned-keys
4243

4344
- name: Determine version
4445
id: version
@@ -180,11 +181,11 @@ jobs:
180181
steps:
181182
- uses: actions/checkout@v6
182183

184+
- uses: actions-rust-lang/setup-rust-toolchain@v1
185+
183186
- name: Verify pinned keys match the signing key
184187
shell: pwsh
185-
run: python .\scripts\verify_pinned_keys.py
186-
187-
- uses: actions-rust-lang/setup-rust-toolchain@v1
188+
run: cargo xtask verify-pinned-keys
188189

189190
- name: Build release binaries
190191
shell: pwsh

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

install.ps1

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,9 @@ function Confirm-ArchiveSignature {
180180

181181
Confirm-Command openssl
182182
foreach ($key in $PublicKeyPaths) {
183-
& openssl dgst -sha256 -verify $key -signature $SignaturePath $ArchivePath | Out-Null
183+
# Quote every path argument so keys or archives under a directory with
184+
# spaces are passed to native openssl as single arguments.
185+
& openssl dgst -sha256 -verify "$key" -signature "$SignaturePath" "$ArchivePath" | Out-Null
184186
if ($LASTEXITCODE -eq 0) {
185187
return
186188
}

scripts/release_readiness.py

Lines changed: 32 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import subprocess
1818
import sys
1919
import tarfile
20-
import tempfile
2120
import zipfile
2221
from pathlib import Path
2322

@@ -334,12 +333,22 @@ def validate_archive_contents(archive: Path) -> list[str]:
334333
raise ReadinessError(f"unsupported archive extension: {archive.name}")
335334

336335

337-
def verify_signature(archive: Path, signature: Path, public_key: Path) -> None:
338-
if not public_key.is_file():
336+
def verify_signature(archive: Path, signature: Path, public_key: Path | None) -> None:
337+
"""Verify ``archive`` against ``public_key`` via ``cargo xtask verify``.
338+
339+
When ``public_key`` is ``None`` the ``--public-key`` flag is omitted and
340+
``cargo xtask verify`` reads the key from ``ROCM_CLI_SIGNING_PUBLIC_KEY_PEM``
341+
(the inline PEM release/nightly CI wire from the signing-key secret), so no
342+
temporary key file has to be materialized here.
343+
"""
344+
if public_key is not None and not public_key.is_file():
339345
raise ReadinessError(f"public key does not exist: {public_key}")
340346
cargo = shutil.which("cargo")
341347
if cargo is None:
342348
raise ReadinessError("cargo is required for signature verification")
349+
key_args = (
350+
["--public-key", str(public_key.resolve())] if public_key is not None else []
351+
)
343352
# Resolve to absolute paths because the subprocess runs from the repo root
344353
# (so the `cargo xtask` alias resolves); relative paths would otherwise be
345354
# interpreted against the repo root rather than the caller's working directory.
@@ -348,8 +357,7 @@ def verify_signature(archive: Path, signature: Path, public_key: Path) -> None:
348357
cargo,
349358
"xtask",
350359
"verify",
351-
"--public-key",
352-
str(public_key.resolve()),
360+
*key_args,
353361
"--in",
354362
str(archive.resolve()),
355363
"--signature",
@@ -374,6 +382,7 @@ def validate_archive(
374382
*,
375383
require_signatures: bool,
376384
public_key: Path | None,
385+
verify_with_env_key: bool = False,
377386
require_rocm_asset_names: bool,
378387
) -> list[str]:
379388
messages: list[str] = []
@@ -400,15 +409,16 @@ def validate_archive(
400409
)
401410
messages.append(f"checksum ok: {archive.name}")
402411

412+
verify = public_key is not None or verify_with_env_key
403413
signature = Path(f"{archive}.sig")
404-
if require_signatures or public_key is not None:
414+
if require_signatures or verify:
405415
if not signature.is_file():
406416
raise ReadinessError(f"missing signature sidecar: {signature}")
407417
if signature.stat().st_size <= 0:
408418
raise ReadinessError(f"signature sidecar is empty: {signature}")
409419
messages.append(f"signature present: {archive.name}.sig")
410420

411-
if public_key is not None:
421+
if verify:
412422
verify_signature(archive, signature, public_key)
413423
messages.append(f"signature verified: {archive.name}")
414424

@@ -464,33 +474,6 @@ def require_existing_env_path(name: str) -> Path:
464474
return path
465475

466476

467-
def resolve_release_public_key() -> Path:
468-
"""Materialize the release signing public key from ROCM_CLI_SIGNING_PUBLIC_KEY_PEM
469-
into a temp file so archives can be verified against it. Raises if it is not set.
470-
471-
Verifying against this key proves the artifact CI is about to publish was signed
472-
by the private half of the key that installers will pin as a trust root, instead
473-
of merely confirming a key input exists — a mismatched pair would otherwise pass
474-
the gate and ship releases that installers reject once default-on verification
475-
lands. The env value MUST be the same public key pinned in install.sh/.ps1.
476-
477-
Only the inline PEM form is accepted (that is what release/nightly CI wire from
478-
the signing-key secret); to verify against a key *file* on disk, pass its path
479-
with the --public-key flag instead.
480-
"""
481-
pem = env_text("ROCM_CLI_SIGNING_PUBLIC_KEY_PEM")
482-
if pem is None:
483-
raise ReadinessError(
484-
"production trust requires the release signing public key: set "
485-
"ROCM_CLI_SIGNING_PUBLIC_KEY_PEM (or pass --public-key)"
486-
)
487-
with tempfile.NamedTemporaryFile(
488-
"w", suffix=".pem", delete=False, encoding="ascii"
489-
) as handle:
490-
handle.write(pem if pem.endswith("\n") else f"{pem}\n")
491-
return Path(handle.name)
492-
493-
494477
def validate_production_trust() -> list[str]:
495478
"""Validate only explicit owner-provided production trust inputs."""
496479

@@ -539,11 +522,13 @@ def validate_release(
539522
assets: list[str],
540523
require_signatures: bool,
541524
public_key: Path | None,
525+
verify_with_env_key: bool = False,
542526
require_production_trust: bool,
543527
require_rocm_asset_names: bool,
544528
require_exact_assets: bool,
545529
) -> list[str]:
546530
archives = discover_archives(dist, assets)
531+
verify = public_key is not None or verify_with_env_key
547532
messages: list[str] = []
548533
if require_exact_assets:
549534
if not assets:
@@ -552,7 +537,7 @@ def validate_release(
552537
validate_exact_dist_assets(
553538
dist,
554539
archives,
555-
expect_signatures=require_signatures or public_key is not None,
540+
expect_signatures=require_signatures or verify,
556541
)
557542
)
558543
for archive in archives:
@@ -561,6 +546,7 @@ def validate_release(
561546
archive,
562547
require_signatures=require_signatures,
563548
public_key=public_key,
549+
verify_with_env_key=verify_with_env_key,
564550
require_rocm_asset_names=require_rocm_asset_names,
565551
)
566552
)
@@ -869,29 +855,6 @@ def run_self_test(root: Path) -> None:
869855
)
870856
print("release readiness self-test: valid production trust inputs accepted")
871857

872-
# resolve_release_public_key(): the PEM env input is materialized to a temp
873-
# file, and a missing input is an error. This is the key CI verifies release
874-
# archives against under production trust.
875-
pem_text = "-----BEGIN PUBLIC KEY-----\nself-test\n-----END PUBLIC KEY-----"
876-
resolved_pem = run_with_env(
877-
{"ROCM_CLI_SIGNING_PUBLIC_KEY_PEM": pem_text},
878-
resolve_release_public_key,
879-
)
880-
if resolved_pem.read_text(encoding="ascii") != f"{pem_text}\n":
881-
raise ReadinessError(
882-
"resolve_release_public_key did not materialize the PEM input"
883-
)
884-
resolved_pem.unlink()
885-
print("release readiness self-test: release public key resolution ok")
886-
887-
expect_failure(
888-
"production trust without a release public key",
889-
lambda: run_with_env(
890-
{"ROCM_CLI_SIGNING_PUBLIC_KEY_PEM": None},
891-
resolve_release_public_key,
892-
),
893-
)
894-
895858
expect_failure(
896859
"missing explicit asset",
897860
lambda: validate_release(
@@ -1011,18 +974,28 @@ def main() -> None:
1011974
# trust; otherwise opportunistic: if a signing public key is present in the
1012975
# environment (release/nightly wire it from the secret), verify against it.
1013976
# An explicit --public-key still wins; with neither, behavior is unchanged.
977+
#
978+
# When relying on the environment key, `cargo xtask verify` reads it directly
979+
# from ROCM_CLI_SIGNING_PUBLIC_KEY_PEM, so no temp key file is materialized here.
1014980
public_key = args.public_key
981+
verify_with_env_key = False
1015982
try:
1016983
if public_key is None and (
1017984
require_production_trust
1018985
or env_text("ROCM_CLI_SIGNING_PUBLIC_KEY_PEM") is not None
1019986
):
1020-
public_key = resolve_release_public_key()
987+
if env_text("ROCM_CLI_SIGNING_PUBLIC_KEY_PEM") is None:
988+
raise ReadinessError(
989+
"production trust requires the release signing public key: set "
990+
"ROCM_CLI_SIGNING_PUBLIC_KEY_PEM (or pass --public-key)"
991+
)
992+
verify_with_env_key = True
1021993
messages = validate_release(
1022994
Path(args.dist),
1023995
assets=args.asset,
1024996
require_signatures=require_signatures,
1025997
public_key=public_key,
998+
verify_with_env_key=verify_with_env_key,
1026999
require_production_trust=require_production_trust,
10271000
require_rocm_asset_names=args.require_rocm_asset_names,
10281001
require_exact_assets=args.require_exact_assets,

0 commit comments

Comments
 (0)