Skip to content

Guard release executable signing - #579

Merged
shanselman merged 1 commit into
masterfrom
scott/release-signing-guard
May 29, 2026
Merged

Guard release executable signing#579
shanselman merged 1 commit into
masterfrom
scott/release-signing-guard

Conversation

@shanselman

Copy link
Copy Markdown
Collaborator

Summary

  • Add a release payload executable signing verifier.
  • Stop recursively signing every .exe in release payloads.
  • Stage hardlinks only for OpenClaw-owned EXEs before Azure Trusted Signing:
    • OpenClaw.Tray.WinUI.exe
    • SetupEngine\OpenClaw.SetupEngine.UI.exe
  • Explicitly classify tools\mxc\<arch>\wxc-exec.exe as third-party/excluded.
  • Add workflow contract tests.

Validation

  • ./build.ps1
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore (2023 passed / 29 skipped)
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore (863 passed)
  • verifier script parse/fake-payload checks
  • git diff --check

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed May 28, 2026, 9:43 PM ET / 01:43 UTC.

Summary
The PR changes release CI to stage only OpenClaw-owned executables for Azure Trusted Signing, adds a PowerShell payload signing-policy verifier, and adds tray workflow contract tests.

Reproducibility: yes. Source inspection of current main shows the existing release workflow recursively signs executable payloads on tag builds, which is the behavior this PR changes.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 3 files changed, +200/-6. The diff touches release CI, adds a PowerShell verifier, and adds workflow contract tests.
  • Release signing blocks: 2 Trusted Signing executable blocks changed. Both changed blocks run only for tag/release signing paths that normal PR CI does not fully exercise.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted tag/release workflow logs or terminal output showing Trusted Signing signed the two staged OpenClaw executables and the verifier passed against the real payload.
  • [P1] Confirm the fail-closed executable allowlist is the intended release policy before merge.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports tests and fake-payload checks, but it does not show after-fix evidence from a real tag/release signing setup; add redacted workflow logs, terminal output, or a linked artifact and redact private endpoints, secrets, IPs, and other sensitive details before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Tag-only Azure Trusted Signing paths are changed, and ordinary non-tag CI will not prove that the hardlinked staging folders are signed in place or that the verifier passes on real signed artifacts.
  • [P1] The verifier intentionally fail-closes on any new executable in the payload, so maintainers should confirm this executable allowlist is the desired release policy before merge.
  • [P1] This changes the release trust boundary from recursively signing all payload executables to signing only two OpenClaw-owned executables while excluding wxc-exec.exe.

Maintainer options:

  1. Require release-signing proof (recommended)
    Ask for redacted tag/release workflow logs or terminal output showing Trusted Signing signed the two staged executables and the verifier passed against a real payload before merge.
  2. Accept with release-owner watch
    Maintainers can intentionally merge after accepting that the next tag run is the first full proof of the new signing path and should be watched closely.
  3. Revise the allowlist first
    If more payload executables are expected, update the verifier allowlist and workflow contract tests before landing this policy.

Next step before merge

  • [P1] Needs human review because the remaining blockers are maintainer acceptance of the release signing policy and contributor-provided real behavior proof, not an automatable code repair.

Security
Cleared: The diff narrows code-signing inputs and adds a release payload verifier; I found no new secret exposure, dependency source change, or third-party code execution path beyond the existing Azure signing actions.

Review details

Best possible solution:

Land the hardening after maintainers confirm the executable allowlist and the contributor adds redacted tag/release workflow evidence showing the staged OpenClaw executables are signed and the third-party executable remains excluded.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection of current main shows the existing release workflow recursively signs executable payloads on tag builds, which is the behavior this PR changes.

Is this the best way to solve the issue?

Yes, the staged signing input plus fail-closed verifier is a narrow maintainable direction. The remaining question is maintainer acceptance of the signing policy and proof from a real release-signing path.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 281656fc017e.

Label changes

Label changes:

  • add P2: This is a normal-priority release signing hardening with limited runtime blast radius but meaningful release-process impact.
  • add merge-risk: 🚨 security-boundary: The PR changes the code-signing trust boundary by limiting which executable payloads receive the OpenClaw release signature.
  • add merge-risk: 🚨 automation: The PR changes tag-only release signing workflow behavior that green non-tag CI cannot fully validate.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports tests and fake-payload checks, but it does not show after-fix evidence from a real tag/release signing setup; add redacted workflow logs, terminal output, or a linked artifact and redact private endpoints, secrets, IPs, and other sensitive details before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a normal-priority release signing hardening with limited runtime blast radius but meaningful release-process impact.
  • merge-risk: 🚨 automation: The PR changes tag-only release signing workflow behavior that green non-tag CI cannot fully validate.
  • merge-risk: 🚨 security-boundary: The PR changes the code-signing trust boundary by limiting which executable payloads receive the OpenClaw release signature.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports tests and fake-payload checks, but it does not show after-fix evidence from a real tag/release signing setup; add redacted workflow logs, terminal output, or a linked artifact and redact private endpoints, secrets, IPs, and other sensitive details before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Current recursive signing behavior: Current main signs every .exe recursively under publish for x64 tag builds and under artifacts/tray-win-arm64 for ARM64 release artifacts. (.github/workflows/ci.yml:399, 281656fc017e)
  • PR narrows signing input: The PR diff replaces recursive signing with hardlinked signing-input and signing-input-arm64 folders, then runs a verifier against the original payload folders. (.github/workflows/ci.yml:396, 1e4609925674)
  • Verifier allowlist: The added verifier classifies only OpenClaw.Tray.WinUI.exe, SetupEngine\OpenClaw.SetupEngine.UI.exe, and tools\mxc\<arch>\wxc-exec.exe; unknown executables fail closed. (scripts/Test-ReleaseExecutableSignatures.ps1:44, 1e4609925674)
  • Published third-party executable path: Current main copies wxc-exec.exe into $(PublishDir)tools\mxc\$(MxcArch)\, matching the verifier's third-party exclusion target. (src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj:213, 281656fc017e)
  • Signing workflow provenance: Blame ties the current signing blocks to e67f5c2f6e1cc26ad3f5bc61f4a38e66f19229e7, with recursive nested executable signing added by aaf50b75850424837f8caf6ef0c26623afd05900. (.github/workflows/ci.yml:399, e67f5c2f6e1c)
  • Reported validation and proof gap: The PR body reports the required build and test commands plus fake-payload verifier checks, but it does not include a real tag/release signing run or redacted Trusted Signing/verifier output against an actual release payload. (1e4609925674)

Likely related people:

  • shanselman: Git history shows Scott Hanselman authored the current signing workflow area and the recent SetupEngine/MXC packaging changes that this PR adjusts. (role: recent release workflow contributor; confidence: high; commits: e67f5c2f6e1c, aaf50b758504, fa3cf0f164c6; files: .github/workflows/ci.yml, src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj, tests/OpenClaw.Tray.Tests/InstallerIssAssertionTests.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels May 29, 2026
@shanselman
shanselman merged commit 528f625 into master May 29, 2026
25 checks passed
@shanselman
shanselman deleted the scott/release-signing-guard branch May 29, 2026 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant