From 3c5f023b76bfc55f978efcbecb1cf9b4a0185079 Mon Sep 17 00:00:00 2001 From: Interlap Date: Wed, 7 Oct 2026 22:35:13 +0200 Subject: [PATCH] signing setup: write the builder.json profile only when its secrets were uploaded A profile in builder.json says the signing set is ready to build with. When the upload failed (a token that cannot write repository secrets, such as an agent platform's), writing it anyway made builds fail late on missing secrets and misled anyone reading builder.json, agents that cannot list secrets included. Both setup modes now skip the profile and say so; the library reports it in SetupResult.ProfileWritten. --- cmd/builder/signing.go | 15 +++++++++++---- cmd/builder/signing_auto.go | 12 +++++++++++- cmd/builder/signing_sets_test.go | 17 ++++++++++++----- internal/signing/sets.go | 16 ++++++++++++---- 4 files changed, 46 insertions(+), 14 deletions(-) diff --git a/cmd/builder/signing.go b/cmd/builder/signing.go index c457fb9..e814d8f 100644 --- a/cmd/builder/signing.go +++ b/cmd/builder/signing.go @@ -395,13 +395,20 @@ func runSigningSetup(cmd *cobra.Command, args []string) error { fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", uploadErr) } - // The profile is written whatever the upload did: the files exist and the - // build that uses them is the same either way. - replaced := writeSigningProfile(cfg, profileName, typ) + // The profile is written only when its secrets reached the repository: + // builder.json must not claim a signing set the repository does not have. + replaced := "" + if uploadErr == nil { + replaced = writeSigningProfile(cfg, profileName, typ) + } if err := config.NewManager().Save(cfg); err != nil { return fmt.Errorf("failed to update config: %w", err) } - fmt.Fprintln(out, profileWritten(profileName, typ, replaced)) + if uploadErr == nil { + fmt.Fprintln(out, profileWritten(profileName, typ, replaced)) + } else { + fmt.Fprintln(out, profileNotWritten(profileName)) + } names := config.SigningSecretNames(set) fmt.Fprintln(out) diff --git a/cmd/builder/signing_auto.go b/cmd/builder/signing_auto.go index 7fe613e..695574a 100644 --- a/cmd/builder/signing_auto.go +++ b/cmd/builder/signing_auto.go @@ -146,7 +146,11 @@ func runSigningAuto(cmd *cobra.Command) error { if uploadErr != nil { fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", uploadErr) } - fmt.Fprintln(out.log, profileWritten(profileName, typ, res.ReplacedDistribution)) + if res.ProfileWritten { + fmt.Fprintln(out.log, profileWritten(profileName, typ, res.ReplacedDistribution)) + } else { + fmt.Fprintln(out.log, profileNotWritten(profileName)) + } // Everything is printed before the exit code, so finish's success-only // hook is not used. @@ -222,6 +226,12 @@ func profileWritten(name string, typ signing.Type, replaced string) string { return line + ")" } +// profileNotWritten replaces that line when the upload failed: the profile +// would claim a signing set the repository does not have. +func profileNotWritten(name string) string { + return fmt.Sprintf(" Not updated: builder.json (profile %q is written once its secrets are uploaded; rerun setup with access to the repository's secrets, or add them by hand and then the profile)", name) +} + // resolveSigningBundleID takes the flag, then builder.json, then the newest // IPA in ./dist, then asks (only in a terminal). func resolveSigningBundleID(cmd *cobra.Command, cfg *config.Config, out output) (string, error) { diff --git a/cmd/builder/signing_sets_test.go b/cmd/builder/signing_sets_test.go index bef3f89..01845c1 100644 --- a/cmd/builder/signing_sets_test.go +++ b/cmd/builder/signing_sets_test.go @@ -706,7 +706,8 @@ func signingSetupCommand(t *testing.T, store secretStore, args ...string) (cmd * // TestSigningSetupManualReportsAFailedUpload: a repository Builder cannot // write to is a message, not a dead end. The values are printed, the build -// profile is written, and only the exit code says it failed. +// profile is not written (builder.json must not claim a set the repository +// does not have), and the exit code says it failed. func TestSigningSetupManualReportsAFailedUpload(t *testing.T) { t.Chdir(t.TempDir()) cfg := &config.Config{Project: "App", Platform: "ios", GitHub: config.GitHubConfig{Owner: "o", Repo: "r"}} @@ -742,8 +743,11 @@ func TestSigningSetupManualReportsAFailedUpload(t *testing.T) { if err != nil { t.Fatal(err) } - if saved.Profiles["store"].Distribution != "store" { - t.Errorf("build profile not written: %+v", saved.Profiles) + if _, ok := saved.Profiles["store"]; ok { + t.Errorf("build profile written although its secrets were not uploaded: %+v", saved.Profiles) + } + if !strings.Contains(stdout.String(), "Not updated: builder.json") { + t.Errorf("the skipped profile is not reported:\n%s", stdout.String()) } } @@ -785,8 +789,11 @@ func TestSigningSetupAutoReportsAFailedUpload(t *testing.T) { if err != nil { t.Fatal(err) } - if saved.Profiles["store"].Distribution != "store" { - t.Errorf("build profile not written: %+v", saved.Profiles) + if _, ok := saved.Profiles["store"]; ok { + t.Errorf("build profile written although its secrets were not uploaded: %+v", saved.Profiles) + } + if !strings.Contains(stdout.String(), "Not updated: builder.json") { + t.Errorf("the skipped profile is not reported:\n%s", stdout.String()) } // --json says the same in github_upload, and still exits non-zero. diff --git a/internal/signing/sets.go b/internal/signing/sets.go index 392d661..33b0843 100644 --- a/internal/signing/sets.go +++ b/internal/signing/sets.go @@ -88,8 +88,11 @@ type SetupResult struct { SecretsUploaded bool `json:"secrets_uploaded"` // GitHubUpload is "ok" or why the upload failed. GitHubUpload string `json:"github_upload"` - // BuildProfile is the builder.json profile written with the distribution. + // BuildProfile is the builder.json profile that builds with the set. BuildProfile string `json:"build_profile"` + // ProfileWritten says BuildProfile was written to builder.json, which + // happens only when the secrets were uploaded. + ProfileWritten bool `json:"profile_written"` // ReplacedDistribution is the distribution the profile had before, when // it was a different one. ReplacedDistribution string `json:"replaced_distribution,omitempty"` @@ -135,9 +138,14 @@ func Setup(ctx context.Context, client *asc.Client, store SecretStore, storeErr res.GitHubUpload = res.UploadError.Error() } - // The profile is written whatever the upload did: the material exists and - // the build that uses it is the same either way. - res.ReplacedDistribution = WriteProfile(cfg, profileName, opts.Type) + // The profile is written only when its secrets reached the repository: a + // profile in builder.json says the set is ready to build with, and one + // whose secrets are missing fails the build late and misleads anyone, an + // agent that cannot list secrets included, who reads builder.json. + if res.SecretsUploaded { + res.ReplacedDistribution = WriteProfile(cfg, profileName, opts.Type) + res.ProfileWritten = true + } RecordDir(cfg, opts.OutDirAsGiven) if cfg.IOS.BundleID == "" { cfg.IOS.BundleID = opts.BundleID