Skip to content

fix(cli): surface osslsigncode output when verify fails#1526

Merged
ChiragAgg5k merged 1 commit into
masterfrom
fix/verify-step-diagnostic
May 11, 2026
Merged

fix(cli): surface osslsigncode output when verify fails#1526
ChiragAgg5k merged 1 commit into
masterfrom
fix/verify-step-diagnostic

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

Problem

On the latest `sdk-for-cli` `20.2.0-rc.1` rerun, SignPath returned "The signing request was successfully processed" — so the binaries are signed — but the next step `Verify Windows signatures` fails with no diagnostic. Step log goes from `set -uo pipefail` setup to `Process completed with exit code 1` in ~0.3s, which is too short for osslsigncode to have run on two ~30MB binaries.

Root cause

The current verify script uses `set -euo pipefail` and assigns `osslsigncode` output to a variable in a single line:

```bash
output="$(osslsigncode verify -in "$file" 2>&1)"
echo "$output"
```

Under `set -e`, when osslsigncode exits non-zero (e.g. CA chain not trusted by the GitHub runner — likely the case while on a test signing policy), the assignment line itself fails, the function aborts, and `echo "$output"` never runs. The captured diagnostic is lost.

Fix

  • Drop `set -e` for this block (errors are handled explicitly).
  • Capture osslsigncode's exit code separately and echo the output before deciding pass/fail.
  • Aggregate per-file results so both binaries are reported.
  • Emit `::error::` annotations for Actions UI visibility.

Companion PR

A direct hotfix has been opened against `appwrite/sdk-for-cli` so the existing `20.2.0-rc.1` release can be reverified after merge: appwrite/sdk-for-cli#314.

Test plan

  • Once merged, the next CLI regeneration carries the diagnostic verify step.
  • On any subsequent SignPath-policy iteration we can see the real osslsigncode reason for failure instead of a bare exit 1.

Drop set -e from the verify step so the assignment of
osslsigncode's captured output no longer aborts the script before
echoing it. Track exit codes per file, aggregate so both binaries
are reported, and emit ::error:: annotations for Actions visibility.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR fixes the "Verify Windows signatures" CI step, which was silently failing with no diagnostic output because set -e caused the shell to abort on a non-zero osslsigncode exit before the captured output could be echoed.

  • Drops -e from set -euo pipefail, keeping -u and -o pipefail, and captures the osslsigncode exit code in rc=$? immediately after the subshell assignment so diagnostic output is always printed.
  • Replaces the early exit 1 inside the function with return 1 and aggregates per-file results via a final variable, so both binaries are always verified and reported even when the first fails.
  • Adds ::error:: GitHub Actions annotations for UI-level visibility when a binary fails verification.

Confidence Score: 5/5

This is a safe, tightly scoped fix to a single CI step with no changes to build logic, published artifacts, or secrets handling.

The root cause (set -e aborting before output is echoed) is correctly diagnosed and fixed. The new code captures the osslsigncode exit code in rc=$? right after the subshell assignment (valid bash behavior without set -e), always prints output before deciding pass/fail, correctly switches from exit 1 to return 1 inside the function, and aggregates both-binary results with the final variable pattern. No unset-variable risks remain with set -uo pipefail still active.

No files require special attention.

Important Files Changed

Filename Overview
templates/cli/.github/workflows/publish.yml Verify Windows signatures step refactored: drops set -e, captures osslsigncode exit code explicitly, adds ::error:: annotations, and aggregates both-binary results before exiting

Reviews (1): Last reviewed commit: "fix(cli): surface osslsigncode output wh..." | Re-trigger Greptile

@ChiragAgg5k ChiragAgg5k merged commit af9ccfc into master May 11, 2026
54 of 56 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix/verify-step-diagnostic branch May 11, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant