diff --git a/CLAUDE.md b/CLAUDE.md index 9c2b1e1..f0674ce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,6 +48,9 @@ go install ./cmd/builder ./builder asc testers add ... --group # also: testers remove, users invite ./builder asc testers invite ... # send/resend the TestFlight email ./builder asc builds expire --build-number N --yes # groups delete needs --yes too +./builder env set API_URL https://... [--profile production] # also: env unset, env list [--json] +./builder secret set SENTRY_TOKEN [--profile p] [--provider x] [--value-stdin] # value on the provider, name in builder.json +./builder secret unset SENTRY_TOKEN [--profile p] # secret list [--json]: names + presence only ``` ## Architecture @@ -247,11 +250,24 @@ internal/ `env` and `distribution` (`config.ResolveProfile`: `--profile`, else `defaultProfile`, else top level unchanged). A profile signs iff it has a `distribution`; `ios.signing` is only the no-profile path - **Profile Transport**: the `profile` dispatch input is one JSON object (`{"name","env","distribution"}`) - to stay under the ten-input limit and is sent only when a profile is selected, since an older - workflow rejects unknown inputs (`triggerError`). `runner.sh` reads `BUILD_ENV` and `DISTRIBUTION` + to stay under the ten-input limit and is sent only when a profile is selected (or a top-level `env`/ + `secrets` exists), since an older workflow rejects unknown inputs (`triggerError`). It also carries + `"secrets"` (names). `runner.sh` reads `BUILD_ENV`, `DISTRIBUTION`, `BUILDER_SECRETS`, `BUILDER_SECRET_SUFFIX` - **Profile Env**: entries are base64 per key/value on the runner and the `$GITHUB_ENV` heredoc uses a random delimiter; names must match `^[A-Za-z_][A-Za-z0-9_]*$` and not hit `reservedEnv`/ - `reservedEnvPrefixes` (`internal/config/profile.go`), which must track what the runners read + `reservedEnvPrefixes` (`internal/config/profile.go`), which must track what the runners read. + A top-level `env` applies to every build; the profile's wins per key (`config.resolveEnv`) +- **Secrets Are Names Only**: builder.json `secrets` (top level, per profile; union) lists names; + values live on the provider (`ci.SecretStore`: GitHub Actions secrets, Codemagic secure vars in the + `builder` group via v3 `variable-groups`, Bitrise protected app secrets). `secret set --profile P` + stores `NAME__` (`config.SecretStorageName`: upper-cased, non-alnum `_`); runners prefer it + over `NAME` and export as `NAME`. Names: `^[A-Z_][A-Z0-9_]*$`, no `__`, reserved rules, never also env. + Value from a hidden prompt or `--value-stdin`, never argv. `secretStoreFor` is a var for tests +- **Secrets On The Runner**: GitHub's Resolve step gets `BUILDER_SECRETS_JSON: ${{ toJSON(secrets) }}` + (that step only; `TestResolveStepReceivesAllSecrets`), exports only listed names with + `::add-mask::` per value line, fails naming `builder secret set` for a missing one. `runner.sh` + `export_build_secrets` runs after `export_build_env`. `ios build` refuses secrets when the local + `ios-build.yml` lacks `toJSON(secrets)` (`checkWorkflowExportsSecrets`). `ios share` gets none - **Signing Sets**: one trio per distribution, `IOS_{CERTIFICATE,CERTIFICATE_PASSWORD,PROVISIONING_PROFILE}_` (DEVELOPMENT, AD_HOC, STORE, ENTERPRISE); the unsuffixed names serve only the legacy no-profile path. The table lives in `config.SigningSet` and the shell `signing_set` (both templates) and must agree @@ -459,7 +475,8 @@ the first IPA exists. `signing.dir` is the last automatic `signing setup`'s `--o A profile's fields are `distribution` (`development`, `ad-hoc`/`internal`, `store`, `enterprise`; the only signing field, omitted = unsigned), `configuration` (else Debug for development, Release -otherwise), `scheme`, `provider`, `env`. `runner`/`submit` are planned on `config.Profile`, not read. +otherwise), `scheme`, `provider`, `env`, `secrets`. `runner`/`submit` are planned on `config.Profile`, +not read. Top-level `env` (map) and `secrets` (names) apply to every build. ## Workflow Features diff --git a/README.md b/README.md index cc24fe8..5b091a5 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,16 @@ builder ios build --unsigned # Build without code signing (if signing is config builder ios build --provider codemagic # Build on another provider (also: bitrise) builder ios build --profile production # Build with a profile from builder.json +# Build environment and secrets (see "Environment and secrets" below) +builder env set API_URL https://api.example.com # Plain value for every build +builder env set API_URL https://staging.example.com --profile preview +builder env unset API_URL [--profile preview] +builder env list [--profile preview] [--json] +builder secret set SENTRY_TOKEN # Hidden prompt; stored on the CI provider +printf %s "$TOKEN" | builder secret set SENTRY_TOKEN --profile production --value-stdin +builder secret unset SENTRY_TOKEN [--profile production] +builder secret list [--json] # Names and where they are stored, never values + # Simulator (free, needs a MOBAI_API_KEY secret) builder ios share # Try the build on a simulator in the MobAI app builder ios share --duration 1h # Keep it available longer while unused @@ -352,7 +362,8 @@ builder ios build --profile preview | `configuration` | Overrides the derived configuration: `Debug` for `development`, `Release` for every other distribution, `ios.configuration` for unsigned profiles | | `scheme` | Overrides `ios.scheme` | | `provider` | Overrides the top-level `provider` (`github`, `codemagic`, `bitrise`) | -| `env` | String map exported as environment variables on the runner before dependencies are installed and the app is built, so `pod install`, `npm install`, `flutter pub get`, Gradle and xcodebuild all see them | +| `env` | String map exported as environment variables on the runner before dependencies are installed and the app is built, so `pod install`, `npm install`, `flutter pub get`, Gradle and xcodebuild all see them. Overrides the top-level `env` key by key | +| `secrets` | Names of provider-held secrets this profile's builds also expose (see [Environment and secrets](#environment-and-secrets)) | How a build's settings are resolved: @@ -370,19 +381,71 @@ How a build's settings are resolved: **`env` values are build-time configuration, not secrets.** They are stored in `builder.json`, sent to the CI provider as plain workflow inputs, and visible in -the run's inputs and logs. Keep tokens and passwords in the provider's secrets -(`gh secret set` on GitHub, or the [Codemagic / Bitrise secrets -guide](docs/provider-secrets.md)); the build reads those as environment -variables too. Names the runner owns are rejected: its own parameters (`SCHEME`, +the run's inputs and logs. Keep tokens and passwords in secrets (next section). +Names the runner owns are rejected: its own parameters (`SCHEME`, `CONFIGURATION`, `USE_SIGNING`, `BUILD_ENV`, ...), the signing secrets, `PATH`, `HOME`, `DEVELOPER_DIR`, and the `GITHUB_`, `RUNNER_`, `CM_`, `BITRISE_`, `BUILDER_` prefixes. +### Environment and secrets + +Plain values go in `builder.json`: a top-level `env` every build gets, and a +profile's `env` on top of it, key by key. `builder env set|unset|list` edits +them with the same name checks a build applies. + +Secret values never touch `builder.json` or Builder's servers (there are none): +`builder secret set NAME` reads the value from a hidden prompt (or stdin with +`--value-stdin`; never an argument, never printed), stores it on the CI +provider, and adds only the **name** to `"secrets"`: + +| Provider | Where the value goes | +|----------|----------------------| +| GitHub | A repository Actions secret, sealed with the repository's public key | +| Codemagic | A secure variable in the app's `builder` variable group (the one the generated `codemagic.yaml` imports; created if missing) | +| Bitrise | A protected app secret, with "replace variables in inputs" and pull-request exposure off | + +The provider is `--provider`, else the profile's `provider`, else the top-level +`provider`, else GitHub. Each build exports the names it lists as environment +variables before dependencies install, and fails by name when one has no value. + +```json +{ + "env": { "API_URL": "https://api.example.com" }, + "secrets": ["SENTRY_TOKEN"], + "profiles": { + "production": { "distribution": "store", "secrets": ["STRIPE_KEY"] } + } +} +``` + +**Per-profile values** use a name suffix. `builder secret set SENTRY_TOKEN +--profile production` stores the value as `SENTRY_TOKEN__PRODUCTION` (the +profile name upper-cased, other characters as `_`) and lists `SENTRY_TOKEN` +under that profile. A build with the profile takes `NAME__` when the +provider has it, else `NAME`, and exports it as `NAME` either way, so the app +reads one name. This works the same on all three providers; GitHub +Environments are not used. + +Secret names are upper case letters, digits and single underscores (GitHub +stores names upper case, and `__` separates the profile suffix), and the +reserved names above apply, so a secret cannot shadow `IOS_CERTIFICATE_*`, +`MOBAI_API_KEY` or the runner's own variables. A name cannot be both a plain +`env` value and a secret. + +On GitHub the `Resolve parameters` step receives `${{ toJSON(secrets) }}` (the +only way a workflow can read secrets whose names are not written in it), +exports just the listed names, and registers every line of each value with +`::add-mask::` first. `builder secret list` shows each listed name, where it is +stored and whether the provider has it. Secrets apply to `ios build`, not to +`ios share`. + Selecting a profile, with `--profile` or `defaultProfile`, needs the workflow file from this version of Builder, which declares a `profile` input; an older committed workflow rejects the dispatch. Run `builder init` again to refresh `.github/workflows/ios-build.yml` (or `builder init --provider ...` for `runner.sh`) in a project set up earlier, then commit and push it to the -default branch. +default branch. The same goes for a top-level `env` or any `secrets`: an older +workflow ignores the secret list, so `ios build` refuses to dispatch while the +local `.github/workflows/ios-build.yml` predates it. ### MobAI Configuration diff --git a/cmd/builder/auth_prompt.go b/cmd/builder/auth_prompt.go index efd4a38..fe93058 100644 --- a/cmd/builder/auth_prompt.go +++ b/cmd/builder/auth_prompt.go @@ -14,12 +14,19 @@ import ( // Hidden terminal input avoids line-editor redraws and wrapping when pasting // long API tokens. Pipes must explicitly opt in with --token-stdin. func readProviderToken(ctx context.Context, input io.Reader, output io.Writer) (string, error) { + return readHidden(ctx, input, output, "API token", "--token-stdin") +} + +// readHidden reads one value from the terminal without echoing it. what names +// the value in the prompt and errors; stdinFlag is the flag that takes it from +// a pipe instead. +func readHidden(ctx context.Context, input io.Reader, output io.Writer, what, stdinFlag string) (string, error) { if err := ctx.Err(); err != nil { - return "", fmt.Errorf("API token input canceled: %w", err) + return "", fmt.Errorf("%s input canceled: %w", what, err) } file, ok := input.(*os.File) if !ok || !term.IsTerminal(int(file.Fd())) { - return "", fmt.Errorf("API token input requires a terminal; use --token-stdin for piped input") + return "", fmt.Errorf("%s input requires a terminal; use %s for piped input", what, stdinFlag) } fd := int(file.Fd()) ctx, stop := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) @@ -35,9 +42,9 @@ func readProviderToken(ctx context.Context, input io.Reader, output io.Writer) ( _ = term.Restore(fd, state) fmt.Fprintln(output) }() - fmt.Fprint(output, "API token (input hidden; paste once, then press Enter): ") + fmt.Fprintf(output, "%s (input hidden; paste once, then press Enter): ", what) type result struct { - token string + value string err error } done := make(chan result, 1) @@ -48,16 +55,16 @@ func readProviderToken(ctx context.Context, input io.Reader, output io.Writer) ( io.Reader io.Writer }{file, io.Discard}, "") - token, err := terminal.ReadPassword("") - done <- result{token, err} + value, err := terminal.ReadPassword("") + done <- result{value, err} }() select { case <-ctx.Done(): - return "", fmt.Errorf("API token input canceled: %w", ctx.Err()) + return "", fmt.Errorf("%s input canceled: %w", what, ctx.Err()) case r := <-done: if r.err != nil { - return "", fmt.Errorf("read API token: %w", r.err) + return "", fmt.Errorf("read %s: %w", what, r.err) } - return r.token, nil + return r.value, nil } } diff --git a/cmd/builder/env.go b/cmd/builder/env.go new file mode 100644 index 0000000..b8374e4 --- /dev/null +++ b/cmd/builder/env.go @@ -0,0 +1,157 @@ +package main + +import ( + "cmp" + "encoding/json" + "fmt" + "maps" + "slices" + "text/tabwriter" + + "github.com/MobAI-App/ios-builder/internal/config" + "github.com/spf13/cobra" +) + +var envCmd = &cobra.Command{ + Use: "env", + Short: "Manage the plain environment variables builds get", + Long: `Plain (non-secret) environment variables live in builder.json: the top-level +"env" applies to every build, and profiles..env overrides it per key. +They are exported on the runner before dependencies install and the app builds. + +Values are committed with builder.json, so never put a secret here; use +builder secret set for those.`, +} + +var envSetCmd = &cobra.Command{ + Use: "set NAME VALUE", + Short: "Set a variable for every build, or for one profile with --profile", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + profile, _ := cmd.Flags().GetString("profile") + cfg, err := loadConfig() + if err != nil { + return err + } + if err := cfg.SetEnv(profile, args[0], args[1]); err != nil { + return err + } + if err := config.NewManager().Save(cfg); err != nil { + return err + } + fmt.Fprintf(cmd.OutOrStdout(), "Set %s for %s in builder.json.\n", args[0], scopeName(profile)) + return nil + }, +} + +var envUnsetCmd = &cobra.Command{ + Use: "unset NAME", + Short: "Remove a variable from the top level, or from one profile with --profile", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + profile, _ := cmd.Flags().GetString("profile") + cfg, err := loadConfig() + if err != nil { + return err + } + removed, err := cfg.UnsetEnv(profile, args[0]) + if err != nil { + return err + } + if !removed { + return fmt.Errorf("%s is not set for %s", args[0], scopeName(profile)) + } + if err := config.NewManager().Save(cfg); err != nil { + return err + } + fmt.Fprintf(cmd.OutOrStdout(), "Removed %s from %s in builder.json.\n", args[0], scopeName(profile)) + return nil + }, +} + +// envEntry is one row of env list. Profile is where the value is set; empty +// for the top level. +type envEntry struct { + Name string `json:"name"` + Value string `json:"value"` + Profile string `json:"profile,omitempty"` +} + +var envListCmd = &cobra.Command{ + Use: "list", + Short: "List the variables: everything in builder.json, or what one profile's builds get", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + profile, _ := cmd.Flags().GetString("profile") + asJSON, _ := cmd.Flags().GetBool("json") + cfg, err := loadConfig() + if err != nil { + return err + } + entries, err := envEntries(cfg, profile) + if err != nil { + return err + } + out := cmd.OutOrStdout() + if asJSON { + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + return enc.Encode(entries) + } + if len(entries) == 0 { + fmt.Fprintln(out, "No env variables in builder.json. Add one with: builder env set NAME VALUE") + return nil + } + w := tabwriter.NewWriter(out, 0, 4, 2, ' ', 0) + fmt.Fprintln(w, "NAME\tVALUE\tSET FOR") + for _, e := range entries { + fmt.Fprintf(w, "%s\t%s\t%s\n", e.Name, e.Value, scopeName(e.Profile)) + } + return w.Flush() + }, +} + +// envEntries lists every level's variables without a profile, and with one +// the variables its builds get, each with the level that sets it. +func envEntries(cfg *config.Config, profile string) ([]envEntry, error) { + entries := []envEntry{} + add := func(level string, env map[string]string, skip map[string]string) { + for _, k := range slices.Sorted(maps.Keys(env)) { + if _, ok := skip[k]; !ok { + entries = append(entries, envEntry{Name: k, Value: env[k], Profile: level}) + } + } + } + if profile == "" { + add("", cfg.Env, nil) + for _, name := range cfg.ProfileNames() { + add(name, cfg.Profiles[name].Env, nil) + } + return entries, nil + } + if _, err := cfg.ResolveProfile(profile); err != nil { + return nil, err + } + own := cfg.Profiles[profile].Env + add(profile, own, nil) + add("", cfg.Env, own) + slices.SortFunc(entries, func(a, b envEntry) int { return cmp.Compare(a.Name, b.Name) }) + return entries, nil +} + +// scopeName describes a level of builder.json for messages. +func scopeName(profile string) string { + if profile == "" { + return "all builds" + } + return "profile " + profile +} + +func init() { + for _, c := range []*cobra.Command{envSetCmd, envUnsetCmd, envListCmd} { + c.Flags().String("profile", "", "builder.json profile (default: the top level, which every build gets)") + } + envListCmd.Flags().Bool("json", false, "Print JSON") + envCmd.AddCommand(envSetCmd, envUnsetCmd, envListCmd) + rootCmd.AddCommand(envCmd) +} diff --git a/cmd/builder/env_test.go b/cmd/builder/env_test.go new file mode 100644 index 0000000..9e48323 --- /dev/null +++ b/cmd/builder/env_test.go @@ -0,0 +1,93 @@ +package main + +import ( + "encoding/json" + "os" + "reflect" + "strings" + "testing" + + "github.com/MobAI-App/ios-builder/internal/config" +) + +// writeSecretProject writes a builder.json with all three providers, a +// top-level env and two profiles into a fresh working directory. +func writeSecretProject(t *testing.T) { + t.Helper() + t.Chdir(t.TempDir()) + cfg := &config.Config{ + Project: "App", Platform: "ios", GitHub: config.GitHubConfig{Owner: "o", Repo: "r"}, + Codemagic: config.CIConfig{AppID: "cm-app", Branch: "main"}, + Bitrise: config.CIConfig{AppID: "br-app", Branch: "main"}, + Env: map[string]string{"API_URL": "https://api.example.com"}, + Profiles: map[string]config.Profile{ + "production": {Distribution: "store", Provider: "codemagic"}, + "staging": {Distribution: "internal"}, + }, + } + if err := config.NewManager().Save(cfg); err != nil { + t.Fatal(err) + } +} + +func loadSaved(t *testing.T) *config.Config { + t.Helper() + cfg, err := config.NewManager().Load() + if err != nil { + t.Fatal(err) + } + return cfg +} + +func TestEnvCommands(t *testing.T) { + writeSecretProject(t) + if _, _, err := run(t, "env", "set", "FEATURE", "on"); err != nil { + t.Fatal(err) + } + if _, _, err := run(t, "env", "set", "API_URL", "https://staging.example.com", "--profile", "staging"); err != nil { + t.Fatal(err) + } + cfg := loadSaved(t) + if cfg.Env["FEATURE"] != "on" || cfg.Profiles["staging"].Env["API_URL"] != "https://staging.example.com" || cfg.Profiles["staging"].Distribution != "internal" { + t.Fatalf("saved %+v", cfg) + } + for name, args := range map[string][]string{ + "reserved": {"env", "set", "SCHEME", "x"}, + "bad name": {"env", "set", "A-B", "x"}, + "unknown profile": {"env", "set", "A", "x", "--profile", "nightly"}, + "not set": {"env", "unset", "NOPE"}, + } { + if _, _, err := run(t, args...); err == nil { + t.Errorf("%s accepted", name) + } + } + + // With a profile: what its builds get, and where each value comes from. + out, _, err := run(t, "env", "list", "--profile", "staging", "--json") + if err != nil { + t.Fatal(err) + } + var entries []envEntry + if err := json.Unmarshal([]byte(out), &entries); err != nil { + t.Fatalf("%v\n%s", err, out) + } + want := []envEntry{{Name: "API_URL", Value: "https://staging.example.com", Profile: "staging"}, {Name: "FEATURE", Value: "on"}} + if !reflect.DeepEqual(entries, want) { + t.Fatalf("list --profile:\n got %+v\nwant %+v", entries, want) + } + out, _, err = run(t, "env", "list") + if err != nil || !strings.Contains(out, "https://api.example.com") || !strings.Contains(out, "profile staging") { + t.Fatalf("list: %v\n%s", err, out) + } + + if _, _, err := run(t, "env", "unset", "API_URL", "--profile", "staging"); err != nil { + t.Fatal(err) + } + if loadSaved(t).Profiles["staging"].Env != nil { + t.Fatal("unset left the profile env") + } + data, _ := os.ReadFile("builder.json") + if !strings.Contains(string(data), `"FEATURE": "on"`) { + t.Fatalf("builder.json:\n%s", data) + } +} diff --git a/cmd/builder/secret.go b/cmd/builder/secret.go new file mode 100644 index 0000000..f464342 --- /dev/null +++ b/cmd/builder/secret.go @@ -0,0 +1,325 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "slices" + "strings" + "text/tabwriter" + + "github.com/MobAI-App/ios-builder/internal/auth" + "github.com/MobAI-App/ios-builder/internal/ci" + "github.com/MobAI-App/ios-builder/internal/config" + "github.com/MobAI-App/ios-builder/internal/github" + "github.com/spf13/cobra" +) + +var secretCmd = &cobra.Command{ + Use: "secret", + Short: "Manage build secrets held by the CI provider", + Long: `Builder never hosts secret values. builder secret set stores the value on the +CI provider and records only the name in builder.json ("secrets", top level or +per profile); each build exports the listed names as environment variables. + + GitHub repository Actions secrets (encrypted with the repository key) + Codemagic secure variables in the app's "builder" variable group + Bitrise protected app secrets + +With --profile the value is stored as NAME__ (the profile name upper +cased, other characters as _) and only that profile's builds list it; a build +with the profile takes NAME__ when it exists, else NAME, and exports it +as NAME either way. The provider is --provider, else the profile's provider, +else builder.json's, else GitHub.`, +} + +// secretStoreFor opens the secrets API of a provider and names where the +// secrets go. A var so tests can point the real clients at fake servers. +var secretStoreFor = func(cfg *config.Config, provider string) (ci.SecretStore, string, error) { + switch provider { + case "github": + gh, err := getGitHubClient() + if err != nil { + return nil, "", err + } + return githubSecrets{gh, cfg.GitHub.Owner, cfg.GitHub.Repo}, "GitHub Actions secrets of " + cfg.GitHub.Owner + "/" + cfg.GitHub.Repo, nil + case "codemagic", "bitrise": + ciCfg, err := cfg.ProviderConfig(provider) + if err != nil { + return nil, "", err + } + token, err := auth.GetProviderToken(provider) + if err != nil { + return nil, "", fmt.Errorf("not authenticated with %s; run builder auth %s or set %s_API_TOKEN", provider, provider, strings.ToUpper(provider)) + } + if provider == "codemagic" { + return ci.NewCodemagicSecrets(ciCfg.AppID, token), "Codemagic app " + ciCfg.AppID + " (variable group " + ci.CodemagicVariableGroup + ")", nil + } + return ci.NewBitriseSecrets(ciCfg.AppID, token), "Bitrise app " + ciCfg.AppID + " secrets", nil + default: + return nil, "", fmt.Errorf("unknown provider %q", provider) + } +} + +// githubSecrets is the repository's Actions secrets as a ci.SecretStore. +type githubSecrets struct { + gh *github.Client + owner, repo string +} + +func (g githubSecrets) List(ctx context.Context) ([]string, error) { + return g.gh.ListSecretNames(ctx, g.owner, g.repo) +} + +func (g githubSecrets) Set(ctx context.Context, name, value string) error { + return g.gh.SetSecret(ctx, g.owner, g.repo, name, value) +} + +func (g githubSecrets) Delete(ctx context.Context, name string) (bool, error) { + return g.gh.DeleteSecret(ctx, g.owner, g.repo, name) +} + +// secretProvider is where a level's secrets live: --provider, else the +// profile's provider, else the top-level one, else GitHub. +func secretProvider(cfg *config.Config, profile, override string) (string, error) { + if override == "" && profile != "" { + override = cfg.Profiles[profile].Provider + } + return cfg.ProviderName(override) +} + +// readSecretValue takes the value from stdin with --value-stdin (one trailing +// newline dropped, as a shell pipe adds it) or from a hidden prompt; never +// from the command line, where it would land in shell history. +func readSecretValue(cmd *cobra.Command, name string) (string, error) { + fromStdin, _ := cmd.Flags().GetBool("value-stdin") + var value string + if fromStdin { + data, err := io.ReadAll(io.LimitReader(cmd.InOrStdin(), 1<<20+1)) + if err != nil { + return "", fmt.Errorf("read %s from stdin: %w", name, err) + } + if len(data) > 1<<20 { + return "", fmt.Errorf("%s is larger than 1 MiB", name) + } + value = strings.TrimSuffix(strings.TrimSuffix(string(data), "\n"), "\r") + } else { + var err error + value, err = readHidden(cmd.Context(), cmd.InOrStdin(), cmd.ErrOrStderr(), name, "--value-stdin") + if err != nil { + return "", err + } + } + if value == "" { + return "", fmt.Errorf("%s is empty; providers do not store empty secrets", name) + } + return value, nil +} + +var secretSetCmd = &cobra.Command{ + Use: "set NAME", + Short: "Store a secret on the CI provider and list its name in builder.json", + Long: `Stores the value on the CI provider and adds NAME to builder.json's "secrets" +(or the profile's with --profile). The value is read from a hidden prompt, or +from stdin with --value-stdin; it is never an argument and never printed. + + builder secret set SENTRY_TOKEN + printf %s "$TOKEN" | builder secret set SENTRY_TOKEN --profile production --value-stdin`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + name := args[0] + profile, _ := cmd.Flags().GetString("profile") + override, _ := cmd.Flags().GetString("provider") + cfg, err := loadConfig() + if err != nil { + return err + } + // Every check that needs no network runs before the value is asked for. + if err := cfg.AddSecret(profile, name); err != nil { + return err + } + provider, err := secretProvider(cfg, profile, override) + if err != nil { + return err + } + store, where, err := secretStoreFor(cfg, provider) + if err != nil { + return err + } + value, err := readSecretValue(cmd, name) + if err != nil { + return err + } + stored := config.SecretStorageName(name, profile) + if err := store.Set(cmd.Context(), stored, value); err != nil { + return fmt.Errorf("failed to store %s in %s: %w", stored, where, err) + } + if err := config.NewManager().Save(cfg); err != nil { + return fmt.Errorf("stored %s in %s, but could not add it to builder.json: %w", stored, where, err) + } + out := cmd.OutOrStdout() + fmt.Fprintf(out, "Stored %s in %s.\n", stored, where) + fmt.Fprintf(out, "Listed %s under %s in builder.json; those builds get it as $%s.\n", name, scopeName(profile), name) + if provider == "github" && workflowLacksSecrets() { + fmt.Fprintln(out, "Note: .github/workflows/ios-build.yml predates secrets; run builder init to refresh it, then commit and push it.") + } + return nil + }, +} + +var secretUnsetCmd = &cobra.Command{ + Use: "unset NAME", + Short: "Delete a secret from the CI provider and drop its name from builder.json", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + name := args[0] + profile, _ := cmd.Flags().GetString("profile") + override, _ := cmd.Flags().GetString("provider") + cfg, err := loadConfig() + if err != nil { + return err + } + listed, err := cfg.RemoveSecret(profile, name) + if err != nil { + return err + } + provider, err := secretProvider(cfg, profile, override) + if err != nil { + return err + } + store, where, err := secretStoreFor(cfg, provider) + if err != nil { + return err + } + stored := config.SecretStorageName(name, profile) + // Listing first separates "not there" from "no access", which a + // delete's 404 cannot. + have, err := store.List(cmd.Context()) + if err != nil { + return err + } + deleted := false + if slices.Contains(have, stored) { + if deleted, err = store.Delete(cmd.Context(), stored); err != nil { + return fmt.Errorf("failed to delete %s from %s: %w", stored, where, err) + } + } + if !listed && !deleted { + return fmt.Errorf("%s is neither listed for %s in builder.json nor stored in %s", name, scopeName(profile), where) + } + if listed { + if err := config.NewManager().Save(cfg); err != nil { + return err + } + } + out := cmd.OutOrStdout() + if deleted { + fmt.Fprintf(out, "Deleted %s from %s.\n", stored, where) + } else { + fmt.Fprintf(out, "%s was not stored in %s.\n", stored, where) + } + if listed { + fmt.Fprintf(out, "Removed %s from %s in builder.json.\n", name, scopeName(profile)) + } + return nil + }, +} + +// secretEntry is one row of secret list: a name builder.json lists, where its +// value is stored and whether the provider has it. Never a value. +type secretEntry struct { + Name string `json:"name"` + Profile string `json:"profile,omitempty"` + StoredAs string `json:"stored_as"` + Provider string `json:"provider"` + Present bool `json:"present"` +} + +var secretListCmd = &cobra.Command{ + Use: "list", + Short: "List the secret names in builder.json and whether the provider holds them", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + profile, _ := cmd.Flags().GetString("profile") + override, _ := cmd.Flags().GetString("provider") + asJSON, _ := cmd.Flags().GetBool("json") + cfg, err := loadConfig() + if err != nil { + return err + } + levels := []string{""} + if profile != "" { + if _, err := cfg.ResolveProfile(profile); err != nil { + return err + } + levels = append(levels, profile) + } else { + levels = append(levels, cfg.ProfileNames()...) + } + entries := []secretEntry{} + names := map[string][]string{} // provider -> names it holds, fetched once + for _, level := range levels { + provider, err := secretProvider(cfg, level, override) + if err != nil { + return err + } + for _, name := range cfg.SecretsOf(level) { + if _, ok := names[provider]; !ok { + store, _, err := secretStoreFor(cfg, provider) + if err != nil { + return err + } + if names[provider], err = store.List(cmd.Context()); err != nil { + return err + } + } + stored := config.SecretStorageName(name, level) + entries = append(entries, secretEntry{Name: name, Profile: level, StoredAs: stored, Provider: provider, Present: slices.Contains(names[provider], stored)}) + } + } + out := cmd.OutOrStdout() + if asJSON { + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + return enc.Encode(entries) + } + if len(entries) == 0 { + fmt.Fprintln(out, "No secrets in builder.json. Add one with: builder secret set NAME") + return nil + } + w := tabwriter.NewWriter(out, 0, 4, 2, ' ', 0) + fmt.Fprintln(w, "NAME\tFOR\tSTORED AS\tPROVIDER\tSTATUS") + for _, e := range entries { + status := "set" + if !e.Present { + status = "missing (builds fail)" + if e.Profile != "" && slices.Contains(names[e.Provider], e.Name) { + status = "uses " + e.Name + } + } + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", e.Name, scopeName(e.Profile), e.StoredAs, e.Provider, status) + } + return w.Flush() + }, +} + +// workflowLacksSecrets reports a local ios-build.yml written before +// secrets were exported; false when there is none to look at. +func workflowLacksSecrets() bool { + data, err := os.ReadFile(filepath.Join(".github", "workflows", "ios-build.yml")) + return err == nil && !strings.Contains(string(data), "toJSON(secrets)") +} + +func init() { + for _, c := range []*cobra.Command{secretSetCmd, secretUnsetCmd, secretListCmd} { + c.Flags().String("profile", "", "builder.json profile (default: the top level, which every build gets)") + c.Flags().String("provider", "", "CI provider holding the secret: github, codemagic or bitrise (default: the profile's, else builder.json's, else github)") + } + secretSetCmd.Flags().Bool("value-stdin", false, "Read the value from stdin instead of a hidden prompt") + secretListCmd.Flags().Bool("json", false, "Print JSON") + secretCmd.AddCommand(secretSetCmd, secretUnsetCmd, secretListCmd) + rootCmd.AddCommand(secretCmd) +} diff --git a/cmd/builder/secret_test.go b/cmd/builder/secret_test.go new file mode 100644 index 0000000..9a465d9 --- /dev/null +++ b/cmd/builder/secret_test.go @@ -0,0 +1,296 @@ +package main + +import ( + "crypto/rand" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "reflect" + "sort" + "strings" + "sync" + "testing" + + "github.com/MobAI-App/ios-builder/internal/ci" + "github.com/MobAI-App/ios-builder/internal/config" + "github.com/MobAI-App/ios-builder/internal/github" + "golang.org/x/crypto/nacl/box" +) + +// providerFakes serves the three secrets APIs from one in-memory store per +// provider, holding what each was sent (GitHub values decrypted). +type providerFakes struct { + mu sync.Mutex + stored map[string]map[string]string // provider -> name -> value + flags map[string]map[string]any // bitrise name -> create body + calls int +} + +func (f *providerFakes) put(provider, name, value string) { + if f.stored[provider] == nil { + f.stored[provider] = map[string]string{} + } + f.stored[provider][name] = value +} + +func (f *providerFakes) names(provider string) []string { + var out []string + for n := range f.stored[provider] { + out = append(out, n) + } + sort.Strings(out) + return out +} + +func newProviderFakes(t *testing.T) *providerFakes { + t.Helper() + f := &providerFakes{stored: map[string]map[string]string{}, flags: map[string]map[string]any{}} + pub, priv, err := box.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + gh := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls++ + name, _ := strings.CutPrefix(r.URL.Path, "/repos/o/r/actions/secrets/") + switch { + case r.URL.Path == "/repos/o/r/actions/secrets/public-key": + fmt.Fprintf(w, `{"key_id":"kid","key":%q}`, base64.StdEncoding.EncodeToString(pub[:])) + case r.Method == "GET" && r.URL.Path == "/repos/o/r/actions/secrets": + var list []map[string]string + for _, n := range f.names("github") { + list = append(list, map[string]string{"name": n}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"total_count": len(list), "secrets": list}) + case r.Method == "PUT": + var body github.CreateSecretRequest + _ = json.NewDecoder(r.Body).Decode(&body) + sealed, _ := base64.StdEncoding.DecodeString(body.EncryptedValue) + plain, ok := box.OpenAnonymous(nil, sealed, pub, priv) + if !ok { + t.Errorf("%s not sealed with the repository key", name) + } + f.put("github", name, string(plain)) + w.WriteHeader(http.StatusCreated) + case r.Method == "DELETE": + delete(f.stored["github"], name) + w.WriteHeader(http.StatusNoContent) + default: + t.Errorf("GitHub: unexpected %s %s", r.Method, r.URL) + } + })) + t.Cleanup(gh.Close) + + // Codemagic: one builder group "g1" that exists from the start; variable + // IDs are the names. + cm := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls++ + if r.Header.Get("x-auth-token") != "cm-token" { + t.Errorf("Codemagic: no token") + } + switch { + case r.Method == "GET" && r.URL.Path == "/apps/cm-app/variable-groups": + fmt.Fprint(w, `{"data":[{"id":"g1","name":"builder"}],"current_page":1,"page_size":100,"total_pages":1}`) + case r.Method == "GET" && r.URL.Path == "/variable-groups/g1/variables": + var data []map[string]any + for _, n := range f.names("codemagic") { + data = append(data, map[string]any{"id": n, "name": n, "value": nil, "secure": true}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"data": data, "current_page": 1, "page_size": 100, "total_pages": 1}) + case r.Method == "POST" && r.URL.Path == "/variable-groups/g1/variables": + var body struct { + Secure bool `json:"secure"` + Variables []map[string]string `json:"variables"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + if !body.Secure { + t.Errorf("Codemagic variable not secure") + } + for _, v := range body.Variables { + f.put("codemagic", v["name"], v["value"]) + } + w.WriteHeader(201) + case r.Method == "DELETE" && strings.HasPrefix(r.URL.Path, "/variable-groups/g1/variables/"): + delete(f.stored["codemagic"], strings.TrimPrefix(r.URL.Path, "/variable-groups/g1/variables/")) + w.WriteHeader(204) + default: + t.Errorf("Codemagic: unexpected %s %s", r.Method, r.URL) + } + })) + t.Cleanup(cm.Close) + + br := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls++ + if r.Header.Get("Authorization") != "br-token" { + t.Errorf("Bitrise: no token") + } + switch { + case r.Method == "GET" && r.URL.Path == "/apps/br-app/secrets": + var list []map[string]string + for _, n := range f.names("bitrise") { + list = append(list, map[string]string{"name": n}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"secrets": list}) + case r.Method == "POST" && r.URL.Path == "/apps/br-app/secrets": + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + name, _ := body["name"].(string) + value, _ := body["value"].(string) + f.put("bitrise", name, value) + f.flags[name] = body + w.WriteHeader(201) + fmt.Fprint(w, `{}`) + case r.Method == "DELETE" && strings.HasPrefix(r.URL.Path, "/apps/br-app/secrets/"): + delete(f.stored["bitrise"], strings.TrimPrefix(r.URL.Path, "/apps/br-app/secrets/")) + w.WriteHeader(204) + default: + t.Errorf("Bitrise: unexpected %s %s", r.Method, r.URL) + } + })) + t.Cleanup(br.Close) + + prev := secretStoreFor + secretStoreFor = func(cfg *config.Config, provider string) (ci.SecretStore, string, error) { + switch provider { + case "github": + return githubSecrets{github.NewClientWithBaseURL("tok", gh.URL), cfg.GitHub.Owner, cfg.GitHub.Repo}, "GitHub (fake)", nil + case "codemagic": + return ci.NewCodemagicSecretsAt(cm.URL, cfg.Codemagic.AppID, "cm-token"), "Codemagic (fake)", nil + case "bitrise": + return ci.NewBitriseSecretsAt(br.URL, cfg.Bitrise.AppID, "br-token"), "Bitrise (fake)", nil + } + return nil, "", fmt.Errorf("unknown provider %q", provider) + } + t.Cleanup(func() { secretStoreFor = prev }) + return f +} + +// runWithStdin is run with stdin set for the command. +func runWithStdin(t *testing.T, stdin string, args ...string) (string, string, error) { + t.Helper() + rootCmd.SetIn(strings.NewReader(stdin)) + defer rootCmd.SetIn(nil) + return run(t, args...) +} + +func TestSecretSetStoresOnTheProviderAndListsTheName(t *testing.T) { + writeSecretProject(t) + f := newProviderFakes(t) + + // Top level, GitHub (no provider configured anywhere). + out, errOut, err := runWithStdin(t, "s3cret-top\n", "secret", "set", "SENTRY_TOKEN", "--value-stdin") + if err != nil { + t.Fatalf("%v\n%s", err, errOut) + } + if f.stored["github"]["SENTRY_TOKEN"] != "s3cret-top" { + t.Fatalf("GitHub got %v", f.stored["github"]) + } + if strings.Contains(out+errOut, "s3cret") { + t.Fatalf("value echoed:\n%s%s", out, errOut) + } + if got := loadSaved(t).Secrets; !reflect.DeepEqual(got, []string{"SENTRY_TOKEN"}) { + t.Fatalf("builder.json secrets = %v", got) + } + + // A profile whose provider is Codemagic: stored with the profile suffix, + // listed under the profile only. + if _, errOut, err = runWithStdin(t, "s3cret-prod", "secret", "set", "SENTRY_TOKEN", "--profile", "production", "--value-stdin"); err != nil { + t.Fatalf("%v\n%s", err, errOut) + } + if f.stored["codemagic"]["SENTRY_TOKEN__PRODUCTION"] != "s3cret-prod" || len(f.stored["github"]) != 1 { + t.Fatalf("Codemagic got %v, GitHub %v", f.stored["codemagic"], f.stored["github"]) + } + saved := loadSaved(t) + if !reflect.DeepEqual(saved.Profiles["production"].Secrets, []string{"SENTRY_TOKEN"}) || saved.Profiles["production"].Distribution != "store" { + t.Fatalf("production profile = %+v", saved.Profiles["production"]) + } + + // --provider beats the rest. + if _, errOut, err = runWithStdin(t, "maps\r\n", "secret", "set", "MAPS_KEY", "--provider", "bitrise", "--value-stdin"); err != nil { + t.Fatalf("%v\n%s", err, errOut) + } + if f.stored["bitrise"]["MAPS_KEY"] != "maps" || f.flags["MAPS_KEY"]["is_protected"] != true || f.flags["MAPS_KEY"]["expand_in_step_inputs"] != false { + t.Fatalf("Bitrise got %v %v", f.stored["bitrise"], f.flags) + } + + // list: names and where they are, never values. + out, _, err = run(t, "secret", "list", "--json") + if err != nil { + t.Fatal(err) + } + var entries []secretEntry + if err := json.Unmarshal([]byte(out), &entries); err != nil { + t.Fatalf("%v\n%s", err, out) + } + // MAPS_KEY sits at the top level, whose provider is GitHub: stored on + // Bitrise only, so a GitHub build would miss it. + want := []secretEntry{ + {Name: "MAPS_KEY", StoredAs: "MAPS_KEY", Provider: "github"}, + {Name: "SENTRY_TOKEN", StoredAs: "SENTRY_TOKEN", Provider: "github", Present: true}, + {Name: "SENTRY_TOKEN", Profile: "production", StoredAs: "SENTRY_TOKEN__PRODUCTION", Provider: "codemagic", Present: true}, + } + if !reflect.DeepEqual(entries, want) { + t.Fatalf("list:\n got %+v\nwant %+v", entries, want) + } + if strings.Contains(out, "s3cret") || strings.Contains(out, "maps\"") { + t.Fatalf("list printed a value:\n%s", out) + } + out, _, err = run(t, "secret", "list", "--provider", "bitrise") + if err != nil || !strings.Contains(out, "MAPS_KEY") || !strings.Contains(out, "missing") { + t.Fatalf("table: %v\n%s", err, out) + } + + // unset deletes the stored value and the name. + if _, errOut, err = run(t, "secret", "unset", "SENTRY_TOKEN", "--profile", "production"); err != nil { + t.Fatalf("%v\n%s", err, errOut) + } + if _, ok := f.stored["codemagic"]["SENTRY_TOKEN__PRODUCTION"]; ok || loadSaved(t).Profiles["production"].Secrets != nil { + t.Fatalf("unset left %v / %+v", f.stored["codemagic"], loadSaved(t).Profiles["production"]) + } + if _, _, err = run(t, "secret", "unset", "SENTRY_TOKEN", "--profile", "production"); err == nil { + t.Fatal("second unset succeeded") + } + if _, _, err = run(t, "secret", "unset", "MAPS_KEY", "--provider", "bitrise"); err != nil || len(f.stored["bitrise"]) != 0 || len(loadSaved(t).Secrets) != 1 { + t.Fatalf("bitrise unset: %v %v %v", err, f.stored["bitrise"], loadSaved(t).Secrets) + } +} + +func TestSecretSetRefusesBeforeAskingForTheValue(t *testing.T) { + writeSecretProject(t) + f := newProviderFakes(t) + for name, args := range map[string][]string{ + "signing secret": {"secret", "set", "IOS_CERTIFICATE_STORE", "--value-stdin"}, + "MobAI key": {"secret", "set", "MOBAI_API_KEY", "--value-stdin"}, + "lower case": {"secret", "set", "sentry_token", "--value-stdin"}, + "shadows env": {"secret", "set", "API_URL", "--value-stdin"}, + "unknown profile": {"secret", "set", "A", "--profile", "nightly", "--value-stdin"}, + "value as arg": {"secret", "set", "A", "value"}, + "empty value": {"secret", "set", "EMPTY", "--value-stdin"}, + } { + stdin := "v" + if name == "empty value" { + stdin = "\n" + } + if _, _, err := runWithStdin(t, stdin, args...); err == nil { + t.Errorf("%s accepted", name) + } + } + // No terminal and no --value-stdin: the hidden prompt says how to pipe. + _, _, err := runWithStdin(t, "v", "secret", "set", "A") + if err == nil || !strings.Contains(err.Error(), "--value-stdin") { + t.Fatalf("non-terminal stdin: %v", err) + } + if len(f.stored) != 0 { + t.Fatalf("something reached a provider: %v", f.stored) + } + if cfg := loadSaved(t); cfg.Secrets != nil { + t.Fatalf("builder.json changed: %v", cfg.Secrets) + } +} diff --git a/docs/provider-secrets.md b/docs/provider-secrets.md index 780243c..a0029e0 100644 --- a/docs/provider-secrets.md +++ b/docs/provider-secrets.md @@ -17,6 +17,15 @@ for the two new providers; use the steps below even if GitHub signing/sharing already works. Existing GitHub secret values cannot be downloaded for copying to another service. +**Your app's own secrets** (an API token the build needs, say) do not need any +of the dashboard steps below: `builder secret set SENTRY_TOKEN --provider +codemagic` (or `bitrise`) stores the value through the provider's API, a secure +variable in the `builder` group on Codemagic and a protected secret on Bitrise, +and lists the name in `builder.json`; the build exports it and fails by name if +it is missing. See [Environment and secrets](../README.md#environment-and-secrets). +The signing secrets and `MOBAI_API_KEY` are reserved names that `secret set` +refuses; add those as described here. + ## 1. Prepare your signing files A repository holds one signing set per distribution (`development`, `ad-hoc` @@ -180,9 +189,9 @@ builder ios build --profile development --provider codemagic builder ios build --profile development --provider bitrise ``` -Codemagic and Bitrise have no secrets API, so `ios build` cannot check or -provision the set the way it does on GitHub; a missing variable fails in the -runner's signing step by name. +`ios build` does not check or provision the signing set on Codemagic and +Bitrise the way it does on GitHub; a missing variable fails in the runner's +signing step by name. A successful run should archive, export, and download an IPA. Install it on a device included in the development profile to verify signing and provisioning. diff --git a/internal/build/coordinator.go b/internal/build/coordinator.go index 4fd5613..201643c 100644 --- a/internal/build/coordinator.go +++ b/internal/build/coordinator.go @@ -134,6 +134,25 @@ func (c *Coordinator) buildInputs(buildID, ref string, s *config.BuildSettings, return inputs } +// workflowSecretsMarker is in every workflow file that exports the secrets +// builder.json lists; an older file ignores the list without a word. +const workflowSecretsMarker = "toJSON(secrets)" + +// checkWorkflowExportsSecrets refuses a build with secrets when the local +// copy of the workflow predates them, since the dispatch would be accepted and +// the build would run without them. Without a local copy there is nothing to +// check; the dispatched file is the default branch's anyway. +func checkWorkflowExportsSecrets(s *config.BuildSettings) error { + if len(s.Secrets) == 0 { + return nil + } + data, err := os.ReadFile(filepath.Join(".github", "workflows", WorkflowFile)) + if err != nil || bytes.Contains(data, []byte(workflowSecretsMarker)) { + return nil + } + return fmt.Errorf(".github/workflows/%s does not export builder.json secrets (%s); run `builder init` to refresh it, then commit and push it to the default branch", WorkflowFile, strings.Join(s.Secrets, ", ")) +} + // triggerError explains a rejected dispatch. GitHub answers 422 "Unexpected // inputs provided" when the committed workflow file does not declare an input, // which for `profile` and `build_number` means the file predates them. @@ -173,6 +192,9 @@ func (c *Coordinator) Build(ctx context.Context, opts *BuildOptions) (*BuildResu if c.github == nil { return nil, fmt.Errorf("GitHub client is required") } + if err := checkWorkflowExportsSecrets(settings); err != nil { + return nil, err + } startTime := time.Now() timeout := opts.Timeout diff --git a/internal/build/progress.go b/internal/build/progress.go index f4c736a..5940bc6 100644 --- a/internal/build/progress.go +++ b/internal/build/progress.go @@ -99,6 +99,9 @@ func (p *Progress) Settings(s *config.BuildSettings, provider string) { keys := slices.Sorted(maps.Keys(s.Env)) fmt.Fprintf(p.writer, " Env: %s\n", strings.Join(keys, ", ")) } + if len(s.Secrets) > 0 { + fmt.Fprintf(p.writer, " Secrets: %s\n", strings.Join(s.Secrets, ", ")) + } if s.Distribution != "" { fmt.Fprintf(p.writer, " Distribution: %s\n", s.Distribution) } diff --git a/internal/build/remote.go b/internal/build/remote.go index 4f48493..14f39a1 100644 --- a/internal/build/remote.go +++ b/internal/build/remote.go @@ -88,6 +88,15 @@ func (c *Coordinator) inputs(buildID, ref, sha string, s *config.BuildSettings) if s.Distribution != "" { v["DISTRIBUTION"] = s.Distribution } + // Only names: the values are the app's own secure variables, already in + // the runner's environment. runner.sh checks each is set and, for a + // profile, prefers NAME__ (config.SecretStorageName). + if len(s.Secrets) > 0 { + v["BUILDER_SECRETS"] = strings.Join(s.Secrets, " ") + if s.Profile != "" { + v["BUILDER_SECRET_SUFFIX"] = config.SecretSuffix(s.Profile) + } + } return v } diff --git a/internal/build/secrets_test.go b/internal/build/secrets_test.go new file mode 100644 index 0000000..a57ea70 --- /dev/null +++ b/internal/build/secrets_test.go @@ -0,0 +1,79 @@ +package build + +import ( + "bytes" + "io" + "os" + "strings" + "testing" + + "github.com/MobAI-App/ios-builder/internal/config" +) + +func TestSecretsReachTheRunners(t *testing.T) { + cfg := profiledConfig() + cfg.Secrets = []string{"SENTRY_TOKEN"} + cfg.Env = map[string]string{"LOG": "debug"} + p := cfg.Profiles["preview"] + p.Secrets = []string{"MAPS_KEY"} + cfg.Profiles["preview"] = p + c := NewCoordinatorWithOutput(cfg, nil, io.Discard) + + // GitHub: names travel inside the profile input, even with no profile. + s, _, _ := c.settings("", "", false) + got := c.buildInputs("abcdef12", "ref", s, "") + if !strings.Contains(got["profile"], `"secrets":["SENTRY_TOKEN"]`) || !strings.Contains(got["profile"], `"LOG":"debug"`) { + t.Fatalf("top-level secrets/env not sent: %v", got) + } + s, _, _ = c.settings("preview", "", false) + got = c.buildInputs("abcdef12", "ref", s, "") + if !strings.Contains(got["profile"], `"secrets":["MAPS_KEY","SENTRY_TOKEN"]`) || len(got) > 10 { + t.Fatalf("profile secrets not sent: %v", got) + } + + // Codemagic/Bitrise: the names and the profile's suffix, never a value. + v := c.inputs("abcdef12", "ref", "sha", s) + if v["BUILDER_SECRETS"] != "MAPS_KEY SENTRY_TOKEN" || v["BUILDER_SECRET_SUFFIX"] != "PREVIEW" { + t.Fatalf("runner variables: %v", v) + } + s, _, _ = c.settings("", "", false) + v = c.inputs("abcdef12", "ref", "sha", s) + if v["BUILDER_SECRETS"] != "SENTRY_TOKEN" || v["BUILDER_SECRET_SUFFIX"] != "" { + t.Fatalf("no-profile runner variables: %v", v) + } + + var out bytes.Buffer + pr := NewProgress(&out) + pr.Start("abcdef12") + pr.Settings(s, "github") + if !strings.Contains(out.String(), "Secrets: SENTRY_TOKEN") { + t.Fatalf("secret names not printed:\n%s", out.String()) + } +} + +func TestOldWorkflowWithSecretsIsRefused(t *testing.T) { + t.Chdir(t.TempDir()) + s := &config.BuildSettings{Secrets: []string{"SENTRY_TOKEN"}} + if err := checkWorkflowExportsSecrets(s); err != nil { + t.Fatalf("no local workflow must not block: %v", err) + } + if err := os.MkdirAll(".github/workflows", 0755); err != nil { + t.Fatal(err) + } + write := func(content string) { + if err := os.WriteFile(".github/workflows/"+WorkflowFile, []byte(content), 0644); err != nil { + t.Fatal(err) + } + } + write("name: iOS Build\n") + if err := checkWorkflowExportsSecrets(s); err == nil || !strings.Contains(err.Error(), "builder init") { + t.Fatalf("old workflow accepted: %v", err) + } + if err := checkWorkflowExportsSecrets(&config.BuildSettings{}); err != nil { + t.Fatalf("a build without secrets must not care: %v", err) + } + write("BUILDER_SECRETS_JSON: ${{ toJSON(secrets) }}\n") + if err := checkWorkflowExportsSecrets(s); err != nil { + t.Fatalf("current workflow refused: %v", err) + } +} diff --git a/internal/ci/secrets.go b/internal/ci/secrets.go new file mode 100644 index 0000000..69ac5ff --- /dev/null +++ b/internal/ci/secrets.go @@ -0,0 +1,249 @@ +package ci + +import ( + "context" + "fmt" + "net/url" + "sort" +) + +// SecretStore keeps build secrets on a provider. Values go in and never come +// back out: List returns names only. +type SecretStore interface { + List(ctx context.Context) ([]string, error) + Set(ctx context.Context, name, value string) error + Delete(ctx context.Context, name string) (bool, error) +} + +// CodemagicVariableGroup is the app variable group the generated +// codemagic.yaml imports (environment.groups: [builder]). +const CodemagicVariableGroup = "builder" + +// CodemagicSecrets stores secrets as secure variables in the app's +// CodemagicVariableGroup, created on the first Set. +type CodemagicSecrets struct { + api apiClient + appID string + baseURL string +} + +// NewCodemagicSecrets talks to Codemagic's v3 API for one app. +func NewCodemagicSecrets(appID, token string) *CodemagicSecrets { + return NewCodemagicSecretsAt("https://codemagic.io/api/v3", appID, token) +} + +// NewCodemagicSecretsAt is NewCodemagicSecrets against another API root (tests). +func NewCodemagicSecretsAt(baseURL, appID, token string) *CodemagicSecrets { + return &CodemagicSecrets{api: newAPI(token, "x-auth-token"), appID: appID, baseURL: baseURL} +} + +type codemagicPage struct { + CurrentPage int `json:"current_page"` + TotalPages int `json:"total_pages"` +} + +// pages calls fetch for page 1, 2, ... until the last page; fetch returns +// the page metadata it decoded. +func pages(fetch func(page int) (codemagicPage, error)) error { + for page := 1; ; page++ { + meta, err := fetch(page) + if err != nil { + return err + } + if page >= meta.TotalPages || page > 1000 { + return nil + } + } +} + +// group finds the builder variable group's ID; empty when the app has none. +func (c *CodemagicSecrets) group(ctx context.Context) (string, error) { + var id string + err := pages(func(page int) (codemagicPage, error) { + var res struct { + codemagicPage + Data []struct { + ID string `json:"id"` + Name string `json:"name"` + } `json:"data"` + } + endpoint := fmt.Sprintf("%s/apps/%s/variable-groups?page_size=100&page=%d", c.baseURL, url.PathEscape(c.appID), page) + if err := c.api.request(ctx, "GET", endpoint, nil, &res); err != nil { + return res.codemagicPage, err + } + for _, g := range res.Data { + if g.Name == CodemagicVariableGroup && id == "" { + id = g.ID + } + } + if id != "" { + res.TotalPages = page // found; stop paging + } + return res.codemagicPage, nil + }) + return id, err +} + +type codemagicVariable struct { + ID string `json:"id"` + Name string `json:"name"` +} + +func (c *CodemagicSecrets) variables(ctx context.Context, group string) ([]codemagicVariable, error) { + var vars []codemagicVariable + err := pages(func(page int) (codemagicPage, error) { + var res struct { + codemagicPage + Data []codemagicVariable `json:"data"` + } + endpoint := fmt.Sprintf("%s/variable-groups/%s/variables?page_size=100&page=%d", c.baseURL, url.PathEscape(group), page) + if err := c.api.request(ctx, "GET", endpoint, nil, &res); err != nil { + return res.codemagicPage, err + } + vars = append(vars, res.Data...) + return res.codemagicPage, nil + }) + return vars, err +} + +func (c *CodemagicSecrets) find(ctx context.Context, name string) (group, id string, err error) { + group, err = c.group(ctx) + if err != nil || group == "" { + return group, "", err + } + vars, err := c.variables(ctx, group) + if err != nil { + return group, "", err + } + for _, v := range vars { + if v.Name == name { + return group, v.ID, nil + } + } + return group, "", nil +} + +// List returns the variable names of the builder group, sorted. +func (c *CodemagicSecrets) List(ctx context.Context) ([]string, error) { + group, err := c.group(ctx) + if err != nil || group == "" { + return nil, err + } + vars, err := c.variables(ctx, group) + if err != nil { + return nil, err + } + names := make([]string, 0, len(vars)) + for _, v := range vars { + names = append(names, v.Name) + } + sort.Strings(names) + return names, nil +} + +// Set updates the variable in place, or adds it to the group as secure, +// creating the group when the app has none. +func (c *CodemagicSecrets) Set(ctx context.Context, name, value string) error { + group, id, err := c.find(ctx, name) + if err != nil { + return err + } + if id != "" { + body := map[string]any{"value": value, "secure": true} + return c.api.request(ctx, "PATCH", c.baseURL+"/variable-groups/"+url.PathEscape(group)+"/variables/"+url.PathEscape(id), body, nil) + } + if group == "" { + var res struct { + Data struct { + ID string `json:"id"` + } `json:"data"` + } + body := map[string]string{"name": CodemagicVariableGroup} + if err := c.api.request(ctx, "POST", c.baseURL+"/apps/"+url.PathEscape(c.appID)+"/variable-groups", body, &res); err != nil { + return err + } + if res.Data.ID == "" { + return fmt.Errorf("Codemagic created the %s variable group but returned no ID", CodemagicVariableGroup) + } + group = res.Data.ID + } + body := map[string]any{"secure": true, "variables": []map[string]string{{"name": name, "value": value}}} + return c.api.request(ctx, "POST", c.baseURL+"/variable-groups/"+url.PathEscape(group)+"/variables", body, nil) +} + +// Delete removes the variable from the builder group. +func (c *CodemagicSecrets) Delete(ctx context.Context, name string) (bool, error) { + group, id, err := c.find(ctx, name) + if err != nil || id == "" { + return false, err + } + return true, c.api.request(ctx, "DELETE", c.baseURL+"/variable-groups/"+url.PathEscape(group)+"/variables/"+url.PathEscape(id), nil, nil) +} + +// BitriseSecrets stores secrets as Bitrise app secrets. +type BitriseSecrets struct { + api apiClient + appSlug string + baseURL string +} + +// NewBitriseSecrets talks to the Bitrise API for one app. +func NewBitriseSecrets(appSlug, token string) *BitriseSecrets { + return NewBitriseSecretsAt("https://api.bitrise.io/v0.1", appSlug, token) +} + +// NewBitriseSecretsAt is NewBitriseSecrets against another API root (tests). +func NewBitriseSecretsAt(baseURL, appSlug, token string) *BitriseSecrets { + return &BitriseSecrets{api: newAPI(token, "Authorization"), appSlug: appSlug, baseURL: baseURL} +} + +func (b *BitriseSecrets) secretsURL() string { + return b.baseURL + "/apps/" + url.PathEscape(b.appSlug) + "/secrets" +} + +// List returns the app's secret names, sorted. +func (b *BitriseSecrets) List(ctx context.Context) ([]string, error) { + var res struct { + Secrets []struct { + Name string `json:"name"` + } `json:"secrets"` + } + if err := b.api.request(ctx, "GET", b.secretsURL(), nil, &res); err != nil { + return nil, err + } + names := make([]string, 0, len(res.Secrets)) + for _, s := range res.Secrets { + names = append(names, s.Name) + } + sort.Strings(names) + return names, nil +} + +// Set updates an existing secret's value, leaving its flags as they are (a +// protected secret accepts nothing else), or creates it protected, with +// variable expansion and pull-request exposure off: a literal value only +// builds Builder dispatches can read. +func (b *BitriseSecrets) Set(ctx context.Context, name, value string) error { + names, err := b.List(ctx) + if err != nil { + return err + } + if i := sort.SearchStrings(names, name); i < len(names) && names[i] == name { + return b.api.request(ctx, "PATCH", b.secretsURL()+"/"+url.PathEscape(name), map[string]any{"value": value}, nil) + } + body := map[string]any{"name": name, "value": value, "is_protected": true, + "expand_in_step_inputs": false, "is_exposed_for_pull_requests": false} + return b.api.request(ctx, "POST", b.secretsURL(), body, nil) +} + +// Delete removes the app secret. +func (b *BitriseSecrets) Delete(ctx context.Context, name string) (bool, error) { + names, err := b.List(ctx) + if err != nil { + return false, err + } + if i := sort.SearchStrings(names, name); i == len(names) || names[i] != name { + return false, nil + } + return true, b.api.request(ctx, "DELETE", b.secretsURL()+"/"+url.PathEscape(name), nil, nil) +} diff --git a/internal/ci/secrets_test.go b/internal/ci/secrets_test.go new file mode 100644 index 0000000..db2ad37 --- /dev/null +++ b/internal/ci/secrets_test.go @@ -0,0 +1,255 @@ +package ci + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "reflect" + "sort" + "strings" + "sync" + "testing" +) + +// codemagicFake is the v3 variable-group API of one app, two variables per +// page so paging is exercised. +type codemagicFake struct { + mu sync.Mutex + groups map[string]string // id -> name + vars map[string]map[string]string // group id -> variable id -> name + values map[string]string // variable id -> value + secure map[string]bool + calls []string + nextID int +} + +func newCodemagicFake(t *testing.T) (*codemagicFake, *CodemagicSecrets) { + f := &codemagicFake{groups: map[string]string{"g-other": "other"}, vars: map[string]map[string]string{}, values: map[string]string{}, secure: map[string]bool{}} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + if r.Header.Get("x-auth-token") != "cm-token" { + t.Errorf("missing token on %s %s", r.Method, r.URL) + } + f.calls = append(f.calls, r.Method+" "+r.URL.Path) + parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + page := 1 + _, _ = fmt.Sscan(r.URL.Query().Get("page"), &page) + switch { + case r.Method == "GET" && r.URL.Path == "/apps/app-1/variable-groups": + var data []map[string]string + for id, name := range f.groups { + data = append(data, map[string]string{"id": id, "name": name}) + } + writeJSONPage(w, data, page, 100) + case r.Method == "POST" && r.URL.Path == "/apps/app-1/variable-groups": + var body map[string]string + _ = json.NewDecoder(r.Body).Decode(&body) + f.nextID++ + id := fmt.Sprintf("g%d", f.nextID) + f.groups[id] = body["name"] + w.WriteHeader(201) + _ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]string{"id": id, "name": body["name"]}}) + case len(parts) == 3 && parts[0] == "variable-groups" && parts[2] == "variables" && r.Method == "GET": + var data []map[string]any + ids := make([]string, 0) + for id := range f.vars[parts[1]] { + ids = append(ids, id) + } + sort.Strings(ids) + for _, id := range ids { + data = append(data, map[string]any{"id": id, "name": f.vars[parts[1]][id], "value": nil, "secure": true}) + } + writeJSONPage(w, data, page, 2) + case len(parts) == 3 && parts[0] == "variable-groups" && parts[2] == "variables" && r.Method == "POST": + var body struct { + Secure bool `json:"secure"` + Variables []map[string]string `json:"variables"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + if f.vars[parts[1]] == nil { + f.vars[parts[1]] = map[string]string{} + } + for _, v := range body.Variables { + f.nextID++ + id := fmt.Sprintf("v%d", f.nextID) + f.vars[parts[1]][id] = v["name"] + f.values[id] = v["value"] + f.secure[id] = body.Secure + } + w.WriteHeader(201) + case len(parts) == 4 && parts[0] == "variable-groups" && r.Method == "PATCH": + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + f.values[parts[3]], _ = body["value"].(string) + f.secure[parts[3]] = body["secure"] == true + w.WriteHeader(204) + case len(parts) == 4 && parts[0] == "variable-groups" && r.Method == "DELETE": + delete(f.vars[parts[1]], parts[3]) + w.WriteHeader(204) + default: + t.Errorf("unexpected %s %s", r.Method, r.URL) + w.WriteHeader(404) + } + })) + t.Cleanup(srv.Close) + return f, NewCodemagicSecretsAt(srv.URL, "app-1", "cm-token") +} + +func writeJSONPage[T any](w http.ResponseWriter, all []T, page, size int) { + total := (len(all) + size - 1) / size + if total == 0 { + total = 1 + } + start := min((page-1)*size, len(all)) + end := min(start+size, len(all)) + _ = json.NewEncoder(w).Encode(map[string]any{"data": all[start:end], "current_page": page, "page_size": size, "total_pages": total}) +} + +func TestCodemagicSecrets(t *testing.T) { + ctx := context.Background() + f, s := newCodemagicFake(t) + + // No builder group yet: nothing listed, and set creates it. + if names, err := s.List(ctx); err != nil || len(names) != 0 { + t.Fatalf("empty list: %v %v", names, err) + } + for _, n := range []string{"SENTRY_TOKEN", "MAPS_KEY", "THIRD"} { + if err := s.Set(ctx, n, "value of "+n); err != nil { + t.Fatal(err) + } + } + if len(f.groups) != 2 { + t.Fatalf("builder group not created exactly once: %v", f.groups) + } + names, err := s.List(ctx) + if err != nil || !reflect.DeepEqual(names, []string{"MAPS_KEY", "SENTRY_TOKEN", "THIRD"}) { + t.Fatalf("list across pages: %v %v", names, err) + } + for id, secure := range f.secure { + if !secure { + t.Errorf("variable %s not secure", id) + } + } + + // A second set updates in place. + if err := s.Set(ctx, "SENTRY_TOKEN", "rotated"); err != nil { + t.Fatal(err) + } + found := 0 + for id, v := range f.values { + for _, vars := range f.vars { + if vars[id] == "SENTRY_TOKEN" { + found++ + if v != "rotated" { + t.Errorf("value not updated: %q", v) + } + } + } + } + if found != 1 { + t.Fatalf("SENTRY_TOKEN stored %d times", found) + } + + if ok, err := s.Delete(ctx, "MAPS_KEY"); !ok || err != nil { + t.Fatalf("delete: %v %v", ok, err) + } + if ok, err := s.Delete(ctx, "MAPS_KEY"); ok || err != nil { + t.Fatalf("second delete: %v %v", ok, err) + } + if names, _ := s.List(ctx); !reflect.DeepEqual(names, []string{"SENTRY_TOKEN", "THIRD"}) { + t.Fatalf("after delete: %v", names) + } +} + +func TestCodemagicSecretErrorsHideValues(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(403) + fmt.Fprint(w, `{"detail":"forbidden","status_code":403,"errors":[]}`) + })) + defer srv.Close() + err := NewCodemagicSecretsAt(srv.URL, "app-1", "cm-token").Set(context.Background(), "A", "super-secret-value") + if err == nil || strings.Contains(err.Error(), "super-secret-value") || strings.Contains(err.Error(), "cm-token") { + t.Fatalf("error: %v", err) + } +} + +type bitriseFake struct { + mu sync.Mutex + secrets map[string]map[string]any + calls []string +} + +func newBitriseFake(t *testing.T) (*bitriseFake, *BitriseSecrets) { + f := &bitriseFake{secrets: map[string]map[string]any{}} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + if r.Header.Get("Authorization") != "br-token" { + t.Errorf("missing token on %s %s", r.Method, r.URL) + } + f.calls = append(f.calls, r.Method+" "+r.URL.Path) + name, isItem := strings.CutPrefix(r.URL.Path, "/apps/slug/secrets/") + switch { + case r.Method == "GET" && r.URL.Path == "/apps/slug/secrets": + var list []map[string]any + for n, s := range f.secrets { + list = append(list, map[string]any{"name": n, "is_protected": s["is_protected"]}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"secrets": list}) + case r.Method == "POST" && r.URL.Path == "/apps/slug/secrets": + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + name, _ := body["name"].(string) + f.secrets[name] = body + w.WriteHeader(201) + fmt.Fprint(w, `{}`) + case r.Method == "PATCH" && isItem: + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + for k, v := range body { + f.secrets[name][k] = v + } + fmt.Fprint(w, `{}`) + case r.Method == "DELETE" && isItem: + delete(f.secrets, name) + w.WriteHeader(204) + default: + t.Errorf("unexpected %s %s", r.Method, r.URL) + w.WriteHeader(404) + } + })) + t.Cleanup(srv.Close) + return f, NewBitriseSecretsAt(srv.URL, "slug", "br-token") +} + +func TestBitriseSecrets(t *testing.T) { + ctx := context.Background() + f, s := newBitriseFake(t) + if err := s.Set(ctx, "SENTRY_TOKEN", "a$HOME"); err != nil { + t.Fatal(err) + } + got := f.secrets["SENTRY_TOKEN"] + if got["value"] != "a$HOME" || got["is_protected"] != true || got["expand_in_step_inputs"] != false || got["is_exposed_for_pull_requests"] != false { + t.Fatalf("created with %v", got) + } + // An update sends only the value: a protected secret accepts nothing else. + f.calls = nil + if err := s.Set(ctx, "SENTRY_TOKEN", "rotated"); err != nil { + t.Fatal(err) + } + if f.secrets["SENTRY_TOKEN"]["value"] != "rotated" || !reflect.DeepEqual(f.calls, []string{"GET /apps/slug/secrets", "PATCH /apps/slug/secrets/SENTRY_TOKEN"}) { + t.Fatalf("update: %v %v", f.secrets, f.calls) + } + if names, err := s.List(ctx); err != nil || !reflect.DeepEqual(names, []string{"SENTRY_TOKEN"}) { + t.Fatalf("list: %v %v", names, err) + } + if ok, err := s.Delete(ctx, "SENTRY_TOKEN"); !ok || err != nil || len(f.secrets) != 0 { + t.Fatalf("delete: %v %v", ok, err) + } + if ok, err := s.Delete(ctx, "SENTRY_TOKEN"); ok || err != nil { + t.Fatalf("delete missing: %v %v", ok, err) + } +} diff --git a/internal/config/env.go b/internal/config/env.go new file mode 100644 index 0000000..b3e4203 --- /dev/null +++ b/internal/config/env.go @@ -0,0 +1,247 @@ +package config + +import ( + "fmt" + "maps" + "regexp" + "slices" + "strings" +) + +// secretNameRe is stricter than envNameRe: GitHub stores secret names upper +// case whatever was sent, and a double underscore is the separator between a +// secret and the profile it belongs to (SecretStorageName). +var secretNameRe = regexp.MustCompile(`^[A-Z_][A-Z0-9_]*$`) + +// ValidateEnvName checks a plain env name: a shell variable name the runners +// do not own. +func ValidateEnvName(name string) error { + if !envNameRe.MatchString(name) { + return fmt.Errorf("env name %q is not a valid environment variable name", name) + } + if reservedEnvName(name) { + return fmt.Errorf("env name %q is reserved for the runner", name) + } + return nil +} + +// ValidateSecretName checks a secret name: upper case letters, digits and +// single underscores, and none the runners own (so a secret cannot shadow +// IOS_CERTIFICATE_*, MOBAI_API_KEY, GITHUB_* and the like). +func ValidateSecretName(name string) error { + if !secretNameRe.MatchString(name) { + return fmt.Errorf("secret name %q must be upper case letters, digits and underscores, not starting with a digit", name) + } + if strings.Contains(name, "__") { + return fmt.Errorf("secret name %q must not contain a double underscore, which separates a secret from its profile", name) + } + if reservedEnvName(name) { + return fmt.Errorf("secret name %q is reserved for the runner", name) + } + return nil +} + +// SecretSuffix is the provider-side suffix of a profile's secrets: the +// profile name upper-cased with every other character than A-Z and 0-9 +// replaced by an underscore. The runners compute the same. +func SecretSuffix(profile string) string { + var b strings.Builder + for _, r := range strings.ToUpper(profile) { + if (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') { + b.WriteRune(r) + } else { + b.WriteByte('_') + } + } + return b.String() +} + +// SecretStorageName is the name a secret's value is stored under on the +// provider: the name itself for every build, NAME__ for a value only +// the profile's builds use. On the runner a profile's builds take NAME__ +// when it exists and NAME otherwise, and export it as NAME. +func SecretStorageName(name, profile string) string { + if profile == "" { + return name + } + return name + "__" + SecretSuffix(profile) +} + +// resolveEnv merges the top-level env and secrets with the named profile's +// (none for an empty name) and validates every name involved. +func (c *Config) resolveEnv(profile string) (map[string]string, []string, error) { + env := map[string]string{} + var secrets []string + add := func(scope string, e map[string]string, s []string) error { + for k, v := range e { + if err := ValidateEnvName(k); err != nil { + return fmt.Errorf("%s: %w", scope, err) + } + env[k] = v + } + for _, n := range s { + if err := ValidateSecretName(n); err != nil { + return fmt.Errorf("%s: %w", scope, err) + } + secrets = append(secrets, n) + } + return nil + } + if err := add("builder.json", c.Env, c.Secrets); err != nil { + return nil, nil, err + } + scope := "builder.json" + if profile != "" { + scope = fmt.Sprintf("profile %q", profile) + p := c.Profiles[profile] + if err := add(scope, p.Env, p.Secrets); err != nil { + return nil, nil, err + } + } + slices.Sort(secrets) + secrets = slices.Compact(secrets) + for _, n := range secrets { + if _, ok := env[n]; ok { + return nil, nil, fmt.Errorf("%s: %s is both a plain env value and a secret; remove one (builder env unset or builder secret unset)", scope, n) + } + } + if len(env) == 0 { + env = nil + } + return env, secrets, nil +} + +// CheckEnv validates the env and secrets of the top level and of every +// profile, the way a build with each of them would. +func (c *Config) CheckEnv() error { + if _, _, err := c.resolveEnv(""); err != nil { + return err + } + for _, name := range c.ProfileNames() { + if _, _, err := c.resolveEnv(name); err != nil { + return err + } + } + return nil +} + +func (c *Config) checkProfile(profile string) error { + if profile == "" { + return nil + } + if _, ok := c.Profiles[profile]; !ok { + if len(c.Profiles) == 0 { + return fmt.Errorf("profile %q is not defined; builder.json has no profiles", profile) + } + return fmt.Errorf("profile %q is not defined; available profiles: %s", profile, strings.Join(c.ProfileNames(), ", ")) + } + return nil +} + +// EnvOf returns the env set at one level: the top level for an empty profile. +func (c *Config) EnvOf(profile string) map[string]string { + if profile == "" { + return c.Env + } + return c.Profiles[profile].Env +} + +// SecretsOf returns the secret names listed at one level. +func (c *Config) SecretsOf(profile string) []string { + if profile == "" { + return c.Secrets + } + return c.Profiles[profile].Secrets +} + +// SetEnv sets a plain env value at the top level or in a profile. The config +// is left unchanged when the result would not validate. +func (c *Config) SetEnv(profile, name, value string) error { + if err := c.checkProfile(profile); err != nil { + return err + } + if err := ValidateEnvName(name); err != nil { + return err + } + next := maps.Clone(c.EnvOf(profile)) + if next == nil { + next = map[string]string{} + } + next[name] = value + return c.update(profile, func(p *Profile) { p.Env = next }, func() { c.Env = next }) +} + +// UnsetEnv removes a plain env value; false when it was not set there. +func (c *Config) UnsetEnv(profile, name string) (bool, error) { + if err := c.checkProfile(profile); err != nil { + return false, err + } + cur := c.EnvOf(profile) + if _, ok := cur[name]; !ok { + return false, nil + } + next := maps.Clone(cur) + delete(next, name) + if len(next) == 0 { + next = nil + } + return true, c.update(profile, func(p *Profile) { p.Env = next }, func() { c.Env = next }) +} + +// AddSecret lists a secret name at the top level or in a profile, keeping +// the list sorted; adding a listed name changes nothing. +func (c *Config) AddSecret(profile, name string) error { + if err := c.checkProfile(profile); err != nil { + return err + } + if err := ValidateSecretName(name); err != nil { + return err + } + cur := c.SecretsOf(profile) + if slices.Contains(cur, name) { + return c.CheckEnv() + } + next := append(slices.Clone(cur), name) + slices.Sort(next) + return c.update(profile, func(p *Profile) { p.Secrets = next }, func() { c.Secrets = next }) +} + +// RemoveSecret drops a secret name from one level; false when it was not listed. +func (c *Config) RemoveSecret(profile, name string) (bool, error) { + if err := c.checkProfile(profile); err != nil { + return false, err + } + cur := c.SecretsOf(profile) + i := slices.Index(cur, name) + if i < 0 { + return false, nil + } + next := slices.Delete(slices.Clone(cur), i, i+1) + if len(next) == 0 { + next = nil + } + return true, c.update(profile, func(p *Profile) { p.Secrets = next }, func() { c.Secrets = next }) +} + +// update applies a change to a profile or the top level, then validates every +// level and rolls the change back when one fails. +func (c *Config) update(profile string, inProfile func(*Profile), topLevel func()) error { + if profile == "" { + env, secrets := c.Env, c.Secrets + topLevel() + if err := c.CheckEnv(); err != nil { + c.Env, c.Secrets = env, secrets + return err + } + return nil + } + old := c.Profiles[profile] + p := old + inProfile(&p) + c.Profiles[profile] = p + if err := c.CheckEnv(); err != nil { + c.Profiles[profile] = old + return err + } + return nil +} diff --git a/internal/config/env_test.go b/internal/config/env_test.go new file mode 100644 index 0000000..c986155 --- /dev/null +++ b/internal/config/env_test.go @@ -0,0 +1,193 @@ +package config + +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +func envConfig() *Config { + return &Config{ + Project: "App", GitHub: GitHubConfig{Owner: "o", Repo: "r"}, + Env: map[string]string{"API_URL": "https://api.example.com", "LOG": "info"}, + Secrets: []string{"SENTRY_TOKEN"}, + Profiles: map[string]Profile{ + "staging": {Distribution: "internal", Env: map[string]string{"API_URL": "https://staging.example.com"}, Secrets: []string{"MAPS_KEY", "SENTRY_TOKEN"}}, + "production": {Distribution: "store"}, + }, + } +} + +func TestResolveProfileMergesTopLevelEnvAndSecrets(t *testing.T) { + cfg := envConfig() + s, err := cfg.ResolveProfile("") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(s.Env, cfg.Env) || !reflect.DeepEqual(s.Secrets, []string{"SENTRY_TOKEN"}) { + t.Fatalf("no profile: env %v secrets %v", s.Env, s.Secrets) + } + // The profile's env wins per key; the rest of the top level stays. + s, err = cfg.ResolveProfile("staging") + if err != nil { + t.Fatal(err) + } + if s.Env["API_URL"] != "https://staging.example.com" || s.Env["LOG"] != "info" || len(s.Env) != 2 { + t.Fatalf("staging env: %v", s.Env) + } + if !reflect.DeepEqual(s.Secrets, []string{"MAPS_KEY", "SENTRY_TOKEN"}) { + t.Fatalf("staging secrets must be the sorted union: %v", s.Secrets) + } + // Resolving must not write into builder.json's maps. + if cfg.Env["API_URL"] != "https://api.example.com" { + t.Fatal("profile env leaked into the top level") + } + s, _ = cfg.ResolveProfile("production") + if s.Env["API_URL"] != "https://api.example.com" || !reflect.DeepEqual(s.Secrets, []string{"SENTRY_TOKEN"}) { + t.Fatalf("production inherits the top level: %v %v", s.Env, s.Secrets) + } + // Nothing configured: nothing to send. + s, _ = (&Config{}).ResolveProfile("") + if s.Env != nil || s.Secrets != nil || s.ProfileInput() != "" || s.EnvJSON() != "" { + t.Fatalf("empty config: %+v", s) + } +} + +func TestResolveProfileRejectsBadSecrets(t *testing.T) { + for name, mutate := range map[string]func(*Config){ + "lower case": func(c *Config) { c.Secrets = []string{"sentry_token"} }, + "double underscore": func(c *Config) { c.Secrets = []string{"A__B"} }, + "signing secret": func(c *Config) { c.Secrets = []string{"IOS_CERTIFICATE_STORE"} }, + "MobAI key": func(c *Config) { c.Secrets = []string{"MOBAI_API_KEY"} }, + "GitHub namespace": func(c *Config) { c.Secrets = []string{"GITHUB_TOKEN"} }, + "builder namespace": func(c *Config) { c.Secrets = []string{"BUILDER_SECRETS"} }, + "leading digit": func(c *Config) { c.Secrets = []string{"1KEY"} }, + "top-level env": func(c *Config) { c.Env = map[string]string{"SCHEME": "x"} }, + "secret shadows env": func(c *Config) { c.Env["SENTRY_TOKEN"] = "plain" }, + "profile secret is env": func(c *Config) { c.Env["MAPS_KEY"] = "plain" }, + } { + cfg := envConfig() + mutate(cfg) + _, err1 := cfg.ResolveProfile("") + _, err2 := cfg.ResolveProfile("staging") + if err1 == nil && err2 == nil { + t.Errorf("%s accepted", name) + } + } +} + +func TestSecretStorageName(t *testing.T) { + for _, tt := range []struct{ name, profile, want string }{ + {"SENTRY_TOKEN", "", "SENTRY_TOKEN"}, + {"SENTRY_TOKEN", "production", "SENTRY_TOKEN__PRODUCTION"}, + {"SENTRY_TOKEN", "debug-store", "SENTRY_TOKEN__DEBUG_STORE"}, + {"K", "Pre.view 2", "K__PRE_VIEW_2"}, + } { + if got := SecretStorageName(tt.name, tt.profile); got != tt.want { + t.Errorf("SecretStorageName(%q, %q) = %q, want %q", tt.name, tt.profile, got, tt.want) + } + } +} + +func TestEnvAndSecretEdits(t *testing.T) { + cfg := envConfig() + if err := cfg.SetEnv("", "FEATURE", "on"); err != nil || cfg.Env["FEATURE"] != "on" { + t.Fatalf("top-level set: %v %v", err, cfg.Env) + } + if err := cfg.SetEnv("production", "API_URL", "https://prod.example.com"); err != nil || cfg.Profiles["production"].Env["API_URL"] != "https://prod.example.com" { + t.Fatalf("profile set: %v %v", err, cfg.Profiles["production"]) + } + // Invalid edits leave the config as it was. + for name, err := range map[string]error{ + "reserved": cfg.SetEnv("", "PATH", "/tmp"), + "bad name": cfg.SetEnv("", "A-B", "x"), + "unknown profile": cfg.SetEnv("nightly", "A", "x"), + "env over secret": cfg.SetEnv("", "SENTRY_TOKEN", "x"), + "secret over env": cfg.AddSecret("", "LOG"), + "reserved secret": cfg.AddSecret("", "IOS_PROVISIONING_PROFILE_STORE"), + "profile conflict": cfg.SetEnv("staging", "MAPS_KEY", "x"), + } { + if err == nil { + t.Errorf("%s accepted", name) + } + } + if _, ok := cfg.Env["SENTRY_TOKEN"]; ok || cfg.Profiles["staging"].Env["MAPS_KEY"] != "" || len(cfg.Secrets) != 1 { + t.Fatalf("rejected edit stuck: %+v", cfg) + } + + if err := cfg.AddSecret("production", "STRIPE_KEY"); err != nil || !reflect.DeepEqual(cfg.Profiles["production"].Secrets, []string{"STRIPE_KEY"}) { + t.Fatalf("add secret: %v %v", err, cfg.Profiles["production"]) + } + if err := cfg.AddSecret("", "A_FIRST"); err != nil || !reflect.DeepEqual(cfg.Secrets, []string{"A_FIRST", "SENTRY_TOKEN"}) { + t.Fatalf("list must stay sorted: %v %v", err, cfg.Secrets) + } + if err := cfg.AddSecret("", "A_FIRST"); err != nil || len(cfg.Secrets) != 2 { + t.Fatalf("re-adding duplicated: %v %v", err, cfg.Secrets) + } + if ok, err := cfg.RemoveSecret("production", "STRIPE_KEY"); !ok || err != nil || cfg.Profiles["production"].Secrets != nil { + t.Fatalf("remove: %v %v %v", ok, err, cfg.Profiles["production"]) + } + if ok, _ := cfg.RemoveSecret("", "MISSING"); ok { + t.Fatal("removed a name that was not listed") + } + if ok, err := cfg.UnsetEnv("production", "API_URL"); !ok || err != nil || cfg.Profiles["production"].Env != nil { + t.Fatalf("unset: %v %v %v", ok, err, cfg.Profiles["production"]) + } + if ok, _ := cfg.UnsetEnv("", "NOPE"); ok { + t.Fatal("unset a name that was not set") + } +} + +func TestEnvAndSecretsRoundTrip(t *testing.T) { + t.Chdir(t.TempDir()) + mgr := NewManager() + cfg := envConfig() + if err := mgr.Save(cfg); err != nil { + t.Fatal(err) + } + got, err := mgr.Load() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got.Env, cfg.Env) || !reflect.DeepEqual(got.Secrets, cfg.Secrets) || !reflect.DeepEqual(got.Profiles["staging"], cfg.Profiles["staging"]) { + t.Fatalf("round trip lost data:\n got %+v\nwant %+v", got, cfg) + } + data, _ := os.ReadFile(filepath.Join(".", ConfigFileName)) + if !strings.Contains(string(data), `"secrets": [`) || !strings.Contains(string(data), `"SENTRY_TOKEN"`) { + t.Fatalf("names not written:\n%s", data) + } + // Empty lists and maps are omitted, so configs without them are unchanged. + out, _ := json.Marshal(&Config{Project: "App", Profiles: map[string]Profile{"p": {}}}) + if strings.Contains(string(out), "secrets") || strings.Contains(string(out), `"env"`) { + t.Fatalf("empty env/secrets written: %s", out) + } +} + +func TestProfileInputCarriesSecrets(t *testing.T) { + cfg := envConfig() + s, _ := cfg.ResolveProfile("staging") + var in struct { + Name string `json:"name"` + Env map[string]string `json:"env"` + Secrets []string `json:"secrets"` + } + if err := json.Unmarshal([]byte(s.ProfileInput()), &in); err != nil { + t.Fatal(err) + } + if in.Name != "staging" || in.Env["LOG"] != "info" || !reflect.DeepEqual(in.Secrets, []string{"MAPS_KEY", "SENTRY_TOKEN"}) { + t.Fatalf("profile input: %+v", in) + } + // No profile but a top-level env or secret still needs the input. + s, _ = cfg.ResolveProfile("") + if err := json.Unmarshal([]byte(s.ProfileInput()), &in); err != nil || in.Name != "" || len(in.Secrets) != 1 { + t.Fatalf("top-level only: %q %v", s.ProfileInput(), err) + } + // A profile without secrets sends no secrets key (older workflows ignore it anyway). + s = BuildSettings{Profile: "development"} + if strings.Contains(s.ProfileInput(), "secrets") { + t.Fatalf("empty secrets sent: %s", s.ProfileInput()) + } +} diff --git a/internal/config/profile.go b/internal/config/profile.go index 86c42ec..8c65c20 100644 --- a/internal/config/profile.go +++ b/internal/config/profile.go @@ -20,7 +20,11 @@ type BuildSettings struct { // profile, when ios.signing is set (the legacy path). Signing bool Provider string // profile provider, else the top-level provider; may be empty (GitHub) - Env map[string]string + // Env is the top-level env with the profile's on top, key by key. + Env map[string]string + // Secrets are the names of the provider secrets to expose, top-level and + // the profile's, sorted and unique. Values never pass through Builder. + Secrets []string // Distribution is the profile's distribution, canonical (internal is // ad-hoc); empty for unsigned builds and the legacy path. Distribution string @@ -87,6 +91,11 @@ func (c *Config) ResolveProfile(name string) (BuildSettings, error) { name, source = c.DefaultProfile, "defaultProfile" } if name == "" { + env, secrets, err := c.resolveEnv("") + if err != nil { + return s, err + } + s.Env, s.Secrets = env, secrets return s, nil } p, ok := c.Profiles[name] @@ -100,13 +109,9 @@ func (c *Config) ResolveProfile(name string) (BuildSettings, error) { if err != nil { return s, fmt.Errorf("profile %q: %w", name, err) } - for k := range p.Env { - if !envNameRe.MatchString(k) { - return s, fmt.Errorf("profile %q: env name %q is not a valid environment variable name", name, k) - } - if reservedEnvName(k) { - return s, fmt.Errorf("profile %q: env name %q is reserved for the runner", name, k) - } + env, secrets, err := c.resolveEnv(name) + if err != nil { + return s, err } s.Profile = name s.Distribution = distribution @@ -125,9 +130,7 @@ func (c *Config) ResolveProfile(name string) (BuildSettings, error) { if p.Provider != "" { s.Provider = p.Provider } - if len(p.Env) > 0 { - s.Env = p.Env - } + s.Env, s.Secrets = env, secrets return s, nil } @@ -142,11 +145,12 @@ func (s *BuildSettings) EnvJSON() string { return string(data) } -// ProfileInput encodes name, env and distribution as the single `profile` -// dispatch input, keeping the workflow under GitHub's limit of ten inputs. It -// is empty when no profile is selected, so older workflow files still work. +// ProfileInput encodes name, env, distribution and the secret names as the +// single `profile` dispatch input, keeping the workflow under GitHub's limit +// of ten inputs. It is empty when no profile is selected and builder.json has +// no top-level env or secrets, so older workflow files still work. func (s *BuildSettings) ProfileInput() string { - if s.Profile == "" { + if s.Profile == "" && len(s.Env) == 0 && len(s.Secrets) == 0 { return "" } env := s.Env @@ -157,6 +161,7 @@ func (s *BuildSettings) ProfileInput() string { Name string `json:"name"` Env map[string]string `json:"env"` Distribution string `json:"distribution"` - }{s.Profile, env, s.Distribution}) + Secrets []string `json:"secrets,omitempty"` + }{s.Profile, env, s.Distribution, s.Secrets}) return string(data) } diff --git a/internal/config/types.go b/internal/config/types.go index 2d38ec0..5f36589 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -25,6 +25,12 @@ type Config struct { // runs have no flags, so it is also the only way they can select a profile. DefaultProfile string `json:"defaultProfile,omitempty"` Profiles map[string]Profile `json:"profiles,omitempty"` + // Env is exported on the runner for every build; a profile's env + // overrides it key by key. + Env map[string]string `json:"env,omitempty"` + // Secrets names the provider-held secrets every build exposes as + // environment variables. Only names live here, never values. + Secrets []string `json:"secrets,omitempty"` } // SigningConfig is where the signing material lives on this machine. @@ -41,7 +47,10 @@ type Profile struct { Configuration string `json:"configuration,omitempty"` // overrides ios.configuration; derived from distribution when empty Scheme string `json:"scheme,omitempty"` // overrides ios.scheme Provider string `json:"provider,omitempty"` // overrides provider - Env map[string]string `json:"env,omitempty"` // exported on the runner before dependencies and the build + Env map[string]string `json:"env,omitempty"` // exported on the runner before dependencies and the build; overrides the top-level env per key + // Secrets names further provider-held secrets this profile's builds + // expose, on top of the top-level list (see SecretStorageName). + Secrets []string `json:"secrets,omitempty"` // Distribution is the only signing setting of a profile (development, // ad-hoc or internal, store, enterprise; empty is unsigned): it selects the // signing set and the type the provisioning profile in it must have. diff --git a/internal/github/repo.go b/internal/github/repo.go index b88e277..bf8dd17 100644 --- a/internal/github/repo.go +++ b/internal/github/repo.go @@ -78,3 +78,38 @@ func (c *Client) CreateOrUpdateSecret(ctx context.Context, owner, repo, name, en return nil } + +// SetSecret encrypts value with the repository's public key (a libsodium +// sealed box) and stores it as the Actions secret name. +func (c *Client) SetSecret(ctx context.Context, owner, repo, name, value string) error { + key, err := c.GetPublicKey(ctx, owner, repo) + if err != nil { + return err + } + encrypted, err := EncryptSecret(key.Key, value) + if err != nil { + return fmt.Errorf("failed to encrypt %s: %w", name, err) + } + if err := c.CreateOrUpdateSecret(ctx, owner, repo, name, encrypted, key.KeyID); err != nil { + return fmt.Errorf("failed to store %s: %w", name, err) + } + return nil +} + +// DeleteSecret removes an Actions secret; false when the repository had none +// by that name. +func (c *Client) DeleteSecret(ctx context.Context, owner, repo, name string) (bool, error) { + resp, err := c.request(ctx, "DELETE", fmt.Sprintf("/repos/%s/%s/actions/secrets/%s", owner, repo, name), nil) + if err != nil { + return false, err + } + defer resp.Body.Close() + switch resp.StatusCode { + case http.StatusNoContent, http.StatusOK: + return true, nil + case http.StatusNotFound: + return false, nil + default: + return false, fmt.Errorf("failed to delete secret %s: status %d", name, resp.StatusCode) + } +} diff --git a/internal/github/secrets_test.go b/internal/github/secrets_test.go new file mode 100644 index 0000000..0ebb82a --- /dev/null +++ b/internal/github/secrets_test.go @@ -0,0 +1,72 @@ +package github + +import ( + "context" + "crypto/rand" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "golang.org/x/crypto/nacl/box" +) + +func TestSetSecretSealsTheValue(t *testing.T) { + pub, priv, err := box.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + var stored CreateSecretRequest + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method + " " + r.URL.Path { + case "GET /repos/o/r/actions/secrets/public-key": + fmt.Fprintf(w, `{"key_id":"kid","key":%q}`, base64.StdEncoding.EncodeToString(pub[:])) + case "PUT /repos/o/r/actions/secrets/SENTRY_TOKEN": + _ = json.NewDecoder(r.Body).Decode(&stored) + w.WriteHeader(http.StatusCreated) + default: + t.Errorf("unexpected %s %s", r.Method, r.URL) + } + })) + defer srv.Close() + if err := NewClientWithBaseURL("tok", srv.URL).SetSecret(context.Background(), "o", "r", "SENTRY_TOKEN", "s3cret"); err != nil { + t.Fatal(err) + } + sealed, err := base64.StdEncoding.DecodeString(stored.EncryptedValue) + if err != nil { + t.Fatal(err) + } + plain, ok := box.OpenAnonymous(nil, sealed, pub, priv) + if !ok || string(plain) != "s3cret" || stored.KeyID != "kid" { + t.Fatalf("stored %+v, opened %q %v", stored, plain, ok) + } +} + +func TestDeleteSecret(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + t.Errorf("unexpected %s", r.Method) + } + switch r.URL.Path { + case "/repos/o/r/actions/secrets/THERE": + w.WriteHeader(http.StatusNoContent) + case "/repos/o/r/actions/secrets/GONE": + w.WriteHeader(http.StatusNotFound) + default: + w.WriteHeader(http.StatusForbidden) + } + })) + defer srv.Close() + c := NewClientWithBaseURL("tok", srv.URL) + if ok, err := c.DeleteSecret(context.Background(), "o", "r", "THERE"); !ok || err != nil { + t.Fatalf("THERE: %v %v", ok, err) + } + if ok, err := c.DeleteSecret(context.Background(), "o", "r", "GONE"); ok || err != nil { + t.Fatalf("GONE: %v %v", ok, err) + } + if _, err := c.DeleteSecret(context.Background(), "o", "r", "DENIED"); err == nil { + t.Fatal("403 accepted") + } +} diff --git a/internal/workflow/secrets_test.go b/internal/workflow/secrets_test.go new file mode 100644 index 0000000..c29b1d4 --- /dev/null +++ b/internal/workflow/secrets_test.go @@ -0,0 +1,232 @@ +package workflow + +import ( + "encoding/json" + "os" + "os/exec" + "runtime" + "strings" + "testing" +) + +// fakeSecrets stands in for ${{ toJSON(secrets) }}: every secret of the +// repository, listed in builder.json or not. +func fakeSecrets(t *testing.T, secrets map[string]string) string { + t.Helper() + data, err := json.Marshal(secrets) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +const secretsBuilderJSON = `{ + "project": "App", "github": {"owner": "o", "repo": "r"}, + "env": {"API_URL": "https://api.example.com", "LOG": "info"}, + "secrets": ["SENTRY_TOKEN"], + "defaultProfile": "staging", + "profiles": { + "staging": {"distribution": "internal", "env": {"API_URL": "https://staging.example.com"}, "secrets": ["MAPS_KEY"]}, + "plain": {} + } +}` + +func requireShellTools(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("shell test") + } + for _, tool := range []string{"bash", "jq", "base64", "tr"} { + if _, err := exec.LookPath(tool); err != nil { + t.Skipf("%s unavailable", tool) + } + } +} + +func TestResolveParametersExportListedSecrets(t *testing.T) { + requireShellTools(t) + build := resolveStep(t, "ios-build.yml") + all := map[string]string{ + "SENTRY_TOKEN": "sentry-top", + "MAPS_KEY": "maps-top", + "MAPS_KEY__STAGING": "maps-staging\nsecond line", + "UNLISTED": "must-not-leak", + "IOS_CERTIFICATE_STORE": "certificate-must-not-leak", + "github_token": "ghs_must-not-leak", + } + + t.Run("tag build exports exactly the listed secrets", func(t *testing.T) { + r := runResolve(t, build, secretsBuilderJSON, map[string]string{"GITHUB_EVENT_NAME": "push", "BUILDER_SECRETS_JSON": fakeSecrets(t, all)}) + if r.err != nil { + t.Fatalf("%v\n%s", r.err, r.log) + } + want := map[string]string{ + "API_URL": "https://staging.example.com", // the profile wins per key + "LOG": "info", // the top level fills the rest + "SENTRY_TOKEN": "sentry-top", // top-level secret, no profile value + "MAPS_KEY": "maps-staging\nsecond line", // the profile's value wins + } + if len(r.env) != len(want) { + t.Fatalf("exported %v, want exactly %v\n%s", keys(r.env), keys(want), r.log) + } + for k, v := range want { + if r.env[k] != v { + t.Errorf("%s = %q, want %q", k, r.env[k], v) + } + } + // Every exported secret line is masked; nothing else is. + for _, line := range []string{"::add-mask::sentry-top", "::add-mask::maps-staging", "::add-mask::second line"} { + if !strings.Contains(r.log, line+"\n") { + t.Errorf("missing %q in log:\n%s", line, r.log) + } + } + if strings.Count(r.log, "::add-mask::") != 3 { + t.Errorf("unexpected masks:\n%s", r.log) + } + for _, leak := range []string{"must-not-leak", "maps-top"} { + if strings.Contains(r.log, leak) && !strings.Contains(r.log, "::add-mask::"+leak) { + t.Errorf("%q printed:\n%s", leak, r.log) + } + } + if !strings.Contains(r.log, "secret: MAPS_KEY (from MAPS_KEY__STAGING)") || !strings.Contains(r.log, "secret: SENTRY_TOKEN (from SENTRY_TOKEN)") { + t.Errorf("sources not logged:\n%s", r.log) + } + }) + + t.Run("dispatch exports the names in the profile input", func(t *testing.T) { + env := map[string]string{"GITHUB_EVENT_NAME": "workflow_dispatch", "IN_BUILD_ID": "12345678", + "IN_PROFILE": `{"name":"","env":{},"distribution":"","secrets":["MAPS_KEY"]}`, + "BUILDER_SECRETS_JSON": fakeSecrets(t, all)} + r := runResolve(t, build, secretsBuilderJSON, env) + if r.err != nil { + t.Fatalf("%v\n%s", r.err, r.log) + } + // No profile: the base name only, never a profile's value. + if len(r.env) != 1 || r.env["MAPS_KEY"] != "maps-top" || !strings.Contains(r.log, "::add-mask::maps-top") { + t.Fatalf("env %v\n%s", r.env, r.log) + } + // A profile name with other characters maps onto the suffix. + env["IN_PROFILE"] = `{"name":"pre-view","env":{},"secrets":["MAPS_KEY"]}` + r = runResolve(t, build, "", map[string]string{"GITHUB_EVENT_NAME": "workflow_dispatch", "IN_PROFILE": env["IN_PROFILE"], + "BUILDER_SECRETS_JSON": fakeSecrets(t, map[string]string{"MAPS_KEY": "a", "MAPS_KEY__PRE_VIEW": "b"})}) + if r.err != nil || r.env["MAPS_KEY"] != "b" { + t.Fatalf("suffix lookup: %v %v\n%s", r.err, r.env, r.log) + } + }) + + t.Run("no secrets listed touches nothing", func(t *testing.T) { + r := runResolve(t, build, `{"defaultProfile": "plain", "profiles": {"plain": {}}}`, map[string]string{"GITHUB_EVENT_NAME": "push", "BUILDER_SECRETS_JSON": fakeSecrets(t, all)}) + if r.err != nil || len(r.env) != 0 || strings.Contains(r.log, "add-mask") { + t.Fatalf("%v %v\n%s", r.err, r.env, r.log) + } + }) + + t.Run("bad or missing secrets fail the job", func(t *testing.T) { + for name, tt := range map[string]struct { + profile string + secrets map[string]string + want string + }{ + "missing": {`{"name":"staging","secrets":["NOPE"]}`, all, "builder secret set NOPE --profile staging"}, + "lower case": {`{"name":"","secrets":["nope"]}`, map[string]string{"nope": "x"}, "upper case"}, + "double __": {`{"name":"","secrets":["A__B"]}`, map[string]string{"A__B": "x"}, "single underscores"}, + "env and secret": {`{"name":"","env":{"A":"1"},"secrets":["A"]}`, map[string]string{"A": "x"}, "both a plain env value and a secret"}, + "not an array": {`{"name":"","secrets":"A"}`, map[string]string{"A": "x"}, "JSON array"}, + "no secrets JSON": {`{"name":"","secrets":["A"]}`, nil, "builder secret set A"}, + } { + env := map[string]string{"GITHUB_EVENT_NAME": "workflow_dispatch", "IN_PROFILE": tt.profile} + if tt.secrets != nil { + env["BUILDER_SECRETS_JSON"] = fakeSecrets(t, tt.secrets) + } + r := runResolve(t, build, "", env) + if r.err == nil || !strings.Contains(r.log, tt.want) { + t.Errorf("%s: err %v, want %q in:\n%s", name, r.err, tt.want, r.log) + } + if _, ok := r.env["A"]; ok && name != "env and secret" { + t.Errorf("%s: exported anyway", name) + } + } + }) +} + +// The step must see the secrets through the toJSON(secrets) expression; that +// is the only way to read secrets whose names are not in the workflow file. +func TestResolveStepReceivesAllSecrets(t *testing.T) { + data, err := GetTemplate("ios-build.yml") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), "BUILDER_SECRETS_JSON: ${{ toJSON(secrets) }}") { + t.Fatal("Resolve parameters does not receive toJSON(secrets)") + } + // Only that one step: every other step would hold every secret too. + if n := strings.Count(string(data), "${{ toJSON(secrets) }}"); n != 1 { + t.Fatalf("toJSON(secrets) appears %d times", n) + } +} + +// runner.sh: Codemagic and Bitrise already put the app's secure variables in +// the environment, so the function only picks the profile's value, checks +// presence, and refuses a name that is also plain env. +func TestRunnerExportsListedSecrets(t *testing.T) { + requireShellTools(t) + runner, err := GetTemplate("runner.sh") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(runner), " export_build_env\n export_build_secrets\n") { + t.Fatal("prepare() must export the secrets right after the env") + } + fn := shellFunc(t, string(runner), "export_build_secrets") + run := func(env map[string]string) (string, error) { + script := "set -euo pipefail\nfail() { echo \"$*\" >&2; exit 1; }\nBUILD_ENV=\"${BUILD_ENV:-}\"\n" + fn + + "\nexport_build_secrets\nfor n in SENTRY_TOKEN MAPS_KEY; do printf '%s=[%s]\\n' \"$n\" \"${!n:-}\"; done\n" + cmd := exec.Command("bash", "-c", script) + cmd.Env = []string{"PATH=" + os.Getenv("PATH")} + for k, v := range env { + cmd.Env = append(cmd.Env, k+"="+v) + } + out, err := cmd.CombinedOutput() + return string(out), err + } + + out, err := run(map[string]string{"BUILDER_SECRETS": "MAPS_KEY SENTRY_TOKEN", "BUILDER_SECRET_SUFFIX": "STAGING", + "SENTRY_TOKEN": "top", "MAPS_KEY": "maps-top", "MAPS_KEY__STAGING": "maps\nstaging"}) + if err != nil || !strings.Contains(out, "SENTRY_TOKEN=[top]") || !strings.Contains(out, "MAPS_KEY=[maps\nstaging]") || + !strings.Contains(out, "secret: MAPS_KEY (from MAPS_KEY__STAGING)") { + t.Fatalf("%v\n%s", err, out) + } + if strings.Contains(out, "maps-top") { + t.Fatalf("the profile value must win:\n%s", out) + } + + // Nothing listed: nothing checked, nothing exported. + if out, err := run(map[string]string{"SENTRY_TOKEN": "x"}); err != nil || strings.Contains(out, "secret:") { + t.Fatalf("%v\n%s", err, out) + } + + for name, env := range map[string]map[string]string{ + "missing": {"BUILDER_SECRETS": "SENTRY_TOKEN MAPS_KEY", "SENTRY_TOKEN": "x"}, + "empty": {"BUILDER_SECRETS": "SENTRY_TOKEN", "SENTRY_TOKEN": ""}, + "lower case": {"BUILDER_SECRETS": "sentry", "sentry": "x"}, + "injection": {"BUILDER_SECRETS": "A$(touch${IFS}x)", "A": "x"}, + "bad suffix": {"BUILDER_SECRETS": "SENTRY_TOKEN", "SENTRY_TOKEN": "x", "BUILDER_SECRET_SUFFIX": "a b"}, + "env and secret": {"BUILDER_SECRETS": "SENTRY_TOKEN", "SENTRY_TOKEN": "x", "BUILD_ENV": `{"SENTRY_TOKEN":"plain"}`}, + } { + out, err := run(env) + if err == nil { + t.Errorf("%s accepted:\n%s", name, out) + } + if name == "missing" && !strings.Contains(out, "builder secret set") { + t.Errorf("missing secret not explained:\n%s", out) + } + } +} + +func keys(m map[string]string) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} diff --git a/internal/workflow/templates/ios-build.yml b/internal/workflow/templates/ios-build.yml index d78a18e..d5b3f6d 100644 --- a/internal/workflow/templates/ios-build.yml +++ b/internal/workflow/templates/ios-build.yml @@ -53,7 +53,7 @@ on: # One input for the profile fields that are not inputs of their own, so # the workflow stays under the ten-input limit of workflow_dispatch. profile: - description: 'Selected builder.json profile as JSON: {"name": "...", "env": {...}, "distribution": "..."}' + description: 'Selected builder.json profile as JSON: {"name": "...", "env": {...}, "distribution": "...", "secrets": ["NAME", ...]}' required: false type: string default: '{}' @@ -102,6 +102,9 @@ jobs: IN_JDK_VERSION: ${{ inputs.jdk_version }} IN_PROFILE: ${{ inputs.profile }} IN_BUILD_NUMBER: ${{ inputs.build_number }} + # Every secret of the repository, for this step only: it exports + # just the names builder.json lists (see below). + BUILDER_SECRETS_JSON: ${{ toJSON(secrets) }} run: | set -e PROFILE="" @@ -142,7 +145,9 @@ jobs: if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then PROFILE_JSON="$IN_PROFILE" elif [ -f builder.json ]; then - PROFILE_JSON=$(jq -c --arg p "$PROFILE" '{name: $p, env: (.profiles[$p].env // {}), distribution: (.profiles[$p].distribution // "")}' builder.json) + # The top-level env and secrets apply to every build; the profile's + # env wins per key, and its secrets add to the list. + PROFILE_JSON=$(jq -c --arg p "$PROFILE" '{name: $p, env: ((.env // {}) + (.profiles[$p].env // {})), distribution: (.profiles[$p].distribution // ""), secrets: (((.secrets // []) + (.profiles[$p].secrets // [])) | unique)}' builder.json) fi [ -n "${PROFILE_JSON:-}" ] || PROFILE_JSON='{}' PROFILE=$(jq -r '.name // ""' <<< "$PROFILE_JSON") @@ -191,6 +196,56 @@ jobs: echo "env: $name" done < <(jq -r '.env // {} | to_entries[] | "\(.key | @base64) \(.value | tostring | @base64)"' <<< "$PROFILE_JSON") + # Secrets: builder.json lists only names (`builder secret set` adds + # them); the values are this repository's Actions secrets, which a + # workflow can only reach by name or all at once through + # toJSON(secrets). Only the listed names are exported, each value + # masked line by line before it is written. A profile's build takes + # NAME__ when that exists (the profile name upper-cased, + # other characters as _), else NAME, and exports it as NAME. + if [ "$(jq -r '.secrets // [] | type' <<< "$PROFILE_JSON")" != "array" ]; then + echo "::error::the profile's secrets must be a JSON array of secret names"; exit 1 + fi + SECRET_SUFFIX=$(printf '%s' "$PROFILE" | LC_ALL=C tr '[:lower:]' '[:upper:]' | LC_ALL=C tr -c 'A-Z0-9' '_') + SECRETS_JSON="${BUILDER_SECRETS_JSON:-}" + [ -n "$SECRETS_JSON" ] || SECRETS_JSON='{}' + MISSING_SECRETS="" + while IFS= read -r key; do + name=$(printf '%s' "$key" | base64 --decode) + if ! [[ "$name" =~ ^[A-Z_][A-Z0-9_]*$ ]] || [[ "$name" == *__* ]]; then + echo "::error::secret name \"$name\" must be upper case letters, digits and single underscores"; exit 1 + fi + if [ "$(jq -r --arg n "$name" '.env // {} | has($n)' <<< "$PROFILE_JSON")" = "true" ]; then + echo "::error::$name is both a plain env value and a secret in builder.json; remove one"; exit 1 + fi + found=$(jq -r --arg a "${name}__${SECRET_SUFFIX}" --arg b "$name" --arg s "$SECRET_SUFFIX" \ + 'if $s != "" and has($a) then "\($a) \(.[$a] | @base64)" elif has($b) then "\($b) \(.[$b] | @base64)" else "" end' \ + <<< "$SECRETS_JSON") + if [ -z "$found" ]; then + MISSING_SECRETS="$MISSING_SECRETS $name" + continue + fi + source_name="${found%% *}" + encoded="${found#* }" + while IFS= read -r line; do + if [ -n "$line" ]; then echo "::add-mask::$line"; fi + done < <(printf '%s' "$encoded" | base64 --decode; echo) + delim="BUILDER_SECRET_${RANDOM}${RANDOM}${RANDOM}" + { + echo "$name<<$delim" + printf '%s' "$encoded" | base64 --decode + echo + echo "$delim" + } >> "$GITHUB_ENV" + echo "secret: $name (from $source_name)" + done < <(jq -r '.secrets // [] | .[] | tostring | @base64' <<< "$PROFILE_JSON") + if [ -n "$MISSING_SECRETS" ]; then + for name in $MISSING_SECRETS; do + echo "::error::secret $name is listed in builder.json but not set in this repository's Actions secrets${SECRET_SUFFIX:+ (as $name or ${name}__$SECRET_SUFFIX)}; run: builder secret set $name${PROFILE:+ --profile $PROFILE}" + done + exit 1 + fi + - name: Setup Xcode uses: maxim-lobanov/setup-xcode@v1 with: diff --git a/internal/workflow/templates/runner.sh b/internal/workflow/templates/runner.sh index caa1b5c..bf638b2 100644 --- a/internal/workflow/templates/runner.sh +++ b/internal/workflow/templates/runner.sh @@ -30,6 +30,29 @@ export_build_env() { done < <(jq -r 'to_entries[] | "\(.key | @base64) \(.value | tostring | @base64)"' <<< "$BUILD_ENV") } +# The secrets builder.json lists (BUILDER_SECRETS, names only) are the app's +# own secure variables, already in the environment. A profile's build takes +# NAME__ when the provider has it and exports it as +# NAME; a listed name with no value fails here, by name, before the build. +export_build_secrets() { + [ -n "${BUILDER_SECRETS:-}" ] || return 0 + local suffix="${BUILDER_SECRET_SUFFIX:-}" name stored alt missing="" + if [ -n "$suffix" ] && ! [[ "$suffix" =~ ^[A-Z0-9_]+$ ]]; then fail "Invalid BUILDER_SECRET_SUFFIX: $suffix"; fi + for name in $BUILDER_SECRETS; do + if ! [[ "$name" =~ ^[A-Z_][A-Z0-9_]*$ ]] || [[ "$name" == *__* ]]; then fail "Invalid secret name in BUILDER_SECRETS: $name"; fi + if [ -n "$BUILD_ENV" ] && [ "$(jq -r --arg n "$name" 'has($n)' <<< "$BUILD_ENV")" = true ]; then + fail "$name is both a plain env value and a secret in builder.json; remove one" + fi + stored="$name" + alt="${name}__${suffix}" + if [ -n "$suffix" ] && [ -n "${!alt:-}" ]; then stored="$alt"; fi + if [ -z "${!stored:-}" ]; then missing="$missing $name"; continue; fi + if [ "$stored" != "$name" ]; then export "$name=${!stored}"; fi + echo "secret: $name (from $stored)" + done + [ -z "$missing" ] || fail "Secrets listed in builder.json but not set on this app:$missing. Run builder secret set ${suffix:+ --profile } --provider for each." +} + snapshot_checkout() { case "${SNAPSHOT_REF:-}" in refs/ios-builder/jobs/*) ;; @@ -211,6 +234,7 @@ prepare() { echo "Project type: $project_type" if ! command -v jq >/dev/null; then brew install jq; fi export_build_env + export_build_secrets # Match the GitHub workflows' committed xcconfig-template convention. find . -path ./DerivedData -prune -o -type f \