Skip to content

fix(publish): surface osslsigncode output when verify fails#314

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

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

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

Problem

On the rerun of `20.2.0-rc.1` the SignPath step finally succeeded — SignPath confirmed "The signing request was successfully processed" — but the next step, `Verify Windows signatures`, fails with no diagnostic. The job log goes from the verify step's `set -uo pipefail` setup straight to `Process completed with exit code 1` in ~0.3s, which is way too short for `osslsigncode` to have run on two ~30 MB binaries.

Root cause

The current script:

```bash
set -euo pipefail

verify_signature() {
local file="$1"
local output

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

`output="$(osslsigncode ...)"` captures both stdout and stderr, but under `set -e` the assignment inherits the subshell's exit code. When osslsigncode exits non-zero (e.g. CA chain not trusted by the runner — likely on the current `test-signing` policy), the assignment fails and `set -e` aborts the function before `echo "$output"` runs. The captured output is discarded and we get a bare `exit 1`.

Fix

  • Drop `set -e` for this block (we already handle errors explicitly).
  • Capture osslsigncode's exit code separately and echo the output before deciding pass/fail.
  • Aggregate per-file results so both binaries are reported even if the first one fails.
  • Emit `::error::` annotations so failures land in the Actions UI.

Test plan

  • Merge and re-run the existing `20.2.0-rc.1` publish workflow (`gh run rerun 25651791691 --repo appwrite/sdk-for-cli --failed`).
  • Confirm the `Verify Windows signatures` step now prints the actual osslsigncode output regardless of exit code.

Upstream

Companion PR will land on `appwrite/sdk-generator` (`templates/cli/.github/workflows/publish.yml`) so the next CLI regeneration carries the same fix.

Drop set -e from the verify step so the assignment of
osslsigncode's captured stderr/stdout no longer aborts the script
before echoing it. Track exit codes per file and aggregate so both
binaries are reported even if the first fails. Also fail with
::error:: annotations for visibility in the Actions UI.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR fixes silent failure in the Verify Windows signatures step by dropping set -e, capturing osslsigncode's exit code explicitly into rc, and switching exit 1 to return 1 so both binaries are always verified even when the first fails.

  • Removes set -e so the output=\"$(osslsigncode ...)\" assignment no longer aborts before the captured output can be echoed; rc=$? correctly captures the subshell's exit code since it follows a plain assignment (not a local declaration).
  • Replaces the old exit 1 inside the function (which would terminate the whole script) with return 1 and uses an explicit final accumulator at the call site, ensuring both binaries are always checked and reported.
  • Adds ::error:: annotations so failures surface in the GitHub Actions UI alongside the raw osslsigncode output.

Confidence Score: 5/5

Safe to merge — the change is confined to one CI step and all failure paths are now explicitly handled.

The root cause (silent set -e abort swallowing the captured output) is correctly identified and fixed. Exit-code capture via rc=$? after a plain assignment (not local) works as expected in bash. The switch from exit 1 to return 1 inside the function is strictly more correct, and the final accumulator ensures both binaries are exercised. set -uo pipefail is retained and remains meaningful for unset-variable protection and pipeline exit-code propagation. No unhandled error paths were introduced.

No files require special attention; .github/workflows/publish.yml is the only changed file and the logic is straightforward.

Important Files Changed

Filename Overview
.github/workflows/publish.yml Drops set -e from the verify step, captures osslsigncode's exit code explicitly, switches from exit 1 to return 1 in the function, and aggregates both binary results before failing — fixing silent early-abort and surfacing diagnostic output.

Reviews (1): Last reviewed commit: "fix(publish): surface osslsigncode outpu..." | Re-trigger Greptile

@ChiragAgg5k ChiragAgg5k merged commit 95a7a4c into master May 11, 2026
2 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.

2 participants