Repository navigation
[3264 by Claude Opus 5.5] Add protected signing-key verification without creating release assets - #3265
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…ted Preview workflow. The full related suite passes with real GnuPG 2.2.40 and RPM 4.18: 53 of 54 pass and none fail. The one skip is the existing root-only acceptance-evidence export test, which is unrelated to signing and was also skipped in #3263. The verify-key check itself can't succeed in tests, because that needs the real stored secret. The tests prove the logic with disposable keys and prove that the real pinned key fails against a disposable private key. Workflow (`.github/workflows/desktop-linux-signed-preview.yml`) - `verify-key` is a new option on the existing `operation` input. The identity job adds one branch for it, which requires `unsigned_run_id` and `signing_run_id` to be empty. `source_revision` still goes through the existing 40-hex and main-reachable checks, and is used only as metadata in the receipt. - The new `verify-key` job runs only for a main-dispatched `workflow_dispatch` with that operation, after the identity and `verify-helpers` jobs. - It reuses the `desktop-linux-rpm-signing` environment, the `PROPR_DESKTOP_RPM_SIGNING_AUTHORIZED=1` check and the existing secret and variable names. Its only permission is `contents: read`. - It checks out only `github.sha` and confirms `git rev-parse HEAD` matches it. It installs only `gnupg`. - It never downloads anything or checks out the frozen source, and it runs no package, release or publish step. - The two secrets appear in exactly one step's environment, and no action receives them. - `sign-draft`, `publish-draft` and their gates are unchanged. - On success the receipt goes to the run summary and to the artifact `propr-desktop-linux-signing-key-verification-<run_id>`. Script (`apps/desktop/scripts/signing-key-verification.mjs`) - Secrets: it reads both secrets and deletes them from its environment before any subprocess starts. The environment fingerprint must equal `OWNER_FINGERPRINT` exactly. - Signing: it reuses the existing `signMetadata` and `verifyMetadata` (built on `withKey` and the two-minute bounded `run`). - Steps: in a fresh 0700 home it applies the existing RSA, expiry and revocation checks to the public key. Next it imports the private key and requires exactly one secret primary key with the full fingerprint. It then makes a SHA256 detached signature of a fixed challenge text, passing the passphrase through a 0600 file. - Independent verification: the signature is verified twice, each in a new public-only home: once with the public key from the environment variable, and once with the reviewed export `docs/static/keys/propr-desktop-release.asc`. - Cleanup: all temporary homes sit under one owned directory. Before reporting success, it confirms that directory is empty and that no GnuPG agent it started is still running. It never inspects or signals any other process. - Output: a short fixed error message on failure, and no receipt. On success the receipt contains the run ID, the control SHA, the full fingerprint, digests of the challenge and signature, and the passed checks. Tests - `signing-key-verification.test.mjs` (new, disposable keys only): - a correct pair succeeds. - each of these fails: wrong passphrase, wrong or multi-key private key, public-key or pinned-export mismatch, expired key, revoked key, and malformed credentials. - tampered, missing, empty, truncated, SHA1, wrong-key and altered-challenge signatures are rejected. - leftover homes and agents that won't stop make cleanup fail. - the receipt rejects bad run IDs, SHAs and fingerprints. - The CLI tests: - the strict production pin rejects other, lowercase, short and missing fingerprints. - the real pinned public key with a disposable private key is a mismatch. - a recording fake `gpg` shows that no secret reaches a subprocess's environment or arguments, and no receipt is written on any failure. - `linux-preview-workflow.test.mjs` now checks the new job's trust boundary and that nothing in it builds, signs or publishes. The existing secrets-count assertion is now checked per job (2 in `sign`, 2 in `verify-key`, 0 elsewhere); none were relaxed. - The new test runs in the `verify-helpers` gate (before the credentialed job) and in PR validation (`desktop-release-guard.yml`). Docs: `docs/docs/operations/linux-rpm-signing.md` has a new section with the operator dispatch command (`gh workflow run desktop-linux-signed-preview.yml --ref main -f operation=verify-key -f source_revision=…`), what to approve, how to download and check the receipt, and what fails. The required test command now includes the new test file. The container had no GnuPG or RPM tools, so I unpacked the Debian packages into `/tmp/tools` without installing anything system-wide. I did not run `verify-key`, touch real secrets or settings, or change any permissions or environment rules. This change has nothing visual, so I made no preview.
|
CI failed: Full Test Suite Shard 4/4 Please investigate and fix this CI failure.
Failure evidence
|
|
ℹ️ Analyzed the follow-up request by @propr-dev[bot] Analysis SummaryThis failure isn't caused by the PR's code, so I made no changes. Re-running the check should clear it. What failed: Shard 4/4 never got as far as running tests. It failed while installing dependencies ( Why it isn't this branch:
Local check: I ran the branch's two test files, Re-run the Full Test Suite (or the whole CI set, since everything else was cancelled) on There are no visual changes, so I didn't create any previews. No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR adds a narrowly scoped ✅ Protected credential boundary — The new job requires a main dispatch, successful identity and helper gates, the existing protected environment, and explicit signing authorization. Secrets are confined to one reviewed script step. ✅ Independent verification — The challenge is signed through the existing exact-primary SHA256 helper, then verified against the reviewed public export in a separate temporary home. Cleanup must complete before a receipt is written. ✅ Focused regression coverage — Added tests cover credential mismatches, invalid signatures, production-pin enforcement, subprocess secret scrubbing, cleanup failures, and workflow isolation. This assessment is a static review of the supplied diff and context; no commands were run. The original issue’s acceptance criteria are explicitly truncated, so requirements beyond the visible excerpt could not be assessed. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe implementation preserves the existing release paths and provides substantial coverage of the new verification boundary. The supplied current-head status has no failures, with 11 checks still pending; verification of the actual protected secret pair remains the intended post-merge operation. Score: 9/10 🤖 Review Details
🤖 Review by ProPR |
|
CI failed: Validate unsigned darwin-arm64 package Please investigate and fix this CI failure.
Failure evidence
|
|
✅ Applied the requested follow-up changes in commit 6f50dae Summary of ChangesI fixed the failure. Five of the new signing-key verification tests failed on the macOS runner, and the fix is to run them only on Linux, which is where the Why they failed. The log shows
The macOS runner has The change is in Testing:
🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…on tests failed on the macOS runner, and the fix is to run them only on Linux, which is where the `verify-key` job runs. The workflow and the verification script are unchanged. I fixed the failure. Five of the new signing-key verification tests failed on the macOS runner, and the fix is to run them only on Linux, which is where the `verify-key` job runs. The workflow and the verification script are unchanged. **Why they failed.** The log shows `# fail 5` in "Typecheck and test unsigned desktop runtime", all from `apps/desktop/scripts/signing-key-verification.test.mjs`: - **Four tests** could not create their disposable keys: `gpg-agent` failed to start (`can't connect to the gpg-agent: IPC connect call failed`). macOS temp paths under `/var/folders/...` are too long for GnuPG's agent socket. - **The cleanup test** failed because the cleanup check reads `/proc`, which macOS doesn't have. The check correctly refuses to report success when it can't confirm cleanup. The macOS runner has `gpg` installed, so these tests ran there instead of skipping as they do on runners without it. The `verify-key` job itself only runs on `ubuntu-24.04`. **The change** is in `apps/desktop/scripts/signing-key-verification.test.mjs`. On anything other than Linux, the five tests that need GnuPG or `/proc` now skip with the reason "Requires the Linux verify-key runner". On Linux nothing changes: they still run, and `PROPR_REQUIRE_RPM_TESTS=1` still fails the run if `gpg` or `gpgconf` is missing. The receipt-binding test doesn't need either, so it still runs everywhere. **Testing:** - **Linux, real GnuPG 2.2.40:** all 6 tests pass, none skipped. The container has no `gpg`, so I extracted the Debian packages and ran them through small wrappers that point at the extracted agent. - **Simulated macOS** (platform forced to `darwin`): 1 passes, 5 skip, none fail. - **Not yet confirmed:** this hasn't been run on a real macOS runner. CI on the next commit will confirm it. PR: #3265 Comment by: @propr-dev[bot] (ID: 6270557043) Model: Claude Opus 5.5
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR adds a narrowly scoped credential-verification operation without introducing a release-publication path. It is merge-ready within the visible scope, conditional on pending checks completing successfully. ✅ Protected credential boundary — The new job requires a main dispatch, successful identity/helper gates, the existing protected environment, and explicit signing authorization. Secrets appear only in the reviewed verification step. ✅ Independent cryptographic verification — The implementation signs a fixed challenge, verifies through the existing public-key helper, and independently verifies against the checked-in export. Receipt creation follows successful verification and cleanup confirmation. ✅ Focused regression coverage — Added tests cover credential mismatches, invalid signatures, strict production pins, subprocess secret scrubbing, and cleanup failures. Existing sign/publish operation conditions and publication gates remain intact. This assessment uses static inspection of the supplied diff and dependency excerpts; no commands were run. The original issue’s acceptance text is explicitly truncated, so requirements beyond the visible portion could not be assessed. The supplied diff is complete. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Scope cleanup assertions per invocationThe new tests scan all processes and temporary directories matching ScoreThe implementation preserves the relevant security boundaries and provides substantial targeted coverage, with no verified merge blocker. The authoritative status reports zero failed checks and 19 pending checks; verification of the actual stored secret pair remains the protected operation’s purpose. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
1 participant
AI Implementation Summary
Closes #3264
Branch:
3264/claude-opus-5-5-add-protected-signing-key-20261019-1449-t1yCommits: ✅ Changes committed (8eb16a6)
AI Processing Completed
Execution Details:
Summary:
I've added the
verify-keyoperation to the Desktop Linux Authenticated Preview workflow. The full related suite passes with real GnuPG 2.2.40 and RPM 4.18: 53 of 54 pass and none fail. The one skip is the existing root-only acceptance-evidence export test, which is unrelated to signing and was also skipped in #3263. The verify-key check itself can't succeed in tests, because that needs the real stored secret. The tests prove the logic with disposable keys and prove that the real pinned key fails against a disposable private key.Workflow (
.github/workflows/desktop-linux-signed-preview.yml)verify-keyis a new option on the existingoperationinput. The identity job adds one branch for it, which requiresunsigned_run_idandsigning_run_idto be empty.source_revisionstill goes through the existing 40-hex and main-reachable checks, and is used only as metadata in the receipt.verify-keyjob runs only for a main-dispatchedworkflow_dispatchwith that operation, after the identity andverify-helpersjobs.desktop-linux-rpm-signingenvironment, thePROPR_DESKTOP_RPM_SIGNING_AUTHORIZED=1check and the existing secret and variable names. Its only permission iscontents: read.github.shaand confirmsgit rev-parse HEADmatches it. It installs onlygnupg.sign-draft,publish-draftand their gates are unchanged.propr-desktop-linux-signing-key-verification-<run_id>.Script (
apps/desktop/scripts/signing-key-verification.mjs)OWNER_FINGERPRINTexactly.signMetadataandverifyMetadata(built onwithKeyand the two-minute boundedrun).docs/static/keys/propr-desktop-release.asc.Tests
signing-key-verification.test.mjs(new, disposable keys only):gpgshows that no secret reaches a subprocess's environment or arguments, and no receipt is written on any failure.linux-preview-workflow.test.mjsnow checks the new job's trust boundary and that nothing in it builds, signs or publishes. The existing secrets-count assertion is now checked per job (2 insign, 2 inverify-key, 0 elsewhere); none were relaxed.verify-helpersgate (before the credentialed job) and in PR validation (desktop-release-guard.yml).Docs:
docs/docs/operations/linux-rpm-signing.mdhas a new section with the operator dispatch command (gh workflow run desktop-linux-signed-preview.yml --ref main -f operation=verify-key -f source_revision=…), what to approve, how to download and check the receipt, and what fails. The required test command now includes the new test file.The container had no GnuPG or RPM tools, so I unpacked the Debian packages into
/tmp/toolswithout installing anything system-wide. I did not runverify-key, touch real secrets or settings, or change any permissions or environment rules. This change has nothing visual, so I made no preview.Detailed Logs:
3a9a632f-763b-4c39-8066-3c343440a115Log files stored at:
/usr/src/app/logs/executions/issue-3264-2026-10-19T15-00-36-399Z-conversation.json/usr/src/app/logs/executions/issue-3264-2026-10-19T15-00-36-399Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3264.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!