From 47dad7143f661a06f74f09c90a81940bf4fe201f Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 8 Sep 2026 19:20:39 +0200 Subject: [PATCH] docs(ci): tighten the code-signing comments in build-agent Public repo: keep the reasoning that a later change could break -- signing must precede the release hashes, the job must never carry an environment, the timestamp is not optional -- and drop the rest. Comments only, no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/build-agent.yml | 67 +++++++++++-------------------- 1 file changed, 23 insertions(+), 44 deletions(-) diff --git a/.github/workflows/build-agent.yml b/.github/workflows/build-agent.yml index 3d3b775e..454fade1 100644 --- a/.github/workflows/build-agent.yml +++ b/.github/workflows/build-agent.yml @@ -57,11 +57,8 @@ env: VERSIONED_ZIP: AutopilotMonitor-Agent-v2.zip STABLE_ZIP: AutopilotMonitor-Agent.zip STABLE_MANIFEST: version.json - # Authenticode via Azure Key Vault. Vault URL and client id stay in repository variables: - # this repo is public and infrastructure names do not belong in it. The signing identity - # lives in the same tenant as the deploy identity, so AZURE_TENANT_ID covers both. - # CODE_SIGN_EXPECTED_SUBJECT is optional -- when set, the verify step pins the publisher - # instead of accepting any trusted signature. + # Authenticode via Azure Key Vault; identity and vault come from repository variables. + # CODE_SIGN_EXPECTED_SUBJECT is optional and pins the publisher in the verify step. CODE_SIGN_CLIENT_ID: ${{ vars.CODE_SIGN_CLIENT_ID }} CODE_SIGN_CERTIFICATE_NAME: ${{ vars.CODE_SIGN_CERTIFICATE_NAME }} CODE_SIGN_KEY_VAULT_URL: ${{ vars.CODE_SIGN_KEY_VAULT_URL }} @@ -77,9 +74,8 @@ jobs: - name: Checkout repository uses: actions/checkout@v7 - # Fail before a build number is burnt if the signing configuration is incomplete. - # Every released build is signed -- there is no unsigned fallback, so a missing - # variable is a stop, not a warning. + # Every released build is signed; a missing variable stops the run before a build + # number is burnt. - name: Check code-signing configuration shell: pwsh run: | @@ -90,14 +86,10 @@ jobs: } Write-Host "Code-signing configuration present (certificate '$env:CODE_SIGN_CERTIFICATE_NAME')" - # The signing identity is a SECOND federated principal in the same tenant, and it logs in - # FIRST on purpose: azure/login sets the default az account, so the storage login below - # would otherwise be overwritten. Only the Key Vault token is taken from this context -- - # a plain string that survives the switch -- and everything after this step runs as the - # deploy principal again. - # Both federated credentials are bound to the same subject - # (repo:okieselbach/AutopilotMonitor:ref:refs/heads/main). Never add an `environment:` to - # this job: that rewrites the OIDC subject claim and breaks BOTH logins at once. + # Logs in FIRST and only to fetch the Key Vault token: azure/login sets the default az + # account, so the storage login below has to be the last one. Both federated credentials + # are bound to the ref, so this job must never be given an `environment:` -- that would + # change the OIDC subject claim and break both logins. - name: Login to Azure (code-signing identity) uses: azure/login@7ddb5af1ef8758cf1353cf3b42f940aee27ba21c # v3 with: @@ -134,8 +126,7 @@ jobs: with: dotnet-version: '10.0.x' - # Version pinned: a new or compromised NuGet release must not flow into signing unreviewed. - # LatestMajor roll-forward because the tool targets an older runtime than the SDK above. + # Version pinned; roll-forward because the tool targets an older runtime than the SDK above. - name: Install AzureSignTool env: DOTNET_ROLL_FORWARD: LatestMajor @@ -196,18 +187,13 @@ jobs: - name: Run Agent unit tests run: dotnet test ${{ env.TEST_PROJECT }} --configuration Release --no-restore --verbosity quiet - # Authenticode over everything we author: the two EXEs that run on a device and our own - # DLLs beside them. Third-party assemblies keep their publisher's signature -- re-signing - # would replace it. + # Authenticode over everything we author; third-party assemblies keep their own signature. # - # THE POSITION IS LOAD-BEARING: signing rewrites the PE file. It must happen before the - # staging copy, the ZIP and every SHA-256 in the next step, because those hashes are the - # bootstrap's integrity oracle (version.json / AdminConfiguration). Signing afterwards - # would publish hashes that no downloaded file can ever match. + # THE POSITION IS LOAD-BEARING: signing rewrites the PE file, so it must run before the + # ZIP and every SHA-256 in the next step -- those hashes are the bootstrap's integrity + # oracle, and hashes taken before signing match no downloaded file. # - # azuresigntool is a native tool, so $LASTEXITCODE must be checked explicitly -- PowerShell - # does not throw on a non-zero exit, and a silently unsigned release is exactly the - # outcome signing exists to prevent. + # $LASTEXITCODE is checked explicitly: PowerShell does not throw on a native non-zero exit. - name: Sign agent binaries (Authenticode) shell: pwsh env: @@ -332,12 +318,9 @@ jobs: "sha256=$sha256Hash" >> $env:GITHUB_OUTPUT "exe_sha256=$exeSha256Hash" >> $env:GITHUB_OUTPUT - # Verified against the STAGING copy -- the exact bytes that went into the ZIP and into - # both hashes, not the build output. The invariant is "every AutopilotMonitor PE in the - # shipped ZIP is signed", so a project added later cannot ride along unsigned. - # A missing timestamp is a failure, not a detail: without it the signature dies with the - # certificate. `Valid` alone only proves *a* trusted signature -- set the repository - # variable CODE_SIGN_EXPECTED_SUBJECT to pin the publisher as well. + # Verified against the STAGING copy -- the exact bytes that ship. The invariant is "every + # AutopilotMonitor PE in the ZIP is signed", so a project added later cannot ride along + # unsigned. A missing timestamp fails too: without it the signature dies with the cert. - name: Verify Authenticode signatures shell: pwsh run: | @@ -434,9 +417,8 @@ jobs: # to main, so a release lands a set that is consistent with what is already live # (dev render, version-bump guard, bootstrapVersion oracles, alias verification). # Runs AFTER version.json so its bootstrapVersion read sees this build's manifest. - # Same signing identity as the binaries above: the loader on every device - # verifies the publisher of the script it downloads, so the publisher refuses - # to upload unsigned scripts. + # Signed with the same identity: the device-side loader verifies the publisher + # of what it downloads, and the publisher refuses to upload unsigned scripts. ./.github/scripts/Publish-BootstrapScripts.ps1 ` -LegacySasToken '${{ secrets.AZURE_BLOB_AGENT_LEGACY_SAS_TOKEN }}' ` -SigningToken $env:CODE_SIGN_AKV_ACCESS_TOKEN ` @@ -473,14 +455,11 @@ jobs: # Sigstore keyless provenance: binds the artifact digests to this workflow run (repo, ref, # commit, workflow file) in a public transparency log. The digests cover the identical # bytes uploaded to blob (versioned AND stable), so one attestation covers all copies. - # Complementary to Authenticode, not a substitute: the signature says who published the - # binary and is what Windows, SmartScreen and WDAC evaluate; the attestation says which - # build produced these exact bytes and is checked with `gh attestation verify`. The two - # EXEs are subjects of their own so an extracted binary can be verified without the ZIP. - # Runs after signing -- the digests must be those of the signed files. + # Complements Authenticode rather than replacing it: the signature says who published a + # binary, the attestation which build produced these exact bytes. Both EXEs are subjects + # of their own so an extracted binary can be verified without the ZIP. # Verify: gh attestation verify --repo okieselbach/AutopilotMonitor - # Skipped when a run neither uploads nor releases: a test dispatch would otherwise write - # a permanent entry to the public transparency log for bytes nobody ever receives. + # Skipped on runs that publish nothing -- no log entry for bytes nobody receives. - name: Attest build provenance if: inputs.upload_blob || inputs.create_release uses: actions/attest-build-provenance@v4